Skip to content

feat(runtime): Add cross-namespace deprecation helpers#249

Merged
ack-prow[bot] merged 2 commits into
aws-controllers-k8s:mainfrom
sapphirew:cross-namespace-helpers
Jun 3, 2026
Merged

feat(runtime): Add cross-namespace deprecation helpers#249
ack-prow[bot] merged 2 commits into
aws-controllers-k8s:mainfrom
sapphirew:cross-namespace-helpers

Conversation

@sapphirew
Copy link
Copy Markdown
Contributor

@sapphirew sapphirew commented May 28, 2026

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.

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.
@ack-prow ack-prow Bot requested review from knottnt and michaelhtm May 28, 2026 23:52
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
sapphirew added a commit to sapphirew/ack-code-generator that referenced this pull request May 29, 2026
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).
@michaelhtm
Copy link
Copy Markdown
Member

/lgtm

@ack-prow ack-prow Bot added the lgtm Indicates that a PR is ready to be merged. label Jun 3, 2026
@ack-prow
Copy link
Copy Markdown

ack-prow Bot commented Jun 3, 2026

[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:

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

@ack-prow ack-prow Bot added the approved label Jun 3, 2026
@ack-prow ack-prow Bot merged commit 14be988 into aws-controllers-k8s:main Jun 3, 2026
9 checks passed
@sapphirew sapphirew deleted the cross-namespace-helpers branch June 3, 2026 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants