feat: Add cross-namespace reference validation to generated code by sapphirew · Pull Request #699 · aws-controllers-k8s/code-generator

sapphirew · 2026-05-08T22:39:16Z

Issue #, if available:

Description of changes:

Replace the legacy unconditional namespace-override block in ResolveReferencesForField with a call to the shared runtime helper ackrt.ValidateCrossNamespaceReference. This centralizes cross-namespace reference validation so that when --enable-cross-namespace is disabled, references targeting a different namespace are rejected with a terminal error.

The generated code captures three return values (namespace, isCrossNs, error) from the helper. The isCrossNs signal is suppressed in generated code since warning conditions are handled centrally by the reconciler.

Helm chart templates are updated to expose the --enable-cross-namespace flag (default: true for Phase 1 warning behavior) covering resource references, secret references, and field exports.

Changes:

resource_reference.go: emit ValidateCrossNamespaceReference call with EnableCrossNamespace field and three return values
resource_reference_test.go: update all expected output strings to match the new helper call shape
deployment.yaml.tpl: add --enable-cross-namespace flag to container args
values.yaml.tpl: add enableCrossNamespace: true with description
values.schema.json.tpl: add enableCrossNamespace schema entry

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Replace the legacy unconditional namespace-override block in ResolveReferencesForField with a call to the shared runtime helper ackrt.ValidateCrossNamespaceReference. This centralizes cross-namespace reference validation so that when --enable-cross-namespace is disabled, references targeting a different namespace are rejected with a terminal error. The generated code captures three return values (namespace, isCrossNs, error) from the helper. The isCrossNs signal is suppressed in generated code since warning conditions are handled centrally by the reconciler. Helm chart templates are updated to expose the --enable-cross-namespace flag (default: true for Phase 1 warning behavior) covering resource references, secret references, and field exports. Changes: - resource_reference.go: emit ValidateCrossNamespaceReference call with EnableCrossNamespace field and three return values - resource_reference_test.go: update all expected output strings to match the new helper call shape - deployment.yaml.tpl: add --enable-cross-namespace flag to container args - values.yaml.tpl: add enableCrossNamespace: true with description - values.schema.json.tpl: add enableCrossNamespace schema entry

kprahulraj · 2026-05-21T23:09:21Z

Doc changes is added - aws-controllers-k8s/docs#37

knottnt · 2026-05-22T17:59:25Z

-			outPrefix += fmt.Sprintf("%snamespace := ko.ObjectMeta.GetNamespace()\n", innerIndent)
-			outPrefix += fmt.Sprintf("%sif arr.Namespace != nil && *arr.Namespace != \"\" {\n", innerIndent)
-			outPrefix += fmt.Sprintf("%s\tnamespace = *arr.Namespace\n", innerIndent)
+			outPrefix += fmt.Sprintf("%snamespace, isCrossNs, err := ackrt.ValidateCrossNamespaceReference(\n", innerIndent)


Q: does the function comment need to be updated to reflect the new output?

Good catch — updated the function comment to reflect the new generated output (ValidateCrossNamespaceReference call with three return values, error handling, and isCrossNs warning/condition logic).

knottnt · 2026-05-22T19:14:54Z

 {{ "{{- end }}" }}
        - {{ "--enable-carm={{ .Values.enableCARM }}" }}
+        - {{ "--enable-cross-namespace={{ .Values.enableCrossNamespace }}" }}
+        - {{ "--enable-cross-namespace-references={{ .Values.enableCrossNamespaceReferences }}" }}


I don't see this flag in the associated runtime changes.

You're right — this was a leftover from the previous spec. The --enable-cross-namespace-references flag was superseded by --enable-cross-namespace in the runtime. Removed it.

knottnt · 2026-05-22T19:15:15Z

+# When false (default), the controller rejects any reference whose
+# Namespace field differs from the namespace of the resource containing
+# the reference.
+enableCrossNamespaceReferences: false


I don't see this flag in the associated runtime changes or in the values.schema.json.tpl.

Same issue — removed the stale enableCrossNamespaceReferences entry. Only enableCrossNamespace: true remains, which matches the runtime flag and is already in values.schema.json.tpl.

@sapphirew

