General fixes (#8)

* Fix misspellings
* Remove commented out code
* Fix ineffective assignments
* Fix golint issues

Signed-off-by: kevdowney <[email protected]>

api/v1alpha1/addon_types.go

@@ -23,6 +23,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/json"
 )
 
+// ClusterContext represents a minimal context that can be provided to an addon
 type ClusterContext struct {
 	// ClusterName name of the cluster
 	// +optional
@@ -35,18 +36,22 @@ type ClusterContext struct {
 	AdditionalConfigs map[string]FlexString `json:"additionalConfigs,omitempty" protobuf:"bytes,2,rep,name=data"`
 }
 
+// AddonParams are the parameters which will be available to the template workflows
 type AddonParams struct {
 	// +kubebuilder:validation:MinLength=1
 	Namespace string `json:"namespace,omitempty"`
-
+	// Context values passed directly to the addon
+	// +optional
 	Context ClusterContext `json:"context,omitempty"`
 	// Data values that will be parameters injected into workflows
 	// +optional
 	Data map[string]FlexString `json:"data,omitempty"`
 }
 
+// FlexString is a ptr to string type that is used to provide additional configs
 type FlexString string
 
+// UnmarshalJSON overrides unmarshaler.UnmarshalJSON in converter
 func (fs *FlexString) UnmarshalJSON(b []byte) error {
 	var out string
 	if b[0] == '"' {
@@ -78,12 +83,14 @@ func (fs *FlexString) UnmarshalJSON(b []byte) error {
 	return fmt.Errorf("unable to unmarshal from bool or int into string")
 }
 
+// KustomizeTemplate is used to specify override patch templates in Kustomize format
 type KustomizeTemplate struct {
 	// Template patch yamls as per Kustomize spec
 	// +optional
 	Template map[string]string `json:"template,omitempty" protobuf:"bytes,2,rep,name=template"`
 }
 
+// KustomizeSpec is used to specify common Kustomize spec features
 type KustomizeSpec struct {
 	// Common labels as per Kustomize spec
 	// +optional
@@ -102,24 +109,31 @@ type KustomizeSpec struct {
 	Overlay KustomizeTemplate `json:"overlay,omitempty"`
 }
 
+// PackageType is a specific deployer type that will be used for deploying templates
 type PackageType string
 
 const (
-	HelmPkg      PackageType = "helm"
-	ShipPkg      PackageType = "ship"
+	// HelmPkg is a deployer package type representing Helm package structure
+	HelmPkg PackageType = "helm"
+	// ShipPkg is a deployer package type representing Ship package structure
+	ShipPkg PackageType = "ship"
+	// KustomizePkg is a deployer package type representing Kustomize package structure
 	KustomizePkg PackageType = "kustomize"
-	CnabPkg      PackageType = "cnab"
+	// CnabPkg is a deployer package type representing CNAB package structure
+	CnabPkg PackageType = "cnab"
+	// CompositePkg is a package type representing a composite package structure, just yamls
 	CompositePkg PackageType = "composite"
 )
 
+// CmdType represents a function that can be performed with arguments
 type CmdType int
 
 const (
 	cert CmdType = iota
 	random
 )
 
-// ApplicationAssemblyPhase tracks the Addon CRD phases: pending, succeded, failed, deleting, deleteFailed
+// ApplicationAssemblyPhase tracks the Addon CRD phases: pending, succeeded, failed, deleting, deleteFailed
 type ApplicationAssemblyPhase string
 
 // Constants
@@ -141,25 +155,33 @@ const (
 	DeleteFailed ApplicationAssemblyPhase = "Delete Failed"
 )
 
+// DeploymentPhase represents the status of observed resources
 type DeploymentPhase string
 
 const (
+	// InProgress deployment phase for resources in addon
 	InProgress DeploymentPhase = "InProgress"
-
+	// Ready deployment phase for resources in addon
 	Ready DeploymentPhase = "Ready"
-
+	// Unknown deployment phase for resources in addon
 	Unknown DeploymentPhase = "Unknown"
 )
 
+// LifecycleStep is a string representation of the lifecycle steps available in Addon spec: prereqs, install, delete, validate
 type LifecycleStep string
 
 const (
-	Prereqs  LifecycleStep = "prereqs"
-	Install  LifecycleStep = "install"
-	Delete   LifecycleStep = "delete"
+	// Prereqs constant
+	Prereqs LifecycleStep = "prereqs"
+	// Install constant
+	Install LifecycleStep = "install"
+	// Delete constant
+	Delete LifecycleStep = "delete"
+	// Validate constant
 	Validate LifecycleStep = "validate"
 )
 
+// AddonOverridesSpec represents a template of the resources that can be deployed or patched alongside the main deployment
 type AddonOverridesSpec struct {
 	// Kustomize specs
 	// +optional
@@ -169,12 +191,14 @@ type AddonOverridesSpec struct {
 	Template map[string]string `json:"template,omitempty" protobuf:"bytes,2,rep,name=template"`
 }
 
+// SecretCmdSpec is a secret list and/or generator for secrets using the available commands: random, cert.
 type SecretCmdSpec struct {
 	Name string   `json:"name"`
 	Cmd  CmdType  `json:"cmd,omitempty"`
 	Args []string `json:"args,omitempty" protobuf:"bytes,4,rep,name=args"`
 }
 
+// WorkflowType allows user to specify workflow templates with optional namePrefix, workflowRole or role.
 type WorkflowType struct {
 	// NamePrefix is a prefix for the name of workflow
 	// +kubebuilder:validation:MaxLength=10
@@ -186,17 +210,19 @@ type WorkflowType struct {
 	// WorkflowRole used to denote the role annotation that should be used by the workflow
 	// +optional
 	WorkflowRole string `json:"workflowRole,omitempty"`
-
+	// Template is used to provide the workflow spec
 	Template string `json:"template"`
 }
 
+// LifecycleWorkflowSpec is where all of the lifecycle workflow templates will be specified under
 type LifecycleWorkflowSpec struct {
 	Prereqs  WorkflowType `json:"prereqs,omitempty"`
 	Install  WorkflowType `json:"install,omitempty"`
 	Delete   WorkflowType `json:"delete,omitempty"`
 	Validate WorkflowType `json:"validate,omitempty"`
 }
 
+// PackageSpec is the package level details needed by addon
 type PackageSpec struct {
 	PkgChannel     string            `json:"pkgChannel,omitempty"`
 	PkgName        string            `json:"pkgName"`
@@ -288,6 +314,7 @@ func init() {
 	SchemeBuilder.Register(&Addon{}, &AddonList{})
 }
 
+// GetPackageSpec returns the addon package details from addon spec
 func (a *Addon) GetPackageSpec() PackageSpec {
 	return PackageSpec{
 		PkgName:        a.Spec.PkgName,
@@ -299,7 +326,7 @@ func (a *Addon) GetPackageSpec() PackageSpec {
 	}
 }
 
-// GetAddonParametersMap returns an object copying the params submitted as part of the addon spec
+// GetAllAddonParameters returns an object copying the params submitted as part of the addon spec
 func (a *Addon) GetAllAddonParameters() map[string]string {
 	params := make(map[string]string)
 	params["namespace"] = a.Spec.Params.Namespace

api/v1alpha1/addon_types_test.go

@@ -79,7 +79,7 @@ var _ = Describe("Addon", func() {
 		// Add any teardown steps that needs to be executed after each test
 	})
 
-	// Add Tests for OpenAPI validation (or additonal CRD features) specified in
+	// Add Tests for OpenAPI validation (or additional CRD features) specified in
 	// your API definition.
 	// Avoid adding tests for vanilla CRUD operations because they would
 	// test Kubernetes API server, which isn't the goal here.

config/crd/bases/addonmgr.orkaproj.io_addons.yaml

@@ -416,6 +416,7 @@ spec:
                         be used by the deployment resource
                       type: string
                     template:
+                      description: Template is used to provide the workflow spec
                       type: string
                     workflowRole:
                       description: WorkflowRole used to denote the role annotation
@@ -435,6 +436,7 @@ spec:
                         be used by the deployment resource
                       type: string
                     template:
+                      description: Template is used to provide the workflow spec
                       type: string
                     workflowRole:
                       description: WorkflowRole used to denote the role annotation
@@ -454,6 +456,7 @@ spec:
                         be used by the deployment resource
                       type: string
                     template:
+                      description: Template is used to provide the workflow spec
                       type: string
                     workflowRole:
                       description: WorkflowRole used to denote the role annotation
@@ -473,6 +476,7 @@ spec:
                         be used by the deployment resource
                       type: string
                     template:
+                      description: Template is used to provide the workflow spec
                       type: string
                     workflowRole:
                       description: WorkflowRole used to denote the role annotation
@@ -526,6 +530,7 @@ spec:
                 addon
               properties:
                 context:
+                  description: Context values passed directly to the addon
                   properties:
                     additionalConfigs:
                       additionalProperties:

controllers/addon_controller.go

@@ -72,6 +72,7 @@ type AddonReconciler struct {
 	recorder        record.EventRecorder
 }
 
+// NewAddonReconciler returns an instance of AddonReconciler
 func NewAddonReconciler(mgr manager.Manager, log logr.Logger) *AddonReconciler {
 	return &AddonReconciler{
 		Client:          mgr.GetClient(),
@@ -94,6 +95,7 @@ func NewAddonReconciler(mgr manager.Manager, log logr.Logger) *AddonReconciler {
 // +kubebuilder:rbac:groups=extensions,resources=deployments;daemonsets;replicasets;ingresses,verbs=get;list;watch;create;update;patch
 // +kubebuilder:rbac:groups=batch,resources=jobs;cronjobs,verbs=get;list;watch;create;update;patch
 
+// Reconcile method for all addon requests
 func (r *AddonReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	ctx := context.Background()
 	log := r.Log.WithValues("addon", req.NamespacedName)
@@ -122,6 +124,7 @@ func (r *AddonReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	return ret, err
 }
 
+// SetupWithManager is called to setup manager and watchers
 func (r *AddonReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	log := r.Log
 	//var addongrp = common.AddonGVR().Group
@@ -441,6 +444,7 @@ func (r *AddonReconciler) observeResources(ctx context.Context, a *addonmgrv1alp
 	return observed, nil
 }
 
+// Finalize runs finalizer for addon
 func (r *AddonReconciler) Finalize(ctx context.Context, addon *addonmgrv1alpha1.Addon, wfl workflows.AddonLifecycle, finalizerName string) error {
 	// Has Delete workflow defined, let's run it.
 	var removeFinalizer = true

pkg/addon/addon_validator.go

@@ -172,6 +172,7 @@ func (av *addonValidator) validateDependencies() error {
 	for pkgName, pkgVersion := range av.addon.Spec.PkgDeps {
 		pkgName = strings.TrimSpace(pkgName)
 		pkgVersion = strings.TrimSpace(pkgVersion)
+
 		if pkgVersion == "*" {
 			// Ignore version
 			versions := av.cache.GetVersions(pkgName)
@@ -188,11 +189,9 @@ func (av *addonValidator) validateDependencies() error {
 				}
 			}
 
-			if versionFound == true {
-				continue
+			if !versionFound {
+				return fmt.Errorf("required dependency %s has no valid versions installed", pkgName)
 			}
-
-			return fmt.Errorf("required dependency %s has no valid versions installed", pkgName)
 		} else {
 			// Check for specific version
 			v := av.cache.GetVersion(pkgName, pkgVersion)

pkg/addon/addon_version_cache.go

@@ -20,6 +20,7 @@ import (
 	"sync"
 )
 
+// VersionCacheClient interface clients must implement for addon version cache.
 type VersionCacheClient interface {
 	AddVersion(Version)
 	GetVersions(pkgName string) map[string]Version
@@ -30,6 +31,7 @@ type VersionCacheClient interface {
 	GetAllVersions() map[string]map[string]Version
 }
 
+// Version data that will be cached
 type Version struct {
 	Name      string
 	Namespace string
@@ -42,6 +44,7 @@ type cached struct {
 	addons map[string]map[string]Version
 }
 
+// NewAddonVersionCacheClient returns a new instance of VersionCacheClient
 func NewAddonVersionCacheClient() VersionCacheClient {
 	return &cached{
 		addons: make(map[string]map[string]Version),
@@ -52,9 +55,9 @@ func (c *cached) AddVersion(v Version) {
 	c.Lock()
 	defer c.Unlock()
 
-	mm, ok := c.addons[v.PkgName]
+	_, ok := c.addons[v.PkgName]
 	if !ok {
-		mm = make(map[string]Version)
+		mm := make(map[string]Version)
 		c.addons[v.PkgName] = mm
 	}
 	c.addons[v.PkgName][v.PkgVersion] = v

pkg/addonctl/addonctl.go

@@ -125,14 +125,14 @@ func newRootCommand() *cobra.Command {
 	rootCmd.PersistentFlags().StringVar(&clusterRegion, "cluster-region", "", "Cluster region")
 	rootCmd.MarkFlagRequired("cluster-region")
 	rootCmd.PersistentFlags().StringVar(&description, "desc", "", "Description of the addon")
-	rootCmd.PersistentFlags().StringVar(&dependencies, "deps", "", "Comma seperated dependencies list in the format 'pkgName:pkgVersion'")
+	rootCmd.PersistentFlags().StringVar(&dependencies, "deps", "", "Comma separated dependencies list in the format 'pkgName:pkgVersion'")
 	rootCmd.PersistentFlags().StringVarP(&pkgChannel, "channel", "c", "", "Channel for the addon package")
 	rootCmd.MarkFlagRequired("channel")
 	rootCmd.PersistentFlags().StringVarP(&pkgType, "type", "t", "", "Addon package type")
 	rootCmd.MarkFlagRequired("type")
 	rootCmd.PersistentFlags().StringVarP(&pkgVersion, "version", "v", "", "Addon package version")
 	rootCmd.MarkFlagRequired("version")
-	rootCmd.PersistentFlags().StringVar(&secretsRaw, "secrets", "", "Comma seperated list of secret names which are validated as part ofthe addon-manager-system namespace")
+	rootCmd.PersistentFlags().StringVar(&secretsRaw, "secrets", "", "Comma separated list of secret names which are validated as part ofthe addon-manager-system namespace")
 	rootCmd.PersistentFlags().StringVar(&selector, "selector", "", "Selector applied to all resources?")
 	rootCmd.PersistentFlags().StringVarP(&namespace, "namespace", "n", "", "Namespace where the addon will be deployed")
 	rootCmd.MarkFlagRequired("namespace")
@@ -219,7 +219,7 @@ func newRootCommand() *cobra.Command {
 				return
 			}
 
-			kClient := dynamic.NewForConfigOrDie(cfg)
+			kubeClient := dynamic.NewForConfigOrDie(cfg)
 
 			addonMap := make(map[string]interface{})
 			jsonInstance, _ := json.Marshal(instance)
@@ -232,21 +232,21 @@ func newRootCommand() *cobra.Command {
 
 			addon := unstructured.Unstructured{}
 			addon.SetUnstructuredContent(addonMap)
-			addonObject, err := kClient.Resource(common.AddonGVR()).Namespace(addonMgrSystemNamespace).Get(addonName, metav1.GetOptions{})
+			addonObject, err := kubeClient.Resource(common.AddonGVR()).Namespace(addonMgrSystemNamespace).Get(addonName, metav1.GetOptions{})
 
 			if err == nil {
 				fmt.Printf("Updating addon %s...\n", addonName)
 				resourceVersion := addonObject.GetResourceVersion()
 				addon.SetResourceVersion(resourceVersion)
-				_, err = kClient.Resource(common.AddonGVR()).Namespace(addonMgrSystemNamespace).Update(&addon, metav1.UpdateOptions{})
+				_, err = kubeClient.Resource(common.AddonGVR()).Namespace(addonMgrSystemNamespace).Update(&addon, metav1.UpdateOptions{})
 				if err != nil {
 					fmt.Println(err)
 					return
 				}
 
 			} else {
 				fmt.Printf("Creating addon %s...\n", addonName)
-				_, err = kClient.Resource(common.AddonGVR()).Namespace(addonMgrSystemNamespace).Create(&addon, metav1.CreateOptions{})
+				_, err = kubeClient.Resource(common.AddonGVR()).Namespace(addonMgrSystemNamespace).Create(&addon, metav1.CreateOptions{})
 				if err != nil {
 					fmt.Println(err)
 					return
@@ -273,9 +273,9 @@ func parseSelector(sel string) error {
 	}
 	s := strings.Split(sel, ":")
 	if len(s) == 1 && s[0] == ":" {
-		return errors.New("Missing ':' seperator in selector")
+		return errors.New("Missing ':' separator in selector")
 	} else if len(s) != 2 {
-		return errors.New("Dependency had multiple seperators")
+		return errors.New("Dependency had multiple separators")
 	}
 	selectorMap[s[0]] = s[1]
 	return nil
@@ -292,7 +292,7 @@ func parseAddonParams(raw string) error {
 	for _, item := range rawList {
 		kv := strings.Split(item, "=")
 		if len(kv) == 1 && kv[0] == "=" {
-			return fmt.Errorf("Unable to parse addon params: '%s'. Key-value pair %s does not have seperator '='", raw, item)
+			return fmt.Errorf("Unable to parse addon params: '%s'. Key-value pair %s does not have separator '='", raw, item)
 		}
 		params[kv[0]] = kv[1]
 	}
@@ -316,9 +316,9 @@ func parseDependencies(deps string) error {
 	for _, dep := range strings.Split(deps, ",") {
 		d := strings.Split(dep, ":")
 		if len(d) == 1 && d[0] == ":" {
-			log.Fatal("Missing ':' seperator in dependency")
+			log.Fatal("Missing ':' separator in dependency")
 		} else if len(d) != 2 {
-			log.Fatal("Dependency had multiple seperators")
+			log.Fatal("Dependency had multiple separators")
 		}
 		dependenciesMap[d[0]] = d[1]
 	}