From b343aecb8e3be6e2b31a698e8eb96f1e07e65b34 Mon Sep 17 00:00:00 2001
From: Arik Hadas <ahadas@redhat.com>
Date: Wed, 20 Sep 2023 09:12:50 +0300
Subject: [PATCH] Single webhook per configuration entity

We've noticed an unexpected behavior in OpenShift:
1. ValidatingWebhookConfiguration or MutatingWebhookConfiguration is
   posted with a webhook
2. CA bundle is injected to the webhook
3. More webhooks are added to the configuration entity
4. Some of the webhooks (in our case it has always been the last one)
   does not get a CA bundle injected

This is a workaround for this issue - avoid adding webhooks to an
existing configuration entity by separating each webhook to a different
configuration entity.

Signed-off-by: Arik Hadas <ahadas@redhat.com>
---
 .../roles/forkliftcontroller/tasks/main.yml   | 31 ++++++-
 ...ngwebhookconfiguration-forklift-api.yml.j2 | 63 +-------------
 .../mutatingwebhookconfiguration-plans.yml.j2 | 38 +++++++++
 ...utatingwebhookconfiguration-secrets.yml.j2 | 42 +++++++++
 ...ngwebhookconfiguration-forklift-api.yml.j2 | 85 +------------------
 ...alidatingwebhookconfiguration-plans.yml.j2 | 37 ++++++++
 ...atingwebhookconfiguration-providers.yml.j2 | 37 ++++++++
 ...idatingwebhookconfiguration-secrets.yml.j2 | 41 +++++++++
 8 files changed, 227 insertions(+), 147 deletions(-)
 create mode 100644 operator/roles/forkliftcontroller/templates/api/mutatingwebhookconfiguration-plans.yml.j2
 create mode 100644 operator/roles/forkliftcontroller/templates/api/mutatingwebhookconfiguration-secrets.yml.j2
 create mode 100644 operator/roles/forkliftcontroller/templates/api/validatingwebhookconfiguration-plans.yml.j2
 create mode 100644 operator/roles/forkliftcontroller/templates/api/validatingwebhookconfiguration-providers.yml.j2
 create mode 100644 operator/roles/forkliftcontroller/templates/api/validatingwebhookconfiguration-secrets.yml.j2

diff --git a/operator/roles/forkliftcontroller/tasks/main.yml b/operator/roles/forkliftcontroller/tasks/main.yml
index 72721fa47..d823f6485 100644
--- a/operator/roles/forkliftcontroller/tasks/main.yml
+++ b/operator/roles/forkliftcontroller/tasks/main.yml
@@ -125,16 +125,41 @@
       state: present
       definition: "{{ lookup('template', 'api/deployment-forklift-api.yml.j2') }}"
 
-  - name: "Setup api validating webhook configuration"
+  - name: "Delete aggregated validation webhook configurations"
     k8s:
-      state: present
+      state: absent
       definition: "{{ lookup('template', 'api/validatingwebhookconfiguration-forklift-api.yml.j2') }}"
 
-  - name: "Setup api mutating webhook configuration"
+  - name: "Setup secrets validating webhook configuration"
+    k8s:
+      state: present
+      definition: "{{ lookup('template', 'api/validatingwebhookconfiguration-secrets.yml.j2') }}"
+
+  - name: "Setup plans validating webhook configuration"
     k8s:
       state: present
+      definition: "{{ lookup('template', 'api/validatingwebhookconfiguration-plans.yml.j2') }}"
+
+  - name: "Setup providers validating webhook configuration"
+    k8s:
+      state: present
+      definition: "{{ lookup('template', 'api/validatingwebhookconfiguration-providers.yml.j2') }}"
+
+  - name: "Delete aggregated mutating webhook configurations"
+    k8s:
+      state: absent
       definition: "{{ lookup('template', 'api/mutatingwebhookconfiguration-forklift-api.yml.j2') }}"
 
+  - name: "Setup secrets mutating webhook configuration"
+    k8s:
+      state: present
+      definition: "{{ lookup('template', 'api/mutatingwebhookconfiguration-secrets.yml.j2') }}"
+
+  - name: "Setup plans mutating webhook configuration"
+    k8s:
+      state: present
+      definition: "{{ lookup('template', 'api/mutatingwebhookconfiguration-plans.yml.j2') }}"
+
   - name: "Setup default provider"
     k8s:
       state: present
