change: Adds k8s secret storage to helm tokengen job #10713
Conversation
|
|
To create a k8s secret we need a Role and RoleBinding that can perform the action. Only adds if enterprise and tokengen are enabled, and federation frontend hasn't disabled other components (prevents tokengen from running in federation-only mode). Signed-off-by: Ryan Brady <ryan.brady@grafana.com>
The tokengen job will now store the result in a k8s secret as it does with Loki. Signed-off-by: Ryan Brady <ryan.brady@grafana.com>
Signed-off-by: Ryan Brady <ryan.brady@grafana.com>
Signed-off-by: Ryan Brady <ryan.brady@grafana.com>
… why it fails call-lint-test in CI Signed-off-by: Ryan Brady <ryan.brady@grafana.com>
Signed-off-by: Ryan Brady <ryan.brady@grafana.com>
Signed-off-by: Ryan Brady <ryan.brady@grafana.com>
Signed-off-by: Ryan Brady <ryan.brady@grafana.com>
Signed-off-by: Ryan Brady <ryan.brady@grafana.com>
Signed-off-by: Ryan Brady <ryan.brady@grafana.com>
Signed-off-by: Ryan Brady <ryan.brady@grafana.com>
rbrady
force-pushed
the
rbrady/218045-store-tokengen-secrets
branch
from
04cf96f to
dc61d91
Compare
|
From the helm test in CI: Admin token generated successfully |
| */}} | ||
| {{- define "mimir.kubectlImage" -}} | ||
| {{ .Values.kubectlImage.repository }}:{{ .Values.kubectlImage.tag | default "latest" }} | ||
| {{- end -}} |
There was a problem hiding this comment.
| {{- end -}} | |
| {{- end -}} | |
| - | | ||
| if [ -f /shared/admin-token ]; then | ||
| echo "Admin token generated successfully" | ||
| {{- if .Values.tokengenJob.storeTokenInSecret | default true }} |
There was a problem hiding this comment.
isn't this true by default already due to the values.yaml? why do we need the |default true?
| # -- Overrides the image tag whose default is the chart's appVersion | ||
| tag: null | ||
| # -- Overrides the image tag with an image digest | ||
| digest: null |
There was a problem hiding this comment.
none of the other images have the digest overridable. Why is this necessary here?
There was a problem hiding this comment.
I'm porting the feature from Loki and the digest override is present there:
https://github.com/grafana/loki/blob/c99771efb875abd905a77c917a5805a6228a4315/production/helm/loki/values.yaml#L620-L630
There was a problem hiding this comment.
ok, but this isn't even used in this chart. I'd rather be consistent within this chart than semi-consistent with another chart.
| {{- end }} | ||
| {{- end }} | ||
| securityContext: | ||
| {{- toYaml .Values.tokengenJob.containerSecurityContext | nindent 12 }} | ||
| containers: |
There was a problem hiding this comment.
is there a way to make this somehow conditional? Otherwise, this will add an additional image to GEM installations. I think some people running GEM are very frugal for what images they allow pulled.
Maybe we can have a regular container which polls for the token in the shared location and then uploads it. That way we don't deploy a new image only to echo "skipping creating secret"
| # -- Controls whether to store the generated admin token in a Kubernetes secret | ||
| storeTokenInSecret: true | ||
| # -- Name of the secret to store the admin token. If not specified, defaults to "<release-name>-admin-token" | ||
| adminTokenSecret: "" |
There was a problem hiding this comment.
can we have the default string here instead? it's self-documenting and it won't go out of date
| kind: Role | ||
| name: {{ include "mimir.resourceName" (dict "ctx" . "component" "tokengen") }} | ||
| apiGroup: rbac.authorization.k8s.io | ||
| {{- end -}} |
There was a problem hiding this comment.
can you add a new line?
| {{- toYaml .Values.tokengenJob.annotations | nindent 4 }} | ||
| {{- end }} | ||
| namespace: {{ .Release.Namespace | quote }} | ||
| {{- end }} |
There was a problem hiding this comment.
can you add a new line?
| @@ -29,6 +29,7 @@ Entries should include a reference to the Pull Request that introduced the chang | |||
|
|
|||
| ## main / unreleased | |||
|
|
|||
| * [CHANGE] Tokengen: Added k8s secret storage for the admin token. #5237 | |||
There was a problem hiding this comment.
can you clarify that now the token is stored in a secret by default?
| kubectl image reference | ||
| */}} | ||
| {{- define "mimir.kubectlImage" -}} | ||
| {{ .Values.kubectlImage.repository }}:{{ .Values.kubectlImage.tag | default "latest" }} |
There was a problem hiding this comment.
It's less surprising if the tag is just latest in the default values.
What this PR does
Adds k8s secret storage to the tokengen job exactly as Loki has it. This is needed for FedRAMP deployment and day 2 operations.
Which issue(s) this PR fixes or relates to
Fixes #5237
Fixes #deployment_tools/218045
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX].about-versioning.mdupdated with experimental features.