Webhook configurations are managed by Helm and updated by operator

[ChenYi015]

🛑 Important:

Please open an issue to discuss significant work before you start. We appreciate your contributions and don't want your efforts to go to waste!

For guidelines on how to contribute, please review the CONTRIBUTING.md document.

Purpose of this PR

Close #2070

Proposed changes:

• Webhooks are created and deleted by Helm, and the caBundle field is populated by operator
• values：
  • change webhook.namespaceSelector and webhook.objectSelector from string type to LabelSelector
  • add webhook.failurePolicy field
  • add webhook.timeoutSeconds field
• RBAC:
  • remove the create and delete verbs against webhook configurations and add resourceNames field
  • remove the create, patch and delete verbs against webhook configurations and add resourceNames field

Change Category

Indicate the type of change by marking the applicable boxes:

• Bugfix (non-breaking change which fixes an issue)
• Feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that could affect existing functionality)
• Documentation update

Rationale

Checklist

Before submitting your PR, please review the following:

• I have conducted a self-review of my own code.
• I have updated documentation accordingly.
• I have added tests that prove my changes are effective or that my feature works.
• Existing unit tests pass locally with my changes.

Additional Notes

[ChenYi015]

/assign @yuchaoran2011
/assign @vara-bonthu

[ChenYi015]

I had did e2e tests as following:

1. Create a kind config kind-config.yaml:

kind: Cluster
apiVersion: kind.x-k8s.io/v1alpha4
nodes:
- role: control-plane
- role: worker
- role: worker

2. Create a kind cluster:

kind create cluster --config kind-config.yaml

3. Build docker image and load into kind cluster:

docker build -t docker.io/kubeflow/spark-operator:local .
kind load docker-image docker.io/kubeflow/spark-operator:local

4. Install the helm chart with webhook enabled:

helm install spark-operator charts/spark-operator-chart \
    --namespace spark-operator \
    --create-namespace \
    --set image.tag=local \
    --set webhook.enable=true \
    --set resourceQuotaEnforcement.enable=true \
    --set 'sparkJobNamespaces[0]=default'

5. Inspect the mutatting webhook configurations to verify caBundle is populated correctly:

$ kubectl get mutatingwebhookconfigurations spark-operator-webhook -o yaml
apiVersion: admissionregistration.k8s.io/v1
kind: MutatingWebhookConfiguration
metadata:
  annotations:
    meta.helm.sh/release-name: spark-operator
    meta.helm.sh/release-namespace: spark-operator
  creationTimestamp: "2024-06-06T12:06:16Z"
  generation: 2
  labels:
    app.kubernetes.io/instance: spark-operator
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: spark-operator
    app.kubernetes.io/version: v1beta2-1.6.1-3.5.0
    helm.sh/chart: spark-operator-1.4.1
  name: spark-operator-webhook
  resourceVersion: "717"
  uid: 5d27c4c4-459d-45e0-be40-f6252249be8c
webhooks:
- admissionReviewVersions:
  - v1
  clientConfig:
    caBundle: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURxakNDQXBLZ0F3SUJBZ0lJUHcwZnFuYkMwWm93RFFZSktvWklodmNOQVFFTEJRQXdVVEVYTUJVR0ExVUUKQ2hNT2MzQmhjbXN0YjNCbGNtRjBiM0l4TmpBMEJnTlZCQU1UTFhOd1lYSnJMVzl3WlhKaGRHOXlMWGRsWW1odgpiMnN0YzNaakxuTndZWEpyTFc5d1pYSmhkRzl5TG5OMll6QWVGdzB5TkRBMk1EWXhNakEyTVRoYUZ3MHpOREEyCk1EWXhNakEyTVRoYU1GRXhGekFWQmdOVkJBb1REbk53WVhKckxXOXdaWEpoZEc5eU1UWXdOQVlEVlFRREV5MXoKY0dGeWF5MXZjR1Z5WVhSdmNpMTNaV0pvYjI5ckxYTjJZeTV6Y0dGeWF5MXZjR1Z5WVhSdmNpNXpkbU13Z2dFaQpNQTBHQ1NxR1NJYjNEUUVCQVFVQUE0SUJEd0F3Z2dFS0FvSUJBUURRMnRpamtQNnFBM2NZaS90Q2FQdXhYS08zCkdYakpKNnM3bWNzMWd1SEhaRHpZYjVIWFhISDQ0S2h6R25CSldZcFI0VGFhSkN1bEtIZkFVY1ZhSExuajZqcjgKWUdNaHBEUC9pQjV5d1NBU3lLU1Npay82VnVSMy9BL0U3NC9CZzhnQkUzVmRoSEFtcHZOOEY1MC9JcVBIZUx2SQoyS1kwOUV3R3phK2dnM2FhOWdZWVFSeWZydDM2bGtrN2dWOWdWTmUrOTl2UnBnQ2hOSC9SenNScWVsNmM2MmFuClRPN2tVWGNuZVltSWJBVXpENnV4eWJTMWIyc2JoaGFmbCtGRjZzaDlQTW1SRVFHNkFRcERFMmVRSGtpWVQ5RVkKZUNBbTI5NURlVW5CVkZ0K3FNd2h3Z1RjWEZ6RUk2MFFJU3hJWlNVaGl4OFFHcUxFY0hNbzRlOW16QTMxQWdNQgpBQUdqZ1lVd2dZSXdEZ1lEVlIwUEFRSC9CQVFEQWdYZ01CMEdBMVVkSlFRV01CUUdDQ3NHQVFVRkJ3TUJCZ2dyCkJnRUZCUWNEQWpBTUJnTlZIUk1CQWY4RUFqQUFNRU1HQTFVZEVRUThNRHFDQ1d4dlkyRnNhRzl6ZElJdGMzQmgKY21zdGIzQmxjbUYwYjNJdGQyVmlhRzl2YXkxemRtTXVjM0JoY21zdGIzQmxjbUYwYjNJdWMzWmpNQTBHQ1NxRwpTSWIzRFFFQkN3VUFBNElCQVFCYzJCOVdYalNkRGUrRUVSamg0NzBaSVZUcE1TdXZHTzlPMEY5SlBTWlpyWVlwCmU1RGY2dURUQUR1MFhTRmF1S2p0aVkxc2ZkVk1XNmFLNEtsMWZuTm9yemV3WlhOT1dQZ2Z2R2NHZ0Q2dDZWN3AKdUxIc1BOY1pKMThJSU13dG8xZFRlV1F0L3JLS1Zjck9XNlpyWmJ1TXh2Tjg3NWhCUUlEQ3J3VDZmWml0bCs0WQpUc0pKR3BGMy9SdjlKNTRXTzhUdEk1TEI2RkhDRTBIc1Raa1BMQ3NxS1paZHpGb2hDMEN0eUhxenFWYTNWY3ZQClU0RUs0ZFpFQlpuNm5XNm85QkpxbXYwRnZFSHJHZ3VDbkx6bnJlNGovMWEzNzVLZ3VVdFR5SmhMSWdrT3ZRWnMKNmtQZmo4RTN6SDV3dEErajJMN25NSm56aUtZQWxXWkt2SGRzNzBFagotLS0tLUVORCBDRVJUSUZJQ0FURS0tLS0tCg==
    service:
      name: spark-operator-webhook-svc
      namespace: spark-operator
      path: /webhook
      port: 8080
  failurePolicy: Ignore
  matchPolicy: Equivalent
  name: webhook.sparkoperator.k8s.io
  namespaceSelector:
    matchExpressions:
    - key: kubernetes.io/metadata.name
      operator: In
      values:
      - default
  objectSelector:
    matchExpressions:
    - key: sparkoperator.k8s.io/app-name
      operator: Exists
    - key: spark-role
      operator: Exists
    matchLabels:
      sparkoperator.k8s.io/launched-by-spark-operator: "true"
  reinvocationPolicy: Never
  rules:
  - apiGroups:
    - ""
    apiVersions:
    - v1
    operations:
    - CREATE
    resources:
    - pods
    scope: '*'
  sideEffects: NoneOnDryRun
  timeoutSeconds: 30

6. Inspect the validating webhook configurations to verify caBundle is populated correctly:

$ kubectl get validatingwebhookconfigurations.admissionregistration.k8s.io spark-operator-webhook -o yaml
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
metadata:
  annotations:
    meta.helm.sh/release-name: spark-operator
    meta.helm.sh/release-namespace: spark-operator
  creationTimestamp: "2024-06-06T12:06:16Z"
  generation: 2
  labels:
    app.kubernetes.io/instance: spark-operator
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: spark-operator
    app.kubernetes.io/version: v1beta2-1.6.1-3.5.0
    helm.sh/chart: spark-operator-1.4.1
  name: spark-operator-webhook
  resourceVersion: "718"
  uid: 7a8cb008-ec02-413b-bdd7-2d7133aa6438
webhooks:
- admissionReviewVersions:
  - v1
  clientConfig:
    caBundle: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURxakNDQXBLZ0F3SUJBZ0lJUHcwZnFuYkMwWm93RFFZSktvWklodmNOQVFFTEJRQXdVVEVYTUJVR0ExVUUKQ2hNT2MzQmhjbXN0YjNCbGNtRjBiM0l4TmpBMEJnTlZCQU1UTFhOd1lYSnJMVzl3WlhKaGRHOXlMWGRsWW1odgpiMnN0YzNaakxuTndZWEpyTFc5d1pYSmhkRzl5TG5OMll6QWVGdzB5TkRBMk1EWXhNakEyTVRoYUZ3MHpOREEyCk1EWXhNakEyTVRoYU1GRXhGekFWQmdOVkJBb1REbk53WVhKckxXOXdaWEpoZEc5eU1UWXdOQVlEVlFRREV5MXoKY0dGeWF5MXZjR1Z5WVhSdmNpMTNaV0pvYjI5ckxYTjJZeTV6Y0dGeWF5MXZjR1Z5WVhSdmNpNXpkbU13Z2dFaQpNQTBHQ1NxR1NJYjNEUUVCQVFVQUE0SUJEd0F3Z2dFS0FvSUJBUURRMnRpamtQNnFBM2NZaS90Q2FQdXhYS08zCkdYakpKNnM3bWNzMWd1SEhaRHpZYjVIWFhISDQ0S2h6R25CSldZcFI0VGFhSkN1bEtIZkFVY1ZhSExuajZqcjgKWUdNaHBEUC9pQjV5d1NBU3lLU1Npay82VnVSMy9BL0U3NC9CZzhnQkUzVmRoSEFtcHZOOEY1MC9JcVBIZUx2SQoyS1kwOUV3R3phK2dnM2FhOWdZWVFSeWZydDM2bGtrN2dWOWdWTmUrOTl2UnBnQ2hOSC9SenNScWVsNmM2MmFuClRPN2tVWGNuZVltSWJBVXpENnV4eWJTMWIyc2JoaGFmbCtGRjZzaDlQTW1SRVFHNkFRcERFMmVRSGtpWVQ5RVkKZUNBbTI5NURlVW5CVkZ0K3FNd2h3Z1RjWEZ6RUk2MFFJU3hJWlNVaGl4OFFHcUxFY0hNbzRlOW16QTMxQWdNQgpBQUdqZ1lVd2dZSXdEZ1lEVlIwUEFRSC9CQVFEQWdYZ01CMEdBMVVkSlFRV01CUUdDQ3NHQVFVRkJ3TUJCZ2dyCkJnRUZCUWNEQWpBTUJnTlZIUk1CQWY4RUFqQUFNRU1HQTFVZEVRUThNRHFDQ1d4dlkyRnNhRzl6ZElJdGMzQmgKY21zdGIzQmxjbUYwYjNJdGQyVmlhRzl2YXkxemRtTXVjM0JoY21zdGIzQmxjbUYwYjNJdWMzWmpNQTBHQ1NxRwpTSWIzRFFFQkN3VUFBNElCQVFCYzJCOVdYalNkRGUrRUVSamg0NzBaSVZUcE1TdXZHTzlPMEY5SlBTWlpyWVlwCmU1RGY2dURUQUR1MFhTRmF1S2p0aVkxc2ZkVk1XNmFLNEtsMWZuTm9yemV3WlhOT1dQZ2Z2R2NHZ0Q2dDZWN3AKdUxIc1BOY1pKMThJSU13dG8xZFRlV1F0L3JLS1Zjck9XNlpyWmJ1TXh2Tjg3NWhCUUlEQ3J3VDZmWml0bCs0WQpUc0pKR3BGMy9SdjlKNTRXTzhUdEk1TEI2RkhDRTBIc1Raa1BMQ3NxS1paZHpGb2hDMEN0eUhxenFWYTNWY3ZQClU0RUs0ZFpFQlpuNm5XNm85QkpxbXYwRnZFSHJHZ3VDbkx6bnJlNGovMWEzNzVLZ3VVdFR5SmhMSWdrT3ZRWnMKNmtQZmo4RTN6SDV3dEErajJMN25NSm56aUtZQWxXWkt2SGRzNzBFagotLS0tLUVORCBDRVJUSUZJQ0FURS0tLS0tCg==
    service:
      name: spark-operator-webhook-svc
      namespace: spark-operator
      path: /webhook
      port: 8080
  failurePolicy: Ignore
  matchPolicy: Equivalent
  name: quotaenforcer.sparkoperator.k8s.io
  namespaceSelector:
    matchExpressions:
    - key: kubernetes.io/metadata.name
      operator: In
      values:
      - default
  objectSelector:
    matchExpressions:
    - key: sparkoperator.k8s.io/app-name
      operator: Exists
    - key: spark-role
      operator: Exists
    matchLabels:
      sparkoperator.k8s.io/launched-by-spark-operator: "true"
  rules:
  - apiGroups:
    - sparkoperator.k8s.io
    apiVersions:
    - v1beta2
    operations:
    - CREATE
    - UPDATE
    resources:
    - sparkapplications
    - scheduledsparkapplications
    scope: '*'
  sideEffects: NoneOnDryRun
  timeoutSeconds: 30

[yuchaoran2011]

@ChenYi015 Could you elaborate on the benefit of making the chart create the web hook configuration?

[ChenYi015]

@ChenYi015 Could you elaborate on the benefit of making the chart create the web hook configuration?

@yuchaoran2011 I believe it can simplify the webhook configuration process and make the process of introducing new features more easily. Specifically, we do not need to add new command-line flags to the spark-operator executable. Instead, webhook configurations can be dynamically generated using Helm templates. For instance, if we want to add namespaceSelector to filter out Spark pods only in the Spark job namespaces, it will be more straightforward with the help of helm template rendering.

[vara-bonthu]

Nice enhancement!

[google-oss-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vara-bonthu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [vara-bonthu]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[yuchaoran2011]

@ChenYi015 Could you resolve the merge conflicts?

[ChenYi015]

@ChenYi015 Could you resolve the merge conflicts?

@yuchaoran2011 I had did a rebase and there is still something we should consider into:

• Whether we should split the webhook configurations into mutaing webhook and validating webhook seperately, since the objectSelector is different.
• We should not run the spark job when the webhook server does not work, it is better to enable webhook by default.

[yuchaoran2011]

Whether we should split the webhook configurations into mutaing webhook and validating webhook seperately, since the objectSelector is different.
We should not run the spark job when the webhook server does not work, it is better to enable webhook by default.
Could you remind me what operator uses validating webhooks for?

I agree that webhook is indispensable at the moment. Longer term though, it makes more sense to ditch it entirely in favor of Spark's native pod template feature. Originally mutating web hooks were introduced in the operator to support pre Spark 3.0 versions. Nowadays, most users run Spark on Kubernetes with Spark 3.0 or higher. Webhooks should no longer be needed

[ChenYi015]

@yuchaoran2011 Totally agree, we should use pod template rather than webhook.

[vara-bonthu]

Agree! Shall we add some examples to use pod templates with instructions to disable webhooks for the helm deployment?