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

Auth config block supports common arguments from env and flags #577

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

uchanchlani
Copy link

Auth config block supports common custom arguments via env variables and CLI flags. Feature #576

Also fixes using AWS IRSA token by mistake if both included in the pod's volume #544 This maybe a better fix then the proposed #545 pull request as this is likely more future-proof
to other third party k8s provider launching their own Service Account Token injection, assuming
the third party k8s provider will follow the unsaid convention of injecting the token in the
<third.party.url>/serviceaccount/token path

@hashicorp-cla
Copy link

hashicorp-cla commented Jan 17, 2024

CLA assistant check
All committers have signed the CLA.

uchanchlani pushed a commit to uchanchlani/vault-k8s that referenced this pull request Jan 17, 2024
…orp#577

Also fixes using AWS IRSA token by mistake if both included in the pod's volume hashicorp#544
This maybe a better fix then the proposed hashicorp#545 pull request as this is likely more future-proof
 to other third party k8s provider launching their own Service Account Token injection, assuming
 the third party k8s provider will follow the unsaid convention of injecting the token in the
 <third.party.url>/serviceaccount/token path
@uchanchlani uchanchlani force-pushed the feature/vault-auth-config-custom-args branch from 224c1ee to 804f57a Compare January 17, 2024 03:38
…orp#577

Also fixes using AWS IRSA token by mistake if both included in the pod's volume hashicorp#544
This maybe a better fix then the proposed hashicorp#545 pull request as this is likely more future-proof
 to other third party k8s provider launching their own Service Account Token injection, assuming
 the third party k8s provider will follow the unsaid convention of injecting the token in the
 <third.party.url>/serviceaccount/token path
@uchanchlani uchanchlani force-pushed the feature/vault-auth-config-custom-args branch from 804f57a to 654386a Compare January 17, 2024 03:47
@uchanchlani
Copy link
Author

@benashz : Can you please review this PR when you get a chance?
I have tested this via unit tests (make test) and on my local minikube K8s cluster.
This is my first-time raising a PR for this repo and Hashicorp org in general. I am not sure if I need to follow any other processes to get traction on this PR.

I am guessing the automated git checks will run after it gets a vote from the maintainers.

@uchanchlani
Copy link
Author

Tagging @tvoran @VioletHynes to get more visibility into this.

@VioletHynes
Copy link

Hey! Thanks for the heads up. This isn't my area, but I'll try and get somebody's eyes on this. Thanks for the contribution!

@adrien-f
Copy link

Hello,

I confirm we are also impacted by this issue, most notably when also running Istio (when configured for the Istio sidecar being the first container).

Istio does not add the kube-api-access-xxxx volume to its volume mounts but the IRSA webhook will add the aws-iam-token volumeMount just after which will be picked up as the Istio sidecar is the first container in the list and cause the pod to be invalid with a duplicated volume mount error:

Error creating: Pod "xxx" is invalid: spec.initContainers[0].volumeMounts[2].mountPath: Invalid value: "/var/run/secrets/eks.amazonaws.com/serviceaccount": must be unique

@uchanchlani
Copy link
Author

@adrien-f : Thank you for confirming you are also facing this issue. I am hoping this fix should solve your issue.
@VioletHynes : can we please get some eyes on this from the hashicorp organization so I can get this contribution merged in. Thank you

@heatherezell
Copy link

Hi there @uchanchlani! Thanks for this PR. I've flagged it for our Vault Ecosystem team, and they'll be adding it to their next refinement meeting for further discussion on when this could be merged. Please note that the team is tight on time due to other committed deliverables, so I can't guarantee a timeline on final outcomes. We definitely appreciate your patience. I'll ask the engineer who picks this up to make sure to give a ping at that time to get any conflicts resolved as well.

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.

5 participants