From 1b03afc748e89487547164f983f1cea5b53a19d9 Mon Sep 17 00:00:00 2001 From: Mitch Shao <62157463+mingqishao@users.noreply.github.com> Date: Wed, 17 Jan 2024 21:15:39 -0800 Subject: [PATCH] feat: parameterize the web hook service name (#656) --- Makefile | 1 + charts/hub-agent/templates/deployment.yaml | 1 + charts/hub-agent/values.yaml | 1 + cmd/hubagent/main.go | 6 +++--- cmd/hubagent/options/options.go | 4 ++++ cmd/hubagent/options/validation.go | 6 +++++- cmd/hubagent/options/validation_test.go | 13 +++++++++++++ pkg/webhook/webhook.go | 15 ++++++++------- 8 files changed, 36 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index 658b74e30..2381e7de4 100644 --- a/Makefile +++ b/Makefile @@ -161,6 +161,7 @@ install-hub-agent-helm: --set logVerbosity=5 \ --set namespace=fleet-system \ --set enableWebhook=true \ + --set webhookServiceName=fleetwebhook \ --set webhookClientConnectionType=service .PHONY: e2e-v1alpha1-hub-kubeconfig-secret diff --git a/charts/hub-agent/templates/deployment.yaml b/charts/hub-agent/templates/deployment.yaml index 34d8f42af..bce793609 100644 --- a/charts/hub-agent/templates/deployment.yaml +++ b/charts/hub-agent/templates/deployment.yaml @@ -22,6 +22,7 @@ spec: args: - --leader-elect=true - --enable-webhook={{ .Values.enableWebhook }} + - --webhook-service-name={{ .Values.webhookServiceName }} - --enable-guard-rail={{ .Values.enableGuardRail }} - --whitelisted-users=system:serviceaccount:fleet-system:hub-agent-sa - --webhook-client-connection-type={{.Values.webhookClientConnectionType}} diff --git a/charts/hub-agent/values.yaml b/charts/hub-agent/values.yaml index 4d5a33f95..5ffcd5d9a 100644 --- a/charts/hub-agent/values.yaml +++ b/charts/hub-agent/values.yaml @@ -13,6 +13,7 @@ image: logVerbosity: 5 enableWebhook: true +webhookServiceName: fleetwebhook enableGuardRail: true webhookClientConnectionType: service diff --git a/cmd/hubagent/main.go b/cmd/hubagent/main.go index 54e2eb85d..615e46c60 100644 --- a/cmd/hubagent/main.go +++ b/cmd/hubagent/main.go @@ -136,7 +136,7 @@ func main() { if opts.EnableWebhook { whiteListedUsers := strings.Split(opts.WhiteListedUsers, ",") - if err := SetupWebhook(mgr, options.WebhookClientConnectionType(opts.WebhookClientConnectionType), whiteListedUsers, opts.EnableGuardRail, opts.EnableV1Beta1APIs); err != nil { + if err := SetupWebhook(mgr, options.WebhookClientConnectionType(opts.WebhookClientConnectionType), opts.WebhookServiceName, whiteListedUsers, opts.EnableGuardRail, opts.EnableV1Beta1APIs); err != nil { klog.ErrorS(err, "unable to set up webhook") exitWithErrorFunc() } @@ -168,9 +168,9 @@ func main() { } // SetupWebhook generates the webhook cert and then set up the webhook configurator. -func SetupWebhook(mgr manager.Manager, webhookClientConnectionType options.WebhookClientConnectionType, whiteListedUsers []string, enableGuardRail, isFleetV1Beta1API bool) error { +func SetupWebhook(mgr manager.Manager, webhookClientConnectionType options.WebhookClientConnectionType, webhookServiceName string, whiteListedUsers []string, enableGuardRail, isFleetV1Beta1API bool) error { // Generate self-signed key and crt files in FleetWebhookCertDir for the webhook server to start. - w, err := webhook.NewWebhookConfig(mgr, FleetWebhookPort, &webhookClientConnectionType, FleetWebhookCertDir, enableGuardRail) + w, err := webhook.NewWebhookConfig(mgr, webhookServiceName, FleetWebhookPort, &webhookClientConnectionType, FleetWebhookCertDir, enableGuardRail) if err != nil { klog.ErrorS(err, "fail to generate WebhookConfig") return err diff --git a/cmd/hubagent/options/options.go b/cmd/hubagent/options/options.go index 0d396ec76..35db336c2 100644 --- a/cmd/hubagent/options/options.go +++ b/cmd/hubagent/options/options.go @@ -35,6 +35,8 @@ type Options struct { MetricsBindAddress string // EnableWebhook indicates if we will run a webhook EnableWebhook bool + // Webhook service name + WebhookServiceName string // EnableGuardRail indicates if we will enable fleet guard rail webhook configurations. EnableGuardRail bool // WhiteListedUsers indicates the list of user who are allowed to modify fleet resources @@ -105,6 +107,8 @@ func (o *Options) AddFlags(flags *flag.FlagSet) { flags.DurationVar(&o.LeaderElection.LeaseDuration.Duration, "leader-lease-duration", 15*time.Second, "This is effectively the maximum duration that a leader can be stopped before someone else will replace it.") flag.StringVar(&o.LeaderElection.ResourceNamespace, "leader-election-namespace", utils.FleetSystemNamespace, "The namespace in which the leader election resource will be created.") flag.BoolVar(&o.EnableWebhook, "enable-webhook", true, "If set, the fleet webhook is enabled.") + // set a defautl value 'fleetwebhook' for webhook service name for backward compatibility. The service name was hard coded to 'fleetwebhook' in the past. + flag.StringVar(&o.WebhookServiceName, "webhook-service-name", "fleetwebhook", "Fleet webhook service name.") flag.BoolVar(&o.EnableGuardRail, "enable-guard-rail", false, "If set, the fleet guard rail webhook configurations are enabled.") flag.StringVar(&o.WhiteListedUsers, "whitelisted-users", "", "If set, white listed users can modify fleet related resources.") flag.StringVar(&o.WebhookClientConnectionType, "webhook-client-connection-type", "url", "Sets the connection type used by the webhook client. Only URL or Service is valid.") diff --git a/cmd/hubagent/options/validation.go b/cmd/hubagent/options/validation.go index f11936acf..482a47edb 100644 --- a/cmd/hubagent/options/validation.go +++ b/cmd/hubagent/options/validation.go @@ -37,9 +37,13 @@ func (o *Options) Validate() field.ErrorList { errs = append(errs, field.Invalid(newPath.Child("WorkPendingGracePeriod"), o.WorkPendingGracePeriod, "Must be greater than 0")) } + if o.EnableWebhook && o.WebhookServiceName == "" { + errs = append(errs, field.Invalid(newPath.Child("WebhookServiceName"), o.WebhookServiceName, "Webhook service name is required when webhook is enabled")) + } + connectionType := o.WebhookClientConnectionType if _, err := parseWebhookClientConnectionString(connectionType); err != nil { - errs = append(errs, field.Invalid(newPath.Child("WebhookClientConnectionType"), o.EnableWebhook, err.Error())) + errs = append(errs, field.Invalid(newPath.Child("WebhookClientConnectionType"), o.WebhookClientConnectionType, err.Error())) } if !o.EnableV1Alpha1APIs && !o.EnableV1Beta1APIs { diff --git a/cmd/hubagent/options/validation_test.go b/cmd/hubagent/options/validation_test.go index 52f33174b..d94cf97bc 100644 --- a/cmd/hubagent/options/validation_test.go +++ b/cmd/hubagent/options/validation_test.go @@ -67,6 +67,19 @@ func TestValidateControllerManagerConfiguration(t *testing.T) { }), want: field.ErrorList{field.Required(newPath.Child("EnableV1Alpha1APIs"), "Either EnableV1Alpha1APIs or EnableV1Beta1APIs is required")}, }, + "invalid WebhookClientConnectionType": { + opt: newTestOptions(func(option *Options) { + option.WebhookClientConnectionType = "invalid" + }), + want: field.ErrorList{field.Invalid(newPath.Child("WebhookClientConnectionType"), "invalid", `must be "service" or "url"`)}, + }, + "WebhookServiceName is empty": { + opt: newTestOptions(func(option *Options) { + option.EnableWebhook = true + option.WebhookServiceName = "" + }), + want: field.ErrorList{field.Invalid(newPath.Child("WebhookServiceName"), "", "Webhook service name is required when webhook is enabled")}, + }, } for name, tc := range testCases { diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index e469bef77..db434a83d 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -57,7 +57,6 @@ const ( fleetWebhookKeyFileName = "tls.key" fleetValidatingWebhookCfgName = "fleet-validating-webhook-configuration" fleetGuardRailWebhookCfgName = "fleet-guard-rail-webhook-configuration" - fleetWebhookSvcName = "fleetwebhook" crdResourceName = "customresourcedefinitions" bindingResourceName = "bindings" @@ -126,6 +125,7 @@ type Config struct { // webhook server info serviceNamespace string + serviceName string servicePort int32 serviceURL string @@ -137,7 +137,7 @@ type Config struct { enableGuardRail bool } -func NewWebhookConfig(mgr manager.Manager, port int, clientConnectionType *options.WebhookClientConnectionType, certDir string, enableGuardRail bool) (*Config, error) { +func NewWebhookConfig(mgr manager.Manager, webhookServiceName string, port int, clientConnectionType *options.WebhookClientConnectionType, certDir string, enableGuardRail bool) (*Config, error) { // We assume the Pod namespace should be passed to env through downward API in the Pod spec. namespace := os.Getenv("POD_NAMESPACE") if namespace == "" { @@ -147,7 +147,8 @@ func NewWebhookConfig(mgr manager.Manager, port int, clientConnectionType *optio mgr: mgr, servicePort: int32(port), serviceNamespace: namespace, - serviceURL: fmt.Sprintf("https://%s.%s.svc.cluster.local:%d", fleetWebhookSvcName, namespace, port), + serviceName: webhookServiceName, + serviceURL: fmt.Sprintf("https://%s.%s.svc.cluster.local:%d", webhookServiceName, namespace, port), clientConnectionType: clientConnectionType, enableGuardRail: enableGuardRail, } @@ -500,7 +501,7 @@ func (w *Config) buildFleetGuardRailValidatingWebhooks() []admv1.ValidatingWebho func (w *Config) createClientConfig(validationPath string) admv1.WebhookClientConfig { serviceRef := admv1.ServiceReference{ Namespace: w.serviceNamespace, - Name: fleetWebhookSvcName, + Name: w.serviceName, Port: pointer.Int32(w.servicePort), } serviceEndpoint := w.serviceURL + validationPath @@ -577,15 +578,15 @@ func (w *Config) genSelfSignedCert() (caPEMByte, certPEMByte, keyPEMByte []byte, caPEMByte = caPEM.Bytes() dnsNames := []string{ - fmt.Sprintf("%s.%s.svc", fleetWebhookSvcName, w.serviceNamespace), - fmt.Sprintf("%s.%s.svc.cluster.local", fleetWebhookSvcName, w.serviceNamespace), + fmt.Sprintf("%s.%s.svc", w.serviceName, w.serviceNamespace), + fmt.Sprintf("%s.%s.svc.cluster.local", w.serviceName, w.serviceNamespace), } // server cert config cert := &x509.Certificate{ DNSNames: dnsNames, SerialNumber: big.NewInt(2022), Subject: pkix.Name{ - CommonName: fmt.Sprintf("%s.cert.server", fleetWebhookSvcName), + CommonName: fmt.Sprintf("%s.cert.server", w.serviceName), OrganizationalUnit: []string{"Azure Kubernetes Service"}, Organization: []string{"Microsoft"}, Locality: []string{"Redmond"},