endpoint for policy mutation + refactor + graceful shutdown

main.go

@@ -37,7 +37,8 @@ func main() {
 	printVersionInfo()
 	// profile cpu and memory consuption
 	prof = enableProfiling(cpu, memory)
-
+	// cleanUp Channel
+	cleanUp := make(chan struct{})
 	// CLIENT CONFIG
 	clientConfig, err := createClientConfig(kubeconfig)
 	if err != nil {
@@ -136,7 +137,7 @@ func main() {
 	// -- annotations on resources with update details on mutation JSON patches
 	// -- generate policy violation resource
 	// -- generate events on policy and resource
-	server, err := webhooks.NewWebhookServer(pclient, client, tlsPair, pInformer.Kyverno().V1alpha1().Policies(), pInformer.Kyverno().V1alpha1().PolicyViolations(), egen, webhookRegistrationClient, pc.GetPolicyStatusAggregator(), filterK8Resources)
+	server, err := webhooks.NewWebhookServer(pclient, client, tlsPair, pInformer.Kyverno().V1alpha1().Policies(), pInformer.Kyverno().V1alpha1().PolicyViolations(), egen, webhookRegistrationClient, pc.GetPolicyStatusAggregator(), filterK8Resources, cleanUp)
 	if err != nil {
 		glog.Fatalf("Unable to create webhook server: %v\n", err)
 	}
@@ -157,6 +158,9 @@ func main() {
 	<-stopCh
 	disableProfiling(prof)
 	server.Stop()
+	// resource cleanup
+	// remove webhook configurations
+	<-cleanUp
 }
 
 func init() {

pkg/webhookconfig/policy.go

@@ -54,7 +54,7 @@ func (wrc *WebhookRegistrationClient) contructDebugPolicyValidatingWebhookConfig
 				"policies/*",
 				"kyverno.io",
 				"v1alpha1",
-				[]admregapi.OperationType{admregapi.Create},
+				[]admregapi.OperationType{admregapi.Create, admregapi.Update},
 			),
 		},
 	}
@@ -79,13 +79,13 @@ func (wrc *WebhookRegistrationClient) contructPolicyMutatingWebhookConfig(caData
 				"policies/*",
 				"kyverno.io",
 				"v1alpha1",
-				[]admregapi.OperationType{admregapi.Create},
+				[]admregapi.OperationType{admregapi.Create, admregapi.Update},
 			),
 		},
 	}
 }
 func (wrc *WebhookRegistrationClient) contructDebugPolicyMutatingWebhookConfig(caData []byte) *admregapi.MutatingWebhookConfiguration {
-	url := fmt.Sprintf("https://%s%s", wrc.serverIP, config.PolicyValidatingWebhookServicePath)
+	url := fmt.Sprintf("https://%s%s", wrc.serverIP, config.PolicyMutatingWebhookServicePath)
 	glog.V(4).Infof("Debug PolicyMutatingWebhookConfig is registered with url %s\n", url)
 
 	return &admregapi.MutatingWebhookConfiguration{
@@ -103,7 +103,7 @@ func (wrc *WebhookRegistrationClient) contructDebugPolicyMutatingWebhookConfig(c
 				"policies/*",
 				"kyverno.io",
 				"v1alpha1",
-				[]admregapi.OperationType{admregapi.Create},
+				[]admregapi.OperationType{admregapi.Create, admregapi.Update},
 			),
 		},
 	}

pkg/webhookconfig/registration.go

@@ -2,6 +2,7 @@ package webhookconfig
 
 import (
 	"errors"
+	"time"
 
 	"github.com/golang/glog"
 	"github.com/nirmata/kyverno/pkg/config"
@@ -71,6 +72,15 @@ func (wrc *WebhookRegistrationClient) Register() error {
 	return nil
 }
 
+// RemovePolicyWebhookConfigurations removes webhook configurations for reosurces and policy
+// called during webhook server shutdown
+func (wrc *WebhookRegistrationClient) RemovePolicyWebhookConfigurations(cleanUp chan<- struct{}) {
+	//TODO: dupliate, but a placeholder to perform more error handlind during cleanup
+	wrc.removeWebhookConfigurations()
+	// close channel to notify cleanup is complete
+	close(cleanUp)
+}
+
 func (wrc *WebhookRegistrationClient) CreateResourceMutatingWebhookConfiguration() error {
 	var caData []byte
 	var config *admregapi.MutatingWebhookConfiguration
@@ -191,6 +201,11 @@ func (wrc *WebhookRegistrationClient) createPolicyMutatingWebhookConfiguration()
 // This function does not fail on error:
 // Register will fail if the config exists, so there is no need to fail on error
 func (wrc *WebhookRegistrationClient) removeWebhookConfigurations() {
+	startTime := time.Now()
+	glog.V(4).Infof("Started cleaning up webhookconfigurations")
+	defer func() {
+		glog.V(4).Infof("Finished cleaning up webhookcongfigurations (%v)", time.Since(startTime))
+	}()
 	// mutating and validating webhook configuration for Kubernetes resources
 	wrc.RemoveResourceMutatingWebhookConfiguration()
 	wrc.removeResourceValidatingWebhookConfiguration()

pkg/webhooks/mutation.go

@@ -12,7 +12,7 @@ import (
 )
 
 // HandleMutation handles mutating webhook admission request
-func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest) (bool, engine.EngineResponse) {
+func (ws *WebhookServer) handleMutation(request *v1beta1.AdmissionRequest) (bool, engine.EngineResponse) {
 	var patches [][]byte
 	var policyInfos []info.PolicyInfo
 	var policyStats []policyctr.PolicyStat

pkg/webhooks/policymutation.go

@@ -0,0 +1,84 @@
+package webhooks
+
+import (
+	"encoding/json"
+	"fmt"
+	"strings"
+
+	"github.com/golang/glog"
+	kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1alpha1"
+	"github.com/nirmata/kyverno/pkg/utils"
+	v1beta1 "k8s.io/api/admission/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func (ws *WebhookServer) handlePolicyMutation(request *v1beta1.AdmissionRequest) *v1beta1.AdmissionResponse {
+	var policy *kyverno.Policy
+	raw := request.Object.Raw
+
+	//TODO: can this happen? wont this be picked by OpenAPI spec schema ?
+	if err := json.Unmarshal(raw, &policy); err != nil {
+		glog.Errorf("Failed to unmarshal policy admission request, err %v\n", err)
+		return &v1beta1.AdmissionResponse{
+			Allowed: true,
+			Result: &metav1.Status{
+				Message: fmt.Sprintf("failed to default value, check kyverno controller logs for details", err),
+			},
+		}
+	}
+	// Generate JSON Patches for defaults
+	patches, updateMsgs := generateJSONPatchesForDefaults(policy)
+	if patches != nil {
+		patchType := v1beta1.PatchTypeJSONPatch
+		glog.V(4).Infof("defaulted values %v policy %s", updateMsgs, policy.Name)
+		return &v1beta1.AdmissionResponse{
+			Allowed: true,
+			Result: &metav1.Status{
+				Message: strings.Join(updateMsgs, "'"),
+			},
+			Patch:     patches,
+			PatchType: &patchType,
+		}
+	}
+	glog.V(4).Info("nothing to default for policy %s", policy.Name)
+	return &v1beta1.AdmissionResponse{
+		Allowed: true,
+	}
+}
+
+func generateJSONPatchesForDefaults(policy *kyverno.Policy) ([]byte, []string) {
+	var patches [][]byte
+	var updateMsgs []string
+
+	// default 'ValidationFailureAction'
+	if patch, updateMsg := defaultvalidationFailureAction(policy); patch != nil {
+		patches = append(patches, patch)
+		updateMsgs = append(updateMsgs, updateMsg)
+	}
+
+	return utils.JoinPatches(patches), updateMsgs
+}
+
+func defaultvalidationFailureAction(policy *kyverno.Policy) ([]byte, string) {
+	// default ValidationFailureAction to "enforce" if not specified
+	if policy.Spec.ValidationFailureAction == "" {
+		glog.V(4).Infof("defaulting policy %s 'ValidationFailureAction' to '%s'", policy.Name, BlockChanges)
+		jsonPatch := struct {
+			Path  string `json:"path"`
+			Op    string `json:"op"`
+			Value string `json:"value"`
+		}{
+			"/spec/validationFailureAction",
+			"add",
+			BlockChanges, //enforce
+		}
+		patchByte, err := json.Marshal(jsonPatch)
+		if err != nil {
+			glog.Error("failed to set default 'ValidationFailureAction' to '%s' for policy %s", BlockChanges, policy.Name)
+			return nil, ""
+		}
+		glog.V(4).Infof("generate JSON Patch to set default 'ValidationFailureAction' to '%s' for policy %s", BlockChanges, policy.Name)
+		return patchByte, fmt.Sprintf("default 'ValidationFailureAction' to '%s'", BlockChanges)
+	}
+	return nil, ""
+}

pkg/webhooks/policyvalidation.go

@@ -13,16 +13,13 @@ import (
 )
 
 //HandlePolicyValidation performs the validation check on policy resource
-func (ws *WebhookServer) HandlePolicyValidation(request *v1beta1.AdmissionRequest) *v1beta1.AdmissionResponse {
+func (ws *WebhookServer) handlePolicyValidation(request *v1beta1.AdmissionRequest) *v1beta1.AdmissionResponse {
 	var policy *kyverno.Policy
 	admissionResp := &v1beta1.AdmissionResponse{
 		Allowed: true,
 	}
-	// nothing to do on DELETE
-	if request.Operation == v1beta1.Delete {
-		return admissionResp
-	}
 
+	//TODO: can this happen? wont this be picked by OpenAPI spec schema ?
 	raw := request.Object.Raw
 	if err := json.Unmarshal(raw, &policy); err != nil {
 		glog.Errorf("Failed to unmarshal policy admission request, err %v\n", err)

pkg/webhooks/server.go

@@ -42,6 +42,7 @@ type WebhookServer struct {
 	// API to send policy stats for aggregation
 	policyStatus      policy.PolicyStatusInterface
 	filterK8Resources []utils.K8Resource
+	cleanUp           chan<- struct{}
 }
 
 // NewWebhookServer creates new instance of WebhookServer accordingly to given configuration
@@ -55,7 +56,8 @@ func NewWebhookServer(
 	eventGen event.Interface,
 	webhookRegistrationClient *webhookconfig.WebhookRegistrationClient,
 	policyStatus policy.PolicyStatusInterface,
-	filterK8Resources string) (*WebhookServer, error) {
+	filterK8Resources string,
+	cleanUp chan<- struct{}) (*WebhookServer, error) {
 
 	if tlsPair == nil {
 		return nil, errors.New("NewWebhookServer is not initialized properly")
@@ -80,11 +82,13 @@ func NewWebhookServer(
 		webhookRegistrationClient: webhookRegistrationClient,
 		policyStatus:              policyStatus,
 		filterK8Resources:         utils.ParseKinds(filterK8Resources),
+		cleanUp:                   cleanUp,
 	}
 	mux := http.NewServeMux()
 	mux.HandleFunc(config.MutatingWebhookServicePath, ws.serve)
 	mux.HandleFunc(config.ValidatingWebhookServicePath, ws.serve)
 	mux.HandleFunc(config.PolicyValidatingWebhookServicePath, ws.serve)
+	mux.HandleFunc(config.PolicyMutatingWebhookServicePath, ws.serve)
 
 	ws.server = http.Server{
 		Addr:         ":443", // Listen on port for HTTPS requests
@@ -114,9 +118,11 @@ func (ws *WebhookServer) serve(w http.ResponseWriter, r *http.Request) {
 		// Resource UPDATE
 		switch r.URL.Path {
 		case config.MutatingWebhookServicePath:
-			admissionReview.Response = ws.HandleAdmissionRequest(admissionReview.Request)
+			admissionReview.Response = ws.handleAdmissionRequest(admissionReview.Request)
 		case config.PolicyValidatingWebhookServicePath:
-			admissionReview.Response = ws.HandlePolicyValidation(admissionReview.Request)
+			admissionReview.Response = ws.handlePolicyValidation(admissionReview.Request)
+		case config.PolicyMutatingWebhookServicePath:
+			admissionReview.Response = ws.handlePolicyMutation(admissionReview.Request)
 		}
 	}
 
@@ -134,10 +140,10 @@ func (ws *WebhookServer) serve(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-func (ws *WebhookServer) HandleAdmissionRequest(request *v1beta1.AdmissionRequest) *v1beta1.AdmissionResponse {
+func (ws *WebhookServer) handleAdmissionRequest(request *v1beta1.AdmissionRequest) *v1beta1.AdmissionResponse {
 	var response *v1beta1.AdmissionResponse
 
-	allowed, engineResponse := ws.HandleMutation(request)
+	allowed, engineResponse := ws.handleMutation(request)
 	if !allowed {
 		// TODO: add failure message to response
 		return &v1beta1.AdmissionResponse{
@@ -145,7 +151,7 @@ func (ws *WebhookServer) HandleAdmissionRequest(request *v1beta1.AdmissionReques
 		}
 	}
 
-	response = ws.HandleValidation(request, engineResponse.PatchedResource)
+	response = ws.handleValidation(request, engineResponse.PatchedResource)
 	if response.Allowed && len(engineResponse.Patches) > 0 {
 		patchType := v1beta1.PatchTypeJSONPatch
 		response.Patch = engine.JoinPatches(engineResponse.Patches)
@@ -169,6 +175,9 @@ func (ws *WebhookServer) RunAsync() {
 
 // Stop TLS server and returns control after the server is shut down
 func (ws *WebhookServer) Stop() {
+	// cleanUp
+	// remove the static webhookconfigurations for policy CRD
+	ws.webhookRegistrationClient.RemovePolicyWebhookConfigurations(ws.cleanUp)
 	err := ws.server.Shutdown(context.Background())
 	if err != nil {
 		// Error from closing listeners, or context timeout:

pkg/webhooks/validation.go

@@ -16,7 +16,7 @@ import (
 
 // HandleValidation handles validating webhook admission request
 // If there are no errors in validating rule we apply generation rules
-func (ws *WebhookServer) HandleValidation(request *v1beta1.AdmissionRequest, resource unstructured.Unstructured) *v1beta1.AdmissionResponse {
+func (ws *WebhookServer) handleValidation(request *v1beta1.AdmissionRequest, resource unstructured.Unstructured) *v1beta1.AdmissionResponse {
 	var policyInfos []info.PolicyInfo
 	var policyStats []policyctr.PolicyStat
 	// gather stats from the engine response