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

add web identity token configuration to ProviderConfig spec #1148

Conversation

erhancagirici
Copy link
Collaborator

@erhancagirici erhancagirici commented Feb 12, 2024

Description of your changes

Adds support for Web Identity token configuration from ProviderConfig
Fixes #1007

Currently Web Identity authentication configuration is only possible with AWS_WEB_IDENTITY_TOKEN_FILE and AWS_ROLE_ARN environment variables set.
This causes confusion and ambiguity in several ways and has limitations:

  • Although consumers specify the role to assume for Web Identity auth at spec.credentials.webIdentity.roleARN, they still need to specify the same role ARN at AWS_ROLE_ARN environment variable to make it work. As reported in WebIdentity credential source relies on eks.amazonaws.com/role-arn ServiceAccount annotation #1007, this seems like the users depend on IRSA annotations, however it is actually a dependency to AWS_ROLE_ARN and AWS_WEB_IDENTITY_TOKEN_FILE environment variables. When consumers do IRSA config, these env vars are automatically set, which satisfies the config.
  • The token to use for Web Identity cannot be specified via ProviderConfig spec and can only be specified through AWS_WEB_IDENTITY_TOKEN_FILE
    • Consumers might forgot to set the AWS_ROLE_ARN which emits uninformative errors.
    • This results in a heterogeneous config UX, where role ARN is provided through ProviderConfig and token through env.
    • Consumers need to prepare ControllerConfigs (deprecated) or DeploymentRuntimeConfigs to mount token files and inject env vars to provider pods.
    • Consumers are limited to a single Web Identity config per provider pod. It is not possible to configure tokens & roleARNs per provider config, where consumers want multiple provider configs referring to different Web Identity configs (as reported in WebIdentity credential source relies on eks.amazonaws.com/role-arn ServiceAccount annotation #1007 (comment))

This change expands ProviderConfig API specification with spec.credentials.webIdentity.tokenConfig, with options to reference a secret or filesystem location for the token to be used for AssumeRoleWithWebIdentity operations.

Consumers can specify the token's source (Secret or Filesystem) and set secretRef or fs respectively.

ℹ️ The change is backward compatible for consumers relying on old behavior where they set AWS_WEB_IDENTITY_TOKEN_FILE and AWS_ROLE_ARN environment variables, to allow a smooth transition

⚠️ However, this approach is deprecated now and consumers are suggested to update their provider configs to specify the tokenConfig.

Example Web identity token configuration with Web Identity Token from a secret:

apiVersion: aws.upbound.io/v1beta1
kind: ProviderConfig
metadata:
  name: webidentity-example
spec:
  credentials:
    source: WebIdentity
    webIdentity:
      roleARN: arn:aws:iam::123456789012:role/providerexamplerole
      tokenConfig:
        source: Secret
        secretRef:
          key: token
          name: example-web-identity-token-secret
          namespace: upbound-system

example using a filesystem location. Note that Filesystem source option still needs the token file mounted to the filesystem of the provider pod.
The difference is that new API allows specifying the token file per ProviderConfig, meaning that you can mount multiple tokens to the provider pod and reference the desired one at each ProviderConfig

apiVersion: aws.upbound.io/v1beta1
kind: ProviderConfig
metadata:
  name: webidentity-example
spec:
  credentials:
    source: WebIdentity
    webIdentity:
      roleARN: arn:aws:iam::123456789012:role/providerexamplerole
      tokenConfig:
        source: Filesystem
        fs:
          path: /path/to/token/file

I have:

  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Tested with provided examples in the PR description for the new feature and uptest for regressions

@erhancagirici erhancagirici force-pushed the web-identity-auth-tokenfile branch from 308961b to 1d55074 Compare February 13, 2024 12:07
@erhancagirici erhancagirici marked this pull request as ready for review February 13, 2024 12:09
@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/iam/v1beta1/role.yaml"

@erhancagirici erhancagirici force-pushed the web-identity-auth-tokenfile branch from 1d55074 to 6481d0c Compare February 14, 2024 08:33
@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/ec2/v1beta1/vpc.yaml"

@erhancagirici
Copy link
Collaborator Author

Manually tested:

  • web identity with new secret ref token config

    • valid and invalid token paths
  • web identity with new filesystem ref token config

    • valid and invalid paths and tokens
  • old approach with environment variables AWS_WEB_IDENTITY_TOKEN_FILE and AWS_ROLE_ARN

  • non-web identity methods (for regressions)

    • access key and secret
    • IRSA
    • Upbound
  • combinations with AssumeRoleChain added

Upbound SaaS is also tested by @turkenf

Copy link
Collaborator

@ulucinar ulucinar left a comment

Choose a reason for hiding this comment

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

Thanks @erhancagirici for working on the ProviderConfig. Left some comments for you to consider.

examples/providerconfig-webidentity.yaml Outdated Show resolved Hide resolved
internal/clients/aws.go Outdated Show resolved Hide resolved
internal/clients/provider_config.go Outdated Show resolved Hide resolved
internal/clients/provider_config.go Outdated Show resolved Hide resolved
internal/clients/provider_config.go Outdated Show resolved Hide resolved
internal/clients/provider_config.go Outdated Show resolved Hide resolved
internal/clients/provider_config.go Outdated Show resolved Hide resolved
internal/clients/provider_config.go Outdated Show resolved Hide resolved
@erhancagirici erhancagirici force-pushed the web-identity-auth-tokenfile branch from 6481d0c to 8abf565 Compare February 15, 2024 15:17
@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/ec2/v1beta1/vpc.yaml"

@erhancagirici erhancagirici force-pushed the web-identity-auth-tokenfile branch from 8abf565 to 7d4f382 Compare February 15, 2024 15:49
Signed-off-by: Erhan Cagirici <[email protected]>
@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/ec2/v1beta1/vpc.yaml"

Copy link
Collaborator

@ulucinar ulucinar left a comment

Choose a reason for hiding this comment

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

Thanks @erhancagirici, lgtm.

@erhancagirici erhancagirici merged commit 77871bb into crossplane-contrib:main Feb 15, 2024
10 checks passed
@erhancagirici erhancagirici deleted the web-identity-auth-tokenfile branch February 15, 2024 16:22
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.

WebIdentity credential source relies on eks.amazonaws.com/role-arn ServiceAccount annotation
2 participants