Add secret generation to main helm chart. #292

ainmosni · 2023-05-23T10:11:39Z

This creates a generic secret template that can be used to generate
secrets for all services. The secret template takes a dict of the scope,
the name of the secret, and the data to be stored in the secret.

This will be used to prefill any non-predefined secrets that are set in
the secretRefs.

Fixes #50

d7oc

I think @wkloucek wanted to use Jobs instead of lookup as he was afraid of race conditions with lookup and lookup won't work with helm template.

mmattel · 2023-05-31T15:33:28Z

This is a very cool feature. My intention is to have that somehow documented/mentioned in the admin docs. Any ideas?

ainmosni · 2023-05-31T15:37:49Z

This is a very cool feature. My intention is to have that somehow documented/mentioned in the admin docs. Any ideas?

@mmattel In theory, all that needs to be documented is that people don't need to add most secrets by default, only when they want to customise certain things. So it's more moving current docs around, and even removing some unneeded examples because they are for existing users.

We can chat about it, but I'll be gone until Tuesday, so if you can wait until then, I'll help.

mmattel · 2023-05-31T15:40:45Z

Tuesday is fine, I am just blocked with other stuff.

d7oc

Taking the ownership of externally-defined Secrets and ConfigMaps fails here:

Error: UPGRADE FAILED: rendered manifests contain a resource that already exists. Unable to continue with update: Secret "machine-auth-api-key" in namespace "default" exists and cannot be imported into the current release: invalid ownership metadata; label validation error: missing key "app.kubernetes.io/managed-by": must be set to "Helm"; annotation validation error: missing key "meta.helm.sh/release-name": must be set to "owncloud"; annotation validation error: missing key "meta.helm.sh/release-namespace": must be set to "default"
helm.go:84: [debug] Secret "machine-auth-api-key" in namespace "default" exists and cannot be imported into the current release: invalid ownership metadata; label validation error: missing key "app.kubernetes.io/managed-by": must be set to "Helm"; annotation validation error: missing key "meta.helm.sh/release-name": must be set to "owncloud"; annotation validation error: missing key "meta.helm.sh/release-namespace": must be set to "default"

Aside this the following places contain secrets which are not handled so far:

ocis-charts/charts/ocis/templates/appprovider/deployment.yaml

Line 79 in 9f85e54

name: {{ $.Values.secretRefs.jwtSecretRef }}
ocis-charts/charts/ocis/templates/policies/deployment.yaml

Line 78 in 9f85e54

name: {{ $.Values.secretRefs.jwtSecretRef }}
ocis-charts/charts/ocis/templates/storageusers/deployment.yaml

Line 97 in 9f85e54

name: {{ .Values.secretRefs.s3CredentialsSecretRef }}
ocis-charts/charts/ocis/templates/storageusers/deployment.yaml

Line 102 in 9f85e54

name: {{ .Values.secretRefs.s3CredentialsSecretRef }}
ocis-charts/charts/ocis/templates/storageusers/jobs.yaml

Line 69 in 9f85e54

name: {{ .Values.secretRefs.s3CredentialsSecretRef }}
ocis-charts/charts/ocis/templates/storageusers/jobs.yaml

Line 74 in 9f85e54

name: {{ .Values.secretRefs.s3CredentialsSecretRef }}

Other comments are inline

charts/ocis/templates/_common/_configvalues.tpl

charts/ocis/templates/_common/_tplvalues.tpl

charts/ocis/templates/authmachine/secret.yaml

charts/ocis/templates/storagesystem/secret.yaml

charts/ocis/templates/idm/secret.yaml

charts/ocis/templates/idp/secret.yaml

ainmosni · 2023-06-06T10:07:01Z

Taking the ownership of externally-defined Secrets and ConfigMaps fails here:

Error: UPGRADE FAILED: rendered manifests contain a resource that already exists. Unable to continue with update: Secret "machine-auth-api-key" in namespace "default" exists and cannot be imported into the current release: invalid ownership metadata; label validation error: missing key "app.kubernetes.io/managed-by": must be set to "Helm"; annotation validation error: missing key "meta.helm.sh/release-name": must be set to "owncloud"; annotation validation error: missing key "meta.helm.sh/release-namespace": must be set to "default"
helm.go:84: [debug] Secret "machine-auth-api-key" in namespace "default" exists and cannot be imported into the current release: invalid ownership metadata; label validation error: missing key "app.kubernetes.io/managed-by": must be set to "Helm"; annotation validation error: missing key "meta.helm.sh/release-name": must be set to "owncloud"; annotation validation error: missing key "meta.helm.sh/release-namespace": must be set to "default"

Aside this the following places contain secrets which are not handled so far:

* https://github.com/owncloud/ocis-charts/blob/9f85e54faaf86f628328d3d56497c47035bafb5f/charts/ocis/templates/appprovider/deployment.yaml#L79

* https://github.com/owncloud/ocis-charts/blob/9f85e54faaf86f628328d3d56497c47035bafb5f/charts/ocis/templates/policies/deployment.yaml#L78

* https://github.com/owncloud/ocis-charts/blob/9f85e54faaf86f628328d3d56497c47035bafb5f/charts/ocis/templates/storageusers/deployment.yaml#L97

