Skip to content

feat: generate EnsureReferences to preserve nested cross-resource references - #738

Open
gustavodiaz7722 wants to merge 4 commits into
aws-controllers-k8s:mainfrom
gustavodiaz7722:feat/ensure-references
Open

gustavodiaz7722 wants to merge 4 commits into
aws-controllers-k8s:mainfrom
gustavodiaz7722:feat/ensure-references

Conversation

@gustavodiaz7722

@gustavodiaz7722 gustavodiaz7722 commented Aug 27, 2026

Copy link
Copy Markdown
Member

Summary

A cross-resource reference (*Ref) is generated as a sibling of the concrete field it resolves into — spec.vpcConfig.subnetRefs next to spec.vpcConfig.subnetIDs. A resource manager builds its return value from an AWS API response, which has no concept of a reference, so rebuilding the containing struct drops every *Ref inside it.

That disables ClearResolvedReferences, which suppresses a resolved value only while it can still see the sibling:

if ko.Spec.VPCConfig != nil {
	if len(ko.Spec.VPCConfig.SubnetRefs) > 0 {   // false once the *Ref is gone
		ko.Spec.VPCConfig.SubnetIDs = nil
	}
}

So the spec patch deletes the declared *Ref and stores the resolved value in its place — what aws-controllers-k8s/community#2431 reports: a declared securityGroupRefs replaced by securityGroupIDs.

Reconciliation continues until the manifest is applied again, from Helm, Argo, Flux or kubectl apply. That apply restores the *Ref beside the now-stored value, and validateReferenceFields rejects the pair:

message: Reference resolution failed
reason: 'both resource reference wrapper and ID cannot be used together:
         VPCConfig.SubnetIDs,VPCConfig.SubnetRefs'

This PR generates an EnsureReferences method that restores the missing reference from the declared resource.

Together with aws-controllers-k8s/runtime#267 this fixes aws-controllers-k8s/community#2431, and addresses the struct-nested half of aws-controllers-k8s/community#2361.

Requires runtime v0.64.0

aws-controllers-k8s/runtime#267 defines the optional ReferenceEnsurer interface and calls the method on what a resource manager returns from Create and from Update. It deliberately does not call it on AdoptionPolicy_Adopt, where replacing a declared spec is the intended behaviour. It is merged and released as v0.64.0.

The go.mod bump to v0.64.0 is included in this PR rather than left as a follow-up. auto-generate-controllers resolves the runtime version from this module's go.mod (cd/auto-generate/auto-generate-controllers.sh:91), not from the latest runtime release, so merging the generator while the pin still read v0.63.0 would open fleet-wide PRs adding a generated EnsureReferences against a runtime that never calls it. The bump is go.mod and go.sum only, with no generated-output changes.

Against an older runtime the method is inert but harmless: it compiles and is simply never invoked, which is the state a controller is in between this merge and its own regeneration. Controllers generated before the method existed are unaffected and opt in by regenerating.

What is emitted

Reference Emitted Why
top-level (spec.xRef) nothing every write path starts from a DeepCopy, and the *Ref is a sibling of the concrete field, so nothing rebuilds it
through structs (spec.a.xRef, spec.a.xRefs) assign the reference it has one fixed address; every value the service reported stands
through a list (spec.l[].xRef) nothing it has no fixed address; out of scope, see below

A reference field that is itself a list (*Refs, whose concrete sibling is a list of scalars) belongs in the struct row: the list is the leaf, not part of the path, so nothing has to be indexed to reach it.

if desiredKO.Spec.VPCConfig != nil {
	if len(desiredKO.Spec.VPCConfig.SecurityGroupRefs) > 0 {
		if latestKO.Spec.VPCConfig == nil {
			latestKO.Spec.VPCConfig = &svcapitypes.VPCConfig{}
		}
		if len(latestKO.Spec.VPCConfig.SecurityGroupRefs) == 0 {
			latestKO.Spec.VPCConfig.SecurityGroupRefs = desiredKO.Spec.VPCConfig.SecurityGroupRefs
		}
	}
	if len(desiredKO.Spec.VPCConfig.SubnetRefs) > 0 {
		if latestKO.Spec.VPCConfig == nil {
			latestKO.Spec.VPCConfig = &svcapitypes.VPCConfig{}
		}
		if len(latestKO.Spec.VPCConfig.SubnetRefs) == 0 {
			latestKO.Spec.VPCConfig.SubnetRefs = desiredKO.Spec.VPCConfig.SubnetRefs
		}
	}
}

Only the reference is written, so nothing the service populated is touched.

Four properties of the shape above are load-bearing.

Each container is materialised on the target. Generated set-output code rebuilds a struct from the response and nils it when the response omits it -- lambda's sdkCreate has } else { ko.Spec.VPCConfig = nil }. Guarding on the container being present on both objects would therefore skip the reference in exactly the case it most needs restoring, and the spec patch would then delete the whole declared block rather than just the reference. Across the controllers, 36 of the 64 struct-nested containers can be nil'd this way inside sdkCreate/sdkUpdate. The hand-maintained hooks named below already materialise the container for this reason.

The source-side guards enclose it. A container is only constructed once the declared resource is known to hold a reference to put in it, so a resource that declares the container but no reference leaves the target untouched and no empty container reaches the patch.

The guards are nested, not concatenated. A single combined condition reached 661 characters on ecs/CapacityProvider's three-level path, which gofmt does not wrap. Nesting also mirrors ClearResolvedReferences, which walks the same paths. This bounds the guards; an assignment line is as long as its two field paths and gofmt wraps neither form, so ecs/CapacityProvider's deepest reference still produces a 210-character one, exactly as the hand-written hooks below already do.

A container's guard is emitted once, however many references sit inside it. The two references in VPCConfig above share one != nil guard, and a container nested inside another is guarded within its parent rather than restating the parent's chain: ecs/CapacityProvider emits one guard tree over its three nested containers instead of four separate chains, 75 lines rather than 87. The materialisation stays per-reference, because it has to sit inside the guard establishing that the source holds a reference to put in the container -- sharing it would need those guards OR'd together, which is the concatenation the nesting exists to avoid. It is a nil-check either way, so repeating it costs nothing.

This codifies an existing pattern

Restoring a nested *Ref from the declared resource is not new — three controllers already hand-maintain this assignment for want of a generated equivalent. eks/cluster:

// templates/hooks/cluster/sdk_create_post_set_output.go.tpl
if desired.ko.Spec.ResourcesVPCConfig.SubnetRefs != nil {
	ko.Spec.ResourcesVPCConfig.SubnetRefs = desired.ko.Spec.ResourcesVPCConfig.SubnetRefs
}
if desired.ko.Spec.ResourcesVPCConfig.SecurityGroupRefs != nil {
	ko.Spec.ResourcesVPCConfig.SecurityGroupRefs = desired.ko.Spec.ResourcesVPCConfig.SecurityGroupRefs
}

lambda/function does the same for VPCConfig, and opensearchservice/domain for VPCOptions — the latter with a comment naming this issue directly:

// To prevent https://github.com/aws-controllers-k8s/community/issues/2431

The generated code is the same assignment with stricter guards: it nil-checks the container on both objects and only writes when the target is actually missing the reference, so it cannot clobber a reference the service did report. What changes is that every controller with struct-nested references gets the behaviour without hand-writing it.

Scope

Classifying each reference by the shape of the path to its *Ref, across the controllers with a generated references.go:

Shape References Resources Controllers This PR
top-level 315 157 53 not needed
struct-nested 117 37 25 fixed
list-nested 38 21 12 unchanged

Testing

Eleven unit tests in pkg/generate/code/resource_reference_test.go: top-level emits nothing, struct-nested single ref, struct-nested list-of-refs, list path emits nothing, a resource mixing struct- and list-nested, indent level, the two rejection paths (a reference within a map, and a model missing an ancestor field), that every container on the path is materialised on the target and only inside the source-side reference guard, that no guard exceeds 120 characters and none concatenates (&&/||), and that a container's guard is emitted once however many references sit inside it.

That last one uses a new eks Cluster fixture carrying both grouping shapes: two references in one container (ResourcesVpcConfig), and a container holding a reference of its own plus a deeper container that holds another (OutpostConfig, OutpostConfig.ControlPlanePlacement). It asserts the exact output and, separately, that each guard appears exactly once, so a reshuffle that reintroduces a duplicate fails on the reason rather than on a diff.

Regenerated lambda-controller, ecs-controller and eks-controller; references.go diffs are purely additive, output is gofmt-clean, and all three build in full. lambda/function restores Code.S3BucketRef, VPCConfig.SecurityGroupRefs and VPCConfig.SubnetRefs — the shape #2431 was filed for.

Verified on a cluster as a three-way comparison, all on current ecs-controller main, using an ecs Service that declares networkConfiguration.awsVPCConfiguration.subnetRefs and .securityGroupRefs -- a two-level struct path:

Generated method Runtime Result
absent published v0.63.0 both refs deleted, resolved subnets/securityGroups written
present published v0.63.0 identical -- the method alone does nothing
present runtime#267 both refs kept, nothing leaked

The middle row is what makes the comparison controlled: it isolates this PR's output from the runtime change, and confirms the method is inert without runtime support. Re-applying the manifest onto the corrupted spec halts the resource on both resource reference wrapper and ID cannot be used together, which is the failure this prevents.

A container mixing forms -- subnetRefs declared alongside a literal securityGroups -- restores only the reference and leaves the literal untouched, with no phantom securityGroupRefs invented.

Not addressed

References reached through a list. No fixed address to assign to, and no sound way to pair an observed element with a declared one: an AWS response need not preserve request order. These behave exactly as they do today.

The existing read-path hooks. lambda/function, eks/cluster and opensearchservice/domain restore these same references by hand in sdk_read_one_post_set_output. Only their create-path halves are subsumed, and the read-path halves must stay: the AdoptionPolicy_Adopt and delete-path spec writes are not covered by runtime#267.

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

@ack-prow
ack-prow Bot requested review from knottnt and michaelhtm August 27, 2026 22:36
gustavodiaz7722 added a commit to gustavodiaz7722/ack-ws-runtime that referenced this pull request Aug 28, 2026
A cross-resource reference (*Ref) is generated as a sibling of the
concrete field it resolves into. A resource manager builds its return
value from an AWS API response, which has no concept of a reference, so
rebuilding the containing struct drops every *Ref inside it.

That disables ClearResolvedReferences, which suppresses a resolved value
only while the sibling *Ref is visible, so the spec patch deletes the
declared *Ref and stores the resolved value in its place. The next apply
of the manifest puts the *Ref back beside that value, a pair
validateReferenceFields rejects, stopping reconciliation.

Add an optional ReferenceEnsurer interface and invoke it on the object a
resource manager hands back from Create and from Update, sourcing the
references from the declared resource. It is kept separate from
ReferenceManager and reached through a type assertion, so controllers
generated before the method existed still satisfy AWSResourceManager and
compile unchanged; they opt in by regenerating.

The source is `desired`, not `reconcileDesired`: the latter is handed to
Update, and a manager may mutate what it is given, so it is not a
reliable record of what the user declared. The restoration is not hooked
into patchResourceMetadataAndSpec because the late-initialization patch
uses the AWS-observed object as its base, which carries no references.

Pairs with aws-controllers-k8s/code-generator#738, which generates the
method.

Issue aws-controllers-k8s/community#2361
Issue aws-controllers-k8s/community#2431
gustavodiaz7722 added a commit to gustavodiaz7722/ack-ws-runtime that referenced this pull request Aug 28, 2026
A cross-resource reference (*Ref) is generated as a sibling of the
concrete field it resolves into. A resource manager builds its return
value from an AWS API response, which has no concept of a reference, so
rebuilding the containing struct drops every *Ref inside it.

That disables ClearResolvedReferences, which suppresses a resolved value
only while the sibling *Ref is visible, so the spec patch deletes the
declared *Ref and stores the resolved value in its place. The next apply
of the manifest puts the *Ref back beside that value, a pair
validateReferenceFields rejects, stopping reconciliation.

Add an optional ReferenceEnsurer interface and invoke it on the object a
resource manager hands back from Create and from Update, sourcing the
references from the declared resource. It is kept separate from
ReferenceManager and reached through a type assertion, so controllers
generated before the method existed still satisfy AWSResourceManager and
compile unchanged; they opt in by regenerating.

The source is `desired`, not `reconcileDesired`: the latter is handed to
Update, and a manager may mutate what it is given, so it is not a
reliable record of what the user declared. The restoration is not hooked
into patchResourceMetadataAndSpec because the late-initialization patch
uses the AWS-observed object as its base, which carries no references.

Pairs with aws-controllers-k8s/code-generator#738, which generates the
method.

Issue aws-controllers-k8s/community#2361
Issue aws-controllers-k8s/community#2431
@gustavodiaz7722

Copy link
Copy Markdown
Member Author

/retest

1 similar comment
@gustavodiaz7722

Copy link
Copy Markdown
Member Author

/retest

Comment thread pkg/generate/code/resource_reference.go Outdated
Comment thread pkg/generate/code/resource_reference.go Outdated
Comment thread pkg/generate/code/resource_reference.go Outdated
Comment thread pkg/generate/code/resource_reference.go Outdated
@knottnt

knottnt commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

@gustavodiaz7722 it would also be help if you could link a draft PR that shows an example of the generated code in the context of a full service controller.

gustavodiaz7722 added a commit to gustavodiaz7722/ack-ws-runtime that referenced this pull request Sep 2, 2026
A cross-resource reference (*Ref) is generated as a sibling of the
concrete field it resolves into. A resource manager builds its return
value from an AWS API response, which has no concept of a reference, so
rebuilding the containing struct drops every *Ref inside it.

That disables ClearResolvedReferences, which suppresses a resolved value
only while the sibling *Ref is visible, so the spec patch deletes the
declared *Ref and stores the resolved value in its place. The next apply
of the manifest puts the *Ref back beside that value, a pair
validateReferenceFields rejects, stopping reconciliation.

Add an optional ReferenceEnsurer interface and invoke it on the object a
resource manager hands back from Create and from Update, sourcing the
references from the declared resource. It is kept separate from
ReferenceManager and reached through a type assertion, so controllers
generated before the method existed still satisfy AWSResourceManager and
compile unchanged; they opt in by regenerating.

The source is `desired`, not `reconcileDesired`: the latter is handed to
Update, and a manager may mutate what it is given, so it is not a
reliable record of what the user declared. The restoration is not hooked
into patchResourceMetadataAndSpec because the late-initialization patch
uses the AWS-observed object as its base, which carries no references.

Which shapes are covered is a property of the generated method rather
than of this interface.

Pairs with aws-controllers-k8s/code-generator#738, which generates the
method.

Issue aws-controllers-k8s/community#2431
Issue aws-controllers-k8s/community#2361
@gustavodiaz7722

Copy link
Copy Markdown
Member Author

@knottnt Created a demo here aws-controllers-k8s/lambda-controller#242

gustavodiaz7722 added a commit to gustavodiaz7722/ack-ws-runtime that referenced this pull request Sep 2, 2026
A cross-resource reference (*Ref) is generated as a sibling of the
concrete field it resolves into. A resource manager builds its return
value from an AWS API response, which has no concept of a reference, so
rebuilding the containing struct drops every *Ref inside it.

That disables ClearResolvedReferences, which suppresses a resolved value
only while the sibling *Ref is visible, so the spec patch deletes the
declared *Ref and stores the resolved value in its place. The next apply
of the manifest puts the *Ref back beside that value, a pair
validateReferenceFields rejects, stopping reconciliation.

Add an optional ReferenceEnsurer interface and invoke it on the object a
resource manager hands back from Create and from Update, sourcing the
references from the declared resource. It is kept separate from
ReferenceManager and reached through a type assertion, so controllers
generated before the method existed still satisfy AWSResourceManager and
compile unchanged; they opt in by regenerating.

On both paths the source has to be a record of what the user declared,
which neither argument to the resource manager reliably is. Update is
given reconcileDesired, a copy the manager may mutate. Create is given
`desired` itself, and generated sdkCreate only deep-copies it after the
point where a custom_implementation returns or a
sdk_create_pre_build_request hook runs, so either can mutate it. Update
therefore sources from `desired` and Create from a snapshot taken before
the call.

The restoration is not hooked into patchResourceMetadataAndSpec because
the late-initialization patch uses the AWS-observed object as its base,
which carries no references.

Which shapes are covered is a property of the generated method rather
than of this interface.

Pairs with aws-controllers-k8s/code-generator#738, which generates the
method.

Issue aws-controllers-k8s/community#2431
Issue aws-controllers-k8s/community#2361
gustavodiaz7722 added a commit to gustavodiaz7722/ack-ws-runtime that referenced this pull request Sep 2, 2026
A cross-resource reference (*Ref) is generated as a sibling of the
concrete field it resolves into. A resource manager builds its return
value from an AWS API response, which has no concept of a reference, so
rebuilding the containing struct drops every *Ref inside it.

That disables ClearResolvedReferences, which suppresses a resolved value
only while the sibling *Ref is visible, so the spec patch deletes the
declared *Ref and stores the resolved value in its place. The next apply
of the manifest puts the *Ref back beside that value, a pair
validateReferenceFields rejects, stopping reconciliation.

Add an optional ReferenceEnsurer interface and invoke it on the object a
resource manager hands back from Create and from Update, sourcing the
references from the declared resource. It is kept separate from
ReferenceManager and reached through a type assertion, so controllers
generated before the method existed still satisfy AWSResourceManager and
compile unchanged; they opt in by regenerating.

Both paths now hand the resource manager a copy and keep `desired` as the
reference source. Update already did this with reconcileDesired; Create
was passing `desired` itself, and generated sdkCreate only deep-copies
the resource it is given partway through -- a custom_implementation
returns before that point and a sdk_create_pre_build_request hook runs
before it -- so either could mutate what the user declared. The copy is
taken after setResourceManaged and EnsureTags so it carries the finalizer
and controller tags.

The restoration runs before the error from Create or Update is inspected.
A resource manager may hand back a non-nil resource alongside a requeue
error while an asynchronous operation is in flight, and many do; that
object reaches the caller either way, so it should carry the declared
references on every path.

The restoration is not hooked into patchResourceMetadataAndSpec because
the late-initialization patch uses the AWS-observed object as its base,
which carries no references.

Which shapes are covered is a property of the generated method rather
than of this interface.

Pairs with aws-controllers-k8s/code-generator#738, which generates the
method.

Issue aws-controllers-k8s/community#2431
Issue aws-controllers-k8s/community#2361
@ack-prow ack-prow Bot added the approved label Sep 11, 2026
@gustavodiaz7722 gustavodiaz7722 added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 14, 2026
gustavodiaz7722 added a commit to gustavodiaz7722/ack-ws-runtime that referenced this pull request Sep 14, 2026
A cross-resource reference (*Ref) is generated as a sibling of the
concrete field it resolves into. A resource manager builds its return
value from an AWS API response, which has no concept of a reference, so
rebuilding the containing struct drops every *Ref inside it.

That disables ClearResolvedReferences, which suppresses a resolved value
only while the sibling *Ref is visible, so the spec patch deletes the
declared *Ref and stores the resolved value in its place. The next apply
of the manifest puts the *Ref back beside that value, a pair
validateReferenceFields rejects, stopping reconciliation.

Add an optional ReferenceEnsurer interface and invoke it on the object a
resource manager hands back from Create and from Update, sourcing the
references from the declared resource. It is kept separate from
ReferenceManager and reached through a type assertion, so controllers
generated before the method existed still satisfy AWSResourceManager and
compile unchanged; they opt in by regenerating.

Both paths now hand the resource manager a copy and keep `desired` as the
reference source. Update already did this with reconcileDesired; Create
was passing `desired` itself, and generated sdkCreate only deep-copies
the resource it is given partway through -- a custom_implementation
returns before that point and a sdk_create_pre_build_request hook runs
before it -- so either could mutate what the user declared. The copy is
taken after setResourceManaged and EnsureTags so it carries the finalizer
and controller tags.

The restoration runs before the error from Create or Update is inspected.
A resource manager may hand back a non-nil resource alongside a requeue
error while an asynchronous operation is in flight, and many do; that
object reaches the caller either way, so it should carry the declared
references on every path.

The restoration is not hooked into patchResourceMetadataAndSpec because
the late-initialization patch uses the AWS-observed object as its base,
which carries no references.

Which shapes are covered is a property of the generated method rather
than of this interface.

Pairs with aws-controllers-k8s/code-generator#738, which generates the
method.

Issue aws-controllers-k8s/community#2431
Issue aws-controllers-k8s/community#2361
gustavodiaz7722 added a commit to gustavodiaz7722/ack-ws-lambda-controller that referenced this pull request Sep 14, 2026
Demonstration only, not for merge. Shows what
aws-controllers-k8s/code-generator#738 emits in the context of a full
service controller.

Regenerated with no other change, so the diff is exactly the generated
EnsureReferences methods. go.mod is untouched: the method compiles
against the current runtime and stays inert until
aws-controllers-k8s/runtime#267 lands, which is what invokes it.

lambda covers all three reference shapes, so the per-shape behaviour is
visible in one controller:

  function       struct-nested   Code.S3BucketRef, VPCConfig.SecurityGroupRefs,
                                 VPCConfig.SubnetRefs -> emitted
  layer_version  struct-nested   emitted
  event_source_mapping           list-nested -> nothing emitted
  alias, code_signing_config, function_url_config, version
                                 top-level only -> nothing emitted

