Skip to content

✨ Networking config for default mode webhooks #379

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

✨ Networking config for default mode webhooks #379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
101 changes: 101 additions & 0 deletions operator/v1/0000_01_operator.open-cluster-management.io_clustermanagers.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,71 @@ spec:
DeployOption contains the options of deploying a cluster-manager
Default mode is used if DeployOption is not set.
properties:
default:
description: Default includes configurations for clustermanager
in the Default mode
properties:
registrationWebhookConfiguration:
description: RegistrationWebhookConfiguration represents the
customized webhook-server configuration of registration.
properties:
healthProbeBindAddress:
default: :8000
description: |-
HealthProbeBindAddress represents the healthcheck address of a webhook-server. The default value is ":8000".
Healthchecks may be disabled by setting a value of "0" or "".
type: string
hostNetwork:
description: |-
HostNetwork enables running webhook pods with hostNetwork: true
This may be required in some installations, such as EKS with Calico CNI,
to allow the API Server to communicate with the webhook pods.
type: boolean
metricsBindAddress:
default: :8080
description: |-
MetricsBindAddress represents the metrics address of a webhook-server. The default value is ":8080"
Metrics may be disabled by setting a value of "0" or "".
type: string
port:
default: 9443
description: Port represents the port of a webhook-server.
The default value of Port is 9443.
format: int32
maximum: 65535
type: integer
type: object
workWebhookConfiguration:
description: WorkWebhookConfiguration represents the customized
webhook-server configuration of work.
properties:
healthProbeBindAddress:
default: :8000
description: |-
HealthProbeBindAddress represents the healthcheck address of a webhook-server. The default value is ":8000".
Healthchecks may be disabled by setting a value of "0" or "".
type: string
hostNetwork:
description: |-
HostNetwork enables running webhook pods with hostNetwork: true
This may be required in some installations, such as EKS with Calico CNI,
to allow the API Server to communicate with the webhook pods.
type: boolean
metricsBindAddress:
default: :8080
description: |-
MetricsBindAddress represents the metrics address of a webhook-server. The default value is ":8080"
Metrics may be disabled by setting a value of "0" or "".
type: string
port:
default: 9443
description: Port represents the port of a webhook-server.
The default value of Port is 9443.
format: int32
maximum: 65535
type: integer
type: object
type: object
hosted:
description: Hosted includes configurations we need for clustermanager
in the Hosted mode.
Expand All @@ -106,6 +171,24 @@ spec:
The Address must be reachable by apiserver of the hub cluster.
pattern: ^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$
type: string
healthProbeBindAddress:
default: :8000
description: |-
HealthProbeBindAddress represents the healthcheck address of a webhook-server. The default value is ":8000".
Healthchecks may be disabled by setting a value of "0" or "".
type: string
hostNetwork:
description: |-
HostNetwork enables running webhook pods with hostNetwork: true
This may be required in some installations, such as EKS with Calico CNI,
to allow the API Server to communicate with the webhook pods.
type: boolean
metricsBindAddress:
default: :8080
description: |-
MetricsBindAddress represents the metrics address of a webhook-server. The default value is ":8080"
Metrics may be disabled by setting a value of "0" or "".
type: string
port:
default: 443
description: Port represents the port of a webhook-server.
Expand All @@ -127,6 +210,24 @@ spec:
The Address must be reachable by apiserver of the hub cluster.
pattern: ^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$
type: string
healthProbeBindAddress:
default: :8000
description: |-
HealthProbeBindAddress represents the healthcheck address of a webhook-server. The default value is ":8000".
Healthchecks may be disabled by setting a value of "0" or "".
type: string
hostNetwork:
description: |-
HostNetwork enables running webhook pods with hostNetwork: true
This may be required in some installations, such as EKS with Calico CNI,
to allow the API Server to communicate with the webhook pods.
type: boolean
metricsBindAddress:
default: :8080
description: |-
MetricsBindAddress represents the metrics address of a webhook-server. The default value is ":8080"
Metrics may be disabled by setting a value of "0" or "".
type: string
port:
default: 443
description: Port represents the port of a webhook-server.
Expand Down
55 changes: 52 additions & 3 deletions operator/v1/types_clustermanager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,19 +232,62 @@ const (
FeatureGateModeTypeDisable FeatureGateModeType = "Disable"
)

// DefaultClusterManagerConfiguration represents customized configurations for clustermanager in the Default mode
type DefaultClusterManagerConfiguration struct {
// RegistrationWebhookConfiguration represents the customized webhook-server configuration of registration.
// +optional
RegistrationWebhookConfiguration DefaultWebhookConfiguration `json:"registrationWebhookConfiguration,omitempty"`

// WorkWebhookConfiguration represents the customized webhook-server configuration of work.
// +optional
WorkWebhookConfiguration DefaultWebhookConfiguration `json:"workWebhookConfiguration,omitempty"`
}

