Skip to content

Commit 1602b76

Browse files
stuggiclaude
stuggi
and
claude
committed
Enable conversion webhook activation in the bindata pipeline
Generalise sync-bindata.sh and the openstack-operator controller to activate the webhook server and stage the serving resources for any service operator that carries a conversion webhook in its CRD, without hardcoding operator names. sync-bindata.sh: - Add write_webhook_serving_resources: generates only the webhook Service + Certificate (no admission webhook configs). Used for operators whose CRs are created by the OpenStackControlPlane reconciler, so direct-user admission webhooks are not needed; the conversion endpoint is. - Refactor write_webhooks to call write_webhook_serving_resources for the serving resources and append admission configs via cat >>. Output for infra-operator and openstack-baremetal-operator is unchanged. - Add csv_has_conversion_crd: inspects the owned CRDs listed in a CSV and returns true if any carries spec.conversion.strategy: Webhook. Warns on stderr if an expected CRD file is absent. - Extend the webhook generation gate: infra/baremetal remain hardcoded (admission operators); any other operator whose CRD declares a conversion webhook now gets write_webhook_serving_resources called automatically (data-driven, keystone-first). openstack_controller.go: - Add webhookOperatorSet field to OpenStackReconciler, populated once in SetupWithManager by globbing bindata/operator/*-webhooks.yaml. Zero filesystem reads on subsequent reconciles. - ENABLE_WEBHOOKS default case: checks webhookOperatorSet instead of a hardcoded name. Conversion operators (e.g. keystone) flip to true once their webhook manifest is staged; infra/baremetal remain hardcoded in their switch cases. - Replace the static isWebhookEndpoint allowlist with webhookServiceNames, a method that derives endpoint names from webhookOperatorSet at runtime. Adding a new conversion operator requires only publishing the bundle and running make bindata — no code change. Jira: OSPRH-30210 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Martin Schuppert <mschuppert@redhat.com>
1 parent b703e9c commit 1602b76

2 files changed

Lines changed: 116 additions & 17 deletions

File tree

‎hack/sync-bindata.sh‎

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,8 @@ cat $CSV_FILENAME | $LOCAL_BINARIES/yq -r ".spec.webhookdefinitions.[] | select(
4444
}
4545

4646

47-
function write_webhooks {
48-
local CSV_FILENAME=$1
49-
local OPERATOR_NAME=$2
50-
51-
MUTATING_WEBHOOKS=$(extract_webhooks "$CSV_FILENAME" "$OPERATOR_NAME" "MutatingAdmissionWebhook")
52-
VALIDATING_WEBHOOKS=$(extract_webhooks "$CSV_FILENAME" "$OPERATOR_NAME" "ValidatingAdmissionWebhook")
47+
function write_webhook_serving_resources {
48+
local OPERATOR_NAME=$1
5349

5450
cat > operator/$OPERATOR_NAME-webhooks.yaml <<EOF_CAT
5551
apiVersion: v1
@@ -95,6 +91,21 @@ spec:
9591
algorithm: ECDSA
9692
size: 256
9793
secretName: $OPERATOR_NAME-webhook-server-cert
94+
EOF_CAT
95+
96+
}
97+
98+
99+
function write_webhooks {
100+
local CSV_FILENAME=$1
101+
local OPERATOR_NAME=$2
102+
103+
MUTATING_WEBHOOKS=$(extract_webhooks "$CSV_FILENAME" "$OPERATOR_NAME" "MutatingAdmissionWebhook")
104+
VALIDATING_WEBHOOKS=$(extract_webhooks "$CSV_FILENAME" "$OPERATOR_NAME" "ValidatingAdmissionWebhook")
105+
106+
write_webhook_serving_resources "$OPERATOR_NAME"
107+
108+
cat >> operator/$OPERATOR_NAME-webhooks.yaml <<EOF_CAT
98109
---
99110
apiVersion: admissionregistration.k8s.io/v1
100111
kind: MutatingWebhookConfiguration
@@ -133,6 +144,34 @@ EOF_CAT
133144

134145
}
135146

147+
148+
# Returns 0 if any CRD owned by the CSV has spec.conversion.strategy: Webhook
149+
function csv_has_conversion_crd {
150+
local CSV_FILE="$1"
151+
local CRD_LIST
152+
if ! CRD_LIST=$($LOCAL_BINARIES/yq -r '.spec.customresourcedefinitions.owned[]?.name' "$CSV_FILE"); then
153+
echo "ERROR: csv_has_conversion_crd: failed to parse CRD list from $CSV_FILE" >&2
154+
return 1
155+
fi
156+
while IFS= read -r CRD_FULLNAME; do
157+
[[ -z "$CRD_FULLNAME" ]] && continue
158+
local RESOURCE="${CRD_FULLNAME%%.*}"
159+
local GROUP="${CRD_FULLNAME#*.}"
160+
local CRD_FILE="manifests/${GROUP}_${RESOURCE}.yaml"
161+
if [[ -f "$CRD_FILE" ]]; then
162+
local STRATEGY
163+
if ! STRATEGY=$(cat "$CRD_FILE" | $LOCAL_BINARIES/yq -r '.spec.conversion.strategy // ""'); then
164+
echo "ERROR: csv_has_conversion_crd: failed to parse conversion strategy from $CRD_FILE" >&2
165+
return 1
166+
fi
167+
[[ "$STRATEGY" == "Webhook" ]] && return 0
168+
else
169+
echo "WARNING: csv_has_conversion_crd: CRD file not found: $CRD_FILE (owned by $CSV_FILE)" >&2
170+
fi
171+
done <<< "$CRD_LIST"
172+
return 1
173+
}
174+
136175
if ! BUNDLES="$(hack/pin-bundle-images.sh)"; then
137176
exit 1
138177
fi
@@ -168,6 +207,10 @@ for X in $(ls manifests/*clusterserviceversion.yaml); do
168207

169208
if [[ "$OPERATOR_NAME" == "infra-operator" || "$OPERATOR_NAME" == "openstack-baremetal-operator" ]]; then
170209
write_webhooks "$X" "$OPERATOR_NAME"
210+
elif csv_has_conversion_crd "$X"; then
211+
# Conversion operator: stage only the serving Service + Certificate.
212+
# Admission webhook configs are not registered (CRs managed by OSCP upstream webhook, not direct users).
213+
write_webhook_serving_resources "$OPERATOR_NAME"
171214
fi
172215

173216
# cert-manager and Service resources go into bindata/services/ for runtime

‎internal/controller/operator/openstack_controller.go‎

Lines changed: 67 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,9 @@ import (
5757
// OpenStackReconciler reconciles a OpenStack object
5858
type OpenStackReconciler struct {
5959
client.Client
60-
Scheme *runtime.Scheme
61-
Kclient kubernetes.Interface
60+
Scheme *runtime.Scheme
61+
Kclient kubernetes.Interface
62+
webhookOperatorSet map[string]bool // short op names with a bindata webhook manifest; populated once at startup
6263
}
6364

6465
// GetLogger returns a logger object with a prefix of "controller.name" and additional controller context fields
@@ -550,11 +551,58 @@ func containerImageMatch(instance *operatorv1beta1.OpenStack) bool {
550551
return false
551552
}
552553

553-
func isWebhookEndpoint(name string) bool {
554-
// NOTE: this is a static list for all operators with webhooks enabled
555-
endpointNames := []string{"openstack-operator-webhook-service", "infra-operator-webhook-service", "openstack-baremetal-operator-webhook-service"}
556-
for _, prefix := range endpointNames {
557-
if strings.HasPrefix(name, prefix) {
554+
// loadWebhookOperatorSet globs bindata/operator/*-webhooks.yaml once at startup and returns
555+
// a set of short operator names (e.g. "keystone", "infra") that have a webhook manifest.
556+
// Returns empty set on errors (safe fallback: no dynamic webhook operators configured).
557+
func loadWebhookOperatorSet(bindir string) map[string]bool {
558+
log := ctrl.Log.WithName("loadWebhookOperatorSet")
559+
result := map[string]bool{}
560+
operatorDir := filepath.Join(bindir, "operator")
561+
562+
// Validate bindata directory exists
563+
if _, err := os.Stat(operatorDir); os.IsNotExist(err) {
564+
log.Info("bindata operator directory not found, no dynamic webhook operators will be configured", "dir", operatorDir)
565+
return result
566+
} else if err != nil {
567+
log.Error(err, "failed to check bindata operator directory", "dir", operatorDir)
568+
return result
569+
}
570+
571+
// Glob webhook manifests
572+
files, err := filepath.Glob(filepath.Join(operatorDir, "*-webhooks.yaml"))
573+
if err != nil {
574+
log.Error(err, "failed to glob webhook manifests", "dir", operatorDir)
575+
return result
576+
}
577+
578+
if len(files) == 0 {
579+
log.Info("no webhook manifests found in bindata", "dir", operatorDir)
580+
return result
581+
}
582+
583+
for _, f := range files {
584+
base := filepath.Base(f)
585+
opName := strings.TrimSuffix(base, "-webhooks.yaml") // e.g. "keystone-operator"
586+
shortName := strings.TrimSuffix(opName, "-operator") // e.g. "keystone"
587+
result[shortName] = true
588+
}
589+
log.Info("loaded webhook operator set", "count", len(result), "operators", result)
590+
return result
591+
}
592+
593+
// webhookServiceNames returns the expected webhook endpoint service names derived from
594+
// the operator's cached webhook set, plus the openstack-operator itself.
595+
func (r *OpenStackReconciler) webhookServiceNames() []string {
596+
names := []string{"openstack-operator-webhook-service"}
597+
for shortName := range r.webhookOperatorSet {
598+
names = append(names, shortName+"-operator-webhook-service")
599+
}
600+
return names
601+
}
602+
603+
func isWebhookEndpoint(name string, serviceNames []string) bool {
604+
for _, svc := range serviceNames {
605+
if strings.HasPrefix(name, svc) {
558606
return true
559607
}
560608
}
@@ -565,6 +613,8 @@ func isWebhookEndpoint(name string) bool {
565613
func (r *OpenStackReconciler) checkServiceEndpoints(ctx context.Context, instance *operatorv1beta1.OpenStack) (ctrl.Result, error) {
566614
Log := r.GetLogger(ctx)
567615

616+
expectedWebhookSvcs := r.webhookServiceNames()
617+
568618
endpointSliceList := &discoveryv1.EndpointSliceList{}
569619
err := r.List(ctx, endpointSliceList, &client.ListOptions{Namespace: instance.Namespace})
570620
if err != nil {
@@ -578,7 +628,7 @@ func (r *OpenStackReconciler) checkServiceEndpoints(ctx context.Context, instanc
578628
for _, endpointSlice := range endpointSliceList.Items {
579629
endpointSliceName := endpointSlice.GetName()
580630

581-
if isWebhookEndpoint(endpointSliceName) {
631+
if isWebhookEndpoint(endpointSliceName, expectedWebhookSvcs) {
582632
// is deployment disabled ?
583633
disabled := false
584634
for _, op := range instance.Spec.OperatorOverrides {
@@ -726,7 +776,7 @@ func (r *OpenStackReconciler) applyOperator(ctx context.Context, instance *opera
726776
serviceOp.Deployment.Manager.Env = append(serviceOp.Deployment.Manager.Env,
727777
relatedImagesEnv...)
728778
case operatorv1beta1.OpenStackBaremetalOperatorName:
729-
// enable webhooks on the openstack-operator
779+
// enable webhooks on the openstack-baremetal-operator
730780
serviceOp.Deployment.Manager.Env = append(serviceOp.Deployment.Manager.Env,
731781
corev1.EnvVar{
732782
Name: "ENABLE_WEBHOOKS",
@@ -743,11 +793,15 @@ func (r *OpenStackReconciler) applyOperator(ctx context.Context, instance *opera
743793
Value: "true",
744794
})
745795
default:
746-
// disable webhooks per default
796+
// Enable webhooks for operators whose manifest was found at startup; others default false.
797+
enableWebhooks := "false"
798+
if r.webhookOperatorSet[op.Name] {
799+
enableWebhooks = "true"
800+
}
747801
serviceOp.Deployment.Manager.Env = append(serviceOp.Deployment.Manager.Env,
748802
corev1.EnvVar{
749803
Name: "ENABLE_WEBHOOKS",
750-
Value: "false",
804+
Value: enableWebhooks,
751805
})
752806
}
753807

@@ -1178,6 +1232,8 @@ func (r *OpenStackReconciler) postCleanupObsoleteResources(ctx context.Context,
11781232

11791233
// SetupWithManager sets up the controller with the Manager.
11801234
func (r *OpenStackReconciler) SetupWithManager(mgr ctrl.Manager) error {
1235+
bindir := util.GetEnvVar("BASE_BINDATA", "/bindata")
1236+
r.webhookOperatorSet = loadWebhookOperatorSet(bindir)
11811237
return ctrl.NewControllerManagedBy(mgr).
11821238
Owns(&appsv1.Deployment{}).
11831239
For(&operatorv1beta1.OpenStack{}).

0 commit comments

Comments
 (0)