From 7f33231d34c5aa738cd5b711b8b7ed8b8c5dc960 Mon Sep 17 00:00:00 2001
From: chgl <chgl@users.noreply.github.com>
Date: Wed, 28 Apr 2021 21:23:32 +0200
Subject: [PATCH] Apply suggestions from code review
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Brandon Mayfield <563214+bmayfi3ld@users.noreply.github.com>
Co-authored-by: Oliver Bähler <oliverbaehler@hotmail.com>
Signed-off-by: chgl <chgl@users.noreply.github.com>
---
 charts/argo-workflows/templates/_helpers.tpl  | 18 ++++++++++++++++
 .../workflow-controller-deployment.yaml       |  4 ++--
 .../controller/workflow-controller-sa.yaml    |  4 +++-
 .../workflow-controller-service.yaml          |  7 ++++---
 .../workflow-controller-servicemonitor.yaml   | 12 +++++------
 .../templates/controller/workflow-rb.yaml     | 10 ++++-----
 .../templates/controller/workflow-role.yaml   |  4 ++--
 .../templates/controller/workflow-sa.yaml     |  4 ++--
 .../server/server-cluster-roles.yaml          | 14 ++++++-------
 .../server/server-deployment-pdb.yaml         |  4 +---
 .../templates/server/server-deployment.yaml   | 21 ++++++++++---------
 .../templates/server/server-ingress.yaml      | 18 +++++++---------
 .../templates/server/server-sa.yaml           |  4 +++-
 .../templates/server/server-service.yaml      | 12 ++++++-----
 charts/argo/Chart.yaml                        |  2 +-
 15 files changed, 79 insertions(+), 59 deletions(-)

diff --git a/charts/argo-workflows/templates/_helpers.tpl b/charts/argo-workflows/templates/_helpers.tpl
index 1f978c0b..a62f314b 100644
--- a/charts/argo-workflows/templates/_helpers.tpl
+++ b/charts/argo-workflows/templates/_helpers.tpl
@@ -94,3 +94,21 @@ Return the appropriate apiVersion for ingress
 {{- print "networking.k8s.io/v1" -}}
 {{- end -}}
 {{- end -}}
+
+{{/*
+Return the appropriate service layout for ingress
+*/}}
+{{- define "argo-workflows.ingress.service" -}}
+{{- if semverCompare "<1.19-0" .kubeVersion -}}
+- backend:
+    serviceName: {{ .serviceName }}
+    servicePort: {{ .servicePort }}
+{{- else -}}
+- backend:
+    service:
+      name: {{ .serviceName }}
+      port: 
+        number: {{ .servicePort }}
+  pathType: ImplementationSpecific
+{{- end -}}
+{{- end -}}
diff --git a/charts/argo-workflows/templates/controller/workflow-controller-deployment.yaml b/charts/argo-workflows/templates/controller/workflow-controller-deployment.yaml
index 98f1b060..c35894ec 100644
--- a/charts/argo-workflows/templates/controller/workflow-controller-deployment.yaml
+++ b/charts/argo-workflows/templates/controller/workflow-controller-deployment.yaml
@@ -24,9 +24,9 @@ spec:
       {{- end }}
     spec:
       serviceAccountName: {{ template "argo-workflows.controllerServiceAccountName" . }}
-      {{- if .Values.controller.podSecurityContext }}
+      {{- with .Values.controller.podSecurityContext }}
       securityContext:
-        {{- toYaml .Values.controller.podSecurityContext | nindent 8 }}
+        {{- toYaml . | nindent 8 }}
       {{- end }}
       containers:
         - name: controller
diff --git a/charts/argo-workflows/templates/controller/workflow-controller-sa.yaml b/charts/argo-workflows/templates/controller/workflow-controller-sa.yaml
index 09715489..77000ef9 100644
--- a/charts/argo-workflows/templates/controller/workflow-controller-sa.yaml
+++ b/charts/argo-workflows/templates/controller/workflow-controller-sa.yaml
@@ -2,5 +2,7 @@ apiVersion: v1
 kind: ServiceAccount
 metadata:
   name: {{ template "argo-workflows.controllerServiceAccountName" . }}
