core-dpl unintentionally loads non-environment-variable secrets (from core-secret) as environment variables

[spantaleev]

The core-secret.yaml resource (typically named harbor-core) contains various secrets:

• environment variables (like HARBOR_ADMIN_PASSWORD, CONFIG_OVERWRITE_JSON, etc)
• other things (like secret, secretKey, tls.key, tls.crt, etc)

The core-dpl.yaml resource blindly loads all fields of the harbor-core ConfigMap and Secret as environment variables, like this:

harbor-helm/templates/core/core-dpl.yaml

Lines 93 to 97 in ebbf6bb

	envFrom:
	- configMapRef:
	name: "{{ template "harbor.core" . }}"
	- secretRef:
	name: "{{ template "harbor.core" . }}"

Individual environment variables (among which HARBOR_ADMIN_PASSWORD) are also loaded like this:

harbor-helm/templates/core/core-dpl.yaml

Lines 98 to 119 in ebbf6bb

	env:
	- name: CORE_SECRET
	valueFrom:
	secretKeyRef:
	name: {{ default (include "harbor.core" .) .Values.core.existingSecret }}
	key: secret
	- name: JOBSERVICE_SECRET
	valueFrom:
	secretKeyRef:
	name: {{ default (include "harbor.jobservice" .) .Values.jobservice.existingSecret }}
	{{- if .Values.jobservice.existingSecret }}
	key: {{ .Values.jobservice.existingSecretKey }}
	{{- else }}
	key: JOBSERVICE_SECRET
	{{- end }}
	{{- if .Values.existingSecretAdminPassword }}
	- name: HARBOR_ADMIN_PASSWORD
	valueFrom:
	secretKeyRef:
	name: {{ .Values.existingSecretAdminPassword }}
	key: {{ .Values.existingSecretAdminPasswordKey }}
	{{- end }}

Because core-secret.yaml contains more than just environment variables (things like secret, secretKey, tls.key, tls.crt), but envFrom is pointing to this Secret, these extra values are also injected as environment variables.

This could be verified by doing something like:

$ kubectl exec -it harbor-core-xxxx-yyyy --namespace registry -- printenv | grep tls

tls.key=-----BEGIN RSA PRIVATE KEY-----
tls.crt=-----BEGIN CERTIFICATE-----

---

A potential solution might be to:

• explicitly load the additional environment variables (CONFIG_OVERWRITE_JSON, POSTGRESQL_PASSWORD, REGISTRY_CREDENTIAL_PASSWORD, CRSF_KEY) that core-dpl might need by adjusting the code here. It's probably a good time to check which ones are actually used by core-dpl and skipping the unnecessary ones (if any). The current core-secret.yaml template also renders {{- template "harbor.traceJaegerPassword" . }} (which potentially defines TRACE_JAEGER_PASSWORD), so this one shouldn't be forgotten as well.

• stop mass-loading environment variables from a secret via envFrom

---

An even better solution may be to stop mixing so many things in that single harbor-core secret.

In my deployment, I'm making extensive use of various existingSecret* variables and splitting my secrets based on what they pertain to:

• a core-main Secret (pointed to by core.existingSecret), containing a secret field
• a core-tls Secret (pointed to by core.secretName - which is confusingly named, btw), containing tls.key and tls.crt fields
• a core-xsrf Secret (pointed to by core.existingXsrfSecretKey), containing a CSRF_KEY field
• a main Secret (pointed to by existingSecretSecretKey), containing a secretKey field
• a main-admin-password Secret (pointed to by existingSecretAdminPassword), containing a HARBOR_ADMIN_PASSWORD field

.. which makes it much easier to reason about. Incidentally, it also helps me avoid this issue.

[MinerYang]

Hi @spantaleev ,

Thanks for exploring harbor-helm and reaching out to us ! I could get the point that you intently to make our template more clean and neat.However I would give a a few history and explanation here why we templates looks like so... given the env core.secret and core.secretKey the as an example.
i. both the core.secret and core.secretKey would be stored in a secret, either be the harbor-core-secret or existing secret. However core.secretshould be mounted as an ENV, andcore.secretKeyshould be mounted as a file inside the container, So both of these are needs to be specified in theharbor-core`
eg. mount as an ENV

env:
          - name: CORE_SECRET
            valueFrom:
              secretKeyRef:
                name: {{ default (include "harbor.core" .) .Values.core.existingSecret }}
                key: secret

eg. mount as a file

 volumeMounts:
        - name: secret-key
          mountPath: /etc/core/key
          subPath: key

ii. We support both clear text secret things in values.yaml at the every beginning at the harbor-helm , and then add support for existingSecret for each of them. So for the better backwards compatibility we support both in our current template