Skip to content

Commit cc7167b

Browse files
committed
HIVE-3133: VWHC for ClusterDeploymentCustomization
This was missed in #1672. Side quest: Dropping `DELETE` from the ClusterPool VWHC added in #2879. It didn't have functional impact, but would have been wastefully sending Delete requests to our hiveadmission service. Also adding some coderabbit config intended to catch this kind of miss in the future.
1 parent 8296ffc commit cc7167b

6 files changed

Lines changed: 149 additions & 50 deletions

File tree

.coderabbit.yaml

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,29 @@ reviews:
3232
of these files entirely, as reviewing code changes in kubernetes would be wildly out of scope.
3333
The one exception to this is `vendor/github.com/openshift/hive/**`, which should be a direct copy
3434
of the `apis` folder found in the root of the repo, referenced by the CRD generation guidance above.
35+
- path: config/hiveadmission/*-webhook.yaml
36+
instructions: |-
37+
These are configuration files for admission webhooks for hive CRs. Files named *-webhook.yaml
38+
contain ValidatingWebhookConfiguration (or, hypothetically, MutatingWebhookConfiguration -- but
39+
we don't have any of those at this time) manifests. Please validate:
40+
- They have been built into pkg/operator/assets/bindata.go (today our `verify` CI job should
41+
also check this).
42+
- They are all in the `webhookAssets` list in pkg/operator/hive/hiveadmission.go -- else they
43+
will not be deployed!
44+
- Each has a corresponding implementation in pkg/validating-webhooks/hive/v1.
45+
- When any file in this directory is added or modified, also cross-check ALL other
46+
existing *-webhook.yaml files in config/hiveadmission/ to verify their operations
47+
are consistent with their implementations. Do not limit the audit to only the file(s)
48+
in the diff.
49+
- The `Operation`s in `.webhooks[].rules[].operations` must exactly match the
50+
operations handled by explicit `case` branches in the implementation's `Validate()`
51+
switch. A `default: Allowed: true` branch does NOT count as handling an operation.
52+
If an operation appears in the YAML but has no corresponding `case`, flag it; and
53+
vice versa.
54+
- path: pkg/validating-webhooks/hive/v1/*_admission_hook.go
55+
instructions: |-
56+
These are implementations for validating (or, hypothetically, mutating -- but we don't have any
57+
of those at this time) webhooks handled by our hiveadmission service. Mirroring the above checks
58+
for config/hiveadmission/*-webhook.yaml, please validate that each has a corresponding
59+
ValidatingWebhookConfiguration (or MutatingWebhookConfiguration) manifest under config/hiveadmission/,
60+
built into bindata, with matching `Operation`s, and is listed for installation in hiveadmission.go.
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
apiVersion: admissionregistration.k8s.io/v1
3+
kind: ValidatingWebhookConfiguration
4+
metadata:
5+
name: clusterdeploymentcustomizationvalidators.admission.hive.openshift.io
6+
webhooks:
7+
- name: clusterdeploymentcustomizationvalidators.admission.hive.openshift.io
8+
admissionReviewVersions:
9+
- v1beta1
10+
clientConfig:
11+
service:
12+
# reach the webhook via the registered aggregated API
13+
namespace: default
14+
name: kubernetes
15+
path: /apis/admission.hive.openshift.io/v1/clusterdeploymentcustomizationvalidators
16+
rules:
17+
- operations:
18+
- CREATE
19+
- UPDATE
20+
apiGroups:
21+
- hive.openshift.io
22+
apiVersions:
23+
- v1
24+
resources:
25+
- clusterdeploymentcustomizations
26+
failurePolicy: Fail
27+
sideEffects: None

config/hiveadmission/clusterpool-webhook.yaml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ webhooks:
1717
- operations:
1818
- CREATE
1919
- UPDATE
20-
- DELETE
2120
apiGroups:
2221
- hive.openshift.io
2322
apiVersions:

pkg/operator/assets/bindata.go

Lines changed: 93 additions & 47 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/operator/hive/hiveadmission.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ const (
5050

5151
var webhookAssets = []string{
5252
"config/hiveadmission/clusterdeployment-webhook.yaml",
53+
"config/hiveadmission/clusterdeploymentcustomization-webhook.yaml",
5354
"config/hiveadmission/clusterimageset-webhook.yaml",
5455
"config/hiveadmission/clusterpool-webhook.yaml",
5556
"config/hiveadmission/clusterprovision-webhook.yaml",

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import (
1919
const (
2020
clusterDeploymentCustomizationGroup = "hive.openshift.io"
2121
clusterDeploymentCustomizationVersion = "v1"
22-
clusterDeploymentCustomizationResource = "clusterdeploymentcustomization"
22+
clusterDeploymentCustomizationResource = "clusterdeploymentcustomizations"
2323

2424
clusterDeploymentCustomizationAdmissionGroup = "admission.hive.openshift.io"
2525
clusterDeploymentCustomizationAdmissionVersion = "v1"
@@ -124,7 +124,7 @@ func (a *ClusterDeploymentCustomizationValidatingAdmissionHook) shouldValidate(a
124124
}
125125

126126
if admissionSpec.Resource.Resource != clusterDeploymentCustomizationResource {
127-
contextLogger.Info("Returning False, it's our group and version, but not the right resource")
127+
contextLogger.WithField("resource", admissionSpec.Resource.Resource).Info("Returning False, it's our group and version, but not the right resource")
128128
return false
129129
}
130130

0 commit comments

Comments
 (0)