function is the shape reported in
aws-controllers-k8s/community#2431.
@gustavodiaz7722 gustavodiaz7722 removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 14, 2026
gustavodiaz7722 added a commit to gustavodiaz7722/ack-ws-runtime that referenced this pull request Sep 14, 2026
A cross-resource reference (*Ref) is generated as a sibling of the
concrete field it resolves into. A resource manager builds its return
value from an AWS API response, which has no concept of a reference, so
rebuilding the containing struct drops every *Ref inside it.

That disables ClearResolvedReferences, which suppresses a resolved value
only while the sibling *Ref is visible, so the spec patch deletes the
declared *Ref and stores the resolved value in its place. The next apply
of the manifest puts the *Ref back beside that value, a pair
validateReferenceFields rejects, stopping reconciliation.

Add an optional ReferenceEnsurer interface and invoke it at the three
spec-patch sites whose base is the declared resource: after Create, after
Update, and on the adoption branch of Sync. It is kept separate from
ReferenceManager and reached through a type assertion, so controllers
generated before the method existed still satisfy AWSResourceManager and
compile unchanged; they opt in by regenerating.

The restoration is not hooked into patchResourceMetadataAndSpec itself
because the late-initialization patch uses the AWS-observed object as its
base, which carries no references. deleteResource patches from a
ReadOne-derived object too and is deliberately left out: the CR is removed
immediately afterwards, so the write is never observed.

Both the Create and Update paths hand the resource manager a copy and keep
`desired` as the reference source. Update already did this with
reconcileDesired; Create was passing `desired` itself, and generated
sdkCreate only deep-copies the resource it is given partway through -- a
custom_implementation returns before that point and a
sdk_create_pre_build_request hook runs before it -- so either could mutate
what the user declared. The copy is taken after setResourceManaged and
EnsureTags so it carries the finalizer and controller tags.

The restoration runs before the error from Create or Update is inspected.
A resource manager may hand back a non-nil resource alongside a requeue
error while an asynchronous operation is in flight, and many do; that
object reaches the caller either way, so it should carry the declared
references on every path.

On the adoption branch the source is `desired` rather than `resolved`,
because `desired` is the patch base there and an unresolved object is a
valid source: a *Ref is user-declared and resolution only fills the
concrete sibling.

Which shapes are covered is a property of the generated method rather
than of this interface.

Pairs with aws-controllers-k8s/code-generator#738, which generates the
method.

Issue aws-controllers-k8s/community#2431
Issue aws-controllers-k8s/community#2361
gustavodiaz7722 added a commit to gustavodiaz7722/ack-ws-runtime that referenced this pull request Sep 14, 2026
A cross-resource reference (*Ref) is generated as a sibling of the
concrete field it resolves into. A resource manager builds its return
value from an AWS API response, which has no concept of a reference, so
rebuilding the containing struct drops every *Ref inside it.

That disables ClearResolvedReferences, which suppresses a resolved value
only while the sibling *Ref is visible, so the spec patch deletes the
declared *Ref and stores the resolved value in its place. The next apply
of the manifest puts the *Ref back beside that value, a pair
validateReferenceFields rejects, stopping reconciliation.

Add an optional ReferenceEnsurer interface and invoke it from
patchResourceMetadataAndSpec, the one point every spec write passes
through, sourcing the references from that patch's base.

Sourcing from the base is what makes a single call cover every path.
Where the base is the declared resource -- create, update, the adoption
branch of Sync, and deleteResource -- it carries the references and they
are restored. Where it is not, as on the late-initialization patch whose
base is the AWS-observed object, the base carries none and the call is
inert: it only ever writes a reference the source actually holds. A new
spec-patch site gets this for free rather than having to remember it.

The alternative, calling it on each resource manager's return value,
needed the source chosen correctly at three separate sites and would also
restore references on the error paths, where the object is handed back
with only its status patched and a restored reference could never reach
the spec.

The interface is kept separate from ReferenceManager and reached through a
type assertion, so controllers generated before the method existed still
satisfy AWSResourceManager and compile unchanged; they opt in by
regenerating.

Independently of the above, Create is now handed a copy of `desired`
rather than `desired` itself. Generated sdkCreate only deep-copies the
resource it is given partway through -- a custom_implementation returns
before that point and a sdk_create_pre_build_request hook runs before it
-- so either could mutate what the user declared, and `desired` is the
patch base. Update already took a copy for the same reason. The copy is
taken after setResourceManaged and EnsureTags so it carries the finalizer
and the controller tags.

Which shapes are covered is a property of the generated method rather
than of this interface.

Pairs with aws-controllers-k8s/code-generator#738, which generates the
method.

Issue aws-controllers-k8s/community#2431
Issue aws-controllers-k8s/community#2361
gustavodiaz7722 added a commit to gustavodiaz7722/ack-ws-runtime that referenced this pull request Sep 14, 2026
A cross-resource reference (*Ref) is generated as a sibling of the
concrete field it resolves into. A resource manager builds its return
value from an AWS API response, which has no concept of a reference, so
rebuilding the containing struct drops every *Ref inside it.

That disables ClearResolvedReferences, which suppresses a resolved value
only while the sibling *Ref is visible, so the spec patch deletes the
declared *Ref and stores the resolved value in its place. The next apply
of the manifest puts the *Ref back beside that value, a pair
validateReferenceFields rejects, stopping reconciliation.

Add an optional ReferenceEnsurer interface and invoke it from
patchResourceMetadataAndSpec, the one point every spec write passes
through, sourcing the references from that patch's base.

Sourcing from the base is what makes a single call cover every path.
Where the base is the declared resource -- create, update, the adoption
branch of Sync, and deleteResource -- it carries the references and they
are restored. Where it is not, as on the late-initialization patch whose
base is the AWS-observed object, the base carries none and the call is
inert: it only ever writes a reference the source actually holds. A new
spec-patch site gets this for free rather than having to remember it.

The alternative, calling it on each resource manager's return value,
needed the source chosen correctly at three separate sites and would also
restore references on the error paths, where the object is handed back
with only its status patched and a restored reference could never reach
the spec.

The interface is kept separate from ReferenceManager and reached through a
type assertion, so controllers generated before the method existed still
satisfy AWSResourceManager and compile unchanged; they opt in by
regenerating.

Independently of the above, Create is now handed a copy of `desired`
rather than `desired` itself. Generated sdkCreate only deep-copies the
resource it is given partway through -- a custom_implementation returns
before that point and a sdk_create_pre_build_request hook runs before it
-- so either could mutate what the user declared, and `desired` is the
patch base. Update already took a copy for the same reason. The copy is
taken after setResourceManaged and EnsureTags so it carries the finalizer
and the controller tags.

Which shapes are covered is a property of the generated method rather
than of this interface.

Pairs with aws-controllers-k8s/code-generator#738, which generates the
method.

Issue aws-controllers-k8s/community#2431
Issue aws-controllers-k8s/community#2361
gustavodiaz7722 added a commit to gustavodiaz7722/ack-ws-lambda-controller that referenced this pull request Sep 14, 2026
Demonstration only, not for merge. Shows what
aws-controllers-k8s/code-generator#738 emits in the context of a full
service controller.

Regenerated with no other change, so the diff is exactly the generated
EnsureReferences methods. go.mod is untouched: the method compiles
against the current runtime and stays inert until
aws-controllers-k8s/runtime#267 lands, which is what invokes it.

