feat(fluent-bit): Add automountServiceAccountToken override at pod level#632
Closed
gsmith-sas wants to merge 6 commits intofluent:mainfrom
Closed
feat(fluent-bit): Add automountServiceAccountToken override at pod level#632gsmith-sas wants to merge 6 commits intofluent:mainfrom
gsmith-sas wants to merge 6 commits intofluent:mainfrom
Conversation
…level Signed-off-by: Greg Smith <Greg.Smith@sas.com>
Signed-off-by: Greg Smith <Greg.Smith@sas.com>
Collaborator
|
Thanks for the PR @gsmith-sas. This is my preferred pattern, although I wish I'd requested the value added in #629 be |
* Add securityContext to reloader pod Signed-off-by: relusc <relusc@gmail.com> * Add securityContext to values and bump version Signed-off-by: relusc <r.schach96@gmail.com> * Update changelog Signed-off-by: relusc <r.schach96@gmail.com> * Set default securityContext values Signed-off-by: relusc <r.schach96@gmail.com> --------- Signed-off-by: relusc <relusc@gmail.com> Signed-off-by: relusc <r.schach96@gmail.com> Co-authored-by: relusc <relusc@gmail.com>
Collaborator
|
@gsmith-sas could you rebase this PR? |
…level Signed-off-by: Greg Smith <Greg.Smith@sas.com>
Signed-off-by: Greg Smith <Greg.Smith@sas.com>
Signed-off-by: Greg Smith <Greg.Smith@sas.com>
Author
|
{heavy sigh} Looks like I messed up the rebase and lost the DCO on the last commit as well. 😦 Closing this PR and have opened a new clean(er) one (#633) Sorry for the churn. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change allows users to override the
automountServiceAccountTokensetting at the pod level. The Kubernetes API allows this property to be set at two levels:ServiceAccount level - when set at the ServiceAccount level, the setting impacts all pods associated with the ServiceAccount. The default value for this property is "true" meaning that the API token of the ServiceAccount is automatically mounted to any pod associated with the ServiceAccount.
Pod level - when set at the Pod level, the setting only impacts that specific pod. When set at both levels, the Pod-level setting overrides the ServiceAccount level.
As described in PR #629, many sites require that this property be disabled for security reasons since the default value of "true" makes API tokens available by default. And, some security scans will flag this default behavior as potentially problematic. The change in PR #629 allows users to disable this setting and, when disabled, disables it at both the ServiceAccount and Pod levels.
However, there are situations where it is appropriate to disable this setting at the ServiceAccount level but enable it at the Pod level. For example, when configuring Fluent Bit to collect Kubernetes Events, access to the API tokens is required. The solution is to disable the setting at the ServiceAccount level (which PR#629 now permits) but enable it at the Pod level (which PR#629 does not permit). This PR is designed to allow users to enable this.
In this PR, I chose to name the new property
automountServiceAccountTokeneven though that is very similr to the new property added in PR #629 (which addedserviceAccount.automountServiceAccountToken). While this is potentially confusing, I think aligning the names of the Helm chart values with the properties they impact in the underlying Kubernetes resources is critical. This naming is consistent with other Helm charts I have reviewed (e.g. the Grafana Helm chart that support setting this property at both the ServiceAccount and Pod levels. I have also attempted to minimize confusion by adding comments immediately before both properties in thevalues.yamlfile explaining the scope of each property.