feat: Helm - custom service account creation and management #17880
Conversation
|
Could you bump version of the chart manually ? |
|
Sure! It's done :) |
wiktor2200
changed the title
|
@amitmiran137 there is something wrong with workflow. It shows error in code which I didn't change. |
| # Create custom service account for Superset. If create: true and name is not provided, superset.fullname will be used. | ||
| # serviceAccountName: superset | ||
| serviceAccount: | ||
| create: false |
There was a problem hiding this comment.
How about:
- Remove "serviceAccountName"
- creating serviceAccount sestion as you did with two properties:
- create as you did
- name
According to: https://helm.sh/docs/chart_best_practices/rbac/#yaml-configuration
There was a problem hiding this comment.
I've thought about it. That was even my first choice, but then I've noticed that, there was ServiceAccountName variable added in the past #15340, so I've implemented new feature as enhancement beside the current solution.
I don't want to destroy any configs that can exist somewhere.
@mvoitko and @craig-rueda can you give some more feedback about this?
There was a problem hiding this comment.
Yeps you are right, we would break backward compatibility but on the other site, we could maintain both for sometime and prepare a communication about that right now.
There was a problem hiding this comment.
If someone have blocked version on helm in their definition it's OK, but when someone is using "latest" it will brake down deployment (or dependent components).
Supporting two solutions for a while should be quite easy (I'll just add one more conditions in _helpers) but this could cause some caveats such as overwriting values if serviceAccountName and serviceAccount.name would be defined in the same time. 🤔
There was a problem hiding this comment.
I think it LGTM. If people are using "latest" in any env that matters, that's generally a bad practice. Using the default of create: false seems reasonable to me.
|
@przemyslawandruszewski I've added support for custom service account in init-job in this commit: 372fed1 |
Yes we became aware lately |
|
@wiktor2200 I cannot find any template for creating service account when serviceAccount.create == true. Could you validate whether you commited all the stuff? |
|
@przemyslawandruszewski you got right, thanks for notifying that. |
|
The rest is fine for me, let's leave final decision for @craig-rueda :) |
|
@craig-rueda Pipeline failed because of missing license header. I've added it, could you re-approve? :) |
|
@wiktor2200 looks like you can merge :) |
|
Unfortunately I can't. :( It has to be clicked by someone with write access to repo. |
Sorry then :) So the request goes to @craig-rueda :) |
…7880) * feat: Custom service account creation and management * bump helm chart version * add custom service account in init-job * service account creation template * changed service account creation template * add license
…7880) * feat: Custom service account creation and management * bump helm chart version * add custom service account in init-job * service account creation template * changed service account creation template * add license
mistercrunch
added
🏷️ bot
SUMMARY
Possibility to create custom service account in helm chart.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
I've tested all possible changes on parameters:
All conditionals in
_helpersfile works as expected.ADDITIONAL INFORMATION