Merge pull request #824 from rikatz/add-new-webhook-markers

✨ Add url marker for webhook manifests
kubernetes-sigs · Jul 18, 2023 · c7b9156 · c7b9156
2 parents dba4afa + 3e161ca
commit c7b9156
Show file tree

Hide file tree

Showing 7 changed files with 430 additions and 10 deletions.
diff --git a/pkg/webhook/parser.go b/pkg/webhook/parser.go
@@ -108,7 +108,7 @@ type Config struct {
 	// are substituted for hyphens. For example, a validating webhook path for type
 	// batch.tutorial.kubebuilder.io/v1,Kind=CronJob would be
 	// /validate-batch-tutorial-kubebuilder-io-v1-cronjob
-	Path string
+	Path string `marker:"path,optional"`
 
 	// WebhookVersions specifies the target API versions of the {Mutating,Validating}WebhookConfiguration objects
 	// itself to generate. The only supported value is v1. Defaults to v1.
@@ -125,6 +125,14 @@ type Config struct {
 	// an object, and mutating webhooks can specify a reinvocationPolicy to control
 	// whether they are reinvoked as well.
 	ReinvocationPolicy string `marker:"reinvocationPolicy,optional"`
+
+	// URL allows mutating webhooks configuration to specify an external URL when generating
+	// the manifests, instead of using the internal service communication. Should be in format of
+	// https://address:port/path
+	// When this option is specified, the serviceConfig.Service is removed from webhook the manifest.
+	// The URL configuration should be between quotes.
+	// `url` cannot be specified when `path` is specified.
+	URL string `marker:"url,optional"`
 }
 
 // verbToAPIVariant converts a marker's verb to the proper value for the API.
@@ -157,12 +165,17 @@ func (c Config) ToMutatingWebhook() (admissionregv1.MutatingWebhook, error) {
 		return admissionregv1.MutatingWebhook{}, err
 	}
 
+	clientConfig, err := c.clientConfig()
+	if err != nil {
+		return admissionregv1.MutatingWebhook{}, err
+	}
+
 	return admissionregv1.MutatingWebhook{
 		Name:                    c.Name,
 		Rules:                   c.rules(),
 		FailurePolicy:           c.failurePolicy(),
 		MatchPolicy:             matchPolicy,
-		ClientConfig:            c.clientConfig(),
+		ClientConfig:            clientConfig,
 		SideEffects:             c.sideEffects(),
 		TimeoutSeconds:          c.timeoutSeconds(),
 		AdmissionReviewVersions: c.AdmissionReviewVersions,
@@ -181,12 +194,17 @@ func (c Config) ToValidatingWebhook() (admissionregv1.ValidatingWebhook, error)
 		return admissionregv1.ValidatingWebhook{}, err
 	}
 
+	clientConfig, err := c.clientConfig()
+	if err != nil {
+		return admissionregv1.ValidatingWebhook{}, err
+	}
+
 	return admissionregv1.ValidatingWebhook{
 		Name:                    c.Name,
 		Rules:                   c.rules(),
 		FailurePolicy:           c.failurePolicy(),
 		MatchPolicy:             matchPolicy,
-		ClientConfig:            c.clientConfig(),
+		ClientConfig:            clientConfig,
 		SideEffects:             c.sideEffects(),
 		TimeoutSeconds:          c.timeoutSeconds(),
 		AdmissionReviewVersions: c.AdmissionReviewVersions,
@@ -251,15 +269,27 @@ func (c Config) matchPolicy() (*admissionregv1.MatchPolicyType, error) {
 }
 
 // clientConfig returns the client config for a webhook.
-func (c Config) clientConfig() admissionregv1.WebhookClientConfig {
+func (c Config) clientConfig() (admissionregv1.WebhookClientConfig, error) {
+	if (c.Path != "" && c.URL != "") || (c.Path == "" && c.URL == "") {
+		return admissionregv1.WebhookClientConfig{}, fmt.Errorf("`url` or `path` markers are required and mutually exclusive")
+	}
+
 	path := c.Path
-	return admissionregv1.WebhookClientConfig{
-		Service: &admissionregv1.ServiceReference{
-			Name:      "webhook-service",
-			Namespace: "system",
-			Path:      &path,
-		},
+	if path != "" {
+		return admissionregv1.WebhookClientConfig{
+			Service: &admissionregv1.ServiceReference{
+				Name:      "webhook-service",
+				Namespace: "system",
+				Path:      &path,
+			},
+		}, nil
 	}
+
+	url := c.URL
+	return admissionregv1.WebhookClientConfig{
+		URL: &url,
+	}, nil
+
 }
 
 // sideEffects returns the sideEffects config for a webhook.

diff --git a/pkg/webhook/parser_integration_test.go b/pkg/webhook/parser_integration_test.go
@@ -253,6 +253,80 @@ var _ = Describe("Webhook Generation From Parsing to CustomResourceDefinition",
 			assertSame(actualManifest, expectedManifest)
 		}
 	})
+
+	It("should properly generate the webhook definition with url instead of service", func() {
+		By("switching into testdata to appease go modules")
+		cwd, err := os.Getwd()
+		Expect(err).NotTo(HaveOccurred())
+		Expect(os.Chdir("./testdata/valid-url")).To(Succeed()) // go modules are directory-sensitive
+		defer func() { Expect(os.Chdir(cwd)).To(Succeed()) }()
+
+		By("loading the roots")
+		pkgs, err := loader.LoadRoots(".")
+		Expect(err).NotTo(HaveOccurred())
+		Expect(pkgs).To(HaveLen(1))
+
+		By("setting up the parser")
+		reg := &markers.Registry{}
+		Expect(reg.Register(webhook.ConfigDefinition)).To(Succeed())
+
+		By("requesting that the manifest be generated")
+		outputDir, err := ioutil.TempDir("", "webhook-integration-test")
+		Expect(err).NotTo(HaveOccurred())
+		defer os.RemoveAll(outputDir)
+		genCtx := &genall.GenerationContext{
+			Collector:  &markers.Collector{Registry: reg},
+			Roots:      pkgs,
+			OutputRule: genall.OutputToDirectory(outputDir),
+		}
+		Expect(webhook.Generator{}.Generate(genCtx)).To(Succeed())
+		for _, r := range genCtx.Roots {
+			Expect(r.Errors).To(HaveLen(0))
+		}
+
+		By("loading the generated v1 YAML")
+		actualFile, err := ioutil.ReadFile(path.Join(outputDir, "manifests.yaml"))
+		Expect(err).NotTo(HaveOccurred())
+		actualMutating, actualValidating := unmarshalBothV1(actualFile)
+
+		By("loading the desired v1 YAML")
+		expectedFile, err := ioutil.ReadFile("manifests.yaml")
+		Expect(err).NotTo(HaveOccurred())
+		expectedMutating, expectedValidating := unmarshalBothV1(expectedFile)
+
+		By("comparing the two")
+		assertSame(actualMutating, expectedMutating)
+		assertSame(actualValidating, expectedValidating)
+	})
+
+	It("should fail to generate when both path and url are set", func() {
+		By("switching into testdata to appease go modules")
+		cwd, err := os.Getwd()
+		Expect(err).NotTo(HaveOccurred())
+		Expect(os.Chdir("./testdata/invalid-path-and-url")).To(Succeed()) // go modules are directory-sensitive
+		defer func() { Expect(os.Chdir(cwd)).To(Succeed()) }()
+
+		By("loading the roots")
+		pkgs, err := loader.LoadRoots(".")
+		Expect(err).NotTo(HaveOccurred())
+		Expect(pkgs).To(HaveLen(1))
+
+		By("setting up the parser")
+		reg := &markers.Registry{}
+		Expect(reg.Register(webhook.ConfigDefinition)).To(Succeed())
+
+		By("requesting that the manifest be generated")
+		outputDir, err := ioutil.TempDir("", "webhook-integration-test")
+		Expect(err).NotTo(HaveOccurred())
+		defer os.RemoveAll(outputDir)
+		genCtx := &genall.GenerationContext{
+			Collector:  &markers.Collector{Registry: reg},
+			Roots:      pkgs,
+			OutputRule: genall.OutputToDirectory(outputDir),
+		}
+		err = webhook.Generator{}.Generate(genCtx)
+		Expect(err).To(HaveOccurred())
+	})
 })
 
 func unmarshalBothV1(in []byte) (mutating admissionregv1.MutatingWebhookConfiguration, validating admissionregv1.ValidatingWebhookConfiguration) {

diff --git a/pkg/webhook/testdata/invalid-path-and-url/cronjob_types.go b/pkg/webhook/testdata/invalid-path-and-url/cronjob_types.go
@@ -0,0 +1,71 @@
+/*
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+//go:generate ../../../../.run-controller-gen.sh webhook paths=. output:dir=.
+
+// +groupName=testdata.kubebuilder.io
+// +versionName=v1
+package cronjob
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// EDIT THIS FILE!  THIS IS SCAFFOLDING FOR YOU TO OWN!
+// NOTE: json tags are required.  Any new fields you add must have json tags for the fields to be serialized.
+
+// CronJobSpec defines the desired state of CronJob
+type CronJobSpec struct {
+	// The schedule in Cron format, see https://en.wikipedia.org/wiki/Cron.
+	Schedule string `json:"schedule"`
+}
+
+// CronJobStatus defines the observed state of CronJob
+type CronJobStatus struct {
+	// INSERT ADDITIONAL STATUS FIELD - define observed state of cluster
+	// Important: Run "make" to regenerate code after modifying this file
+
+	// Information when was the last time the job was successfully scheduled.
+	// +optional
+	LastScheduleTime *metav1.Time `json:"lastScheduleTime,omitempty"`
+}
+
+// +kubebuilder:object:root=true
+// +kubebuilder:subresource:status
+// +kubebuilder:resource:singular=mycronjob
+
+// CronJob is the Schema for the cronjobs API
+type CronJob struct {
+	/*
+	 */
+	metav1.TypeMeta   `json:",inline"`
+	metav1.ObjectMeta `json:"metadata,omitempty"`
+
+	Spec   CronJobSpec   `json:"spec,omitempty"`
+	Status CronJobStatus `json:"status,omitempty"`
+}
+
+// +kubebuilder:object:root=true
+
+// CronJobList contains a list of CronJob
+type CronJobList struct {
+	metav1.TypeMeta `json:",inline"`
+	metav1.ListMeta `json:"metadata,omitempty"`
+	Items           []CronJob `json:"items"`
+}
+
+func init() {
+	SchemeBuilder.Register(&CronJob{}, &CronJobList{})
+}
diff --git a/pkg/webhook/testdata/invalid-path-and-url/webhook.go b/pkg/webhook/testdata/invalid-path-and-url/webhook.go
@@ -0,0 +1,48 @@
+/*
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package cronjob
+
+import (
+	"k8s.io/apimachinery/pkg/runtime"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
+)
+
+func (c *CronJob) SetupWebhookWithManager(mgr ctrl.Manager) error {
+	return ctrl.NewWebhookManagedBy(mgr).
+		For(c).
+		Complete()
+}
+
+// +kubebuilder:webhook:url="https://anothersomewebhook:9443/validate-testdata-kubebuilder-io-v1-cronjob",path="/somepath",verbs=create;update,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuiler.io,resources=cronjobs,versions=v1,name=validation.cronjob.testdata.kubebuilder.io,sideEffects=NoneOnDryRun,timeoutSeconds=10,admissionReviewVersions=v1;v1beta1
+
+var _ webhook.Defaulter = &CronJob{}
+var _ webhook.Validator = &CronJob{}
+
+func (c *CronJob) Default() {
+}
+
+func (c *CronJob) ValidateCreate() error {
+	return nil
+}
+
+func (c *CronJob) ValidateUpdate(_ runtime.Object) error {
+	return nil
+}
+
+func (c *CronJob) ValidateDelete() error {
+	return nil
+}
diff --git a/pkg/webhook/testdata/valid-url/cronjob_types.go b/pkg/webhook/testdata/valid-url/cronjob_types.go
@@ -0,0 +1,71 @@
+/*
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+//go:generate ../../../../.run-controller-gen.sh webhook paths=. output:dir=.
+
+// +groupName=testdata.kubebuilder.io
+// +versionName=v1
+package cronjob
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// EDIT THIS FILE!  THIS IS SCAFFOLDING FOR YOU TO OWN!
+// NOTE: json tags are required.  Any new fields you add must have json tags for the fields to be serialized.
+
+// CronJobSpec defines the desired state of CronJob
+type CronJobSpec struct {
+	// The schedule in Cron format, see https://en.wikipedia.org/wiki/Cron.
+	Schedule string `json:"schedule"`
+}
+
+// CronJobStatus defines the observed state of CronJob
+type CronJobStatus struct {
+	// INSERT ADDITIONAL STATUS FIELD - define observed state of cluster
+	// Important: Run "make" to regenerate code after modifying this file
+
+	// Information when was the last time the job was successfully scheduled.
+	// +optional
+	LastScheduleTime *metav1.Time `json:"lastScheduleTime,omitempty"`
+}
+
+// +kubebuilder:object:root=true
+// +kubebuilder:subresource:status
+// +kubebuilder:resource:singular=mycronjob
+
+// CronJob is the Schema for the cronjobs API
+type CronJob struct {
+	/*
+	 */
+	metav1.TypeMeta   `json:",inline"`
+	metav1.ObjectMeta `json:"metadata,omitempty"`
+
+	Spec   CronJobSpec   `json:"spec,omitempty"`
+	Status CronJobStatus `json:"status,omitempty"`
+}
+
+// +kubebuilder:object:root=true
+
+// CronJobList contains a list of CronJob
+type CronJobList struct {
+	metav1.TypeMeta `json:",inline"`
+	metav1.ListMeta `json:"metadata,omitempty"`
+	Items           []CronJob `json:"items"`
+}
+
+func init() {
+	SchemeBuilder.Register(&CronJob{}, &CronJobList{})
+}