diff --git a/pkg/generate/code/common.go b/pkg/generate/code/common.go index 2aef4f59..6d23538d 100644 --- a/pkg/generate/code/common.go +++ b/pkg/generate/code/common.go @@ -131,12 +131,6 @@ func FindPrimaryIdentifierFieldNames( ) (crField string, shapeField string) { shape := op.InputRef.Shape - // Attempt to fetch the primary identifier override from the configuration - opConfig, ok := cfg.Operations[op.Name] - if ok { - shapeField = opConfig.PrimaryIdentifierFieldName - } - if shapeField == "" { // For ReadOne, search for a direct identifier if op == r.Ops.ReadOne { @@ -151,8 +145,8 @@ func FindPrimaryIdentifierFieldNames( default: panic("Found multiple possible primary identifiers for " + r.Names.Original + ". Set " + - "`primary_identifier_field_name` for the " + op.Name + - " operation in the generator config.") + "`is_primary_key` for the primary field in the " + + r.Names.Camel + " resource.") } } else { // For ReadMany, search for pluralized identifiers @@ -162,12 +156,12 @@ func FindPrimaryIdentifierFieldNames( // Require override if still can't find any identifiers if shapeField == "" { panic("Could not find primary identifier for " + r.Names.Original + - ". Set `primary_identifier_field_name` for the " + op.Name + - " operation in the generator config.") + ". Set `is_primary_key` for the primary field in the " + + r.Names.Camel + " resource.") } } - if r.IsPrimaryARNField(shapeField) || shapeField == PrimaryIdentifierARNOverride { + if r.IsPrimaryARNField(shapeField) { return "", PrimaryIdentifierARNOverride } diff --git a/pkg/generate/code/set_resource.go b/pkg/generate/code/set_resource.go index 66c725db..0be84e63 100644 --- a/pkg/generate/code/set_resource.go +++ b/pkg/generate/code/set_resource.go @@ -788,12 +788,14 @@ func SetResourceGetAttributes( // 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. +// The method will attempt to look for the field denoted with a value of true +// for `is_primary_key`, or will use the ARN if the resource has a value of true +// for `is_arn_primary_key`. Otherwise, the method will attempt to use the +// `ReadOne` operation, if present, falling 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 field with a +// name that matches the primary identifier from the operation, pulling from +// top-level spec or status fields. // // An example of code with no additional keys: // @@ -873,21 +875,42 @@ func SetResourceIdentifiers( primaryKeyConditionalOut := "\n" primaryKeyConditionalOut += identifierNameOrIDGuardConstructor(sourceVarName, indentLevel) - primaryCRField, primaryShapeField := FindPrimaryIdentifierFieldNames(cfg, r, op) - if primaryShapeField == PrimaryIdentifierARNOverride { - // if r.ko.Status.ACKResourceMetadata == nil { - // r.ko.Status.ACKResourceMetadata = &ackv1alpha1.ResourceMetadata{} - // } - // r.ko.Status.ACKResourceMetadata.ARN = identifier.ARN - arnOut := "\n" - arnOut += ackResourceMetadataGuardConstructor(fmt.Sprintf("%s.Status", targetVarName), indentLevel) - arnOut += fmt.Sprintf( - "%s%s.Status.ACKResourceMetadata.ARN = %s.ARN\n", - indent, targetVarName, sourceVarName, - ) + // if r.ko.Status.ACKResourceMetadata == nil { + // r.ko.Status.ACKResourceMetadata = &ackv1alpha1.ResourceMetadata{} + // } + // r.ko.Status.ACKResourceMetadata.ARN = identifier.ARN + arnOut := "\n" + arnOut += ackResourceMetadataGuardConstructor(fmt.Sprintf("%s.Status", targetVarName), indentLevel) + arnOut += fmt.Sprintf( + "%s%s.Status.ACKResourceMetadata.ARN = %s.ARN\n", + indent, targetVarName, sourceVarName, + ) + // Check if the CRD defines the primary keys + if r.IsARNPrimaryKey() { return arnOut } + primaryField, err := r.GetPrimaryKeyField() + if err != nil { + panic(err) + } + + var primaryCRField, primaryShapeField string + isPrimarySet := primaryField != nil + if isPrimarySet { + memberPath, _ := findFieldInCR(cfg, r, primaryField.Names.Original) + targetVarPath := fmt.Sprintf("%s%s", targetVarName, memberPath) + primaryKeyOut += setResourceIdentifierPrimaryIdentifier(cfg, r, + primaryField, + targetVarPath, + sourceVarName, + indentLevel) + } else { + primaryCRField, primaryShapeField = FindPrimaryIdentifierFieldNames(cfg, r, op) + if primaryShapeField == PrimaryIdentifierARNOverride { + return arnOut + } + } paginatorFieldLookup := []string{ "NextToken", @@ -924,6 +947,11 @@ func SetResourceIdentifiers( op.Name, memberName, ) + // Check to see if we've already set the field as the primary identifier + if isPrimarySet && renamedName == primaryField.Names.Camel { + continue + } + isPrimaryIdentifier := memberName == primaryShapeField searchField := "" @@ -934,7 +962,7 @@ func SetResourceIdentifiers( } memberPath, targetField := findFieldInCR(cfg, r, searchField) - if targetField == nil { + if targetField == nil || (isPrimarySet && targetField == primaryField) { continue } diff --git a/pkg/generate/config/field.go b/pkg/generate/config/field.go index 7e72bfdf..a3ce9839 100644 --- a/pkg/generate/config/field.go +++ b/pkg/generate/config/field.go @@ -170,11 +170,11 @@ type FieldConfig struct { // Required indicates whether this field is a required member or not. // This field is used to configure '+kubebuilder:validation:Required' on API object's members. IsRequired *bool `json:"is_required,omitempty"` - // IsName indicates the field represents the name/string identifier field - // for the resource. This allows the generator config to override the - // default behaviour of considering a field called "Name" or + // IsPrimaryKey indicates the field represents the primary name/string + // identifier field for the resource. This allows the generator config to + // override the default behaviour of considering a field called "Name" or // "{Resource}Name" or "{Resource}Id" as the "name field" for the resource. - IsName bool `json:"is_name"` + IsPrimaryKey bool `json:"is_primary_key"` // IsOwnerAccountID indicates the field contains the AWS Account ID // that owns the resource. This is a special field that we direct to // storage in the common `Status.ACKResourceMetadata.OwnerAccountID` field. diff --git a/pkg/generate/config/operation.go b/pkg/generate/config/operation.go index 471dbb6e..723fee96 100644 --- a/pkg/generate/config/operation.go +++ b/pkg/generate/config/operation.go @@ -18,7 +18,7 @@ import ( awssdkmodel "github.com/aws/aws-sdk-go/private/model/api" - "github.com/aws-controllers-k8s/code-generator/pkg/util" + "github.com/aws-controllers-k8s/code-generator/pkg/util" ) // StringArray is a type that can be represented in JSON as *either* a string @@ -45,10 +45,6 @@ type OperationConfig struct { // An example of this is `Put...` or `Register...` API operations not being correctly classified as `Create` op type // OperationType []string `json:"operation_type"` OperationType StringArray `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 diff --git a/pkg/generate/config/resource.go b/pkg/generate/config/resource.go index 35ebc922..319e7662 100644 --- a/pkg/generate/config/resource.go +++ b/pkg/generate/config/resource.go @@ -82,6 +82,9 @@ type ResourceConfig struct { // Print contains instructions for the code generator to generate kubebuilder printcolumns // marker comments. Print *PrintConfig `json:"print,omitempty"` + // IsARNPrimaryKey determines whether the CRD uses the ARN as the primary + // identifier in the ReadOne operations. + IsARNPrimaryKey bool `json:"is_arn_primary_key"` } // HooksConfig instructs the code generator how to inject custom callback hooks diff --git a/pkg/model/crd.go b/pkg/model/crd.go index f2ccd4e1..4254d776 100644 --- a/pkg/model/crd.go +++ b/pkg/model/crd.go @@ -353,6 +353,46 @@ func (r *CRD) HasImmutableFieldChanges() bool { return false } +// IsARNPrimaryKey returns true if the CRD uses its ARN as its primary key in +// ReadOne calls. +func (r *CRD) IsARNPrimaryKey() bool { + if r.cfg == nil { + return false + } + resGenConfig, found := r.cfg.Resources[r.Names.Original] + if !found { + return false + } + return resGenConfig.IsARNPrimaryKey +} + +// GetPrimaryKeyField returns the field designated as the primary key, nil if +// none are specified or an error if multiple are designated. +func (r *CRD) GetPrimaryKeyField() (*Field, error) { + fConfigs := r.cfg.ResourceFields(r.Names.Original) + + var primaryField *Field + for fieldName, fieldConfig := range fConfigs { + if !fieldConfig.IsPrimaryKey { + continue + } + + // Multiple primary fields + if primaryField != nil { + return nil, fmt.Errorf("multiple fields are marked with is_primary_key") + } + + fieldNames := names.New(fieldName) + fPath := fieldNames.Camel + var notFound bool + primaryField, notFound = r.Fields[fPath] + if !notFound { + return nil, fmt.Errorf("what the frick") + } + } + return primaryField, nil +} + // SetOutputCustomMethodName returns custom set output operation as *string for // given operation on custom resource, if specified in generator config func (r *CRD) SetOutputCustomMethodName( @@ -536,14 +576,12 @@ func (r *CRD) GetCustomCheckRequiredFieldsMissingMethod( // 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] if found { for fName, fConfig := range rConfig.Fields { - if fConfig.IsName { + if fConfig.IsPrimaryKey { return &fName } } diff --git a/pkg/testdata/models/apis/ecr/0000-00-00/generator-with-field-config.yaml b/pkg/testdata/models/apis/ecr/0000-00-00/generator-with-field-config.yaml index 5fb7cba4..5b2a4758 100644 --- a/pkg/testdata/models/apis/ecr/0000-00-00/generator-with-field-config.yaml +++ b/pkg/testdata/models/apis/ecr/0000-00-00/generator-with-field-config.yaml @@ -2,7 +2,7 @@ resources: Repository: fields: Name: - is_name: true + is_primary_key: true exceptions: errors: 404: diff --git a/pkg/testdata/models/apis/rds/0000-00-00/generator.yaml b/pkg/testdata/models/apis/rds/0000-00-00/generator.yaml index 60127152..0b6c3371 100644 --- a/pkg/testdata/models/apis/rds/0000-00-00/generator.yaml +++ b/pkg/testdata/models/apis/rds/0000-00-00/generator.yaml @@ -1,13 +1,15 @@ ignore: shape_names: - DBSecurityGroupMembershipList -operations: - DescribeDBInstances: - primary_identifier_field_name: DBInstanceIdentifier - DescribeDBSubnetGroups: - primary_identifier_field_name: DBSubnetGroupName resources: + DBInstance: + fields: + DBInstanceIdentifier: + is_primary_key: true DBSubnetGroup: + fields: + Name: + is_primary_key: true renames: operations: DescribeDBSubnetGroups: diff --git a/pkg/testdata/models/apis/sagemaker/0000-00-00/generator.yaml b/pkg/testdata/models/apis/sagemaker/0000-00-00/generator.yaml index 305f18fb..cc24de4b 100644 --- a/pkg/testdata/models/apis/sagemaker/0000-00-00/generator.yaml +++ b/pkg/testdata/models/apis/sagemaker/0000-00-00/generator.yaml @@ -22,9 +22,8 @@ resources: Endpoint: reconcile: requeue_on_success_seconds: 10 -operations: - DescribeModelPackage: - primary_identifier_field_name: ARN + ModelPackage: + is_arn_primary_key: true ignore: resource_names: - Algorithm