diff --git a/operator/roles/forkliftcontroller/templates/api/mutatingwebhookconfiguration-forklift-api.yml.j2 b/operator/roles/forkliftcontroller/templates/api/mutatingwebhookconfiguration-forklift-api.yml.j2
index 6cdbc6050..f34ba079b 100644
--- a/operator/roles/forkliftcontroller/templates/api/mutatingwebhookconfiguration-forklift-api.yml.j2
+++ b/operator/roles/forkliftcontroller/templates/api/mutatingwebhookconfiguration-forklift-api.yml.j2
@@ -1,67 +1,8 @@
+# Do not add webhooks to this file
+# This configuration has been deprecated
 ---
 apiVersion: admissionregistration.k8s.io/v1
 kind: MutatingWebhookConfiguration
 metadata:
   name: {{ api_deployment_name }}
   namespace: ""
-  annotations:
-{% if k8s_cluster|bool %}
-    cert-manager.io/inject-ca-from: {{ app_namespace }}/{{ api_certificate_name }}
-{% else %}
-    service.beta.openshift.io/inject-cabundle: "true"
-{% endif %}
-webhooks:
-- admissionReviewVersions:
-  - v1
-  clientConfig:
-    service:
-      name: {{ api_service_name }}
-      namespace: {{ app_namespace }}
-      path: /secret-mutate
-      port: 443
-  failurePolicy: Fail
-  matchPolicy: Equivalent
-  name: ca-mutatur.forklift.konveyor
-  namespaceSelector: {}
-  objectSelector: 
-    matchExpressions:
-    - key: createdForProviderType
-      operator: Exists
-  rules:
-  - apiGroups:
-    - ''
-    apiVersions:
-    - v1
-    operations:
-    - CREATE
-    - UPDATE
-    resources:
-    - secrets
-    scope: Namespaced
-  sideEffects: None
-  timeoutSeconds: 30
-- admissionReviewVersions:
-  - v1
-  clientConfig:
-    service:
-      name: {{ api_service_name }}
-      namespace: {{ app_namespace }}
-      path: /plan-mutate
-      port: 443
-  failurePolicy: Fail
-  matchPolicy: Equivalent
-  name: plans.forklift.konveyor
-  namespaceSelector: {}
-  objectSelector: {}
-  rules:
-  - apiGroups:
-    - forklift.konveyor.io
-    resources:
-    - plans
-    apiVersions:
-    - v1beta1
-    operations:
-    - CREATE
-    - UPDATE
-  sideEffects: None
-  timeoutSeconds: 30
diff --git a/operator/roles/forkliftcontroller/templates/api/mutatingwebhookconfiguration-plans.yml.j2 b/operator/roles/forkliftcontroller/templates/api/mutatingwebhookconfiguration-plans.yml.j2
new file mode 100644
index 000000000..a764b6a29
--- /dev/null
+++ b/operator/roles/forkliftcontroller/templates/api/mutatingwebhookconfiguration-plans.yml.j2
@@ -0,0 +1,38 @@
+---
+apiVersion: admissionregistration.k8s.io/v1
+kind: MutatingWebhookConfiguration
+metadata:
+  name: {{ api_deployment_name }}-plans
+  namespace: ""
+  annotations:
+{% if k8s_cluster|bool %}
+    cert-manager.io/inject-ca-from: {{ app_namespace }}/{{ api_certificate_name }}
+{% else %}
+    service.beta.openshift.io/inject-cabundle: "true"
+{% endif %}
+webhooks:
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: {{ api_service_name }}
+      namespace: {{ app_namespace }}
+      path: /plan-mutate
+      port: 443
+  failurePolicy: Fail
+  matchPolicy: Equivalent
+  name: plans.forklift.konveyor
+  namespaceSelector: {}
+  objectSelector: {}
+  rules:
+  - apiGroups:
+    - forklift.konveyor.io
+    resources:
+    - plans
+    apiVersions:
+    - v1beta1
+    operations:
+    - CREATE
+    - UPDATE
+  sideEffects: None
+  timeoutSeconds: 30
diff --git a/operator/roles/forkliftcontroller/templates/api/mutatingwebhookconfiguration-secrets.yml.j2 b/operator/roles/forkliftcontroller/templates/api/mutatingwebhookconfiguration-secrets.yml.j2
new file mode 100644
index 000000000..92717d6e7
--- /dev/null
+++ b/operator/roles/forkliftcontroller/templates/api/mutatingwebhookconfiguration-secrets.yml.j2
@@ -0,0 +1,42 @@
+---
+apiVersion: admissionregistration.k8s.io/v1
+kind: MutatingWebhookConfiguration
+metadata:
+  name: {{ api_deployment_name }}-secrets
+  namespace: ""
+  annotations:
+{% if k8s_cluster|bool %}
+    cert-manager.io/inject-ca-from: {{ app_namespace }}/{{ api_certificate_name }}
+{% else %}
+    service.beta.openshift.io/inject-cabundle: "true"
+{% endif %}
+webhooks:
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: {{ api_service_name }}
+      namespace: {{ app_namespace }}
+      path: /secret-mutate
+      port: 443
+  failurePolicy: Fail
+  matchPolicy: Equivalent
+  name: ca-mutatur.forklift.konveyor
+  namespaceSelector: {}
+  objectSelector: 
+    matchExpressions:
+    - key: createdForProviderType
+      operator: Exists
+  rules:
+  - apiGroups:
+    - ''
+    apiVersions:
+    - v1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - secrets
+    scope: Namespaced
+  sideEffects: None
+  timeoutSeconds: 30
diff --git a/operator/roles/forkliftcontroller/templates/api/validatingwebhookconfiguration-forklift-api.yml.j2 b/operator/roles/forkliftcontroller/templates/api/validatingwebhookconfiguration-forklift-api.yml.j2
index 7a3659a13..1fb07014c 100644
--- a/operator/roles/forkliftcontroller/templates/api/validatingwebhookconfiguration-forklift-api.yml.j2
+++ b/operator/roles/forkliftcontroller/templates/api/validatingwebhookconfiguration-forklift-api.yml.j2
@@ -1,89 +1,8 @@
+# Do not add webhooks to this file
+# This configuration has been deprecated
 ---
 apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingWebhookConfiguration
 metadata:
   name: {{ api_deployment_name }}
   namespace: ""
-  annotations:
-{% if k8s_cluster|bool %}
-    cert-manager.io/inject-ca-from: {{ app_namespace }}/{{ api_certificate_name }}
-{% else %}
-    service.beta.openshift.io/inject-cabundle: "true"
-{% endif %}
-webhooks:
-- admissionReviewVersions:
-  - v1
-  clientConfig:
-    service:
-      name: {{ api_service_name }}
-      namespace: {{ app_namespace }}
-      path: /secret-validate
-      port: 443
-  failurePolicy: Fail
-  matchPolicy: Equivalent
-  name: secrets.forklift.konveyor
-  namespaceSelector: {}
-  objectSelector:
-    matchExpressions:
-    - key: createdForResourceType
-      operator: Exists
-  rules:
-  - apiGroups:
-    - ''
-    apiVersions:
-    - v1
-    operations:
-    - CREATE
-    - UPDATE
-    resources:
-    - secrets
-    scope: Namespaced
-  sideEffects: None
-- admissionReviewVersions:
-  - v1
-  clientConfig:
-    service:
-      name: {{ api_service_name }}
-      namespace: {{ app_namespace }}
-      path: /plan-validate
-      port: 443
-  failurePolicy: Fail
-  matchPolicy: Equivalent
-  name: plans.forklift.konveyor
-  namespaceSelector: {}
-  objectSelector: {}
-  rules:
-  - apiGroups:
-    - forklift.konveyor.io
-    resources:
-    - plans
-    apiVersions:
-    - v1beta1
-    operations:
-    - CREATE
-    - UPDATE
-  sideEffects: None
-- admissionReviewVersions:
-  - v1
-  clientConfig:
-    service:
-      name: {{ api_service_name }}
-      namespace: {{ app_namespace }}
-      path: /provider-validate
-      port: 443
-  failurePolicy: Fail
-  matchPolicy: Equivalent
-  name: providers.forklift.konveyor
-  namespaceSelector: {}
-  objectSelector: {}
-  rules:
-  - apiGroups:
-    - forklift.konveyor.io
-    resources:
-    - providers
-    apiVersions:
-    - v1beta1
-    operations:
-    - CREATE
-    - UPDATE
-  sideEffects: None
diff --git a/operator/roles/forkliftcontroller/templates/api/validatingwebhookconfiguration-plans.yml.j2 b/operator/roles/forkliftcontroller/templates/api/validatingwebhookconfiguration-plans.yml.j2
new file mode 100644
index 000000000..569493681
--- /dev/null
+++ b/operator/roles/forkliftcontroller/templates/api/validatingwebhookconfiguration-plans.yml.j2
@@ -0,0 +1,37 @@
+---
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingWebhookConfiguration
+metadata:
+  name: {{ api_deployment_name }}-plans
+  namespace: ""
+  annotations:
+{% if k8s_cluster|bool %}
+    cert-manager.io/inject-ca-from: {{ app_namespace }}/{{ api_certificate_name }}
+{% else %}
+    service.beta.openshift.io/inject-cabundle: "true"
+{% endif %}
+webhooks:
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: {{ api_service_name }}
+      namespace: {{ app_namespace }}
+      path: /plan-validate
+      port: 443
+  failurePolicy: Fail
+  matchPolicy: Equivalent
+  name: plans.forklift.konveyor
+  namespaceSelector: {}
+  objectSelector: {}
+  rules:
+  - apiGroups:
+    - forklift.konveyor.io
+    resources:
+    - plans
+    apiVersions:
+    - v1beta1
+    operations:
+    - CREATE
+    - UPDATE
+  sideEffects: None
diff --git a/operator/roles/forkliftcontroller/templates/api/validatingwebhookconfiguration-providers.yml.j2 b/operator/roles/forkliftcontroller/templates/api/validatingwebhookconfiguration-providers.yml.j2
new file mode 100644
index 000000000..351e0da49
--- /dev/null
+++ b/operator/roles/forkliftcontroller/templates/api/validatingwebhookconfiguration-providers.yml.j2
@@ -0,0 +1,37 @@
+---
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingWebhookConfiguration
+metadata:
+  name: {{ api_deployment_name }}-providers
+  namespace: ""
+  annotations:
+{% if k8s_cluster|bool %}
+    cert-manager.io/inject-ca-from: {{ app_namespace }}/{{ api_certificate_name }}
+{% else %}
+    service.beta.openshift.io/inject-cabundle: "true"
+{% endif %}
+webhooks:
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: {{ api_service_name }}
+      namespace: {{ app_namespace }}
+      path: /provider-validate
+      port: 443
+  failurePolicy: Fail
+  matchPolicy: Equivalent
+  name: providers.forklift.konveyor
+  namespaceSelector: {}
+  objectSelector: {}
+  rules:
+  - apiGroups:
+    - forklift.konveyor.io
+    resources:
+    - providers
+    apiVersions:
+    - v1beta1
+    operations:
+    - CREATE
+    - UPDATE
+  sideEffects: None
diff --git a/operator/roles/forkliftcontroller/templates/api/validatingwebhookconfiguration-secrets.yml.j2 b/operator/roles/forkliftcontroller/templates/api/validatingwebhookconfiguration-secrets.yml.j2
new file mode 100644
index 000000000..8f24080b0
--- /dev/null
+++ b/operator/roles/forkliftcontroller/templates/api/validatingwebhookconfiguration-secrets.yml.j2
@@ -0,0 +1,41 @@
+---
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingWebhookConfiguration
+metadata:
+  name: {{ api_deployment_name }}-secrets
+  namespace: ""
+  annotations:
+{% if k8s_cluster|bool %}
+    cert-manager.io/inject-ca-from: {{ app_namespace }}/{{ api_certificate_name }}
+{% else %}
+    service.beta.openshift.io/inject-cabundle: "true"
+{% endif %}
+webhooks:
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: {{ api_service_name }}
+      namespace: {{ app_namespace }}
+      path: /secret-validate
+      port: 443
+  failurePolicy: Fail
+  matchPolicy: Equivalent
+  name: secrets.forklift.konveyor
+  namespaceSelector: {}
+  objectSelector:
+    matchExpressions:
+    - key: createdForResourceType
+      operator: Exists
+  rules:
+  - apiGroups:
+    - ''
+    apiVersions:
+    - v1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - secrets
+    scope: Namespaced
+  sideEffects: None