Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pkg/generate/ack/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ var (
"GoCodeRequiredFieldsMissingFromSetAttributesInput": func(r *ackmodel.CRD, koVarName string, indentLevel int) string {
return code.CheckRequiredFieldsMissingFromShape(r, ackmodel.OpTypeSetAttributes, koVarName, indentLevel)
},
"GoCodeSetResourceIdentifiers": func(r *ackmodel.CRD, sourceVarName string, targetVarName string, indentLevel int) string {
return code.SetResourceIdentifiers(r.Config(), r, sourceVarName, targetVarName, indentLevel)
},
}
)

Expand Down
200 changes: 199 additions & 1 deletion pkg/generate/code/set_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,6 @@ func SetResource(
return out
}


func ListMemberNameInReadManyOutput(
r *model.CRD,
) string {
Expand Down Expand Up @@ -770,6 +769,205 @@ func SetResourceGetAttributes(
return out
}

// SetResourceIdentifiers returns the Go code that sets an empty CR object with
// Spec and Status field values that correspond to the primary identifier (be
// that an ARN, ID or Name) and any other "additional keys" required for the AWS
// service to uniquely identify the object.
//
// The method will attempt to use the `ReadOne` operation, if present, otherwise
// will fall back to using `ReadMany`. If it detects the operation uses an ARN
// to identify the resource it will read it from the metadata status field.
// Otherwise it will use any fields with a matching name in the operation,
// pulling from spec or status - requiring that exactly one of those fields is
// marked as the "primary" identifier.
//
// An example of code with no additional keys:
//
// ```
// if identifier.NameOrID == nil {
// return ackerrors.MissingNameIdentifier
// }
// r.ko.Status.BrokerID = identifier.NameOrID
// ```
//
// An example of code with additional keys:
//
// ```
// if identifier.NameOrID == nil {
// return ackerrors.MissingNameIdentifier
// }
// r.ko.Spec.ResourceID = identifier.NameOrID
//
// f0, f0ok := identifier.AdditionalKeys["scalableDimension"]
// if f0ok {
// r.ko.Spec.ScalableDimension = f0
// }
// f1, f1ok := identifier.AdditionalKeys["serviceNamespace"]
// if f1ok {
// r.ko.Spec.ServiceNamespace = f1
// }
// ```
func SetResourceIdentifiers(
cfg *ackgenconfig.Config,
r *model.CRD,
// String representing the name of the variable that we will grab the Input
// shape from. This will likely be "identifier" since in the templates that
// call this method, the "source variable" is the CRD struct which is used
// to populate the target variable, which is the struct of unique
// identifiers
sourceVarName string,
// String representing the name of the variable that we will be **setting**
// with values we get from the Output shape. This will likely be
// "r.ko" since that is the name of the "target variable" that the
// templates that call this method use for the Input shape.
targetVarName string,
// Number of levels of indentation to use
indentLevel int,
) string {
op := r.Ops.ReadOne
if op == nil {
if r.Ops.GetAttributes != nil {
// TODO(RedbackThomson): Support attribute maps for resource identifiers
return ""
}
// If single lookups can only be done using ReadMany
op = r.Ops.ReadMany
}
inputShape := op.InputRef.Shape
if inputShape == nil {
return ""
}

primaryKeyOut := "\n"
arnOut := ""
additionalKeyOut := "\n"

indent := strings.Repeat("\t", indentLevel)

primaryKeyOut += fmt.Sprintf("%sif %s.NameOrID == nil {\n", indent, sourceVarName)
primaryKeyOut += fmt.Sprintf("%s\treturn ackerrors.MissingNameIdentifier\n", indent)
primaryKeyOut += fmt.Sprintf("%s}\n", indent)

primaryIdentifier := ""

// Attempt to fetch the primary identifier from the configuration
opConfig, ok := cfg.Operations[op.Name]
if ok {
primaryIdentifier = opConfig.PrimaryIdentifierFieldName
}

// Determine the "primary identifier" based on the names of each field
if primaryIdentifier == "" {
primaryIdentifierLookup := []string{
"Name",
r.Names.Original + "Name",
r.Names.Original + "Id",
}

for _, memberName := range inputShape.MemberNames() {
if util.InStrings(memberName, primaryIdentifierLookup) {
if primaryIdentifier == "" {
primaryIdentifier = memberName
} else {
panic("Found multiple possible primary identifiers for " +
r.Names.Original + ". Set " +
"`primary_identifier_field_name` for the " + op.Name +
" operation in the generator config.")
}
}
}

// Still haven't determined the identifier? Panic
if primaryIdentifier == "" {
panic("Could not find primary identifier for " + r.Names.Original +
". Set `primary_identifier_field_name` for the " + op.Name +
" operation in the generator config.")
}
}

paginatorFieldLookup := []string{
"NextToken",
"MaxResults",
}

additionalKeyCount := 0
for _, memberName := range inputShape.MemberNames() {
if util.InStrings(memberName, paginatorFieldLookup) {
continue
}

memberShapeRef, _ := inputShape.MemberRefs[memberName]
memberShape := memberShapeRef.Shape

// Only strings are currently accepted as valid inputs for
// additional key fields
if memberShape.Type != "string" {
continue
}

if r.IsSecretField(memberName) {
// Secrets cannot be used as fields in identifiers
continue
}

if r.IsPrimaryARNField(memberName) {
// r.ko.Status.ACKResourceMetadata.ARN = identifier.ARN
arnOut += fmt.Sprintf(
"\n%s%s.Status.ACKResourceMetadata.ARN = %s.ARN\n",
indent, targetVarName, sourceVarName,
)
continue

}

isPrimaryIdentifier := memberName == primaryIdentifier
cleanMemberNames := names.New(memberName)
cleanMemberName := cleanMemberNames.Camel

memberPath := ""
_, inSpec := r.SpecFields[memberName]
_, inStatus := r.StatusFields[memberName]
switch {
case inSpec:
memberPath = cfg.PrefixConfig.SpecField
case inStatus:
memberPath = cfg.PrefixConfig.StatusField
case isPrimaryIdentifier:
panic("Primary identifier field '" + memberName + "' cannot be found in either spec or status.")
default:
continue
}

if isPrimaryIdentifier {
// r.ko.Status.BrokerID = identifier.NameOrID
primaryKeyOut += fmt.Sprintf("%s%s%s.%s = %s.NameOrID\n", indent, targetVarName, memberPath, cleanMemberName, sourceVarName)
} else {
// f0, f0ok := identifier.AdditionalKeys["scalableDimension"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we plan to support case-insensitive lookups? Because if the Spec or Status field is PascalCased, per Go export rules, and we're asking users to use lowerCamelCased, things might get confusing here.

I think we should probably support case-insensitive key lookups here.

Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All other fields in the k8s spec are lowerCamelCased, I am trying to make the additionalKeys map feel more "k8s native".

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RedbackThomson yeah, I guess that's a fair point. I just worry that without the TODO below on line 953 being implemented, that simple typos here will be quite difficult to detect.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah totally understandable. A typo in an additionalKeys key would not be detectable and would not produce any error statements at the moment. It would be interesting to pop values out of the additionalKeys map as they are consumed and return error conditions for any remaining values.

// if f0ok {
// r.ko.Spec.ScalableDimension = f0
// }

fieldIndexName := fmt.Sprintf("f%d", additionalKeyCount)
sourceAdaptedVarName := fmt.Sprintf("%s.AdditionalKeys[\"%s\"]", sourceVarName, cleanMemberNames.CamelLower)

// TODO(RedbackThomson): If the identifiers don't exist, we should be
// throwing an error accessible to the user
additionalKeyOut += fmt.Sprintf("%s%s, %sok := %s\n", indent, fieldIndexName, fieldIndexName, sourceAdaptedVarName)
additionalKeyOut += fmt.Sprintf("%sif %sok {\n", indent, fieldIndexName)
additionalKeyOut += fmt.Sprintf("%s\t%s%s.%s = %s\n", indent, targetVarName, memberPath, cleanMemberName, fieldIndexName)
additionalKeyOut += fmt.Sprintf("%s}\n", indent)

additionalKeyCount++
}
}

// Only use at most one of ARN or nameOrID as primary identifier outputs
if arnOut != "" {
return arnOut + additionalKeyOut
}
return primaryKeyOut + additionalKeyOut
}

// setResourceForContainer returns a string of Go code that sets the value of a
// target variable to that of a source variable. When the source variable type
// is a map, struct or slice type, then this function is called recursively on
Expand Down
70 changes: 70 additions & 0 deletions pkg/generate/code/set_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2645,3 +2645,73 @@ func TestGetWrapperOutputShape(t *testing.T) {
})
}
}

func TestSetResource_MQ_Broker_SetResourceIdentifiers(t *testing.T) {
assert := assert.New(t)
require := require.New(t)

g := testutil.NewGeneratorForService(t, "mq")

crd := testutil.GetCRDByName(t, g, "Broker")
require.NotNil(crd)

expected := `
if identifier.NameOrID == nil {
return ackerrors.MissingNameIdentifier
}
r.ko.Status.BrokerID = identifier.NameOrID

`
assert.Equal(
expected,
code.SetResourceIdentifiers(crd.Config(), crd, "identifier", "r.ko", 1),
)
}

func TestSetResource_RDS_DBInstances_SetResourceIdentifiers(t *testing.T) {
assert := assert.New(t)
require := require.New(t)

g := testutil.NewGeneratorForService(t, "rds")

crd := testutil.GetCRDByName(t, g, "DBInstance")
require.NotNil(crd)

expected := `
if identifier.NameOrID == nil {
return ackerrors.MissingNameIdentifier
}
r.ko.Spec.DBInstanceIdentifier = identifier.NameOrID

`
assert.Equal(
expected,
code.SetResourceIdentifiers(crd.Config(), crd, "identifier", "r.ko", 1),
)
}

func TestSetResource_APIGWV2_ApiMapping_SetResourceIdentifiers(t *testing.T) {
assert := assert.New(t)
require := require.New(t)

g := testutil.NewGeneratorForService(t, "apigatewayv2")

crd := testutil.GetCRDByName(t, g, "ApiMapping")
require.NotNil(crd)

expected := `
if identifier.NameOrID == nil {
return ackerrors.MissingNameIdentifier
}
r.ko.Status.APIMappingID = identifier.NameOrID

f0, f0ok := identifier.AdditionalKeys["domainName"]
if f0ok {
r.ko.Spec.DomainName = f0
}
`
assert.Equal(
expected,
code.SetResourceIdentifiers(crd.Config(), crd, "identifier", "r.ko", 1),
)
}
4 changes: 4 additions & 0 deletions pkg/generate/config/operation.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ type OperationConfig struct {
// Override for operation type in case of heuristic failure
// An example of this is `Put...` or `Register...` API operations not being correctly classified as `Create` op type
OperationType string `json:"operation_type"`
// PrimaryIdentifierFieldName provides the name of the field that should be
// interpreted as the "primary" identifier field. This field will be used as
// the primary field for resource adoption.
PrimaryIdentifierFieldName string `json:"primary_identifier_field_name,omitempty"`
}

