Skip to content

Conversation

@michaelhtm
Copy link
Member

@michaelhtm michaelhtm commented Feb 10, 2025

Issue #2248

Description of changes:
By generating these two functions, we ensure that AWS tags will not
be present in resource Spec during adoption/regular reconciliations.

FilterSystemTags will be called right after the ReadOne operation during
adoption, and it will ensure we don't patch AWS (aws:cloudformation..)
or Controller tags (services.aws.k8s/...) into the resource.

MirrowAWSTags instead, is called during the regular reconciliation loop,
after each ReadOne operation. It ensures that if the resource in AWS has
AWS tags, they need to be reflected in the desired Spec. This will ensure
that those tags will not be detected in the Delta and trigger an update.
Since the controller only patches the difference between the latest and
desired resource, mirroring the AWS tags ensures we don't include them
in our Spec.

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

We are adding a new generated code that handles the filtering of tags.
These tags may be injected AWS resources during creation, or are added
to resources managed by cloudformation for example, and the controller
would see them in the diff and attempt to remove them. With these changes
we ensure that any tag that has `aws:` prefix will be filtered out after
the Find operation. Doing so will ensure that we don't attempt to remove
these tags.
@michaelhtm michaelhtm force-pushed the adress/tag-issue branch 2 times, most recently from 7582c00 to 7015914 Compare February 15, 2025 00:14
@michaelhtm
Copy link
Member Author

/retest

@michaelhtm michaelhtm changed the title Filter AWS injected tags Generate FilterSystemTags and mirrorAWSTags in controllers Feb 17, 2025
@michaelhtm
Copy link
Member Author

/retest

1 similar comment
@michaelhtm
Copy link
Member Author

/retest

@ack-prow
Copy link

ack-prow bot commented Feb 17, 2025

@michaelhtm: 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
verify-attribution 5a0736d link false /test verify-attribution
s3-olm-test 5a0736d link false /test s3-olm-test

Full PR test history. Your PR dashboard.

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/test-infra repository. I understand the commands that are listed here.

Copy link
Member

@a-hilaly a-hilaly left a comment

Choose a reason for hiding this comment

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

👍
/lgtm

@ack-prow ack-prow bot added the lgtm Indicates that a PR is ready to be merged. label Feb 17, 2025
@ack-prow
Copy link

ack-prow bot commented Feb 17, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: a-hilaly, michaelhtm

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@ack-prow ack-prow bot added the approved label Feb 17, 2025
@ack-prow ack-prow bot merged commit c2d3b95 into aws-controllers-k8s:main Feb 17, 2025
16 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants