Skip to content

OCPBUGS-105193: extract HCCO webhook validation into a dedicated controller #9239

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 2 commits into from
Aug 7, 2026
Merged

OCPBUGS-105193: extract HCCO webhook validation into a dedicated controller #9239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f0c7f02
refactor(hcco): extract webhook validation into a dedicated controller
bryan-cox Aug 6, 2026
c9cb6d2
test: increase WebhookValidation e2e timeout to 3 minutes
bryan-cox Aug 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions control-plane-operator/hostedclusterconfigoperator/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/openshift/hypershift/control-plane-operator/hostedclusterconfigoperator/controllers/reencryption"
"github.com/openshift/hypershift/control-plane-operator/hostedclusterconfigoperator/controllers/resources"
"github.com/openshift/hypershift/control-plane-operator/hostedclusterconfigoperator/controllers/spotremediation"
"github.com/openshift/hypershift/control-plane-operator/hostedclusterconfigoperator/controllers/webhookvalidation"
"github.com/openshift/hypershift/control-plane-operator/hostedclusterconfigoperator/operator"
hyperapi "github.com/openshift/hypershift/support/api"
"github.com/openshift/hypershift/support/capabilities"
Expand Down Expand Up @@ -64,16 +65,17 @@ func NewCommand() *cobra.Command {
}

var controllerFuncs = map[string]operator.ControllerSetupFunc{
"controller-manager-ca": cmca.Setup,
resources.ControllerName: resources.Setup,
"inplaceupgrader": inplaceupgrader.Setup,
"node": node.Setup,
nodecount.ControllerName: nodecount.Setup,
"machine": machine.Setup,
"drainer": drainer.Setup,
hcpstatus.ControllerName: hcpstatus.Setup,
spotremediation.ControllerName: spotremediation.Setup,
reencryption.ControllerName: reencryption.Setup,
"controller-manager-ca": cmca.Setup,
resources.ControllerName: resources.Setup,
"inplaceupgrader": inplaceupgrader.Setup,
"node": node.Setup,
nodecount.ControllerName: nodecount.Setup,
"machine": machine.Setup,
"drainer": drainer.Setup,
hcpstatus.ControllerName: hcpstatus.Setup,
spotremediation.ControllerName: spotremediation.Setup,
reencryption.ControllerName: reencryption.Setup,
webhookvalidation.ControllerName: webhookvalidation.Setup,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

type HostedClusterConfigOperator struct {
Expand Down
66 changes: 0 additions & 66 deletions control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,6 @@ func (r *reconciler) reconcileStorageAndMisc(ctx context.Context, log logr.Logge
log.Info("reconciling observed configuration")
errs = append(errs, r.reconcileObservedConfiguration(ctx, hcp)...)

errs = append(errs, r.ensureGuestAdmissionWebhooksAreValid(ctx))
return errs
}

Expand Down Expand Up @@ -2935,61 +2934,6 @@ func (r *reconciler) reconcileRestoredCluster(ctx context.Context, hcp *hyperv1.
return false, nil
}

func (r *reconciler) ensureGuestAdmissionWebhooksAreValid(ctx context.Context) error {
log := ctrl.LoggerFrom(ctx)

cpServices := &corev1.ServiceList{}
if err := r.cpClient.List(ctx, cpServices, client.InNamespace(r.hcpNamespace)); err != nil {
return fmt.Errorf("failed to list control plane services: %w", err)
}

// disallow all urls targeting services in the hcp namespace by default unless 'hypershift.openshift.io/allow-guest-webhooks' label is present.
disallowedUrls := make([]string, 0)
for _, svc := range cpServices.Items {
if _, exist := svc.Labels[hyperv1.AllowGuestWebhooksServiceLabel]; exist {
continue
}

disallowedUrls = append(disallowedUrls, fmt.Sprintf("https://%s", svc.Name))
disallowedUrls = append(disallowedUrls, fmt.Sprintf("https://%s.%s.svc", svc.Name, svc.Namespace))
disallowedUrls = append(disallowedUrls, fmt.Sprintf("https://%s.%s.svc.cluster.local", svc.Name, svc.Namespace))
}

validatingWebhookConfigurations := &admissionregistrationv1.ValidatingWebhookConfigurationList{}
if err := r.client.List(ctx, validatingWebhookConfigurations); err != nil {
return fmt.Errorf("failed to list validatingWebhookConfigurations: %w", err)
}

errs := make([]error, 0)
for _, configuration := range validatingWebhookConfigurations.Items {
for _, webhook := range configuration.Webhooks {
if webhook.ClientConfig.URL != nil && !isAllowedWebhookUrl(disallowedUrls, *webhook.ClientConfig.URL) {
log.Info("deleting validating webhook configuration with a disallowed url", "webhook_name", configuration.Name, "disallowed_url", *webhook.ClientConfig.URL)
errs = append(errs, r.client.Delete(ctx, &configuration))
break
}
}
}

mutatingWebhookConfigurations := &admissionregistrationv1.MutatingWebhookConfigurationList{}
if err := r.client.List(ctx, mutatingWebhookConfigurations); err != nil {
errs = append(errs, fmt.Errorf("failed to list mutatingWebhookConfigurations: %w", err))
return utilerrors.NewAggregate(errs)
}

for _, configuration := range mutatingWebhookConfigurations.Items {
for _, webhook := range configuration.Webhooks {
if webhook.ClientConfig.URL != nil && !isAllowedWebhookUrl(disallowedUrls, *webhook.ClientConfig.URL) {
log.Info("deleting mutating webhook configuration with a disallowed url", "webhook_name", configuration.Name, "disallowed_url", *webhook.ClientConfig.URL)
errs = append(errs, r.client.Delete(ctx, &configuration))
break
}
}
}

return utilerrors.NewAggregate(errs)
}

// reconcileKubeletConfig Lists the KubeletConfig ConfigMaps from the controlPlane cluster
// and copies them to the hosted cluster.
// In addition, it deletes KubeletConfig ConfigMaps from the hosted cluster which are no longer relevant.
Expand Down Expand Up @@ -3118,16 +3062,6 @@ func mutateKubeletConfig(controlPlaneConfigMap, hostedClusterConfigMap *corev1.C
return nil
}

func isAllowedWebhookUrl(disallowedUrls []string, url string) bool {
for i := range disallowedUrls {
if strings.Contains(url, disallowedUrls[i]) {
return false
}
}

return true
}

func (r *reconciler) ensureResourceCreationIsBlocked(ctx context.Context, hcp *hyperv1.HostedControlPlane) error {
wh := manifests.ResourceCreationBlockerWebhook()
if _, err := r.CreateOrUpdate(ctx, r.client, wh, func() error {
Expand Down
Loading