Certifictes are generated by operator rather than gencerts.sh (#2016)

Signed-off-by: Yi Chen <[email protected]>
kubeflow · Jun 5, 2024 · 5ce3dba · 5ce3dba
1 parent 089546e
commit 5ce3dba
Show file tree

Hide file tree

Showing 22 changed files with 530 additions and 499 deletions.
diff --git a/Dockerfile b/Dockerfile
@@ -40,9 +40,9 @@ USER root
 COPY --from=builder /usr/bin/spark-operator /usr/bin/
 RUN apt-get update --allow-releaseinfo-change \
     && apt-get update \
-    && apt-get install -y openssl curl tini \
+    && apt-get install -y tini \
     && rm -rf /var/lib/apt/lists/*
-COPY hack/gencerts.sh /usr/bin/
 
 COPY entrypoint.sh /usr/bin/
+
 ENTRYPOINT ["/usr/bin/entrypoint.sh"]
diff --git a/Makefile b/Makefile
@@ -45,7 +45,7 @@ build-api-docs:
 			-out-file /repo/docs/api-docs.md"
 
 helm-unittest:
-	helm unittest charts/spark-operator-chart --strict
+	helm unittest charts/spark-operator-chart --strict --file "tests/**/*_test.yaml"
 
 helm-lint:
 	docker run --rm --workdir /workspace --volume "$$(pwd):/workspace" quay.io/helmpack/chart-testing:latest ct lint

diff --git a/charts/spark-operator-chart/Chart.yaml b/charts/spark-operator-chart/Chart.yaml
@@ -1,8 +1,8 @@
 apiVersion: v2
 name: spark-operator
 description: A Helm chart for Spark on Kubernetes operator
-version: 1.3.2
-appVersion: v1beta2-1.5.0-3.5.0
+version: 1.4.0
+appVersion: v1beta2-1.6.0-3.5.0
 keywords:
   - spark
 home: https://github.com/kubeflow/spark-operator

diff --git a/charts/spark-operator-chart/README.md b/charts/spark-operator-chart/README.md
@@ -1,6 +1,6 @@
 # spark-operator
 
-![Version: 1.3.2](https://img.shields.io/badge/Version-1.3.2-informational?style=flat-square) ![AppVersion: v1beta2-1.5.0-3.5.0](https://img.shields.io/badge/AppVersion-v1beta2--1.5.0--3.5.0-informational?style=flat-square)
+![Version: 1.4.0](https://img.shields.io/badge/Version-1.4.0-informational?style=flat-square) ![AppVersion: v1beta2-1.6.0-3.5.0](https://img.shields.io/badge/AppVersion-v1beta2--1.6.0--3.5.0-informational?style=flat-square)
 
 A Helm chart for Spark on Kubernetes operator
 
@@ -131,17 +131,11 @@ See [helm uninstall](https://helm.sh/docs/helm/helm_uninstall) for command docum
 | uiService.enable | bool | `true` | Enable UI service creation for Spark application |
 | volumeMounts | list | `[]` |  |
 | volumes | list | `[]` |  |
-| webhook.cleanupAnnotations | object | `{"helm.sh/hook":"pre-delete, pre-upgrade","helm.sh/hook-delete-policy":"hook-succeeded"}` | The annotations applied to the cleanup job, required for helm lifecycle hooks |
-| webhook.cleanupPodLabels | object | `{}` | The podLabels applied to the pod of the cleanup job |
-| webhook.cleanupResources | object | `{}` | Resources applied to cleanup job |
 | webhook.enable | bool | `false` | Enable webhook server |
-| webhook.initAnnotations | object | `{"helm.sh/hook":"pre-install, pre-upgrade","helm.sh/hook-weight":"50"}` | The annotations applied to init job, required to restore certs deleted by the cleanup job during upgrade |
-| webhook.initPodLabels | object | `{}` | The podLabels applied to the pod of the init job |
-| webhook.initResources | object | `{}` | Resources applied to init job |
 | webhook.namespaceSelector | string | `""` | The webhook server will only operate on namespaces with this label, specified in the form key1=value1,key2=value2. Empty string (default) will operate on all namespaces |
 | webhook.port | int | `8080` | Webhook service port |
 | webhook.portName | string | `"webhook"` | Webhook container port name and service target port name |
-| webhook.timeout | int | `30` |  |
+| webhook.timeout | int | `30` | The annotations applied to init job, required to restore certs deleted by the cleanup job during upgrade |
 
 ## Maintainers
 

diff --git a/charts/spark-operator-chart/templates/deployment.yaml b/charts/spark-operator-chart/templates/deployment.yaml
@@ -92,11 +92,13 @@ spec:
         {{- end }}
         {{- if .Values.webhook.enable }}
         - -enable-webhook=true
+        - -webhook-secret-name={{ include "spark-operator.webhookSecretName" . }}
+        - -webhook-secret-namespace={{ .Release.Namespace }}
+        - -webhook-svc-name={{ include "spark-operator.webhookServiceName" . }}
         - -webhook-svc-namespace={{ .Release.Namespace }}
+        - -webhook-config-name={{ include "spark-operator.fullname" . }}-webhook-config
         - -webhook-port={{ .Values.webhook.port }}
         - -webhook-timeout={{ .Values.webhook.timeout }}
-        - -webhook-svc-name={{ include "spark-operator.fullname" . }}-webhook
-        - -webhook-config-name={{ include "spark-operator.fullname" . }}-webhook-config
         - -webhook-namespace-selector={{ .Values.webhook.namespaceSelector }}
         {{- end }}
         - -enable-resource-quota-enforcement={{ .Values.resourceQuotaEnforcement.enable }}
@@ -109,30 +111,17 @@ spec:
         resources:
           {{- toYaml . | nindent 10 }}
         {{- end }}
-        {{- if or .Values.webhook.enable (ne (len .Values.volumeMounts) 0 ) }}
-        volumeMounts:
-        {{- end }}
-          {{- if .Values.webhook.enable }}
-          - name: webhook-certs
-            mountPath: /etc/webhook-certs
-          {{- end }}
         {{- with .Values.volumeMounts }}
-        {{- toYaml . | nindent 10 }}
+        volumeMounts:
+          {{- toYaml . | nindent 10 }}
         {{- end }}
       {{- with .Values.sidecars }}
         {{- toYaml . | nindent 6 }}
       {{- end }}
-      {{- if or .Values.webhook.enable (ne (len .Values.volumes) 0 ) }}
+      {{- with .Values.volumes }}
       volumes:
-      {{- end }}
-        {{- if .Values.webhook.enable }}
-        - name: webhook-certs
-          secret:
-            secretName: {{ include "spark-operator.fullname" . }}-webhook-certs
-        {{- end }}
-        {{- with .Values.volumes }}
         {{- toYaml . | nindent 8 }}
-        {{- end }}
+      {{- end }}
       {{- with .Values.nodeSelector }}
       nodeSelector:
         {{- toYaml . | nindent 8 }}

diff --git a/charts/spark-operator-chart/templates/rbac.yaml b/charts/spark-operator-chart/templates/rbac.yaml
@@ -3,15 +3,12 @@ apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRole
 metadata:
   name: {{ include "spark-operator.fullname" . }}
-  annotations:
-    "helm.sh/hook": pre-install, pre-upgrade
-    "helm.sh/hook-delete-policy": hook-failed, before-hook-creation
-    "helm.sh/hook-weight": "-10"
-    {{- with .Values.rbac.annotations }}
-    {{- toYaml . | nindent 4 }}
-    {{- end }}
   labels:
     {{- include "spark-operator.labels" . | nindent 4 }}
+  {{- with .Values.rbac.annotations }}
+  annotations:
+    {{- toYaml . | nindent 4 }}
+  {{- end }}
 rules:
 - apiGroups:
   - ""
@@ -134,12 +131,12 @@ apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRoleBinding
 metadata:
   name: {{ include "spark-operator.fullname" . }}
-  annotations:
-    "helm.sh/hook": pre-install, pre-upgrade
-    "helm.sh/hook-delete-policy": hook-failed, before-hook-creation
-    "helm.sh/hook-weight": "-10"
   labels:
     {{- include "spark-operator.labels" . | nindent 4 }}
+  {{- with .Values.rbac.annotations }}
+  annotations:
+    {{- toYaml . | nindent 4 }}
+  {{- end }}
 subjects:
   - kind: ServiceAccount
     name: {{ include "spark-operator.serviceAccountName" . }}

diff --git a/charts/spark-operator-chart/templates/serviceaccount.yaml b/charts/spark-operator-chart/templates/serviceaccount.yaml
@@ -3,13 +3,10 @@ apiVersion: v1
 kind: ServiceAccount
 metadata:
   name: {{ include "spark-operator.serviceAccountName" . }}
-  annotations:
-    "helm.sh/hook": pre-install, pre-upgrade
-    "helm.sh/hook-delete-policy": hook-failed, before-hook-creation
-    "helm.sh/hook-weight": "-10"
-{{- with .Values.serviceAccounts.sparkoperator.annotations }}
-{{ toYaml . | indent 4 }}
-{{- end }}
   labels:
     {{- include "spark-operator.labels" . | nindent 4 }}
+  {{- with .Values.serviceAccounts.sparkoperator.annotations }}
+  annotations:
+    {{- toYaml . | nindent 4 }}
+  {{- end }}
 {{- end }}
diff --git a/charts/spark-operator-chart/templates/webhook-cleanup-job.yaml b/charts/spark-operator-chart/templates/webhook-cleanup-job.yaml
diff --git a/charts/spark-operator-chart/templates/webhook-init-job.yaml b/charts/spark-operator-chart/templates/webhook-init-job.yaml
diff --git a/charts/spark-operator-chart/templates/webhook/_helpers.tpl b/charts/spark-operator-chart/templates/webhook/_helpers.tpl
@@ -0,0 +1,14 @@
+{{/*
+Create the name of the secret to be used by webhook
+*/}}
+{{- define "spark-operator.webhookSecretName" -}}
+{{ include "spark-operator.fullname" . }}-webhook-certs
+{{- end -}}
+
+
+{{/*
+Create the name of the service to be used by webhook
+*/}}
+{{- define "spark-operator.webhookServiceName" -}}
+{{ include "spark-operator.fullname" . }}-webhook-svc
+{{- end -}}
diff --git a/charts/spark-operator-chart/templates/webhook/secret.yaml b/charts/spark-operator-chart/templates/webhook/secret.yaml
@@ -0,0 +1,13 @@
+{{- if .Values.webhook.enable -}}
+apiVersion: v1
+kind: Secret
+metadata:
+  name: {{ include "spark-operator.webhookSecretName" . }}
+  labels:
+    {{- include "spark-operator.labels" . | nindent 4 }}
+data:
+  ca-key.pem: ""
+  ca-cert.pem: ""
+  server-key.pem: ""
+  server-cert.pem: ""
+{{- end }}
diff --git a/...ator-chart/templates/webhook-service.yaml → ...ator-chart/templates/webhook/service.yaml b/...ator-chart/templates/webhook-service.yaml → ...ator-chart/templates/webhook/service.yaml
@@ -1,15 +1,15 @@
-{{ if .Values.webhook.enable }}
-kind: Service
+{{- if .Values.webhook.enable -}}
 apiVersion: v1
+kind: Service
 metadata:
-  name: {{ include "spark-operator.fullname" . }}-webhook
+  name: {{ include "spark-operator.webhookServiceName" . }}
   labels:
     {{- include "spark-operator.labels" . | nindent 4 }}
 spec:
+  selector:
+    {{- include "spark-operator.selectorLabels" . | nindent 4 }}
   ports:
   - port: 443
     targetPort: {{ .Values.webhook.portName | quote }}
-    name: webhook
-  selector:
-    {{- include "spark-operator.selectorLabels" . | nindent 4 }}
-{{ end }}
+    name: {{ .Values.webhook.portName }}
+{{- end }}
diff --git a/charts/spark-operator-chart/tests/deployment_test.yaml b/charts/spark-operator-chart/tests/deployment_test.yaml
@@ -160,31 +160,6 @@ tests:
               memory: "128Mi"
               cpu: "500m"
 
-  - it: Should add webhook certs volume if webhook.enable is true
-    set:
-      webhook:
-        enable: true
-    asserts:
-      - contains:
-          path: spec.template.spec.volumes
-          content:
-            name: webhook-certs
-            secret:
-              secretName: spark-operator-webhook-certs
-          count: 1
-
-  - it: Should add webhook certs volume mounts if webhook.enable is true
-    set:
-      webhook:
-        enable: true
-    asserts:
-      - contains:
-          path: spec.template.spec.containers[0].volumeMounts
-          content:
-            name: webhook-certs
-            mountPath: /etc/webhook-certs
-          count: 1
-
   - it: Should add sidecars if sidecars is set
     set:
       sidecars:

diff --git a/charts/spark-operator-chart/tests/webhook/secret_test.yaml b/charts/spark-operator-chart/tests/webhook/secret_test.yaml
@@ -0,0 +1,31 @@
+suite: Test spark operator webhook secret
+
+templates:
+  - webhook/secret.yaml
+
+release:
+  name: spark-operator
+  namespace: spark-operator
+
+tests:
+  - it: Should not render the webhook secret if webhook.enable is false
+    asserts:
+      - hasDocuments:
+          count: 0
+
+  - it: Should render the webhook secret with empty data fields
+    set:
+      webhook:
+        enable: true
+    asserts:
+      - containsDocument:
+          apiVersion: v1
+          kind: Secret
+          name: spark-operator-webhook-certs
+      - equal:
+          path: data
+          value:
+            ca-key.pem: ""
+            ca-cert.pem: ""
+            server-key.pem: ""
+            server-cert.pem: ""