Merge pull request kubernetes-sigs#300 from mengqiy/decouple

⚠️ split webhook server and manifest generation

Author: k8s-ci-robot

Gopkg.lock

@@ -20,17 +20,15 @@ import (
 	"flag"
 	"os"
 
-	admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
-	apitypes "k8s.io/apimachinery/pkg/types"
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
-	"sigs.k8s.io/controller-runtime/pkg/manager"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
+	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/manager/signals"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -81,52 +79,22 @@ func main() {
 
 	// Setup webhooks
 	entryLog.Info("setting up webhooks")
-	mutatingWebhook, err := builder.NewWebhookBuilder().
-		Name("mutating.k8s.io").
+	mutatingWebhook := builder.NewWebhookBuilder().
+		Path("/mutate-pods").
 		Mutating().
-		Operations(admissionregistrationv1beta1.Create, admissionregistrationv1beta1.Update).
-		WithManager(mgr).
-		ForType(&corev1.Pod{}).
 		Handlers(&podAnnotator{}).
 		Build()
-	if err != nil {
-		entryLog.Error(err, "unable to setup mutating webhook")
-		os.Exit(1)
-	}
 
-	validatingWebhook, err := builder.NewWebhookBuilder().
-		Name("validating.k8s.io").
+	validatingWebhook := builder.NewWebhookBuilder().
+		Path("/validate-pods").
 		Validating().
-		Operations(admissionregistrationv1beta1.Create, admissionregistrationv1beta1.Update).
-		WithManager(mgr).
-		ForType(&corev1.Pod{}).
 		Handlers(&podValidator{}).
 		Build()
-	if err != nil {
-		entryLog.Error(err, "unable to setup validating webhook")
-		os.Exit(1)
-	}
 
 	entryLog.Info("setting up webhook server")
-	as, err := webhook.NewServer("foo-admission-server", mgr, webhook.ServerOptions{
-		Port:                          9876,
-		CertDir:                       "/tmp/cert",
-		DisableWebhookConfigInstaller: &disableWebhookConfigInstaller,
-		BootstrapOptions: &webhook.BootstrapOptions{
-			Secret: &apitypes.NamespacedName{
-				Namespace: "default",
-				Name:      "foo-admission-server-secret",
-			},
-
-			Service: &webhook.Service{
-				Namespace: "default",
-				Name:      "foo-admission-server-service",
-				// Selectors should select the pods that runs this webhook server.
-				Selectors: map[string]string{
-					"app": "foo-admission-server",
-				},
-			},
-		},
+	as, err := webhook.NewServer(mgr, webhook.ServerOptions{
+		Port:    9876,
+		CertDir: "/tmp/cert",
 	})
 	if err != nil {
 		entryLog.Error(err, "unable to create a new webhook server")
@@ -136,7 +104,7 @@ func main() {
 	entryLog.Info("registering webhooks to the webhook server")
 	err = as.Register(mutatingWebhook, validatingWebhook)
 	if err != nil {
-		entryLog.Error(err, "unable to register webhooks in the admission server")
+		entryLog.Error(err, "unable to setup the admission server")
 		os.Exit(1)
 	}
 

example/main.go

@@ -17,14 +17,6 @@ limitations under the License.
 package builder
 
 import (
-	"errors"
-	"fmt"
-
-	admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
-	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/types"
 )
@@ -46,26 +38,6 @@ type WebhookBuilder struct {
 	// t specifies the type of the webhook.
 	// Currently, Mutating and Validating are supported.
 	t *types.WebhookType
-
-	// operations define the operations this webhook cares.
-	// only one of operations and Rules can be set.
-	operations []admissionregistrationv1beta1.OperationType
-	// apiType represents the resource that this webhook cares.
-	// Only one of apiType and Rules can be set.
-	apiType runtime.Object
-	// rules contain a list of admissionregistrationv1beta1.RuleWithOperations
-	// It overrides operations and apiType.
-	rules []admissionregistrationv1beta1.RuleWithOperations
-
-	// failurePolicy maps to the FailurePolicy in the admissionregistrationv1beta1.Webhook
-	failurePolicy *admissionregistrationv1beta1.FailurePolicyType
-
-	// namespaceSelector maps to the NamespaceSelector in the admissionregistrationv1beta1.Webhook
-	namespaceSelector *metav1.LabelSelector
-
-	// manager is the manager for the webhook.
-	// It is used for provisioning various dependencies for the webhook. e.g. RESTMapper.
-	manager manager.Manager
 }
 
 // NewWebhookBuilder creates an empty WebhookBuilder.
@@ -98,126 +70,32 @@ func (b *WebhookBuilder) Validating() *WebhookBuilder {
 
 // Path sets the path for the webhook.
 // Path needs to be unique among different webhooks.
-// This is optional. If not set, it will be built from the type and resource name.
+// This is required. If not set, it will be built from the type and resource name.
 // For example, a webhook that mutates pods has a default path of "/mutate-pods"
 // If the defaulting logic can't find a unique path for it, user need to set it manually.
 func (b *WebhookBuilder) Path(path string) *WebhookBuilder {
 	b.path = path
 	return b
 }
 
-// Operations sets the operations that this webhook cares.
-// It will be overridden by Rules if Rules are not empty.
-// This is optional
-func (b *WebhookBuilder) Operations(ops ...admissionregistrationv1beta1.OperationType) *WebhookBuilder {
-	b.operations = ops
-	return b
-}
-
-// ForType sets the type of resources that the webhook will operate.
-// It will be overridden by Rules if Rules are not empty.
-func (b *WebhookBuilder) ForType(obj runtime.Object) *WebhookBuilder {
-	b.apiType = obj
-	return b
-}
-
-// Rules sets the RuleWithOperations for the webhook.
-// It overrides ForType and Operations.
-// This is optional and for advanced user.
-func (b *WebhookBuilder) Rules(rules ...admissionregistrationv1beta1.RuleWithOperations) *WebhookBuilder {
-	b.rules = rules
-	return b
-}
-
-// FailurePolicy sets the FailurePolicy of the webhook.
-// If not set, it will be defaulted by the server.
-// This is optional
-func (b *WebhookBuilder) FailurePolicy(policy admissionregistrationv1beta1.FailurePolicyType) *WebhookBuilder {
-	b.failurePolicy = &policy
-	return b
-}
-
-// NamespaceSelector sets the NamespaceSelector for the webhook.
-// This is optional
-func (b *WebhookBuilder) NamespaceSelector(namespaceSelector *metav1.LabelSelector) *WebhookBuilder {
-	b.namespaceSelector = namespaceSelector
-	return b
-}
-
-// WithManager set the manager for the webhook for provisioning various dependencies. e.g. client etc.
-func (b *WebhookBuilder) WithManager(mgr manager.Manager) *WebhookBuilder {
-	b.manager = mgr
-	return b
-}
-
 // Handlers sets the handlers of the webhook.
 func (b *WebhookBuilder) Handlers(handlers ...admission.Handler) *WebhookBuilder {
 	b.handlers = handlers
 	return b
 }
 
-func (b *WebhookBuilder) validate() error {
-	if b.t == nil {
-		return errors.New("webhook type cannot be nil")
-	}
-	if b.rules == nil && b.apiType == nil {
-		return fmt.Errorf("ForType should be set")
-	}
-	if b.rules != nil && b.apiType != nil {
-		return fmt.Errorf("at most one of ForType and Rules can be set")
-	}
-	return nil
-}
-
 // Build creates the Webhook based on the options provided.
-func (b *WebhookBuilder) Build() (*admission.Webhook, error) {
-	err := b.validate()
-	if err != nil {
-		return nil, err
+func (b *WebhookBuilder) Build() *admission.Webhook {
+	if b.t == nil {
+		b.Mutating()
 	}
 
 	w := &admission.Webhook{
-		Name:              b.name,
-		Type:              *b.t,
-		Path:              b.path,
-		FailurePolicy:     b.failurePolicy,
-		NamespaceSelector: b.namespaceSelector,
-		Handlers:          b.handlers,
-	}
-
-	if b.rules != nil {
-		w.Rules = b.rules
-	} else {
-		if b.manager == nil {
-			return nil, errors.New("manager should be set using WithManager")
-		}
-		gvk, err := apiutil.GVKForObject(b.apiType, b.manager.GetScheme())
-		if err != nil {
-			return nil, err
-		}
-		mapper := b.manager.GetRESTMapper()
-		mapping, err := mapper.RESTMapping(gvk.GroupKind(), gvk.Version)
-		if err != nil {
-			return nil, err
-		}
-
-		if b.operations == nil {
-			b.operations = []admissionregistrationv1beta1.OperationType{
-				admissionregistrationv1beta1.Create,
-				admissionregistrationv1beta1.Update,
-			}
-		}
-		w.Rules = []admissionregistrationv1beta1.RuleWithOperations{
-			{
-				Operations: b.operations,
-				Rule: admissionregistrationv1beta1.Rule{
-					APIGroups:   []string{gvk.Group},
-					APIVersions: []string{gvk.Version},
-					Resources:   []string{mapping.Resource.Resource},
-				},
-			},
-		}
+		Name:     b.name,
+		Type:     *b.t,
+		Path:     b.path,
+		Handlers: b.handlers,
 	}
 
-	return w, nil
+	return w
 }

pkg/webhook/admission/builder/builder.go

@@ -19,27 +19,19 @@ Package builder provides methods to build admission webhooks.
 
 The following are 2 examples for building mutating webhook and validating webhook.
 
-	webhook1, err := NewWebhookBuilder().
+	webhook1 := NewWebhookBuilder().
 		Mutating().
-		Operations(admissionregistrationv1beta1.Create).
+		Path("/mutatepods").
 		ForType(&corev1.Pod{}).
-		WithManager(mgr).
 		Handlers(mutatingHandler11, mutatingHandler12).
 		Build()
-	if err != nil {
-		// handle error
-	}
 
-	webhook2, err := NewWebhookBuilder().
+	webhook2 := NewWebhookBuilder().
 		Validating().
-		Operations(admissionregistrationv1beta1.Create, admissionregistrationv1beta1.Update).
+		Path("/validatepods").
 		ForType(&appsv1.Deployment{}).
-		WithManager(mgr).
 		Handlers(validatingHandler21).
 		Build()
-	if err != nil {
-		// handle error
-	}
 
 Note: To build a webhook for a CRD, you need to ensure the manager uses the scheme that understands your CRD.
 This is necessary, because if the scheme doesn't understand your CRD types, the decoder won't be able to decode
@@ -62,46 +54,5 @@ The following snippet shows how to register CRD types with manager's scheme.
 	if err != nil {
 		// handle error
 	}
-
-There are more options for configuring a webhook. e.g. Name, Path, FailurePolicy, NamespaceSelector.
-Here is another example:
-
-	webhook3, err := NewWebhookBuilder().
-		Name("foo.example.com").
-		Path("/mutatepods").
-		Mutating().
-		Operations(admissionregistrationv1beta1.Create).
-		ForType(&corev1.Pod{}).
-		FailurePolicy(admissionregistrationv1beta1.Fail).
-		WithManager(mgr).
-		Handlers(mutatingHandler31, mutatingHandler32).
-		Build()
-	if err != nil {
-		// handle error
-	}
-
-For most users, we recommend to use Operations and ForType instead of Rules to construct a webhook,
-since it is more intuitive and easier to pass the target operations to Operations method and
-a empty target object to ForType method than passing a complex RuleWithOperations struct to Rules method.
-
-Rules may be useful for some more advanced use cases like subresources, wildcard resources etc.
-Here is an example:
-
-	webhook4, err := NewWebhookBuilder().
-		Validating().
-		Rules(admissionregistrationv1beta1.RuleWithOperations{
-			Operations: []admissionregistrationv1beta1.OperationType{admissionregistrationv1beta1.Create},
-			Rule: admissionregistrationv1beta1.Rule{
-				APIGroups:   []string{"apps", "batch"},
-				APIVersions: []string{"v1"},
-				Resources:   []string{"*"},
-			},
-		}).
-		WithManager(mgr).
-		Handlers(validatingHandler41).
-		Build()
-	if err != nil {
-		// handle error
-	}
 */
 package builder