PR code review changes

client/certificates.go

@@ -42,23 +42,18 @@ func (c *Client) GenerateTlsPemPair(props tls.TlsCertificateProps) (*tls.TlsPemP
 
 // Submits and approves certificate request, returns request which need to be fetched
 func (c *Client) submitAndApproveCertificateRequest(req *certificates.CertificateSigningRequest) (*certificates.CertificateSigningRequest, error) {
-	//TODO: using the CSR interface from the kubeclient
 	certClient, err := c.GetCSRInterface()
 	if err != nil {
 		return nil, err
 	}
-	//	certClient := kc.client.CertificatesV1beta1().CertificateSigningRequests()
-	csrList, err := c.ListResource("certificatesigningrequests", "")
-	//	csrList, err := certClient.List(metav1.ListOptions{})
+	csrList, err := c.ListResource(CSR, "")
 	if err != nil {
 		return nil, errors.New(fmt.Sprintf("Unable to list existing certificate requests: %v", err))
 	}
 
 	for _, csr := range csrList.Items {
-		csr.GetName()
 		if csr.GetName() == req.ObjectMeta.Name {
-			// Delete
-			err := c.DeleteResouce("certificatesigningrequests", "", csr.GetName())
+			err := c.DeleteResouce(CSR, "", csr.GetName())
 			if err != nil {
 				return nil, errors.New(fmt.Sprintf("Unable to delete existing certificate request: %v", err))
 			}
@@ -67,9 +62,7 @@ func (c *Client) submitAndApproveCertificateRequest(req *certificates.Certificat
 		}
 	}
 
-	// Create
-	unstrRes, err := c.CreateResource("certificatesigningrequests", "", req)
-	//	res, err := certClient.Create(req)
+	unstrRes, err := c.CreateResource(CSR, "", req)
 	if err != nil {
 		return nil, err
 	}
@@ -93,15 +86,12 @@ func (c *Client) submitAndApproveCertificateRequest(req *certificates.Certificat
 	return res, nil
 }
 
-const certificateFetchWaitInterval time.Duration = 200 * time.Millisecond
-
 // Fetches certificate from given request. Tries to obtain certificate for maxWaitSeconds
 func (c *Client) fetchCertificateFromRequest(req *certificates.CertificateSigningRequest, maxWaitSeconds uint8) ([]byte, error) {
 	// TODO: react of SIGINT and SIGTERM
 	timeStart := time.Now()
-	c.GetResource("certificatesigningrequests", "", req.ObjectMeta.Name)
 	for time.Now().Sub(timeStart) < time.Duration(maxWaitSeconds)*time.Second {
-		unstrR, err := c.GetResource("certificatesigningrequests", "", req.ObjectMeta.Name)
+		unstrR, err := c.GetResource(CSR, "", req.ObjectMeta.Name)
 		if err != nil {
 			return nil, err
 		}
@@ -129,7 +119,7 @@ const certificateField string = "certificate"
 // Reads the pair of TLS certificate and key from the specified secret.
 func (c *Client) ReadTlsPair(props tls.TlsCertificateProps) *tls.TlsPemPair {
 	name := generateSecretName(props)
-	unstrSecret, err := c.GetResource("secrets", props.Namespace, name)
+	unstrSecret, err := c.GetResource(Secret, props.Namespace, name)
 	if err != nil {
 		c.logger.Printf("Unable to get secret %s/%s: %s", props.Namespace, name, err)
 		return nil
@@ -157,8 +147,8 @@ func (c *Client) ReadTlsPair(props tls.TlsCertificateProps) *tls.TlsPemPair {
 // Updates existing secret or creates new one.
 func (c *Client) WriteTlsPair(props tls.TlsCertificateProps, pemPair *tls.TlsPemPair) error {
 	name := generateSecretName(props)
-	unstrSecret, err := c.GetResource("secrets", props.Namespace, name)
-	if err == nil { // Update existing secret
+	unstrSecret, err := c.GetResource(Secret, props.Namespace, name)
+	if err == nil {
 		secret, err := convertToSecret(unstrSecret)
 		if err != nil {
 			return nil
@@ -169,12 +159,12 @@ func (c *Client) WriteTlsPair(props tls.TlsCertificateProps, pemPair *tls.TlsPem
 		}
 		secret.Data[certificateField] = pemPair.Certificate
 		secret.Data[privateKeyField] = pemPair.PrivateKey
-		c.UpdateResource("secrets", props.Namespace, secret)
+		_, err = c.UpdateResource(Secret, props.Namespace, secret)
 		if err == nil {
 			c.logger.Printf("Secret %s is updated", name)
 		}
 
-	} else { // Create new secret
+	} else {
 
 		secret := &v1.Secret{
 			TypeMeta: metav1.TypeMeta{
@@ -191,7 +181,7 @@ func (c *Client) WriteTlsPair(props tls.TlsCertificateProps, pemPair *tls.TlsPem
 			},
 		}
 
-		_, err := c.CreateResource("secrets", props.Namespace, secret)
+		_, err := c.CreateResource(Secret, props.Namespace, secret)
 		if err == nil {
 			c.logger.Printf("Secret %s is created", name)
 		}

client/client.go

@@ -29,7 +29,7 @@ type Client struct {
 	clientConfig *rest.Config
 }
 
-func NewDynamicClient(config *rest.Config, logger *log.Logger) (*Client, error) {
+func NewClient(config *rest.Config, logger *log.Logger) (*Client, error) {
 	client, err := dynamic.NewForConfig(config)
 	if err != nil {
 		return nil, err
@@ -179,7 +179,7 @@ func (c *Client) GenerateSecret(generator types.Generation, namespace string) er
 	//	if generator.CopyFrom != nil {
 	c.logger.Printf("Copying data from secret %s/%s", generator.CopyFrom.Namespace, generator.CopyFrom.Name)
 	// Get configMap resource
-	unstrSecret, err := c.GetResource("secrets", generator.CopyFrom.Namespace, generator.CopyFrom.Name)
+	unstrSecret, err := c.GetResource(Secret, generator.CopyFrom.Namespace, generator.CopyFrom.Name)
 	if err != nil {
 		return err
 	}
@@ -286,7 +286,7 @@ func (c *Client) createConfigMapAfterNamespaceIsCreated(configMap v1.ConfigMap,
 func (c *Client) createSecretAfterNamespaceIsCreated(secret v1.Secret, namespace string) {
 	err := c.waitUntilNamespaceIsCreated(namespace)
 	if err == nil {
-		_, err = c.CreateResource("secrets", namespace, secret)
+		_, err = c.CreateResource(Secret, namespace, secret)
 	}
 	if err != nil {
 		c.logger.Printf("Can't create a secret: %s", err)

client/utils.go

@@ -11,10 +11,15 @@ import (
 	"k8s.io/client-go/rest"
 )
 
+const (
+	CSR    string = "certificatesigningrequests"
+	Secret string = "secrets"
+)
 const namespaceCreationMaxWaitTime time.Duration = 30 * time.Second
 const namespaceCreationWaitInterval time.Duration = 100 * time.Millisecond
 
 var groupVersionMapper map[string]schema.GroupVersionResource
+var kubeClient *kubernetes.Clientset
 
 func getGrpVersionMapper(kind string, clientConfig *rest.Config, refresh bool) schema.GroupVersionResource {
 	// build the GVK mapper
@@ -83,9 +88,12 @@ func refreshRegisteredResources(mapper map[string]schema.GroupVersionResource, c
 }
 
 func newKubeClient(clientConfig *rest.Config) (*kubernetes.Clientset, error) {
-	client, err := kubernetes.NewForConfig(clientConfig)
-	if err != nil {
-		return nil, err
+	var err error
+	if kubeClient == nil {
+		kubeClient, err = kubernetes.NewForConfig(clientConfig)
+		if err != nil {
+			return nil, err
+		}
 	}
-	return client, nil
+	return kubeClient, nil
 }

examples/ConfigMapGenerator-SecretGenerator/policy-cm-test.yaml

@@ -4,15 +4,17 @@ metadata :
   name: "policy-configmapgenerator-test"
 spec:
   rules:
-  - name: "Policy ConfigMap sample rule"
-    resource:
+  - name: "copyCM"
+    resource :
       kind : Namespace
-      name: "ns2"
-    generate:
+      selector:
+        matchLabels:
+          LabelForSelector : "namespace2"
+    generate :
       kind: ConfigMap
-      name: copied-cm
-      copyFrom:
-        namespace: default
-        name: game-config
-      data:
+      name : copied-cm
+      copyFrom :
+        namespace : default
+        name : game-config
+      data :
         secretData: "data from cmg"

examples/ConfigMapGenerator-SecretGenerator/policy-namespace-patch-cmgCG-sgCG.yaml

@@ -3,42 +3,47 @@
 # To apply this policy you need to create secret and configMap in "default" namespace 
 # and then create a namespace
 
-apiVersion : policy.nirmata.io/v1alpha1
+apiVersion : kubepolicy.nirmata.io/v1alpha1
 kind : Policy
 metadata :
   name : "policy-ns-patch-cmg-sg"
 spec :
-  failurePolicy: stopOnError
   rules:
-  - resource :
+  - name: "patchNamespace2"
+    resource :
       kind : Namespace
       selector:
         matchLabels:
           LabelForSelector : "namespace2"
-    patch:
-    - path: "/metadata/labels/isMutatedByPolicy"
-      op: add
-      value: "true"
+    mutate:
+      patches:
+        - path: "/metadata/labels/isMutatedByPolicy"
+          op: add
+          value: "true"
 
-  - resource :
+  - name: "copyCM"
+    resource :
       kind : Namespace
       selector:
         matchLabels:
           LabelForSelector : "namespace2"
-    configMapGenerator :
+    generate :
+      kind: ConfigMap
       name : copied-cm
       copyFrom :
         namespace : default
         name : game-config
       data :
         secretData: "data from cmg"
-        
-  - resource :
+       
+  - name: "generateCM"
+    resource :
       kind : Namespace
       selector:
         matchLabels:
           LabelForSelector : "namespace2"
-    configMapGenerator :
+    generate :
+      kind: ConfigMap
       name : generated-cm
       data :
         secretData: "very sensitive data from cmg"
@@ -49,13 +54,12 @@ spec :
           image.public.key=771 
           rsa.public.key=42
   
-  - resource :
+  - name: "generateSecret"
+    resource :
       kind : Namespace
-      selector:
-        matchLabels:
-          LabelForSelector : "namespace2"
-
-    secretGenerator :
+      name: ns2
+    generate :
+      kind: Secret
       name : generated-secrets
       data :
         foo : bar
@@ -66,13 +70,12 @@ spec :
           foo1=bar1
           foo2=bar2
 
-  - resource :
+  - name: "copySecret"
+    resource :
       kind : Namespace
-      selector:
-        matchLabels:
-          LabelForSelector : "namespace2"
-
-    secretGenerator :
+      name: ns2
+    generate :
+      kind: Secret
       name : copied-secrets
       copyFrom :
         namespace : default
@@ -80,4 +83,3 @@ spec :
       data :
         foo : bar
         secretData: "data from sg"
-

main.go

@@ -26,7 +26,7 @@ func main() {
 		log.Fatalf("Error building kubeconfig: %v\n", err)
 	}
 
-	client, err := client.NewDynamicClient(clientConfig, nil)
+	client, err := client.NewClient(clientConfig, nil)
 	if err != nil {
 		log.Fatalf("Error creating client: %v\n", err)
 	}

pkg/controller/controller.go

@@ -160,8 +160,6 @@ func (pc *PolicyController) syncHandler(obj interface{}) error {
 	if key, ok = obj.(string); !ok {
 		return fmt.Errorf("expected string in workqueue but got %#v", obj)
 	}
-	// convert the namespace/name string into distinct namespace and name
-	//TODO: currently policies are clustered, but the above code is to support namespaced as well
 	_, name, err := cache.SplitMetaNamespaceKey(key)
 	if err != nil {
 		utilruntime.HandleError(fmt.Errorf("invalid policy key: %s", key))

pkg/controller/processPolicy.go

@@ -1,175 +0,0 @@
-package controller
-
-import (
-	"fmt"
-
-	types "github.com/nirmata/kube-policy/pkg/apis/policy/v1alpha1"
-	engine "github.com/nirmata/kube-policy/pkg/engine"
-	"github.com/nirmata/kube-policy/pkg/engine/mutation"
-	event "github.com/nirmata/kube-policy/pkg/event"
-	violation "github.com/nirmata/kube-policy/pkg/violation"
-	"k8s.io/apimachinery/pkg/labels"
-	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
-)
-
-func (pc *PolicyController) runForPolicy(key string) {
-
-	policy, err := pc.getPolicyByKey(key)
-	if err != nil {
-		utilruntime.HandleError(fmt.Errorf("invalid resource key: %s, err: %v", key, err))
-		return
-	}
-
-	if policy == nil {
-		pc.logger.Printf("Could not find policy by key %s", key)
-		return
-	}
-
-	violations, events, err := pc.processPolicy(*policy)
-	if err != nil {
-		// add Error processing policy event
-	}
-
-	pc.logger.Printf("%v, %v", violations, events)
-	// TODO:
-	// create violations
-	//	pc.violationBuilder.Add()
-	// create events
-	//	pc.eventBuilder.Add()
-
-}
-
-// processPolicy process the policy to all the matched resources
-func (pc *PolicyController) processPolicy(policy types.Policy) (
-	violations []violation.Info, events []event.Info, err error) {
-
-	for _, rule := range policy.Spec.Rules {
-		resources, err := pc.filterResourceByRule(rule)
-		if err != nil {
-			pc.logger.Printf("Failed to filter resources by rule %s, err: %v\n", rule.Name, err)
-		}
-
-		for _, resource := range resources {
-			if err != nil {
-				pc.logger.Printf("Failed to marshal resources map to rule %s, err: %v\n", rule.Name, err)
-				continue
-			}
-
-			violation, eventInfos, err := engine.ProcessExisting(policy, resource)
-			if err != nil {
-				pc.logger.Printf("Failed to process rule %s, err: %v\n", rule.Name, err)
-				continue
-			}
-
-			violations = append(violations, violation...)
-			events = append(events, eventInfos...)
-		}
-	}
-	return violations, events, nil
-}
-
-func (pc *PolicyController) filterResourceByRule(rule types.Rule) ([][]byte, error) {
-	var targetResources [][]byte
-	// TODO: make this namespace all
-	var namespace = "default"
-	if err := rule.Validate(); err != nil {
-		return nil, fmt.Errorf("invalid rule detected: %s, err: %v", rule.Name, err)
-	}
-
-	// Get the resource list from kind
-	resources, err := pc.client.ListResource(rule.ResourceDescription.Kind, namespace)
-	if err != nil {
-		return nil, err
-	}
-
-	for _, resource := range resources.Items {
-		// TODO:
-		//rawResource, err := json.Marshal(resource)
-		// objKind := resource.GetObjectKind()
-		// codecFactory := serializer.NewCodecFactory(runtime.NewScheme())
-		// codecFactory.EncoderForVersion()
-
-		if err != nil {
-			pc.logger.Printf("failed to marshal object %v", resource)
-			continue
-		}
-
-		// filter the resource by name and label
-		//if ok, _ := mutation.ResourceMeetsRules(rawResource, rule.ResourceDescription); ok {
-		//	targetResources = append(targetResources, resource)
-		//}
-	}
-	return targetResources, nil
-}
-
-func (pc *PolicyController) getPolicyByKey(key string) (*types.Policy, error) {
-	// Create nil Selector to grab all the policies
-	selector := labels.NewSelector()
-	cachedPolicies, err := pc.policyLister.List(selector)
-	if err != nil {
-		return nil, err
-	}
-
-	for _, elem := range cachedPolicies {
-		if elem.Name == key {
-			return elem, nil
-		}
-	}
-	return nil, nil
-}
-
-//TODO wrap the generate, mutation & validation functions for the existing resources
-//ProcessExisting processes the policy rule types for the existing resources
-func (pc *PolicyController) processExisting(policy types.Policy, rawResource []byte) ([]violation.Info, []event.Info, error) {
-	// Generate
-	// generatedDataList := engine.Generate(pc.logger, policy, rawResource)
-	// // apply the generateData using the kubeClient
-	// err = pc.applyGenerate(generatedDataList)
-	// if err != nil {
-	// 	return nil, nil, err
-	// }
-	// // Mutation
-	// mutationPatches, err := engine.Mutation(pc.logger, policy, rawResource)
-	// if err != nil {
-	// 	return nil, nil, err
-	// }
-	// // Apply mutationPatches on the rawResource
-	// err = pc.applyPatches(mutationPatches, rawResource)
-	// if err != nil {
-	// 	return nil, nil, err
-	// }
-	// //Validation
-	// validate, _, _ := engine.Validation(policy, rawResource)
-	// if !validate {
-	// 	// validation has errors -> so there will be violations
-	// 	// call the violatio builder to apply the violations
-	// }
-	// // Generate events
-
-	return nil, nil, nil
-}
-
-//TODO: return events and policy violations
-// func (pc *PolicyController) applyGenerate(generatedDataList []engine.GenerationResponse) error {
-// 	// for _, generateData := range generatedDataList {
-// 	// 	switch generateData.Generator.Kind {
-// 	// 	case "ConfigMap":
-// 	// 		err := pc.client.GenerateConfigMap(generateData.Generator, generateData.Namespace)
-// 	// 		if err != nil {
-// 	// 			return err
-// 	// 		}
-// 	// 	case "Secret":
-// 	// 		err := pc.client.GenerateSecret(generateData.Generator, generateData.Namespace)
-// 	// 		if err != nil {
-// 	// 			return err
-// 	// 		}
-// 	// 	default:
-// 	// 		return errors.New("Unsuported config kind")
-// 	// 	}
-// 	// }
-// 	return nil
-// }
-
-func (pc *PolicyController) applyPatches([]mutation.PatchBytes, []byte) error {
-	return nil
-}

pkg/event/controller.go

@@ -62,7 +62,11 @@ func NewEventController(client *client.Client,
 
 func initRecorder(client *client.Client) record.EventRecorder {
 	// Initliaze Event Broadcaster
-	policyscheme.AddToScheme(scheme.Scheme)
+	err := policyscheme.AddToScheme(scheme.Scheme)
+	if err != nil {
+		utilruntime.HandleError(err)
+		return nil
+	}
 	eventBroadcaster := record.NewBroadcaster()
 	eventBroadcaster.StartLogging(log.Printf)
 	eventInterface, err := client.GetEventsInterface()
@@ -151,13 +155,6 @@ func (c *controller) SyncHandler(key Info) error {
 		if err != nil {
 			return err
 		}
-		//TODO: Test if conversion from unstructured to runtime.Object is implicit or explicit conversion is required
-		// resourceobj, err := client.ConvertToRuntimeObject(resource)
-		// if err != nil {
-		// 	return err
-		// }
-
-		//		resource, err = c.kubeClient.GetResource(key.Kind, key.Resource)
 		if err != nil {
 			utilruntime.HandleError(fmt.Errorf("unable to create event for resource %s, will retry ", key.Resource))
 			return err

pkg/violation/builder.go

@@ -104,9 +104,7 @@ func (b *builder) isActive(kind string, resource string) (bool, error) {
 		return false, err
 	}
 	// Generate Merge Patch
-	//TODO: test for namspace and clustered object
 	_, err = b.client.GetResource(kind, namespace, name)
-	//	_, err := b.kubeClient.GetResource(kind, resource)
 	if err != nil {
 		utilruntime.HandleError(fmt.Errorf("unable to get resource %s ", resource))
 		return false, err