✨ Support setting role path and permissions boundary on managed IAM roles

[robinkb]

What type of PR is this?

/kind feature

What this PR does / why we need it:

Allows setting the role path and permissions boundary on managed IAM roles for EKS control plane, EKS Fargate profile, and managed machine pools. This is typically needed in enterprise environments with more stringent IAM requirements.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):

Fixes #5285

Special notes for your reviewer:

This is my first PR on this repository. I noticed a warning when generating code, and I wasn't sure if this needed to be fixed:

E0116 12:53:58.750053   55614 conversion.go:744] Warning: could not find nor generate a final Conversion function for sigs.k8s.io/cluster-api-provider-aws/v2/exp/api/v1beta2.FargateProfileSpec -> sigs.k8s.io/cluster-api-provider-aws/v2/exp/api/v1beta1.FargateProfileSpec
E0116 12:53:58.750274   55614 conversion.go:745]   the following fields need manual conversion:
E0116 12:53:58.750281   55614 conversion.go:747]       - RolePath
E0116 12:53:58.750286   55614 conversion.go:747]       - RolePermissionsBoundary

Checklist:

• squashed commits
• includes documentation
• includes emojis
• adds unit tests
• adds or updates e2e tests

Release note:

action required
Support setting role path and permissions boundary on managed IAM roles. If you want to use permission boundaries, then you will need to update your IAM permissions by running **clusterawsadm bootstrap iam create-cloudformation-stack** again.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: robinkb / name: Robin Ketelbuters (9c4d4d0)

[k8s-ci-robot]

Hi @robinkb. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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-sigs/prow repository.

[pavansokkenagaraj]

@robinkb PermissionsBoundary are required for CloudFormation template rendering for creating roles and permissions.
https://github.com/kubernetes-sigs/cluster-api-provider-aws/blob/main/cmd/clusterawsadm/cloudformation/bootstrap/template.go#L139-L160

[robinkb]

Hi @pavansokkenagaraj, thanks for the feedback.

I've amended my commit. However, I see more roles generated in the template:
https://github.com/kubernetes-sigs/cluster-api-provider-aws/blob/main/cmd/clusterawsadm/cloudformation/bootstrap/template.go#L183-L208

Do these need to be changed, too? I didn't use the bootstrap proces, so I'm not really familiar with how it works.

[robinkb]

We are in the process of testing this on our own environment now. The most recent update includes some shenanigans around pointers. I'm not sure which way I am expected to handle them, as the codebase is inconsistent. The Spec for AWSManagedControlPlane uses pointers to strings for optional values, while the Spec for AWSFargateProfile does not, for example.

[richardcase]

/ok-to-test

[pavansokkenagaraj]

@robinkb

Do these need to be changed, too? I didn't use the bootstrap proces, so I'm not really familiar with how it works.

Yes, roles/permissions which are created in template function for eks need to be updated.

ref: - https://github.com/pavansokkenagaraj/cluster-api-provider-aws/blob/c518150b3a4aeabb38cab546d8a89988307cd053/cmd/clusterawsadm/cloudformation/bootstrap/template.go#L201-L238

The other thing I had to do was to set the permissionsBoundary if it is not nil/empty
if permissionsBoundary is empty, AWS IAM role creation returns an err expecting permissionsBoundary to be length of greater than 0(This needs some testing)

https://github.com/pavansokkenagaraj/cluster-api-provider-aws/blob/c518150b3a4aeabb38cab546d8a89988307cd053/pkg/cloud/services/eks/iam/iam.go#L190-L203

[robinkb]

@pavansokkenagaraj Thanks for the testing, I will get that fixed.

Regarding the failing apidiff: How do we handle that? I don't see how I could fix this issue without breaking the API of that particular package, unless we want to add a whole other function.

[richardcase]

@robinkb - we don't need to worry about the apidiff if its outside an api directory. The project doesn't provider guarantees for the exported API types outside of the APIs.

[richardcase]

/override pull-cluster-api-provider-aws-apidiff-main

[richardcase]

/test ?

[k8s-ci-robot]

@richardcase: The following commands are available to trigger required jobs:

/test pull-cluster-api-provider-aws-build

/test pull-cluster-api-provider-aws-build-docker

/test pull-cluster-api-provider-aws-test

/test pull-cluster-api-provider-aws-verify

The following commands are available to trigger optional jobs:

/test pull-cluster-api-provider-aws-apidiff-main

/test pull-cluster-api-provider-aws-e2e

/test pull-cluster-api-provider-aws-e2e-blocking

/test pull-cluster-api-provider-aws-e2e-clusterclass

/test pull-cluster-api-provider-aws-e2e-conformance

/test pull-cluster-api-provider-aws-e2e-conformance-with-ci-artifacts

/test pull-cluster-api-provider-aws-e2e-eks

/test pull-cluster-api-provider-aws-e2e-eks-gc

/test pull-cluster-api-provider-aws-e2e-eks-testing

Use /test all to run the following jobs that were automatically triggered:

pull-cluster-api-provider-aws-apidiff-main

pull-cluster-api-provider-aws-build

pull-cluster-api-provider-aws-build-docker

pull-cluster-api-provider-aws-test

pull-cluster-api-provider-aws-verify

In response to this:

/test ?

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.

[richardcase]

/test pull-cluster-api-provider-aws-e2e-eks
/test pull-cluster-api-provider-aws-e2e

[robinkb]

/retest

[damdo]

@robinkb have you looked into the e2e-eks failure? It seems related to identities so worth checking if it is legit.

[robinkb]

@damdo The tests did not run, failing in the BeforeSuite. The failure in the AfterSuite comes from trying to clean up a resource that was never created in the first place.

[robinkb]

I will try to run the test again to see if anything changed.

/retest

[robinkb]

/retest

[damdo]

Let's get you reviews on this one
I'll try to have a look

also
/assign @nrb @richardcase @dlipovetsky @AndiDog @Ankitasw

[richardcase]

The e2e tests are passing.

@robinkb - do you think it would be worth us following up on this in the future with an e2e that covers this?

[richardcase]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: richardcase

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:

• OWNERS [richardcase]

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

[aristosvo]

Thanks @robinkb and maintainers👌🚀

[robinkb]

@richardcase Thank you! 🎉

Because it's only passing through two strings fields, I don't think that adding an e2e test for this is really necessary. Maybe an integration test to verify that the parameters are correctly passed to the AWS API? But even that feels a bit superfluous. The rolePath parameter could be set in an existing e2e test, but that could break IAM policies.

Probably not worth the effort.