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

Read external ID for assume role authentication from k8s Secret #1602

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

baburciu
Copy link

@baburciu baburciu commented Dec 12, 2024

Description of your changes

Fixes #1548

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

Adds the ProviderConfig ability to define the external ID presented by third parties to access to AWS accounts via:

spec:
  assumeRoleChain:
  - externalIDSecretRef:
      namespace: ..
      name: ..
      key: ..

on top of existing functionality

spec:
  assumeRoleChain:
  - externalID: ..

and if both are used, the value defined through externalIDSecretRef takes precedence (considered more secure).

A function to read from a Kubernetes Secret was needed and seeing ExtractSecret in crossplane-runtime/apis/common/v1 I thought I could piggyback on that, to avoid defining another in provider-upjet-aws. Maybe a better way would be to allow something more generic in crossplane-runtime (today it expects a CommonCredentialSelectors)
I don't have much Go development experience, please feel free to suggest if a better approach could be used.

Trying to run make reviewable takes a lot of time, can't say I passed that step.

How has this code been tested

Tested in an EKS cluster where the provider is using IRSA for authentication

$ kubectl get userpoolclient.cognitoidp.aws.upbound.io demo-mr-externalidsecretref
NAME                                        SYNCED   READY   EXTERNAL-NAME                AGE
demo-mr-externalidsecretref   True     True    7g4kcls3pv3373noulem3gs378   3h5m
$ kubectl get userpoolclient.cognitoidp.aws.upbound.io demo-mr-externalidsecretref -o yaml | yq .spec.providerConfigRef.name
provider-aws-upbound-cognitoidp
$ kubectl get providerconfig.aws.upbound.io provider-aws-upbound-cognitoidp -o yaml | yq .spec
assumeRoleChain:
- externalIDSecretRef:
    key: externalid
    name: externalid-secret
    namespace: crossplane-system
  roleARN: arn:aws:iam::112233445566:role/eks-crossplane-cognito
credentials:
  source: IRSA
$

with

$ kubectl create secret generic externalid-secret-failing -n crossplane-system --from-literal externalid=foobartoto

and the assumed role having a trust relationship like:

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Principal": {
                "AWS": [
                    "arn:aws:iam::778899101011:root"
                ]
            },
            "Action": "sts:AssumeRole",
            "Condition": {
                "StringEquals": {
                    "sts:ExternalId": "foobartoto"
                }
            }
        }
    ]
}

(and provider identity allowed to sts:AssumeRole).

Similarly tested in kind with provider authentication method Secret, all testing done using dirty builds like

xpkg pushed to index.docker.io/bogdanadrianburciu/provider-aws-cognitoidp:v1.18.0-rc.0.24.g9a1e8e066.dirty
xpkg pushed to index.docker.io/bogdanadrianburciu/provider-family-aws:v1.18.0-rc.0.24.g9a1e8e066.dirty

…of type github.com/crossplane/crossplane-runtime/apis/common/v1.SecretKeySelector and wrap SecretKeySelector into CommonCredentialSelectors to extract external ID from K8s Secret

Signed-off-by: Bogdan-Adrian Burciu <[email protected]>
@baburciu baburciu changed the title Introduce ProviderConfig.spec.assumeRoleChain[].externalIDSecretRef option Read external ID for assume role authentication from k8s Secret Dec 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.

ProviderConfig externalID should take a secretRef
1 participant