+  {{ with .Values.controller.serviceAccountAnnotations }}
   annotations:
-{{ toYaml .Values.controller.serviceAccountAnnotations | indent 4 }}
+    {{- toYaml .| nindent 4 }}
+  {{- end }}
diff --git a/charts/argo-workflows/templates/controller/workflow-controller-service.yaml b/charts/argo-workflows/templates/controller/workflow-controller-service.yaml
index 8072f84b..abde6aad 100644
--- a/charts/argo-workflows/templates/controller/workflow-controller-service.yaml
+++ b/charts/argo-workflows/templates/controller/workflow-controller-service.yaml
@@ -6,8 +6,8 @@ metadata:
   labels:
     {{- include "argo-workflows.labels" (dict "context" . "component" .Values.controller.name "name" .Values.controller.name) | nindent 4 }}
     app.kubernetes.io/version: {{ default .Chart.AppVersion .Values.controller.image.tag | quote }}
-    {{- if .Values.controller.serviceLabels }}
-    {{ toYaml .Values.controller.serviceLabels | nindent 4}}
+    {{- with .Values.controller.serviceLabels }}
+      {{ toYaml . | nindent 4 }}
     {{- end }}
   {{- with .Values.controller.serviceAnnotations }}
   annotations:
@@ -33,5 +33,6 @@ spec:
   type: {{ .Values.controller.serviceType }}
   {{- if and (eq .Values.controller.serviceType "LoadBalancer") .Values.controller.loadBalancerSourceRanges }}
   loadBalancerSourceRanges:
-{{ toYaml .Values.controller.loadBalancerSourceRanges | indent 4 }}{{- end }}
+    {{- toYaml .Values.controller.loadBalancerSourceRanges | nindent 4 }}
+  {{- end }}
 {{- end -}}
diff --git a/charts/argo-workflows/templates/controller/workflow-controller-servicemonitor.yaml b/charts/argo-workflows/templates/controller/workflow-controller-servicemonitor.yaml
index 06888a8f..78c16ec4 100644
--- a/charts/argo-workflows/templates/controller/workflow-controller-servicemonitor.yaml
+++ b/charts/argo-workflows/templates/controller/workflow-controller-servicemonitor.yaml
@@ -5,19 +5,19 @@ metadata:
   name: {{ template "argo-workflows.controller.fullname" . }}
   labels:
     {{- include "argo-workflows.labels" (dict "context" . "component" .Values.controller.name "name" .Values.controller.name) | nindent 4 }}
-    {{- if .Values.controller.serviceMonitor.additionalLabels }}
-{{ toYaml .Values.controller.serviceMonitor.additionalLabels | indent 4 }}
+    {{- with .Values.controller.serviceMonitor.additionalLabels }}
+      {{- toYaml . | nindent 4 }}
     {{- end }}
 spec:
   endpoints:
-  {{- if .Values.controller.metricsConfig.enabled }}
+  {{- with .Values.controller.metricsConfig.enabled }}
     - port: metrics
-      path: {{ .Values.controller.metricsConfig.path }}
+      path: {{ . }}
       interval: 30s
   {{- end }}
-  {{- if .Values.controller.telemetryConfig.enabled }}
+  {{- with .Values.controller.telemetryConfig.enabled }}
     - port: telemetry
-      path: {{ .Values.controller.telemetryConfig.path }}
+      path: {{ . }}
       interval: 30s
   {{- end }}
   namespaceSelector:
diff --git a/charts/argo-workflows/templates/controller/workflow-rb.yaml b/charts/argo-workflows/templates/controller/workflow-rb.yaml
index c2d54b7c..9b27c045 100644
--- a/charts/argo-workflows/templates/controller/workflow-rb.yaml
+++ b/charts/argo-workflows/templates/controller/workflow-rb.yaml
@@ -3,9 +3,9 @@ apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding
 metadata:
   name: {{ template "argo-workflows.fullname" . }}-workflow
-{{- if .Values.workflow.namespace }}
-  namespace: {{ .Values.workflow.namespace }}
-{{- end }}
+  {{- with .Values.workflow.namespace }}
+  namespace: {{ . }} 
+  {{- end }}
 roleRef:
   apiGroup: rbac.authorization.k8s.io
   kind: Role
