Refactor admission webhook (#656)

* Refactor admission webhook

pkg/hook/controller/admission_bindings_controller.go

@@ -2,11 +2,11 @@ package controller
 
 import (
 	log "github.com/sirupsen/logrus"
+	v1 "k8s.io/api/admission/v1"
 
 	. "github.com/flant/shell-operator/pkg/hook/binding_context"
 	. "github.com/flant/shell-operator/pkg/hook/types"
 	"github.com/flant/shell-operator/pkg/webhook/admission"
-	. "github.com/flant/shell-operator/pkg/webhook/admission/types"
 )
 
 // AdmissionBindingToWebhookLink is a link between a hook and a webhook configuration.
@@ -20,20 +20,7 @@ type AdmissionBindingToWebhookLink struct {
 	Group            string
 }
 
-// AdmissionBindingsController handles admission bindings for one hook.
-type AdmissionBindingsController interface {
-	WithValidatingBindings([]ValidatingConfig)
-	WithMutatingBindings([]MutatingConfig)
-	WithWebhookManager(*admission.WebhookManager)
-	EnableValidatingBindings()
-	EnableMutatingBindings()
-	DisableValidatingBindings()
-	DisableMutatingBindings()
-	CanHandleEvent(event AdmissionEvent) bool
-	HandleEvent(event AdmissionEvent) BindingExecutionInfo
-}
-
-type admissionBindingsController struct {
+type AdmissionBindingsController struct {
 	// Controller holds validating/mutating bindings from one hook. Hook always belongs to one configurationId.
 	ConfigurationId string
 	// WebhookId -> link
@@ -45,28 +32,26 @@ type admissionBindingsController struct {
 	webhookManager *admission.WebhookManager
 }
 
-var _ AdmissionBindingsController = &admissionBindingsController{}
-
 // NewValidatingBindingsController returns an implementation of AdmissionBindingsController
-var NewValidatingBindingsController = func() *admissionBindingsController {
-	return &admissionBindingsController{
+var NewValidatingBindingsController = func() *AdmissionBindingsController {
+	return &AdmissionBindingsController{
 		AdmissionLinks: make(map[string]*AdmissionBindingToWebhookLink),
 	}
 }
 
-func (c *admissionBindingsController) WithValidatingBindings(bindings []ValidatingConfig) {
+func (c *AdmissionBindingsController) WithValidatingBindings(bindings []ValidatingConfig) {
 	c.ValidatingBindings = bindings
 }
 
-func (c *admissionBindingsController) WithMutatingBindings(bindings []MutatingConfig) {
+func (c *AdmissionBindingsController) WithMutatingBindings(bindings []MutatingConfig) {
 	c.MutatingBindings = bindings
 }
 
-func (c *admissionBindingsController) WithWebhookManager(mgr *admission.WebhookManager) {
+func (c *AdmissionBindingsController) WithWebhookManager(mgr *admission.WebhookManager) {
 	c.webhookManager = mgr
 }
 
-func (c *admissionBindingsController) EnableValidatingBindings() {
+func (c *AdmissionBindingsController) EnableValidatingBindings() {
 	confId := ""
 
 	if len(c.ValidatingBindings) == 0 {
@@ -100,7 +85,7 @@ func (c *admissionBindingsController) EnableValidatingBindings() {
 	}
 }
 
-func (c *admissionBindingsController) EnableMutatingBindings() {
+func (c *AdmissionBindingsController) EnableMutatingBindings() {
 	confId := ""
 
 	if len(c.MutatingBindings) == 0 {
@@ -134,23 +119,23 @@ func (c *admissionBindingsController) EnableMutatingBindings() {
 	}
 }
 
-func (c *admissionBindingsController) DisableValidatingBindings() {
+func (c *AdmissionBindingsController) DisableValidatingBindings() {
 	// TODO dynamic enable/disable validating webhooks.
 }
 
-func (c *admissionBindingsController) DisableMutatingBindings() {
+func (c *AdmissionBindingsController) DisableMutatingBindings() {
 	// TODO dynamic enable/disable mutating webhooks.
 }
 
-func (c *admissionBindingsController) CanHandleEvent(event AdmissionEvent) bool {
+func (c *AdmissionBindingsController) CanHandleEvent(event admission.Event) bool {
 	if c.ConfigurationId != event.ConfigurationId {
 		return false
 	}
 	_, has := c.AdmissionLinks[event.WebhookId]
 	return has
 }
 
-func (c *admissionBindingsController) HandleEvent(event AdmissionEvent) BindingExecutionInfo {
+func (c *AdmissionBindingsController) HandleEvent(event admission.Event) BindingExecutionInfo {
 	if c.ConfigurationId != event.ConfigurationId {
 		log.Errorf("Possible bug!!! Unknown validating event: no binding for configurationId '%s' (webhookId '%s')", event.ConfigurationId, event.WebhookId)
 		return BindingExecutionInfo{
@@ -170,7 +155,7 @@ func (c *admissionBindingsController) HandleEvent(event AdmissionEvent) BindingE
 
 	bc := BindingContext{
 		Binding:         link.BindingName,
-		AdmissionReview: event.Review,
+		AdmissionReview: &v1.AdmissionReview{Request: event.Request},
 	}
 	bc.Metadata.BindingType = link.BindingType
 	bc.Metadata.IncludeSnapshots = link.IncludeSnapshots

pkg/hook/controller/hook_controller.go

@@ -9,7 +9,6 @@ import (
 	. "github.com/flant/shell-operator/pkg/kube_events_manager/types"
 	"github.com/flant/shell-operator/pkg/schedule_manager"
 	"github.com/flant/shell-operator/pkg/webhook/admission"
-	. "github.com/flant/shell-operator/pkg/webhook/admission/types"
 	"github.com/flant/shell-operator/pkg/webhook/conversion"
 )
 
@@ -42,7 +41,7 @@ func NewHookController() *HookController {
 type HookController struct {
 	KubernetesController KubernetesBindingsController
 	ScheduleController   ScheduleBindingsController
-	AdmissionController  AdmissionBindingsController
+	AdmissionController  *AdmissionBindingsController
 	ConversionController *ConversionBindingsController
 	kubernetesBindings   []OnKubernetesEventConfig
 	scheduleBindings     []ScheduleConfig
@@ -128,7 +127,7 @@ func (hc *HookController) CanHandleScheduleEvent(crontab string) bool {
 	return false
 }
 
-func (hc *HookController) CanHandleAdmissionEvent(event AdmissionEvent) bool {
+func (hc *HookController) CanHandleAdmissionEvent(event admission.Event) bool {
 	if hc.AdmissionController != nil {
 		return hc.AdmissionController.CanHandleEvent(event)
 	}
@@ -167,7 +166,7 @@ func (hc *HookController) HandleKubeEvent(event KubeEvent, createTasksFn func(Bi
 	}
 }
 
-func (hc *HookController) HandleAdmissionEvent(event AdmissionEvent, createTasksFn func(BindingExecutionInfo)) {
+func (hc *HookController) HandleAdmissionEvent(event admission.Event, createTasksFn func(BindingExecutionInfo)) {
 	if hc.AdmissionController == nil {
 		return
 	}

pkg/hook/hook.go

@@ -19,19 +19,19 @@ import (
 	"github.com/flant/shell-operator/pkg/hook/controller"
 	. "github.com/flant/shell-operator/pkg/hook/types"
 	"github.com/flant/shell-operator/pkg/metric_storage/operation"
-	. "github.com/flant/shell-operator/pkg/webhook/admission/types"
+	"github.com/flant/shell-operator/pkg/webhook/admission"
 	"github.com/flant/shell-operator/pkg/webhook/conversion"
 )
 
 type CommonHook interface {
 	Name() string
 }
 
-type HookResult struct {
+type Result struct {
 	Usage                *executor.CmdUsage
 	Metrics              []operation.MetricOperation
 	ConversionResponse   *conversion.Response
-	AdmissionResponse    *AdmissionResponse
+	AdmissionResponse    *admission.Response
 	KubernetesPatchBytes []byte
 }
 
@@ -81,7 +81,7 @@ func (h *Hook) WithHookController(hookController *controller.HookController) {
 	h.HookController = hookController
 }
 
-func (h *Hook) Run(_ BindingType, context []BindingContext, logLabels map[string]string) (*HookResult, error) {
+func (h *Hook) Run(_ BindingType, context []BindingContext, logLabels map[string]string) (*Result, error) {
 	// Refresh snapshots
 	freshBindingContext := h.HookController.UpdateSnapshots(context)
 
@@ -136,7 +136,7 @@ func (h *Hook) Run(_ BindingType, context []BindingContext, logLabels map[string
 
 	hookCmd := executor.MakeCommand(path.Dir(h.Path), h.Path, []string{}, envs)
 
-	result := &HookResult{}
+	result := &Result{}
 
 	result.Usage, err = executor.RunAndLogLines(hookCmd, logLabels)
 	if err != nil {
@@ -148,7 +148,7 @@ func (h *Hook) Run(_ BindingType, context []BindingContext, logLabels map[string
 		return result, fmt.Errorf("got bad metrics: %s", err)
 	}
 
-	result.AdmissionResponse, err = AdmissionResponseFromFile(admissionPath)
+	result.AdmissionResponse, err = admission.ResponseFromFile(admissionPath)
 	if err != nil {
 		return result, fmt.Errorf("got bad validating response: %s", err)
 	}

pkg/hook/hook_manager.go

@@ -19,7 +19,6 @@ import (
 	"github.com/flant/shell-operator/pkg/schedule_manager"
 	utils_file "github.com/flant/shell-operator/pkg/utils/file"
 	"github.com/flant/shell-operator/pkg/webhook/admission"
-	. "github.com/flant/shell-operator/pkg/webhook/admission/types"
 	"github.com/flant/shell-operator/pkg/webhook/conversion"
 )
 
@@ -295,7 +294,7 @@ func (hm *Manager) HandleScheduleEvent(crontab string, createTaskFn func(*Hook,
 	}
 }
 
-func (hm *Manager) HandleAdmissionEvent(event AdmissionEvent, createTaskFn func(*Hook, controller.BindingExecutionInfo)) {
+func (hm *Manager) HandleAdmissionEvent(event admission.Event, createTaskFn func(*Hook, controller.BindingExecutionInfo)) {
 	vHooks, _ := hm.GetHooksInOrder(KubernetesValidating)
 	for _, hookName := range vHooks {
 		h := hm.GetHook(hookName)
@@ -321,7 +320,7 @@ func (hm *Manager) HandleAdmissionEvent(event AdmissionEvent, createTaskFn func(
 	}
 }
 
-func (hm *Manager) DetectAdmissionEventType(event AdmissionEvent) BindingType {
+func (hm *Manager) DetectAdmissionEventType(event admission.Event) BindingType {
 	vHooks, _ := hm.GetHooksInOrder(KubernetesValidating)
 	for _, hookName := range vHooks {
 		h := hm.GetHook(hookName)
@@ -338,7 +337,7 @@ func (hm *Manager) DetectAdmissionEventType(event AdmissionEvent) BindingType {
 		}
 	}
 
-	log.Errorf("Possible bug!!! No linked hook for admission event %s %s kind=%s name=%s ns=%s", event.ConfigurationId, event.WebhookId, event.Review.Request.Kind, event.Review.Request.Name, event.Review.Request.Namespace)
+	log.Errorf("Possible bug!!! No linked hook for admission event %s %s kind=%s name=%s ns=%s", event.ConfigurationId, event.WebhookId, event.Request.Kind, event.Request.Name, event.Request.Namespace)
 	return ""
 }
 

pkg/hook/hook_manager_test.go

@@ -11,7 +11,6 @@ import (
 	"github.com/flant/shell-operator/pkg/hook/controller"
 	"github.com/flant/shell-operator/pkg/hook/types"
 	"github.com/flant/shell-operator/pkg/webhook/admission"
-	. "github.com/flant/shell-operator/pkg/webhook/admission/types"
 	"github.com/flant/shell-operator/pkg/webhook/conversion"
 )
 
@@ -91,10 +90,10 @@ func TestHookController_HandleValidatingEvent(t *testing.T) {
 		t.Fatalf("Hook manager Init should not fail: %v", err)
 	}
 
-	ev := AdmissionEvent{
+	ev := admission.Event{
 		WebhookId:       "test-policy-example-com",
 		ConfigurationId: "hooks",
-		Review:          nil,
+		Request:         nil,
 	}
 
 	h := hm.GetHook("hook.sh")

pkg/shell-operator/operator.go

@@ -25,7 +25,6 @@ import (
 	utils "github.com/flant/shell-operator/pkg/utils/labels"
 	"github.com/flant/shell-operator/pkg/utils/measure"
 	"github.com/flant/shell-operator/pkg/webhook/admission"
-	. "github.com/flant/shell-operator/pkg/webhook/admission/types"
 	"github.com/flant/shell-operator/pkg/webhook/conversion"
 )
 
@@ -204,7 +203,7 @@ func (op *ShellOperator) initValidatingWebhookManager() (err error) {
 	}
 
 	// Define handler for AdmissionEvent
-	op.AdmissionWebhookManager.WithAdmissionEventHandler(func(event AdmissionEvent) (*AdmissionResponse, error) {
+	op.AdmissionWebhookManager.WithAdmissionEventHandler(func(event admission.Event) (*admission.Response, error) {
 		eventBindingType := op.HookManager.DetectAdmissionEventType(event)
 		logLabels := map[string]string{
 			"event.id": uuid.Must(uuid.NewV4()).String(),
@@ -213,7 +212,7 @@ func (op *ShellOperator) initValidatingWebhookManager() (err error) {
 		logEntry := log.WithFields(utils.LabelsToLogFields(logLabels))
 		logEntry.Debugf("Handle '%s' event '%s' '%s'", eventBindingType, event.ConfigurationId, event.WebhookId)
 
-		var tasks []task.Task
+		var admissionTask task.Task
 		op.HookManager.HandleAdmissionEvent(event, func(hook *hook.Hook, info controller.BindingExecutionInfo) {
 			newTask := task.NewTask(HookRun).
 				WithMetadata(HookMetadata{
@@ -225,30 +224,26 @@ func (op *ShellOperator) initValidatingWebhookManager() (err error) {
 					Group:          info.Group,
 				}).
 				WithLogLabels(logLabels)
-			tasks = append(tasks, newTask)
+			admissionTask = newTask
 		})
 
 		// Assert exactly one task is created.
-		if len(tasks) == 0 {
+		if admissionTask == nil {
 			logEntry.Errorf("Possible bug!!! No hook found for '%s' event '%s' '%s'", string(KubernetesValidating), event.ConfigurationId, event.WebhookId)
 			return nil, fmt.Errorf("no hook found for '%s' '%s'", event.ConfigurationId, event.WebhookId)
 		}
 
-		if len(tasks) > 1 {
-			logEntry.Errorf("Possible bug!!! %d hooks found for '%s' event '%s' '%s'", len(tasks), string(KubernetesValidating), event.ConfigurationId, event.WebhookId)
-		}
-
-		res := op.taskHandler(tasks[0])
+		res := op.taskHandler(admissionTask)
 
 		if res.Status == "Fail" {
-			return &AdmissionResponse{
+			return &admission.Response{
 				Allowed: false,
 				Message: "Hook failed",
 			}, nil
 		}
 
-		admissionProp := tasks[0].GetProp("admissionResponse")
-		admissionResponse, ok := admissionProp.(*AdmissionResponse)
+		admissionProp := admissionTask.GetProp("admissionResponse")
+		admissionResponse, ok := admissionProp.(*admission.Response)
 		if !ok {
 			logEntry.Errorf("'admissionResponse' task prop is not of type *AdmissionResponse: %T", admissionProp)
 			return nil, fmt.Errorf("hook task prop error")

pkg/webhook/admission/types/event.go → pkg/webhook/admission/event.go

@@ -1,11 +1,11 @@
-package types
+package admission
 
 import (
 	v1 "k8s.io/api/admission/v1"
 )
 
-type AdmissionEvent struct {
+type Event struct {
 	WebhookId       string
 	ConfigurationId string
-	Review          *v1.AdmissionReview
+	Request         *v1.AdmissionRequest
 }