Merge pull request #736 from knottnt/feat/allow-adoption-with-no-primary-key

Add mutually_exclusive_identifiers config for resources with no single primary key

Author: knottnt

pkg/config/resource.go

@@ -126,6 +126,17 @@ type ResourceConfig struct {
 	// IsARNPrimaryKey determines whether the CRD uses the ARN as the primary
 	// identifier in the ReadOne operations.
 	IsARNPrimaryKey bool `json:"is_arn_primary_key"`
+	// MutuallyExclusiveIdentifiers lists the resource's identifier fields (by
+	// their configuration/original names, e.g. PolicyName, ResourceArn) when a
+	// resource has no single mandatory identifier but is instead identified by
+	// exactly one of several mutually-exclusive fields. When set, adoption
+	// treats each listed field as optional (populating whichever the user
+	// supplies) but requires that exactly one of them is present:
+	// PopulateResourceFromAnnotation returns a terminal error if none or more
+	// than one is supplied. This prevents an empty or misspelled adoption
+	// annotation from silently matching an arbitrary resource. Requires at
+	// least two fields and is incompatible with is_arn_primary_key.
+	MutuallyExclusiveIdentifiers []string `json:"mutually_exclusive_identifiers,omitempty"`
 	// TagConfig contains instructions for the code generator to generate
 	// custom code for ensuring tags
 	TagConfig *TagConfig `json:"tags,omitempty"`
@@ -512,6 +523,20 @@ func (c *Config) ResourceIsAdoptable(resourceName string) bool {
 	return *rConfig.IsAdoptable
 }
 
+// ResourceMutuallyExclusiveIdentifiers returns the list of mutually-exclusive
+// identifier field names configured for the resource (see
+// mutually_exclusive_identifiers), or nil if none are configured.
+func (c *Config) ResourceMutuallyExclusiveIdentifiers(resourceName string) []string {
+	if c == nil {
+		return nil
+	}
+	rConfig, ok := c.Resources[resourceName]
+	if !ok {
+		return nil
+	}
+	return rConfig.MutuallyExclusiveIdentifiers
+}
+
 // ResourceContainsAttributesMap returns true if the underlying API has
 // Get{Resource}Attributes/Set{Resource}Attributes API calls that map real,
 // schema'd fields to a raw `map[string]*string` for a given resource name (see SNS and

pkg/config/validate.go

@@ -17,6 +17,8 @@ import (
 	"fmt"
 	"sort"
 	"strings"
+
+	"github.com/aws-controllers-k8s/pkg/names"
 )
 
 // ValidateConfig checks that generator.yaml references to SDK operations are
@@ -44,10 +46,57 @@ func ValidateConfig(
 
 	errs = append(errs, validateRenameOperations(cfg, sdkOperations)...)
 	errs = append(errs, validateIgnoredOperations(cfg, sdkOperations)...)
+	errs = append(errs, validateMutuallyExclusiveIdentifiers(cfg)...)
 
 	return errs
 }
 
+// validateMutuallyExclusiveIdentifiers checks that a resource's
+// mutually_exclusive_identifiers configuration is internally consistent: it
+// must list at least two fields (a single identifier is not mutually exclusive
+// with anything), the fields must be distinct after name normalization (two
+// entries that resolve to the same CR field can never be "exactly one"), and it
+// cannot be combined with is_arn_primary_key, since an ARN-primary resource
+// always requires its ARN.
+func validateMutuallyExclusiveIdentifiers(cfg *Config) []error {
+	var errs []error
+	for resName, resCfg := range cfg.Resources {
+		identifiers := resCfg.MutuallyExclusiveIdentifiers
+		if len(identifiers) == 0 {
+			continue
+		}
+		if len(identifiers) < 2 {
+			errs = append(errs, fmt.Errorf(
+				"resources.%s.mutually_exclusive_identifiers: must list at least two fields, got %d",
+				resName, len(identifiers),
+			))
+		}
+		// Entries are resolved to CR fields by camel-casing the name (see
+		// CRD.GetMutuallyExclusiveIdentifierFields), so e.g. "PolicyName" and
+		// "policyName" collapse to the same field. Reject duplicates after
+		// normalization: the generated exactly-one guard would otherwise count
+		// the same field twice and adoption could never satisfy it.
+		seen := make(map[string]bool, len(identifiers))
+		for _, id := range identifiers {
+			norm := names.New(id).Camel
+			if seen[norm] {
+				errs = append(errs, fmt.Errorf(
+					"resources.%s.mutually_exclusive_identifiers: %q resolves to the same field as another entry",
+					resName, id,
+				))
+			}
+			seen[norm] = true
+		}
+		if resCfg.IsARNPrimaryKey {
+			errs = append(errs, fmt.Errorf(
+				"resources.%s.mutually_exclusive_identifiers: cannot be combined with is_arn_primary_key",
+				resName,
+			))
+		}
+	}
+	return errs
+}
+
 // validateRenameOperations checks that operation names referenced in
 // resources[R].renames.operations[OpName] exist in the SDK.
 func validateRenameOperations(

pkg/config/validate_test.go

@@ -238,6 +238,64 @@ func TestValidateConfig_ErrorMessageIncludesAvailable(t *testing.T) {
 	}
 }
 
+func TestValidateMutuallyExclusiveIdentifiers(t *testing.T) {
+	testCases := []struct {
+		name         string
+		resourceCfg  ResourceConfig
+		expectedErrs int
+	}{
+		{
+			name: "valid two identifiers",
+			resourceCfg: ResourceConfig{
+				MutuallyExclusiveIdentifiers: []string{"PolicyName", "ResourceArn"},
+			},
+			expectedErrs: 0,
+		},
+		{
+			name:         "unset is allowed",
+			resourceCfg:  ResourceConfig{},
+			expectedErrs: 0,
+		},
+		{
+			name: "single identifier is invalid",
+			resourceCfg: ResourceConfig{
+				MutuallyExclusiveIdentifiers: []string{"PolicyName"},
+			},
+			expectedErrs: 1,
+		},
+		{
+			name: "incompatible with is_arn_primary_key",
+			resourceCfg: ResourceConfig{
+				MutuallyExclusiveIdentifiers: []string{"PolicyName", "ResourceArn"},
+				IsARNPrimaryKey:              true,
+			},
+			expectedErrs: 1,
+		},
+		{
+			name: "single identifier and arn primary key reports both",
+			resourceCfg: ResourceConfig{
+				MutuallyExclusiveIdentifiers: []string{"PolicyName"},
+				IsARNPrimaryKey:              true,
+			},
+			expectedErrs: 2,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			cfg := &Config{
+				Resources: map[string]ResourceConfig{
+					"ResourcePolicy": tc.resourceCfg,
+				},
+			}
+			errs := validateMutuallyExclusiveIdentifiers(cfg)
+			if len(errs) != tc.expectedErrs {
+				t.Errorf("expected %d errors, got %d: %v", tc.expectedErrs, len(errs), errs)
+			}
+		})
+	}
+}
+
 func TestFormatAvailableTruncated(t *testing.T) {
 	items := []string{"A", "B", "C", "D", "E"}
 	got := formatAvailableTruncated(items, 3)

pkg/generate/code/check.go

@@ -77,7 +77,7 @@ func CheckRequiredFieldsMissingFromShape(
 	case model.OpTypeList:
 		op = r.Ops.ReadMany
 		return checkRequiredFieldsMissingFromShapeReadMany(
-			r, koVarName, indentLevel, op, op.InputRef.Shape), nil
+			r, koVarName, indentLevel, op, op.InputRef.Shape)
 	case model.OpTypeGetAttributes:
 		op = r.Ops.GetAttributes
 	case model.OpTypeSetAttributes:
@@ -96,6 +96,42 @@ func CheckRequiredFieldsMissingFromShape(
 	)
 }
 
