From 28be15de566914fbf84e6cc75591ae1d2e1bbcfb Mon Sep 17 00:00:00 2001 From: Alex Collins Date: Wed, 7 Aug 2019 09:59:02 -0700 Subject: [PATCH] Adds support for setting Helm string parameters via CLI. Closes #2078 (#2109) --- .codecov.yml | 9 +-- cmd/argocd/commands/app.go | 72 ++++++++++++--------- cmd/argocd/commands/app_test.go | 30 +++++++++ pkg/apis/application/v1alpha1/types.go | 28 ++++++++ pkg/apis/application/v1alpha1/types_test.go | 30 +++++++++ test/e2e/helm_test.go | 49 ++++++++++++++ 6 files changed, 185 insertions(+), 33 deletions(-) diff --git a/.codecov.yml b/.codecov.yml index 5e9a6caaf6139..4ecd53db2250f 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -1,17 +1,18 @@ ignore: - "**/*.pb.go" - "**/*.pb.gw.go" +- "**/*generated.go" - "**/*_test.go" -- "pkg/apis/.*" +- "pkg/apis/client/.*" - "pkg/client/.*" - "test/.*" coverage: status: - # allow test coverage to drop by 1%, assume that it's typically due to CI problems patch: default: - enabled: no - if_not_found: success + # allow test coverage to drop by 10%, assume that it's typically due to CI problems + threshold: 10 project: default: + # allow test coverage to drop by 1%, assume that it's typically due to CI problems threshold: 1 \ No newline at end of file diff --git a/cmd/argocd/commands/app.go b/cmd/argocd/commands/app.go index 270821a63ecb5..de3b5a3e7a585 100644 --- a/cmd/argocd/commands/app.go +++ b/cmd/argocd/commands/app.go @@ -12,7 +12,6 @@ import ( "path" "path/filepath" "reflect" - "regexp" "sort" "strconv" "strings" @@ -404,9 +403,13 @@ func setAppOptions(flags *pflag.FlagSet, app *argoappv1.Application, appOpts *ap case "revision": app.Spec.Source.TargetRevision = appOpts.revision case "values": - setHelmOpt(&app.Spec.Source, appOpts.valuesFiles, nil) + setHelmOpt(&app.Spec.Source, helmOpts{valueFiles: appOpts.valuesFiles}) case "release-name": - setHelmOpt(&app.Spec.Source, nil, &appOpts.releaseName) + setHelmOpt(&app.Spec.Source, helmOpts{releaseName: appOpts.releaseName}) + case "helm-set": + setHelmOpt(&app.Spec.Source, helmOpts{helmSets: appOpts.helmSets}) + case "helm-set-string": + setHelmOpt(&app.Spec.Source, helmOpts{helmSetStrings: appOpts.helmSetStrings}) case "directory-recurse": app.Spec.Source.Directory = &argoappv1.ApplicationSourceDirectory{Recurse: appOpts.directoryRecurse} case "config-management-plugin": @@ -483,15 +486,36 @@ func setKustomizeImages(src *argoappv1.ApplicationSource, images []string) { } } -func setHelmOpt(src *argoappv1.ApplicationSource, valueFiles []string, releaseName *string) { +type helmOpts struct { + valueFiles []string + releaseName string + helmSets []string + helmSetStrings []string +} + +func setHelmOpt(src *argoappv1.ApplicationSource, opts helmOpts) { if src.Helm == nil { src.Helm = &argoappv1.ApplicationSourceHelm{} } - if valueFiles != nil { - src.Helm.ValueFiles = valueFiles + if len(opts.valueFiles) > 0 { + src.Helm.ValueFiles = opts.valueFiles + } + if opts.releaseName != "" { + src.Helm.ReleaseName = opts.releaseName + } + for _, text := range opts.helmSets { + p, err := argoappv1.NewHelmParameter(text, false) + if err != nil { + log.Fatal(err) + } + src.Helm.AddParameter(*p) } - if releaseName != nil { - src.Helm.ReleaseName = *releaseName + for _, text := range opts.helmSetStrings { + p, err := argoappv1.NewHelmParameter(text, true) + if err != nil { + log.Fatal(err) + } + src.Helm.AddParameter(*p) } if src.Helm.IsZero() { src.Helm = nil @@ -541,6 +565,8 @@ type appOptions struct { parameters []string valuesFiles []string releaseName string + helmSets []string + helmSetStrings []string project string syncPolicy string autoPrune bool @@ -562,6 +588,8 @@ func addAppFlags(command *cobra.Command, opts *appOptions) { command.Flags().StringArrayVarP(&opts.parameters, "parameter", "p", []string{}, "set a parameter override (e.g. -p guestbook=image=example/guestbook:latest)") command.Flags().StringArrayVar(&opts.valuesFiles, "values", []string{}, "Helm values file(s) to use") command.Flags().StringVar(&opts.releaseName, "release-name", "", "Helm release-name") + command.Flags().StringArrayVar(&opts.helmSets, "helm-set", []string{}, "Helm set values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2)") + command.Flags().StringArrayVar(&opts.helmSetStrings, "helm-set-string", []string{}, "Helm set STRING values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2)") command.Flags().StringVar(&opts.project, "project", "", "Application project name") command.Flags().StringVar(&opts.syncPolicy, "sync-policy", "", "Set the sync policy (one of: automated, none)") command.Flags().BoolVar(&opts.autoPrune, "auto-prune", false, "Set automatic pruning when sync is automated") @@ -631,7 +659,7 @@ func NewApplicationUnsetCommand(clientOpts *argocdclient.ClientOptions) *cobra.C } } } - setHelmOpt(&app.Spec.Source, specValueFiles, nil) + setHelmOpt(&app.Spec.Source, helmOpts{valueFiles: specValueFiles}) if !updated { return } @@ -1566,27 +1594,13 @@ func setParameterOverrides(app *argoappv1.Application, parameters []string) { if app.Spec.Source.Helm == nil { app.Spec.Source.Helm = &argoappv1.ApplicationSourceHelm{} } - re := regexp.MustCompile(`([^\\]),`) - for _, paramStr := range parameters { - parts := strings.SplitN(paramStr, "=", 2) - if len(parts) != 2 { - log.Fatalf("Expected helm parameter of the form: param=value. Received: %s", paramStr) - } - newParam := argoappv1.HelmParameter{ - Name: parts[0], - Value: re.ReplaceAllString(parts[1], `$1\,`), - } - found := false - for i, cp := range app.Spec.Source.Helm.Parameters { - if cp.Name == newParam.Name { - found = true - app.Spec.Source.Helm.Parameters[i] = newParam - break - } - } - if !found { - app.Spec.Source.Helm.Parameters = append(app.Spec.Source.Helm.Parameters, newParam) + for _, p := range parameters { + newParam, err := argoappv1.NewHelmParameter(p, false) + if err != nil { + log.Error(err) + continue } + app.Spec.Source.Helm.AddParameter(*newParam) } default: log.Fatalf("Parameters can only be set against Ksonnet or Helm applications") diff --git a/cmd/argocd/commands/app_test.go b/cmd/argocd/commands/app_test.go index 0e693962a5662..960ee41399ada 100644 --- a/cmd/argocd/commands/app_test.go +++ b/cmd/argocd/commands/app_test.go @@ -4,6 +4,8 @@ import ( "testing" "github.com/stretchr/testify/assert" + + "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1" ) func TestParseLabels(t *testing.T) { @@ -23,3 +25,31 @@ func TestParseLabels(t *testing.T) { assert.Len(t, result, 0) } + +func Test_setHelmOpt(t *testing.T) { + t.Run("Zero", func(t *testing.T) { + src := v1alpha1.ApplicationSource{} + setHelmOpt(&src, helmOpts{}) + assert.Nil(t, src.Helm) + }) + t.Run("ValueFiles", func(t *testing.T) { + src := v1alpha1.ApplicationSource{} + setHelmOpt(&src, helmOpts{valueFiles: []string{"foo"}}) + assert.Equal(t, []string{"foo"}, src.Helm.ValueFiles) + }) + t.Run("ReleaseName", func(t *testing.T) { + src := v1alpha1.ApplicationSource{} + setHelmOpt(&src, helmOpts{releaseName: "foo"}) + assert.Equal(t, "foo", src.Helm.ReleaseName) + }) + t.Run("HelmSets", func(t *testing.T) { + src := v1alpha1.ApplicationSource{} + setHelmOpt(&src, helmOpts{helmSets: []string{"foo=bar"}}) + assert.Equal(t, []v1alpha1.HelmParameter{{Name: "foo", Value: "bar"}}, src.Helm.Parameters) + }) + t.Run("HelmSetStrings", func(t *testing.T) { + src := v1alpha1.ApplicationSource{} + setHelmOpt(&src, helmOpts{helmSetStrings: []string{"foo=bar"}}) + assert.Equal(t, []v1alpha1.HelmParameter{{Name: "foo", Value: "bar", ForceString: true}}, src.Helm.Parameters) + }) +} diff --git a/pkg/apis/application/v1alpha1/types.go b/pkg/apis/application/v1alpha1/types.go index 8c140706b824b..e264b3841c4da 100644 --- a/pkg/apis/application/v1alpha1/types.go +++ b/pkg/apis/application/v1alpha1/types.go @@ -161,6 +161,34 @@ type HelmParameter struct { ForceString bool `json:"forceString,omitempty" protobuf:"bytes,3,opt,name=forceString"` } +var helmParameterRx = regexp.MustCompile(`([^\\]),`) + +func NewHelmParameter(text string, forceString bool) (*HelmParameter, error) { + parts := strings.SplitN(text, "=", 2) + if len(parts) != 2 { + return nil, fmt.Errorf("Expected helm parameter of the form: param=value. Received: %s", text) + } + return &HelmParameter{ + Name: parts[0], + Value: helmParameterRx.ReplaceAllString(parts[1], `$1\,`), + ForceString: forceString, + }, nil +} + +func (in *ApplicationSourceHelm) AddParameter(p HelmParameter) { + found := false + for i, cp := range in.Parameters { + if cp.Name == p.Name { + found = true + in.Parameters[i] = p + break + } + } + if !found { + in.Parameters = append(in.Parameters, p) + } +} + func (h *ApplicationSourceHelm) IsZero() bool { return h == nil || (h.ReleaseName == "") && len(h.ValueFiles) == 0 && len(h.Parameters) == 0 } diff --git a/pkg/apis/application/v1alpha1/types_test.go b/pkg/apis/application/v1alpha1/types_test.go index d3a76a92e0c96..e573d366f1b0e 100644 --- a/pkg/apis/application/v1alpha1/types_test.go +++ b/pkg/apis/application/v1alpha1/types_test.go @@ -628,6 +628,36 @@ func TestApplicationSource_IsZero(t *testing.T) { } } +func TestApplicationSourceHelm_AddParameter(t *testing.T) { + src := ApplicationSourceHelm{} + t.Run("Add", func(t *testing.T) { + src.AddParameter(HelmParameter{Value: "bar"}) + assert.ElementsMatch(t, []HelmParameter{{Value: "bar"}}, src.Parameters) + + }) + t.Run("Replace", func(t *testing.T) { + src.AddParameter(HelmParameter{Value: "baz"}) + assert.ElementsMatch(t, []HelmParameter{{Value: "baz"}}, src.Parameters) + }) +} + +func TestNewHelmParameter(t *testing.T) { + t.Run("Invalid", func(t *testing.T) { + _, err := NewHelmParameter("garbage", false) + assert.EqualError(t, err, "Expected helm parameter of the form: param=value. Received: garbage") + }) + t.Run("NonString", func(t *testing.T) { + p, err := NewHelmParameter("foo=bar", false) + assert.NoError(t, err) + assert.Equal(t, &HelmParameter{Name: "foo", Value: "bar"}, p) + }) + t.Run("String", func(t *testing.T) { + p, err := NewHelmParameter("foo=bar", true) + assert.NoError(t, err) + assert.Equal(t, &HelmParameter{Name: "foo", Value: "bar", ForceString: true}, p) + }) +} + func TestApplicationSourceHelm_IsZero(t *testing.T) { tests := []struct { name string diff --git a/test/e2e/helm_test.go b/test/e2e/helm_test.go index af2876ac5e463..3dd89c7f4cba8 100644 --- a/test/e2e/helm_test.go +++ b/test/e2e/helm_test.go @@ -3,6 +3,7 @@ package e2e import ( "testing" + "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" . "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1" @@ -64,3 +65,51 @@ func TestDeclarativeHelmInvalidValuesFile(t *testing.T) { Expect(SyncStatusIs(SyncStatusCodeUnknown)). Expect(Condition(ApplicationConditionComparisonError, "open does-not-exist-values.yaml: no such file or directory")) } + +func TestHelmValues(t *testing.T) { + Given(t). + Path("helm"). + When(). + Create(). + AppSet("--values", "foo"). + Then(). + And(func(app *Application) { + assert.Equal(t, []string{"foo"}, app.Spec.Source.Helm.ValueFiles) + }) +} + +func TestHelmReleaseName(t *testing.T) { + Given(t). + Path("helm"). + When(). + Create(). + AppSet("--release-name", "foo"). + Then(). + And(func(app *Application) { + assert.Equal(t, "foo", app.Spec.Source.Helm.ReleaseName) + }) +} + +func TestHelmSet(t *testing.T) { + Given(t). + Path("helm"). + When(). + Create(). + AppSet("--helm-set", "foo=bar", "--helm-set", "foo=baz"). + Then(). + And(func(app *Application) { + assert.Equal(t, []HelmParameter{{Name: "foo", Value: "baz"}}, app.Spec.Source.Helm.Parameters) + }) +} + +func TestHelmSetString(t *testing.T) { + Given(t). + Path("helm"). + When(). + Create(). + AppSet("--helm-set-string", "foo=bar", "--helm-set-string", "foo=baz"). + Then(). + And(func(app *Application) { + assert.Equal(t, []HelmParameter{{Name: "foo", Value: "baz", ForceString: true}}, app.Spec.Source.Helm.Parameters) + }) +}