harbor-helm icon indicating copy to clipboard operation
harbor-helm copied to clipboard

Add ability to use external secrets

Open golgoth31 opened this issue 2 years ago • 4 comments

Hi, Thank you for this amazing job. please find here an update of the charts files to allow usage of existing secrets instead of storing secret values inside values file.

regards

David Sabatie

golgoth31 avatar Apr 20 '22 10:04 golgoth31

@golgoth31 could you please help to resolve these comments in this PR.

zyyw avatar Jun 13 '22 01:06 zyyw

@golgoth31 can we get this going ?

ekarlso avatar Aug 13 '22 09:08 ekarlso

@ekarlso we need to wait for @golgoth31 to address the open points. @ekarlso if you want you can take this over and complete the open issues

also the readme needs an update

Vad1mo avatar Aug 14 '22 13:08 Vad1mo

Hi all, I'm terribly sorry for delay. I update the PR now.

golgoth31 avatar Aug 14 '22 19:08 golgoth31

One additional comment: could you please also update the README.md for the config you introduced in values.yaml file.

Last but not the least: thank you for contributing to harbor-helm!

zyyw avatar Aug 18 '22 13:08 zyyw

@golgoth31 please help to resolve the new comments in this PR, thanks.

zyyw avatar Aug 18 '22 13:08 zyyw

One additional comment: could you please also update the README.md for the config you introduced in values.yaml file.

Last but not the least: thank you for contributing to harbor-helm!

Hi, thank you for your review. I have updated the REAME file

golgoth31 avatar Aug 18 '22 20:08 golgoth31

Hi, I don't see you change templates/registry/registry-secret.yaml file or templates/registry/registry-dpl.yaml file to use the secret in .Values.registry.credentials.existingSecret

https://github.com/goharbor/harbor-helm/blob/master/templates/registry/registry-secret.yaml#L47

REGISTRY_HTPASSWD: {{ htpasswd .Values.registry.credentials.username .Values.registry.credentials.password | b64enc | quote }}

It means that registry container still using .Values.registry.credentials.password as it password, instead using password from .Values.registry.credentials.existingSecret after you set value for it. It is not the behavior we want.

Maybe we need to add REGISTRY_HTPASSWD key to .Values.registry.credentials.existingSecret, and use this key in registry-dpl.yaml file to make this configuration works.

haminhcong avatar Aug 19 '22 06:08 haminhcong

@haminhcong I fixed the registry credentials issue

golgoth31 avatar Aug 19 '22 20:08 golgoth31

@golgoth31 Sorry, but I see that in file templates/core/core-dpl.yaml and file templates/exporter/exporter-dpl.yaml you are using variable password with lowercase

        {{- if .Values.database.external.existingSecret }}
          - name: POSTGRESQL_PASSWORD
            valueFrom:
              secretKeyRef:
                name: {{ .Values.database.external.existingSecret }}
                key: password

but in values.yaml file and README.md file you are writing examples with uppercase PASSWORD

  external:
    host: "192.168.0.1"
    port: "5432"
    username: "user"
    password: "password"
    coreDatabase: "registry"
    notaryServerDatabase: "notary_server"
    notarySignerDatabase: "notary_signer"
    # if using existing secret, the key must be PASSWORD
    existingSecret: ""
 | `database.external.existingSecret`                             | An existing password containing the database password. the key must be `PASSWORD`.                                                                                                                                                                                                                                                                                                                                                                                                                                 | `""`                       |

It make me quite confused when using these config options. Could you create a pull request to edit the document using lowercase variable. to remove the differences between document and code?

haminhcong avatar Aug 24 '22 14:08 haminhcong

@haminhcong here is the PR #1269

golgoth31 avatar Aug 24 '22 22:08 golgoth31

It looks like this does not work for the notary-server and notary signer. They appear to only get .Values.database.external.password as it shows in _helpers.tpl and never from .Values.database.external.existingSecret.

ke5C2Fin avatar Dec 19 '22 15:12 ke5C2Fin

Can confirm that Notary server and signer doesn't use the existingSecret value.

The problem is on _helpers.tpl:

{{- define "harbor.database.rawPassword" -}} {{- if eq .Values.database.type "internal" -}} {{- .Values.database.internal.password -}} {{- else -}} {{- .Values.database.external.password -}} {{- end -}} {{- end -}}

the 'else' part just try to use the external.password. Probably need to fix the secret that invoque this helper and other templates. Like notary/notary-secret.yaml

NOTARY_SERVER_DB_URL: {{ include "harbor.database.notaryServer" . | b64enc }} NOTARY_SIGNER_DB_URL: {{ include "harbor.database.notarySigner" . | b64enc }}

This is where the trail begin and ends up on the rawPassword in the helper.

koshrf avatar Jan 18 '23 19:01 koshrf