Skip to content

Using projected volume mount to read SA token#41

Draft
sadovnikov wants to merge 2 commits into
nmaupu:masterfrom
sadovnikov:master
Draft

Using projected volume mount to read SA token#41
sadovnikov wants to merge 2 commits into
nmaupu:masterfrom
sadovnikov:master

Conversation

@sadovnikov

Copy link
Copy Markdown

This PR fixes #40 without the need to create kubernetes.io/service-account-token secrets and link them to SA's.
The projected volume mounts also work with earlier versions of Kubernetes.

A new image was built with docker build . -t xxxxxxx/vault-secret:0.0.1-fix02 and tested with k8s 1.24 and 1.23

Comment thread pkg/k8sutils/resources.go
)

// GetTokenFromSA gets the token associated to the first secret located in a k8s' service account
func GetTokenFromSA(cli client.Client, ns, saName string) (string, error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I might be completely misunderstanding this ... but with this change it seems to me that now we are just reading the secret for the SA the vault-secrets-manager pod is running with instead of the one defined in the CR

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, you are right! I missed this part :(

We deploy the vault-secrets-manager to every namespace (with a single value for WATCH_NAMESPACE env) and then using the SA of the operator makes more sense... However, indeed, this kills the possibility of using single-operator installation and keeping more granular access to Vault. I guess, the PR will be declined

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Yes security wise, it's bad. The operator should connect on behalf of the client SA.

I currently have very little access to internet and only have my phone, so I can't really work on that before Sept. I am afraid... I can merge PR though ;)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Merging this PR will break installations, which watch multiple namespaces. I'm converting the PR into a draft

@sadovnikov sadovnikov marked this pull request as draft February 22, 2023 09:44
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.

Kubernetest 1.24 Compatibility

3 participants