lambda covers all three reference shapes, so the per-shape behaviour is
visible in one controller:

  function       struct-nested   Code.S3BucketRef, VPCConfig.SecurityGroupRefs,
                                 VPCConfig.SubnetRefs -> emitted
  layer_version  struct-nested   emitted
  event_source_mapping           list-nested -> nothing emitted
  alias, code_signing_config, function_url_config, version
                                 top-level only -> nothing emitted

function is the shape reported in
aws-controllers-k8s/community#2431.
gustavodiaz7722 added a commit to gustavodiaz7722/ack-ws-runtime that referenced this pull request Sep 14, 2026
A cross-resource reference (*Ref) is generated as a sibling of the
concrete field it resolves into. A resource manager builds its return
value from an AWS API response, which has no concept of a reference, so
rebuilding the containing struct drops every *Ref inside it.

That disables ClearResolvedReferences, which suppresses a resolved value
only while the sibling *Ref is visible, so the spec patch deletes the
declared *Ref and stores the resolved value in its place. The next apply
of the manifest puts the *Ref back beside that value, a pair
validateReferenceFields rejects, stopping reconciliation.

Add an optional ReferenceEnsurer interface and invoke it on what a
resource manager returns from Create and from Update -- the two paths
where the object about to be patched back was rebuilt from an API
response while the patch base is still the resource the user declared.
The source is `desired`, not the copy handed to the manager, because a
manager may mutate what it is given: apigateway's ApiKey sdkUpdate
assigns desired.ko.Spec.StageKeys straight from the response.

The restoration runs before the error from Create or Update is inspected.
A resource manager may hand back a non-nil resource alongside a requeue
error while an asynchronous operation is in flight, and many do; that
object reaches the caller either way.

Deliberately not applied to AdoptionPolicy_Adopt. Under that policy the
spec is populated from the observed AWS resource, so a declared spec is
expected to be replaced rather than preserved, and a declared *Ref is
replaced along with every other declared field. Restoring it would make
the reference the one exception.
AdoptionPolicy_AdoptOrCreate does keep a declared reference, for a
structural reason rather than because the restoration runs: that branch
marks the resource managed and adopted and requeues, and that patch's
base is a DeepCopy of its own target, so nothing ever patches the spec
with the declared resource as the base. Both halves are asserted, because
moving the restoration into patchResourceMetadataAndSpec would pick up the
adopt branch automatically and silently change it.

Also not applied to the late-initialization patch, whose base is the
AWS-observed object and carries no references, nor in deleteResource,
where the CR is removed immediately afterwards and the write is never
observed.

Independently of the above, Create is now handed a copy of `desired`
rather than `desired` itself. Generated sdkCreate only deep-copies the
resource it is given partway through -- a custom_implementation returns
before that point and a sdk_create_pre_build_request hook runs before it
-- so either could mutate what the user declared, and `desired` is both
the patch base and the reference source. Update already took a copy for
the same reason. The copy is taken after setResourceManaged and
EnsureTags so it carries the finalizer and the controller tags.

Which shapes are covered is a property of the generated method rather
than of this interface.

Pairs with aws-controllers-k8s/code-generator#738, which generates the
method.

Issue aws-controllers-k8s/community#2431
Issue aws-controllers-k8s/community#2361
A cross-resource reference (*Ref) is generated as a sibling of the
concrete field it resolves into. A resource manager builds its return
value from an AWS API response, which has no concept of a reference, so
rebuilding the containing struct drops every *Ref inside it.

That disables ClearResolvedReferences, which suppresses a resolved value
only while the sibling *Ref is visible, so the spec patch deletes the
declared *Ref and stores the resolved value in its place. The next apply
of the manifest puts the *Ref back beside that value, a pair
validateReferenceFields rejects, stopping reconciliation.

Generate an EnsureReferences method that restores such a reference from
the declared resource. Only a reference reached through structs is
emitted, at its one fixed address, so every value the service reported
stands. A top-level *Ref is skipped because it cannot be lost. One
reached through a list is also skipped and behaves as it does today:
neither position nor resolved value is a sound key for pairing an
observed element with a declared one.

This codifies a pattern eks/cluster, lambda/function and
opensearchservice/domain already hand-maintain in set-output hooks, with
stricter guards.

Requires the runtime's optional ReferenceEnsurer interface, which invokes
the method after Create and after Update. Controllers generated before
the method existed are unaffected and opt in by regenerating.

Issue aws-controllers-k8s/community#2431
Issue aws-controllers-k8s/community#2361
gustavodiaz7722 added a commit to gustavodiaz7722/ack-ws-runtime that referenced this pull request Sep 14, 2026
A cross-resource reference (*Ref) is generated as a sibling of the
concrete field it resolves into. A resource manager builds its return
value from an AWS API response, which has no concept of a reference, so
rebuilding the containing struct drops every *Ref inside it.

That disables ClearResolvedReferences, which suppresses a resolved value
only while the sibling *Ref is visible, so the spec patch deletes the
declared *Ref and stores the resolved value in its place. The next apply
of the manifest puts the *Ref back beside that value, a pair
validateReferenceFields rejects, stopping reconciliation.

Add an optional ReferenceEnsurer interface and invoke it on what a
resource manager returns from Create and from Update -- the two paths
where the object about to be patched back was rebuilt from an API
response while the patch base is still the resource the user declared.
The source is `desired`, not the copy handed to the manager, because a
manager may mutate what it is given: apigateway's ApiKey sdkUpdate
assigns desired.ko.Spec.StageKeys straight from the response.

The restoration runs before the error from Create or Update is inspected.
A resource manager may hand back a non-nil resource alongside a requeue
error while an asynchronous operation is in flight, and many do; that
object reaches the caller either way.

Deliberately not applied to AdoptionPolicy_Adopt. Under that policy the
spec is populated from the observed AWS resource, so a declared spec is
expected to be replaced rather than preserved, and a declared *Ref is
replaced along with every other declared field. Restoring it would make
the reference the one exception.
AdoptionPolicy_AdoptOrCreate does keep a declared reference, for a
structural reason rather than because the restoration runs: that branch
marks the resource managed and adopted and requeues, and that patch's
base is a DeepCopy of its own target, so nothing ever patches the spec
with the declared resource as the base. Both halves are asserted, because
moving the restoration into patchResourceMetadataAndSpec would pick up the
adopt branch automatically and silently change it.

Also not applied to the late-initialization patch, whose base is the
AWS-observed object and carries no references, nor in deleteResource,
where the CR is removed immediately afterwards and the write is never
observed.

Independently of the above, Create is now handed a copy of `desired`
rather than `desired` itself. Generated sdkCreate only deep-copies the
resource it is given partway through -- a custom_implementation returns
before that point and a sdk_create_pre_build_request hook runs before it
-- so either could mutate what the user declared, and `desired` is both
the patch base and the reference source. Update already took a copy for
the same reason. The copy is taken after setResourceManaged and
EnsureTags so it carries the finalizer and the controller tags.

Which shapes are covered is a property of the generated method rather
than of this interface.

Pairs with aws-controllers-k8s/code-generator#738, which generates the
method.

Issue aws-controllers-k8s/community#2431
Issue aws-controllers-k8s/community#2361
gustavodiaz7722 added a commit to gustavodiaz7722/ack-ws-lambda-controller that referenced this pull request Sep 14, 2026
Demonstration only, not for merge. Shows what
aws-controllers-k8s/code-generator#738 emits in the context of a full
service controller.

