HIVE-3132: Switch to Admission v1 by 2uasimojo · Pull Request #2881 · openshift/hive

2uasimojo · 2026-03-25T19:47:33Z

v1beta1 has been deprecated, and the last k8s that didn't support v1 (1.16, I think) has been "out of support", for long enough that we should be safe to move fully to v1 now.

Summary by CodeRabbit

Chores
- Migrated admission integrations to Kubernetes AdmissionReview v1 for broader cluster compatibility.
New Features
- Added validation support for ClusterDeploymentCustomization resources.
Bug Fixes / Behavior
- ClusterPool webhook no longer runs on DELETE (now only CREATE/UPDATE), reducing unnecessary validation on deletes.

openshift-ci-robot · 2026-03-25T19:47:38Z

@2uasimojo: This pull request references HIVE-3132 which is a valid jira issue.

Details

In response to this:

v1beta1 has been deprecated, and the last k8s that didn't support v1 (1.16, I think) has been "out of support", for long enough that we should be safe to move fully to v1 now.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

coderabbitai · 2026-03-25T19:48:23Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

Replaced admission.k8s.io/v1beta1 with admission.k8s.io/v1 across webhook YAMLs, embedded assets, scheme registration, validating-webhook implementations, unit/e2e tests, and AdmissionReview fixtures; added a new clusterdeploymentcustomization webhook asset and removed DELETE from the clusterpool webhook operations.

Changes

Cohort / File(s)	Summary
Webhook Configuration Files `config/hiveadmission/clusterdeployment-webhook.yaml`, `config/hiveadmission/clusterimageset-webhook.yaml`, `config/hiveadmission/clusterprovision-webhook.yaml`, `config/hiveadmission/dnszones-webhook.yaml`, `config/hiveadmission/machinepool-webhook.yaml`, `config/hiveadmission/selectorsyncset-webhook.yaml`, `config/hiveadmission/syncset-webhook.yaml`, `config/hiveadmission/clusterpool-webhook.yaml`	Updated `webhooks[].admissionReviewVersions` from `v1beta1` → `v1`. Additionally, removed `DELETE` from `clusterpool` webhook `rules[].operations` (now CREATE/UPDATE).
New Webhook Manifest `config/hiveadmission/clusterdeploymentcustomization-webhook.yaml`	Added a new `ValidatingWebhookConfiguration` for `clusterdeploymentcustomizations` with `admissionReviewVersions: ["v1"]`, operations CREATE/UPDATE, `failurePolicy: Fail`.
Embedded Assets / Bindata `pkg/operator/assets/bindata.go`	Updated embedded YAMLs to v1 AdmissionReview versions, removed DELETE in embedded clusterpool rules, and added the new clusterdeploymentcustomization asset and generator/lookup entries.
Operator Webhook Asset List `pkg/operator/hive/hiveadmission.go`	Added `config/hiveadmission/clusterdeploymentcustomization-webhook.yaml` to `webhookAssets` so it is included in reconciliation/installation.
Scheme Registration `pkg/util/scheme/scheme.go`	Switched import and AddToScheme registration from `admission/v1beta1` → `admission/v1`.
Validating Webhook Implementations `pkg/validating-webhooks/hive/v1/...` `clusterdeployment_validating_admission_hook.go`, `clusterdeploymentcustomization_validating_admission_hook.go`, `clusterimageset_validating_admission_hook.go`, `clusterpool_validating_admission_hook.go`, `clusterprovision_validating_admission_hook.go`, `dnszone_validating_admission_hook.go`, `machinepool_validating_admission_hook.go`, `selector_syncset_validating_admission_hook.go`, `syncset_validating_admission_hook.go`	Migrated handler and helper signatures and all AdmissionRequest/AdmissionResponse usages from v1beta1 → v1; updated operation constants to `admissionv1` and adjusted resource name check for clusterdeploymentcustomization. Validation logic otherwise preserved.
Unit Tests for Webhooks `pkg/validating-webhooks/hive/v1/*_test.go` (clusterdeployment, clusterimageset, clusterpool, clusterprovision, dnszone, machinepool, selector_syncset, syncset)	Replaced test imports and constructed objects to use `admissionv1` types and `admissionv1.Operation` constants; test assertions unchanged.
E2E Admission Tests & Fixtures `test/e2e/postdeploy/admission/admission_test.go`, `test/e2e/postdeploy/admission/testdata/hiveadmission-review-failure.json`, `...-success.json`, `...-update-failure.json`	Switched `AdmissionReview` usage/imports to `admission.k8s.io/v1` in test code and updated JSON fixtures `apiVersion` from `admission.k8s.io/v1beta1` → `admission.k8s.io/v1`.
Review Metadata / Linting Rules `.coderabbit.yaml`	Added review instructions requiring consistency between `config/hiveadmission/*-webhook.yaml` manifests, embedded bindata, `pkg/operator/hive/hiveadmission.go` asset listing, and corresponding webhook implementations; added reciprocal checks for implementation files.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hopped through code both old and new,

Swapped betas for v1 — a tidy chew,
New webhook joined the meadow bright,
DELETE hopped off into the night,
A carrot wink — I’m done, goodnight.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 3.85% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'HIVE-3132: Switch to Admission v1' clearly and specifically summarizes the main change: migrating Hive admission webhooks from the deprecated v1beta1 API to the stable v1 API version.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

openshift-ci-robot · 2026-03-25T19:51:22Z

@2uasimojo: This pull request references HIVE-3132 which is a valid jira issue.

Details

In response to this:

v1beta1 has been deprecated, and the last k8s that didn't support v1 (1.16, I think) has been "out of support", for long enough that we should be safe to move fully to v1 now.

Summary by CodeRabbit

Release Notes

Chores

Updated all Hive admission webhooks to use the stable Kubernetes admission API version v1 instead of the deprecated v1beta1. This includes webhook configuration files and implementation updates across cluster deployment, image sets, provisions, DNS zones, machine pools, sync sets, and selector sync sets resources to enhance stability and ensure compatibility with current Kubernetes standards.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

coderabbitai

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

pkg/validating-webhooks/hive/v1/clusterdeployment_validating_admission_hook.go (1)

978-1029: ⚠️ Potential issue | 🟠 Major

Guard empty ingress domains before wildcard validation.

