Fix: Avoid recreating random secrets

[xoanmi]

Avoid recreating secrets with dynamic/random content on every helm update. This causes the core, registry and jobservice services to be redeployed in each update.

Related to https://cloud-native.slack.com/archives/CC1E09J6S/p1689925408303219

Close #1549

[Cjericho4]

Anyway we can get this reviewed so it can get merged in?

[Kajot-dev]

Actually, I think that tls.key and tls.cert can be defined using existing Secret via core.secretName which is something that is separate from the secret created by the chart (harbor.core template). This PR removes it and assumes that all the required values are already present it the secret if it exists.

While this PR will work for new deployment it removes defining certificate for token signing from external secret (you can use CertManager to do just that) - it will break the installation for those who already use core.secretName by not using it and inserting nil values into the -core secret (since -core secret exists and does not have tls.cert and tls.key)

The idea is good though - but it needs to preserve the core.secretName feature and it needs to check for presence of each individual value inside the secret.

[xoanmi]

Actually, I think that tls.key and tls.cert can be defined using existing Secret via core.secretName which is something that is separate from the secret created by the chart (harbor.core template). This PR removes it and assumes that all the required values are already present it the secret if it exists.

While this PR will work for new deployment it removes defining certificate for token signing from external secret (you can use CertManager to do just that) - it will break the installation for those who already use core.secretName by not using it and inserting nil values into the -core secret (since -core secret exists and does not have tls.cert and tls.key)

The idea is good though - but it needs to preserve the core.secretName feature and it needs to check for presence of each individual value inside the secret.

Hi @Kajot-dev it should be solved. Could you review it again please?

[Kajot-dev]

First, you did a lot of work, thanks!!

Nice changes, but some of them apply only when generating new data.
Also there is much code duplication like this:

{{- $secret := lookup ... }}
{(- if $secret }}
old data..
{{- else }}
new data
{{- end }}

Also not all secret values are reused - for example if user won't put S3 secrets in external secret it will not use lookup for them

If you want to you can avoid code duplications by leveraging the fact that when secret is not present helm returns an empty object. Instead of having two blocks for reusing data and generation of the new data which share most of the code you can do something like this:

{{/* template definition */}}
{{- define "harbor.keyExtractor" -}}
{{- if hasKey .data .key }}
{{- $val := (index .data .key) }}
{{- if kindIs "string" $val }}
{{ .key }}: {{ $val | quote }}
{{- else }}
{{ .key }}: {{ $val }}
{{- end }}
{{- else }}
{{ .default }}
{{- end }}
{{- end -}}

{{/* and then */}}
{{- $secret := lookup ... }}
{{- template "harbor.keyExtractor" (dict "key" "SOME_PASSWORD_KEY" "data" $secret.data "default" "DEFAULT_VALUE" ) }}

[Kajot-dev]

looks good to me
Nice work!

[Kajot-dev]

Could we get this reviewed and merged please?

[Kajot-dev]

fix: #1609

[zyyw]

Thanks for contributing to harbor-helm!
Closing this PR now as it was fixed in PR:

• Fix random values regenerating #1612