From 4eb656abceb24c581c20999230c276e4a3d4a687 Mon Sep 17 00:00:00 2001 From: Hao Wang Date: Wed, 6 May 2026 14:53:12 -0700 Subject: [PATCH] feat(runtime): Add unified --enable-cross-namespace flag Introduce a single CLI flag that controls all three categories of cross-namespace behavior: resource references, secret references, and field exports. Phase 1 defaults to true with deprecation warnings. Changes: - Rename --enable-cross-namespace-references to --enable-cross-namespace - Change default from false to true (Phase 1 warning behavior) - Update ValidateCrossNamespaceReference to return (string, bool, error) - Add ValidateCrossNamespaceReferenceString convenience wrapper - Add ACK.CrossNamespaceDeprecation condition type - Integrate cross-namespace check into SecretValueFromReference - Integrate cross-namespace check into field export reconciler - Add table-driven unit tests for both helper functions - Update error messages to reference new flag name --- apis/core/v1alpha1/conditions.go | 5 + pkg/condition/condition.go | 3 +- pkg/config/config.go | 9 + pkg/errors/resource_reference.go | 23 +++ pkg/runtime/field_export_reconciler.go | 88 +++++++-- pkg/runtime/reconciler.go | 17 +- pkg/runtime/reconciler_test.go | 71 ++++++++ pkg/runtime/resource_reference.go | 81 +++++++++ pkg/runtime/resource_reference_test.go | 238 +++++++++++++++++++++++++ 9 files changed, 515 insertions(+), 20 deletions(-) create mode 100644 pkg/runtime/resource_reference.go create mode 100644 pkg/runtime/resource_reference_test.go diff --git a/apis/core/v1alpha1/conditions.go b/apis/core/v1alpha1/conditions.go index 5a9a5eed..d9d4e88b 100644 --- a/apis/core/v1alpha1/conditions.go +++ b/apis/core/v1alpha1/conditions.go @@ -70,6 +70,11 @@ const ( // to manage the AWSResource. If none are selected, this condition will be removed // and we'll use the custom role to manage the AWSResource ConditionTypeIAMRoleSelected ConditionType = "ACK.IAMRoleSelected" + // ConditionTypeCrossNamespaceOptInRequired indicates that the resource uses + // cross-namespace behavior (resource references, secret references, or + // field exports) that will require explicit opt-in via + // --enable-cross-namespace=true in a future release. + ConditionTypeCrossNamespaceOptInRequired ConditionType = "ACK.CrossNamespaceOptInRequired" ) // Condition is the common struct used by all CRDs managed by ACK service diff --git a/pkg/condition/condition.go b/pkg/condition/condition.go index 3eb370c9..83d083a6 100644 --- a/pkg/condition/condition.go +++ b/pkg/condition/condition.go @@ -314,7 +314,8 @@ func WithReferencesResolvedCondition( if err != nil { errString := err.Error() conditionStatus := corev1.ConditionUnknown - if strings.Contains(errString, ackerr.ResourceReferenceTerminal.Error()) { + if strings.Contains(errString, ackerr.ResourceReferenceTerminal.Error()) || + strings.Contains(errString, ackerr.ResourceReferenceCrossNamespaceNotAllowed.Error()) { conditionStatus = corev1.ConditionFalse } SetReferencesResolved(ko, conditionStatus, &FailedReferenceResolutionMessage, &errString) diff --git a/pkg/config/config.go b/pkg/config/config.go index 3bc86eca..07a033fa 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -43,6 +43,7 @@ import ( ) const ( + flagEnableCrossNamespace = "enable-cross-namespace" flagEnableFieldExportReconciler = "enable-field-export-reconciler" flagEnableLeaderElection = "enable-leader-election" flagEnableCARM = "enable-carm" @@ -92,6 +93,7 @@ type Config struct { EnableLeaderElection bool EnableFieldExportReconciler bool EnableCARM bool + EnableCrossNamespace bool LeaderElectionNamespace string EnableDevelopmentLogging bool AccountID string @@ -158,6 +160,13 @@ func (cfg *Config) BindFlags() { true, "Enable Cross Account Resource Management.", ) + flag.BoolVar( + &cfg.EnableCrossNamespace, flagEnableCrossNamespace, + true, + "Enable cross-namespace behavior (resource references, secret references, "+ + "field exports). When false, the controller rejects any operation that "+ + "crosses namespace boundaries.", + ) flag.StringVar( // In the context of the controller-runtime library, if the LeaderElectionNamespace parametere is not // explicitly set, the library will automatically default its value to the content of the file diff --git a/pkg/errors/resource_reference.go b/pkg/errors/resource_reference.go index 2cd46e65..056d7cd4 100644 --- a/pkg/errors/resource_reference.go +++ b/pkg/errors/resource_reference.go @@ -47,6 +47,14 @@ var ( ResourceReferenceMissingTargetField = fmt.Errorf( "the referenced resource is missing the target field", ) + // ResourceReferenceCrossNamespaceNotAllowed indicates that a resource + // reference specifies a Namespace that differs from the namespace of the + // resource containing the reference, and the + // --enable-cross-namespace flag is not enabled. + ResourceReferenceCrossNamespaceNotAllowed = fmt.Errorf( + "cross-namespace resource reference is not allowed. " + + "Set '--enable-cross-namespace' flag to allow it", + ) ) // ResourceReferenceOrIDRequiredFor returns a ResourceReferenceOrIDRequired error @@ -90,3 +98,18 @@ func ResourceReferenceMissingTargetFieldFor(resource string, namespace string, ", targetField:%s", ResourceReferenceMissingTargetField, resource, namespace, name, targetField) } + +// ResourceReferenceCrossNamespaceNotAllowedFor returns a +// ResourceReferenceCrossNamespaceNotAllowed error annotated with the +// source namespace, target namespace, and referenced resource name. +func ResourceReferenceCrossNamespaceNotAllowedFor( + sourceNamespace string, + targetNamespace string, + name string, +) error { + return fmt.Errorf( + "%w. sourceNamespace:%s, targetNamespace:%s, name:%s", + ResourceReferenceCrossNamespaceNotAllowed, + sourceNamespace, targetNamespace, name, + ) +} diff --git a/pkg/runtime/field_export_reconciler.go b/pkg/runtime/field_export_reconciler.go index c10461a6..50bfdde1 100644 --- a/pkg/runtime/field_export_reconciler.go +++ b/pkg/runtime/field_export_reconciler.go @@ -153,6 +153,23 @@ func (r *fieldExportReconciler) Sync( r.patchResourceStatus(ctx, &desired, latest) }() + // Validate cross-namespace access for the field export target + resolvedNamespace, isCrossNamespace, nsErr := r.validateFieldExportNamespace(&desired) + if nsErr != nil { + return desired, r.onError(ctx, &desired, ackerr.NewTerminalError(nsErr)) + } + if isCrossNamespace { + r.log.V(0).Info( + "cross-namespace field export detected; this behavior will be "+ + "disabled by default in a future release. Set --enable-cross-namespace "+ + "to preserve this behavior.", + "fieldExportNamespace", desired.Namespace, + "targetNamespace", r.getTargetNamespace(&desired), + "targetName", *desired.Spec.To.Name, + ) + r.setCrossNsOptInRequiredCondition(&desired) + } + // Get the field from the resource value, err := r.getSourcePathFromResource(from, *desired.Spec.From.Path) if err != nil { @@ -163,11 +180,11 @@ func (r *fieldExportReconciler) Sync( switch desired.Spec.To.Kind { case ackv1alpha1.FieldExportOutputTypeConfigMap: - if err = r.writeToConfigMap(ctx, *value, &desired); err != nil { + if err = r.writeToConfigMap(ctx, *value, &desired, resolvedNamespace); err != nil { return desired, r.onError(ctx, &desired, err) } case ackv1alpha1.FieldExportOutputTypeSecret: - if err = r.writeToSecret(ctx, *value, &desired); err != nil { + if err = r.writeToSecret(ctx, *value, &desired, resolvedNamespace); err != nil { return desired, r.onError(ctx, &desired, err) } } @@ -177,6 +194,55 @@ func (r *fieldExportReconciler) Sync( return desired, r.onSuccess(ctx, &desired) } +// validateFieldExportNamespace checks whether the field export target namespace +// is allowed given the current cross-namespace flag setting. +func (r *fieldExportReconciler) validateFieldExportNamespace( + desired *ackv1alpha1.FieldExport, +) (string, bool, error) { + targetNamespace := r.getTargetNamespace(desired) + targetName := "" + if desired.Spec.To != nil && desired.Spec.To.Name != nil { + targetName = *desired.Spec.To.Name + } + + return ValidateCrossNamespaceReferenceString( + r.cfg.EnableCrossNamespace, + desired.Namespace, + targetNamespace, + targetName, + ) +} + +// getTargetNamespace returns the target namespace for the field export write. +// If Spec.To.Namespace is set, it returns that value; otherwise it returns +// the FieldExport's own namespace. +func (r *fieldExportReconciler) getTargetNamespace( + desired *ackv1alpha1.FieldExport, +) string { + if desired.Spec.To != nil && desired.Spec.To.Namespace != nil && *desired.Spec.To.Namespace != "" { + return *desired.Spec.To.Namespace + } + return desired.Namespace +} + +// setCrossNsOptInRequiredCondition sets the ACK.CrossNamespaceOptInRequired +// condition on the FieldExport resource to notify users that cross-namespace +// behavior will require explicit opt-in in a future release. +func (r *fieldExportReconciler) setCrossNsOptInRequiredCondition( + desired *ackv1alpha1.FieldExport, +) { + message := "Cross-namespace field export detected: FieldExport in namespace \"" + + desired.Namespace + "\" targets namespace \"" + r.getTargetNamespace(desired) + + "\". Cross-namespace behavior will require explicit opt-in in a future release. " + + "Set --enable-cross-namespace=true to preserve this behavior." + condition := &ackv1alpha1.Condition{ + Type: ackv1alpha1.ConditionTypeCrossNamespaceOptInRequired, + Status: corev1.ConditionTrue, + Message: &message, + } + desired.Status.Conditions = append(desired.Status.Conditions, condition) +} + // cleanup removes the finalizer from FieldExport so that k8s object can // be deleted. func (r *fieldExportReconciler) cleanup( @@ -293,6 +359,7 @@ func (r *fieldExportReconciler) writeToConfigMap( ctx context.Context, sourceValue string, desired *ackv1alpha1.FieldExport, + resolvedNamespace string, ) error { // Construct the data key key := fmt.Sprintf("%s.%s", desired.Namespace, desired.Name) @@ -302,12 +369,8 @@ func (r *fieldExportReconciler) writeToConfigMap( // Get the initial configmap nsn := types.NamespacedName{ - Name: *desired.Spec.To.Name, - } - if desired.Spec.To.Namespace != nil { - nsn.Namespace = *desired.Spec.To.Namespace - } else { - nsn.Namespace = desired.Namespace + Name: *desired.Spec.To.Name, + Namespace: resolvedNamespace, } cm := &corev1.ConfigMap{} @@ -340,6 +403,7 @@ func (r *fieldExportReconciler) writeToSecret( ctx context.Context, sourceValue string, desired *ackv1alpha1.FieldExport, + resolvedNamespace string, ) error { // Construct the data key key := fmt.Sprintf("%s.%s", desired.Namespace, desired.Name) @@ -349,12 +413,8 @@ func (r *fieldExportReconciler) writeToSecret( // Get the initial secret nsn := types.NamespacedName{ - Name: *desired.Spec.To.Name, - } - if desired.Spec.To.Namespace != nil { - nsn.Namespace = *desired.Spec.To.Namespace - } else { - nsn.Namespace = desired.Namespace + Name: *desired.Spec.To.Name, + Namespace: resolvedNamespace, } secret := &corev1.Secret{} diff --git a/pkg/runtime/reconciler.go b/pkg/runtime/reconciler.go index 5d354762..44489cb7 100644 --- a/pkg/runtime/reconciler.go +++ b/pkg/runtime/reconciler.go @@ -131,21 +131,28 @@ func (r *reconciler) SecretValueFromReference( return "", nil } - namespace := ref.Namespace // During the reconcile process, the resourceNamespace is stored in the context // and can be used to fetch the secret if the namespace is not provided in the // SecretKeyReference. // // NOTE(a-hilaly): When refactoring the runtime, we might want to consider passing // the ObjectMeta in the context. + ownerNamespace := "" ctxResourceNamespace := ctx.Value("resourceNamespace") - if namespace == "" && ctxResourceNamespace != nil { - ctxNamespace, ok := ctxResourceNamespace.(string) - if ok { - namespace = ctxNamespace + if ctxResourceNamespace != nil { + if ctxNamespace, ok := ctxResourceNamespace.(string); ok { + ownerNamespace = ctxNamespace } } + // Determine the namespace to use for fetching the secret. + // Cross-namespace validation, logging, and condition-setting are handled + // by the generated code before calling this method. + namespace := ref.Namespace + if namespace == "" { + namespace = ownerNamespace + } + nsn := client.ObjectKey{ Namespace: namespace, Name: ref.Name, diff --git a/pkg/runtime/reconciler_test.go b/pkg/runtime/reconciler_test.go index 8c57ab6c..96967f8a 100644 --- a/pkg/runtime/reconciler_test.go +++ b/pkg/runtime/reconciler_test.go @@ -2405,3 +2405,74 @@ func TestPreDeleteSync_UpdateAndDeleteBothFail(t *testing.T) { // Delete should NOT have been called — fail fast on pre-delete sync error rm.AssertNotCalled(t, "Delete", mock.Anything, mock.Anything) } + +func TestReconcilerSync_CrossNamespaceReferenceRejected(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + + ctx := context.TODO() + + desired, _, _ := resourceMocks() + desired.On("ReplaceConditions", []*ackv1alpha1.Condition{}).Return() + + crossNsErr := ackerr.ResourceReferenceCrossNamespaceNotAllowedFor( + "default", "other-ns", "my-ref", + ) + + // When ResolveReferences returns a cross-namespace error, the reconciler + // should set ReferencesResolved=False on the resource and return the + // error without calling ReadOne. + desired.On("Conditions").Return([]*ackv1alpha1.Condition{}) + desired.On( + "ReplaceConditions", + mock.AnythingOfType("[]*v1alpha1.Condition"), + ).Return().Run(func(args mock.Arguments) { + conditions := args.Get(0).([]*ackv1alpha1.Condition) + // Find the ReferencesResolved condition + var refCond *ackv1alpha1.Condition + for _, c := range conditions { + if c.Type == ackv1alpha1.ConditionTypeReferencesResolved { + refCond = c + break + } + } + require.NotNil(refCond, "expected ReferencesResolved condition to be set") + assert.Equal(corev1.ConditionFalse, refCond.Status, + "ReferencesResolved should be False for cross-namespace rejection") + assert.Equal(ackcondition.FailedReferenceResolutionMessage, *refCond.Message) + assert.Contains(*refCond.Reason, "cross-namespace resource reference is not allowed") + assert.Contains(*refCond.Reason, "default") + assert.Contains(*refCond.Reason, "other-ns") + assert.Contains(*refCond.Reason, "my-ref") + }) + + rm := &ackmocks.AWSResourceManager{} + rm.On("ResolveReferences", ctx, nil, desired).Return( + desired, true, crossNsErr, + ) + rm.On("ClearResolvedReferences", desired).Return(desired) + + latest, latestRTObj, _ := resourceMocks() + rm.On("ClearResolvedReferences", latest).Return(latest) + // ReadOne should NOT be called — set it up but assert it's never invoked + rm.On("ReadOne", ctx, desired).Return(latest, nil) + + rmf, _ := managedResourceManagerFactoryMocks(desired, latest) + + r, kc, scmd := reconcilerMocks(rmf) + rm.On("EnsureTags", ctx, desired, scmd).Return(nil) + kc.On("Patch", withoutCancelContextMatcher, latestRTObj, mock.AnythingOfType("*client.mergeFromPatch")).Return(nil) + + _, err := r.Sync(ctx, rm, desired) + require.NotNil(err) + assert.True(errors.Is(err, ackerr.ResourceReferenceCrossNamespaceNotAllowed), + "error should wrap ResourceReferenceCrossNamespaceNotAllowed sentinel") + + // ReadOne must NOT be invoked when references fail to resolve + rm.AssertNotCalled(t, "ReadOne", ctx, desired) + // No downstream operations should occur + rm.AssertNotCalled(t, "Create", ctx, desired) + rm.AssertNotCalled(t, "Update") + rm.AssertNotCalled(t, "LateInitialize") + rm.AssertNotCalled(t, "EnsureTags", ctx, desired, scmd) +} diff --git a/pkg/runtime/resource_reference.go b/pkg/runtime/resource_reference.go new file mode 100644 index 00000000..9e42a735 --- /dev/null +++ b/pkg/runtime/resource_reference.go @@ -0,0 +1,81 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package runtime + +import ( + ackerr "github.com/aws-controllers-k8s/runtime/pkg/errors" +) + +// ValidateCrossNamespaceReference inspects a user-supplied reference +// namespace and decides which namespace the controller should use when +// resolving the reference. +// +// Parameters: +// - enableCrossNamespace: the value of Config.EnableCrossNamespace +// - ownerNamespace: the namespace of the resource containing the +// reference (never empty) +// - refNamespace: the Namespace field on the AWSResourceReference +// (may be nil or empty) +// - refName: the Name field on the AWSResourceReference; used only for +// error message context +// +// Return values: +// - resolvedNamespace: the namespace to pass to apiReader.Get +// - isCrossNamespace: true if the reference targets a different namespace +// (callers should emit a deprecation warning when flag=true) +// - err: nil on success; a ResourceReferenceCrossNamespaceNotAllowed +// error when cross-namespace refs are disabled and the reference +// targets a different namespace +// +// Same-namespace behavior (nil, empty, or equal refNamespace) is +// unaffected by the flag. +func ValidateCrossNamespaceReference( + enableCrossNamespace bool, + ownerNamespace string, + refNamespace *string, + refName string, +) (string, bool, error) { + // Nil or empty: always use owner namespace. + if refNamespace == nil || *refNamespace == "" { + return ownerNamespace, false, nil + } + // Same namespace: always allowed, not cross-namespace. + if *refNamespace == ownerNamespace { + return ownerNamespace, false, nil + } + // Differing namespace: gated by CLI flag. + if enableCrossNamespace { + return *refNamespace, true, nil + } + return "", false, ackerr.ResourceReferenceCrossNamespaceNotAllowedFor( + ownerNamespace, *refNamespace, refName, + ) +} + +// ValidateCrossNamespaceReferenceString is a convenience wrapper for callers +// that have a string namespace (e.g., SecretKeyReference.Namespace, +// FieldExportTarget.Namespace) rather than a *string. +func ValidateCrossNamespaceReferenceString( + enableCrossNamespace bool, + ownerNamespace string, + refNamespace string, + refName string, +) (string, bool, error) { + if refNamespace == "" { + return ownerNamespace, false, nil + } + return ValidateCrossNamespaceReference( + enableCrossNamespace, ownerNamespace, &refNamespace, refName, + ) +} diff --git a/pkg/runtime/resource_reference_test.go b/pkg/runtime/resource_reference_test.go new file mode 100644 index 00000000..e269c539 --- /dev/null +++ b/pkg/runtime/resource_reference_test.go @@ -0,0 +1,238 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package runtime + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + ackerr "github.com/aws-controllers-k8s/runtime/pkg/errors" +) + +func TestValidateCrossNamespaceReference(t *testing.T) { + const ownerNs = "owner-ns" + const otherNs = "other-ns" + const refName = "my-ref" + + tests := []struct { + name string + flag bool + refNs *string + expectedNs string + expectedCrossNs bool + expectErr bool + }{ + // Same-namespace references always resolve to owner namespace + // regardless of flag state. + { + name: "nil namespace, flag disabled", + flag: false, + refNs: nil, + expectedNs: ownerNs, + expectedCrossNs: false, + expectErr: false, + }, + { + name: "nil namespace, flag enabled", + flag: true, + refNs: nil, + expectedNs: ownerNs, + expectedCrossNs: false, + expectErr: false, + }, + { + name: "empty namespace, flag disabled", + flag: false, + refNs: strPtr(""), + expectedNs: ownerNs, + expectedCrossNs: false, + expectErr: false, + }, + { + name: "empty namespace, flag enabled", + flag: true, + refNs: strPtr(""), + expectedNs: ownerNs, + expectedCrossNs: false, + expectErr: false, + }, + { + name: "same namespace, flag disabled", + flag: false, + refNs: strPtr(ownerNs), + expectedNs: ownerNs, + expectedCrossNs: false, + expectErr: false, + }, + { + name: "same namespace, flag enabled", + flag: true, + refNs: strPtr(ownerNs), + expectedNs: ownerNs, + expectedCrossNs: false, + expectErr: false, + }, + + // Cross-namespace resolves to the specified namespace with warning + // indicator when the flag is enabled. + { + name: "different namespace, flag enabled", + flag: true, + refNs: strPtr(otherNs), + expectedNs: otherNs, + expectedCrossNs: true, + expectErr: false, + }, + + // Cross-namespace rejected with terminal error when the flag is + // disabled. + { + name: "different namespace, flag disabled", + flag: false, + refNs: strPtr(otherNs), + expectedNs: "", + expectedCrossNs: false, + expectErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ns, isCrossNs, err := ValidateCrossNamespaceReference( + tt.flag, ownerNs, tt.refNs, refName, + ) + assert.Equal(t, tt.expectedNs, ns) + assert.Equal(t, tt.expectedCrossNs, isCrossNs) + if tt.expectErr { + require.Error(t, err) + assert.True(t, + errors.Is(err, ackerr.ResourceReferenceCrossNamespaceNotAllowed), + "error should wrap ResourceReferenceCrossNamespaceNotAllowed sentinel", + ) + } else { + assert.NoError(t, err) + } + }) + } + + // Rejection error carries diagnostic context. + t.Run("error contains source namespace", func(t *testing.T) { + _, _, err := ValidateCrossNamespaceReference(false, ownerNs, strPtr(otherNs), refName) + require.Error(t, err) + assert.Contains(t, err.Error(), ownerNs) + }) + + t.Run("error contains target namespace", func(t *testing.T) { + _, _, err := ValidateCrossNamespaceReference(false, ownerNs, strPtr(otherNs), refName) + require.Error(t, err) + assert.Contains(t, err.Error(), otherNs) + }) + + t.Run("error contains ref name", func(t *testing.T) { + _, _, err := ValidateCrossNamespaceReference(false, ownerNs, strPtr(otherNs), refName) + require.Error(t, err) + assert.Contains(t, err.Error(), refName) + }) + + t.Run("error contains flag name", func(t *testing.T) { + _, _, err := ValidateCrossNamespaceReference(false, ownerNs, strPtr(otherNs), refName) + require.Error(t, err) + assert.Contains(t, err.Error(), "enable-cross-namespace") + }) + + t.Run("error wraps sentinel", func(t *testing.T) { + _, _, err := ValidateCrossNamespaceReference(false, ownerNs, strPtr(otherNs), refName) + require.Error(t, err) + assert.True(t, + errors.Is(err, ackerr.ResourceReferenceCrossNamespaceNotAllowed), + "error should be unwrappable to the sentinel via errors.Is", + ) + }) +} + +func TestValidateCrossNamespaceReferenceString(t *testing.T) { + const ownerNs = "owner-ns" + const otherNs = "other-ns" + const refName = "my-ref" + + tests := []struct { + name string + flag bool + refNs string + expectedNs string + expectedCrossNs bool + expectErr bool + }{ + // Empty string namespace resolves to owner namespace. + { + name: "empty string namespace", + flag: false, + refNs: "", + expectedNs: ownerNs, + expectedCrossNs: false, + expectErr: false, + }, + // Same namespace string resolves to owner namespace. + { + name: "same namespace string", + flag: true, + refNs: ownerNs, + expectedNs: ownerNs, + expectedCrossNs: false, + expectErr: false, + }, + // Different namespace string with flag enabled resolves to the + // specified namespace with cross-namespace indicator. + { + name: "different namespace string, flag enabled", + flag: true, + refNs: otherNs, + expectedNs: otherNs, + expectedCrossNs: true, + expectErr: false, + }, + // Different namespace string with flag disabled returns a terminal + // error. + { + name: "different namespace string, flag disabled", + flag: false, + refNs: otherNs, + expectedNs: "", + expectedCrossNs: false, + expectErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ns, isCrossNs, err := ValidateCrossNamespaceReferenceString( + tt.flag, ownerNs, tt.refNs, refName, + ) + assert.Equal(t, tt.expectedNs, ns) + assert.Equal(t, tt.expectedCrossNs, isCrossNs) + if tt.expectErr { + require.Error(t, err) + assert.True(t, + errors.Is(err, ackerr.ResourceReferenceCrossNamespaceNotAllowed), + "error should wrap ResourceReferenceCrossNamespaceNotAllowed sentinel", + ) + } else { + assert.NoError(t, err) + } + }) + } +}