Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update authentication documentation #582

Conversation

smcavallo
Copy link
Contributor

Update authentication documentation with serviceAccount recommendation for provider upgrade
Signed-off-by: smcavallo smcavallo@hotmail.com

Description of your changes

We use pod-iam which requires assigning the service account name to the aws role. The problem is that the service account name is the provider-aws revision and it changes during the release, breaking crossplane permissions
Ex. v.0.16.0 - provider-aws-6b36ca882d8e and v.0.17.0 - provider-aws-4ebd5d8c7cb7
Using a static StringEquals condition with a hard coded provider will break when the SERVICE_ACCOUNT_NAME changes when the provider is upgraded

"StringEquals": {
          "${OIDC_PROVIDER}:sub": "system:serviceaccount:${SERVICE_ACCOUNT_NAMESPACE}:${SERVICE_ACCOUNT_NAME}"
        }

This condition can be avoided by using an alternative IAM policy which uses StringLike and a wildcard SERVICE_ACCOUNT_NAME:

"StringLike": {
          "${OIDC_PROVIDER}:sub": "system:serviceaccount:${SERVICE_ACCOUNT_NAMESPACE}:provider-aws-*"
        }

This trust policy would cover both service account names - provider-aws-6b36ca882d8e and provider-aws-4ebd5d8c7cb7
As long as provider-aws- will not change as part of a release the above should cover future cases of provider names changing during a release without needing to update the role trust policy.

Fixes #

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Copy link
Member

@hasheddan hasheddan left a comment

Choose a reason for hiding this comment

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

Thanks so much @smcavallo! This looks great, one nitpick below and I think you need DCO as well.

AUTHENTICATION.md Outdated Show resolved Hide resolved
for provider upgrade
Signed-off-by: smcavallo <smcavallo@hotmail.com>
@smcavallo smcavallo force-pushed the authentication_documentation_pod-iam branch from ff7cda2 to 3756918 Compare March 8, 2021 21:29
@hasheddan hasheddan merged commit 0faa6de into crossplane-contrib:master Mar 8, 2021
tektondeploy pushed a commit to gtn3010/provider-aws that referenced this pull request Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants