Add ability to use external secrets

[golgoth31]

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

[zyyw]

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

[ekarlso]

@golgoth31 can we get this going ?

[Vad1mo]

@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

[golgoth31]

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

[zyyw]

Could you please justify why we need the configuration of:

• .Values.persistence.imageChartStorage.gcs.useWorkloadIdentity

Same questions for subsequent useWorkloadIdentity config. If there is not a strong justification of useWorkloadIdentity. I would suggest deleting it.

[golgoth31]

Hi,
as stated here for exemple: https://gcloud.readthedocs.io/en/latest/google-cloud-auth.html#credential-discovery-precedence , if the "GOOGLE_APPLICATION_CREDENTIALS" variable will take precedence and the application will not be able to use the current kubernetes service account to authenticate.
When using workloadidentity, we need to remove any other gcloud sdk authentication mechanism.

[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!

[zyyw]

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

[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!

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

[haminhcong]

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.

[golgoth31]

@haminhcong I fixed the registry credentials issue

[haminhcong]

@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?

[golgoth31]

@haminhcong here is the PR #1269

[ke5C2Fin]

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.

[koshrf]

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.