@@ -13,7 +13,7 @@ roleRef:
 subjects:
 - kind: ServiceAccount
   name: {{ .Values.workflow.serviceAccount.name }}
-  {{- if .Values.workflow.namespace }}
-  namespace: {{ .Values.workflow.namespace }}
+  {{- with .Values.workflow.namespace }}
+  namespace: {{ . }}
   {{- end }}
 {{- end }}
diff --git a/charts/argo-workflows/templates/controller/workflow-role.yaml b/charts/argo-workflows/templates/controller/workflow-role.yaml
index 7fb0bbc6..bf8b3cff 100644
--- a/charts/argo-workflows/templates/controller/workflow-role.yaml
+++ b/charts/argo-workflows/templates/controller/workflow-role.yaml
@@ -3,8 +3,8 @@ apiVersion: rbac.authorization.k8s.io/v1
 kind: Role
 metadata:
   name: {{ template "argo-workflows.fullname" . }}-workflow
-  {{- if .Values.workflow.namespace }}
-  namespace: {{ .Values.workflow.namespace }}
+  {{- with .Values.workflow.namespace }}
+  namespace: {{ . }}
   {{- end }}
 rules:
 - apiGroups:
diff --git a/charts/argo-workflows/templates/controller/workflow-sa.yaml b/charts/argo-workflows/templates/controller/workflow-sa.yaml
index 45d97cf0..dc84f2b1 100644
--- a/charts/argo-workflows/templates/controller/workflow-sa.yaml
+++ b/charts/argo-workflows/templates/controller/workflow-sa.yaml
@@ -3,8 +3,8 @@ apiVersion: v1
 kind: ServiceAccount
 metadata:
   name: {{ .Values.workflow.serviceAccount.name }}
-  {{- if .Values.workflow.namespace }}
-  namespace: {{ .Values.workflow.namespace }}
+  {{- with .Values.workflow.namespace }}
+  namespace: {{ . }}
   {{- end }}
   {{- with .Values.workflow.serviceAccount.annotations }}
   annotations:
diff --git a/charts/argo-workflows/templates/server/server-cluster-roles.yaml b/charts/argo-workflows/templates/server/server-cluster-roles.yaml
index 976a25c3..5901bbd1 100644
--- a/charts/argo-workflows/templates/server/server-cluster-roles.yaml
+++ b/charts/argo-workflows/templates/server/server-cluster-roles.yaml
@@ -1,10 +1,10 @@
 {{- if .Values.server.enabled }}
 apiVersion: rbac.authorization.k8s.io/v1
-{{- if .Values.singleNamespace }}
+  {{- if .Values.singleNamespace }}
 kind: Role
-{{- else }}
+  {{- else }}
 kind: ClusterRole
-{{- end }}
+  {{- end }}
 metadata:
   name: {{ template "argo-workflows.server.fullname" . }}
   labels:
@@ -30,7 +30,7 @@ rules:
   - list
   - watch
   - delete
-{{- if .Values.server.sso }}
+  {{- if .Values.server.sso }}
 - apiGroups:
   - ""
   resources:
@@ -46,9 +46,7 @@ rules:
   - secrets
   verbs:
   - create
-{{- end}}
-{{- if .Values.server.sso }}
-  {{- if .Values.server.sso.rbac }}
+    {{- if .Values.server.sso.rbac }}
 - apiGroups:
   - ""
   resources:
@@ -56,8 +54,8 @@ rules:
   verbs:
   - get
   - list
+    {{- end }}
   {{- end }}
-{{- end }}
 - apiGroups:
   - ""
   resources:
diff --git a/charts/argo-workflows/templates/server/server-deployment-pdb.yaml b/charts/argo-workflows/templates/server/server-deployment-pdb.yaml
index 452fa90a..aeba6547 100644
--- a/charts/argo-workflows/templates/server/server-deployment-pdb.yaml
+++ b/charts/argo-workflows/templates/server/server-deployment-pdb.yaml
@@ -1,5 +1,4 @@
-{{- if .Values.server.enabled -}}
-{{- if .Values.server.pdb.enabled -}}
+{{- if and .Values.server.enabled .Values.server.pdb.enabled -}}
 apiVersion: policy/v1beta1
 kind: PodDisruptionBudget
 metadata:
