Merge pull request #568 from Praqma/small_fixes

Small fixes

Dockerfile

@@ -15,10 +15,10 @@ ENV HELM_DIFF_VERSION=$GLOBAL_HELM_DIFF_VERSION
 
 RUN apk add --update --no-cache ca-certificates git openssh ruby curl tar gzip make bash
 
-RUN curl -L https://storage.googleapis.com/kubernetes-release/release/${KUBE_VERSION}/bin/linux/amd64/kubectl -o /usr/local/bin/kubectl
+RUN curl --retry 5 -L https://storage.googleapis.com/kubernetes-release/release/${KUBE_VERSION}/bin/linux/amd64/kubectl -o /usr/local/bin/kubectl
 RUN chmod +x /usr/local/bin/kubectl
 
-RUN curl -Lk https://get.helm.sh/helm-${HELM_VERSION}-linux-amd64.tar.gz | tar zxv -C /tmp
+RUN curl --retry 5 -Lk https://get.helm.sh/helm-${HELM_VERSION}-linux-amd64.tar.gz | tar zxv -C /tmp
 RUN mv /tmp/linux-amd64/helm /usr/local/bin/helm && rm -rf /tmp/linux-amd64
 RUN chmod +x /usr/local/bin/helm
 

internal/app/cli.go

@@ -79,7 +79,7 @@ func printUsage() {
 
 // Cli parses cmd flags, validates them and performs some initializations
 func (c *cli) parse() {
-	//parsing command line flags
+	// parsing command line flags
 	flag.Var(&c.files, "f", "desired state file name(s), may be supplied more than once to merge state files")
 	flag.Var(&c.envFiles, "e", "file(s) to load environment variables from (default .env), may be supplied more than once")
 	flag.Var(&c.target, "target", "limit execution to specific app.")
@@ -211,7 +211,7 @@ func (c *cli) readState(s *state) error {
 
 	// wipe & create a temporary directory
 	os.RemoveAll(tempFilesDir)
-	_ = os.MkdirAll(tempFilesDir, 0755)
+	_ = os.MkdirAll(tempFilesDir, 0o755)
 
 	// read the TOML/YAML desired state file
 	var fileState state
@@ -265,10 +265,13 @@ func (c *cli) readState(s *state) error {
 	return nil
 }
 
-// getDryRunFlags returns dry-run flag
-func (c *cli) getDryRunFlags() []string {
+// getRunFlags returns dry-run and debug flags
+func (c *cli) getRunFlags() []string {
 	if c.dryRun {
 		return []string{"--dry-run", "--debug"}
 	}
+	if c.debug {
+		return []string{"--debug"}
+	}
 	return []string{}
 }

internal/app/decision_maker.go

@@ -174,8 +174,10 @@ func (cs *currentState) releaseExists(r *release, status string) bool {
 	return true
 }
 
-var resourceNameExtractor = regexp.MustCompile(`(^\w+/|\.v\d+$)`)
-var releaseNameExtractor = regexp.MustCompile(`sh\.helm\.release\.v\d+\.`)
+var (
+	resourceNameExtractor = regexp.MustCompile(`(^\w+/|\.v\d+$)`)
+	releaseNameExtractor  = regexp.MustCompile(`sh\.helm\.release\.v\d+\.`)
+)
 
 // getHelmsmanReleases returns a map of all releases that are labeled with "MANAGED-BY=HELMSMAN"
 // The releases are categorized by the namespaces in which they are deployed
@@ -252,7 +254,6 @@ func (cs *currentState) getHelmsmanReleases(s *state) map[string]map[string]bool
 				}
 				mutex.Unlock()
 			}
-
 		}(ns)
 	}
 	wg.Wait()

internal/app/helm_release.go