Regenerated with no other change, so the diff is exactly the generated
EnsureReferences methods. go.mod is untouched: the method compiles
against the current runtime and stays inert until
aws-controllers-k8s/runtime#267 lands, which is what invokes it.

lambda covers all three reference shapes, so the per-shape behaviour is
visible in one controller:

  function       struct-nested   Code.S3BucketRef, VPCConfig.SecurityGroupRefs,
                                 VPCConfig.SubnetRefs -> emitted
  layer_version  struct-nested   emitted
  event_source_mapping           list-nested -> nothing emitted
  alias, code_signing_config, function_url_config, version
                                 top-level only -> nothing emitted

function is the shape reported in
aws-controllers-k8s/community#2431.

@knottnt knottnt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gustavodiaz7722 holding off on the lgtm until aws-controllers-k8s/runtime#267 is merged

@ack-prow

ack-prow Bot commented Sep 15, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gustavodiaz7722, knottnt

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

gustavodiaz7722 added a commit to gustavodiaz7722/ack-ws-runtime that referenced this pull request Sep 15, 2026
A cross-resource reference (*Ref) is generated as a sibling of the
concrete field it resolves into. A resource manager builds its return
value from an AWS API response, which has no concept of a reference, so
rebuilding the containing struct drops every *Ref inside it.

That disables ClearResolvedReferences, which suppresses a resolved value
only while the sibling *Ref is visible, so the spec patch deletes the
declared *Ref and stores the resolved value in its place. The next apply
of the manifest puts the *Ref back beside that value, a pair
validateReferenceFields rejects, stopping reconciliation.

Add an optional ReferenceEnsurer interface and invoke it on what a
resource manager returns from Create and from Update. The source is the
resource the user declared, not the copy handed to the manager, because a
manager may mutate what it is given: apigateway's ApiKey sdkUpdate assigns
desired.ko.Spec.StageKeys straight from the response.

The restoration runs before the error from Create or Update is inspected.
A resource manager may hand back a non-nil resource alongside a requeue
error while an asynchronous operation is in flight, and many do; that
object reaches the caller either way.

Extract callUpdate so that both rm.Update sites go through one funnel. The
reconciler calls rm.Update from two places far apart -- updateResource and
preDeleteSync, which pushes fields like DeletionProtectionEnabled before
the Delete call -- and both feed their result into a spec patch, so both
need the restoration. Its result is the input to rm.Delete and then the
target of a patch based on the stored CR, so a *Ref dropped there is
written out of the spec exactly as on the normal update path. Keeping the
declared resource and the object handed to Update as separate parameters
makes the choice of reference source a property of the signature rather
than of a comment.

Deliberately not applied to AdoptionPolicy_Adopt. Under that policy the
spec is populated from the observed AWS resource, so a declared spec is
expected to be replaced rather than preserved, and a declared *Ref is
replaced along with every other declared field. Restoring it would make
the reference the one exception.
AdoptionPolicy_AdoptOrCreate does keep a declared reference, for a
structural reason rather than because the restoration runs: that branch
marks the resource managed and adopted and requeues, and that patch's base
is a DeepCopy of its own target, so nothing ever patches the spec with the
declared resource as the base. Both halves are asserted, because moving
the restoration into patchResourceMetadataAndSpec would pick up the adopt
branch automatically and silently change it.

Also not applied to the late-initialization patch, whose base is the
AWS-observed object and carries no references.

Independently of the above, Create is now handed a copy of `desired`
rather than `desired` itself. Generated sdkCreate only deep-copies the
resource it is given partway through -- a custom_implementation returns
before that point and a sdk_create_pre_build_request hook runs before it
-- so either could mutate what the user declared, and `desired` is both
the patch base and the reference source. Update already took a copy for
the same reason. The copy is taken after setResourceManaged and
EnsureTags so it carries the finalizer and the controller tags.

Which shapes are covered is a property of the generated method rather
than of this interface.

Pairs with aws-controllers-k8s/code-generator#738, which generates the
method.

Issue aws-controllers-k8s/community#2431
Issue aws-controllers-k8s/community#2361
gustavodiaz7722 added a commit to gustavodiaz7722/ack-ws-runtime that referenced this pull request Sep 15, 2026
A cross-resource reference (*Ref) is generated as a sibling of the
concrete field it resolves into. A resource manager builds its return
value from an AWS API response, which has no concept of a reference, so
rebuilding the containing struct drops every *Ref inside it.

That disables ClearResolvedReferences, which suppresses a resolved value
only while the sibling *Ref is visible, so the spec patch deletes the
declared *Ref and stores the resolved value in its place. The next apply
of the manifest puts the *Ref back beside that value, a pair
validateReferenceFields rejects, stopping reconciliation.

Add an optional ReferenceEnsurer interface and invoke it on what a
resource manager returns from Create and from Update. The source is the
resource the user declared, not the copy handed to the manager, because a
manager may mutate what it is given: apigateway's ApiKey sdkUpdate assigns
desired.ko.Spec.StageKeys straight from the response.

The restoration runs before the error from Create or Update is inspected.
A resource manager may hand back a non-nil resource alongside a requeue
error while an asynchronous operation is in flight, and many do; that
object reaches the caller either way.

Extract callUpdate so that both rm.Update sites go through one funnel. The
reconciler calls rm.Update from two places far apart -- updateResource and
preDeleteSync, which pushes fields like DeletionProtectionEnabled before
the Delete call -- and both feed their result into a spec patch, so both
need the restoration. Its result is the input to rm.Delete and then the
target of a patch based on the stored CR, so a *Ref dropped there is
written out of the spec exactly as on the normal update path. Keeping the
declared resource and the object handed to Update as separate parameters
makes the choice of reference source a property of the signature rather
than of a comment.

Deliberately not applied to AdoptionPolicy_Adopt. Under that policy the
spec is populated from the observed AWS resource, so a declared spec is
expected to be replaced rather than preserved, and a declared *Ref is
replaced along with every other declared field. Restoring it would make
the reference the one exception.
AdoptionPolicy_AdoptOrCreate does keep a declared reference, for a
structural reason rather than because the restoration runs: that branch
marks the resource managed and adopted and requeues, and that patch's base
is a DeepCopy of its own target, so nothing ever patches the spec with the
declared resource as the base. Both halves are asserted, because moving
the restoration into patchResourceMetadataAndSpec would pick up the adopt
branch automatically and silently change it.

Also not applied to the late-initialization patch, whose base is the
AWS-observed object and carries no references.

Independently of the above, Create is now handed a copy of `desired`
rather than `desired` itself. Generated sdkCreate only deep-copies the
resource it is given partway through -- a custom_implementation returns
before that point and a sdk_create_pre_build_request hook runs before it
-- so either could mutate what the user declared, and `desired` is both
the patch base and the reference source. Update already took a copy for
the same reason. The copy is taken after setResourceManaged and
EnsureTags so it carries the finalizer and the controller tags.

Which shapes are covered is a property of the generated method rather
than of this interface.

Pairs with aws-controllers-k8s/code-generator#738, which generates the
method.

Issue aws-controllers-k8s/community#2431
Issue aws-controllers-k8s/community#2361
gustavodiaz7722 added a commit to gustavodiaz7722/ack-ws-runtime that referenced this pull request Sep 15, 2026
A cross-resource reference (*Ref) is generated as a sibling of the
concrete field it resolves into. A resource manager builds its return
value from an AWS API response, which has no concept of a reference, so
rebuilding the containing struct drops every *Ref inside it.

