Chart always expects accessKey and secretKey to be defined when using s3 storage

[robmyersrobmyers]

The current chart always secrets.s3.accessKey and secrets.s3.secretKey to be defined when using s3 storage, which can break if you rely on ec2 instance profiles.

The diff below checks to make sure .Values.secrets.s3 is defined before using it, which appears to resolve my issue.

diff -uNrp twuni-docker-registry.helm-cb69658/templates/deployment.yaml twuni-docker-registry.helm-cb69658-working/templates/deployment.yaml
--- twuni-docker-registry.helm-cb69658/templates/deployment.yaml        2022-08-09 17:13:42.000000000 +0000
+++ twuni-docker-registry.helm-cb69658-working/templates/deployment.yaml        2022-08-15 19:01:12.357116958 +0000
@@ -124,6 +124,7 @@ spec:
                   name: {{ template "docker-registry.fullname" . }}-secret
                   key: azureContainer
 {{- else if eq .Values.storage "s3" }}
+            {{- if .Values.secrets.s3 }}
             {{- if or (and .Values.secrets.s3.secretKey .Values.secrets.s3.accessKey) .Values.secrets.s3.secretRef }}
             - name: REGISTRY_STORAGE_S3_ACCESSKEY
               valueFrom:
@@ -136,6 +137,7 @@ spec:
                   name: {{ if .Values.secrets.s3.secretRef }}{{ .Values.secrets.s3.secretRef }}{{ else }}{{ template "docker-registry.fullname" . }}-secret{{ end }}
                   key: s3SecretKey
             {{- end }}
+            {{- end }}
             - name: REGISTRY_STORAGE_S3_REGION
               value: {{ required ".Values.s3.region is required" .Values.s3.region }}
           {{- if .Values.s3.regionEndpoint }}
diff -uNrp twuni-docker-registry.helm-cb69658/templates/secret.yaml twuni-docker-registry.helm-cb69658-working/templates/secret.yaml
--- twuni-docker-registry.helm-cb69658/templates/secret.yaml    2022-08-09 17:13:42.000000000 +0000
+++ twuni-docker-registry.helm-cb69658-working/templates/secret.yaml    2022-08-15 18:58:39.077118130 +0000
@@ -25,7 +25,7 @@ data:
   azureAccountKey: {{ .Values.secrets.azure.accountKey | b64enc | quote }}
   azureContainer: {{ .Values.secrets.azure.container | b64enc | quote }}
     {{- end }}
-  {{- else if eq .Values.storage "s3" }}
+  {{- else if and (eq .Values.storage "s3") .Values.secrets.s3 }}
     {{- if and .Values.secrets.s3.secretKey .Values.secrets.s3.accessKey }}
   s3AccessKey: {{ .Values.secrets.s3.accessKey | b64enc | quote }}
   s3SecretKey: {{ .Values.secrets.s3.secretKey | b64enc | quote }}

Please let me know if I'm doing it wrong or missed some documentation. Thanks!

[canterberry]

Thanks, @robmyersrobmyers! This diff looks fine to me, but conflicts, in part, with #62, since it touches some of the same code.

cc @ddelange @kuzaxak This might have been why there was a default value being set for secrets.s3 in an iteration of #68. The diff above seems like a less fragile solution, though it does add some more complexity. Is there a maybe syntax/function/pattern that could be used for deep object traversal? Something akin to JavaScript's foo?.bar syntax, maybe?

[ddelange]

For the 'functional' approach ref #62 (comment)