@@ -79,7 +79,7 @@ func (r *helmRelease) key() string {
 
 // uninstall creates the helm command to uninstall an untracked release
 func (r *helmRelease) uninstall(p *plan) {
-	cmd := helmCmd(concat([]string{"uninstall", r.Name, "--namespace", r.Namespace}, flags.getDryRunFlags()), "Delete untracked release [ "+r.Name+" ] in namespace [ "+r.Namespace+" ]")
+	cmd := helmCmd(concat([]string{"uninstall", r.Name, "--namespace", r.Namespace}, flags.getRunFlags()), "Delete untracked release [ "+r.Name+" ] in namespace [ "+r.Namespace+" ]")
 
 	p.addCommand(cmd, -800, nil, []hookCmd{}, []hookCmd{})
 }
@@ -92,7 +92,6 @@ func (r *helmRelease) getRevision() string {
 // getChartName extracts and returns the Helm chart name from the chart info in a release state.
 // example: chart in release state is "jenkins-0.9.0" and this function will extract "jenkins" from it.
 func (r *helmRelease) getChartName() string {
-
 	chart := r.Chart
 	runes := []rune(chart)
 	return string(runes[0:strings.LastIndexByte(chart[0:strings.IndexByte(chart, '.')], '-')])

internal/app/helm_time.go

@@ -6,8 +6,10 @@ import (
 	"time"
 )
 
-const ctLayout = "2006-01-02 15:04:05.000000000 -0700 MST"
-const ctLayout2 = "2006-01-02 15:04:05.000000000 -0700 -0700"
+const (
+	ctLayout  = "2006-01-02 15:04:05.000000000 -0700 MST"
+	ctLayout2 = "2006-01-02 15:04:05.000000000 -0700 -0700"
+)
 
 var nilTime = (time.Time{}).UnixNano()
 

internal/app/hooks.go

@@ -40,7 +40,7 @@ func validateHooks(hooks map[string]interface{}) error {
 				return nil
 			}
 			if err := isValidFile(hook, validFiles); err != nil {
-				return err
+				return fmt.Errorf("invalid hook manifest: %w", err)
 			}
 		case "successCondition", "successTimeout", "deleteOnSuccess":
 			continue

internal/app/kube_helpers.go

@@ -100,7 +100,6 @@ func annotateNamespace(ns string, annotations map[string]string) {
 
 // setLimits creates a LimitRange resource in the provided Namespace
 func setLimits(ns string, lims limits) {
-
 	if len(lims) == 0 {
 		return
 	}
@@ -121,7 +120,7 @@ spec:
 
 	definition = definition + Indent(string(d), strings.Repeat(" ", 4))
 	targetFile := path.Join(createTempDir(tempFilesDir, "tmp"), "temp-LimitRange.yaml")
-	if err := ioutil.WriteFile(targetFile, []byte(definition), 0666); err != nil {
+	if err := ioutil.WriteFile(targetFile, []byte(definition), 0o666); err != nil {
 		log.Fatal(err.Error())
 	}
 
@@ -133,7 +132,6 @@ spec:
 	}
 
 	deleteFile(targetFile)
-
 }
 
 func setQuotas(ns string, quotas *quotas) {
@@ -155,7 +153,7 @@ spec:
 		definition = definition + Indent(customQuota.Name+": '"+customQuota.Value+"'\n", strings.Repeat(" ", 4))
 	}
 
-	//Special formatting for custom quotas so manually write these and then set to nil for marshalling
+	// Special formatting for custom quotas so manually write these and then set to nil for marshalling
 	quotas.CustomQuotas = nil
 
 	d, err := yaml.Marshal(&quotas)
@@ -165,7 +163,7 @@ spec:
 
 	definition = definition + Indent(string(d), strings.Repeat(" ", 4))
 
-	if err := ioutil.WriteFile("temp-ResourceQuota.yaml", []byte(definition), 0666); err != nil {
+	if err := ioutil.WriteFile("temp-ResourceQuota.yaml", []byte(definition), 0o666); err != nil {
 		log.Fatal(err.Error())
 	}
 

internal/app/plan.go

@@ -140,13 +140,13 @@ func releaseWithHooks(cmd orderedCommand, storageBackend string, wg *sync.WaitGr
 		for _, c := range cmd.beforeCommands {
 			if err := execOne(c.Command, cmd.targetRelease); err != nil {
 				errors <- err
-				if key, err := c.getAnnotationKey(); err != nil {
+				if key, err := c.getAnnotationKey(); err == nil {
 					annotations = append(annotations, key+"=failed")
 				}
 				log.Verbose(err.Error())
 				return
 			}
-			if key, err := c.getAnnotationKey(); err != nil {
+			if key, err := c.getAnnotationKey(); err == nil {
 				annotations = append(annotations, key+"=ok")
 			}
 		}
@@ -166,12 +166,12 @@ func releaseWithHooks(cmd orderedCommand, storageBackend string, wg *sync.WaitGr
 		for _, c := range cmd.afterCommands {
 			if err := execOne(c.Command, cmd.targetRelease); err != nil {
 				errors <- err
-				if key, err := c.getAnnotationKey(); err != nil {
+				if key, err := c.getAnnotationKey(); err == nil {
 					annotations = append(annotations, key+"=failed")
 				}
 				log.Verbose(err.Error())
 			} else {
-				if key, err := c.getAnnotationKey(); err != nil {
+				if key, err := c.getAnnotationKey(); err == nil {
 					annotations = append(annotations, key+"=ok")
 				}
 			}

internal/app/release.go

@@ -80,12 +80,12 @@ func (r *release) validate(appLabel string, seen map[string]map[string]bool, s *
 		return errors.New("valuesFile and valuesFiles should not be used together")
 	} else if r.ValuesFile != "" {
 		if err := isValidFile(r.ValuesFile, validFiles); err != nil {
-			return err
+			return fmt.Errorf("invalid values file: %w", err)
 		}
 	} else if len(r.ValuesFiles) > 0 {
 		for _, filePath := range r.ValuesFiles {
 			if err := isValidFile(filePath, validFiles); err != nil {
-				return err
+				return fmt.Errorf("invalid values file: %w", err)
 			}
 		}
 	}
@@ -94,18 +94,18 @@ func (r *release) validate(appLabel string, seen map[string]map[string]bool, s *
 		return errors.New("secretsFile and secretsFiles should not be used together")
 	} else if r.SecretsFile != "" {
 		if err := isValidFile(r.SecretsFile, validFiles); err != nil {
-			return err
+			return fmt.Errorf("invalid secrets file: %w", err)
 		}
 	} else if len(r.SecretsFiles) > 0 {
 		for _, filePath := range r.SecretsFiles {
 			if err := isValidFile(filePath, validFiles); err != nil {
-				return err
+				return fmt.Errorf("invalid secrets file: %w", err)
 			}
 		}
 	}
 
 	if r.PostRenderer != "" && !ToolExists(r.PostRenderer) {
-		return fmt.Errorf("%s must be valid relative (from dsf file) file path", r.PostRenderer)
+		return fmt.Errorf("%s must be executable and available in your PATH", r.PostRenderer)
 	}
 
 	if r.Priority != 0 && r.Priority > 0 {
@@ -159,7 +159,6 @@ func (r *release) uninstall(p *plan, optionalNamespace ...string) {
 
 	cmd := helmCmd(r.getHelmArgsFor("uninstall", ns), "Delete release [ "+r.Name+" ] in namespace [ "+ns+" ]")
 	p.addCommand(cmd, priority, r, before, after)
-
 }
 
 // diffRelease diffs an existing release with the specified values.yaml
@@ -186,7 +185,6 @@ func (r *release) diff() (string, error) {
 
 // upgradeRelease upgrades an existing release with the specified values.yaml
 func (r *release) upgrade(p *plan) {
-
 	before, after := r.checkHooks("upgrade")
 
 	if r.Test {
@@ -196,7 +194,6 @@ func (r *release) upgrade(p *plan) {
 	cmd := helmCmd(r.getHelmArgsFor("upgrade"), "Upgrade release [ "+r.Name+" ] to version [ "+r.Version+" ] in namespace [ "+r.Namespace+" ]")
 
 	p.addCommand(cmd, r.Priority, r, before, after)
-
 }
 
 // reInstall uninstalls a release and reinstalls it.
@@ -223,7 +220,7 @@ func (r *release) rollback(cs *currentState, p *plan) {
 
 	if r.Namespace == rs.Namespace {
 
-		cmd := helmCmd(concat([]string{"rollback", r.Name, rs.getRevision()}, r.getWait(), r.getTimeout(), r.getNoHooks(), flags.getDryRunFlags()), "Rolling back release [ "+r.Name+" ] in namespace [ "+r.Namespace+" ]")
+		cmd := helmCmd(concat([]string{"rollback", r.Name, rs.getRevision()}, r.getWait(), r.getTimeout(), r.getNoHooks(), flags.getRunFlags()), "Rolling back release [ "+r.Name+" ] in namespace [ "+r.Namespace+" ]")
 		p.addCommand(cmd, r.Priority, r, []hookCmd{}, []hookCmd{})
 		r.upgrade(p) // this is to reflect any changes in values file(s)
 		p.addDecision("Release [ "+r.Name+" ] was deleted and is desired to be rolled back to "+
@@ -369,7 +366,7 @@ func (r *release) getHelmFlags() []string {
 	}
 
 	flgs = append(flgs, r.HelmFlags...)
-	return concat(r.getNoHooks(), r.getWait(), r.getTimeout(), r.getMaxHistory(), flags.getDryRunFlags(), []string{force}, flgs)
+	return concat(r.getNoHooks(), r.getWait(), r.getTimeout(), r.getMaxHistory(), flags.getRunFlags(), []string{force}, flgs)
 }
 
 // getPostRenderer returns the post-renderer Helm flag
@@ -393,7 +390,7 @@ func (r *release) getHelmArgsFor(action string, optionalNamespaceOverride ...str
 	case "diff":
 		return concat([]string{"upgrade", r.Name, r.Chart, "--version", r.Version, "--namespace", r.Namespace}, r.getValuesFiles(), r.getSetValues(), r.getSetStringValues(), r.getSetFileValues(), r.getPostRenderer())
 	case "uninstall":
-		return concat([]string{action, "--namespace", ns, r.Name}, flags.getDryRunFlags())
+		return concat([]string{action, "--namespace", ns, r.Name}, flags.getRunFlags())
 	default:
 		return []string{action, "--namespace", ns, r.Name}
 	}

internal/app/release_test.go

@@ -54,7 +54,7 @@ func Test_release_validate(t *testing.T) {
 				},
 				s: st,
 			},
-			want: "xyz.yaml must be valid relative (from dsf file) file path",
+			want: "invalid values file: xyz.yaml must be valid relative (from dsf file) file path: stat xyz.yaml: no such file or directory",
 		}, {
 			name: "test case 3",
 			args: args{
@@ -70,7 +70,7 @@ func Test_release_validate(t *testing.T) {
 				},
 				s: st,
 			},
-			want: "../../tests/values.xml must be of one the following file formats: .yaml, .yml, .json",
+			want: "invalid values file: ../../tests/values.xml must be of one the following file formats: .yaml, .yml, .json",
 		}, {
 			name: "test case 4",
 			args: args{
@@ -215,7 +215,7 @@ func Test_release_validate(t *testing.T) {
 				},
 				s: st,
 			},
-			want: "xyz.yaml must be valid relative (from dsf file) file path",
+			want: "invalid values file: xyz.yaml must be valid relative (from dsf file) file path: stat xyz.yaml: no such file or directory",
 		}, {
 			name: "test case 13",
 			args: args{
@@ -247,7 +247,7 @@ func Test_release_validate(t *testing.T) {
 				},
 				s: st,
 			},
-			want: "xyz.fake must be valid relative (from dsf file) file path",
+			want: "invalid hook manifest: xyz.fake must be valid relative (from dsf file) file path: stat xyz.fake: no such file or directory",
 		}, {
 			name: "test case 15 - invalid hook file type",
 			args: args{
@@ -263,7 +263,7 @@ func Test_release_validate(t *testing.T) {
 				},
 				s: st,
 			},
-			want: "../../tests/values.xml must be of one the following file formats: .yaml, .yml, .json",
+			want: "invalid hook manifest: ../../tests/values.xml must be of one the following file formats: .yaml, .yml, .json",
 		}, {
 			name: "test case 16 - valid hook file type",
 			args: args{
@@ -311,7 +311,7 @@ func Test_release_validate(t *testing.T) {
 				},
 				s: st,
 			},
-			want: "https//raw.githubusercontent.com/jetstack/cert-manager/release-0.14/deploy/manifests/00-crds.yaml must be valid URL path to a raw file",
+			want: "invalid hook manifest: https//raw.githubusercontent.com/jetstack/cert-manager/release-0.14/deploy/manifests/00-crds.yaml must be valid URL path to a raw file",
 		}, {
 			name: "test case 19 - invalid hook type 1",
 			args: args{
@@ -377,7 +377,7 @@ func Test_release_validate(t *testing.T) {
 				},
 				s: st,
 			},
-			want: "doesnt-exist.sh must be valid relative (from dsf file) file path",
+			want: "doesnt-exist.sh must be executable and available in your PATH",
 		}, {
 			name: "test case 23 - executable hook type",
 			args: args{

internal/app/state.go

@@ -84,7 +84,6 @@ func (s *state) initializeNamespaces() {
 // validate validates that the values specified in the desired state are valid according to the desired state spec.
 // check https://github.com/Praqma/helmsman/blob/master/docs/desired_state_specification.md for the detailed specification
 func (s *state) validate() error {
-
 	// apps
 	if s.Apps == nil {
 		log.Info("No apps specified. Nothing to be executed.")
@@ -151,7 +150,6 @@ func (s *state) validate() error {
 				return errors.New("certificates validation failed -- connection to cluster is required " +
 					"but no cert/key was given. Please add [caCrt] and [caKey] under Certifications. You might also need to provide [clientCrt]")
 			}
-
 		} else if s.Settings.ClusterURI != "" && s.Settings.BearerToken {
 			if !caCrt {
 				return errors.New("certificates validation failed -- cluster connection with bearer token is enabled but " +
@@ -251,7 +249,7 @@ func (s *state) getReleaseChartsInfo() error {
 					mutex.Unlock()
 				}
 
-				//validateChart(concattedApps, chart, version, chartErrors)
+				// validateChart(concattedApps, chart, version, chartErrors)
 			}(concattedApps, chart, version)
 		}
 	}
@@ -337,7 +335,6 @@ func (s *state) updateContextLabels() {
 
 // print prints the desired state
 func (s *state) print() {
-
 	fmt.Println("\nMetadata: ")
 	fmt.Println("--------- ")
 	printMap(s.Metadata, 0)

internal/app/state_files.go

@@ -146,7 +146,7 @@ func (s *state) expand(relativeToFile string) {
 
 		r.resolvePaths(dir, downloadDest)
 		if r.Chart != "" {
-			var repoOrDir = filepath.Dir(r.Chart)
+			repoOrDir := filepath.Dir(r.Chart)
 			_, isRepo := s.HelmRepos[repoOrDir]
 			isRepo = isRepo || stringInSlice(repoOrDir, s.PreconfiguredHelmRepos)
 			if !isRepo {

internal/app/state_files_test.go

@@ -8,7 +8,7 @@ import (
 
 func setupStateFileTestCase(t *testing.T) func(t *testing.T) {
 	t.Log("setup test case")
-	os.MkdirAll(tempFilesDir, 0755)
+	os.MkdirAll(tempFilesDir, 0o755)
 
 	return func(t *testing.T) {
 		t.Log("teardown test case")
@@ -57,6 +57,7 @@ func Test_fromTOML(t *testing.T) {
 	os.Unsetenv("ORG_PATH")
 	os.Unsetenv("VALUE")
 }
+
 func Test_fromTOML_Expand(t *testing.T) {
 	type args struct {
 		file string

internal/app/state_test.go

@@ -8,7 +8,7 @@ import (
 
 func setupTestCase(t *testing.T) func(t *testing.T) {
 	t.Log("setup test case")
-	os.MkdirAll(tempFilesDir, 0755)
+	os.MkdirAll(tempFilesDir, 0o755)
 	os.MkdirAll(os.TempDir()+"/helmsman-tests/myapp", os.ModePerm)
 	os.MkdirAll(os.TempDir()+"/helmsman-tests/dir-with space/myapp", os.ModePerm)
 	cmd := helmCmd([]string{"create", os.TempDir() + "/helmsman-tests/dir-with space/myapp"}, "creating an empty local chart directory")