@@ -18,4 +17,3 @@ spec:
     matchLabels:
       {{- include "argo-workflows.selectorLabels" (dict "context" . "name" .Values.server.name) | nindent 6 }}
 {{- end -}}
-{{- end -}}
diff --git a/charts/argo-workflows/templates/server/server-deployment.yaml b/charts/argo-workflows/templates/server/server-deployment.yaml
index f5139a9f..cd6d45c6 100644
--- a/charts/argo-workflows/templates/server/server-deployment.yaml
+++ b/charts/argo-workflows/templates/server/server-deployment.yaml
@@ -16,17 +16,18 @@ spec:
       labels:
         {{- include "argo-workflows.labels" (dict "context" . "component" .Values.server.name "name" .Values.server.name) | nindent 8 }}
         app.kubernetes.io/version: {{ default .Chart.AppVersion .Values.server.image.tag | quote }}
-        {{- if .Values.server.podLabels }}
-        {{- toYaml .Values.server.podLabels | nindent 8 }}
+        {{- with .Values.server.podLabels }}
+          {{- toYaml . | nindent 8 }}
         {{- end }}
-      {{- if .Values.server.podAnnotations }}
+      {{- with .Values.server.podAnnotations }}
       annotations:
-{{ toYaml .Values.server.podAnnotations | indent 8}}{{- end }}
+        {{- toYaml .Values.server.podAnnotations | nindent 8 }}
+      {{- end }}
     spec:
       serviceAccountName: {{ .Values.server.serviceAccount | quote }}
-      {{- if .Values.server.podSecurityContext }}
+      {{- with .Values.server.podSecurityContext }}
       securityContext:
-        {{- toYaml .Values.server.podSecurityContext | nindent 8 }}
+        {{- toYaml . | nindent 8 }}
       {{- end }}
       containers:
         - name: argo-server
@@ -37,8 +38,8 @@ spec:
           args:
           - server
           - --configmap={{ .Release.Name }}-{{ .Values.controller.name }}-configmap
-          {{- if .Values.server.extraArgs }}
-          {{- toYaml .Values.server.extraArgs | nindent 10 }}
+          {{- with .Values.server.extraArgs }}
+            {{- toYaml . | nindent 10 }}
           {{- end }}
           - "--secure={{ .Values.server.secure }}"
           {{- if .Values.singleNamespace }}
@@ -98,7 +99,7 @@ spec:
       affinity:
         {{- toYaml . | nindent 8 }}
       {{- end }}
-      {{- if .Values.server.priorityClassName }}
-      priorityClassName: {{ .Values.server.priorityClassName }}
+      {{- with .Values.server.priorityClassName }}
+      priorityClassName: {{ . }}
       {{- end }}
 {{- end -}}
diff --git a/charts/argo-workflows/templates/server/server-ingress.yaml b/charts/argo-workflows/templates/server/server-ingress.yaml
index 4594fb18..48a84d24 100644
--- a/charts/argo-workflows/templates/server/server-ingress.yaml
+++ b/charts/argo-workflows/templates/server/server-ingress.yaml
@@ -3,10 +3,11 @@
 {{- $serviceName := include "argo-workflows.server.fullname" . -}}
 {{- $servicePort := .Values.server.servicePort -}}
 apiVersion: {{ include "argo-workflows.ingress.apiVersion" . }}
+kind: Ingress
 metadata:
   name: {{ template "argo-workflows.server.fullname" . }}
   labels:
-    {{- include "argo-workflows.labels" $ | nindent 4 }}
+   {{- include "argo-workflows.labels" (dict "context" . "component" .Values.server.name "name" .Values.server.name) | nindent 4 }}
     {{- range $key, $value := .Values.server.ingress.labels }}
     {{ $key }}: {{ $value | quote }}
     {{- end }}
@@ -16,24 +17,21 @@ metadata:
     {{- end }}
 spec:
   rules:
+    {{- $kubeVersion := .Capabilities.KubeVersion.GitVersion}}
     {{- range .Values.server.ingress.hosts }}
     - host: {{ . }}
       http:
         paths:
           {{- if $.Values.server.ingress.paths }}
           {{- range $.Values.server.ingress.paths }}
