chore(chart): be more careful about undefined values -- 0.4.x edition (…

…#1603)

Signed-off-by: Kent Rancourt <[email protected]>

api/v1alpha1/stage_types.go

@@ -1,8 +1,6 @@
 package v1alpha1
 
 import (
-	"os"
-
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -392,16 +390,6 @@ type ArgoCDAppUpdate struct {
 	SourceUpdates []ArgoCDSourceUpdate `json:"sourceUpdates,omitempty"`
 }
 
-func (a *ArgoCDAppUpdate) AppNamespaceOrDefault() string {
-	if a.AppNamespace != "" {
-		return a.AppNamespace
-	}
-	if envArgocdNs := os.Getenv("ARGOCD_NAMESPACE"); envArgocdNs != "" {
-		return envArgocdNs
-	}
-	return "argocd"
-}
-
 // ArgoCDSourceUpdate describes updates that should be applied to one of an Argo
 // CD Application resource's sources.
 type ArgoCDSourceUpdate struct {

charts/kargo/templates/api/cert.yaml

@@ -9,7 +9,7 @@ metadata:
     {{- include "kargo.api.labels" . | nindent 4 }}
 spec:
   dnsNames:
-  - {{ .Values.api.host }}
+  - {{ quote .Values.api.host }}
   issuerRef:
     kind: Issuer
     name: kargo-selfsigned-cert-issuer

charts/kargo/templates/api/configmap.yaml

@@ -9,7 +9,7 @@ metadata:
     {{- include "kargo.api.labels" . | nindent 4 }}
 data:
   KARGO_NAMESPACE: {{ .Release.Namespace }}
-  LOG_LEVEL: {{ .Values.api.logLevel }}
+  LOG_LEVEL: {{ quote .Values.api.logLevel }}
   {{- if .Values.kubeconfigSecrets.kargo }}
   KUBECONFIG: /etc/kargo/kubeconfig.yaml
   {{- end }}
@@ -26,8 +26,8 @@ data:
   {{- else }}
   ADMIN_ACCOUNT_TOKEN_ISSUER: http://{{ .Values.api.host }}
   {{- end }}
-  ADMIN_ACCOUNT_TOKEN_AUDIENCE: {{ .Values.api.host }}
-  ADMIN_ACCOUNT_TOKEN_TTL: {{ .Values.api.adminAccount.tokenTTL }}
+  ADMIN_ACCOUNT_TOKEN_AUDIENCE: {{ quote .Values.api.host }}
+  ADMIN_ACCOUNT_TOKEN_TTL: {{ quote .Values.api.adminAccount.tokenTTL }}
   {{- end }}
   {{- if .Values.api.oidc.enabled }}
   OIDC_ENABLED: "true"
@@ -38,21 +38,21 @@ data:
   {{- else }}
   OIDC_ISSUER_URL: http://{{ .Values.api.host }}/dex
   {{- end }}
-  OIDC_CLIENT_ID: {{ .Values.api.host }}
+  OIDC_CLIENT_ID: {{ quote .Values.api.host }}
   OIDC_CLI_CLIENT_ID: {{ .Values.api.host }}-cli
   DEX_ENABLED: "true"
   DEX_SERVER_ADDRESS: https://kargo-dex-server.{{ .Release.Namespace }}.svc
   DEX_CA_CERT_PATH: /etc/kargo/idp-ca.crt
   {{- else }}
-  OIDC_ISSUER_URL: {{ .Values.api.oidc.issuerURL }}
-  OIDC_CLIENT_ID: {{ .Values.api.oidc.clientID }}
+  OIDC_ISSUER_URL: {{ quote .Values.api.oidc.issuerURL }}
+  OIDC_CLIENT_ID: {{ quote .Values.api.oidc.clientID }}
   {{- if .Values.api.oidc.cliClientID }}
-  OIDC_CLI_CLIENT_ID: {{ .Values.api.oidc.cliClientID }}
+  OIDC_CLI_CLIENT_ID: {{ quote .Values.api.oidc.cliClientID }}
   {{- end }}
   {{- end }}
   {{- end }}
   {{- if .Values.api.argocd.urls }}
-  ARGOCD_NAMESPACE: {{ .Values.controller.argocd.namespace }}
+  ARGOCD_NAMESPACE: {{ .Values.controller.argocd.namespace | default "argocd" }}
   ARGOCD_URLS: {{ range $key, $val := .Values.api.argocd.urls }}{{ $key }}={{ $val }},{{- end }}
   {{- end }}
   ROLLOUTS_INTEGRATION_ENABLED: {{ quote .Values.api.rollouts.integrationEnabled }}

charts/kargo/templates/api/ingress-cert.yaml

@@ -9,7 +9,7 @@ metadata:
     {{- include "kargo.api.labels" . | nindent 4 }}
 spec:
   dnsNames:
-  - {{ .Values.api.host }}
+  - {{ quote .Values.api.host }}
   issuerRef:
     kind: Issuer
     name: kargo-selfsigned-cert-issuer

charts/kargo/templates/api/ingress.yaml

@@ -16,10 +16,10 @@ spec:
   ingressClassName: {{ .Values.api.ingress.ingressClassName }}
   {{- end }}
   rules:
-  - host: {{ .Values.api.host }}
+  - host: {{ quote .Values.api.host }}
     http:
       paths:
-      - pathType: {{ .Values.api.ingress.pathType }}
+      - pathType: {{ .Values.api.ingress.pathType | default "ImplementationSpecific" }}
         path: /
         backend:
           service:
@@ -33,7 +33,7 @@ spec:
   {{- if .Values.api.ingress.tls.enabled }}
   tls:
   - hosts:
-    - {{ .Values.api.host }}
+    - {{ quote .Values.api.host }}
     secretName: kargo-api-ingress-cert
   {{- end }}
 {{- end }}

charts/kargo/templates/argocd/role-binding.yaml

@@ -3,7 +3,7 @@ apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding
 metadata:
   name: kargo-controller
-  namespace: {{ .Values.controller.argocd.namespace }}
+  namespace: {{ .Values.controller.argocd.namespace | default "argocd" }}
   labels:
     {{- include "kargo.labels" . | nindent 4 }}
     {{- include "kargo.controller.labels" . | nindent 4 }}

charts/kargo/templates/argocd/role.yaml

@@ -3,7 +3,7 @@ apiVersion: rbac.authorization.k8s.io/v1
 kind: Role
 metadata:
   name: kargo-controller
-  namespace: {{ .Values.controller.argocd.namespace }}
+  namespace: {{ .Values.controller.argocd.namespace | default "argocd" }}
   labels:
     {{- include "kargo.labels" . | nindent 4 }}
     {{- include "kargo.controller.labels" . | nindent 4 }}

charts/kargo/templates/controller/configmap.yaml

@@ -8,28 +8,28 @@ metadata:
     {{- include "kargo.labels" . | nindent 4 }}
     {{- include "kargo.controller.labels" . | nindent 4 }}
 data:
-  LOG_LEVEL: {{ .Values.controller.logLevel }}
+  LOG_LEVEL: {{ quote .Values.controller.logLevel }}
   {{- if .Values.controller.shardName }}
   SHARD_NAME: {{ .Values.controller.shardName }}
   {{- end }}
   {{- if .Values.kubeconfigSecrets.kargo }}
   KUBECONFIG: /etc/kargo/kubeconfigs/kubeconfig.yaml
   {{- end }}
-  GLOBAL_CREDENTIALS_NAMESPACES: {{ join "," .Values.controller.globalCredentials.namespaces }}
+  GLOBAL_CREDENTIALS_NAMESPACES: {{ quote (join "," .Values.controller.globalCredentials.namespaces) }}
   ARGOCD_INTEGRATION_ENABLED: {{ quote .Values.controller.argocd.integrationEnabled }}
   {{- if .Values.controller.argocd.integrationEnabled }}
   {{- if .Values.kubeconfigSecrets.argocd }}
   ARGOCD_KUBECONFIG: /etc/kargo/kubeconfigs/argocd-kubeconfig.yaml
   {{- end }}
-  ARGOCD_NAMESPACE: {{ .Values.controller.argocd.namespace }}
+  ARGOCD_NAMESPACE: {{ .Values.controller.argocd.namespace | default "argocd" }}
   ARGOCD_WATCH_ARGOCD_NAMESPACE_ONLY: {{ quote .Values.controller.argocd.watchArgocdNamespaceOnly }}
   {{- end }}
   ROLLOUTS_INTEGRATION_ENABLED: {{ quote .Values.controller.rollouts.integrationEnabled }}
   {{- if .Values.controller.rollouts.integrationEnabled }}
   {{- if .Values.kubeconfigSecrets.rollouts }}
   ROLLOUTS_KUBECONFIG: /etc/kargo/kubeconfigs/rollouts-kubeconfig.yaml
   {{- end }}
-  ROLLOUTS_ANALYSIS_RUNS_NAMESPACE: {{ .Values.controller.rollouts.analysisRunsNamespace }}
-  ROLLOUTS_CONTROLLER_INSTANCE_ID: {{ .Values.controller.rollouts.controllerInstanceID }}
+  ROLLOUTS_ANALYSIS_RUNS_NAMESPACE: {{ quote .Values.controller.rollouts.analysisRunsNamespace }}
+  ROLLOUTS_CONTROLLER_INSTANCE_ID: {{ quote .Values.controller.rollouts.controllerInstanceID }}
   {{- end }}
 {{- end }}

charts/kargo/templates/dex-server/secret.yaml

@@ -27,10 +27,10 @@ stringData:
       http: 0.0.0.0:5558
 
     oauth2:
-      skipApprovalScreen: {{ .Values.api.oidc.dex.skipApprovalScreen }}
+      skipApprovalScreen: {{ .Values.api.oidc.dex.skipApprovalScreen | default "true" }}
 
     staticClients:
-    - id: {{ .Values.api.host }}
+    - id: {{ quote .Values.api.host }}
       name: Kargo
       public: true
       redirectURIs:

charts/kargo/templates/garbage-collector/configmap.yaml

@@ -8,7 +8,7 @@ metadata:
     {{- include "kargo.labels" . | nindent 4 }}
     {{- include "kargo.garbageCollector.labels" . | nindent 4 }}
 data:
-  LOG_LEVEL: {{ .Values.garbageCollector.logLevel }}
+  LOG_LEVEL: {{ quote .Values.garbageCollector.logLevel }}
   NUM_WORKERS: {{ quote .Values.garbageCollector.workers }}
   MAX_RETAINED_PROMOTIONS: {{ quote .Values.garbageCollector.maxRetainedPromotions }}
 {{- end }}

charts/kargo/templates/management-controller/configmap.yaml

@@ -9,7 +9,7 @@ metadata:
     {{- include "kargo.managementController.labels" . | nindent 4 }}
 data:
   KARGO_NAMESPACE: {{ .Release.Namespace }}
-  LOG_LEVEL: {{ .Values.managementController.logLevel }}
+  LOG_LEVEL: {{ quote .Values.managementController.logLevel }}
   {{- if .Values.kubeconfigSecrets.kargo }}
   KUBECONFIG: /etc/kargo/kubeconfigs/kubeconfig.yaml
   {{- end }}

charts/kargo/templates/webhooks-server/configmap.yaml

@@ -9,7 +9,7 @@ metadata:
     {{- include "kargo.webhooksServer.labels" . | nindent 4 }}
 data:
   KARGO_NAMESPACE: {{ .Release.Namespace }}
-  LOG_LEVEL: {{ .Values.webhooksServer.logLevel }}
+  LOG_LEVEL: {{ quote .Values.webhooksServer.logLevel }}
   {{- if .Values.kubeconfigSecrets.kargo }}
   KUBECONFIG: /etc/kargo/kubeconfigs/kubeconfig.yaml
   {{- end }}

cmd/controlplane/controller.go

@@ -15,6 +15,7 @@ import (
 
 	kargoapi "github.com/akuity/kargo/api/v1alpha1"
 	"github.com/akuity/kargo/internal/api/kubernetes"
+	libargocd "github.com/akuity/kargo/internal/argocd"
 	argocd "github.com/akuity/kargo/internal/controller/argocd/api/v1alpha1"
 	"github.com/akuity/kargo/internal/controller/promotions"
 	rollouts "github.com/akuity/kargo/internal/controller/rollouts/api/v1alpha1"
@@ -122,7 +123,7 @@ func newControllerCommand() *cobra.Command {
 				}
 				restCfg.ContentType = runtime.ContentTypeJSON
 
-				argocdNamespace := os.GetEnv("ARGOCD_NAMESPACE", "argocd")
+				argocdNamespace := libargocd.Namespace()
 
 				// There's a chance there is only permission to interact with Argo CD
 				// Application resources in a single namespace, so we will use that

internal/api/config/config.go

@@ -79,7 +79,7 @@ type AdminConfig struct {
 	TokenSigningKey []byte `envconfig:"ADMIN_ACCOUNT_TOKEN_SIGNING_KEY" required:"true"`
 	// TokenTTL specifies how long ID tokens for the admin account are valid. i.e.
 	// The expiry will be the time of issue plus this duration.
-	TokenTTL time.Duration `envconfig:"ADMIN_ACCOUNT_TOKEN_TTL" default:"1h"`
+	TokenTTL time.Duration `envconfig:"ADMIN_ACCOUNT_TOKEN_TTL" default:"24h"`
 }
 
 // AdminConfigFromEnv returns an AdminConfig populated from environment
@@ -114,7 +114,6 @@ func (a *ArgoCDURLMap) Decode(value string) error {
 }
 
 type ArgoCDConfig struct {
-	Namespace string `envconfig:"ARGOCD_NAMESPACE" default:"argocd"`
 	// URLs is a mapping from shard name to Argo CD URL
 	URLs ArgoCDURLMap `envconfig:"ARGOCD_URLS"`
 }

internal/api/get_config_v1alpha1.go

@@ -5,6 +5,7 @@ import (
 
 	"connectrpc.com/connect"
 
+	libargocd "github.com/akuity/kargo/internal/argocd"
 	svcv1alpha1 "github.com/akuity/kargo/pkg/api/service/v1alpha1"
 )
 
@@ -19,7 +20,7 @@ func (s *server) GetConfig(
 		resp.ArgocdShards[shardName] = &svcv1alpha1.ArgoCDShard{
 			Url: url,
 			// TODO: currently, all shards must use the same namespace
-			Namespace: s.cfg.ArgoCDConfig.Namespace,
+			Namespace: libargocd.Namespace(),
 		}
 	}
 	return connect.NewResponse(&resp), nil

internal/api/get_config_v1alpha1_test.go

@@ -22,7 +22,6 @@ func TestGetConfig(t *testing.T) {
 			req: &svcv1alpha1.GetConfigRequest{},
 			cfg: config.ServerConfig{
 				ArgoCDConfig: config.ArgoCDConfig{
-					Namespace: "argocd",
 					URLs: map[string]string{
 						"": "https://argocd.example.com",
 					},

internal/argocd/namespace.go

@@ -0,0 +1,9 @@
+package argocd
+
+import "github.com/akuity/kargo/internal/os"
+
+var namespace = os.GetEnv("ARGOCD_NAMESPACE", "argocd")
+
+func Namespace() string {
+	return namespace
+}

internal/controller/promotion/argocd.go

@@ -12,6 +12,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	kargoapi "github.com/akuity/kargo/api/v1alpha1"
+	libargocd "github.com/akuity/kargo/internal/argocd"
 	argocd "github.com/akuity/kargo/internal/controller/argocd/api/v1alpha1"
 	"github.com/akuity/kargo/internal/git"
 	"github.com/akuity/kargo/internal/logging"
@@ -114,21 +115,24 @@ func (a *argoCDMechanism) doSingleUpdate(
 	update kargoapi.ArgoCDAppUpdate,
 	newFreight kargoapi.FreightReference,
 ) error {
-	app, err :=
-		a.getArgoCDAppFn(ctx, update.AppNamespaceOrDefault(), update.AppName)
+	namespace := update.AppNamespace
+	if namespace == "" {
+		namespace = libargocd.Namespace()
+	}
+	app, err := a.getArgoCDAppFn(ctx, namespace, update.AppName)
 	if err != nil {
 		return errors.Wrapf(
 			err,
 			"error finding Argo CD Application %q in namespace %q",
 			update.AppName,
-			update.AppNamespaceOrDefault(),
+			namespace,
 		)
 	}
 	if app == nil {
 		return errors.Errorf(
 			"unable to find Argo CD Application %q in namespace %q",
 			update.AppName,
-			update.AppNamespaceOrDefault(),
+			namespace,
 		)
 	}
 	// Make sure this is allowed!
@@ -148,7 +152,7 @@ func (a *argoCDMechanism) doSingleUpdate(
 					err,
 					"error updating source of Argo CD Application %q in namespace %q",
 					update.AppName,
-					update.AppNamespaceOrDefault(),
+					namespace,
 				)
 			}
 			app.Spec.Source = &source
@@ -163,7 +167,7 @@ func (a *argoCDMechanism) doSingleUpdate(
 					err,
 					"error updating source(s) of Argo CD Application %q in namespace %q",
 					update.AppName,
-					update.AppNamespaceOrDefault(),
+					namespace,
 				)
 			}
 			app.Spec.Sources[i] = source

internal/controller/stages/health.go

@@ -6,6 +6,7 @@ import (
 	"path"
 
 	kargoapi "github.com/akuity/kargo/api/v1alpha1"
+	libargocd "github.com/akuity/kargo/internal/argocd"
 	argocd "github.com/akuity/kargo/internal/controller/argocd/api/v1alpha1"
 	"github.com/akuity/kargo/internal/git"
 )
@@ -35,18 +36,18 @@ func (r *reconciler) checkHealth(
 		return &h
 	}
 
-	for i, updates := range argoCDAppUpdates {
+	for i, update := range argoCDAppUpdates {
+		namespace := update.AppNamespace
+		if namespace == "" {
+			namespace = libargocd.Namespace()
+		}
+
 		h.ArgoCDApps[i] = kargoapi.ArgoCDAppStatus{
-			Namespace: updates.AppNamespaceOrDefault(),
-			Name:      updates.AppName,
+			Namespace: namespace,
+			Name:      update.AppName,
 		}
 
-		app, err := r.getArgoCDAppFn(
-			ctx,
-			r.argocdClient,
-			updates.AppNamespaceOrDefault(),
-			updates.AppName,
-		)
+		app, err := r.getArgoCDAppFn(ctx, r.argocdClient, namespace, update.AppName)
 
 		if err != nil {
 			h.ArgoCDApps[i].HealthStatus = kargoapi.ArgoCDAppHealthStatus{
@@ -60,8 +61,8 @@ func (r *reconciler) checkHealth(
 				h.Issues,
 				fmt.Sprintf(
 					"error finding Argo CD Application %q in namespace %q: %s",
-					updates.AppName,
-					updates.AppNamespaceOrDefault(),
+					update.AppName,
+					namespace,
 					err,
 				),
 			)
@@ -80,8 +81,8 @@ func (r *reconciler) checkHealth(
 				h.Issues,
 				fmt.Sprintf(
 					"unable to find Argo CD Application %q in namespace %q",
-					updates.AppName,
-					updates.AppNamespaceOrDefault(),
+					update.AppName,
+					namespace,
 				),
 			)
 			continue
@@ -106,8 +107,8 @@ func (r *reconciler) checkHealth(
 				fmt.Sprintf(
 					"bugs in Argo CD currently prevent a comprehensive assessment of "+
 						"the health of multi-source Application %q in namespace %q",
-					updates.AppName,
-					updates.AppNamespaceOrDefault(),
+					update.AppName,
+					namespace,
 				),
 			)
 			continue