Skip to content

feat: Store resource partition in Status#615

Merged
ack-prow[bot] merged 1 commit into
aws-controllers-k8s:mainfrom
michaelhtm:feat/storeresourcepartitioninstatus
May 15, 2026
Merged

feat: Store resource partition in Status#615
ack-prow[bot] merged 1 commit into
aws-controllers-k8s:mainfrom
michaelhtm:feat/storeresourcepartitioninstatus

Conversation

@michaelhtm
Copy link
Copy Markdown
Member

@michaelhtm michaelhtm commented Jul 30, 2025

Description of changes:
Store resource partition as ACKResourceMetadata

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 rushmash91 July 30, 2025 21:49
@ack-prow ack-prow Bot added the approved label Jul 30, 2025
@michaelhtm michaelhtm force-pushed the feat/storeresourcepartitioninstatus branch from 300e3ab to 5680f13 Compare July 30, 2025 23:47
@michaelhtm
Copy link
Copy Markdown
Member Author

michaelhtm commented Aug 1, 2025

tested in aws-controllers-k8s/s3-controller#173

@ack-bot
Copy link
Copy Markdown
Collaborator

ack-bot commented Jan 28, 2026

Issues go stale after 180d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 60d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
Provide feedback via https://github.com/aws-controllers-k8s/community.
/lifecycle stale

@ack-prow ack-prow Bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 28, 2026
@ack-bot
Copy link
Copy Markdown
Collaborator

ack-bot commented Mar 30, 2026

Stale issues rot after 60d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 60d of inactivity.
If this issue is safe to close now please do so with /close.
Provide feedback via https://github.com/aws-controllers-k8s/community.
/lifecycle rotten

@ack-prow ack-prow Bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 30, 2026
@michaelhtm michaelhtm force-pushed the feat/storeresourcepartitioninstatus branch from 5680f13 to fa1c267 Compare May 11, 2026 20:37
@michaelhtm michaelhtm removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label May 12, 2026
knottnt
knottnt reviewed May 14, 2026
View reviewed changes
Comment thread templates/pkg/resource/identifiers.go.tpl
Comment on lines +44 to +45
// Partition returns the AWS partition in which the reosurce exists, or
// nil if this information is not known.
Copy link
Copy Markdown
Contributor

@knottnt knottnt May 14, 2026

Choose a reason for hiding this comment

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

nit

Suggested change
// Partition returns the AWS partition in which the reosurce exists, or
// nil if this information is not known.
// Partition returns the AWS partition in which the resource exists, or
// nil if this information is not known.

Comment thread templates/pkg/resource/manager_factory.go.tpl
// We use the account ID, region, partition, and role ARN to uniquely identify a
// resource manager. This helps us to avoid creating multiple resource
// managers for the same account/region/roleARN combination.
rmId := fmt.Sprintf("%s/%s/%s", id, region, roleARN)
Copy link
Copy Markdown
Contributor

@knottnt knottnt May 14, 2026

Choose a reason for hiding this comment

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

This doesn't appear to actually use partition.

Copy link
Copy Markdown
Member Author

@michaelhtm michaelhtm May 14, 2026

Choose a reason for hiding this comment

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

there is no cross partition support..using it as a cache key would not be necessary

Copy link
Copy Markdown
Contributor

@knottnt knottnt May 14, 2026

Choose a reason for hiding this comment

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

Okay, we might want to update the comment to reflect that then.

We use the account ID, region, partition, and role ARN to uniquely identify a
// resource manager

@michaelhtm michaelhtm force-pushed the feat/storeresourcepartitioninstatus branch from fa1c267 to c0efeb7 Compare May 14, 2026 22:45
@knottnt knottnt mentioned this pull request May 14, 2026
Merged
@michaelhtm michaelhtm force-pushed the feat/storeresourcepartitioninstatus branch from c0efeb7 to ffcb8e3 Compare May 14, 2026 23:01
knottnt
knottnt approved these changes May 14, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@knottnt knottnt 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 May 14, 2026
@michaelhtm michaelhtm force-pushed the feat/storeresourcepartitioninstatus branch from ffcb8e3 to 2ea63bd Compare May 15, 2026 00:54
@ack-prow ack-prow Bot removed the lgtm Indicates that a PR is ready to be merged. label May 15, 2026
@michaelhtm
Copy link
Copy Markdown
Member Author

michaelhtm commented May 15, 2026

/retest

knottnt
knottnt approved these changes May 15, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@knottnt knottnt 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 May 15, 2026
@ack-prow
Copy link
Copy Markdown

ack-prow Bot commented May 15, 2026

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@knottnt
Copy link
Copy Markdown
Contributor

knottnt commented May 15, 2026

/test ec2-controller-test

@michaelhtm
Copy link
Copy Markdown
Member Author

michaelhtm commented May 15, 2026

/retest

@ack-prow ack-prow Bot merged commit 96dcae8 into aws-controllers-k8s:main May 15, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@knottnt knottnt knottnt approved these changes

@rushmash91 rushmash91 Awaiting requested review from rushmash91

Assignees

@knottnt knottnt

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@michaelhtm @ack-bot @knottnt