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

fix: allow GH priv key from env in helm chart #884

Merged
merged 6 commits into from
Dec 14, 2021

Conversation

apr-1985
Copy link
Contributor

Allowing the ability to get the GH priv key from the env when using the helm chart.
Feature Code added in #383

I have tested switching between Env and file by toggling the new helm chart variable and this works without issue.

This feature increases the interop with secrets managers that inject to ENV at run time.

@stale
Copy link

stale bot commented Nov 17, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 17, 2021
@apr-1985
Copy link
Contributor Author

Is there anything else I need to do get this reviewed please?

@stale stale bot removed the stale label Nov 17, 2021
@marcencov
Copy link

I have the same issue - we are using vault and all secrets must be passed as ENV (not as file /etc/actions-runner-controller/github_app_private_key).
Is there any chance to review/merge this PR?
Thanks

@mumoshu mumoshu added this to the v0.21.0 milestone Nov 17, 2021
@mumoshu
Copy link
Collaborator

mumoshu commented Nov 17, 2021

We're going to review and merge this before our next controller minor release! Thanks for your patience.

@mumoshu
Copy link
Collaborator

mumoshu commented Nov 17, 2021

@Marchenkov Hey! How does this PR solve your issue?

we are using vault and all secrets must be passed as ENV

If you're using vault-k8s' injector you should get files instead of envvars.
How are you integrating vault with actions-runner-controller in your setup?

@apr-1985
Copy link
Contributor Author

We're going to review and merge this before our next controller minor release! Thanks for your patience.

Many thanks mainly commented to stop the stale bot 😀

@marcencov
Copy link

@Marchenkov Hey! How does this PR solve your issue?

we are using vault and all secrets must be passed as ENV

If you're using vault-k8s' injector you should get files instead of envvars. How are you integrating vault with actions-runner-controller in your setup?

I'm use vault-secrets-webhook and no way to inject to file.
https://banzaicloud.com/docs/bank-vaults/mutating-webhook/configuration/
So this PR can solve my problem, because I can read PEM from secret (now I receive error that file /etc/actions-runner-controller/github_app_private_key not contain PEM cert).

@mumoshu
Copy link
Collaborator

mumoshu commented Dec 12, 2021

@Marchenkov I was also wondering if this hard-coded env name and secretKeyRef values might affect your use-case

key: github_app_private_key
name: {{ include "actions-runner-controller.secretName" . }}

But after reading how bank-vaults mutating webhook works, perhaps that's also no issue.

Thanks for confirming!

@mumoshu
Copy link
Collaborator

mumoshu commented Dec 12, 2021

I'm now wondering if authSecret.enabled=false added via #937 with the existing env helm value can be an alternative to your use-case @apr-1985 @Marchenkov WDYT?

Copy link
Collaborator

@mumoshu mumoshu left a comment

Choose a reason for hiding this comment

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

Merging. Thanks for your contribution and support @apr-1985 @Marchenkov ☺️

@mumoshu mumoshu merged commit 6f51f56 into actions:master Dec 14, 2021
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.

3 participants