+// mutuallyExclusiveIdentifierNilConditions returns, for each of the resource's
+// configured mutually-exclusive identifier fields, a "<path> == nil" condition
+// string, along with the set of CR paths so callers can exclude them from
+// per-field required checks. The resource is uniquely identified by exactly one
+// of these fields, so it is considered incomplete only when all of them are
+// nil. Returns nil slices when the resource has no mutually-exclusive
+// identifiers.
+func mutuallyExclusiveIdentifierNilConditions(
+	r *model.CRD,
+	koVarName string,
+) ([]string, map[string]bool, error) {
+	if !r.HasMutuallyExclusiveIdentifiers() {
+		return nil, nil, nil
+	}
+	identifierFields, err := r.GetMutuallyExclusiveIdentifierFields()
+	if err != nil {
+		return nil, nil, err
+	}
+	cfg := r.Config()
+	conditions := make([]string, 0, len(identifierFields))
+	paths := make(map[string]bool, len(identifierFields))
+	for _, identifierField := range identifierFields {
+		memberPath, targetField := findFieldInCR(cfg, r, identifierField.Names.Original)
+		if targetField == nil {
+			return nil, nil, fmt.Errorf(
+				"resource %q: mutually_exclusive_identifiers field %q is not in the CR's Spec or Status",
+				r.Names.Original, identifierField.Names.Original,
+			)
+		}
+		path := fmt.Sprintf("%s%s.%s", koVarName, memberPath, targetField.Path)
+		conditions = append(conditions, fmt.Sprintf("%s == nil", path))
+		paths[path] = true
+	}
+	return conditions, paths, nil
+}
+
 func checkRequiredFieldsMissingFromShape(
 	r *model.CRD,
 	koVarName string,
@@ -104,7 +140,26 @@ func checkRequiredFieldsMissingFromShape(
 	shape *awssdkmodel.Shape,
 ) (string, error) {
 	indent := strings.Repeat("\t", indentLevel)
+
+	// When the resource declares mutually-exclusive identifiers, the resource is
+	// uniquely identified by exactly one of the declared fields. Build a single
+	// grouped condition that is true only when none of them are set, and collect
+	// their CR paths so they are not required individually below. This makes the
+	// generated check treat the input as incomplete unless at least one
+	// identifier is present, mirroring the ReadMany handling.
+	exclusiveConditions, exclusivePaths, err := mutuallyExclusiveIdentifierNilConditions(r, koVarName)
+	if err != nil {
+		return "", err
+	}
+	exclusiveGroupCondition := ""
+	if len(exclusiveConditions) > 0 {
+		exclusiveGroupCondition = fmt.Sprintf("(%s)", strings.Join(exclusiveConditions, " && "))
+	}
+
 	if shape == nil || len(shape.Required) == 0 {
+		if exclusiveGroupCondition != "" {
+			return fmt.Sprintf("%sreturn %s\n", indent, exclusiveGroupCondition), nil
+		}
 		return fmt.Sprintf("%sreturn false", indent), nil
 	}
 
@@ -144,8 +199,16 @@ func checkRequiredFieldsMissingFromShape(
 				r.Names.Original, memberName, shape.ShapeName,
 			)
 		}
+		// Mutually-exclusive identifiers are not required individually; they are
+		// covered by the grouped condition appended below.
+		if exclusivePaths[resVarPath] {
+			continue
+		}
 		missing = append(missing, fmt.Sprintf("%s == nil", resVarPath))
 	}
