IAMRoleSelector - support for resource label selector #222

zarko-a · 2025-12-01T23:33:03Z

Description of changes:
This PR adds the ability to select resources based on their labels in addition to namespace and resource type. The new resourceLabelSelector field works identically to the existing namespaceSelector.labelSelector - when specified, all labels must match (AND logic), and when omitted, all resources are matched. All selector conditions (namespace, resource type, and resource labels) are ANDed together.

Idea for this change was mentioned here.

I built a custom image of IAM controller and I tested two scenarios:

ACK pod -> pod identity assumes a role in a local account -> creates an IAM resource in a local account when there are no label selectors (default behavior)
ACK pod -> pod identity assumes a role in a local account -> IAMRoleSelector tells it to chain to a role in target account -> creates an IAM resource in target account based on a resourceLabelSelector (new functionality)

apiVersion: services.k8s.aws/v1alpha1
kind: IAMRoleSelector
metadata:
  name: internal-services
spec:
  arn: arn:aws:iam::222222222222:role/ack_iam_cross_account_role_non-prod
  resourceLabelSelector:
    matchLabels:
      example.com/aws-account: internal-services

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

ack-prow · 2025-12-01T23:33:10Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: zarko-a
Once this PR has been reviewed and has the lgtm label, please assign rushmash91 for approval by writing /assign @rushmash91 in a comment. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

ack-prow · 2025-12-01T23:33:16Z

Hi @zarko-a. Thanks for your PR.

I'm waiting for a aws-controllers-k8s member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

zarko-a · 2025-12-01T23:43:31Z

@a-hilaly . This is based on our conversation here. Please take a look.

For us, ability to do a cross account provisioning on an individual resource level is super important (not just based on namespace). Crossplane is able to do this by having multiple providers and it would great if this was merged so ACK could do the same.

michaelhtm

great work @zarko-a 🚀
left a few nits below

michaelhtm · 2025-12-02T00:03:21Z

pkg/runtime/iamroleselector/matcher.go

 	// Validate label selector
 	if len(nsSelector.LabelSelector.MatchLabels) > 0 {
 		for key := range nsSelector.LabelSelector.MatchLabels {
 			if key == "" {
 				return fmt.Errorf("label key cannot be empty")
 			}
 			// Kubernetes label values can be empty, so we don't validate value
 		}
 	}

 	return nil
 }

+// validateResourceLabelSelector checks that the resource label selector has valid label keys
+func validateResourceLabelSelector(labelSelector ackv1alpha1.LabelSelector) error {
+	if len(labelSelector.MatchLabels) > 0 {
+		for key := range labelSelector.MatchLabels {
+			if key == "" {
+				return fmt.Errorf("label key cannot be empty")
+			}
+			// Kubernetes label values can be empty, so we don't validate value
+		}
+	}
+	return nil
+}


nit: this seems to be repeated code..should we have some sort of helper function?

or rename validateResourceLabelSelector to validateLabelSelector

Good point. I've renamed the function to validateLabelSelector and changed validateNamespaceSelector to use the same helper.

Please see the latest commit.

michaelhtm · 2025-12-02T00:06:27Z

pkg/runtime/iamroleselector/matcher.go

+	// If no label selector specified, matches all resources
+	if len(labelSelector.MatchLabels) == 0 {
+		return true
+	}
+
+	// Check if all specified labels match (AND logic within label selector)
+	selector := labels.SelectorFromSet(labelSelector.MatchLabels)
+	return selector.Matches(labels.Set(resourceLabels))


nit: same here..we can rename this to matchesLabels?

Done.

Please see the latest commit.

a-hilaly · 2025-12-03T05:37:53Z

/ok-to-test

ack-prow · 2025-12-03T08:02:39Z

@zarko-a: 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	`69f297b`	link	false	`/test verify-attribution`
unit-test	`69f297b`	link	true	`/test unit-test`
s3-controller-test	`69f297b`	link	true	`/test s3-controller-test`
ec2-controller-test	`69f297b`	link	true	`/test ec2-controller-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.

IAMRoleSelector - support for resource label selector

2762512

ack-prow bot requested review from a-hilaly and michaelhtm December 1, 2025 23:33

ack-prow bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 1, 2025

michaelhtm reviewed Dec 2, 2025

View reviewed changes

Use helper for label validation and rename functions

69f297b

ack-prow bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 3, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

IAMRoleSelector - support for resource label selector #222

IAMRoleSelector - support for resource label selector #222

zarko-a commented Dec 1, 2025

Uh oh!

ack-prow bot commented Dec 1, 2025

Uh oh!

ack-prow bot commented Dec 1, 2025

Uh oh!

zarko-a commented Dec 1, 2025

Uh oh!

michaelhtm left a comment

Uh oh!

michaelhtm Dec 2, 2025

Uh oh!

michaelhtm Dec 2, 2025

Uh oh!

zarko-a Dec 2, 2025

Uh oh!

michaelhtm Dec 2, 2025

Uh oh!

zarko-a Dec 2, 2025

Uh oh!

a-hilaly commented Dec 3, 2025

Uh oh!

ack-prow bot commented Dec 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

IAMRoleSelector - support for resource label selector #222

Are you sure you want to change the base?

IAMRoleSelector - support for resource label selector #222

Conversation

zarko-a commented Dec 1, 2025

Uh oh!

ack-prow bot commented Dec 1, 2025

Uh oh!

ack-prow bot commented Dec 1, 2025

Uh oh!

zarko-a commented Dec 1, 2025

Uh oh!

michaelhtm left a comment

Choose a reason for hiding this comment

Uh oh!

michaelhtm Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

michaelhtm Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

zarko-a Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

michaelhtm Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

zarko-a Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

a-hilaly commented Dec 3, 2025

Uh oh!

ack-prow bot commented Dec 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants