diff --git a/.github/workflows/check-crd-webhook-coverage.yaml b/.github/workflows/check-crd-webhook-coverage.yaml new file mode 100644 index 0000000000..0d1d95cd39 --- /dev/null +++ b/.github/workflows/check-crd-webhook-coverage.yaml @@ -0,0 +1,78 @@ +name: Check CRD Webhook Coverage + +on: + pull_request: + paths: + - 'config/crd/bases/**' + - 'config/crd/patches/**' + - 'config/crd/kustomization.yaml' + +jobs: + check-webhook-patches: + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - uses: actions/checkout@v4 + with: + persist-credentials: false + + - name: Check all CRDs have conversion webhook patches + run: | + set -e + + echo "Checking that all openstack-operator CRDs have webhook + CA-injection patches..." + + MISSING_PATCHES=0 + + # Check each CRD in config/crd/bases/ + for crd_file in config/crd/bases/*.yaml; do + # Skip if not a file or if it's the commented-out openstacks.yaml + [[ ! -f "$crd_file" ]] && continue + basename=$(basename "$crd_file") + [[ "$basename" == "operator.openstack.org_openstacks.yaml" ]] && continue + + # Extract CRD name (e.g., "core.openstack.org_openstackcontrolplanes") + crd_name="${basename%.yaml}" + + # Check for webhook patch + webhook_patch="config/crd/patches/webhook_in_${crd_name}.yaml" + if [[ ! -f "$webhook_patch" ]]; then + echo "ERROR: Missing webhook patch for $basename" + echo " Expected: $webhook_patch" + MISSING_PATCHES=1 + fi + + # Check for CA injection patch + cainjection_patch="config/crd/patches/cainjection_in_${crd_name}.yaml" + if [[ ! -f "$cainjection_patch" ]]; then + echo "ERROR: Missing CA injection patch for $basename" + echo " Expected: $cainjection_patch" + MISSING_PATCHES=1 + fi + + # Check that patches are referenced in kustomization.yaml (and not commented out) + if ! grep -q "^- path: patches/webhook_in_${crd_name}.yaml" config/crd/kustomization.yaml; then + echo "ERROR: Webhook patch for $basename not referenced (or commented out) in kustomization.yaml" + MISSING_PATCHES=1 + fi + + if ! grep -q "^- path: patches/cainjection_in_${crd_name}.yaml" config/crd/kustomization.yaml; then + echo "ERROR: CA injection patch for $basename not referenced (or commented out) in kustomization.yaml" + MISSING_PATCHES=1 + fi + done + + if [[ $MISSING_PATCHES -eq 1 ]]; then + echo "" + echo "One or more CRDs are missing conversion webhook patches." + echo "For each CRD in config/crd/bases/, you must create:" + echo " 1. config/crd/patches/webhook_in_.yaml" + echo " 2. config/crd/patches/cainjection_in_.yaml" + echo " 3. Reference both in config/crd/kustomization.yaml" + echo "" + echo "See existing patches for examples." + exit 1 + fi + + echo "✓ All CRDs have conversion webhook patches" diff --git a/bindata/crds/crds.yaml b/bindata/crds/crds.yaml index 9953ed6ee9..eb5594b41b 100644 --- a/bindata/crds/crds.yaml +++ b/bindata/crds/crds.yaml @@ -2,9 +2,20 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: + cert-manager.io/inject-ca-from: '{{ .OperatorNamespace }}/openstack-operator-serving-cert' controller-gen.kubebuilder.io/version: v0.18.0 name: openstackassistants.assistant.openstack.org spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + name: openstack-operator-webhook-service + namespace: '{{ .OperatorNamespace }}' + path: /convert + conversionReviewVersions: + - v1 group: assistant.openstack.org names: kind: OpenStackAssistant @@ -471,6 +482,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: + cert-manager.io/inject-ca-from: '{{ .OperatorNamespace }}/openstack-operator-serving-cert' controller-gen.kubebuilder.io/version: v0.18.0 labels: backup.openstack.org/category: controlplane @@ -478,6 +490,16 @@ metadata: backup.openstack.org/restore-order: "20" name: openstackbackupconfigs.backup.openstack.org spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + name: openstack-operator-webhook-service + namespace: '{{ .OperatorNamespace }}' + path: /convert + conversionReviewVersions: + - v1 group: backup.openstack.org names: kind: OpenStackBackupConfig @@ -740,9 +762,20 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: + cert-manager.io/inject-ca-from: '{{ .OperatorNamespace }}/openstack-operator-serving-cert' controller-gen.kubebuilder.io/version: v0.18.0 name: openstackclients.client.openstack.org spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + name: openstack-operator-webhook-service + namespace: '{{ .OperatorNamespace }}' + path: /convert + conversionReviewVersions: + - v1 group: client.openstack.org names: kind: OpenStackClient @@ -1032,6 +1065,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: + cert-manager.io/inject-ca-from: '{{ .OperatorNamespace }}/openstack-operator-serving-cert' controller-gen.kubebuilder.io/version: v0.18.0 labels: backup.openstack.org/category: controlplane @@ -1039,6 +1073,16 @@ metadata: backup.openstack.org/restore-order: "30" name: openstackcontrolplanes.core.openstack.org spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + name: openstack-operator-webhook-service + namespace: '{{ .OperatorNamespace }}' + path: /convert + conversionReviewVersions: + - v1 group: core.openstack.org names: kind: OpenStackControlPlane @@ -20630,9 +20674,20 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: + cert-manager.io/inject-ca-from: '{{ .OperatorNamespace }}/openstack-operator-serving-cert' controller-gen.kubebuilder.io/version: v0.18.0 name: openstackdataplanedeployments.dataplane.openstack.org spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + name: openstack-operator-webhook-service + namespace: '{{ .OperatorNamespace }}' + path: /convert + conversionReviewVersions: + - v1 group: dataplane.openstack.org names: kind: OpenStackDataPlaneDeployment @@ -20938,6 +20993,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: + cert-manager.io/inject-ca-from: '{{ .OperatorNamespace }}/openstack-operator-serving-cert' controller-gen.kubebuilder.io/version: v0.18.0 labels: backup.openstack.org/category: dataplane @@ -20945,6 +21001,16 @@ metadata: backup.openstack.org/restore-order: "60" name: openstackdataplanenodesets.dataplane.openstack.org spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + name: openstack-operator-webhook-service + namespace: '{{ .OperatorNamespace }}' + path: /convert + conversionReviewVersions: + - v1 group: dataplane.openstack.org names: kind: OpenStackDataPlaneNodeSet @@ -22585,6 +22651,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: + cert-manager.io/inject-ca-from: '{{ .OperatorNamespace }}/openstack-operator-serving-cert' controller-gen.kubebuilder.io/version: v0.18.0 labels: backup.openstack.org/category: dataplane @@ -22592,6 +22659,16 @@ metadata: backup.openstack.org/restore-order: "40" name: openstackdataplaneservices.dataplane.openstack.org spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + name: openstack-operator-webhook-service + namespace: '{{ .OperatorNamespace }}' + path: /convert + conversionReviewVersions: + - v1 group: dataplane.openstack.org names: kind: OpenStackDataPlaneService @@ -22897,6 +22974,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: + cert-manager.io/inject-ca-from: '{{ .OperatorNamespace }}/openstack-operator-serving-cert' controller-gen.kubebuilder.io/version: v0.18.0 labels: backup.openstack.org/category: controlplane @@ -22904,6 +22982,16 @@ metadata: backup.openstack.org/restore-order: "20" name: openstackversions.core.openstack.org spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + name: openstack-operator-webhook-service + namespace: '{{ .OperatorNamespace }}' + path: /convert + conversionReviewVersions: + - v1 group: core.openstack.org names: kind: OpenStackVersion diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 60bb20ef18..adf0adbaad 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -14,11 +14,27 @@ resources: # +kubebuilder:scaffold:crdkustomizeresource patches: -# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix. -# patches here are for enabling the conversion webhook for each CRD +# [WEBHOOK] patches for enabling the conversion webhook for each CRD +- path: patches/webhook_in_core.openstack.org_openstackcontrolplanes.yaml +- path: patches/webhook_in_dataplane.openstack.org_openstackdataplanenodesets.yaml +- path: patches/webhook_in_dataplane.openstack.org_openstackdataplaneservices.yaml +- path: patches/webhook_in_dataplane.openstack.org_openstackdataplanedeployments.yaml +- path: patches/webhook_in_core.openstack.org_openstackversions.yaml +- path: patches/webhook_in_client.openstack.org_openstackclients.yaml +- path: patches/webhook_in_backup.openstack.org_openstackbackupconfigs.yaml +- path: patches/webhook_in_assistant.openstack.org_openstackassistants.yaml # +kubebuilder:scaffold:crdkustomizewebhookpatch -# [WEBHOOK] To enable webhook, uncomment the following section -# the following config is for teaching kustomize how to do kustomization for CRDs. -#configurations: -#- kustomizeconfig.yaml +# [CERTMANAGER] patches for enabling the CA injection for each CRD +- path: patches/cainjection_in_core.openstack.org_openstackcontrolplanes.yaml +- path: patches/cainjection_in_dataplane.openstack.org_openstackdataplanenodesets.yaml +- path: patches/cainjection_in_dataplane.openstack.org_openstackdataplaneservices.yaml +- path: patches/cainjection_in_dataplane.openstack.org_openstackdataplanedeployments.yaml +- path: patches/cainjection_in_core.openstack.org_openstackversions.yaml +- path: patches/cainjection_in_client.openstack.org_openstackclients.yaml +- path: patches/cainjection_in_backup.openstack.org_openstackbackupconfigs.yaml +- path: patches/cainjection_in_assistant.openstack.org_openstackassistants.yaml +# +kubebuilder:scaffold:crdkustomizecainjectionpatch + +configurations: +- kustomizeconfig.yaml diff --git a/config/crd/patches/cainjection_in_assistant.openstack.org_openstackassistants.yaml b/config/crd/patches/cainjection_in_assistant.openstack.org_openstackassistants.yaml new file mode 100644 index 0000000000..a815d3d090 --- /dev/null +++ b/config/crd/patches/cainjection_in_assistant.openstack.org_openstackassistants.yaml @@ -0,0 +1,8 @@ +# Adds the cert-manager CA-injection annotation to the openstackassistants.assistant.openstack.org CRD so +# the conversion webhook's clientConfig.caBundle is populated at deploy time. +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: openstackassistants.assistant.openstack.org + annotations: + cert-manager.io/inject-ca-from: '{{ .OperatorNamespace }}/openstack-operator-serving-cert' diff --git a/config/crd/patches/cainjection_in_backup.openstack.org_openstackbackupconfigs.yaml b/config/crd/patches/cainjection_in_backup.openstack.org_openstackbackupconfigs.yaml new file mode 100644 index 0000000000..d27499c5b9 --- /dev/null +++ b/config/crd/patches/cainjection_in_backup.openstack.org_openstackbackupconfigs.yaml @@ -0,0 +1,8 @@ +# Adds the cert-manager CA-injection annotation to the openstackbackupconfigs.backup.openstack.org CRD so +# the conversion webhook's clientConfig.caBundle is populated at deploy time. +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: openstackbackupconfigs.backup.openstack.org + annotations: + cert-manager.io/inject-ca-from: '{{ .OperatorNamespace }}/openstack-operator-serving-cert' diff --git a/config/crd/patches/cainjection_in_client.openstack.org_openstackclients.yaml b/config/crd/patches/cainjection_in_client.openstack.org_openstackclients.yaml new file mode 100644 index 0000000000..1558c5cf89 --- /dev/null +++ b/config/crd/patches/cainjection_in_client.openstack.org_openstackclients.yaml @@ -0,0 +1,8 @@ +# Adds the cert-manager CA-injection annotation to the openstackclients.client.openstack.org CRD so +# the conversion webhook's clientConfig.caBundle is populated at deploy time. +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: openstackclients.client.openstack.org + annotations: + cert-manager.io/inject-ca-from: '{{ .OperatorNamespace }}/openstack-operator-serving-cert' diff --git a/config/crd/patches/cainjection_in_core.openstack.org_openstackcontrolplanes.yaml b/config/crd/patches/cainjection_in_core.openstack.org_openstackcontrolplanes.yaml new file mode 100644 index 0000000000..28025ada21 --- /dev/null +++ b/config/crd/patches/cainjection_in_core.openstack.org_openstackcontrolplanes.yaml @@ -0,0 +1,8 @@ +# Adds the cert-manager CA-injection annotation to the openstackcontrolplanes.core.openstack.org CRD so +# the conversion webhook's clientConfig.caBundle is populated at deploy time. +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: openstackcontrolplanes.core.openstack.org + annotations: + cert-manager.io/inject-ca-from: '{{ .OperatorNamespace }}/openstack-operator-serving-cert' diff --git a/config/crd/patches/cainjection_in_core.openstack.org_openstackversions.yaml b/config/crd/patches/cainjection_in_core.openstack.org_openstackversions.yaml new file mode 100644 index 0000000000..6feaceb461 --- /dev/null +++ b/config/crd/patches/cainjection_in_core.openstack.org_openstackversions.yaml @@ -0,0 +1,8 @@ +# Adds the cert-manager CA-injection annotation to the openstackversions.core.openstack.org CRD so +# the conversion webhook's clientConfig.caBundle is populated at deploy time. +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: openstackversions.core.openstack.org + annotations: + cert-manager.io/inject-ca-from: '{{ .OperatorNamespace }}/openstack-operator-serving-cert' diff --git a/config/crd/patches/cainjection_in_dataplane.openstack.org_openstackdataplanedeployments.yaml b/config/crd/patches/cainjection_in_dataplane.openstack.org_openstackdataplanedeployments.yaml new file mode 100644 index 0000000000..4b0b6f87ff --- /dev/null +++ b/config/crd/patches/cainjection_in_dataplane.openstack.org_openstackdataplanedeployments.yaml @@ -0,0 +1,8 @@ +# Adds the cert-manager CA-injection annotation to the openstackdataplanedeployments.dataplane.openstack.org CRD so +# the conversion webhook's clientConfig.caBundle is populated at deploy time. +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: openstackdataplanedeployments.dataplane.openstack.org + annotations: + cert-manager.io/inject-ca-from: '{{ .OperatorNamespace }}/openstack-operator-serving-cert' diff --git a/config/crd/patches/cainjection_in_dataplane.openstack.org_openstackdataplanenodesets.yaml b/config/crd/patches/cainjection_in_dataplane.openstack.org_openstackdataplanenodesets.yaml new file mode 100644 index 0000000000..0259bfab6f --- /dev/null +++ b/config/crd/patches/cainjection_in_dataplane.openstack.org_openstackdataplanenodesets.yaml @@ -0,0 +1,8 @@ +# Adds the cert-manager CA-injection annotation to the openstackdataplanenodesets.dataplane.openstack.org CRD so +# the conversion webhook's clientConfig.caBundle is populated at deploy time. +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: openstackdataplanenodesets.dataplane.openstack.org + annotations: + cert-manager.io/inject-ca-from: '{{ .OperatorNamespace }}/openstack-operator-serving-cert' diff --git a/config/crd/patches/cainjection_in_dataplane.openstack.org_openstackdataplaneservices.yaml b/config/crd/patches/cainjection_in_dataplane.openstack.org_openstackdataplaneservices.yaml new file mode 100644 index 0000000000..6d5c4f30a0 --- /dev/null +++ b/config/crd/patches/cainjection_in_dataplane.openstack.org_openstackdataplaneservices.yaml @@ -0,0 +1,8 @@ +# Adds the cert-manager CA-injection annotation to the openstackdataplaneservices.dataplane.openstack.org CRD so +# the conversion webhook's clientConfig.caBundle is populated at deploy time. +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: openstackdataplaneservices.dataplane.openstack.org + annotations: + cert-manager.io/inject-ca-from: '{{ .OperatorNamespace }}/openstack-operator-serving-cert' diff --git a/config/crd/patches/webhook_in_assistant.openstack.org_openstackassistants.yaml b/config/crd/patches/webhook_in_assistant.openstack.org_openstackassistants.yaml new file mode 100644 index 0000000000..8ee18bf4ce --- /dev/null +++ b/config/crd/patches/webhook_in_assistant.openstack.org_openstackassistants.yaml @@ -0,0 +1,18 @@ +# Enables the conversion webhook for the openstackassistants.assistant.openstack.org CRD. +# Stays dormant while the CRD has a single served version; activates automatically +# once a second version is added. Namespace is resolved at deploy time. +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: openstackassistants.assistant.openstack.org +spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + namespace: '{{ .OperatorNamespace }}' + name: openstack-operator-webhook-service + path: /convert + conversionReviewVersions: + - v1 diff --git a/config/crd/patches/webhook_in_backup.openstack.org_openstackbackupconfigs.yaml b/config/crd/patches/webhook_in_backup.openstack.org_openstackbackupconfigs.yaml new file mode 100644 index 0000000000..abcc0531dd --- /dev/null +++ b/config/crd/patches/webhook_in_backup.openstack.org_openstackbackupconfigs.yaml @@ -0,0 +1,18 @@ +# Enables the conversion webhook for the openstackbackupconfigs.backup.openstack.org CRD. +# Stays dormant while the CRD has a single served version; activates automatically +# once a second version is added. Namespace is resolved at deploy time. +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: openstackbackupconfigs.backup.openstack.org +spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + namespace: '{{ .OperatorNamespace }}' + name: openstack-operator-webhook-service + path: /convert + conversionReviewVersions: + - v1 diff --git a/config/crd/patches/webhook_in_client.openstack.org_openstackclients.yaml b/config/crd/patches/webhook_in_client.openstack.org_openstackclients.yaml new file mode 100644 index 0000000000..dc3c5c2ca2 --- /dev/null +++ b/config/crd/patches/webhook_in_client.openstack.org_openstackclients.yaml @@ -0,0 +1,18 @@ +# Enables the conversion webhook for the openstackclients.client.openstack.org CRD. +# Stays dormant while the CRD has a single served version; activates automatically +# once a second version is added. Namespace is resolved at deploy time. +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: openstackclients.client.openstack.org +spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + namespace: '{{ .OperatorNamespace }}' + name: openstack-operator-webhook-service + path: /convert + conversionReviewVersions: + - v1 diff --git a/config/crd/patches/webhook_in_core.openstack.org_openstackcontrolplanes.yaml b/config/crd/patches/webhook_in_core.openstack.org_openstackcontrolplanes.yaml new file mode 100644 index 0000000000..8e8d6f6f8a --- /dev/null +++ b/config/crd/patches/webhook_in_core.openstack.org_openstackcontrolplanes.yaml @@ -0,0 +1,18 @@ +# Enables the conversion webhook for the openstackcontrolplanes.core.openstack.org CRD. +# Stays dormant while the CRD has a single served version; activates automatically +# once a second version is added. Namespace is resolved at deploy time. +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: openstackcontrolplanes.core.openstack.org +spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + namespace: '{{ .OperatorNamespace }}' + name: openstack-operator-webhook-service + path: /convert + conversionReviewVersions: + - v1 diff --git a/config/crd/patches/webhook_in_core.openstack.org_openstackversions.yaml b/config/crd/patches/webhook_in_core.openstack.org_openstackversions.yaml new file mode 100644 index 0000000000..367717badd --- /dev/null +++ b/config/crd/patches/webhook_in_core.openstack.org_openstackversions.yaml @@ -0,0 +1,18 @@ +# Enables the conversion webhook for the openstackversions.core.openstack.org CRD. +# Stays dormant while the CRD has a single served version; activates automatically +# once a second version is added. Namespace is resolved at deploy time. +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: openstackversions.core.openstack.org +spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + namespace: '{{ .OperatorNamespace }}' + name: openstack-operator-webhook-service + path: /convert + conversionReviewVersions: + - v1 diff --git a/config/crd/patches/webhook_in_dataplane.openstack.org_openstackdataplanedeployments.yaml b/config/crd/patches/webhook_in_dataplane.openstack.org_openstackdataplanedeployments.yaml new file mode 100644 index 0000000000..e7d4d9fbba --- /dev/null +++ b/config/crd/patches/webhook_in_dataplane.openstack.org_openstackdataplanedeployments.yaml @@ -0,0 +1,18 @@ +# Enables the conversion webhook for the openstackdataplanedeployments.dataplane.openstack.org CRD. +# Stays dormant while the CRD has a single served version; activates automatically +# once a second version is added. Namespace is resolved at deploy time. +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: openstackdataplanedeployments.dataplane.openstack.org +spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + namespace: '{{ .OperatorNamespace }}' + name: openstack-operator-webhook-service + path: /convert + conversionReviewVersions: + - v1 diff --git a/config/crd/patches/webhook_in_dataplane.openstack.org_openstackdataplanenodesets.yaml b/config/crd/patches/webhook_in_dataplane.openstack.org_openstackdataplanenodesets.yaml new file mode 100644 index 0000000000..eaf43f7840 --- /dev/null +++ b/config/crd/patches/webhook_in_dataplane.openstack.org_openstackdataplanenodesets.yaml @@ -0,0 +1,18 @@ +# Enables the conversion webhook for the openstackdataplanenodesets.dataplane.openstack.org CRD. +# Stays dormant while the CRD has a single served version; activates automatically +# once a second version is added. Namespace is resolved at deploy time. +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: openstackdataplanenodesets.dataplane.openstack.org +spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + namespace: '{{ .OperatorNamespace }}' + name: openstack-operator-webhook-service + path: /convert + conversionReviewVersions: + - v1 diff --git a/config/crd/patches/webhook_in_dataplane.openstack.org_openstackdataplaneservices.yaml b/config/crd/patches/webhook_in_dataplane.openstack.org_openstackdataplaneservices.yaml new file mode 100644 index 0000000000..534bf57f36 --- /dev/null +++ b/config/crd/patches/webhook_in_dataplane.openstack.org_openstackdataplaneservices.yaml @@ -0,0 +1,18 @@ +# Enables the conversion webhook for the openstackdataplaneservices.dataplane.openstack.org CRD. +# Stays dormant while the CRD has a single served version; activates automatically +# once a second version is added. Namespace is resolved at deploy time. +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: openstackdataplaneservices.dataplane.openstack.org +spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + namespace: '{{ .OperatorNamespace }}' + name: openstack-operator-webhook-service + path: /convert + conversionReviewVersions: + - v1 diff --git a/hack/sync-bindata.sh b/hack/sync-bindata.sh index 91377700b1..5b6961247b 100755 --- a/hack/sync-bindata.sh +++ b/hack/sync-bindata.sh @@ -44,12 +44,8 @@ cat $CSV_FILENAME | $LOCAL_BINARIES/yq -r ".spec.webhookdefinitions.[] | select( } -function write_webhooks { -local CSV_FILENAME=$1 -local OPERATOR_NAME=$2 - -MUTATING_WEBHOOKS=$(extract_webhooks "$CSV_FILENAME" "$OPERATOR_NAME" "MutatingAdmissionWebhook") -VALIDATING_WEBHOOKS=$(extract_webhooks "$CSV_FILENAME" "$OPERATOR_NAME" "ValidatingAdmissionWebhook") +function write_webhook_serving_resources { +local OPERATOR_NAME=$1 cat > operator/$OPERATOR_NAME-webhooks.yaml <> operator/$OPERATOR_NAME-webhooks.yaml <&2 + return 2 +fi +while IFS= read -r CRD_FULLNAME; do + [[ -z "$CRD_FULLNAME" ]] && continue + local RESOURCE="${CRD_FULLNAME%%.*}" + local GROUP="${CRD_FULLNAME#*.}" + local CRD_FILE="manifests/${GROUP}_${RESOURCE}.yaml" + if [[ -f "$CRD_FILE" ]]; then + local STRATEGY + if ! STRATEGY=$(cat "$CRD_FILE" | $LOCAL_BINARIES/yq -r '.spec.conversion.strategy // ""'); then + echo "ERROR: csv_has_conversion_crd: failed to parse conversion strategy from $CRD_FILE" >&2 + return 2 + fi + [[ "$STRATEGY" == "Webhook" ]] && return 0 + else + echo "WARNING: csv_has_conversion_crd: CRD file not found: $CRD_FILE (owned by $CSV_FILE)" >&2 + fi +done <<< "$CRD_LIST" +return 1 +} + if ! BUNDLES="$(hack/pin-bundle-images.sh)"; then exit 1 fi @@ -168,6 +207,17 @@ for X in $(ls manifests/*clusterserviceversion.yaml); do if [[ "$OPERATOR_NAME" == "infra-operator" || "$OPERATOR_NAME" == "openstack-baremetal-operator" ]]; then write_webhooks "$X" "$OPERATOR_NAME" +else + HAS_CONVERSION=0 + csv_has_conversion_crd "$X" || HAS_CONVERSION=$? + if [[ $HAS_CONVERSION -eq 2 ]]; then + echo "FATAL: parse error checking conversion webhook for $OPERATOR_NAME" >&2 + exit 1 + elif [[ $HAS_CONVERSION -eq 0 ]]; then + # Conversion operator: stage only the serving Service + Certificate. + # Admission webhook configs are not registered (CRs managed by OSCP upstream webhook, not direct users). + write_webhook_serving_resources "$OPERATOR_NAME" + fi fi # cert-manager and Service resources go into bindata/services/ for runtime diff --git a/internal/controller/operator/crd_namespace_rewrite_test.go b/internal/controller/operator/crd_namespace_rewrite_test.go new file mode 100644 index 0000000000..3913bc8c7a --- /dev/null +++ b/internal/controller/operator/crd_namespace_rewrite_test.go @@ -0,0 +1,149 @@ +/* +Copyright 2024. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package operator + +import ( + "testing" + + uns "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +func makeKeystoneCRD(annotationNS, serviceNS string) *uns.Unstructured { + obj := &uns.Unstructured{Object: map[string]interface{}{ + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": map[string]interface{}{ + "name": "keystoneapis.keystone.openstack.org", + "annotations": map[string]interface{}{ + "cert-manager.io/inject-ca-from": annotationNS + "/keystone-operator-serving-cert", + }, + }, + "spec": map[string]interface{}{ + "conversion": map[string]interface{}{ + "strategy": "Webhook", + "webhook": map[string]interface{}{ + "clientConfig": map[string]interface{}{ + "service": map[string]interface{}{ + "name": "keystone-operator-webhook-service", + "namespace": serviceNS, + "path": "/convert", + }, + }, + }, + }, + }, + }} + obj.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "apiextensions.k8s.io", Version: "v1", Kind: "CustomResourceDefinition", + }) + return obj +} + +func TestRewriteCRDNamespaces_RewritesBothFields(t *testing.T) { + obj := makeKeystoneCRD("keystone-operator-system", "keystone-operator-system") + if err := rewriteCRDNamespaces(obj, "openstack-operators"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + annos := obj.GetAnnotations() + got := annos["cert-manager.io/inject-ca-from"] + want := "openstack-operators/keystone-operator-serving-cert" + if got != want { + t.Errorf("inject-ca-from: got %q, want %q", got, want) + } + + svcNs, _, _ := uns.NestedString(obj.Object, "spec", "conversion", "webhook", "clientConfig", "service", "namespace") + if svcNs != "openstack-operators" { + t.Errorf("service.namespace: got %q, want %q", svcNs, "openstack-operators") + } +} + +func TestRewriteCRDNamespaces_AlreadyCorrect(t *testing.T) { + obj := makeKeystoneCRD("openstack-operators", "openstack-operators") + if err := rewriteCRDNamespaces(obj, "openstack-operators"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + annos := obj.GetAnnotations() + if got := annos["cert-manager.io/inject-ca-from"]; got != "openstack-operators/keystone-operator-serving-cert" { + t.Errorf("annotation changed unexpectedly: %q", got) + } +} + +func TestRewriteCRDNamespaces_AnnotationOnlyNoConversionSection(t *testing.T) { + // CRD has the inject-ca-from annotation but no spec.conversion.webhook.clientConfig.service + // (e.g. a CRD that uses cainjection for a non-conversion purpose). Service rewrite is a no-op. + obj := &uns.Unstructured{Object: map[string]interface{}{ + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": map[string]interface{}{ + "name": "somecrd.example.org", + "annotations": map[string]interface{}{ + "cert-manager.io/inject-ca-from": "source-ns/some-cert", + }, + }, + "spec": map[string]interface{}{}, + }} + obj.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "apiextensions.k8s.io", Version: "v1", Kind: "CustomResourceDefinition", + }) + if err := rewriteCRDNamespaces(obj, "openstack-operators"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + annos := obj.GetAnnotations() + if got := annos["cert-manager.io/inject-ca-from"]; got != "openstack-operators/some-cert" { + t.Errorf("inject-ca-from not rewritten: got %q", got) + } +} + +func TestRewriteCRDNamespaces_NoAnnotation(t *testing.T) { + obj := &uns.Unstructured{Object: map[string]interface{}{ + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": map[string]interface{}{"name": "no-annotation.example.org"}, + "spec": map[string]interface{}{}, + }} + obj.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "apiextensions.k8s.io", Version: "v1", Kind: "CustomResourceDefinition", + }) + if err := rewriteCRDNamespaces(obj, "openstack-operators"); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestRewriteCRDNamespaces_SkipsNonCRD(t *testing.T) { + obj := &uns.Unstructured{Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "test", + "annotations": map[string]interface{}{ + "cert-manager.io/inject-ca-from": "wrong-ns/some-cert", + }, + }, + }} + obj.SetGroupVersionKind(schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}) + if err := rewriteCRDNamespaces(obj, "openstack-operators"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Non-CRD: annotation must NOT be touched + if got := obj.GetAnnotations()["cert-manager.io/inject-ca-from"]; got != "wrong-ns/some-cert" { + t.Errorf("non-CRD annotation was rewritten: %q", got) + } +} diff --git a/internal/controller/operator/openstack_controller.go b/internal/controller/operator/openstack_controller.go index 6c0e1297f9..f59a296d4b 100644 --- a/internal/controller/operator/openstack_controller.go +++ b/internal/controller/operator/openstack_controller.go @@ -59,6 +59,11 @@ type OpenStackReconciler struct { client.Client Scheme *runtime.Scheme Kclient kubernetes.Interface + // webhookOperatorSet maps short op names with a bindata webhook manifest (populated once + // at startup) to whether that manifest registers an always-active admission webhook, as + // opposed to carrying only the serving Service+Certificate for a dormant CRD conversion + // webhook. See loadWebhookOperatorSet. + webhookOperatorSet map[string]bool } // GetLogger returns a logger object with a prefix of "controller.name" and additional controller context fields @@ -550,21 +555,123 @@ func containerImageMatch(instance *operatorv1beta1.OpenStack) bool { return false } -func isWebhookEndpoint(name string) bool { - // NOTE: this is a static list for all operators with webhooks enabled - endpointNames := []string{"openstack-operator-webhook-service", "infra-operator-webhook-service", "openstack-baremetal-operator-webhook-service"} - for _, prefix := range endpointNames { - if strings.HasPrefix(name, prefix) { - return true +// admissionWebhookKinds are the manifest kinds that only appear in a bindata webhook +// manifest when hack/sync-bindata.sh ran write_webhooks (always-active admission +// webhooks), as opposed to write_webhook_serving_resources (dormant CRD conversion +// webhook: just the serving Service + Certificate). +var admissionWebhookKinds = []string{"kind: MutatingWebhookConfiguration", "kind: ValidatingWebhookConfiguration"} + +// loadWebhookOperatorSet globs bindata/operator/*-webhooks.yaml once at startup and returns +// a map of short operator names (e.g. "keystone", "infra") that have a webhook manifest to +// whether that manifest registers an always-active admission webhook (true) or only the +// serving resources for a dormant CRD conversion webhook (false). +// Returns empty set on errors (safe fallback: no dynamic webhook operators configured). +func loadWebhookOperatorSet(ctx context.Context, bindir string) map[string]bool { + log := ctrl.LoggerFrom(ctx) + result := map[string]bool{} + operatorDir := filepath.Join(bindir, "operator") + + // Validate bindata directory exists + if _, err := os.Stat(operatorDir); os.IsNotExist(err) { + log.Info("bindata operator directory not found, no dynamic webhook operators will be configured", "dir", operatorDir) + return result + } else if err != nil { + log.Error(err, "failed to check bindata operator directory", "dir", operatorDir) + return result + } + + // Glob webhook manifests + files, err := filepath.Glob(filepath.Join(operatorDir, "*-webhooks.yaml")) + if err != nil { + log.Error(err, "failed to glob webhook manifests", "dir", operatorDir) + return result + } + + if len(files) == 0 { + log.Info("no webhook manifests found in bindata", "dir", operatorDir) + return result + } + + for _, f := range files { + base := filepath.Base(f) + opName := strings.TrimSuffix(base, "-webhooks.yaml") // e.g. "keystone-operator" + shortName := strings.TrimSuffix(opName, "-operator") // e.g. "keystone" + + content, err := os.ReadFile(f) + if err != nil { + // Fail closed: treat an unreadable manifest as blocking rather than silently + // downgrading a possible admission webhook to non-blocking. Worst case this + // costs an extra requeue; the opposite risks serving admission requests + // against a webhook nobody is checking. + log.Error(err, "failed to read webhook manifest, treating as blocking admission webhook", "file", f) + result[shortName] = true + continue } + hasAdmissionWebhook := false + for _, kind := range admissionWebhookKinds { + if strings.Contains(string(content), kind) { + hasAdmissionWebhook = true + break + } + } + result[shortName] = hasAdmissionWebhook } - return false + log.Info("loaded webhook operator set", "count", len(result), "operators", result) + return result +} + +// webhookServiceNames returns the expected webhook endpoint service names derived from +// the operator's cached webhook set, plus the openstack-operator itself. +func (r *OpenStackReconciler) webhookServiceNames() []string { + names := []string{"openstack-operator-webhook-service"} + for shortName := range r.webhookOperatorSet { + names = append(names, shortName+"-operator-webhook-service") + } + return names +} + +// isBlockingWebhookService reports whether svc backs an always-active admission webhook +// (openstack-operator's own, or a service discovered via webhookOperatorSet with an +// admission webhook manifest), as opposed to a service-operator CRD conversion webhook, +// which stays dormant until a v1beta2 version is added. Blocking services must be ready +// before OpenStack can be Ready, since requests against their admission webhooks would +// otherwise fail. +func (r *OpenStackReconciler) isBlockingWebhookService(svc string) bool { + if svc == "openstack-operator-webhook-service" { + return true + } + shortName := strings.TrimSuffix(svc, "-operator-webhook-service") + return r.webhookOperatorSet[shortName] +} + +func matchWebhookService(endpointName string, serviceNames []string) (string, bool) { + for _, svc := range serviceNames { + if strings.HasPrefix(endpointName, svc) { + return svc, true + } + } + return "", false } // checkServiceEndpoints - func (r *OpenStackReconciler) checkServiceEndpoints(ctx context.Context, instance *operatorv1beta1.OpenStack) (ctrl.Result, error) { Log := r.GetLogger(ctx) + expectedWebhookSvcs := r.webhookServiceNames() + validatedSvcs := map[string]bool{} + + // Pre-filter disabled services to avoid requeuing when they have no EndpointSlice + for _, svc := range expectedWebhookSvcs { + for _, op := range instance.Spec.OperatorOverrides { + if svc == op.Name+"-operator-webhook-service" && + op.Replicas != nil && *op.Replicas == 0 { + Log.Info("Webhook service disabled, skipping endpoint check", "service", svc) + validatedSvcs[svc] = true + break + } + } + } + endpointSliceList := &discoveryv1.EndpointSliceList{} err := r.List(ctx, endpointSliceList, &client.ListOptions{Namespace: instance.Namespace}) if err != nil { @@ -578,37 +685,67 @@ func (r *OpenStackReconciler) checkServiceEndpoints(ctx context.Context, instanc for _, endpointSlice := range endpointSliceList.Items { endpointSliceName := endpointSlice.GetName() - if isWebhookEndpoint(endpointSliceName) { - // is deployment disabled ? - disabled := false - for _, op := range instance.Spec.OperatorOverrides { - if strings.HasPrefix(endpointSliceName, op.Name+"-operator") && - op.Replicas != nil && *op.Replicas == 0 { - - disabled = true - Log.Info(fmt.Sprintf("Webhook %s disabled, skipping endpoint slice check", endpointSliceName), "name", endpointSlice.GetName()) - break - } - } - if disabled { + if matchedSvc, ok := matchWebhookService(endpointSliceName, expectedWebhookSvcs); ok { + // Skip if already validated (e.g., disabled) + if validatedSvcs[matchedSvc] { continue } + isBlocking := r.isBlockingWebhookService(matchedSvc) + if len(endpointSlice.Endpoints) == 0 { - Log.Info("Webhook endpoint not configured. Requeuing...", "name", endpointSlice.GetName()) - return ctrl.Result{RequeueAfter: time.Duration(5) * time.Second}, nil + if isBlocking { + Log.Info("Webhook endpoint not configured. Requeuing...", "name", endpointSlice.GetName()) + return ctrl.Result{RequeueAfter: time.Duration(5) * time.Second}, nil + } + Log.Info("Service operator webhook endpoint not configured (non-blocking)", "name", endpointSlice.GetName()) + continue } for _, endpoint := range endpointSlice.Endpoints { if len(endpoint.Addresses) == 0 { - Log.Info("Webhook endpoint addresses aren't healthy. Requeuing...", "name", endpointSlice.GetName()) - return ctrl.Result{RequeueAfter: time.Duration(5) * time.Second}, nil + if isBlocking { + Log.Info("Webhook endpoint addresses aren't healthy. Requeuing...", "name", endpointSlice.GetName()) + return ctrl.Result{RequeueAfter: time.Duration(5) * time.Second}, nil + } + Log.Info("Service operator webhook endpoint addresses aren't healthy (non-blocking)", "name", endpointSlice.GetName()) + continue } - bFalse := false - if endpoint.Conditions.Ready == &bFalse || endpoint.Conditions.Serving == &bFalse { - Log.Info("Webhook endpoint addresses aren't serving. Requeuing...", "name", endpointSlice.GetName()) - return ctrl.Result{RequeueAfter: time.Duration(5) * time.Second}, nil + if endpoint.Conditions.Ready != nil && !*endpoint.Conditions.Ready { + if isBlocking { + Log.Info("Webhook endpoint not ready. Requeuing...", "name", endpointSlice.GetName()) + return ctrl.Result{RequeueAfter: time.Duration(5) * time.Second}, nil + } + Log.Info("Service operator webhook endpoint not ready (non-blocking)", "name", endpointSlice.GetName()) + continue + } + if endpoint.Conditions.Serving != nil && !*endpoint.Conditions.Serving { + if isBlocking { + Log.Info("Webhook endpoint not serving. Requeuing...", "name", endpointSlice.GetName()) + return ctrl.Result{RequeueAfter: time.Duration(5) * time.Second}, nil + } + Log.Info("Service operator webhook endpoint not serving (non-blocking)", "name", endpointSlice.GetName()) + continue } } + validatedSvcs[matchedSvc] = true + } + } + + // Ensure webhook services backing always-active admission webhooks are validated + // (hard requirement). Service operator conversion webhooks are optional - they're + // dormant until v1beta2 is added, and a misconfigured one shouldn't block the + // entire OpenStack deployment. + for _, svc := range expectedWebhookSvcs { + if r.isBlockingWebhookService(svc) && !validatedSvcs[svc] { + Log.Info("Webhook endpoint not found. Requeuing...", "service", svc) + return ctrl.Result{RequeueAfter: time.Duration(5) * time.Second}, nil + } + } + + // Warn about missing service operator webhooks but don't block + for _, svc := range expectedWebhookSvcs { + if !r.isBlockingWebhookService(svc) && !validatedSvcs[svc] { + Log.Info("Service operator webhook endpoint not found (non-blocking - dormant until v1beta2)", "service", svc) } } @@ -641,7 +778,61 @@ func (r *OpenStackReconciler) applyManifests(ctx context.Context, instance *oper func (r *OpenStackReconciler) applyCRDs(ctx context.Context, instance *operatorv1beta1.OpenStack) error { data := bindata.MakeRenderData() - return r.renderAndApply(ctx, instance, data, "crds", false) + data.Data["OperatorNamespace"] = instance.Namespace + + bindir := util.GetEnvVar("BASE_BINDATA", "/bindata") + objs, err := bindata.RenderDir(filepath.Join(bindir, "crds"), &data) + if err != nil { + return errors.Wrapf(err, "failed to render openstack-operator - crds") + } + if len(objs) == 0 { + return fmt.Errorf("no manifests rendered from %s/crds", bindir) + } + for _, obj := range objs { + if obj.GetName() == "" { + continue + } + if err := rewriteCRDNamespaces(obj, instance.Namespace); err != nil { + return err + } + if err := bindata.ApplyObject(ctx, r.Client, obj); err != nil { + return errors.Wrapf(err, "failed to apply object %v", obj) + } + } + return nil +} + +// rewriteCRDNamespaces rewrites namespace-sensitive fields on CRDs at install time: +// - cert-manager.io/inject-ca-from annotation ("/" → "/") +// - spec.conversion.webhook.clientConfig.service.namespace +// +// CRDs shipped in operator bundles encode the operator's own install namespace; +// openstack-operator installs into a different namespace, so both must be rewritten. +func rewriteCRDNamespaces(obj *uns.Unstructured, ns string) error { + gvk := obj.GroupVersionKind() + if gvk.Group != "apiextensions.k8s.io" || gvk.Kind != "CustomResourceDefinition" { + return nil + } + const caInjectAnno = "cert-manager.io/inject-ca-from" + annotations := obj.GetAnnotations() + if v, ok := annotations[caInjectAnno]; ok { + if idx := strings.LastIndex(v, "/"); idx >= 0 { + annotations[caInjectAnno] = ns + v[idx:] + obj.SetAnnotations(annotations) + } + } + svcNs, found, err := uns.NestedString(obj.Object, + "spec", "conversion", "webhook", "clientConfig", "service", "namespace") + if err != nil { + return fmt.Errorf("failed to read service namespace on CRD %s: %w", obj.GetName(), err) + } + if found && svcNs != "" { + if err := uns.SetNestedField(obj.Object, ns, + "spec", "conversion", "webhook", "clientConfig", "service", "namespace"); err != nil { + return fmt.Errorf("failed to rewrite service namespace on CRD %s: %w", obj.GetName(), err) + } + } + return nil } func (r *OpenStackReconciler) applyServices(ctx context.Context, instance *operatorv1beta1.OpenStack) error { @@ -725,30 +916,25 @@ func (r *OpenStackReconciler) applyOperator(ctx context.Context, instance *opera // set related images on the openstack-operator serviceOp.Deployment.Manager.Env = append(serviceOp.Deployment.Manager.Env, relatedImagesEnv...) - case operatorv1beta1.OpenStackBaremetalOperatorName: - // enable webhooks on the openstack-operator - serviceOp.Deployment.Manager.Env = append(serviceOp.Deployment.Manager.Env, - corev1.EnvVar{ - Name: "ENABLE_WEBHOOKS", - Value: "true", - }) - // set related images on the openstack-baremetal-operator - serviceOp.Deployment.Manager.Env = append(serviceOp.Deployment.Manager.Env, - relatedImagesEnv...) - case operatorv1beta1.InfraOperatorName: - // enable webhooks on the infra-operator - serviceOp.Deployment.Manager.Env = append(serviceOp.Deployment.Manager.Env, - corev1.EnvVar{ - Name: "ENABLE_WEBHOOKS", - Value: "true", - }) default: - // disable webhooks per default + // Enable webhooks for operators whose manifest was found at startup (admission or + // dormant conversion-only - either way the webhook server must run); others default + // false. Deriving this from webhookOperatorSet - the same source isBlockingWebhookService + // uses for readiness - keeps webhook-enablement and readiness-blocking from diverging. + enableWebhooks := "false" + if _, ok := r.webhookOperatorSet[op.Name]; ok { + enableWebhooks = "true" + } serviceOp.Deployment.Manager.Env = append(serviceOp.Deployment.Manager.Env, corev1.EnvVar{ Name: "ENABLE_WEBHOOKS", - Value: "false", + Value: enableWebhooks, }) + if op.Name == operatorv1beta1.OpenStackBaremetalOperatorName { + // set related images on the openstack-baremetal-operator + serviceOp.Deployment.Manager.Env = append(serviceOp.Deployment.Manager.Env, + relatedImagesEnv...) + } } // Set METRICS_CERTS for each operator @@ -1178,6 +1364,8 @@ func (r *OpenStackReconciler) postCleanupObsoleteResources(ctx context.Context, // SetupWithManager sets up the controller with the Manager. func (r *OpenStackReconciler) SetupWithManager(mgr ctrl.Manager) error { + bindir := util.GetEnvVar("BASE_BINDATA", "/bindata") + r.webhookOperatorSet = loadWebhookOperatorSet(context.TODO(), bindir) return ctrl.NewControllerManagedBy(mgr). Owns(&appsv1.Deployment{}). For(&operatorv1beta1.OpenStack{}). diff --git a/internal/controller/operator/openstack_controller_test.go b/internal/controller/operator/openstack_controller_test.go index 8183004abd..1fa66641af 100644 --- a/internal/controller/operator/openstack_controller_test.go +++ b/internal/controller/operator/openstack_controller_test.go @@ -18,14 +18,18 @@ package operator import ( "context" + "os" + "path/filepath" "testing" . "github.com/onsi/gomega" //revive:disable:dot-imports operatorv1beta1 "github.com/openstack-k8s-operators/openstack-operator/api/operator/v1beta1" corev1 "k8s.io/api/core/v1" + discoveryv1 "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -36,10 +40,124 @@ func newTestScheme(t *testing.T) *runtime.Scheme { s := runtime.NewScheme() g := NewWithT(t) g.Expect(corev1.AddToScheme(s)).To(Succeed()) + g.Expect(discoveryv1.AddToScheme(s)).To(Succeed()) g.Expect(operatorv1beta1.AddToScheme(s)).To(Succeed()) return s } +// emptyEndpointSlice builds an EndpointSlice with no endpoints, as if the backing +// Service has no ready pods yet. +func emptyEndpointSlice(name, namespace string) *discoveryv1.EndpointSlice { + return &discoveryv1.EndpointSlice{ + ObjectMeta: metav1.ObjectMeta{Name: name + "-abcde", Namespace: namespace, + Labels: map[string]string{discoveryv1.LabelServiceName: name}}, + AddressType: discoveryv1.AddressTypeIPv4, + } +} + +// TestCheckServiceEndpointsBlocksOnInfraAndBaremetal covers a regression where +// switching from a hardcoded webhook-service list to dynamic discovery +// (webhookServiceNames) stopped blocking readiness on the infra-operator and +// openstack-baremetal-operator webhook endpoints. Those two back always-active +// admission webhooks (see hack/sync-bindata.sh's special-casing), unlike +// service-operator conversion webhooks which stay dormant until v1beta2 lands and +// must not block readiness. +func TestCheckServiceEndpointsBlocksOnInfraAndBaremetal(t *testing.T) { + g := NewWithT(t) + s := newTestScheme(t) + ns := "openstack-operators" + + instance := &operatorv1beta1.OpenStack{ + ObjectMeta: metav1.ObjectMeta{Name: "openstack", Namespace: ns}, + } + + // openstack-operator's own webhook is up. + ownSlice := emptyEndpointSlice("openstack-operator-webhook-service", ns) + ownSlice.Endpoints = []discoveryv1.Endpoint{{ + Addresses: []string{"10.0.0.1"}, + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true), Serving: ptr.To(true)}, + }} + + // infra-operator's and openstack-baremetal-operator's webhooks have no endpoints yet + // (e.g. pods not scheduled): either one must block. + infraSlice := emptyEndpointSlice("infra-operator-webhook-service", ns) + baremetalSlice := emptyEndpointSlice("openstack-baremetal-operator-webhook-service", ns) + + fakeClient := fakeclient.NewClientBuilder(). + WithScheme(s). + WithObjects(instance, ownSlice, infraSlice, baremetalSlice). + Build() + + r := &OpenStackReconciler{ + Client: fakeClient, + Scheme: s, + // true: both manifests register an admission webhook (see loadWebhookOperatorSet). + webhookOperatorSet: map[string]bool{"infra": true, "openstack-baremetal": true}, + } + + result, err := r.checkServiceEndpoints(context.Background(), instance) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(result.RequeueAfter).To(BeNumerically(">", 0), "missing infra-operator and baremetal webhook endpoints must requeue, not be treated as non-blocking") + + // infra-operator's webhook becomes ready, but baremetal's doesn't yet: must still block. + infraSlice.Endpoints = []discoveryv1.Endpoint{{ + Addresses: []string{"10.0.0.2"}, + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true), Serving: ptr.To(true)}, + }} + g.Expect(fakeClient.Update(context.Background(), infraSlice)).To(Succeed()) + + result, err = r.checkServiceEndpoints(context.Background(), instance) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(result.RequeueAfter).To(BeNumerically(">", 0), "missing openstack-baremetal-operator webhook endpoint alone must still requeue") + + // Once openstack-baremetal-operator's webhook is also ready, reconciliation proceeds. + baremetalSlice.Endpoints = []discoveryv1.Endpoint{{ + Addresses: []string{"10.0.0.3"}, + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true), Serving: ptr.To(true)}, + }} + g.Expect(fakeClient.Update(context.Background(), baremetalSlice)).To(Succeed()) + + result, err = r.checkServiceEndpoints(context.Background(), instance) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(result.RequeueAfter).To(BeZero()) +} + +// TestCheckServiceEndpointsDormantServiceOperatorWebhookNonBlocking covers the +// companion case: a service-operator's conversion webhook (dormant until v1beta2) +// must not block readiness even with no endpoints. +func TestCheckServiceEndpointsDormantServiceOperatorWebhookNonBlocking(t *testing.T) { + g := NewWithT(t) + s := newTestScheme(t) + ns := "openstack-operators" + + instance := &operatorv1beta1.OpenStack{ + ObjectMeta: metav1.ObjectMeta{Name: "openstack", Namespace: ns}, + } + + ownSlice := emptyEndpointSlice("openstack-operator-webhook-service", ns) + ownSlice.Endpoints = []discoveryv1.Endpoint{{ + Addresses: []string{"10.0.0.1"}, + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true), Serving: ptr.To(true)}, + }} + + fakeClient := fakeclient.NewClientBuilder(). + WithScheme(s). + WithObjects(instance, ownSlice). + Build() + + r := &OpenStackReconciler{ + Client: fakeClient, + Scheme: s, + // false: keystone's manifest carries only the dormant conversion webhook's + // serving resources, not an admission webhook (see loadWebhookOperatorSet). + webhookOperatorSet: map[string]bool{"keystone": false}, + } + + result, err := r.checkServiceEndpoints(context.Background(), instance) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(result.RequeueAfter).To(BeZero(), "missing dormant conversion webhook endpoint must not block readiness") +} + // TestAdoptAwayServiceAccounts covers the ownership-migration logic: sub-operator // ServiceAccounts left over from before openstack-operator#2109 are still // controlled by the OpenStack instance, but this controller no longer manages them @@ -85,3 +203,48 @@ func TestAdoptAwayServiceAccounts(t *testing.T) { // Idempotent: calling it again with the reference already gone must not error. g.Expect(r.adoptAwayServiceAccounts(context.Background(), instance)).To(Succeed()) } + +// TestLoadWebhookOperatorSet covers the startup discovery that drives whether a webhook +// service blocks readiness: a manifest with a Mutating/ValidatingWebhookConfiguration +// (written by hack/sync-bindata.sh's write_webhooks, e.g. for infra-operator) must be +// flagged as an admission webhook, while one with only the serving Service+Certificate +// (write_webhook_serving_resources, a dormant CRD conversion webhook) must not. +func TestLoadWebhookOperatorSet(t *testing.T) { + g := NewWithT(t) + bindir := t.TempDir() + operatorDir := filepath.Join(bindir, "operator") + g.Expect(os.MkdirAll(operatorDir, 0o755)).To(Succeed()) + + admissionManifest := `apiVersion: v1 +kind: Service +metadata: + name: infra-operator-webhook-service +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: MutatingWebhookConfiguration +metadata: + name: infra-operator-mutating-webhook-configuration +` + conversionOnlyManifest := `apiVersion: v1 +kind: Service +metadata: + name: keystone-operator-webhook-service +--- +apiVersion: cert-manager.io/v1 +kind: Certificate +metadata: + name: keystone-operator-serving-cert +` + g.Expect(os.WriteFile(filepath.Join(operatorDir, "infra-operator-webhooks.yaml"), []byte(admissionManifest), 0o644)).To(Succeed()) + g.Expect(os.WriteFile(filepath.Join(operatorDir, "keystone-operator-webhooks.yaml"), []byte(conversionOnlyManifest), 0o644)).To(Succeed()) + + // Unreadable manifest (a directory, not a file) must fail closed: treated as an + // admission webhook rather than silently downgraded to dormant/non-blocking. + g.Expect(os.MkdirAll(filepath.Join(operatorDir, "broken-operator-webhooks.yaml"), 0o755)).To(Succeed()) + + result := loadWebhookOperatorSet(context.Background(), bindir) + + g.Expect(result).To(HaveKeyWithValue("infra", true), "manifest with MutatingWebhookConfiguration must be flagged as an admission webhook") + g.Expect(result).To(HaveKeyWithValue("keystone", false), "manifest with only serving resources must be flagged as dormant conversion-only") + g.Expect(result).To(HaveKeyWithValue("broken", true), "unreadable manifest must fail closed as an admission webhook, not silently become non-blocking") +} diff --git a/internal/operator/bindata/merge.go b/internal/operator/bindata/merge.go index eee7a10ebf..47bc5d4e0b 100644 --- a/internal/operator/bindata/merge.go +++ b/internal/operator/bindata/merge.go @@ -29,6 +29,10 @@ func MergeMetadataForUpdate(current, updated *uns.Unstructured) error { // Some objects, such as Deployments and Services require // some semantic-aware updates func MergeObjectForUpdate(current, updated *uns.Unstructured) error { + if err := MergeCRDForUpdate(current, updated); err != nil { + return err + } + if err := MergeWebhookConfigurationForUpdate(current, updated); err != nil { return err } @@ -55,6 +59,24 @@ func MergeObjectForUpdate(current, updated *uns.Unstructured) error { return nil } +// MergeCRDForUpdate preserves the cert-manager-injected caBundle in the CRD's +// conversion webhook clientConfig on re-apply. Without this, each reconcile +// would clear the caBundle that cert-manager wrote, breaking conversion TLS. +func MergeCRDForUpdate(current, updated *uns.Unstructured) error { + gvk := updated.GroupVersionKind() + if gvk.Group != "apiextensions.k8s.io" || gvk.Kind != "CustomResourceDefinition" { + return nil + } + caBundle, found, err := uns.NestedString(current.Object, "spec", "conversion", "webhook", "clientConfig", "caBundle") + if err != nil { + return errors.Wrapf(err, "failed to read caBundle from CRD %s", updated.GetName()) + } + if !found || caBundle == "" { + return nil + } + return uns.SetNestedField(updated.Object, caBundle, "spec", "conversion", "webhook", "clientConfig", "caBundle") +} + // MergeWebhookConfigurationForUpdate merges the caBundle from the current // webhook configuration into the updated one, matching webhooks by name. // This preserves the CA certificate injected by cert-manager while keeping diff --git a/internal/operator/bindata/merge_test.go b/internal/operator/bindata/merge_test.go index f7843e4a74..2208303207 100644 --- a/internal/operator/bindata/merge_test.go +++ b/internal/operator/bindata/merge_test.go @@ -212,6 +212,116 @@ func TestMergeWebhookSkipsNonWebhookResources(t *testing.T) { } } +func makeCRD(caBundle string) *uns.Unstructured { + svc := map[string]interface{}{ + "name": "keystone-operator-webhook-service", + "namespace": "openstack-operators", + "path": "/convert", + } + cc := map[string]interface{}{"service": svc} + if caBundle != "" { + cc["caBundle"] = caBundle + } + obj := &uns.Unstructured{Object: map[string]interface{}{ + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": map[string]interface{}{"name": "keystoneapis.keystone.openstack.org"}, + "spec": map[string]interface{}{ + "conversion": map[string]interface{}{ + "strategy": "Webhook", + "webhook": map[string]interface{}{"clientConfig": cc}, + }, + }, + }} + obj.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "apiextensions.k8s.io", Version: "v1", Kind: "CustomResourceDefinition", + }) + return obj +} + +func TestMergeCRDPreservesCaBundle(t *testing.T) { + current := makeCRD("CERT-MANAGER-INJECTED-CA") + updated := makeCRD("") + + if err := MergeCRDForUpdate(current, updated); err != nil { + t.Fatalf("MergeCRDForUpdate failed: %v", err) + } + + got, found, err := uns.NestedString(updated.Object, "spec", "conversion", "webhook", "clientConfig", "caBundle") + if err != nil || !found { + t.Fatalf("caBundle not found after merge: err=%v found=%v", err, found) + } + if got != "CERT-MANAGER-INJECTED-CA" { + t.Errorf("expected caBundle CERT-MANAGER-INJECTED-CA, got %q", got) + } +} + +func TestMergeCRDNoCaBundleInCurrent(t *testing.T) { + current := makeCRD("") + updated := makeCRD("") + + if err := MergeCRDForUpdate(current, updated); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + _, found, _ := uns.NestedString(updated.Object, "spec", "conversion", "webhook", "clientConfig", "caBundle") + if found { + t.Error("expected no caBundle when current has none") + } +} + +func TestMergeCRDSkipsNonCRDs(t *testing.T) { + current := makeWebhookConfig(makeWebhook("mfoo.kb.io", "/mutate-foo", "CABUNDLE")) + updated := makeWebhookConfig(makeWebhook("mfoo.kb.io", "/mutate-foo", nil)) + + if err := MergeCRDForUpdate(current, updated); err != nil { + t.Fatalf("unexpected error: %v", err) + } + // caBundle should NOT be copied — wrong object type + if bundle := getWebhookCABundle(updated, 0); bundle != nil { + t.Errorf("MergeCRDForUpdate should not touch non-CRD objects, but caBundle=%v", bundle) + } +} + +func TestMergeCRDInvalidCaBundleType(t *testing.T) { + // When caBundle is not a string (e.g., malformed CRD with caBundle as a map), + // MergeCRDForUpdate should return an error instead of silently ignoring it. + current := &uns.Unstructured{Object: map[string]interface{}{ + "apiVersion": "apiextensions.k8s.io/v1", + "kind": "CustomResourceDefinition", + "metadata": map[string]interface{}{"name": "keystoneapis.keystone.openstack.org"}, + "spec": map[string]interface{}{ + "conversion": map[string]interface{}{ + "strategy": "Webhook", + "webhook": map[string]interface{}{ + "clientConfig": map[string]interface{}{ + "service": map[string]interface{}{ + "name": "keystone-operator-webhook-service", + "namespace": "openstack-operators", + "path": "/convert", + }, + // Invalid: caBundle should be a string, but it's a map + "caBundle": map[string]interface{}{"invalid": "structure"}, + }, + }, + }, + }, + }} + current.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "apiextensions.k8s.io", Version: "v1", Kind: "CustomResourceDefinition", + }) + + updated := makeCRD("") + + err := MergeCRDForUpdate(current, updated) + if err == nil { + t.Fatal("expected error for invalid caBundle type, got nil") + } + if err.Error() == "" { + t.Error("error message should not be empty") + } +} + func TestMergeWebhookValidatingConfig(t *testing.T) { // The fix should also work for ValidatingWebhookConfiguration. current := &uns.Unstructured{Object: map[string]interface{}{ diff --git a/test/kuttl/tests/ctlplane-conversion-webhook/01-assert-deploy-openstack.yaml b/test/kuttl/tests/ctlplane-conversion-webhook/01-assert-deploy-openstack.yaml new file mode 120000 index 0000000000..762a8cf31d --- /dev/null +++ b/test/kuttl/tests/ctlplane-conversion-webhook/01-assert-deploy-openstack.yaml @@ -0,0 +1 @@ +../../common/assert-sample-deployment.yaml \ No newline at end of file diff --git a/test/kuttl/tests/ctlplane-conversion-webhook/01-deploy-openstack.yaml b/test/kuttl/tests/ctlplane-conversion-webhook/01-deploy-openstack.yaml new file mode 100644 index 0000000000..6c9d0887d8 --- /dev/null +++ b/test/kuttl/tests/ctlplane-conversion-webhook/01-deploy-openstack.yaml @@ -0,0 +1,5 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +commands: + - script: | + oc kustomize ../../../../config/samples/base/openstackcontrolplane | oc apply -n $NAMESPACE -f - diff --git a/test/kuttl/tests/ctlplane-conversion-webhook/02-assert-crd-namespaces.yaml b/test/kuttl/tests/ctlplane-conversion-webhook/02-assert-crd-namespaces.yaml new file mode 100644 index 0000000000..b5ea897770 --- /dev/null +++ b/test/kuttl/tests/ctlplane-conversion-webhook/02-assert-crd-namespaces.yaml @@ -0,0 +1,48 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +timeout: 120 +# Joint gate for OSPRH-30208 + OSPRH-30209 + OSPRH-30210: +# Assert that openstack-operator's CRDs carry the correct install-namespace in both +# the cert-manager annotation and the conversion webhook service reference, and that +# cert-manager has populated the caBundle. +# +# INSTALL_NS: the namespace the openstack-operator controller runs in (where it installs +# CRDs and writes the annotation/service.namespace). Derived from the operator deployment, +# NOT from $NAMESPACE which is the kuttl test namespace (openstack-kuttl-tests). +commands: + - script: | + set -e + CRD=openstackcontrolplanes.core.openstack.org + INSTALL_NS=$(oc get deployment -A -l control-plane=controller-manager,app.kubernetes.io/name=openstack-operator -o jsonpath='{.items[0].metadata.namespace}') + if [ -z "$INSTALL_NS" ]; then + echo "FAIL: openstack-operator deployment not found — is the operator deployed?"; exit 1 + fi + + # 1. inject-ca-from annotation namespace == install namespace (OSPRH-30208) + ANNO=$(oc get crd "$CRD" -o jsonpath='{.metadata.annotations.cert-manager\.io/inject-ca-from}') + if [ -z "$ANNO" ]; then + echo "FAIL: cert-manager.io/inject-ca-from annotation is missing on $CRD" + exit 1 + fi + ANNO_NS="${ANNO%%/*}" + if [ "$ANNO_NS" != "$INSTALL_NS" ]; then + echo "FAIL: inject-ca-from annotation namespace '$ANNO_NS' != install namespace '$INSTALL_NS'" + exit 1 + fi + echo "OK: inject-ca-from annotation namespace = $ANNO_NS" + + # 2. conversion webhook service namespace == install namespace (OSPRH-30209) + SVC_NS=$(oc get crd "$CRD" -o jsonpath='{.spec.conversion.webhook.clientConfig.service.namespace}') + if [ "$SVC_NS" != "$INSTALL_NS" ]; then + echo "FAIL: conversion service namespace '$SVC_NS' != install namespace '$INSTALL_NS'" + exit 1 + fi + echo "OK: conversion service namespace = $SVC_NS" + + # 3. caBundle non-empty — cert-manager injected (OSPRH-30208, async) + CA=$(oc get crd "$CRD" -o jsonpath='{.spec.conversion.webhook.clientConfig.caBundle}') + if [ -z "$CA" ]; then + echo "FAIL: caBundle is empty — cert-manager has not injected yet" + exit 1 + fi + echo "OK: caBundle is non-empty (cert-manager injected)" diff --git a/test/kuttl/tests/ctlplane-conversion-webhook/03-assert-conversion-roundtrip.yaml b/test/kuttl/tests/ctlplane-conversion-webhook/03-assert-conversion-roundtrip.yaml new file mode 100644 index 0000000000..914b4cd0be --- /dev/null +++ b/test/kuttl/tests/ctlplane-conversion-webhook/03-assert-conversion-roundtrip.yaml @@ -0,0 +1,114 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +# Phase-1 milestone gate for conversion webhook infrastructure. +# Step 02 already verified the CRD field namespaces and caBundle; this step +# focuses on the conversion round-trip: read a service API CR at each served +# version and assert key fields are equal (proves the webhook fires correctly). +# +# CONDITIONAL: Discovers any service operator CRD with v1beta2 support; skips +# gracefully if none found yet. Test activates automatically once any service +# operator adds v1beta2. +# +# Two namespaces in play: +# INSTALL_NS — operator namespace where openstack-operator runs and writes +# the CRD annotation/service.namespace (derived from operator deployment) +# CR_NS — kuttl test namespace where service API CRs are deployed by the +# OpenStackControlPlane reconciler (= $NAMESPACE set by kuttl) +timeout: 120 +commands: + - script: | + set -e + + INSTALL_NS=$(oc get deployment -A -l control-plane=controller-manager,app.kubernetes.io/name=openstack-operator -o jsonpath='{.items[0].metadata.namespace}') + if [ -z "$INSTALL_NS" ]; then + echo "FAIL: openstack-operator deployment not found — is the operator deployed?"; exit 1 + fi + CR_NS="$NAMESPACE" + + # Discover service operator CRDs with v1beta2 support and webhook conversion + # Pattern: apis..openstack.org (keystoneapis, novaapis, etc.) + CRD_JSON=$(oc get crds -o json) || { + echo "FAIL: failed to retrieve CRDs"; exit 1 + } + CANDIDATE_CRDS=$(echo "$CRD_JSON" | jq -r ' + .items[] | + select(.metadata.name | test("^[a-z]+apis\\.[a-z]+\\.openstack\\.org$")) | + select(.spec.versions | map(select(.name == "v1beta1" and .served)) | length > 0) | + select(.spec.versions | map(select(.name == "v1beta2" and .served)) | length > 0) | + select(.spec.conversion.strategy == "Webhook") | + .metadata.name + ') + + if [ -z "$CANDIDATE_CRDS" ]; then + echo "SKIP: no service operator CRDs with v1beta2 support found yet" + echo " (test will activate automatically once any service operator adds v1beta2)" + exit 0 + fi + + # Get OpenStackControlPlane CR to check which services are enabled + CTLPLANE_CR=$(oc get openstackcontrolplane -n "$CR_NS" -o jsonpath='{.items[0].metadata.name}' 2>/dev/null || echo "") + if [ -z "$CTLPLANE_CR" ]; then + echo "FAIL: no OpenStackControlPlane CR found in $CR_NS" + exit 1 + fi + + # Test all CRDs that have v1beta2 support + TESTED_COUNT=0 + for candidate in $CANDIDATE_CRDS; do + API_GROUP=$(echo "$candidate" | cut -d. -f2-) + RESOURCE=$(echo "$candidate" | cut -d. -f1) + + # Derive service name from CRD (e.g., keystoneapis.keystone.openstack.org -> keystone) + SERVICE_NAME=$(echo "$candidate" | cut -d. -f2) + + # Check if service is enabled in OpenStackControlPlane + # Empty/missing field means enabled (default), only explicit "false" means disabled + SERVICE_ENABLED=$(oc get openstackcontrolplane "$CTLPLANE_CR" -n "$CR_NS" \ + -o jsonpath="{.spec.$SERVICE_NAME.enabled}" 2>/dev/null || echo "true") + # Treat empty as enabled (matches API defaulting) + [ -z "$SERVICE_ENABLED" ] && SERVICE_ENABLED="true" + + CR_NAME=$(oc get "$candidate" -n "$CR_NS" -o jsonpath='{.items[0].metadata.name}' 2>/dev/null || echo "") + + if [ -z "$CR_NAME" ]; then + if [ "$SERVICE_ENABLED" = "false" ]; then + echo "SKIP: $candidate has v1beta2, but $SERVICE_NAME is disabled (no CR expected)" + continue + else + echo "FAIL: $candidate has v1beta2 and $SERVICE_NAME is enabled, but no CR instance found" + exit 1 + fi + fi + echo "Testing conversion webhook with $candidate (CR: $CR_NAME)" + + # Read at v1beta2 (storage version — no conversion needed; proves CRD is installed) + oc get --raw \ + "/apis/$API_GROUP/v1beta2/namespaces/$CR_NS/$RESOURCE/$CR_NAME" \ + | jq -e '.spec' > /dev/null || { + echo "FAIL: $candidate v1beta2 read failed or spec missing (CRD install/conversion issue)" + exit 1 + } + echo "OK: $candidate v1beta2 read succeeded" + + # Read at v1beta1 (legacy version — fires conversion webhook: v1beta2 storage -> v1beta1) + oc get --raw \ + "/apis/$API_GROUP/v1beta1/namespaces/$CR_NS/$RESOURCE/$CR_NAME" \ + | jq -e '.spec' > /dev/null || { + echo "FAIL: $candidate v1beta1 read failed or spec missing (conversion webhook not reachable)" + exit 1 + } + echo "OK: $candidate v1beta1 read succeeded (conversion webhook is reachable)" + + # Phase-1 milestone: verify both API versions are readable (proves webhook infrastructure works). + # We do NOT assert spec equality — once service operators implement non-trivial conversion + # (renamed/restructured fields), v1beta1 and v1beta2 specs will legitimately differ. + # Full conversion correctness tests belong in service operator repos. + TESTED_COUNT=$((TESTED_COUNT + 1)) + done + + if [ "$TESTED_COUNT" -eq 0 ]; then + echo "SKIP: found CRDs with v1beta2 support, but all services are disabled (no CR instances to test)" + exit 0 + fi + + echo "PASS: conversion webhook Phase-1 milestone gate complete ($TESTED_COUNT service(s) tested)"