Issue #, if available: Description of changes: Introduce a single CLI flag that controls all three categories of cross-namespace behavior: resource references, secret references, and field exports. It defaults to true with deprecation warnings to start with. It will default to false later in order to follow best security practice. Changes: - Add `--enable-cross-namespace` CLI flag with default value `true` - Update ValidateCrossNamespaceReference to return (string, bool, error) - Add ValidateCrossNamespaceReferenceString convenience wrapper - Add `ACK.CrossNamespaceOptInRequired` condition type - Integrate cross-namespace check into field export reconciler - Refactor SecretValueFromReference to extract ownerNamespace (cross-namespace validation for secrets is handled by generated code in aws-controllers-k8s/code-generator#699) - Add table-driven unit tests for both helper functions - Update error messages to reference new flag name Note: This PR builds on @sapphirew work in [#239](#239) and addresses review feedback from @cheeseandcereal : - Use nil-safe targetName in field export log line - Use lookup-or-create pattern in setCrossNsOptInRequiredCondition to avoid duplicate conditions - Return ownerNamespace (not empty string) on error for defensive fail-closed behavior By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

- Update ResolveReferencesForField function comment to reflect new generated output with ValidateCrossNamespaceReference call, three return values, and cross-namespace warning/condition logic - Remove stale --enable-cross-namespace-references flag from deployment.yaml.tpl (superseded by --enable-cross-namespace) - Remove stale enableCrossNamespaceReferences value from values.yaml.tpl (superseded by enableCrossNamespace)

michaelhtm · 2026-05-28T19:25:29Z

-//
-// Sample output (resolving nested lists of structs containing references):
-//
-//	if ko.Spec.Notification != nil {
-//		for f0idx, f0iter := range ko.Spec.Notification.LambdaFunctionConfigurations {
-//			if f0iter.Filter != nil {
-//				if f0iter.Filter.Key != nil {
-//					for f1idx, f1iter := range f0iter.Filter.Key.FilterRules {
-//						if f1iter.ValueRef != nil && f1iter.ValueRef.From != nil {
-//							hasReferences = true
-//							arr := f1iter.ValueRef.From
-//							if arr.Name == nil || *arr.Name == "" {
-//								return hasReferences, fmt.Errorf("provided resource reference is nil or empty: Notification.LambdaFunctionConfigurations.Filter.Key.FilterRules.ValueRef")
-//							}
-//							obj := &svcapitypes.Bucket{}
-//							if err := getReferencedResourceState_Bucket(ctx, apiReader, obj, *arr.Name, namespace); err != nil {
-//								return hasReferences, err
-//							}
-//							ko.Spec.Notification.LambdaFunctionConfigurations[f0idx].Filter.Key.FilterRules[f1idx].Value = (*string)(obj.Spec.Name)
-//						}
-//					}
-//				}
-//			}
-//		}
-//	}


nit: why is this removed?

Good catch — restored. The new sample shows the simpler generated output that uses ackrt.HandleCrossNamespaceReference from runtime#249.

michaelhtm · 2026-05-28T20:48:20Z

