Merge pull request #207 from nirmata/173_uniqueness_of_ruleName

Validate uniqueness of the rule name within the policy

pkg/config/config.go

@@ -15,6 +15,10 @@ const (
 	ValidatingWebhookConfigurationDebug = "kyverno-validating-webhook-cfg-debug"
 	ValidatingWebhookName               = "nirmata.kyverno.validating-webhook"
 
+	PolicyValidatingWebhookConfigurationName  = "kyverno-policy-validating-webhook-cfg"
+	PolicyValidatingWebhookConfigurationDebug = "kyverno-policy-validating-webhook-cfg-debug"
+	PolicyValidatingWebhookName               = "nirmata.kyverno.policy-validating-webhook"
+
 	// Due to kubernetes issue, we must use next literal constants instead of deployment TypeMeta fields
 	// Issue: https://github.com/kubernetes/kubernetes/pull/63972
 	// When the issue is closed, we should use TypeMeta struct instead of this constants
@@ -24,9 +28,10 @@ const (
 )
 
 var (
-	MutatingWebhookServicePath   = "/mutate"
-	ValidatingWebhookServicePath = "/validate"
-	KubePolicyAppLabels          = map[string]string{
+	MutatingWebhookServicePath         = "/mutate"
+	ValidatingWebhookServicePath       = "/validate"
+	PolicyValidatingWebhookServicePath = "/policyvalidate"
+	KubePolicyAppLabels                = map[string]string{
 		"app": "kyverno",
 	}
 

pkg/utils/util.go

@@ -0,0 +1,10 @@
+package utils
+
+func Contains(list []string, element string) bool {
+	for _, e := range list {
+		if e == element {
+			return true
+		}
+	}
+	return false
+}

pkg/utils/util_test.go

@@ -0,0 +1,40 @@
+package utils
+
+import (
+	"testing"
+
+	"gotest.tools/assert"
+)
+
+func Test_allEmpty(t *testing.T) {
+	var list []string
+	var element string
+	res := Contains(list, element)
+	assert.Assert(t, res == false)
+}
+
+func Test_emptyList(t *testing.T) {
+	var list []string
+	element := "foo"
+	res := Contains(list, element)
+	assert.Assert(t, res == false)
+}
+
+func Test_emptyElement(t *testing.T) {
+	list := []string{"foo", "bar"}
+	var element string
+	res := Contains(list, element)
+	assert.Assert(t, res == false)
+}
+
+func Test_emptyElementInList(t *testing.T) {
+	list := []string{"foo", "bar", ""}
+	var element string
+	res := Contains(list, element)
+	assert.Assert(t, res == true)
+
+	list = []string{"foo", "bar", "bar"}
+	element = "bar"
+	res = Contains(list, element)
+	assert.Assert(t, res == true)
+}

pkg/webhooks/registration.go

@@ -4,12 +4,14 @@ import (
 	"errors"
 	"fmt"
 	"io/ioutil"
+	"strings"
 
 	"github.com/golang/glog"
 	"github.com/nirmata/kyverno/pkg/config"
 	client "github.com/nirmata/kyverno/pkg/dclient"
 
 	admregapi "k8s.io/api/admissionregistration/v1beta1"
+	errorsapi "k8s.io/apimachinery/pkg/api/errors"
 	meta "k8s.io/apimachinery/pkg/apis/meta/v1"
 	admregclient "k8s.io/client-go/kubernetes/typed/admissionregistration/v1beta1"
 	rest "k8s.io/client-go/rest"
@@ -67,6 +69,16 @@ func (wrc *WebhookRegistrationClient) Register() error {
 		return err
 	}
 
+	policyValidationWebhookConfig, err := wrc.contructPolicyValidatingWebhookConfig()
+	if err != nil {
+		return err
+	}
+
+	_, err = wrc.registrationClient.ValidatingWebhookConfigurations().Create(policyValidationWebhookConfig)
+	if err != nil {
+		return err
+	}
+
 	return nil
 }
 
@@ -75,13 +87,27 @@ func (wrc *WebhookRegistrationClient) Register() error {
 // Register will fail if the config exists, so there is no need to fail on error
 func (wrc *WebhookRegistrationClient) Deregister() {
 	if wrc.serverIP != "" {
-		wrc.registrationClient.MutatingWebhookConfigurations().Delete(config.MutatingWebhookConfigurationDebug, &meta.DeleteOptions{})
-		wrc.registrationClient.ValidatingWebhookConfigurations().Delete(config.ValidatingWebhookConfigurationDebug, &meta.DeleteOptions{})
+		if err := wrc.registrationClient.MutatingWebhookConfigurations().Delete(config.MutatingWebhookConfigurationDebug, &meta.DeleteOptions{}); err != nil {
+			if !errorsapi.IsNotFound(err) {
+				glog.Errorf("Failed to deregister debug mutatingWebhookConfiguratinos, err: %v\n", err)
+			}
+		}
+		if err := wrc.registrationClient.ValidatingWebhookConfigurations().Delete(config.ValidatingWebhookConfigurationDebug, &meta.DeleteOptions{}); err != nil {
+			if !errorsapi.IsNotFound(err) {
+				glog.Errorf("Failed to deregister debug validatingWebhookConfiguratinos, err: %v\n", err)
+			}
+		}
+		if err := wrc.registrationClient.ValidatingWebhookConfigurations().Delete(config.PolicyValidatingWebhookConfigurationDebug, &meta.DeleteOptions{}); err != nil {
+			if !errorsapi.IsNotFound(err) {
+				glog.Errorf("Failed to deregister debug policyValidatingWebhookConfiguratinos, err: %v\n", err)
+			}
+		}
 		return
 	}
 
 	wrc.registrationClient.MutatingWebhookConfigurations().Delete(config.MutatingWebhookConfigurationName, &meta.DeleteOptions{})
 	wrc.registrationClient.ValidatingWebhookConfigurations().Delete(config.ValidatingWebhookConfigurationName, &meta.DeleteOptions{})
+	wrc.registrationClient.ValidatingWebhookConfigurations().Delete(config.PolicyValidatingWebhookConfigurationName, &meta.DeleteOptions{})
 }
 
 func (wrc *WebhookRegistrationClient) constructMutatingWebhookConfig(configuration *rest.Config) (*admregapi.MutatingWebhookConfiguration, error) {
@@ -175,7 +201,7 @@ func (wrc *WebhookRegistrationClient) contructDebugValidatingWebhookConfig(caDat
 
 	return &admregapi.ValidatingWebhookConfiguration{
 		ObjectMeta: meta.ObjectMeta{
-			Name:   config.ValidatingWebhookConfigurationName,
+			Name:   config.ValidatingWebhookConfigurationDebug,
 			Labels: config.KubePolicyAppLabels,
 		},
 		Webhooks: []admregapi.Webhook{
@@ -187,7 +213,67 @@ func (wrc *WebhookRegistrationClient) contructDebugValidatingWebhookConfig(caDat
 	}
 }
 
+func (wrc *WebhookRegistrationClient) contructPolicyValidatingWebhookConfig() (*admregapi.ValidatingWebhookConfiguration, error) {
+	// Check if ca is defined in the secret tls-ca
+	// assume the key and signed cert have been defined in secret tls.kyverno
+	caData := wrc.client.ReadRootCASecret()
+	if len(caData) == 0 {
+		// load the CA from kubeconfig
+		caData = extractCA(wrc.clientConfig)
+	}
+	if len(caData) == 0 {
+		return nil, errors.New("Unable to extract CA data from configuration")
+	}
+
+	if wrc.serverIP != "" {
+		return wrc.contructDebugPolicyValidatingWebhookConfig(caData), nil
+	}
+
+	return &admregapi.ValidatingWebhookConfiguration{
+		ObjectMeta: meta.ObjectMeta{
+			Name:   config.PolicyValidatingWebhookConfigurationName,
+			Labels: config.KubePolicyAppLabels,
+			OwnerReferences: []meta.OwnerReference{
+				wrc.constructOwner(),
+			},
+		},
+		Webhooks: []admregapi.Webhook{
+			constructWebhook(
+				config.PolicyValidatingWebhookName,
+				config.PolicyValidatingWebhookServicePath,
+				caData),
+		},
+	}, nil
+}
+
+func (wrc *WebhookRegistrationClient) contructDebugPolicyValidatingWebhookConfig(caData []byte) *admregapi.ValidatingWebhookConfiguration {
+	url := fmt.Sprintf("https://%s%s", wrc.serverIP, config.PolicyValidatingWebhookServicePath)
+	glog.V(3).Infof("Debug PolicyValidatingWebhookConfig is registered with url %s\n", url)
+
+	return &admregapi.ValidatingWebhookConfiguration{
+		ObjectMeta: meta.ObjectMeta{
+			Name:   config.PolicyValidatingWebhookConfigurationDebug,
+			Labels: config.KubePolicyAppLabels,
+		},
+		Webhooks: []admregapi.Webhook{
+			constructDebugWebhook(
+				config.PolicyValidatingWebhookName,
+				url,
+				caData),
+		},
+	}
+}
+
 func constructWebhook(name, servicePath string, caData []byte) admregapi.Webhook {
+	resource := "*/*"
+	apiGroups := "*"
+	apiversions := "*"
+	if servicePath == config.PolicyValidatingWebhookServicePath {
+		resource = "policies/*"
+		apiGroups = "kyverno.io"
+		apiversions = "v1alpha1"
+	}
+
 	return admregapi.Webhook{
 		Name: name,
 		ClientConfig: admregapi.WebhookClientConfig{
@@ -206,13 +292,13 @@ func constructWebhook(name, servicePath string, caData []byte) admregapi.Webhook
 				},
 				Rule: admregapi.Rule{
 					APIGroups: []string{
-						"*",
+						apiGroups,
 					},
 					APIVersions: []string{
-						"*",
+						apiversions,
 					},
 					Resources: []string{
-						"*/*",
+						resource,
 					},
 				},
 			},
@@ -221,6 +307,16 @@ func constructWebhook(name, servicePath string, caData []byte) admregapi.Webhook
 }
 
 func constructDebugWebhook(name, url string, caData []byte) admregapi.Webhook {
+	resource := "*/*"
+	apiGroups := "*"
+	apiversions := "*"
+
+	if strings.Contains(url, config.PolicyValidatingWebhookServicePath) {
+		resource = "policies/*"
+		apiGroups = "kyverno.io"
+		apiversions = "v1alpha1"
+	}
+
 	return admregapi.Webhook{
 		Name: name,
 		ClientConfig: admregapi.WebhookClientConfig{
@@ -235,13 +331,13 @@ func constructDebugWebhook(name, url string, caData []byte) admregapi.Webhook {
 				},
 				Rule: admregapi.Rule{
 					APIGroups: []string{
-						"*",
+						apiGroups,
 					},
 					APIVersions: []string{
-						"*",
+						apiversions,
 					},
 					Resources: []string{
-						"*/*",
+						resource,
 					},
 				},
 			},

pkg/webhooks/server.go

@@ -12,6 +12,7 @@ import (
 	"time"
 
 	"github.com/golang/glog"
+	policyv1 "github.com/nirmata/kyverno/pkg/apis/policy/v1alpha1"
 	"github.com/nirmata/kyverno/pkg/client/listers/policy/v1alpha1"
 	"github.com/nirmata/kyverno/pkg/config"
 	client "github.com/nirmata/kyverno/pkg/dclient"
@@ -20,11 +21,14 @@ import (
 	"github.com/nirmata/kyverno/pkg/info"
 	"github.com/nirmata/kyverno/pkg/sharedinformer"
 	tlsutils "github.com/nirmata/kyverno/pkg/tls"
+	"github.com/nirmata/kyverno/pkg/utils"
 	v1beta1 "k8s.io/api/admission/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 )
 
+const policyKind = "Policy"
+
 // WebhookServer contains configured TLS server with MutationWebhook.
 // MutationWebhook gets policies from policyController and takes control of the cluster with kubeclient.
 type WebhookServer struct {
@@ -64,6 +68,7 @@ func NewWebhookServer(
 	mux := http.NewServeMux()
 	mux.HandleFunc(config.MutatingWebhookServicePath, ws.serve)
 	mux.HandleFunc(config.ValidatingWebhookServicePath, ws.serve)
+	mux.HandleFunc(config.PolicyValidatingWebhookServicePath, ws.serve)
 
 	ws.server = http.Server{
 		Addr:         ":443", // Listen on port for HTTPS requests
@@ -86,6 +91,7 @@ func (ws *WebhookServer) serve(w http.ResponseWriter, r *http.Request) {
 	admissionReview.Response = &v1beta1.AdmissionResponse{
 		Allowed: true,
 	}
+
 	// Do not process the admission requests for kinds that are in filterKinds for filtering
 	if !StringInSlice(admissionReview.Request.Kind.Kind, ws.filterKinds) {
 
@@ -94,13 +100,14 @@ func (ws *WebhookServer) serve(w http.ResponseWriter, r *http.Request) {
 			admissionReview.Response = ws.HandleMutation(admissionReview.Request)
 		case config.ValidatingWebhookServicePath:
 			admissionReview.Response = ws.HandleValidation(admissionReview.Request)
+		case config.PolicyValidatingWebhookServicePath:
+			admissionReview.Response = ws.HandlePolicyValidation(admissionReview.Request)
 		}
 	}
 
 	admissionReview.Response.UID = admissionReview.Request.UID
 
 	responseJSON, err := json.Marshal(admissionReview)
-
 	if err != nil {
 		http.Error(w, fmt.Sprintf("Could not encode response: %v", err), http.StatusInternalServerError)
 		return
@@ -382,7 +389,37 @@ func (ws *WebhookServer) bodyToAdmissionReview(request *http.Request, writer htt
 	return admissionReview
 }
 
-const policyKind = "Policy"
+func (ws *WebhookServer) HandlePolicyValidation(request *v1beta1.AdmissionRequest) *v1beta1.AdmissionResponse {
+	return ws.validateUniqueRuleName(request.Object.Raw)
+}
+
+func (ws *WebhookServer) validateUniqueRuleName(rawPolicy []byte) *v1beta1.AdmissionResponse {
+	var policy *policyv1.Policy
+	var ruleNames []string
+
+	json.Unmarshal(rawPolicy, &policy)
+
+	for _, rule := range policy.Spec.Rules {
+		if utils.Contains(ruleNames, rule.Name) {
+			msg := fmt.Sprintf(`The policy "%s" is invalid: duplicate rule name: "%s"`, policy.Name, rule.Name)
+			glog.Errorln(msg)
+
+			return &v1beta1.AdmissionResponse{
+				Allowed: false,
+				Result: &metav1.Status{
+					Message: msg,
+				},
+			}
+		} else {
+			ruleNames = append(ruleNames, rule.Name)
+		}
+	}
+
+	glog.V(3).Infof("Policy validation passed.")
+	return &v1beta1.AdmissionResponse{
+		Allowed: true,
+	}
+}
 
 func newEventInfoFromPolicyInfo(policyInfoList []*info.PolicyInfo, onUpdate bool) []*event.Info {
 	var eventsInfo []*event.Info

scripts/deploy-controller-debug.sh

@@ -33,5 +33,3 @@ chmod +x "${certsGenerator}"
 
 ${certsGenerator} "--service=${service}" "--serverIP=${serverIP}" || exit 2
 echo -e "\n### You can build and run kyverno project locally.\n### To check its work, run it with flags --kubeconfig and --serverIP parameters."
-
-sudo ./kyverno --kubeconfig ~/.kube/config --serverIP=10.0.0.11 -v 4

scripts/generate-self-signed-cert-and-k8secrets-debug.sh

@@ -55,7 +55,6 @@ openssl x509 -req -in ${destdir}/webhook.csr -CA ${destdir}/rootCA.crt -CAkey ${
 
 
 kubectl delete -f definitions/install_debug.yaml 2>/dev/null
-kubectl delete csr,MutatingWebhookConfiguration,ValidatingWebhookConfiguration --all 2>/dev/null
 
 echo "Generating corresponding kubernetes secrets for TLS pair and root CA"
 # create project namespace