NK-23: Thre creation of default loggers moved to inside classes.

Removed fatal termination from object constructors.
Implemented new KubeClient class with test method which creates a Secret.
Improved comments for the types structures.
Added WebhookServerConfig structure instead of the most parameters to NewWebhookServer.

controller/controller.go

@@ -1,13 +1,14 @@
 package controller
 
 import (
+	"errors"
 	"log"
 	"os"
 	"time"
 
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/cache"
-    "k8s.io/client-go/tools/clientcmd"
 
 	types "github.com/nirmata/kube-policy/pkg/apis/policy/v1alpha1"
 	clientset "github.com/nirmata/kube-policy/pkg/client/clientset/versioned"
@@ -23,20 +24,17 @@ type PolicyController struct {
 }
 
 // NewPolicyController from cmd args
-func NewPolicyController(masterURL, kubeconfigPath string, logger *log.Logger) (*PolicyController, error) {
+func NewPolicyController(config *rest.Config, logger *log.Logger) (*PolicyController, error) {
 	if logger == nil {
-        logger = log.New(os.Stdout, "", log.LstdFlags|log.Lshortfile)
+		logger = log.New(os.Stdout, "Policy Controller: ", log.LstdFlags|log.Lshortfile)
 	}
 
-    cfg, err := clientcmd.BuildConfigFromFlags(masterURL, kubeconfigPath)
-    if err != nil {
-        logger.Printf("Error building kubeconfig: %v\n", err)
-        return nil, err
+	if config == nil {
+		return nil, errors.New("Client Config should be set for controller")
 	}
 
-    policyClientset, err := clientset.NewForConfig(cfg)
+	policyClientset, err := clientset.NewForConfig(config)
 	if err != nil {
-        logger.Printf("Error building policy clientset: %v\n", err)
 		return nil, err
 	}
 

kubeclient/kubeclient.go

@@ -0,0 +1,49 @@
+package kubeclient
+
+import (
+	"log"
+	"os"
+
+	types "github.com/nirmata/kube-policy/pkg/apis/policy/v1alpha1"
+
+	v1 "k8s.io/api/core/v1"
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/rest"
+)
+
+type KubeClient struct {
+	logger *log.Logger
+	client *kubernetes.Clientset
+}
+
+func NewKubeClient(config *rest.Config, logger *log.Logger) (*KubeClient, error) {
+	if logger == nil {
+		logger = log.New(os.Stdout, "Policy Controller: ", log.LstdFlags|log.Lshortfile)
+	}
+
+	client, err := kubernetes.NewForConfig(config)
+	if err != nil {
+		return nil, err
+	}
+
+	return &KubeClient{
+		logger: logger,
+		client: client,
+	}, nil
+}
+
+func (kc *KubeClient) CopySecret(from *types.PolicyCopyFrom, namespaceTo string) error {
+	// This is the test code, which works
+	var secret v1.Secret
+	secret.Namespace = namespaceTo
+	secret.ObjectMeta.SetName("test-secret")
+	secret.StringData = make(map[string]string)
+	secret.StringData["test-data"] = "test-value"
+	newSecret, err := kc.client.CoreV1().Secrets(namespaceTo).Create(&secret)
+	if err != nil {
+		kc.logger.Printf("Unable to create secret: %s", err)
+	} else {
+		kc.logger.Printf("Secret created: %s", newSecret.Name)
+	}
+	return err
+}

main.go

@@ -4,12 +4,14 @@ import (
 	"flag"
 	"fmt"
 	"log"
-    "os"
 
 	"github.com/nirmata/kube-policy/controller"
+	"github.com/nirmata/kube-policy/kubeclient"
 	"github.com/nirmata/kube-policy/server"
 
-    "k8s.io/sample-controller/pkg/signals"
+	rest "k8s.io/client-go/rest"
+	clientcmd "k8s.io/client-go/tools/clientcmd"
+	signals "k8s.io/sample-controller/pkg/signals"
 )
 
 var (
@@ -19,6 +21,15 @@ var (
 	key        string
 )
 
+func createClientConfig(masterURL, kubeconfig string) (*rest.Config, error) {
+	// TODO: make possible to create config within a cluster with proper rights
+	config, err := clientcmd.BuildConfigFromFlags(masterURL, kubeconfig)
+	if err != nil {
+		return nil, err
+	}
+	return config, nil
+}
+
 func main() {
 	flag.Parse()
 
@@ -26,22 +37,39 @@ func main() {
 		log.Fatal("TLS certificate or/and key is not set")
 	}
 
-    crdcLogger := log.New(os.Stdout, "Policy Controller: ", log.LstdFlags|log.Lshortfile)
-    controller, err := controller.NewPolicyController(masterURL, kubeconfig, crdcLogger)
+	clientConfig, err := createClientConfig(masterURL, kubeconfig)
 	if err != nil {
-        fmt.Printf("Error creating PolicyController! Error: %s\n", err)
+		log.Fatalf("Error building kubeconfig: %v\n", err)
 		return
 	}
 
-    httpLogger := log.New(os.Stdout, "HTTPS Server: ", log.LstdFlags|log.Lshortfile)
-    server := server.NewWebhookServer(cert, key, controller, httpLogger)
+	controller, err := controller.NewPolicyController(clientConfig, nil)
+	if err != nil {
+		log.Fatalf("Error creating PolicyController! Error: %s\n", err)
+		return
+	}
+
+	kubeclient, err := kubeclient.NewKubeClient(clientConfig, nil)
+	if err != nil {
+		log.Fatalf("Error creating kubeclient: %v\n", err)
+	}
+
+	serverConfig := server.WebhookServerConfig{
+		CertFile:   cert,
+		KeyFile:    key,
+		Controller: controller,
+		Kubeclient: kubeclient,
+	}
+
+	server, err := server.NewWebhookServer(serverConfig, nil)
 	server.RunAsync()
 
 	stopCh := signals.SetupSignalHandler()
 	controller.Run(stopCh)
 
 	if err != nil {
-        fmt.Printf("Error running PolicyController! Error: %s\n", err)
+		log.Fatalf("Error running PolicyController! Error: %s\n", err)
+		return
 	}
 
 	fmt.Println("Policy Controller has started")

pkg/apis/policy/v1alpha1/types.go

@@ -7,7 +7,8 @@ import (
 // +genclient
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 
-// Policy is a specification for a Policy resource
+// An example of the YAML representation of this structure is here:
+// <project_root>/crd/policy-example.yaml
 type Policy struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
@@ -15,13 +16,14 @@ type Policy struct {
 	Status            PolicyStatus `json:"status"`
 }
 
-// PolicySpec is the spec for a Policy resource
+// Specification of the Policy.
 type PolicySpec struct {
 	FailurePolicy *string      `json:"failurePolicy"`
 	Rules         []PolicyRule `json:"rules"`
 }
 
-// PolicyRule is policy rule that will be applied to resource
+// The rule of mutation for the single resource definition.
+// Details are listed in the description of each of the substructures.
 type PolicyRule struct {
 	Resource           PolicyResource         `json:"resource"`
 	Patches            []PolicyPatch          `json:"patch,omitempty"`
@@ -29,41 +31,45 @@ type PolicyRule struct {
 	SecretGenerator    *PolicyConfigGenerator `json:"secretGenerator,omitempty"`
 }
 
-// PolicyResource describes the resource rule applied to
+// Describes the resource to which the PolicyRule will apply.
+// Either the name or selector must be specified.
+// IMPORTANT: If neither is specified, the policy rule will not apply (TBD).
 type PolicyResource struct {
 	Kind     string                `json:"kind"`
 	Name     *string               `json:"name"`
 	Selector *metav1.LabelSelector `json:"selector,omitempty"`
 }
 
-// PolicyPatch is TODO
+// PolicyPatch declares patch operation for created object according to the JSONPatch spec:
+// http://jsonpatch.com/
 type PolicyPatch struct {
 	Path      string `json:"path"`
 	Operation string `json:"op"`
 	Value     string `json:"value"`
 }
 
-// PolicyConfigGenerator is TODO
+// The declaration for a Secret or a ConfigMap, which will be created in the new namespace.
+// Can be applied only when PolicyRule.Resource.Kind is "Namespace".
 type PolicyConfigGenerator struct {
 	Name     string            `json:"name"`
 	CopyFrom *PolicyCopyFrom   `json:"copyFrom"`
 	Data     map[string]string `json:"data"`
 }
 
-// PolicyCopyFrom is TODO
+// Location of a Secret or a ConfigMap which will be used as source when applying PolicyConfigGenerator
 type PolicyCopyFrom struct {
 	Namespace string `json:"namespace"`
 	Name      string `json:"name"`
 }
 
-// PolicyStatus is the status for a Policy resource
+// Contains logs about policy application
 type PolicyStatus struct {
 	Logs []string `json:"log"`
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 
-// PolicyList is a list of Policy resources
+// List of Policy resources
 type PolicyList struct {
 	metav1.TypeMeta `json:",inline"`
 	metav1.ListMeta `json:"metadata"`

server/server.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/tls"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"log"
@@ -12,43 +13,57 @@ import (
 	"time"
 
 	controller "github.com/nirmata/kube-policy/controller"
+	kubeclient "github.com/nirmata/kube-policy/kubeclient"
 	webhooks "github.com/nirmata/kube-policy/webhooks"
 	v1beta1 "k8s.io/api/admission/v1beta1"
 )
 
-// WebhookServer is a struct that describes
-// TLS server with mutation webhook
+// WebhookServer contains configured TLS server with MutationWebhook.
+// MutationWebhook gets policies from policyController and takes control of the cluster with kubeclient.
 type WebhookServer struct {
 	server           http.Server
-    logger           *log.Logger
 	policyController *controller.PolicyController
+	kubeclient       *kubeclient.KubeClient
 	mutationWebhook  *webhooks.MutationWebhook
+	logger           *log.Logger
 }
 
-// NewWebhookServer creates new instance of WebhookServer and configures it
-func NewWebhookServer(certFile string, keyFile string, controller *controller.PolicyController, logger *log.Logger) *WebhookServer {
+// Configuration struct for WebhookServer used in NewWebhookServer
+// Controller and Kubeclient should be initialized and valid
+type WebhookServerConfig struct {
+	CertFile   string
+	KeyFile    string
+	Controller *controller.PolicyController
+	Kubeclient *kubeclient.KubeClient
+}
+
+// NewWebhookServer creates new instance of WebhookServer accordingly to given configuration
+// Policy Controller and Kubernetes Client should be initialized in configuration
+func NewWebhookServer(config WebhookServerConfig, logger *log.Logger) (*WebhookServer, error) {
 	if logger == nil {
-        logger = log.New(os.Stdout, "", log.LstdFlags|log.Lshortfile)
-    }
-    if controller == nil {
-        logger.Fatal("Controller is not specified for webhook server")
+		logger = log.New(os.Stdout, "HTTPS Server: ", log.LstdFlags|log.Lshortfile)
 	}
 
-    var config tls.Config
-    pair, err := tls.LoadX509KeyPair(certFile, keyFile)
-    if err != nil {
-        logger.Fatal("Unable to load certificate and key: ", err)
+	if config.Controller == nil || config.Kubeclient == nil {
+		return nil, errors.New("WebHook server requires initialized Policy Controller and Kubernetes Client")
 	}
-    config.Certificates = []tls.Certificate{pair}
+
+	var tlsConfig tls.Config
+	pair, err := tls.LoadX509KeyPair(config.CertFile, config.KeyFile)
+	if err != nil {
+		return nil, err
+	}
+	tlsConfig.Certificates = []tls.Certificate{pair}
 
 	mw, err := webhooks.NewMutationWebhook(logger)
 	if err != nil {
-        logger.Fatal("Unable to create mutation webhook: ", err)
+		return nil, err
 	}
 
 	ws := &WebhookServer{
 		logger:           logger,
-        policyController: controller,
+		policyController: config.Controller,
+		kubeclient:       config.Kubeclient,
 		mutationWebhook:  mw,
 	}
 
@@ -57,14 +72,14 @@ func NewWebhookServer(certFile string, keyFile string, controller *controller.Po
 
 	ws.server = http.Server{
 		Addr:         ":443", // Listen on port for HTTPS requests
-        TLSConfig:    &config,
+		TLSConfig:    &tlsConfig,
 		Handler:      mux,
 		ErrorLog:     logger,
 		ReadTimeout:  15 * time.Second,
 		WriteTimeout: 15 * time.Second,
 	}
 
-    return ws
+	return ws, nil
 }
 
 func (ws *WebhookServer) serve(w http.ResponseWriter, r *http.Request) {

webhooks/admission.go

@@ -48,7 +48,7 @@ func AdmissionIsRequired(request *v1beta1.AdmissionRequest) bool {
 // IsRuleApplicableToRequest checks requests kind, name and labels to fit the policy
 func IsRuleApplicableToRequest(policyResource types.PolicyResource, request *v1beta1.AdmissionRequest) bool {
 	if policyResource.Selector == nil && policyResource.Name == nil {
-        // TODO: selector or name MUST be specified
+		// TBD: selector or name MUST be specified
 		return false
 	}