+	if exclusiveGroupCondition != "" {
+		missing = append(missing, exclusiveGroupCondition)
+	}
 	// Use '||' because if any of the required fields are missing the object
 	// is not created yet
 	missingCondition := strings.Join(missing, " || ")
@@ -174,18 +237,34 @@ func checkRequiredFieldsMissingFromShapeReadMany(
 	indentLevel int,
 	op *awssdkmodel.Operation,
 	shape *awssdkmodel.Shape,
-) string {
+) (string, error) {
 	indent := strings.Repeat("\t", indentLevel)
 	result := fmt.Sprintf("%sreturn false", indent)
 
+	// When the resource declares mutually-exclusive identifiers, the ReadMany
+	// input typically has no required members, so the default `return false`
+	// would let sdkFind list every resource and match an arbitrary one. Instead,
+	// treat the read input as incomplete (returning true so sdkFind bails out
+	// with NotFound) unless at least one of the declared identifiers is set on
+	// the resource.
+	if r.HasMutuallyExclusiveIdentifiers() {
+		exclusiveConditions, _, err := mutuallyExclusiveIdentifierNilConditions(r, koVarName)
+		if err != nil {
+			return "", err
+		}
+		// Parenthesize the grouped condition to mirror the ReadOne handling and
+		// stay correct if a future term is ever joined here with `||`.
+		return fmt.Sprintf("%sreturn (%s)\n", indent, strings.Join(exclusiveConditions, " && ")), nil
+	}
+
 	reqIdentifier, _ := FindPluralizedIdentifiersInShape(r, shape, op)
 	resVarPath, err := r.GetSanitizedMemberPath(reqIdentifier, op, koVarName)
 	if err != nil {
-		return result
+		return result, nil
 	}
 
 	result = fmt.Sprintf("%s == nil", resVarPath)
-	return fmt.Sprintf("%sreturn %s\n", indent, result)
+	return fmt.Sprintf("%sreturn %s\n", indent, result), nil
 }
 
 // CheckNilFieldPath returns the condition statement for Nil check