// IsIgnoredOperation returns true if Operation Name is configured to be ignored
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
ignore:
shape_names:
- DBSecurityGroupMembershipList
operations:
DescribeDBInstances:
primary_identifier_field_name: DBInstanceIdentifier
resources:
DBSubnetGroup:
renames:
Expand Down
5 changes: 4 additions & 1 deletion pkg/model/crd.go
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,10 @@ func (r *CRD) UpdateConditionsCustomMethodName() string {
return resGenConfig.UpdateConditionsCustomMethodName
}

// SpecIdentifierField returns the name of the "Name" or string identifier field in the Spec
// SpecIdentifierField returns the name of the "Name" or string identifier field
// in the Spec.
// This method does not guarantee that the identifier field returned is the
// primary identifier used in any of the `Read*` operations.
func (r *CRD) SpecIdentifierField() *string {
if r.cfg != nil {
rConfig, found := r.cfg.Resources[r.Names.Original]
Expand Down
9 changes: 1 addition & 8 deletions templates/pkg/resource/resource.go.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,6 @@ func (r *resource) SetObjectMeta(meta metav1.ObjectMeta) {
// SetIdentifiers sets the Spec or Status field that is referenced as the unique
// resource identifier
func (r *resource) SetIdentifiers(identifier *ackv1alpha1.AWSIdentifiers) error {
{{- if $idField := .CRD.SpecIdentifierField }}
if identifier.NameOrID == nil {
return ackerrors.MissingNameIdentifier
}
r.ko.Spec.{{ $idField }} = identifier.NameOrID
{{- else }}
r.ko.Status.ACKResourceMetadata.ARN = identifier.ARN
{{- end }}
{{- GoCodeSetResourceIdentifiers .CRD "identifier" "r.ko" 1}}
return nil
}