* https://github.com/owncloud/ocis-charts/blob/9f85e54faaf86f628328d3d56497c47035bafb5f/charts/ocis/templates/storageusers/deployment.yaml#L102

* https://github.com/owncloud/ocis-charts/blob/9f85e54faaf86f628328d3d56497c47035bafb5f/charts/ocis/templates/storageusers/jobs.yaml#L69

* https://github.com/owncloud/ocis-charts/blob/9f85e54faaf86f628328d3d56497c47035bafb5f/charts/ocis/templates/storageusers/jobs.yaml#L74

Other comments are inline

These are all fixed, looking for a way around the first problem you mentioned now.

This creates a generic secret template that can be used to generate secrets for all services. The secret template takes a dict of the scope, the name of the secret, and the data to be stored in the secret. This will be used to prefill any non-predefined secrets that are set in the secretRefs. Fixes #50

This empties the value of adminUserSecretRef, which will signal that the secret should be generated by the chart. I've also started a template to be inserted where we used to use the value of adminUserSecretRef.

Co-authored-by: Dennis Sieben <[email protected]>

mmattel · 2023-06-07T13:45:23Z

@ainmosni Minor text bug in commit Update docs.:

If not filled in, will attempt to to use values in .storageusers.storageBackend.s3.driverConfig.s3ng instead.

to to --> to

mmattel · 2023-06-14T08:13:42Z

Maybe needs a rebase because merging of 305/306/307/308 yesterday.

d7oc

PR is fine for me after discussion. Ownership stuff will be documented.

mmattel · 2023-06-14T12:21:54Z

Ownership stuff will be documented.

Pls tell more so docs can address this.

d7oc · 2023-06-14T12:29:03Z

Ownership stuff will be documented.

Pls tell more so docs can address this.

My understanding was that the text fro this will be provided from @ainmosni or @wkloucek. If that's not the case I could also jump in here if needed.

ainmosni requested review from d7oc, Excds and wkloucek May 23, 2023 10:12

d7oc reviewed May 23, 2023

View reviewed changes

ainmosni marked this pull request as ready for review May 31, 2023 14:40

d7oc requested changes Jun 2, 2023

View reviewed changes

mmattel mentioned this pull request Jun 7, 2023

Helm Chart Breaking Changes from 0.1.0 to 0.3.0 #304

Closed

ainmosni and others added 12 commits June 7, 2023 15:20

Empty out adminUserSecretRef.

86728c2

This empties the value of adminUserSecretRef, which will signal that the secret should be generated by the chart. I've also started a template to be inserted where we used to use the value of adminUserSecretRef.

Add secret files.

3f64402

Finish all the secret/config maps conversion.

6349d86

Format multi-secrets better

b0ad4c9

Add needed values for CI.

d5aaa40

Apply suggestions from code review

f6a8646

Co-authored-by: Dennis Sieben <[email protected]>

Add spaces after quote pipes.

51098d1

Fix issues found by @d7oc.

9baf052

Change jwtSecrets to templated.

9f5c863

Add missing secret templates.

e38f629

Move quoting to where it's actually used.

3430acb

ainmosni force-pushed the ainmosni/issue/50-make-it-easier-to-set-up-secrets branch from 516ce46 to 3430acb Compare June 7, 2023 13:25

Update docs.

d599d80

Fix typo.

f3d72bf

wkloucek mentioned this pull request Jun 7, 2023

use service names #301

Merged

Fix bugs found in more extensive testing.

07c36a8

d7oc self-requested a review June 14, 2023 12:16

d7oc approved these changes Jun 14, 2023

View reviewed changes

ainmosni merged commit 0a4a6f4 into master Jun 14, 2023

delete-merged-branch bot deleted the ainmosni/issue/50-make-it-easier-to-set-up-secrets branch June 14, 2023 12:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add secret generation to main helm chart. #292

Add secret generation to main helm chart. #292

ainmosni commented May 23, 2023 •

edited

Loading

d7oc left a comment

mmattel commented May 31, 2023

ainmosni commented May 31, 2023

mmattel commented May 31, 2023

d7oc left a comment

ainmosni commented Jun 6, 2023

mmattel commented Jun 7, 2023 •

edited

Loading

mmattel commented Jun 14, 2023

d7oc left a comment

mmattel commented Jun 14, 2023

d7oc commented Jun 14, 2023 •

edited

Loading

Add secret generation to main helm chart. #292

Add secret generation to main helm chart. #292

Conversation

ainmosni commented May 23, 2023 • edited Loading

d7oc left a comment

Choose a reason for hiding this comment

mmattel commented May 31, 2023

ainmosni commented May 31, 2023

mmattel commented May 31, 2023

d7oc left a comment

Choose a reason for hiding this comment

ainmosni commented Jun 6, 2023

mmattel commented Jun 7, 2023 • edited Loading

mmattel commented Jun 14, 2023

d7oc left a comment

Choose a reason for hiding this comment

mmattel commented Jun 14, 2023

d7oc commented Jun 14, 2023 • edited Loading

ainmosni commented May 23, 2023 •

edited

Loading

mmattel commented Jun 7, 2023 •

edited

Loading

d7oc commented Jun 14, 2023 •

edited

Loading