pkg/generate/code/check_test.go

@@ -165,6 +165,65 @@ func TestCheckRequiredFields_StatusField_ReadMany_EgressOnlyIGW(t *testing.T) {
 	)
 }
 
+func TestCheckRequiredFields_MutuallyExclusiveIdentifiers_ReadOne(t *testing.T) {
+	assert := assert.New(t)
+	require := require.New(t)
+
+	g := testutil.NewModelForServiceWithOptions(t, "opensearchserverless", &testutil.TestingModelOptions{
+		GeneratorConfigFile: "generator-with-mutually-exclusive-identifiers.yaml",
+	})
+
+	crd := testutil.GetCRDByName(t, g, "SecurityPolicy")
+	require.NotNil(crd)
+	require.True(crd.HasMutuallyExclusiveIdentifiers())
+
+	// GetSecurityPolicy (ReadOne) marks both `name` and `type` required.
+	// Declaring them mutually exclusive drops each from the per-field `||` list
+	// and collapses them into a single grouped condition that is true only when
+	// neither identifier is set.
+	expRequiredFieldsCode := `
+	return (r.ko.Spec.Name == nil && r.ko.Spec.Type == nil)
+`
+	gotCode, err := code.CheckRequiredFieldsMissingFromShape(
+		crd, model.OpTypeGet, "r.ko", 1,
+	)
+	require.NoError(err)
+	assert.Equal(
+		strings.TrimSpace(expRequiredFieldsCode),
+		strings.TrimSpace(gotCode),
+	)
+}
+
+func TestCheckRequiredFields_MutuallyExclusiveIdentifiers_ReadMany(t *testing.T) {
+	assert := assert.New(t)
+	require := require.New(t)
+
+	g := testutil.NewModelForServiceWithOptions(t, "cloudwatch-logs", &testutil.TestingModelOptions{
+		GeneratorConfigFile: "generator.yaml",
+	})
+
+	crd := testutil.GetCRDByName(t, g, "ResourcePolicy")
+	require.NotNil(crd)
+	require.True(crd.HasMutuallyExclusiveIdentifiers())
+
+	// DescribeResourcePolicies has no required input members, so without
+	// mutually_exclusive_identifiers this would generate `return false` and let
+	// sdkFind match an arbitrary policy. Instead, the read input is treated as
+	// incomplete unless at least one of the declared identifiers is set, which
+	// is what the controller previously had to supply via a custom method.
+	expRequiredFieldsCode := `
+	return (r.ko.Spec.PolicyName == nil && r.ko.Spec.ResourceARN == nil)
+`
+	gotCode, err := code.CheckRequiredFieldsMissingFromShape(
+		crd, model.OpTypeList, "r.ko", 1,
+	)
+	require.NoError(err)
+	assert.Equal(
+		strings.TrimSpace(expRequiredFieldsCode),
+		strings.TrimSpace(gotCode),
+	)
+}
+
 func TestCheckNilFieldPath(t *testing.T) {
 	// Empty FieldPath
 	field := model.Field{Path: ""}