// HostedClusterManagerConfiguration represents customized configurations we need to set for clustermanager in the Hosted mode.
type HostedClusterManagerConfiguration struct {
// RegistrationWebhookConfiguration represents the customized webhook-server configuration of registration.
// +optional
RegistrationWebhookConfiguration WebhookConfiguration `json:"registrationWebhookConfiguration,omitempty"`
RegistrationWebhookConfiguration HostedWebhookConfiguration `json:"registrationWebhookConfiguration,omitempty"`

// WorkWebhookConfiguration represents the customized webhook-server configuration of work.
// +optional
WorkWebhookConfiguration WebhookConfiguration `json:"workWebhookConfiguration,omitempty"`
WorkWebhookConfiguration HostedWebhookConfiguration `json:"workWebhookConfiguration,omitempty"`
}

// WebhookConfiguration has two properties: Address and Port.
// WebhookConfiguration represents customization of webhook servers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually it is not WebhookConfiguration, I think we could also use this for controller to set healthiness and metrics endpoint?

Copy link
Contributor Author

@bhperry bhperry Jul 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. Maybe BindConfiguration?

Realized there is an issue for Hosted mode. The Port field is only the external endpoint for k8s API to reach the validating webhook, but if you set HostNetwork then you would also want to control the internal port bind to avoid port collision on the host net.

Perhaps a better API would be something like this:

spec:
  ...
  deployOption:
    hosted:
      workWebhookConfiguration:
        port: 443
        address: work.webhook.example.com
        bindConfiguration:
          port: 19443
          healthProbeBindAddress: ":18000"
          metricsBindAddress: ":18080"
          hostNetwork: true
      registrationWebhookConfiguration:
        ...
    mode: Hosted

---

spec:
  ...
  deployOption:
    default:
      workWebhookConfiguration:
        bindConfiguration:
          ...
      registrationWebhookConfiguration:
        ...
    mode: Default

bindConfiguration would be optional and nullable

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like controller servers work very differently from the webhooks. They use the openshift/library-go controller builder, which only takes a single listen address:port. Whereas the webhooks use controller-runtime which has separate binds for each. So I don't think it makes sense to re-use this configuration for controllers (unless they get migrated to controller-runtime in the future)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we do not need to consider how the implementation is when considering this API. The controller-runtime allows bind endpoint separately does not mean we need to support that in the API level. Is there scenario that we need to let metrics/healthiness bind to the different address?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well it's not just that it can bind separately, they are separate listeners. No option to share port with the webhook server. As a result, when using hostNetwork mode, it is important to have control over the binds.

Copy link
Member

@qiujian16 qiujian16 Jul 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the port should be different, I am wondering if it is necessary to bind different HOST/IP for metrics/healthz endpoint.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I figured if you can bind hostNet then it is reasonable to allow fully configuring the bind for metrics/healthz. Not strictly necessary though (I had no intention of using it, I'm just binding ":" for all of these).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, I think it is sufficient to specify port for each

type WebhookConfiguration struct {
// HealthProbeBindAddress represents the healthcheck address of a webhook-server. The default value is ":8000".
// Healthchecks may be disabled by setting a value of "0" or "".
// +optional
// +kubebuilder:default=":8000"
HealthProbeBindAddress string `json:"healthProbeBindAddress"`

// MetricsBindAddress represents the metrics address of a webhook-server. The default value is ":8080"
// Metrics may be disabled by setting a value of "0" or "".
// +optional
// +kubebuilder:default=":8080"
MetricsBindAddress string `json:"metricsBindAddress"`

// HostNetwork enables running webhook pods with hostNetwork: true
// This may be required in some installations, such as EKS with Calico CNI,
// to allow the API Server to communicate with the webhook pods.
// +optional
HostNetwork bool `json:"hostNetwork,omitempty"`
}

// DefaultWebhookConfiguration represents customization of webhook servers running in default installation mode
type DefaultWebhookConfiguration struct {
// Port represents the port of a webhook-server. The default value of Port is 9443.
// +optional
// +kubebuilder:default=9443
// +kubebuilder:validation:Maximum=65535
Port int32 `json:"port,omitempty"`

WebhookConfiguration `json:",inline"`
}

// HostedWebhookConfiguration represents customization of webhook servers running in hosted installation mode
type HostedWebhookConfiguration struct {
// Address represents the address of a webhook-server.
// It could be in IP format or fqdn format.
// The Address must be reachable by apiserver of the hub cluster.
Expand All @@ -258,6 +301,8 @@ type WebhookConfiguration struct {
// +kubebuilder:default=443
// +kubebuilder:validation:Maximum=65535
Port int32 `json:"port,omitempty"`

WebhookConfiguration `json:",inline"`
}

// ClusterManagerDeployOption describes the deployment options for cluster-manager
Expand All @@ -274,6 +319,10 @@ type ClusterManagerDeployOption struct {
// +kubebuilder:validation:Enum=Default;Hosted
Mode InstallMode `json:"mode,omitempty"`

// Default includes configurations for clustermanager in the Default mode
// +optional
Default *DefaultClusterManagerConfiguration `json:"default,omitempty"`

// Hosted includes configurations we need for clustermanager in the Hosted mode.
// +optional
Hosted *HostedClusterManagerConfiguration `json:"hosted,omitempty"`
Expand Down
57 changes: 57 additions & 0 deletions operator/v1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 37 additions & 6 deletions operator/v1/zz_generated.swagger_doc_generated.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading