validate image cache in webhook server

Makefile

@@ -201,8 +201,10 @@ deploy-using-operator:
 	kubectl create -f deploy/kubefledged-operator/deploy/crds/charts.helm.k8s.io_v1alpha1_kubefledged_cr.yaml
 
 update:
-	kubectl scale deployment kubefledged --replicas=0 -n kube-fledged && sleep 1 && \
+	kubectl scale deployment kubefledged --replicas=0 -n kube-fledged && \
+	kubectl scale deployment kubefledged-webhook-server --replicas=0 -n kube-fledged && sleep 1 && \
 	kubectl scale deployment kubefledged --replicas=1 -n kube-fledged && sleep 1 && \
+	kubectl scale deployment kubefledged-webhook-server --replicas=1 -n kube-fledged && sleep 1 && \
 	kubectl get pods -l app=kubefledged -n kube-fledged
 
 remove:

cmd/fledged/app/controller.go

@@ -452,20 +452,6 @@ func (c *Controller) syncHandler(wqKey images.WorkQueueKey) error {
 			return err
 		}
 
-		err = validateCacheSpec(c, imageCache)
-		if err != nil {
-			status.Status = fledgedv1alpha1.ImageCacheActionStatusFailed
-			status.Reason = fledgedv1alpha1.ImageCacheReasonCacheSpecValidationFailed
-			status.Message = err.Error()
-
-			if err := c.updateImageCacheStatus(imageCache, status); err != nil {
-				glog.Errorf("Error updating imagecache status to %s: %v", status.Status, err)
-				return err
-			}
-
-			return err
-		}
-
 		if wqKey.WorkType == images.ImageCacheUpdate && wqKey.OldImageCache == nil {
 			status.Status = fledgedv1alpha1.ImageCacheActionStatusFailed
 			status.Reason = fledgedv1alpha1.ImageCacheReasonOldImageCacheNotFound
@@ -479,36 +465,6 @@ func (c *Controller) syncHandler(wqKey images.WorkQueueKey) error {
 			return fmt.Errorf("%s: %s", fledgedv1alpha1.ImageCacheReasonOldImageCacheNotFound, fledgedv1alpha1.ImageCacheMessageOldImageCacheNotFound)
 		}
 
-		if wqKey.WorkType == images.ImageCacheUpdate {
-			if len(wqKey.OldImageCache.Spec.CacheSpec) != len(imageCache.Spec.CacheSpec) {
-				status.Status = fledgedv1alpha1.ImageCacheActionStatusFailed
-				status.Reason = fledgedv1alpha1.ImageCacheReasonCacheSpecValidationFailed
-				status.Message = fledgedv1alpha1.ImageCacheMessageNotSupportedUpdates
-
-				if err = c.updateImageCacheSpecAndStatus(imageCache, wqKey.OldImageCache.Spec, status); err != nil {
-					glog.Errorf("Error updating imagecache spec and status to %s: %v", status.Status, err)
-					return err
-				}
-				glog.Errorf("%s: %s", fledgedv1alpha1.ImageCacheReasonCacheSpecValidationFailed, "Mismatch in no. of image lists")
-				return fmt.Errorf("%s: %s", fledgedv1alpha1.ImageCacheReasonCacheSpecValidationFailed, "Mismatch in no. of image lists")
-			}
-
-			for i := range wqKey.OldImageCache.Spec.CacheSpec {
-				if !reflect.DeepEqual(wqKey.OldImageCache.Spec.CacheSpec[i].NodeSelector, imageCache.Spec.CacheSpec[i].NodeSelector) {
-					status.Status = fledgedv1alpha1.ImageCacheActionStatusFailed
-					status.Reason = fledgedv1alpha1.ImageCacheReasonCacheSpecValidationFailed
-					status.Message = fledgedv1alpha1.ImageCacheMessageNotSupportedUpdates
-
-					if err = c.updateImageCacheSpecAndStatus(imageCache, wqKey.OldImageCache.Spec, status); err != nil {
-						glog.Errorf("Error updating imagecache spec and status to %s: %v", status.Status, err)
-						return err
-					}
-					glog.Errorf("%s: %s", fledgedv1alpha1.ImageCacheReasonCacheSpecValidationFailed, "Mismatch in node selector")
-					return fmt.Errorf("%s: %s", fledgedv1alpha1.ImageCacheReasonCacheSpecValidationFailed, "Mismatch in node selector")
-				}
-			}
-		}
-
 		cacheSpec := imageCache.Spec.CacheSpec
 		glog.V(4).Infof("cacheSpec: %+v", cacheSpec)
 		var nodes []*corev1.Node

cmd/webhook-server/main.go

@@ -24,6 +24,7 @@ import (
 	"io/ioutil"
 	"net/http"
 
+	"github.com/golang/glog"
 	"github.com/senthilrch/kube-fledged/pkg/webhook"
 
 	admissionv1 "k8s.io/api/admission/v1"
@@ -36,7 +37,6 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
-	"k8s.io/klog"
 	// TODO: try this library to see if it generates correct json patch
 	// https://github.com/mattbaird/jsonpatch
 )
@@ -89,7 +89,7 @@ type Config struct {
 func configTLS(config Config) *tls.Config {
 	sCert, err := tls.LoadX509KeyPair(config.CertFile, config.KeyFile)
 	if err != nil {
-		klog.Fatal(err)
+		glog.Fatal(err)
 	}
 	return &tls.Config{
 		Certificates: []tls.Certificate{sCert},
@@ -126,17 +126,17 @@ func serve(w http.ResponseWriter, r *http.Request, admit admitHandler) {
 	// verify the content type is accurate
 	contentType := r.Header.Get("Content-Type")
 	if contentType != "application/json" {
-		klog.Errorf("contentType=%s, expect application/json", contentType)
+		glog.Errorf("contentType=%s, expect application/json", contentType)
 		return
 	}
 
-	klog.V(2).Info(fmt.Sprintf("handling request: %s", body))
+	glog.V(2).Info(fmt.Sprintf("handling request: %s", body))
 
 	deserializer := codecs.UniversalDeserializer()
 	obj, gvk, err := deserializer.Decode(body, nil, nil)
 	if err != nil {
 		msg := fmt.Sprintf("Request could not be decoded: %v", err)
-		klog.Error(msg)
+		glog.Error(msg)
 		http.Error(w, msg, http.StatusBadRequest)
 		return
 	}
@@ -146,7 +146,7 @@ func serve(w http.ResponseWriter, r *http.Request, admit admitHandler) {
 	case v1beta1.SchemeGroupVersion.WithKind("AdmissionReview"):
 		requestedAdmissionReview, ok := obj.(*v1beta1.AdmissionReview)
 		if !ok {
-			klog.Errorf("Expected v1beta1.AdmissionReview but got: %T", obj)
+			glog.Errorf("Expected v1beta1.AdmissionReview but got: %T", obj)
 			return
 		}
 		responseAdmissionReview := &v1beta1.AdmissionReview{}
@@ -157,7 +157,7 @@ func serve(w http.ResponseWriter, r *http.Request, admit admitHandler) {
 	case v1.SchemeGroupVersion.WithKind("AdmissionReview"):
 		requestedAdmissionReview, ok := obj.(*v1.AdmissionReview)
 		if !ok {
-			klog.Errorf("Expected v1.AdmissionReview but got: %T", obj)
+			glog.Errorf("Expected v1.AdmissionReview but got: %T", obj)
 			return
 		}
 		responseAdmissionReview := &v1.AdmissionReview{}
@@ -167,21 +167,21 @@ func serve(w http.ResponseWriter, r *http.Request, admit admitHandler) {
 		responseObj = responseAdmissionReview
 	default:
 		msg := fmt.Sprintf("Unsupported group version kind: %v", gvk)
-		klog.Error(msg)
+		glog.Error(msg)
 		http.Error(w, msg, http.StatusBadRequest)
 		return
 	}
 
-	klog.V(2).Info(fmt.Sprintf("sending response: %v", responseObj))
+	glog.V(2).Info(fmt.Sprintf("sending response: %v", responseObj))
 	respBytes, err := json.Marshal(responseObj)
 	if err != nil {
-		klog.Error(err)
+		glog.Error(err)
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 	w.Header().Set("Content-Type", "application/json")
 	if _, err := w.Write(respBytes); err != nil {
-		klog.Error(err)
+		glog.Error(err)
 	}
 }
 
@@ -279,6 +279,7 @@ func main() {
 		Addr:      fmt.Sprintf(":%d", port),
 		TLSConfig: configTLS(config),
 	}
+	glog.Infof("Wehook server listening on :%d", port)
 	err := server.ListenAndServeTLS("", "")
 	if err != nil {
 		panic(err)

deploy/kubefledged-deployment-webhook-server.yaml

@@ -17,6 +17,7 @@ spec:
       - image: senthilrch/fledged-webhook-server:v0.7.0
         command: ["/opt/bin/fledged-webhook-server"]
         args:
+        - "--stderrthreshold=INFO"
         - "--cert-file=/var/run/secrets/webhook-server/cert.pem"
         - "--key-file=/var/run/secrets/webhook-server/key.pem"
         - "--port=443"

pkg/webhook/imagecache.go

@@ -18,10 +18,13 @@ package webhook
 
 import (
 	"encoding/json"
+	"fmt"
+	"reflect"
 
+	"github.com/golang/glog"
+	fledgedv1alpha1 "github.com/senthilrch/kube-fledged/pkg/apis/fledged/v1alpha1"
 	v1 "k8s.io/api/admission/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/klog"
 )
 
 const (
@@ -35,7 +38,7 @@ const (
 
 // MutateImageCache modifies image cache resource
 func MutateImageCache(ar v1.AdmissionReview) *v1.AdmissionResponse {
-	klog.Info("mutating custom resource")
+	glog.V(4).Info("mutating custom resource")
 	cr := struct {
 		metav1.ObjectMeta
 		Data map[string]string
@@ -44,7 +47,7 @@ func MutateImageCache(ar v1.AdmissionReview) *v1.AdmissionResponse {
 	raw := ar.Request.Object.Raw
 	err := json.Unmarshal(raw, &cr)
 	if err != nil {
-		klog.Error(err)
+		glog.Error(err)
 		return toV1AdmissionResponse(err)
 	}
 
@@ -66,41 +69,85 @@ func MutateImageCache(ar v1.AdmissionReview) *v1.AdmissionResponse {
 
 // ValidateImageCache validates image cache resource
 func ValidateImageCache(ar v1.AdmissionReview) *v1.AdmissionResponse {
-	klog.Info("admitting custom resource")
-	cr := struct {
-		metav1.ObjectMeta
-		Data map[string]string
-	}{}
+	glog.V(4).Info("admitting image cache")
+	var raw, oldraw []byte
+	var imageCache, oldImageCache fledgedv1alpha1.ImageCache
 
-	var raw []byte
-	if ar.Request.Operation == v1.Delete {
-		raw = ar.Request.OldObject.Raw
-	} else {
-		raw = ar.Request.Object.Raw
-	}
-	err := json.Unmarshal(raw, &cr)
+	reviewResponse := v1.AdmissionResponse{}
+	reviewResponse.Allowed = true
+
+	raw = ar.Request.Object.Raw
+	err := json.Unmarshal(raw, &imageCache)
 	if err != nil {
-		klog.Error(err)
+		glog.Error(err)
 		return toV1AdmissionResponse(err)
 	}
 
-	reviewResponse := v1.AdmissionResponse{}
-	reviewResponse.Allowed = true
-	for k, v := range cr.Data {
-		if k == "webhook-e2e-test" && v == "webhook-disallow" &&
-			(ar.Request.Operation == v1.Create || ar.Request.Operation == v1.Update) {
-			reviewResponse.Allowed = false
-			reviewResponse.Result = &metav1.Status{
-				Reason: "the custom resource contains unwanted data",
+	if ar.Request.Operation == v1.Update {
+		oldraw = ar.Request.OldObject.Raw
+		err := json.Unmarshal(oldraw, &oldImageCache)
+		if err != nil {
+			glog.Error(err)
+			return toV1AdmissionResponse(err)
+		}
+		if reflect.DeepEqual(oldImageCache.Spec, imageCache.Spec) {
+			glog.V(4).Info("No change in image cache spec: skipping validation")
+			return &reviewResponse
+		}
+	}
+
+	cacheSpec := imageCache.Spec.CacheSpec
+	glog.V(4).Infof("cacheSpec: %+v", cacheSpec)
+
+	for _, i := range cacheSpec {
+		if len(i.Images) == 0 {
+			glog.Error("No images specified within image list")
+			return toV1AdmissionResponse(fmt.Errorf("No images specified within image list"))
+		}
+
+		for m := range i.Images {
+			for p := 0; p < m; p++ {
+				if i.Images[p] == i.Images[m] {
+					glog.Errorf("Duplicate image names within image list: %s", i.Images[m])
+					return toV1AdmissionResponse(fmt.Errorf("Duplicate image names within image list: %s", i.Images[m]))
+				}
+			}
+		}
+		/*
+			if len(i.NodeSelector) > 0 {
+				if nodes, err = c.nodesLister.List(labels.Set(i.NodeSelector).AsSelector()); err != nil {
+					glog.Errorf("Error listing nodes using nodeselector %+v: %v", i.NodeSelector, err)
+					return err
+				}
+			} else {
+				if nodes, err = c.nodesLister.List(labels.Everything()); err != nil {
+					glog.Errorf("Error listing nodes using nodeselector labels.Everything(): %v", err)
+					return err
+				}
+			}
+			glog.V(4).Infof("No. of nodes in %+v is %d", i.NodeSelector, len(nodes))
+			if len(nodes) == 0 {
+				glog.Errorf("NodeSelector %s did not match any nodes.", labels.Set(i.NodeSelector).String())
+				return fmt.Errorf("NodeSelector %s did not match any nodes", labels.Set(i.NodeSelector).String())
 			}
+		*/
+	}
+
+	if ar.Request.Operation == v1.Update {
+		if len(oldImageCache.Spec.CacheSpec) != len(imageCache.Spec.CacheSpec) {
+			glog.Errorf("Mismatch in no. of image lists")
+			return toV1AdmissionResponse(fmt.Errorf("Mismatch in no. of image lists"))
 		}
-		if k == "webhook-e2e-test" && v == "webhook-nondeletable" && ar.Request.Operation == v1.Delete {
-			reviewResponse.Allowed = false
-			reviewResponse.Result = &metav1.Status{
-				Reason: "the custom resource cannot be deleted because it contains unwanted key and value",
+
+		for i := range oldImageCache.Spec.CacheSpec {
+			if !reflect.DeepEqual(oldImageCache.Spec.CacheSpec[i].NodeSelector, imageCache.Spec.CacheSpec[i].NodeSelector) {
+				glog.Errorf("Mismatch in node selector")
+				return toV1AdmissionResponse(fmt.Errorf("Mismatch in node selector"))
 			}
 		}
 	}
+
+	glog.Info("Image cache creation/update validated successfully")
 	return &reviewResponse
 }