feat: generate EnsureReferences to preserve nested cross-resource references - #738
gustavodiaz7722 wants to merge 1 commit into
Conversation
e861ec6 to
5cedd00
Compare
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
5cedd00 to
8ccc75d
Compare
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
|
/retest |
1 similar comment
|
/retest |
|
@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. |
8ccc75d to
4d1336b
Compare
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
|
@knottnt Created a demo here aws-controllers-k8s/lambda-controller#242 |
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
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
|
[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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
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
4d1336b to
17aac1a
Compare
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.
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
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
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
17aac1a to
8341dc6
Compare
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.
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
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
8341dc6 to
c76a277
Compare
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: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
Summary
A cross-resource reference (
*Ref) is generated as a sibling of the concrete field it resolves into —spec.vpcConfig.subnetRefsnext tospec.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*Refinside it.That disables
ClearResolvedReferences, which suppresses a resolved value only while it can still see the sibling:So the spec patch deletes the declared
*Refand stores the resolved value in its place — what aws-controllers-k8s/community#2431 reports: a declaredsecurityGroupRefsreplaced bysecurityGroupIDs.Reconciliation continues until the manifest is applied again, from Helm, Argo, Flux or
kubectl apply. That apply restores the*Refbeside the now-stored value, andvalidateReferenceFieldsrejects the pair:This PR generates an
EnsureReferencesmethod 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 the runtime change
aws-controllers-k8s/runtime#267 defines the optional
ReferenceEnsurerinterface and calls the method on what a resource manager returns fromCreateand fromUpdate. It deliberately does not call it onAdoptionPolicy_Adopt, where replacing a declared spec is the intended behaviour. Without the runtime change this method is inert but harmless. Controllers generated before it existed are unaffected and opt in by regenerating.What is emitted
spec.xRef)DeepCopy, and the*Refis a sibling of the concrete field, so nothing rebuilds itspec.a.xRef,spec.a.xRefs)spec.l[].xRef)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.Only the reference is written, so nothing the service populated is touched.
Three 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
sdkCreatehas} 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 insidesdkCreate/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 mirrorsClearResolvedReferences, which walks the same paths.This codifies an existing pattern
Restoring a nested
*Reffrom the declared resource is not new — three controllers already hand-maintain this assignment for want of a generated equivalent.eks/cluster:lambda/functiondoes the same forVPCConfig, andopensearchservice/domainforVPCOptions— the latter with a comment naming this issue directly:// To prevent https://github.com/aws-controllers-k8s/community/issues/2431The 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 generatedreferences.go:Testing
Ten 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, and that no generated line exceeds 120 characters or concatenates its guards.Regenerated
lambda-controllerandecs-controller;references.godiffs are purely additive, output is gofmt-clean, and both build in full.lambda/functionrestoresCode.S3BucketRef,VPCConfig.SecurityGroupRefsandVPCConfig.SubnetRefs— the shape #2431 was filed for.Verified on a cluster, A/B against the unmodified controller: an
ecsServicedeclaringnetworkConfiguration.awsVPCConfiguration.subnetRefsand.securityGroupRefskeeps both through create and across repeated resyncs with no resolved IDs written, where the same manifest under the unmodified controller loses both and stores the resolvedsubnets/securityGroupsin their place. Re-applying it in that state halts the resource onboth resource reference wrapper and ID cannot be used together, which is the failure this prevents.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/clusterandopensearchservice/domainrestore these same references by hand insdk_read_one_post_set_output. Only their create-path halves are subsumed; this PR does not remove either half.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.