Line 992 can still reach validateIngressDomainsNotWildcard, and that helper indexes ingress.Domain[0]. An empty spec.ingress[*].domain will panic the webhook instead of returning a validation error.

🛠️ Suggested hardening

 func validateIngress(cd *hivev1.ClusterDeployment, contextLogger *log.Entry) *admissionv1.AdmissionResponse {
 	if !validateIngressList(&cd.Spec) {
 		message := "Ingress list must include a default entry"
 		contextLogger.Infof("Failed validation: %v", message)

 		return &admissionv1.AdmissionResponse{
 			Allowed: false,
 			Result: &metav1.Status{
 				Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest,
 				Message: message,
 			},
 		}
 	}
+
+	for _, ingress := range cd.Spec.Ingress {
+		if ingress.Domain == "" {
+			message := "Ingress domain must not be empty"
+			contextLogger.Infof("Failed validation: %v", message)
+			return &admissionv1.AdmissionResponse{
+				Allowed: false,
+				Result: &metav1.Status{
+					Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest,
+					Message: message,
+				},
+			}
+		}
+	}

 	if !validateIngressDomainsNotWildcard(&cd.Spec) {
 		message := "Ingress domains must not lead with *"
 		contextLogger.Infof("Failed validation: %v", message)
 		return &admissionv1.AdmissionResponse{

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@pkg/validating-webhooks/hive/v1/clusterdeployment_validating_admission_hook.go`
around lines 978 - 1029, The webhook can panic because
validateIngressDomainsNotWildcard assumes ingress.Domain[0] exists; update the
validation to guard empty domain slices (either add a pre-check in
validateIngress to reject ingress entries with len(ingress.Domain)==0 before
calling validateIngressDomainsNotWildcard, or modify
validateIngressDomainsNotWildcard itself to first check each ingress.Domain
length and return false if any are empty) so that accessing ingress.Domain[0] is
safe and an appropriate AdmissionResponse is returned; refer to validateIngress
and validateIngressDomainsNotWildcard to locate the change.

🧹 Nitpick comments (1)

pkg/validating-webhooks/hive/v1/selector_syncset_validating_admission_hook_test.go (1)

100-109: Remove duplicate table cases to reduce noise.

Line 100 and Line 106 duplicate the earlier SecretReference create/update cases (Line 88 and Line 94) with the same setup and expected result.

♻️ Proposed cleanup

-		{
-			name:            "Test valid SecretReference create",
-			operation:       admissionv1.Create,
-			selectorSyncSet: testSecretReferenceSelectorSyncSet(),
-			expectedAllowed: true,
-		},
-		{
-			name:            "Test valid SecretReference update",
-			operation:       admissionv1.Update,
-			selectorSyncSet: testSecretReferenceSelectorSyncSet(),
-			expectedAllowed: true,
-		},

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@pkg/validating-webhooks/hive/v1/selector_syncset_validating_admission_hook_test.go`
around lines 100 - 109, Remove the duplicate table-driven test cases that repeat
the SecretReference create/update checks: locate the test entries using
selectorSyncSet: testSecretReferenceSelectorSyncSet() with operation:
admissionv1.Create and admissionv1.Update and expectedAllowed: true and delete
the redundant instances (they mirror the earlier cases). Ensure only the
original create/update cases remain so the test table has no duplicate entries
and the test name strings ("Test valid SecretReference create"/"Test valid
SecretReference update") are unique.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@pkg/validating-webhooks/hive/v1/clusterdeploymentcustomization_validating_admission_hook.go`:
- Line 72: Create a ValidatingWebhookConfiguration manifest named
clusterdeploymentcustomization-webhook.yaml that matches the migrated v1 hook
(ensure the webhook rules, clientConfig URL/Service, admissionReviewVersions:
["v1"], and the same metadata as the
ClusterDeploymentCustomizationValidatingAdmissionHook Validate handler expect),
place it in config/hiveadmission/, and add the new file to the embed list in
pkg/operator/assets/bindata.go so it is bundled into the operator binary and the
apiserver can route AdmissionReview v1 requests to the
ClusterDeploymentCustomizationValidatingAdmissionHook.Validate endpoint.

---

Outside diff comments:
In
`@pkg/validating-webhooks/hive/v1/clusterdeployment_validating_admission_hook.go`:
- Around line 978-1029: The webhook can panic because
validateIngressDomainsNotWildcard assumes ingress.Domain[0] exists; update the
validation to guard empty domain slices (either add a pre-check in
validateIngress to reject ingress entries with len(ingress.Domain)==0 before
calling validateIngressDomainsNotWildcard, or modify
validateIngressDomainsNotWildcard itself to first check each ingress.Domain
length and return false if any are empty) so that accessing ingress.Domain[0] is
safe and an appropriate AdmissionResponse is returned; refer to validateIngress
and validateIngressDomainsNotWildcard to locate the change.

---

Nitpick comments:
In
`@pkg/validating-webhooks/hive/v1/selector_syncset_validating_admission_hook_test.go`:
- Around line 100-109: Remove the duplicate table-driven test cases that repeat
the SecretReference create/update checks: locate the test entries using
selectorSyncSet: testSecretReferenceSelectorSyncSet() with operation:
admissionv1.Create and admissionv1.Update and expectedAllowed: true and delete
the redundant instances (they mirror the earlier cases). Ensure only the
original create/update cases remain so the test table has no duplicate entries
and the test name strings ("Test valid SecretReference create"/"Test valid
SecretReference update") are unique.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 30d18991-2a7e-4431-9d72-c15861c63590

📥 Commits

Reviewing files that changed from the base of the PR and between 910c602 and f8d8e86.

📒 Files selected for processing (27)

config/hiveadmission/clusterdeployment-webhook.yaml
config/hiveadmission/clusterimageset-webhook.yaml
config/hiveadmission/clusterprovision-webhook.yaml
config/hiveadmission/dnszones-webhook.yaml
config/hiveadmission/machinepool-webhook.yaml
config/hiveadmission/selectorsyncset-webhook.yaml
config/hiveadmission/syncset-webhook.yaml
pkg/operator/assets/bindata.go
pkg/util/scheme/scheme.go
pkg/validating-webhooks/hive/v1/clusterdeployment_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterdeployment_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/clusterdeploymentcustomization_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterimageset_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterimageset_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/clusterpool_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterpool_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/clusterprovision_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterprovision_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/dnszone_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/dnszone_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/machinepool_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/machinepool_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/selector_syncset_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/selector_syncset_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/syncset_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/syncset_validating_admission_hook_test.go
test/e2e/postdeploy/admission/admission_test.go

pkg/validating-webhooks/hive/v1/clusterdeploymentcustomization_validating_admission_hook.go

codecov · 2026-03-25T20:31:05Z

Codecov Report

❌ Patch coverage is 80.39216% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.28%. Comparing base (2297bc8) to head (5a53841).
⚠️ Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
...ploymentcustomization_validating_admission_hook.go	0.00%	14 Missing ⚠️
.../v1/clusterdeployment_validating_admission_hook.go	86.11%	5 Missing ⚠️
...e/v1/selector_syncset_validating_admission_hook.go	71.42%	4 Missing ⚠️
...hooks/hive/v1/syncset_validating_admission_hook.go	71.42%	4 Missing ⚠️
...s/hive/v1/clusterpool_validating_admission_hook.go	87.50%	2 Missing ⚠️
...ve/v1/clusterimageset_validating_admission_hook.go	93.33%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2881   +/-   ##
=======================================
  Coverage   50.28%   50.28%           
=======================================
  Files         280      280           
  Lines       34332    34332           
=======================================
  Hits        17264    17264           
  Misses      15707    15707           
  Partials     1361     1361

Files with missing lines	Coverage Δ
pkg/operator/assets/bindata.go	`0.00% <ø> (ø)`
pkg/util/scheme/scheme.go	`100.00% <100.00%> (ø)`
...e/v1/clusterprovision_validating_admission_hook.go	`85.79% <100.00%> (ø)`
...hooks/hive/v1/dnszone_validating_admission_hook.go	`100.00% <100.00%> (ø)`
...s/hive/v1/machinepool_validating_admission_hook.go	`85.27% <100.00%> (ø)`
...ve/v1/clusterimageset_validating_admission_hook.go	`94.40% <93.33%> (ø)`
...s/hive/v1/clusterpool_validating_admission_hook.go	`80.85% <87.50%> (ø)`
...e/v1/selector_syncset_validating_admission_hook.go	`74.19% <71.42%> (ø)`
...hooks/hive/v1/syncset_validating_admission_hook.go	`81.81% <71.42%> (ø)`
.../v1/clusterdeployment_validating_admission_hook.go	`84.73% <86.11%> (ø)`
... and 1 more

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

2uasimojo · 2026-03-25T21:13:14Z

Okay @coderabbitai, we're seeing the exact same e2e failure here as we were in #2880, despite there being no remnants of v1beta1 in our code. Why is the server still sending v1beta1?

coderabbitai · 2026-03-25T21:16:35Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Look for how the webhook server decodes the AdmissionReview request
# and check if there's a decoder/handler that still references v1beta1
rg -n "v1beta1" --type go -C3

Length of output: 263475

🏁 Script executed:

#!/bin/bash
# Look for the webhook server setup and how it handles incoming requests
rg -n "AdmissionReview\|webhookHandler\|webhook.*server\|http.*handler\|ServeHTTP\|Serve\b" --type go -C3 | head -200

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Find hive's own webhook server HTTP handler (not vendor)
rg -n "ServeHTTP\|http.HandleFunc\|mux\|admissionScheme\|json.Unmarshal\|json.NewDecoder\|Decode.*AdmissionReview\|AdmissionReview" --type go -g '!vendor/**' -C3

# Also look for what scheme is used to decode incoming webhook requests
rg -n "hive_scheme\|Decode\|UnmarshalJSON" --type go -g '!vendor/**' -C3 | head -150

Length of output: 0

🏁 Script executed:

#!/bin/bash
# 1) What do our manifests advertise?
rg -n --glob '!vendor/**' 'admissionReviewVersions' -C3

# 2) Do we embed these updated assets in bindata?
rg -n --glob '!vendor/**' 'ValidatingWebhookConfiguration|admissionReviewVersions' pkg/operator/assets/bindata.go -C3 || true

# 3) Where are these applied/reconciled? Look for code that creates/updates ValidatingWebhookConfiguration.
rg -n --glob '!vendor/**' 'ValidatingWebhookConfiguration|admissionregistration.k8s.io' -C4

# 4) Also check if any kustomize overlays might still render v1beta1
fd -a --glob 'config/**/webhook*.yaml' 2>/dev/null || true

Length of output: 24053

@2uasimojo Thanks for the pointer. Based on the repo, there are no first‑party v1beta1 references left; all Hive webhook configs and the embedded assets now advertise only admissionReviewVersions: [v1], and the operator loads those v1 objects at startup (pkg/operator/assets/bindata.go and pkg/operator/hive/hiveadmission.go). From here, the API server decides which AdmissionReview version to send based on what’s in the live ValidatingWebhookConfiguration. If the cluster still has an older ValidatingWebhookConfiguration with v1beta1 (or lists v1beta1 ahead of v1), the apiserver will continue to send v1beta1 even though the code supports v1.

Given the e2e is still seeing v1beta1, the most likely cause is stale webhook configuration on the cluster that wasn’t updated yet by the operator during the test run.

Can you grab these from the failing job’s cluster to confirm?

kubectl get validatingwebhookconfiguration clusterdeploymentvalidators.admission.hive.openshift.io -o yaml
kubectl get validatingwebhookconfiguration clusterimagesetvalidators.admission.hive.openshift.io -o yaml
…and check spec.webhooks[].admissionReviewVersions

If any of them still include v1beta1 (or have both with v1beta1 first), that explains the behavior. Ensuring the operator rolls out first and reconciles these objects so they read ["v1"] only should make the apiserver send v1.

Repo evidence:

Updated embedded/manifests have admissionReviewVersions: [v1]: pkg/operator/assets/bindata.go lines 278–281, 323–326, 367–370, 496–499, 640–643, 709–712, 808–811; and config/hiveadmission/*-webhook.yaml show the same.
Operator reads these as admregv1.ValidatingWebhookConfiguration objects: pkg/operator/hive/hiveadmission.go lines 167–171.

If you’d like, I can also dig into the operator reconcile to verify it patches admissionReviewVersions on existing resources during upgrades.

2uasimojo · 2026-03-25T21:44:51Z

@coderabbitai Well, each CI run builds the code from scratch, installs a fresh cluster, and deploys the webhook configs from bindata.go, so there should be zero chance anything is stale. The test cluster gets torn down immediately, so I can't query the VWHCs live, but we do save several of them, e.g. this one and you can see it lists admissionReviewVersions: ["v1"] as expected. I checked all the others and they're the same.

Interestingly, e2e-pool passed here, whereas it encountered this error in #2880.

What say you to that?

coderabbitai · 2026-03-25T21:47:01Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find the webhook HTTP server handler - how it reads and writes AdmissionReview
rg -n --glob '!vendor/**' "ServeHTTP\|webhook.*[Hh]andler\|http\.HandlerFunc\|HandleFunc\|Mux\|mux\b\|ListenAndServe\|admission.*[Ss]erver\|[Ss]erver.*admission" --type go -C4

echo "---DECODER---"
# Look for how AdmissionReview is decoded from the HTTP request body
rg -n --glob '!vendor/**' "AdmissionReview\|Decode\b\|UnmarshalJSON\|json\.Unmarshal\|runtime\.Decode\|Codec\b" --type go -C3 | head -300

echo "---SCHEME CODEC---"
# Look for how the scheme codec is used with AdmissionReview
rg -n --glob '!vendor/**' "hive_scheme\|admissionScheme\|Codec\|CodecForVersions\|LegacyCodec\|UniversalDeserializer" --type go -C3

Length of output: 32

🏁 Script executed:

#!/bin/bash
# Find webhook server / HTTP handler files (avoid vendor)
fd -e go -E vendor | xargs grep -l "ServeHTTP\|ListenAndServe\|HandleFunc\|AdmissionReview" 2>/dev/null

echo "---SCHEME FILE---"
cat pkg/util/scheme/scheme.go

echo "---WEBHOOK SERVER FILES---"
# Find the webhook server entry point
fd -e go -E vendor | xargs grep -l "webhook" 2>/dev/null | grep -v test | grep -v "_test.go"

echo "---E2E ADMISSION TEST---"
cat test/e2e/postdeploy/admission/admission_test.go

echo "---HIVE ADMISSION MAIN/SERVER---"
fd -e go -E vendor -x grep -l "AdmissionReview\|webhook.*[Ss]erver\|hiveadmission" {} \; 2>/dev/null | head -20

Length of output: 8158

🏁 Script executed:

#!/bin/bash
echo "---HIVEADMISSION MAIN---"
cat cmd/hiveadmission/main.go

echo "---TESTDATA FILES---"
fd -e json . test/e2e/postdeploy/admission/testdata/ 2>/dev/null | xargs ls 2>/dev/null || find test/e2e/postdeploy/admission -name "*.json" 2>/dev/null
find test/e2e/postdeploy/admission -name "*.json" -exec echo "=== {} ===" \; -exec cat {} \; 2>/dev/null

echo "---ANY REMAINING V1BETA1 IN NON-VENDOR GO FILES---"
grep -rn "v1beta1" --include="*.go" --exclude-dir=vendor . 2>/dev/null | grep -v "_test.go" | head -40

echo "---ANY REMAINING V1BETA1 IN YAML CONFIGS---"
grep -rn "v1beta1" --include="*.yaml" --exclude-dir=vendor . 2>/dev/null | head -40

echo "---ANY V1BETA1 IN TESTDATA---"
grep -rn "v1beta1" test/ --include="*.json" 2>/dev/null

Length of output: 9502

@2uasimojo Found it! The three testdata JSON fixture files for the e2e admission test were never updated — they all still declare "apiVersion": "admission.k8s.io/v1beta1":

test/e2e/postdeploy/admission/testdata/hiveadmission-review-failure.json
test/e2e/postdeploy/admission/testdata/hiveadmission-review-success.json
test/e2e/postdeploy/admission/testdata/hiveadmission-review-update-failure.json

The TestAdmission e2e test posts these raw JSON blobs directly to the admission APIService endpoint. The server's decoder is built from scheme.GetScheme() which now only has admissionv1 registered (no v1beta1). When the server receives a body with apiVersion: admission.k8s.io/v1beta1, it can no longer decode/dispatch the request — same symptom as before, different root cause.

The fix is a one-line change in each file:

-  "apiVersion": "admission.k8s.io/v1beta1",
+  "apiVersion": "admission.k8s.io/v1",

Applied to all three files. For example, hiveadmission-review-failure.json becomes:

{
  "apiVersion": "admission.k8s.io/v1",
  "kind": "AdmissionReview",
  "request": {
    "kind": {
      "group": "hive.openshift.io",
      "version": "v1",
      "kind": "DNSZone"
    },
    "resource": {
      "group": "hive.openshift.io",
      "version": "v1",
      "resource": "dnszones"
    },
    "operation": "CREATE",
    "object": {
      "metadata": {
        "name": "twiest"
      },
      "spec": {
        "zone": "this_is_invalid_dns"
      }
    }
  }
}

As for why e2e-pool passes here but failed in #2880: e2e-pool doesn't exercise TestAdmission (the fixture-file path), so it's unaffected by this particular bug. Its failure in #2880 was a separate pre-existing issue, and it passes in #2881 likely because this PR's v1 migration fixed whatever the pool tests were tripping over there.

coderabbitai · 2026-03-25T21:48:07Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find the webhook HTTP handler that receives and decodes AdmissionReview requests
rg -n "ServeHTTP\|http\.HandleFunc\|HandleAdmission\|admissionHandler\|webhookServer\|router\|mux\b" --type go -g '!vendor/**' -C4

echo "---"

# How is the incoming AdmissionReview decoded from the HTTP body?
rg -n "Decode\|Unmarshal\|NewDecoder\|AdmissionReview" --type go -g '!vendor/**' -g '!*_test.go' -C4 | head -200

echo "---"

# What does the e2e admission test actually do?
cat test/e2e/postdeploy/admission/admission_test.go 2>/dev/null || fd -a 'admission_test.go' --type f | head -5

echo "---"

# What scheme/codec is used for admission webhook request parsing in Hive?
rg -n "scheme\|codec\|Codec\|runtime\.Codec\|Serializer" --type go -g '!vendor/**' -g '!*_test.go' -C3 | head -200

echo "---"

# Is there a webhook HTTP handler that checks Content-Type or version of incoming request?
rg -n "Content-Type\|application/json\|GroupVersionKind\|GVK\|TypeMeta\|apiVersion" --type go -g '!vendor/**' -g '!*_test.go' | grep -v "//.*apiVersion" | head -80

Length of output: 2847

The e2e test reads JSON testdata files and posts them as unstructured.Unstructured via the dynamic client — those files likely still declare apiVersion: admission.k8s.io/v1beta1. Let me verify:

🏁 Script executed:

#!/bin/bash
# Find the testdata JSON files used by the admission e2e test
fd -t f 'hiveadmission-review.*\.json' 
echo "---"
# Also check any testdata dirs under admission e2e
find test/e2e -name "*.json" 2>/dev/null | xargs ls -la 2>/dev/null
echo "---"
# Cat them all
find test/e2e -name "hiveadmission-review*.json" 2>/dev/null | sort | while read f; do
  echo "=== $f ==="
  cat "$f"
  echo ""
done

Length of output: 2412

@2uasimojo Found it! All three testdata JSON files were missed in this PR — they still declare "apiVersion": "admission.k8s.io/v1beta1":

test/e2e/postdeploy/admission/testdata/hiveadmission-review-failure.json
test/e2e/postdeploy/admission/testdata/hiveadmission-review-success.json
test/e2e/postdeploy/admission/testdata/hiveadmission-review-update-failure.json

The e2e test reads those files as unstructured.Unstructured and POSTs them verbatim to the webhook endpoint via the dynamic client. Because the payload itself says v1beta1, that's what the server receives and responds with — completely independent of what the VWHC advertises. Since pkg/util/scheme/scheme.go now only registers admissionv1 (not v1beta1), the server can no longer properly handle that v1beta1 body, and the subsequent scheme.Convert(result, reviewResult, nil) into *admissionv1.AdmissionReview breaks.

The fix is a one-liner in each of the three files:

-  "apiVersion": "admission.k8s.io/v1beta1",
+  "apiVersion": "admission.k8s.io/v1",

That also explains the e2e-pool asymmetry: e2e-pool tests ClusterPool admission through a different path that doesn't touch these testdata files, so it passes cleanly.

2uasimojo · 2026-03-25T22:27:26Z

Gah! Thanks @coderabbitai -- I caught that in #2880 but missed it here!

Fixing.

coderabbitai · 2026-03-25T22:27:41Z

Tip

For best results, initiate chat on the files or code changes.

@2uasimojo No worries — easy to miss a few static testdata files when the main focus is on Go source and YAML manifests! Once you bump those three files to "apiVersion": "admission.k8s.io/v1" the e2e TestAdmission should pass cleanly. Good luck with the re-run! 🚀

openshift-ci-robot · 2026-03-25T22:30:00Z

@2uasimojo: This pull request references HIVE-3132 which is a valid jira issue.

Details

In response to this:

v1beta1 has been deprecated, and the last k8s that didn't support v1 (1.16, I think) has been "out of support", for long enough that we should be safe to move fully to v1 now.

Summary by CodeRabbit

Chores

Updated all Hive admission webhooks to use the Kubernetes admission API version v1 (replacing v1beta1) across webhook configurations, implementations, unit tests, and end-to-end test fixtures.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

2uasimojo · 2026-03-26T14:26:16Z

Test name Commit Details Required Rerun command
ci/prow/e2e-pool f03a4c0 link true /test e2e-pool

Missed the clusterpool vwhc after #2879 merged.

openshift-ci-robot · 2026-03-26T14:27:35Z

@2uasimojo: This pull request references HIVE-3132 which is a valid jira issue.

Details

In response to this:

v1beta1 has been deprecated, and the last k8s that didn't support v1 (1.16, I think) has been "out of support", for long enough that we should be safe to move fully to v1 now.

Summary by CodeRabbit

Chores

Migrated Hive admission components to the stable Kubernetes admission API v1 (replacing v1beta1) across webhook configurations, runtime registrations, webhook implementations, unit tests, and end-to-end test fixtures to ensure compatibility with the current AdmissionReview API.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

pkg/operator/assets/bindata.go (1)
361-389: ⚠️ Potential issue | 🔴 Critical

Critical: bindata.go out of sync—clusterpool webhook still contains stale v1beta1.

The source file config/hiveadmission/clusterpool-webhook.yaml has been correctly updated to admissionReviewVersions: [v1], but pkg/operator/assets/bindata.go was not regenerated. Line 369 still contains the stale v1beta1 value, while all other webhooks in bindata.go correctly show v1.

Since bindata.go is auto-generated (@generated marker), you must regenerate it from the source YAML files. The clusterpool webhook will otherwise advertise v1beta1 support to the API server despite the Go implementation expecting v1 admission requests.

Regenerate the bindata.go file using the appropriate go-bindata command (typically go generate ./... or similar tooling as defined in your Makefile/build scripts).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/operator/assets/bindata.go` around lines 361 - 389, The embedded YAML in
variable _configHiveadmissionClusterpoolWebhookYaml in
pkg/operator/assets/bindata.go still advertises admissionReviewVersions:
v1beta1; regenerate the generated bindata to sync with the updated source
config/hiveadmission/clusterpool-webhook.yaml (which uses v1). Run the project’s
generation step (e.g. the Makefile/build script or go generate ./... / the
repo-specific go-bindata invocation) to recreate bindata.go so the
_configHiveadmissionClusterpoolWebhookYaml byte slice contains
admissionReviewVersions: [v1]; verify the file is updated and commit the
regenerated file.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@pkg/operator/assets/bindata.go`:
- Around line 361-389: The embedded YAML in variable
_configHiveadmissionClusterpoolWebhookYaml in pkg/operator/assets/bindata.go
still advertises admissionReviewVersions: v1beta1; regenerate the generated
bindata to sync with the updated source
config/hiveadmission/clusterpool-webhook.yaml (which uses v1). Run the project’s
generation step (e.g. the Makefile/build script or go generate ./... / the
repo-specific go-bindata invocation) to recreate bindata.go so the
_configHiveadmissionClusterpoolWebhookYaml byte slice contains
admissionReviewVersions: [v1]; verify the file is updated and commit the
regenerated file.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7f19f9ab-269c-4305-b3d9-b94dcb04c7f3

📥 Commits

Reviewing files that changed from the base of the PR and between f03a4c0 and 2c0a2b9.

📒 Files selected for processing (31)

config/hiveadmission/clusterdeployment-webhook.yaml
config/hiveadmission/clusterimageset-webhook.yaml
config/hiveadmission/clusterpool-webhook.yaml
config/hiveadmission/clusterprovision-webhook.yaml
config/hiveadmission/dnszones-webhook.yaml
config/hiveadmission/machinepool-webhook.yaml
config/hiveadmission/selectorsyncset-webhook.yaml
config/hiveadmission/syncset-webhook.yaml
pkg/operator/assets/bindata.go
pkg/util/scheme/scheme.go
pkg/validating-webhooks/hive/v1/clusterdeployment_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterdeployment_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/clusterdeploymentcustomization_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterimageset_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterimageset_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/clusterpool_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterpool_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/clusterprovision_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterprovision_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/dnszone_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/dnszone_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/machinepool_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/machinepool_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/selector_syncset_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/selector_syncset_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/syncset_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/syncset_validating_admission_hook_test.go
test/e2e/postdeploy/admission/admission_test.go
test/e2e/postdeploy/admission/testdata/hiveadmission-review-failure.json
test/e2e/postdeploy/admission/testdata/hiveadmission-review-success.json
test/e2e/postdeploy/admission/testdata/hiveadmission-review-update-failure.json

✅ Files skipped from review due to trivial changes (15)

config/hiveadmission/clusterprovision-webhook.yaml
config/hiveadmission/clusterimageset-webhook.yaml
test/e2e/postdeploy/admission/testdata/hiveadmission-review-failure.json
test/e2e/postdeploy/admission/admission_test.go
config/hiveadmission/dnszones-webhook.yaml
config/hiveadmission/clusterpool-webhook.yaml
config/hiveadmission/syncset-webhook.yaml
test/e2e/postdeploy/admission/testdata/hiveadmission-review-update-failure.json
pkg/util/scheme/scheme.go
config/hiveadmission/clusterdeployment-webhook.yaml
pkg/validating-webhooks/hive/v1/machinepool_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/clusterpool_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/selector_syncset_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/clusterdeployment_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/syncset_validating_admission_hook_test.go

🚧 Files skipped from review as they are similar to previous changes (13)

config/hiveadmission/machinepool-webhook.yaml
pkg/validating-webhooks/hive/v1/clusterimageset_validating_admission_hook_test.go
config/hiveadmission/selectorsyncset-webhook.yaml
pkg/validating-webhooks/hive/v1/clusterprovision_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/syncset_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterdeployment_validating_admission_hook.go
test/e2e/postdeploy/admission/testdata/hiveadmission-review-success.json
pkg/validating-webhooks/hive/v1/dnszone_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/clusterdeploymentcustomization_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterpool_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/dnszone_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterprovision_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterimageset_validating_admission_hook.go

openshift-ci-robot · 2026-03-26T14:41:17Z

@2uasimojo: This pull request references HIVE-3132 which is a valid jira issue.

Details

In response to this:

v1beta1 has been deprecated, and the last k8s that didn't support v1 (1.16, I think) has been "out of support", for long enough that we should be safe to move fully to v1 now.

Summary by CodeRabbit

Chores

Migrated Hive admission components to the stable Kubernetes admission API v1 (replacing v1beta1) across validating webhook configurations, runtime scheme registration, webhook handlers, unit tests, and end-to-end test fixtures to ensure AdmissionReview v1 compatibility and smoother cluster integration.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/operator/assets/bindata.go`:
- Around line 279-280: Update the webhook assets and scheme to support a staged
migration: in the generated asset that contains the admission webhook config
(the block with admissionReviewVersions in pkg/operator/assets/bindata.go) add
v1beta1 so it advertises ["v1","v1beta1"], and in the scheme initialization
(where admission.k8s.io/v1 is registered in pkg/util/scheme/scheme.go) call
admissionv1beta1.AddToScheme() as well; this lets old pods accept v1beta1 during
a rolling upgrade so v1-only pods and v1beta1-expecting pods coexist for one
release, after which you can remove admissionv1beta1.AddToScheme() and drop
v1beta1 from admissionReviewVersions in a subsequent release.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bf074f98-47f0-4ede-ba2a-675f864baab9

📥 Commits

Reviewing files that changed from the base of the PR and between 2c0a2b9 and d72cf4c.

📒 Files selected for processing (31)

config/hiveadmission/clusterdeployment-webhook.yaml
config/hiveadmission/clusterimageset-webhook.yaml
config/hiveadmission/clusterpool-webhook.yaml
config/hiveadmission/clusterprovision-webhook.yaml
config/hiveadmission/dnszones-webhook.yaml
config/hiveadmission/machinepool-webhook.yaml
config/hiveadmission/selectorsyncset-webhook.yaml
config/hiveadmission/syncset-webhook.yaml
pkg/operator/assets/bindata.go
pkg/util/scheme/scheme.go
pkg/validating-webhooks/hive/v1/clusterdeployment_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterdeployment_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/clusterdeploymentcustomization_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterimageset_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterimageset_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/clusterpool_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterpool_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/clusterprovision_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterprovision_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/dnszone_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/dnszone_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/machinepool_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/machinepool_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/selector_syncset_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/selector_syncset_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/syncset_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/syncset_validating_admission_hook_test.go
test/e2e/postdeploy/admission/admission_test.go
test/e2e/postdeploy/admission/testdata/hiveadmission-review-failure.json
test/e2e/postdeploy/admission/testdata/hiveadmission-review-success.json
test/e2e/postdeploy/admission/testdata/hiveadmission-review-update-failure.json

✅ Files skipped from review due to trivial changes (17)

config/hiveadmission/clusterprovision-webhook.yaml
test/e2e/postdeploy/admission/testdata/hiveadmission-review-failure.json
config/hiveadmission/clusterimageset-webhook.yaml
test/e2e/postdeploy/admission/admission_test.go
test/e2e/postdeploy/admission/testdata/hiveadmission-review-success.json
pkg/validating-webhooks/hive/v1/machinepool_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/syncset_validating_admission_hook_test.go
config/hiveadmission/clusterdeployment-webhook.yaml
config/hiveadmission/selectorsyncset-webhook.yaml
pkg/validating-webhooks/hive/v1/clusterimageset_validating_admission_hook_test.go
test/e2e/postdeploy/admission/testdata/hiveadmission-review-update-failure.json
pkg/validating-webhooks/hive/v1/clusterprovision_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/clusterdeployment_validating_admission_hook_test.go
config/hiveadmission/clusterpool-webhook.yaml
pkg/validating-webhooks/hive/v1/selector_syncset_validating_admission_hook_test.go
pkg/util/scheme/scheme.go
pkg/validating-webhooks/hive/v1/clusterdeploymentcustomization_validating_admission_hook.go

🚧 Files skipped from review as they are similar to previous changes (11)

config/hiveadmission/dnszones-webhook.yaml
config/hiveadmission/machinepool-webhook.yaml
config/hiveadmission/syncset-webhook.yaml
pkg/validating-webhooks/hive/v1/clusterpool_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/syncset_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterpool_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/dnszone_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/clusterdeployment_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/dnszone_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/selector_syncset_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/machinepool_validating_admission_hook.go

pkg/operator/assets/bindata.go

2uasimojo · 2026-03-26T16:38:44Z

/assign @suhanime

2uasimojo · 2026-03-27T17:52:36Z

config/hiveadmission/clusterdeploymentcustomization-webhook.yaml

 - name: clusterdeploymentcustomizationvalidators.admission.hive.openshift.io
  admissionReviewVersions:
-  - v1beta1
+  - v1


openshift-ci-robot · 2026-03-27T17:54:41Z

@2uasimojo: This pull request references HIVE-3132 which is a valid jira issue.

Details

In response to this:

v1beta1 has been deprecated, and the last k8s that didn't support v1 (1.16, I think) has been "out of support", for long enough that we should be safe to move fully to v1 now.

Summary by CodeRabbit

Chores

Migrated admission integrations to Kubernetes AdmissionReview v1 for broader cluster compatibility.

New Features

Added validation support for ClusterDeploymentCustomization resources.

Bug Fixes / Behavior

ClusterPool webhook no longer runs on DELETE (now only CREATE/UPDATE), reducing unnecessary validation on deletes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

2uasimojo · 2026-03-27T17:54:46Z

Now stacked on #2882, which should merge after retesting.

coderabbitai

🧹 Nitpick comments (1)

pkg/validating-webhooks/hive/v1/clusterdeploymentcustomization_validating_admission_hook.go (1)

22-22: Please pin this fix with a regression test.

A typo here turns the webhook into a silent no-op, so a small Validate or shouldValidate unit test for clusterdeploymentcustomizations would be worth adding.

Also applies to: 107-129

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@pkg/validating-webhooks/hive/v1/clusterdeploymentcustomization_validating_admission_hook.go`
at line 22, The constant clusterDeploymentCustomizationResource is mis-typed
causing the webhook to no-op; fix the constant value to exactly match the
resource name your shouldValidate/Validate logic expects and any
GroupVersionResource checks, then add a regression unit test that constructs an
admission request for "clusterdeploymentcustomizations" (the exact expected
resource) and asserts shouldValidate(...) returns true and Validate(...) runs
non-trivially (returns expected error/allow decision). Update tests covering the
logic in the same area referenced (the code around shouldValidate/Validate,
previously noted lines ~107-129) so the test will fail if the resource string
regresses again.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@pkg/validating-webhooks/hive/v1/clusterdeploymentcustomization_validating_admission_hook.go`:
- Line 22: The constant clusterDeploymentCustomizationResource is mis-typed
causing the webhook to no-op; fix the constant value to exactly match the
resource name your shouldValidate/Validate logic expects and any
GroupVersionResource checks, then add a regression unit test that constructs an
admission request for "clusterdeploymentcustomizations" (the exact expected
resource) and asserts shouldValidate(...) returns true and Validate(...) runs
non-trivially (returns expected error/allow decision). Update tests covering the
logic in the same area referenced (the code around shouldValidate/Validate,
previously noted lines ~107-129) so the test will fail if the resource string
regresses again.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bb4ce6e5-db28-4170-91d2-fdb005b7eaba

📥 Commits

Reviewing files that changed from the base of the PR and between d72cf4c and 3569285.

📒 Files selected for processing (34)

.coderabbit.yaml
config/hiveadmission/clusterdeployment-webhook.yaml
config/hiveadmission/clusterdeploymentcustomization-webhook.yaml
config/hiveadmission/clusterimageset-webhook.yaml
config/hiveadmission/clusterpool-webhook.yaml
config/hiveadmission/clusterprovision-webhook.yaml
config/hiveadmission/dnszones-webhook.yaml
config/hiveadmission/machinepool-webhook.yaml
config/hiveadmission/selectorsyncset-webhook.yaml
config/hiveadmission/syncset-webhook.yaml
pkg/operator/assets/bindata.go
pkg/operator/hive/hiveadmission.go
pkg/util/scheme/scheme.go
pkg/validating-webhooks/hive/v1/clusterdeployment_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterdeployment_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/clusterdeploymentcustomization_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterimageset_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterimageset_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/clusterpool_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterpool_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/clusterprovision_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterprovision_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/dnszone_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/dnszone_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/machinepool_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/machinepool_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/selector_syncset_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/selector_syncset_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/syncset_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/syncset_validating_admission_hook_test.go
test/e2e/postdeploy/admission/admission_test.go
test/e2e/postdeploy/admission/testdata/hiveadmission-review-failure.json
test/e2e/postdeploy/admission/testdata/hiveadmission-review-success.json
test/e2e/postdeploy/admission/testdata/hiveadmission-review-update-failure.json

✅ Files skipped from review due to trivial changes (18)

config/hiveadmission/clusterimageset-webhook.yaml
config/hiveadmission/clusterprovision-webhook.yaml
config/hiveadmission/machinepool-webhook.yaml
config/hiveadmission/syncset-webhook.yaml
config/hiveadmission/clusterdeployment-webhook.yaml
config/hiveadmission/dnszones-webhook.yaml
test/e2e/postdeploy/admission/testdata/hiveadmission-review-success.json
test/e2e/postdeploy/admission/testdata/hiveadmission-review-update-failure.json
test/e2e/postdeploy/admission/admission_test.go
config/hiveadmission/selectorsyncset-webhook.yaml
config/hiveadmission/clusterdeploymentcustomization-webhook.yaml
pkg/validating-webhooks/hive/v1/clusterimageset_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/clusterprovision_validating_admission_hook_test.go
.coderabbit.yaml
test/e2e/postdeploy/admission/testdata/hiveadmission-review-failure.json
pkg/validating-webhooks/hive/v1/clusterdeployment_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterdeployment_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/clusterpool_validating_admission_hook.go

🚧 Files skipped from review as they are similar to previous changes (9)

pkg/util/scheme/scheme.go
pkg/validating-webhooks/hive/v1/clusterpool_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/dnszone_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/selector_syncset_validating_admission_hook_test.go
config/hiveadmission/clusterpool-webhook.yaml
pkg/validating-webhooks/hive/v1/syncset_validating_admission_hook_test.go
pkg/validating-webhooks/hive/v1/selector_syncset_validating_admission_hook.go
pkg/validating-webhooks/hive/v1/clusterimageset_validating_admission_hook.go
pkg/operator/assets/bindata.go

suhanime · 2026-03-27T18:47:04Z

I have a few questions:

Can we switch machine api to also use v1? Or does it still have to import "github.com/openshift/api/machine/v1beta1"? Same for autoscaling and other apis we import. On that topic, we have known contract configured for AgentClusterInstall with v1beta1 version.
Might want to check docs, I saw atleast the syncset ones referencing v1beta1 in their samples. Hive consumers often refer to them first, we should keep them up-to-date

2uasimojo · 2026-03-27T19:24:46Z

Can we switch machine api to also use v1? Or does it still have to import "github.com/openshift/api/machine/v1beta1"? Same for autoscaling and other apis we import. On that topic, we have known contract configured for AgentClusterInstall with v1beta1 version.

Might want to check docs, I saw atleast the syncset ones referencing v1beta1 in their samples. Hive consumers often refer to them first, we should keep them up-to-date

This PR only addresses k8s.io/api/admission/v1beta1 => k8s.io/api/admission/v1. Machine API is not involved. Could we update that one? Perhaps, but not here.

What AgentClusterInstall piece are you referring to?

What syncset sample are you referring to?

Again, suspect these ⬆️ are talking about different APIs.

suhanime · 2026-03-27T21:06:44Z

What AgentClusterInstall piece are you referring to?

Here, but not related to Admission so we can ignore

What syncset sample are you referring to?

From here. I apologize, most of my mentions had nothing to do with admission v1

/lgtm

suhanime · 2026-03-27T21:16:19Z

/lgtm

v1beta1 has been deprecated, and the last k8s that didn't support v1 (1.16, I think) has been "out of support", for long enough that we should be safe to move fully to v1 now.

suhanime · 2026-03-27T22:47:48Z

/lgtm

suhanime · 2026-03-29T19:51:50Z

/lgtm

openshift-ci · 2026-03-29T19:52:22Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 2uasimojo, suhanime

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [2uasimojo,suhanime]

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

openshift-ci · 2026-03-29T21:54:10Z

@2uasimojo: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Mar 25, 2026

openshift-ci bot requested review from jstuever and suhanime March 25, 2026 19:47

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 25, 2026

2uasimojo mentioned this pull request Mar 25, 2026

HIVE-3132: admissionReviewVersions += v1 #2880

Closed

coderabbitai bot reviewed Mar 25, 2026

View reviewed changes

pkg/validating-webhooks/hive/v1/clusterdeploymentcustomization_validating_admission_hook.go Show resolved Hide resolved

2uasimojo force-pushed the HIVE-3132/admission-v1 branch from f8d8e86 to f03a4c0 Compare March 25, 2026 22:27

2uasimojo force-pushed the HIVE-3132/admission-v1 branch from f03a4c0 to 2c0a2b9 Compare March 26, 2026 14:24

coderabbitai bot reviewed Mar 26, 2026

View reviewed changes

2uasimojo force-pushed the HIVE-3132/admission-v1 branch 2 times, most recently from 7cee0ee to d72cf4c Compare March 26, 2026 14:37

coderabbitai bot reviewed Mar 26, 2026

View reviewed changes

pkg/operator/assets/bindata.go Show resolved Hide resolved

2uasimojo mentioned this pull request Mar 26, 2026

HIVE-3133: VWHC for ClusterDeploymentCustomization #2882

Merged

openshift-ci bot assigned suhanime Mar 26, 2026

2uasimojo force-pushed the HIVE-3132/admission-v1 branch from d72cf4c to 3569285 Compare March 27, 2026 17:51

2uasimojo commented Mar 27, 2026

View reviewed changes

coderabbitai bot reviewed Mar 27, 2026

View reviewed changes

HIVE-3132: Switch to Admission v1

5a53841

v1beta1 has been deprecated, and the last k8s that didn't support v1 (1.16, I think) has been "out of support", for long enough that we should be safe to move fully to v1 now.

2uasimojo force-pushed the HIVE-3132/admission-v1 branch from 3569285 to 5a53841 Compare March 27, 2026 22:46

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 29, 2026

openshift-merge-bot bot merged commit aec89c6 into openshift:master Mar 29, 2026
25 checks passed

Conversation

2uasimojo commented Mar 25, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

openshift-ci-robot commented Mar 25, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

openshift-ci-robot commented Mar 25, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

2uasimojo commented Mar 25, 2026

Uh oh!

coderabbitai bot commented Mar 25, 2026

Uh oh!

2uasimojo commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Mar 25, 2026

Uh oh!

coderabbitai bot commented Mar 25, 2026

Uh oh!

2uasimojo commented Mar 25, 2026

Uh oh!

coderabbitai bot commented Mar 25, 2026

Uh oh!

openshift-ci-robot commented Mar 25, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

2uasimojo commented Mar 26, 2026

Uh oh!

openshift-ci-robot commented Mar 26, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Mar 26, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

2uasimojo commented Mar 26, 2026

Uh oh!

2uasimojo Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Mar 27, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

2uasimojo commented Mar 27, 2026

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

2uasimojo commented Mar 25, 2026 •

edited by coderabbitai bot

Loading

openshift-ci-robot commented Mar 25, 2026 •

edited by openshift-ci bot

Loading

coderabbitai bot commented Mar 25, 2026 •

edited

Loading

openshift-ci-robot commented Mar 25, 2026 •

edited by openshift-ci bot

Loading

codecov bot commented Mar 25, 2026 •

edited

Loading

2uasimojo commented Mar 25, 2026 •

edited

Loading

openshift-ci-robot commented Mar 25, 2026 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Mar 26, 2026 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Mar 26, 2026 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Mar 27, 2026 •

edited by openshift-ci bot

Loading

suhanime commented Mar 27, 2026 •

edited

Loading