-          - backend:
-              serviceName: {{ .serviceName }}
-              servicePort: {{ .servicePort }}
+          {{- include "argo-workflows.ingress.service" (dict "kubeVersion" $kubeVersion "serviceName" .serviceName "servicePort" .servicePort) | nindent 10 }}
           {{- end }}
           {{- end }}
-          - backend:
-              serviceName: {{ $serviceName }}
-              servicePort: {{ $servicePort }}
+          {{- include "argo-workflows.ingress.service" (dict "kubeVersion" $kubeVersion "serviceName" $serviceName "servicePort" $servicePort) | nindent 10 }}
     {{- end -}}
-  {{- if .Values.server.ingress.tls }}
+    {{- with .Values.server.ingress.tls }}
   tls:
-{{ toYaml .Values.server.ingress.tls | indent 4 }}
+      {{- toYaml . | nindent 4 }}
+    {{- end -}}
   {{- end -}}
 {{- end -}}
-{{- end -}}
diff --git a/charts/argo-workflows/templates/server/server-sa.yaml b/charts/argo-workflows/templates/server/server-sa.yaml
index c0e2a255..1c9d20dc 100644
--- a/charts/argo-workflows/templates/server/server-sa.yaml
+++ b/charts/argo-workflows/templates/server/server-sa.yaml
@@ -3,6 +3,8 @@ apiVersion: v1
 kind: ServiceAccount
 metadata:
   name: {{ template "argo-workflows.serverServiceAccountName" . }}
+  {{- with .Values.server.serviceAccountAnnotations  }}
   annotations:
-{{ toYaml .Values.server.serviceAccountAnnotations | indent 4 }}
+    {{- toYaml . | indent 4 }}
+  {{- end }}
 {{- end -}}
diff --git a/charts/argo-workflows/templates/server/server-service.yaml b/charts/argo-workflows/templates/server/server-service.yaml
index 7938a8a7..5d161ee8 100644
--- a/charts/argo-workflows/templates/server/server-service.yaml
+++ b/charts/argo-workflows/templates/server/server-service.yaml
@@ -6,14 +6,15 @@ metadata:
   labels:
     {{- include "argo-workflows.labels" (dict "context" . "component" .Values.server.name "name" .Values.server.name) | nindent 4 }}
     app.kubernetes.io/version: {{ default .Chart.AppVersion .Values.server.image.tag | quote }}
-  {{- if .Values.server.serviceAnnotations }}
+  {{- with .Values.server.serviceAnnotations }}
   annotations:
-{{ toYaml .Values.server.serviceAnnotations | indent 4}}{{- end }}
+    {{- toYaml . | nindent 4 }}
+  {{- end }}
 spec:
   ports:
   - port: {{ .Values.server.servicePort }}
-    {{- if .Values.server.servicePortName }}
-    name: {{ .Values.server.servicePortName }}
+    {{- with .Values.server.servicePortName }}
+    name: {{ . }}
     {{- end }}
     targetPort: 2746
   selector:
@@ -25,5 +26,6 @@ spec:
   {{- end }}
   {{- if and (eq .Values.server.serviceType "LoadBalancer") .Values.server.loadBalancerSourceRanges }}
   loadBalancerSourceRanges:
-{{ toYaml .Values.server.loadBalancerSourceRanges | indent 4 }}{{- end }}
+    {{- toYaml .Values.server.loadBalancerSourceRanges | nindent 4 }}
+  {{- end }}
 {{- end -}}
diff --git a/charts/argo/Chart.yaml b/charts/argo/Chart.yaml
index 437df633..8e82de87 100644
--- a/charts/argo/Chart.yaml
+++ b/charts/argo/Chart.yaml
@@ -2,7 +2,7 @@ apiVersion: v2
 appVersion: v2.12.5
 description: A Helm chart for Argo Workflows
 name: argo
-version: 0.16.11
+version: 1.0.0
 icon: https://raw.githubusercontent.com/argoproj/argo/master/docs/assets/argo.png
 home: https://github.com/argoproj/argo-helm
 dependencies: