Add helm_args to the course file. Refactor some code smells (#557)

* Add helm_args to the course file

* review suggestion

* remove code smell by refactoring client initialization

* Fix code smell for plot_test

* another code smell

* fix more code smells

* Add sonar configuration

* fix sonar file maybe

* remove sonar properties

* Refactor slightly to reduce complexity

* Actually add coursefile to client struct

* more logging. fix version reference. comments on client struct

* remove aparently unused field in client

Makefile

@@ -29,4 +29,4 @@ clean:
 build-linux:
 	CGO_ENABLED=0 GOOS=linux GOARCH=amd64 $(GOBUILD) -o $(BINARY_NAME) -ldflags "-X main.version=$(VERSION) -X main.commit=$(COMMIT) -s -w" -v
 build-docker: build-linux
-	docker build --build-arg version=$(VERSION) --build-arg commit=$(COMMIT) -t quay.io/fairwinds/reckoner:go-dev -f Dockerfile-go .
+	docker build -t quay.io/fairwinds/reckoner:go-dev -f Dockerfile .

cmd/root.go

@@ -56,6 +56,8 @@ var (
 	importRelease string
 	// importRepository is the helm repository for the imported release
 	importRepository string
+	// additionalHelmArgs is a list of arguments to add to all helm commands
+	additionalHelmArgs []string
 )
 
 func init() {
@@ -64,6 +66,7 @@ func init() {
 	rootCmd.PersistentFlags().BoolVar(&dryRun, "dry-run", false, "Implies helm --dry-run --debug and skips any hooks")
 	rootCmd.PersistentFlags().BoolVar(&createNamespaces, "create-namespaces", true, "If true, allow reckoner to create namespaces.")
 	rootCmd.PersistentFlags().BoolVar(&noColor, "no-color", false, "If true, don't colorize output.")
+	rootCmd.PersistentFlags().StringSliceVar(&additionalHelmArgs, "helm-args", nil, "Additional arguments to pass to helm commands. Can be passed multiple times. used more than once. WARNING: Setting this will completely override any helm_args in the course.")
 
 	plotCmd.PersistentFlags().BoolVar(&continueOnError, "continue-on-error", false, "If true, continue plotting releases even if one or more has errors.")
 	updateCmd.PersistentFlags().BoolVar(&continueOnError, "continue-on-error", false, "If true, continue plotting releases even if one or more has errors.")
@@ -75,15 +78,16 @@ func init() {
 	importCmd.Flags().StringVar(&importRelease, "release_name", "", "The name of the release to import.")
 	importCmd.Flags().StringVar(&importRepository, "repository", "", "The helm repository for the imported release.")
 
-	// add commands here
-	rootCmd.AddCommand(plotCmd)
-	rootCmd.AddCommand(convertCmd)
-	rootCmd.AddCommand(templateCmd)
-	rootCmd.AddCommand(diffCmd)
-	rootCmd.AddCommand(lintCmd)
-	rootCmd.AddCommand(getManifestsCmd)
-	rootCmd.AddCommand(updateCmd)
-	rootCmd.AddCommand(importCmd)
+	rootCmd.AddCommand(
+		plotCmd,
+		convertCmd,
+		templateCmd,
+		diffCmd,
+		lintCmd,
+		getManifestsCmd,
+		updateCmd,
+		importCmd,
+	)
 
 	klog.InitFlags(nil)
 	pflag.CommandLine.AddGoFlag(flag.CommandLine.Lookup("v"))
@@ -109,7 +113,16 @@ var plotCmd = &cobra.Command{
 	Long:    "Runs a helm install on a release or several releases.",
 	PreRunE: validateCobraArgs,
 	Run: func(cmd *cobra.Command, args []string) {
-		client, err := reckoner.NewClient(courseFile, version, runAll, onlyRun, true, dryRun, createNamespaces, courseSchema, continueOnError)
+		client := reckoner.Client{
+			ReckonerVersion:  version,
+			Schema:           courseSchema,
+			HelmArgs:         additionalHelmArgs,
+			DryRun:           dryRun,
+			CreateNamespaces: createNamespaces,
+			ContinueOnError:  continueOnError,
+			Releases:         onlyRun,
+		}
+		err := client.Init(courseFile, true)
 		if err != nil {
 			color.Red(err.Error())
 			os.Exit(1)
@@ -131,7 +144,17 @@ var templateCmd = &cobra.Command{
 	Long:    "Templates a helm chart for a release or several releases. Automatically sets --create-namespaces=false --dry-run=true",
 	PreRunE: validateCobraArgs,
 	Run: func(cmd *cobra.Command, args []string) {
-		client, err := reckoner.NewClient(courseFile, version, runAll, onlyRun, false, true, false, courseSchema, false)
+		client := reckoner.Client{
+			ReckonerVersion:  version,
+			Schema:           courseSchema,
+			HelmArgs:         additionalHelmArgs,
+			DryRun:           true,
+			CreateNamespaces: false,
+			ContinueOnError:  continueOnError,
+			Releases:         onlyRun,
+		}
+
+		err := client.Init(courseFile, false)
 		if err != nil {
 			color.Red(err.Error())
 			os.Exit(1)
@@ -151,11 +174,21 @@ var getManifestsCmd = &cobra.Command{
 	Long:    "Gets the manifests currently in the cluster.",
 	PreRunE: validateCobraArgs,
 	Run: func(cmd *cobra.Command, args []string) {
-		client, err := reckoner.NewClient(courseFile, version, runAll, onlyRun, true, true, false, courseSchema, false)
+		client := reckoner.Client{
+			ReckonerVersion:  version,
+			Schema:           courseSchema,
+			HelmArgs:         additionalHelmArgs,
+			DryRun:           true,
+			CreateNamespaces: false,
+			ContinueOnError:  false,
+			Releases:         onlyRun,
+		}
+		err := client.Init(courseFile, true)
 		if err != nil {
 			color.Red(err.Error())
 			os.Exit(1)
 		}
+
 		manifests, err := client.GetManifests()
 		if err != nil {
 			color.Red(err.Error())
@@ -171,11 +204,21 @@ var diffCmd = &cobra.Command{
 	Long:    "Diffs the currently defined release and the one in the cluster",
 	PreRunE: validateCobraArgs,
 	Run: func(cmd *cobra.Command, args []string) {
-		client, err := reckoner.NewClient(courseFile, version, runAll, onlyRun, true, true, false, courseSchema, continueOnError)
+		client := reckoner.Client{
+			ReckonerVersion:  version,
+			Schema:           courseSchema,
+			HelmArgs:         additionalHelmArgs,
+			DryRun:           true,
+			CreateNamespaces: false,
+			ContinueOnError:  continueOnError,
+			Releases:         onlyRun,
+		}
+		err := client.Init(courseFile, true)
 		if err != nil {
 			color.Red(err.Error())
 			os.Exit(1)
 		}
+
 		if err := client.UpdateHelmRepos(); err != nil {
 			color.Red(err.Error())
 			os.Exit(1)
@@ -200,11 +243,21 @@ var lintCmd = &cobra.Command{
 		return validateCobraArgs(cmd, args)
 	},
 	Run: func(cmd *cobra.Command, args []string) {
-		_, err := reckoner.NewClient(courseFile, version, runAll, onlyRun, false, true, false, courseSchema, false)
+		client := reckoner.Client{
+			ReckonerVersion:  version,
+			Schema:           courseSchema,
+			HelmArgs:         additionalHelmArgs,
+			DryRun:           true,
+			CreateNamespaces: false,
+			ContinueOnError:  false,
+			Releases:         onlyRun,
+		}
+		err := client.Init(courseFile, false)
 		if err != nil {
 			color.Red(err.Error())
 			os.Exit(1)
 		}
+
 		color.Green("No schema validation errors found in course file: %s", courseFile)
 	},
 }
@@ -257,7 +310,16 @@ var updateCmd = &cobra.Command{
 	Long:    "Only install/upgrade a release if there are changes.",
 	PreRunE: validateCobraArgs,
 	Run: func(cmd *cobra.Command, args []string) {
-		client, err := reckoner.NewClient(courseFile, version, runAll, onlyRun, true, dryRun, createNamespaces, courseSchema, continueOnError)
+		client := reckoner.Client{
+			ReckonerVersion:  version,
+			Schema:           courseSchema,
+			HelmArgs:         additionalHelmArgs,
+			DryRun:           dryRun,
+			CreateNamespaces: createNamespaces,
+			ContinueOnError:  continueOnError,
+			Releases:         onlyRun,
+		}
+		err := client.Init(courseFile, true)
 		if err != nil {
 			color.Red(err.Error())
 			os.Exit(1)

go.mod

@@ -4,6 +4,7 @@ go 1.17
 
 require (
 	github.com/Masterminds/semver/v3 v3.1.1
+	github.com/davecgh/go-spew v1.1.1
 	github.com/fatih/color v1.13.0
 	github.com/go-git/go-git/v5 v5.4.2
 	github.com/sergi/go-diff v1.1.0
@@ -31,7 +32,6 @@ require (
 	github.com/Microsoft/go-winio v0.4.16 // indirect
 	github.com/ProtonMail/go-crypto v0.0.0-20210428141323-04723f9f07d7 // indirect
 	github.com/acomagu/bufpipe v1.0.3 // indirect
-	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/emirpasic/gods v1.12.0 // indirect
 	github.com/evanphx/json-patch v4.12.0+incompatible // indirect
 	github.com/form3tech-oss/jwt-go v3.2.3+incompatible // indirect

pkg/course/course.go

@@ -68,6 +68,8 @@ type FileV2 struct {
 	Secrets SecretsList `yaml:"secrets,omitempty" json:"secrets,omitempty"`
 	// Releases is the list of releases that should be maintained by this course file.
 	Releases []*Release `yaml:"releases,omitempty" json:"releases,omitempty"`
+	// HelmArgs is a list of arguments to pass to helm commands
+	HelmArgs []string `yaml:"helm_args,omitempty" json:"helm_args,omitempty"`
 }
 
 // FileV2Unmarshal is a helper type that allows us to have a custom unmarshal function for the FileV2 struct
@@ -191,6 +193,8 @@ type FileV1 struct {
 	// Charts is the list of releases. In the actual file this will be a map, but we must convert to a list to preserve order.
 	// This conversion is done in the ChartsListV1 UnmarshalYAML function.
 	Charts ChartsListV1 `yaml:"charts" json:"charts"`
+	// HelmArgs is a list of arguments to pass to helm
+	HelmArgs []string `yaml:"helm_args,omitempty" json:"helm_args,omitempty"`
 }
 
 // ChartsListV1 is a list of releases which we convert from a map of releases to preserve order
@@ -245,6 +249,7 @@ func convertV1toV2(fileName string) (*FileV2, error) {
 	newFile.Releases = make([]*Release, len(oldFile.Charts))
 	newFile.Hooks = oldFile.Hooks
 	newFile.MinimumVersions = oldFile.MinimumVersions
+	newFile.HelmArgs = oldFile.HelmArgs
 
 	for releaseIndex, release := range oldFile.Charts {
 		repositoryName, ok := release.Repository.(string)

pkg/course/course_test.go

@@ -48,6 +48,7 @@ func TestConvertV1toV2(t *testing.T) {
 				DefaultNamespace:  "namespace",
 				DefaultRepository: "stable",
 				Context:           "farglebargle",
+				HelmArgs:          []string{"--atomic"},
 				Repositories: RepositoryMap{
 					"git-repo-test": {
 						Git:  "https://github.com/FairwindsOps/charts",

pkg/course/testdata/convert1.yaml

@@ -1,6 +1,8 @@
 namespace: namespace
 context: farglebargle
 repository: stable
+helm_args:
+- --atomic
 repositories:
   git-repo-test:
     git: https://github.com/FairwindsOps/charts

pkg/reckoner/client.go

@@ -29,73 +29,86 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
 
 	"github.com/Masterminds/semver/v3"
+	"github.com/davecgh/go-spew/spew"
 	"github.com/fairwindsops/reckoner/pkg/course"
 	"github.com/fairwindsops/reckoner/pkg/helm"
 	"github.com/thoas/go-funk"
 )
 
 // Client is a configuration struct
 type Client struct {
-	KubeClient       kubernetes.Interface
-	Helm             helm.Client
-	ReckonerVersion  string
-	CourseFile       course.FileV2
-	PlotAll          bool
-	Releases         []string
-	BaseDirectory    string
-	DryRun           bool
+	// KubClient is kubernetes client interface. Will be populated in the cilent.Init() function
+	KubeClient kubernetes.Interface
+	// Helm is a reckoner helm client. Will be populated in the client.Init() function
+	Helm helm.Client
+	// The version of Reckoner that is being used
+	ReckonerVersion string
+	// CourseFile will be populated in the client.Init() function
+	CourseFile course.FileV2
+	// PlotAll should be set to true if operating on all releases in the course
+	PlotAll bool
+	// Releases is a list of releases to operate on if PlotAll is false
+	Releases []string
+	// BaseDirectory is the directory where the course file is located
+	BaseDirectory string
+	// DryRun is a flag to indicate if the client should be run in dry run mode
+	DryRun bool
+	// CreateNamespaces is a flag to indicate if the client should create namespaces
 	CreateNamespaces bool
-	ContinueOnError  bool
-	Errors           int
+	// ContinueOnError is a flag to indicate if the client should continue if one release fails
+	ContinueOnError bool
+	// Errors is a counter of errors encountered during the use of this cilent
+	Errors int
+	// HelmArgs is a list of helm args to pass to helm when running commands
+	HelmArgs []string
+	// Schema is a byte slice representation of the coursev2 json schema
+	Schema []byte
 }
 
 var once sync.Once
 var clientset *kubernetes.Clientset
 
-// NewClient returns a client. Attempts to open a v2 schema course file
+// Init initializes a client. Attempts to open a v2 schema course file
 // If getClient is true, attempts to get a Kubernetes client from config
-func NewClient(fileName, version string, plotAll bool, releases []string, kubeClient bool, dryRun bool, createNamespaces bool, schema []byte, continueOnError bool) (*Client, error) {
+func (c *Client) Init(fileName string, initKubeClient bool) error {
 	// Get the course file
-	courseFile, err := course.OpenCourseFile(fileName, schema)
+	courseFile, err := course.OpenCourseFile(fileName, c.Schema)
 	if err != nil {
-		return nil, fmt.Errorf("%w - error opening course file %s: %s", course.SchemaValidationError, fileName, err)
+		return fmt.Errorf("%w - error opening course file %s: %s", course.SchemaValidationError, fileName, err)
 	}
+	c.CourseFile = *courseFile
 
 	// Get a helm client
 	helmClient, err := helm.NewClient()
 	if err != nil {
-		return nil, err
+		return err
 	}
+	c.Helm = *helmClient
 
-	client := &Client{
-		CourseFile:       *courseFile,
-		PlotAll:          plotAll,
-		Releases:         releases,
-		Helm:             *helmClient,
-		ReckonerVersion:  version,
-		BaseDirectory:    path.Dir(fileName),
-		DryRun:           dryRun,
-		CreateNamespaces: createNamespaces,
-		ContinueOnError:  continueOnError,
-	}
+	c.BaseDirectory = path.Dir(fileName)
 
 	// Check versions
-	if !client.helmVersionValid() {
-		return nil, fmt.Errorf("helm version check failed")
+	if !c.helmVersionValid() {
+		return fmt.Errorf("helm version check failed")
+	}
+	if !c.reckonerVersionValid() {
+		return fmt.Errorf("reckoner version check failed")
 	}
-	if !client.reckonerVersionValid() {
-		return nil, fmt.Errorf("reckoner version check failed")
+
+	if err := c.filterReleases(); err != nil {
+		return err
 	}
 
-	if err := client.filterReleases(); err != nil {
-		return nil, err
+	if initKubeClient {
+		c.KubeClient = getKubeClient(courseFile.Context)
 	}
 
-	if kubeClient {
-		client.KubeClient = getKubeClient(courseFile.Context)
+	klog.V(5).Infof("successfully initialized client:")
+	if klog.V(5).Enabled() {
+		spew.Dump(c)
 	}
 
-	return client, nil
+	return nil
 }
 
 func (c *Client) Continue() bool {
@@ -147,7 +160,8 @@ func (c Client) helmVersionValid() bool {
 }
 
 // reckonerVersionValid determines if the current helm version high enough
-func (c Client) reckonerVersionValid() bool {
+func (c *Client) reckonerVersionValid() bool {
+	klog.V(5).Infof("checking current reckoner version: %s", c.ReckonerVersion)
 	if c.CourseFile.MinimumVersions.Reckoner == "" {
 		klog.V(2).Infof("no minimum reckoner version found, assuming okay")
 		return true