That disables ClearResolvedReferences, which suppresses a resolved value
only while the sibling *Ref is visible, so the spec patch deletes the
declared *Ref and stores the resolved value in its place. The next apply
of the manifest puts the *Ref back beside that value, a pair
validateReferenceFields rejects, stopping reconciliation.

Add an optional ReferenceEnsurer interface and invoke it at the three
points where a resource manager hands back an object whose spec is about
to be patched: after Create, after Update, and after the out-of-band
Update in preDeleteSync, which pushes fields like
DeletionProtectionEnabled before the Delete call. That last one does not
go through updateResource, so it needs the restoration repeated; its
result becomes the input to rm.Delete and then the target of a spec patch
based on the stored CR, so a *Ref dropped there is written out of the spec
exactly as on the normal update path.

The source is always the resource the user declared, never the object
handed to the manager. On the update path that object is a transformed copy
-- applyIgnoredFields merges AWS-observed values into it, and a manager may
mutate it further, as apigateway's ApiKey sdkUpdate does by assigning
desired.ko.Spec.StageKeys from the response. In preDeleteSync it is built
from the ReadOne result, so for a nested *Ref it carries nothing to restore
at all.

On the Create and Update paths the restoration runs before the error is
inspected, because a manager may hand back a resource alongside a requeue
error while an asynchronous operation is in flight and that object reaches
the caller either way. preDeleteSync discards its result on the error path,
so there it runs after.

Deliberately not applied to AdoptionPolicy_Adopt. Under that policy the
spec is populated from the observed AWS resource, so a declared spec is
expected to be replaced rather than preserved, and a declared *Ref is
replaced along with every other declared field. Restoring it would make
the reference the one exception.
AdoptionPolicy_AdoptOrCreate does keep a declared reference, for a
structural reason rather than because the restoration runs: that branch
marks the resource managed and adopted and requeues, and that patch's base
is a DeepCopy of its own target, so nothing ever patches the spec with the
declared resource as the base. Both halves are asserted, because moving
the restoration into patchResourceMetadataAndSpec would pick up the adopt
branch automatically and silently change it.

Also not applied to the late-initialization patch, whose base is the
AWS-observed object and carries no references.

Independently of the above, Create is now handed a copy of `desired`
rather than `desired` itself. Generated sdkCreate only deep-copies the
resource it is given partway through -- a custom_implementation returns
before that point and a sdk_create_pre_build_request hook runs before it
-- so either could mutate what the user declared, and `desired` is both
the patch base and the reference source. Update already took a copy for
the same reason. The copy is taken after setResourceManaged and
EnsureTags so it carries the finalizer and the controller tags.

Which shapes are covered is a property of the generated method rather
than of this interface.

Pairs with aws-controllers-k8s/code-generator#738, which generates the
method.

Issue aws-controllers-k8s/community#2431
Issue aws-controllers-k8s/community#2361
ack-prow Bot pushed a commit to aws-controllers-k8s/runtime that referenced this pull request Sep 16, 2026
#267)

## Summary

A cross-resource reference (`*Ref`) is generated as a **sibling** of the concrete field it resolves into — `spec.vpcConfig.subnetRefs` next to `spec.vpcConfig.subnetIDs`. A resource manager builds its return value from an AWS API response, which has no concept of a reference, so rebuilding the containing struct drops every `*Ref` inside it.

That disables `ClearResolvedReferences`, which suppresses a resolved value only while it can still see the sibling:

```go
if ko.Spec.VPCConfig != nil {
	if len(ko.Spec.VPCConfig.SubnetRefs) > 0 {   // false once the *Ref is gone
		ko.Spec.VPCConfig.SubnetIDs = nil
	}
}
```

So the spec patch **deletes the declared `*Ref` and stores the resolved value in its place** — what aws-controllers-k8s/community#2431 reports: a declared `securityGroupRefs` replaced by `securityGroupIDs`.

Reconciliation continues until the manifest is applied again, from Helm, Argo, Flux or `kubectl apply`. That apply restores the `*Ref` beside the now-stored value, and `validateReferenceFields` rejects the pair:

```
message: Reference resolution failed
reason: 'both resource reference wrapper and ID cannot be used together:
         VPCConfig.SubnetIDs,VPCConfig.SubnetRefs'
```

This PR adds an optional `ReferenceEnsurer` interface and invokes it on the object a resource manager hands back from `Create` and from `Update`, sourcing the references from the declared resource.

Fixes aws-controllers-k8s/community#2431, and addresses the struct-nested half of aws-controllers-k8s/community#2361. Pairs with aws-controllers-k8s/code-generator#738, which generates the method.

## Backwards compatible

`ReferenceEnsurer` is deliberately separate from `ReferenceManager` and reached through a type assertion, so every controller generated before the method existed still satisfies `AWSResourceManager` and compiles unchanged. Those controllers take the existing path untouched and opt in by regenerating. `TestReconcilerUpdate_WithoutEnsurerIsUnaffected` pins that.

## Where the restoration runs

On what a resource manager returns from `Create` and from `Update` — the two paths where the object about to be patched back was rebuilt from an API response while the patch base is still the resource the user declared. It runs before the error is inspected, since a manager may hand back a resource alongside a requeue error while an asynchronous operation is in flight and that object reaches the caller either way.

**Not on `AdoptionPolicy_Adopt`.** Under that policy the spec is populated from the observed AWS resource, so a declared spec is expected to be replaced rather than preserved, and a declared `*Ref` is replaced along with every other declared field. Restoring it would make the reference the one exception. `TestReconcilerAdopt_DoesNotEnsureReferences` asserts that.

**`AdoptionPolicy_AdoptOrCreate` does keep a declared reference**, for a structural reason rather than because the restoration runs: that branch marks the resource managed and adopted and requeues, and that patch's base is a `DeepCopy` of its own target, so nothing ever patches the spec with the declared resource as the base. `TestReconcilerAdoptOrCreate_PreservesReferences` asserts that too — both halves, because moving the restoration into `patchResourceMetadataAndSpec` would pick up the adopt branch automatically and silently change it.

Also not on the late-initialization patch, whose base is the AWS-observed object and carries no references — `TestReconcilerUpdate_LateInitializeIsNotAffectedByEnsureReferences` keeps that boundary asserted.

## Why `desired` and not `reconcileDesired`

`reconcileDesired` is what gets handed to `rm.Update`, and a resource manager may mutate the object it is given: `apigateway`'s `ApiKey` `sdkUpdate` assigns `desired.ko.Spec.StageKeys` straight from the `UpdateApiKey` response (`sdk.go:331`), and `applyIgnoredFields` merges observed values into it for a resource carrying the ignore-field-drift annotation. Only `desired` is a clean record of what the user declared, and it is also the base of the spec patch that follows, so the two agree by construction.

## Create now gets a copy of `desired`

`createResource` passed `desired` itself to `rm.Create`. Generated `sdkCreate` only
deep-copies the resource it is given partway through -- a `custom_implementation`
returns before that point, and a `sdk_create_pre_build_request` hook runs before it
-- so either could mutate the object the user declared. `desired` is both the patch
base and the reference source, so it has to stay a clean record, and `Create` gets
`desired.DeepCopy()`. This mirrors `updateResource`, which already hands `Update` a
copy for the same reason.

