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

feat(pc): add providerconfig example for aws eks pod identity feature #1254

Conversation

haarchri
Copy link
Member

@haarchri haarchri commented Apr 3, 2024

Description of your changes

add providerconfig example for aws eks pod identity feature (https://aws.amazon.com/blogs/containers/amazon-eks-pod-identity-a-new-way-for-applications-on-eks-to-obtain-iam-credentials/) and additional hints for IAM Role and OpenIDConnectProvider
realted to this convo: #1252

Fixes #1249 #1252

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.

How has this code been tested

roleARN: arn:aws:iam::123456789012:role/pod-identity-role
tokenConfig:
fs:
path: /var/run/secrets/pods.eks.amazonaws.com/serviceaccount/eks-pod-identity-token
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe this is a future improvement in crossplane but it would be nice if we could have this be easier to implement without having to hard code path 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm surprised that it's necessary to set this explicitly, and that the SDK doesn't "Just Work" when instantiated in a pod that's assigned a PodIdentity. Maybe what we really need is to add an explicit PodIdentity to the list of supported auth types?

Copy link
Member Author

Choose a reason for hiding this comment

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

today we need to set it explicit - because our GO SDK is not up2date with this feature

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there an issue to track that?

Copy link

@danielloader danielloader May 13, 2024

Choose a reason for hiding this comment

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

Quick question, if not a clarification:

IRSAv1 had a huge pain point in needing to include the roleARN that you're going to annotate service accounts with, so that the OIDC trust loop worked.

With IRSAv2 (Pod Identity Controller) I can configure all the AWS IAM/EKS components out of cluster, and as long as a pod starts up in the nominated namespace, and with the service account I picked, it should inherit the permissions at start via the injected token provided by the pod access controller.

Given this; if I have to mangle my helm to be coupled to the IAM ARN, I'm not sure I'm saving any hassle here - I want to just add a pod identity out of band (outside EKS, not involving Crossplane, since the host cluster is managed with terraform) and have Crossplane start up and use the IAM permissions granted to it.

Am I misunderstanding things here?

It's a relatively new concept but I've had success with this decoupled model with External Secrets controller for example.

Additionally as pointed out by @mbbush if we need to configure this, rather than letting it fall through the SDK default authentication methods a lot of the benefit of "it just works". Namely that; the pod identity controller, the associated admission mutation webhook and the tokens just being in the expected on the filsystem... start to lose some value.

Copy link
Collaborator

@mbbush mbbush left a comment

Choose a reason for hiding this comment

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

I haven't used the PodIdentity feature in EKS yet, so I may just be misinformed about how this works, but there are a few things I find surprising here, that I think should at least be better documented, if not corrected.

I also think that this example probably needs to be more than just a single manifest. Including both managed resources that must exist before the provider config will work, and the provider config used to create those resources, in the same manifest, creates a circular dependency.

examples/providerconfig/v1beta1/pod-identity.yaml Outdated Show resolved Hide resolved
Comment on lines +50 to +57
"Effect": "Allow",
"Principal": {
"Service": "pods.eks.amazonaws.com"
},
"Action": [
"sts:AssumeRole",
"sts:TagSession"
]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are there no conditions in the trust policy? If we're creating an example for others to follow, we should demonstrate best practices, like ensuring that only the specified pods can assume the role. As I read it, this trust policy allows any pod in any EKS cluster in this aws account to assume the role.

Copy link
Member Author

Choose a reason for hiding this comment

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

we don't need any other conditions because you have a pod-identity-association which doing the association between pod identity and this role

Copy link
Member Author

Choose a reason for hiding this comment

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

apiVersion: eks.aws.upbound.io/v1beta1
kind: PodIdentityAssociation
metadata:
  name: example
spec:
  forProvider:
    clusterNameSelector:
      matchLabels:
        testing.upbound.io/example-name: example-cluster
    namespace: crossplane-system
    region: eu-central-1
    roleArnSelector:
      matchLabels:
        testing.upbound.io/example-name: pod-identity
    serviceAccount: provider-aws-xxxxxxxx

Signed-off-by: Christopher Haar <[email protected]>
@wesleyorama2
Copy link

This helped me greatly with getting this up and running using pod identities. Please be sure to also include the findings in:
#1252 in any documentation as I did have to reference it for the trust policy changes that are needed for this.

@aiell0
Copy link
Contributor

aiell0 commented May 10, 2024

Hey there! So I recently dug into Pod Identity and Crossplane and was able to get the following configuration to work:

apiVersion: pkg.crossplane.io/v1beta1
kind: DeploymentRuntimeConfig
metadata:
  name: patch-service-account
spec:
  deploymentTemplate:
    spec:
      selector: {}
      template:
        spec:
          serviceAccountName: crossplane
          containers: []
---
apiVersion: aws.upbound.io/v1beta1
kind: ProviderConfig
metadata:
  name: prod
spec:
  credentials:
    source: IRSA
---
apiVersion: pkg.crossplane.io/v1
kind: Provider
metadata:
  name: provider-aws-s3
spec:
  package: xpkg.upbound.io/upbound/provider-aws-s3:v1.1.0
  runtimeConfigRef:
    name: patch-service-account
---
apiVersion: s3.aws.upbound.io/v1beta1
kind: Bucket
metadata:
  generateName: crossplane-bucket-
spec:
  forProvider:
    region: us-east-1
  providerConfigRef:
    name: prod

The crossplane service account is linked to a pod identity with a role that has Admin privileges (I was just trying to get this to work) and the following trust policy:

apiVersion: s3.aws.upbound.io/v1beta1
kind: Bucket
metadata:
  generateName: crossplane-bucket-
spec:
  forProvider:
    region: us-east-1
  providerConfigRef:
    name: prod

I only bring this up because it is a lot different from the example provided here in that I did not have to use a web identity at all. Is there something I am missing?

@danielloader
Copy link

danielloader commented May 13, 2024

@aiell0 interestingly your example works for me with aws upbound provider v1.1.0, but if I bump the provider to v1.4.0 it falls apart.

The error is: cannot initialize the Terraform plugin SDK async external client: cannot get terraform setup: cache manager failure: cannot calculate the hash for the credentials file: token file name cannot be empty

I think the issue is likely covered in the v1.3.0 release: https://github.com/crossplane-contrib/provider-upjet-aws/releases/tag/v1.3.0

This release also introduces a credential cache for IRSA authentication, which greatly reduces the number of AWS STS calls the provider makes. This cache is currently only employed for IRSA configurations. Please refer to the description #1235 (comment) for the results of some experiments and the observed improvements in those experiments.

Can confirm 1.1.0, 1.2.1 works and 1.3.0, 1.4.0 don't.

Been banging my head on wall trying to understand why your example applied to the cluster worked a treat, and my own in argocd didn't work at all.

Given this, either the change is a regression, or the IRSA mode working with Pod Identity was a fluke (seems like it was) and the cache breaks it when you use the Pod Identity token, which is in a different format to the IRSA one.

We might need a dedicated source for PodIdentity.

Edit: Noticed this was raised above by wesleyorama2 but it doesn't change the fact it's a more explicit configuration of what in 1.2.1 just works implicitly - that is, binding a SA/Namespace to a Pod Identity, just implicitly seems to work with no further configuration - and I'd really like to maintain this decoupling of the IAC that provisions an EKS cluster (terraform in my case) and the AWS Upbound provider running in the cluster, if possible.

Longer term I'd like to deprecate IRSA auth completely in my EKS environments, and potentially drop OIDC at the same time if possible and just move everything into this new paradigm.

@haarchri
Copy link
Member Author

let's double check the Issues with 1.3 and 1.4 and include in the fix an working example without any workaround for hardcoded filesystem files

@haarchri haarchri closed this May 14, 2024
@danielloader
Copy link

Thanks for looking at this for us.

@truongnht
Copy link

@haarchri I am curious if you have update on this issue 🤔

@quanlk2511
Copy link

Hi @haarchri , guys, do we have progress on this?

@haarchri haarchri deleted the feature/providerconfig-pod-identities branch August 13, 2024 11:37
@haarchri
Copy link
Member Author

please check out this: #1459

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.

Support Pod Identity Controller associations for provider IAM permissions
8 participants