Skip to content
This repository has been archived by the owner on Feb 22, 2022. It is now read-only.

[incubator/vault] Add support for creating a service account #9429

Merged
merged 3 commits into from
Apr 22, 2019
Merged

[incubator/vault] Add support for creating a service account #9429

merged 3 commits into from
Apr 22, 2019

Conversation

jbialy
Copy link
Contributor

@jbialy jbialy commented Nov 20, 2018 •
edited
Loading

What this PR does / why we need it:

In situations where we want Vault to have access to the api server a service account is required. An example for this is when using Vault's Kubernetes auth method.

https://www.vaultproject.io/docs/auth/kubernetes.html#configuring-kubernetes

This PR enables service account creation if needed.

Special notes for your reviewer:

Checklist

  • DCO signed
  • Chart Version bumped
  • Variables are documented in the README.md

@helm-bot helm-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 20, 2018
@jbialy
Copy link
Contributor Author

jbialy commented Nov 20, 2018

/assign @viglesiasce

@helm-bot helm-bot added Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 20, 2018
@jbialy
Copy link
Contributor Author

jbialy commented Dec 5, 2018

/assign @seanknox

@helm-bot helm-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 17, 2018
@jbialy
Copy link
Contributor Author

jbialy commented Dec 17, 2018

👀 PTAL!

@jbialy
Copy link
Contributor Author

jbialy commented Dec 27, 2018

/assign

@helm-bot helm-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 21, 2019
@jbialy
Copy link
Contributor Author

jbialy commented Jan 21, 2019

Bumped chart version

@helm-bot helm-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). labels Jan 21, 2019
@viglesiasce
Copy link
Contributor

/ok-to-test

@viglesiasce
Copy link
Contributor

@jpds can you take a look?

@stale
Copy link

stale bot commented Mar 3, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

@stale stale bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 3, 2019
@helm-bot helm-bot removed the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 7, 2019
@qlikcoe
Copy link
Contributor

qlikcoe commented Mar 19, 2019

@jbialy , There is a conflict because the chart version was bumped by other PRs...

@helm-bot helm-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). labels Mar 20, 2019
@jbialy
Copy link
Contributor Author

jbialy commented Mar 20, 2019

The tests are ✅. @viglesiasce @unguiculus @jpds . Can you guys give this a /lgtm ?

Janusz Bialy added 2 commits March 25, 2019 11:38
@jbialy
option for creating a service account
9f05788 
Signed-off-by: Janusz Bialy <janusz.bialy@qlik.com>
@jbialy
bump chart version
c8e4712 
Signed-off-by: Janusz Bialy <janusz.bialy@qlik.com>
@jbialy
Copy link
Contributor Author

jbialy commented Apr 2, 2019

Bump! PTAL!

@mattfarina
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 22, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jbialy, mattfarina

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 22, 2019
@k8s-ci-robot k8s-ci-robot merged commit f1416a2 into helm:master Apr 22, 2019
hajnej pushed a commit to hajnej/charts that referenced this pull request Apr 24, 2019
@jbialy @hajnej
[incubator/vault] Add support for creating a service account (helm#9429)
4ec0ba3 
* option for creating a service account

Signed-off-by: Janusz Bialy <janusz.bialy@qlik.com>

* bump chart version

Signed-off-by: Janusz Bialy <janusz.bialy@qlik.com>
@jbialy jbialy deleted the add-sa branch April 25, 2019 13:54
devnulled pushed a commit to devnulled/charts that referenced this pull request Apr 25, 2019
@jbialy @devnulled
[incubator/vault] Add support for creating a service account (helm#9429)
380f205 
* option for creating a service account

Signed-off-by: Janusz Bialy <janusz.bialy@qlik.com>

* bump chart version

Signed-off-by: Janusz Bialy <janusz.bialy@qlik.com>
dermorz pushed a commit to dermorz/charts that referenced this pull request Apr 26, 2019
@jbialy
[incubator/vault] Add support for creating a service account (helm#9429)
e370ad7 
* option for creating a service account

Signed-off-by: Janusz Bialy <janusz.bialy@qlik.com>

* bump chart version

Signed-off-by: Janusz Bialy <janusz.bialy@qlik.com>
therealgambo
therealgambo reviewed Apr 29, 2019
View reviewed changes
Copy link

@therealgambo therealgambo left a comment

Choose a reason for hiding this comment

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

This line is causing a templating error because the template function doesn't exist.

incubator/vault/templates/serviceaccount.yaml
name: {{ template "vault.serviceAccountName" . }}
labels:
app: {{ template "vault.name" . }}
chart: {{ template "vault.chart" . }}
Copy link

Choose a reason for hiding this comment

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

This is incorrect as the vault.chart template doesn't exist - did you forget to add it?

Or you can use what the other vault manifests use: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}

Any chance of a quick fix for this @jbialy ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, good catch! I likely grabbed the serviceaccount definition from another chart, and overlooked the way the chart label is set!

I'll adjust it to use {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }} as you suggested!

@jbialy jbialy mentioned this pull request Apr 29, 2019
Merged
3 tasks
dpkirchner pushed a commit to dpkirchner/charts that referenced this pull request May 9, 2019
@jbialy @dpkirchner
[incubator/vault] Add support for creating a service account (helm#9429)
42bad80 
* option for creating a service account

Signed-off-by: Janusz Bialy <janusz.bialy@qlik.com>

* bump chart version

Signed-off-by: Janusz Bialy <janusz.bialy@qlik.com>
goshlanguage pushed a commit to goshlanguage/charts that referenced this pull request May 17, 2019
@jbialy @goshlanguage
[incubator/vault] Add support for creating a service account (helm#9429)
9ab42ad 
* option for creating a service account

Signed-off-by: Janusz Bialy <janusz.bialy@qlik.com>

* bump chart version

Signed-off-by: Janusz Bialy <janusz.bialy@qlik.com>
eyenx pushed a commit to eyenx/charts that referenced this pull request May 28, 2019
@jbialy @eyenx
[incubator/vault] Add support for creating a service account (helm#9429)
0355dca 
* option for creating a service account

Signed-off-by: Janusz Bialy <janusz.bialy@qlik.com>

* bump chart version

Signed-off-by: Janusz Bialy <janusz.bialy@qlik.com>
@jbialy jbialy mentioned this pull request Jun 18, 2019
Merged
3 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@therealgambo therealgambo therealgambo left review comments

@mattfarina mattfarina mattfarina approved these changes

@foxish foxish Awaiting requested review from foxish

@lachie83 lachie83 Awaiting requested review from lachie83

Assignees

@mattfarina mattfarina

@viglesiasce viglesiasce

@seanknox seanknox

@jbialy jbialy

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). lgtm Indicates that a PR is ready to be merged. ok-to-test size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@jbialy @viglesiasce @qlikcoe @mattfarina @k8s-ci-robot @therealgambo @seanknox @helm-bot