The copy is taken after `setResourceManaged` and `EnsureTags`, so it carries the
finalizer and the controller tags.

No hook template in the fleet writes to `desired.ko`, and neither custom Create
implementation (`elasticache`'s `CustomCreateSnapshot`, `apigatewayv2`'s
`customCreateApi`) mutates its input -- the former deep-copies first, the latter
only reads -- so this is hardening rather than a fix.
`TestReconcilerCreate_PassesCopyToCreate` pins it.

## What the generated method does

Detailed in aws-controllers-k8s/code-generator#738. Summarised here because it bounds this PR's blast radius:

- **top-level `*Ref`** — nothing emitted; it cannot be lost, since every write path starts from a `DeepCopy` of the object it was handed.
- **reached through structs** — only the reference field is assigned, so every concrete value the service reported stands. This is the whole of what is emitted, and it codifies a pattern `eks/cluster`, `lambda/function` and `opensearchservice/domain` already hand-maintain in `sdk_*_post_set_output` hooks.
- **reached through a list** — nothing emitted; no fixed address to assign to, and no sound way to pair an observed element with a declared one. These behave exactly as they do today.

So **117 struct-nested references across 37 resources in 25 controllers** change behaviour; the 315 top-level and 38 list-nested ones are untouched.

## Testing

Eleven tests in `pkg/runtime/reconciler_test.go`:

| Test | Asserts |
|---|---|
| `TestReconcilerCreate_EnsuresReferencesAfterCreate` | runs after `Create`, sourced from `desired` |
| `TestReconcilerUpdate_EnsuresReferencesAfterUpdate` | the same after `Update`, and the returned object is what gets patched |
| `TestReconcilerCreate_EnsuresReferencesOnCreateError` | still runs when `Create` returns an error |
| `TestReconcilerUpdate_EnsuresReferencesOnUpdateError` | the same on the update error path |
| `TestReconcilerAdopt_DoesNotEnsureReferences` | `AdoptionPolicy_Adopt` deliberately does not restore |
| `TestReconcilerAdoptOrCreate_PreservesReferences` | `AdoptionPolicy_AdoptOrCreate` keeps a declared reference |
| `TestReconcilerUpdate_LateInitializeIsNotAffectedByEnsureReferences` | never runs on the late-init patch |
| `TestReconcilerUpdate_EnsureReferencesToleratesNilLatest` | a manager returning `(nil, err)` passes through instead of panicking |
| `TestReconcilerUpdate_WithoutEnsurerIsUnaffected` | a manager not implementing the interface is untouched |
| `TestReconcilerCreate_PassesCopyToCreate` | `Create` gets a copy, so `desired` stays a clean record |
| `TestReconcilerUpdate_PassesCopyOfDesiredToUpdate` | the pre-existing counterpart on the update path |

Full runtime suite passes.

Verified on a cluster, A/B against the unmodified controller. An `ecs` `Service` declaring `networkConfiguration.awsVPCConfiguration.subnetRefs` and `.securityGroupRefs` keeps both through create, update, re-apply and repeated resyncs, with no resolved IDs written to the spec. The same manifest under the unmodified controller loses both and stores the resolved `subnets`/`securityGroups` in their place; re-applying it then halts the resource on `both resource reference wrapper and ID cannot be used together`, which is the failure this prevents. Deletion completes cleanly. `adopt-or-create` against a pre-existing AWS resource keeps the references; strict `adopt` replaces them, as intended. Separately, an `ecr` `Repository` whose `EncryptionConfiguration.KMSKey` is both a reference and `late_initialize` reaches `ACK.LateInitialized=True` with the reference kept and no churn.

## Not addressed

**References reached through a list.** No fixed address to assign to, and no sound way to pair an observed element with a declared one: an AWS response need not preserve request order, and 75 of the roughly 470 configured references resolve to a `Spec.*` path with no uniqueness guarantee. These behave exactly as they do today. This is the remaining half of #2361.

**`AdoptionPolicy_Adopt`** replaces a declared reference along with the rest of the declared spec. That is intentional, not a gap — see above.

**The existing read-path hooks.** `lambda/function`, `eks/cluster` and `opensearchservice/domain` restore these references by hand in `sdk_read_one_post_set_output`. Their create-path halves are subsumed. **The read-path halves must stay** — the `AdoptionPolicy_Adopt` and delete-path spec writes are not covered here.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Picks up aws-controllers-k8s/runtime#267, which restores struct-nested
cross-resource references after Create and Update and adds the optional
ReferenceEnsurer interface this PR generates an implementation of.

Bundled here rather than as a follow-up because the two halves are only
useful together. auto-generate-controllers resolves the runtime version from
this module's go.mod (cd/auto-generate/auto-generate-controllers.sh:91)
rather than from the latest runtime release, so merging the generator change
while this pin still said v0.63.0 would open fleet-wide PRs adding a
generated EnsureReferences method against a runtime that never calls it.

The bump is go.mod and go.sum only, with no generated-output changes.
`go build ./...` and `go test ./pkg/generate/...` pass.
EnsureReferences emitted the full guard chain and materialisation once per
reference, so two references in one container restated every enclosing
guard. Order the references by enclosing container and emit each container's
guard once, nesting a deeper container's guard inside its parent's.

ecs/CapacityProvider, the deepest case in the fleet, goes from 87 to 75
lines and from four separate guard chains to one tree; ecs/Service 42 to 38
and lambda/Function 46 to 40. The generated behaviour is unchanged.

The materialisation stays per-reference. It has to sit inside the guard
establishing that the source holds a reference to put in the container --
otherwise an empty container reaches the spec patch -- and sharing it would
need the source-side guards OR'd together, which is the concatenation the
nesting exists to avoid. It is a nil-check either way, so repeating it costs
nothing.

Also narrow the line-length assertion in
Test_EnsureReferences_GuardsAreNestedNotConcatenated to the guard lines,
which is what nesting controls. It read as a property of the whole output,
but an assignment line is as long as its two field paths and gofmt wraps
neither form: ecs/CapacityProvider's deepest reference produces a
210-character one, as the hand-written hooks this replaces already do.

Add an eks Cluster fixture carrying both shapes -- two references in one
container, and a container nested inside one that holds a reference of its
own -- and a test pinning that each guard is emitted once.

Regenerated ecs-, lambda- and eks-controller: all build, all gofmt-clean,
and the diffs are the same restorations under fewer guards.
The comment claimed the rejection was "kept in one place", but
iterReferenceValues makes the same check while walking the path itself, so
there are two. Say what is actually true: this keeps it in one place for
callers that do not walk the path.

Comment only. Sharing the check would mean extracting the lookup-and-reject
core into a third helper, since referenceAncestors stops at the first list
while iterReferenceValues has to emit a range loop and keep descending -- so
the former cannot serve as the latter's walker. That touches the function
behind ReferenceFieldsValidation, ResolveReferencesForField and
ClearResolvedReferencesForField, and belongs in its own change rather than
here.
@ack-prow

ack-prow Bot commented Sep 16, 2026

Copy link
Copy Markdown

@gustavodiaz7722: 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
acm-controller-test 0d2d97e link true /test acm-controller-test
ec2-controller-test 0d2d97e link true /test ec2-controller-test
dynamodb-controller-test 0d2d97e link true /test dynamodb-controller-test
eks-controller-test 0d2d97e link true /test eks-controller-test

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The ACK Lambda Controller modifies the object spec

2 participants