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

Make serviceAccount name a configuration option #367

Merged
merged 9 commits into from
Aug 19, 2020

Conversation

burningalchemist
Copy link
Contributor

This PR is a continuation of #338 with fixed tests to finally bring the feature to the chart.

@hashicorp-cla
Copy link

hashicorp-cla commented Jul 31, 2020

CLA assistant check
All committers have signed the CLA.

@burningalchemist
Copy link
Contributor Author

@jasonodonnell I've fixed the tests, but CLA on @davidholsgrove side seems to be a blocker now. 😢

@davidholsgrove
Copy link
Contributor

Apologies, working through approvals to sign the CLA at the moment. Happy for anyone to take over and get this merged if that's an option too.
Thanks!

burningalchemist and others added 4 commits August 8, 2020 12:36
Co-authored-by: David Holsgrove <david@apnic.net>
to address hashicorp#56 (review)

Co-authored-by: David Holsgrove <david@apnic.net>
Co-authored-by: David Holsgrove <david@apnic.net>
Copy link
Member

@tvoran tvoran left a comment

Choose a reason for hiding this comment

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

Thanks for pushing this forward! I tested it a bit and it seems to work. Just left a couple suggestions.

values.yaml Outdated Show resolved Hide resolved
test/unit/server-serviceaccount.bats Outdated Show resolved Hide resolved
test/unit/server-serviceaccount.bats Outdated Show resolved Hide resolved
@burningalchemist
Copy link
Contributor Author

Hi @tvoran ! Thanks for your review, please take a look, whenever you have time.

Copy link
Member

@tvoran tvoran left a comment

Choose a reason for hiding this comment

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

Nicely done, thanks again!

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