+							namespace, isCrossNs, err := ackrt.ValidateCrossNamespaceReference(
+								rm.cfg.EnableCrossNamespace,
+								ko.ObjectMeta.GetNamespace(),
+								arr.Namespace,
+								*arr.Name,
+							)
+							if err != nil {
+								return hasReferences, err
+							}
+							if isCrossNs {
+								ackrtlog.FromContext(ctx).Info("cross-namespace resource reference detected; "+
+									"this behavior will be disabled by default in a future release. "+
+									"Set --enable-cross-namespace to preserve this behavior.",
+									"ownerNamespace", ko.ObjectMeta.GetNamespace(),
+									"targetNamespace", *arr.Namespace,
+									"referenceName", *arr.Name,
+								)
+								crossNsMsg := fmt.Sprintf("Cross-namespace resource reference detected: "+
+									"resource in namespace %q references %q in namespace %q. "+
+									"Cross-namespace behavior will be disabled by default in a future release. "+
+									"Set --enable-cross-namespace=true to preserve this behavior.",
+									ko.ObjectMeta.GetNamespace(), *arr.Name, *arr.Namespace)
+								setCrossNamespaceCondition(ko, crossNsMsg)


can we make this block a helper function? maybe in runtime?

Done — moved into runtime as ackrt.HandleCrossNamespaceReference (see aws-controllers-k8s/runtime#249). The block in generated code collapses from ~14 lines to a single helper call.

michaelhtm · 2026-05-28T20:51:24Z

+
+// setCrossNamespaceCondition sets the ACK.CrossNamespaceOptInRequired condition
+// on the resource to notify users that their cross-namespace usage will require
+// explicit opt-in in a future release.
+func setCrossNamespaceCondition(ko *svcapitypes.{{ .CRD.Names.Camel }}, message string) {
+	if ko.Status.Conditions == nil {
+		ko.Status.Conditions = []*ackv1alpha1.Condition{}
+	}
+	var crossNsCond *ackv1alpha1.Condition
+	for _, c := range ko.Status.Conditions {
+		if c.Type == ackv1alpha1.ConditionTypeCrossNamespaceOptInRequired {
+			crossNsCond = c
+			break
+		}
+	}
+	if crossNsCond == nil {
+		crossNsCond = &ackv1alpha1.Condition{
+			Type: ackv1alpha1.ConditionTypeCrossNamespaceOptInRequired,
+		}
+		ko.Status.Conditions = append(ko.Status.Conditions, crossNsCond)
+	}
+	now := metav1.Now()
+	crossNsCond.LastTransitionTime = &now
+	crossNsCond.Status = corev1.ConditionTrue
+	crossNsCond.Message = &message
+}


nit: this could also be a runtime function, similar to https://github.com/aws-controllers-k8s/runtime/blob/main/pkg/runtime/reconciler.go#L555

Good call — done. The lookup-or-create logic now lives in the runtime as ackrt.SetCrossNamespaceOptInRequired, mirroring the field export reconciler pattern you linked. The per-resource generated function is gone.

michaelhtm · 2026-05-28T20:53:03Z

+{{ if .CRD.HasReferenceFields -}}
+{{ end -}}
+


what does this part do?

Nothing — that was leftover dead code from when setCrossNamespaceCondition was briefly defined here before being moved. Removed it.

- Replace inlined cross-namespace warning + condition logic with calls to the new ackrt.HandleCrossNamespaceReference runtime helper. This collapses ~14 lines of generated code per cross-namespace check into a single helper call. Applies to both resource references (resource_reference.go) and secret references (set_sdk.go). - Remove the per-package setCrossNamespaceCondition function from the sdk.go.tpl template; the runtime helper now owns the lookup-or-create pattern, avoiding 26 lines of generated code per resource. - Drop the now-unused ackrtlog import and var-block from the references.go.tpl template since the runtime helper handles logging. - Remove the empty {{ if .CRD.HasReferenceFields -}}{{ end -}} block at the bottom of references.go.tpl that was left over from the previous refactor. - Restore the third sample output (resolving nested lists of structs containing references) in the ResolveReferencesForField doc comment, updated to reflect the new generated output. - Update set_sdk_test.go and resource_reference_test.go expected outputs to match the simplified generated code. Requires runtime helpers added in aws-controllers-k8s/runtime PR ( HandleCrossNamespaceReference, SetCrossNamespaceOptInRequired, CrossNamespaceRefKindResource, CrossNamespaceRefKindSecret).

michaelhtm · 2026-05-29T19:32:16Z

+//		namespace, isCrossNs, err := ackrt.ValidateCrossNamespaceReference(
+//			rm.cfg.EnableCrossNamespace,
+//			ko.ObjectMeta.GetNamespace(),
+//			arr.Namespace,
+//			*arr.Name,
+//		)
+//		if err != nil {
+//			return hasReferences, err
+//		}
+//		if isCrossNs {
+//			ko.Status.Conditions = ackrt.HandleCrossNamespaceReference(
+//				ctx, ko.Status.Conditions,
+//				ackrt.CrossNamespaceRefKindResource,
+//				ko.ObjectMeta.GetNamespace(), *arr.Namespace, *arr.Name,
+//			)
+//		}


i was hoping this entire section would be inside the helper function..and we won't need to return isCrossNs

sure, moved to the helper function

Address review feedback on aws-controllers-k8s/code-generator#699 to collapse the validate + handle flow into a single helper call so generated code does not need to inspect or branch on the isCrossNamespace return value. - Add ResolveCrossNamespaceReference(*string namespace): orchestrates ValidateCrossNamespaceReference + HandleCrossNamespaceReference, returning only (resolvedNamespace, error) - Add ResolveCrossNamespaceReferenceString(string namespace): the string-namespace counterpart for SecretKeyReference/FieldExportTarget callers - Both accept a *[]*Condition pointer so the conditions slice is updated in place; passing nil is safe and skips condition handling - Add unit tests covering same-namespace, nil/empty namespace, cross namespace with flag enabled, cross namespace with flag disabled, and nil conditions pointer for both helpers

Address review feedback from michaelhtm (aws-controllers-k8s#699) to fold the entire validate + handle flow into a single helper call. The generated code no longer needs to inspect or branch on isCrossNs. - resource_reference.go: replace ValidateCrossNamespaceReference + Handle block with single ResolveCrossNamespaceReference call (3 returns -> 2) - set_sdk.go: same simplification for secret references using ResolveCrossNamespaceReferenceString - Update doc comments in both generators to reflect the new output - Update 7 test expectations in resource_reference_test.go and 7 in set_sdk_test.go Requires runtime helpers added in aws-controllers-k8s/runtime#249 (ResolveCrossNamespaceReference, ResolveCrossNamespaceReferenceString).

Issue #, if available: Description of changes: Extract the lookup-or-create condition pattern and the warning-log + condition combo into shared helpers so generated code can call a single runtime function instead of inlining ~14 lines per cross-namespace check. - Add SetCrossNamespaceOptInRequired(conditions, message): updates or appends the ACK.CrossNamespaceOptInRequired condition using a lookup-or-create pattern (avoids duplicate conditions on repeated reconciles) - Add HandleCrossNamespaceReference(ctx, conditions, refKind, ownerNs, targetNs, refName): emits the structured warning log and sets the condition; intended to be called from generated code after ValidateCrossNamespaceReference reports isCrossNamespace=true - Refactor fieldExportReconciler.setCrossNsOptInRequiredCondition to use SetCrossNamespaceOptInRequired so all condition handling shares one implementation Addresses review feedback on aws-controllers-k8s/code-generator#699. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Point at runtime pseudo-version v0.59.2-0.20260603224255-14be98880374 (commit 14be988, PR aws-controllers-k8s#249) which adds the ResolveCrossNamespaceReference and ResolveCrossNamespaceReferenceString helpers that generated code now calls. This is a temporary pin for testing; revert to the tagged release (v0.60.0) once it is published upstream.

sapphirew · 2026-06-04T00:58:02Z

/retest

michaelhtm · 2026-06-04T18:05:54Z

/retest

ack-prow · 2026-06-04T18:06:51Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: michaelhtm, sapphirew

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [michaelhtm]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

knottnt · 2026-06-04T20:55:46Z

Including this in the generated sdk.go files will get most of the SecretKeyReference usages, but I believe there are cases where Secrets are used in custom update functions. For example, RDS's MasterUserPassword is set in a custom_update.go file and won't be updated automatically. Is there any way we can move this validation to happen in SecretValueFromReference entirely in runtime?

Good call, going to move secret validation into runtime's SecretValueFromReference from the current change in sdk.go.

runtime PR: aws-controllers-k8s/runtime#254

Drop the per-call secret validation block from generated sdk.go. The generated code only wrapped SecretValueFromReference call sites in sdk.go, missing custom update functions (e.g. RDS db_cluster) and hooks that call SecretValueFromReference directly. Validation now lives entirely in the runtime's SecretValueFromReference (aws-controllers-k8s/runtime), so every caller is covered without generated boilerplate. - set_sdk.go: generated secret code reverts to a plain SecretValueFromReference call; no ResolveCrossNamespaceReferenceString - sdk.go.tpl: drop the now-unused ackrt import and its suppression entry - set_sdk_test.go: remove the validation block from 7 expected outputs Addresses review feedback from knottnt on aws-controllers-k8s#699.

…mReference Move cross-namespace validation for secret references into SecretValueFromReference so every caller is covered, including custom update functions (e.g. RDS db_cluster/custom_update.go) and hooks that call it directly rather than going through generated sdk.go code. - When --enable-cross-namespace is false and the secret targets a different namespace, return a terminal error (wrapping the ResourceReferenceCrossNamespaceNotAllowed sentinel). The caller's existing condition machinery sets ACK.Terminal from the message. - When the flag is true, log a deprecation warning and resolve the secret from the target namespace (Phase 1 behavior). - Same-namespace and empty-namespace refs are unaffected. This lets the code-generator drop the per-call secret validation block it currently emits in set_sdk.go, which only covered generated sdk.go call sites. Addresses review feedback on aws-controllers-k8s/code-generator#699.

…mReference Move cross-namespace validation for secret references into SecretValueFromReference so every caller is covered, including custom update functions (e.g. RDS db_cluster/custom_update.go) and hooks that call it directly rather than going through generated sdk.go code. - When --enable-cross-namespace is false and the secret targets a different namespace, return a terminal error (wrapping the ResourceReferenceCrossNamespaceNotAllowed sentinel). The caller's existing condition machinery sets ACK.Terminal from the message. - When the flag is true, log a deprecation warning, resolve the secret from the target namespace, and set the ACK.CrossNamespaceOptInRequired condition on the resource being reconciled. Because SecretValueFromReference does not receive the resource handle, the reconciler stashes the resource (a ConditionManager) in the context via WithConditionManager; SecretValueFromReference retrieves it with ConditionManagerFromContext and sets the condition through SetCrossNamespaceOptInRequiredOnSubject. The condition is set on the desired resource before sdkCreate/sdkUpdate deep-copies it into the returned object, so it persists to the status patch. Passing no ConditionManager is safe (the condition is simply skipped). This lets the code-generator drop the per-call secret validation block it currently emits in set_sdk.go, which only covered generated sdk.go call sites. Addresses review feedback on aws-controllers-k8s/code-generator#699.

ack-prow · 2026-06-05T20:56:26Z

@sapphirew: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ecr-controller-test	`f0dc0d8`	link	true	`/test ecr-controller-test`
s3-controller-test	`f0dc0d8`	link	true	`/test s3-controller-test`
dynamodb-controller-test	`f0dc0d8`	link	true	`/test dynamodb-controller-test`
eks-controller-test	`f0dc0d8`	link	true	`/test eks-controller-test`
crd-compat-check	`f0dc0d8`	link	true	`/test crd-compat-check`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

ack-prow Bot requested review from a-hilaly and knottnt May 8, 2026 22:39

sapphirew changed the title ~~feat: Add cross-namespace reference validation to generated code~~ [Draft]feat: Add cross-namespace reference validation to generated code May 8, 2026

sapphirew force-pushed the cross-namespace-refs branch from 6f2fb42 to 0511503 Compare May 20, 2026 00:35

kprahulraj mentioned this pull request May 21, 2026

docs: add breaking changes page for cross-namespace default change aws-controllers-k8s/docs#37

Open

This was referenced May 22, 2026

feat(runtime): Add --enable-cross-namespace flag aws-controllers-k8s/runtime#239

Closed

feat(runtime): Add --enable-cross-namespace flag aws-controllers-k8s/runtime#248

Merged

knottnt reviewed May 22, 2026

View reviewed changes

sapphirew force-pushed the cross-namespace-refs branch from 0511503 to 30bdd70 Compare May 27, 2026 22:36

sapphirew changed the title ~~[Draft]feat: Add cross-namespace reference validation to generated code~~ feat: Add cross-namespace reference validation to generated code May 27, 2026

sapphirew requested a review from knottnt May 27, 2026 22:55

michaelhtm reviewed May 28, 2026

View reviewed changes

sapphirew mentioned this pull request May 28, 2026

feat(runtime): Add cross-namespace deprecation helpers aws-controllers-k8s/runtime#249

Merged

sapphirew requested a review from michaelhtm May 29, 2026 17:58

michaelhtm reviewed May 29, 2026

View reviewed changes

sapphirew requested a review from michaelhtm May 29, 2026 21:24

michaelhtm approved these changes Jun 4, 2026

View reviewed changes

ack-prow Bot added the approved label Jun 4, 2026

knottnt reviewed Jun 4, 2026

View reviewed changes

sapphirew mentioned this pull request Jun 5, 2026

feat(runtime): Validate cross-namespace secret refs in SecretValueFromReference aws-controllers-k8s/runtime#254

Open

Conversation

sapphirew commented May 8, 2026

Uh oh!

kprahulraj commented May 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sapphirew commented Jun 4, 2026

Uh oh!

michaelhtm commented Jun 4, 2026

Uh oh!

ack-prow Bot commented Jun 4, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ack-prow Bot commented Jun 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants