From 6c5dba3cddd33d170540c7162d16e43ca40b30b3 Mon Sep 17 00:00:00 2001 From: Meng JiaFeng Date: Wed, 3 Aug 2022 14:53:51 +0800 Subject: [PATCH 1/3] feat: support helm plugin default value Signed-off-by: Meng JiaFeng --- docs/plugins/argocd.md | 13 +- internal/pkg/plugin/argocd/argocd.go | 30 ++--- internal/pkg/plugin/argocd/create.go | 2 +- internal/pkg/plugin/argocd/delete.go | 2 +- internal/pkg/plugin/argocd/read.go | 2 +- internal/pkg/plugin/argocd/update.go | 2 +- .../plugininstaller/helm/helm_suit_test.go | 13 ++ internal/pkg/plugininstaller/helm/option.go | 19 ++- .../pkg/plugininstaller/helm/option_test.go | 117 ++++++++++++++++++ internal/pkg/plugininstaller/helm/validate.go | 14 ++- .../pkg/plugininstaller/helm/validate_test.go | 107 ++++++++++++++++ pkg/util/helm/helm.go | 16 ++- pkg/util/helm/helm_operation_test.go | 18 +-- pkg/util/helm/helm_test.go | 10 +- pkg/util/helm/param.go | 48 +++++-- pkg/util/helm/validation.go | 15 +-- pkg/util/helm/validation_test.go | 78 +----------- 17 files changed, 361 insertions(+), 145 deletions(-) create mode 100644 internal/pkg/plugininstaller/helm/helm_suit_test.go create mode 100644 internal/pkg/plugininstaller/helm/option_test.go create mode 100644 internal/pkg/plugininstaller/helm/validate_test.go diff --git a/docs/plugins/argocd.md b/docs/plugins/argocd.md index 6ce5b6792..3d07e36c7 100644 --- a/docs/plugins/argocd.md +++ b/docs/plugins/argocd.md @@ -8,4 +8,15 @@ This plugin installs [ArgoCD](https://argoproj.github.io/cd/) in an existing Kub --8<-- "argocd.yaml" ``` -Currently, except for `values_yaml`, all the parameters in the example above are mandatory. +### Default Configs + +| key | default value | description | +| ---- | ---- | ---- | +| chart.chart_name | argo/argo-cd | argocd's official chart name | +| chart.timeout | 5m | this config will wait 5 minutes to deploy argocd | +| upgradeCRDs | true | default update CRD config | +| chart.wait | true | whether to wait until install is complete | +| repo.url | https://argoproj.github.io/argo-helm | helm repo address | +| repo.name | argo | helm repo name | + +Currently, except for `values_yaml` and default configs, all the parameters in the example above are mandatory. \ No newline at end of file diff --git a/internal/pkg/plugin/argocd/argocd.go b/internal/pkg/plugin/argocd/argocd.go index 1ed389e97..e5de17226 100644 --- a/internal/pkg/plugin/argocd/argocd.go +++ b/internal/pkg/plugin/argocd/argocd.go @@ -1,25 +1,19 @@ package argocd import ( - "github.com/devstream-io/devstream/internal/pkg/plugininstaller" "github.com/devstream-io/devstream/internal/pkg/plugininstaller/helm" + helmCommon "github.com/devstream-io/devstream/pkg/util/helm" ) -var ( - defaultRepoURL = "https://argoproj.github.io/argo-helm" - defaultRepoName = "argo" -) - -func defaultMissedOption(options plugininstaller.RawOptions) (plugininstaller.RawOptions, error) { - opts, err := helm.NewOptions(options) - if err != nil { - return nil, err - } - if opts.Repo.URL == "" { - opts.Repo.URL = defaultRepoURL - } - if opts.Repo.Name == "" { - opts.Repo.Name = defaultRepoName - } - return opts.Encode() +var defaultHelmConfig = helm.Options{ + Chart: helmCommon.Chart{ + ChartName: "argo/argo-cd", + Timeout: "5m", + UpgradeCRDs: helmCommon.GetBoolTrueAddress(), + Wait: helmCommon.GetBoolTrueAddress(), + }, + Repo: helmCommon.Repo{ + URL: "https://argoproj.github.io/argo-helm", + Name: "argo", + }, } diff --git a/internal/pkg/plugin/argocd/create.go b/internal/pkg/plugin/argocd/create.go index 81cade770..bf2789b94 100644 --- a/internal/pkg/plugin/argocd/create.go +++ b/internal/pkg/plugin/argocd/create.go @@ -11,7 +11,7 @@ func Create(options map[string]interface{}) (map[string]interface{}, error) { // 1. config install operations runner := &plugininstaller.Runner{ PreExecuteOperations: []plugininstaller.MutableOperation{ - defaultMissedOption, + helm.SetDefaultConfig(&defaultHelmConfig), helm.Validate, }, ExecuteOperations: helm.DefaultCreateOperations, diff --git a/internal/pkg/plugin/argocd/delete.go b/internal/pkg/plugin/argocd/delete.go index 73fddba80..f39ed557f 100644 --- a/internal/pkg/plugin/argocd/delete.go +++ b/internal/pkg/plugin/argocd/delete.go @@ -9,7 +9,7 @@ func Delete(options map[string]interface{}) (bool, error) { // 1. config delete operations runner := &plugininstaller.Runner{ PreExecuteOperations: []plugininstaller.MutableOperation{ - defaultMissedOption, + helm.SetDefaultConfig(&defaultHelmConfig), helm.Validate, }, ExecuteOperations: helm.DefaultDeleteOperations, diff --git a/internal/pkg/plugin/argocd/read.go b/internal/pkg/plugin/argocd/read.go index 058a4a62a..82d81cbdf 100644 --- a/internal/pkg/plugin/argocd/read.go +++ b/internal/pkg/plugin/argocd/read.go @@ -14,7 +14,7 @@ func Read(options map[string]interface{}) (map[string]interface{}, error) { // 1. config read operations runner := &plugininstaller.Runner{ PreExecuteOperations: []plugininstaller.MutableOperation{ - defaultMissedOption, + helm.SetDefaultConfig(&defaultHelmConfig), helm.Validate, }, GetStatusOperation: helm.GetPluginAllState, diff --git a/internal/pkg/plugin/argocd/update.go b/internal/pkg/plugin/argocd/update.go index 5e8257d22..2080e60d6 100644 --- a/internal/pkg/plugin/argocd/update.go +++ b/internal/pkg/plugin/argocd/update.go @@ -10,7 +10,7 @@ func Update(options map[string]interface{}) (map[string]interface{}, error) { // 1. config update operations runner := &plugininstaller.Runner{ PreExecuteOperations: []plugininstaller.MutableOperation{ - defaultMissedOption, + helm.SetDefaultConfig(&defaultHelmConfig), helm.Validate, }, ExecuteOperations: helm.DefaultUpdateOperations, diff --git a/internal/pkg/plugininstaller/helm/helm_suit_test.go b/internal/pkg/plugininstaller/helm/helm_suit_test.go new file mode 100644 index 000000000..a7a9da004 --- /dev/null +++ b/internal/pkg/plugininstaller/helm/helm_suit_test.go @@ -0,0 +1,13 @@ +package helm_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestPlanmanager(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "PluginInstaller Helm Suite") +} diff --git a/internal/pkg/plugininstaller/helm/option.go b/internal/pkg/plugininstaller/helm/option.go index bc2d71d9b..abf741bdb 100644 --- a/internal/pkg/plugininstaller/helm/option.go +++ b/internal/pkg/plugininstaller/helm/option.go @@ -9,9 +9,9 @@ import ( // Options is the struct for parameters used by the helm install config. type Options struct { - CreateNamespace bool `mapstructure:"create_namespace"` - Repo helm.Repo - Chart helm.Chart + CreateNamespace bool `mapstructure:"create_namespace"` + Repo helm.Repo `mapstructure:"repo"` + Chart helm.Chart `mapstructure:"chart"` } func (opts *Options) GetHelmParam() *helm.HelmParam { @@ -41,11 +41,18 @@ func (opts *Options) Encode() (map[string]interface{}, error) { return options, nil } +func (opts *Options) fillDefaultValue(defaultOpts *Options) { + chart := &opts.Chart + chart.FillDefaultValue(&defaultOpts.Chart) + repo := &opts.Repo + repo.FillDefaultValue(&defaultOpts.Repo) +} + // NewOptions create options by raw options -func NewOptions(options plugininstaller.RawOptions) (Options, error) { +func NewOptions(options plugininstaller.RawOptions) (*Options, error) { var opts Options if err := mapstructure.Decode(options, &opts); err != nil { - return opts, err + return nil, err } - return opts, nil + return &opts, nil } diff --git a/internal/pkg/plugininstaller/helm/option_test.go b/internal/pkg/plugininstaller/helm/option_test.go new file mode 100644 index 000000000..f43fb60d2 --- /dev/null +++ b/internal/pkg/plugininstaller/helm/option_test.go @@ -0,0 +1,117 @@ +package helm_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/devstream-io/devstream/internal/pkg/plugininstaller" + "github.com/devstream-io/devstream/internal/pkg/plugininstaller/helm" + helmCommon "github.com/devstream-io/devstream/pkg/util/helm" +) + +var _ = Describe("Options struct", func() { + var ( + testOpts helm.Options + testChartName string + testRepoName string + testNameSpace string + expectMap map[string]interface{} + emptyBool *bool + ) + + BeforeEach(func() { + testChartName = "test_chart" + testRepoName = "test_repo" + testNameSpace = "test_nameSpace" + testOpts = helm.Options{ + Chart: helmCommon.Chart{ + ChartName: testChartName, + Namespace: testNameSpace, + }, + Repo: helmCommon.Repo{ + Name: testRepoName, + }, + } + expectMap = map[string]interface{}{ + "create_namespace": false, + "repo": map[string]interface{}{ + "name": "test_repo", + "url": "", + }, + "chart": map[string]interface{}{ + "version": "", + "release_name": "", + "wait": emptyBool, + "chart_name": "test_chart", + "name_space": "test_nameSpace", + "create_namespace": emptyBool, + "timeout": "", + "upgradeCRDs": emptyBool, + "values_yaml": "", + }, + } + }) + + Context("GetHelmParam method", func() { + It("should pass chart and repo field", func() { + helmParam := testOpts.GetHelmParam() + Expect(helmParam.Chart).Should(Equal(testOpts.Chart)) + Expect(helmParam.Repo).Should(Equal(testOpts.Repo)) + }) + }) + + Context("CheckIfCreateNamespace method", func() { + It("should equal opts config", func() { + Expect(testOpts.CheckIfCreateNamespace()).Should(Equal(testOpts.CreateNamespace)) + }) + }) + + Context("GetNamespace method", func() { + It("should return chart's nameSpace", func() { + Expect(testOpts.GetNamespace()).Should(Equal(testOpts.Chart.Namespace)) + }) + }) + + Context("GetReleaseName method", func() { + It("should return chart's ReleaseName", func() { + Expect(testOpts.GetReleaseName()).Should(Equal(testOpts.Chart.ReleaseName)) + }) + }) + + Context("Encode method", func() { + It("should return opts map", func() { + result, err := testOpts.Encode() + Expect(err).Error().ShouldNot(HaveOccurred()) + Expect(result).Should(Equal(expectMap)) + }) + }) +}) + +var _ = Describe("NewOptions func", func() { + + var ( + inputOptions plugininstaller.RawOptions + testRepoName string + testChartName string + ) + + BeforeEach(func() { + testRepoName = "test_repo" + testChartName = "test_chart" + inputOptions = map[string]interface{}{ + "repo": map[string]interface{}{ + "name": testRepoName, + }, + "chart": map[string]interface{}{ + "chart_name": testChartName, + }, + } + }) + + It("should work normal", func() { + opts, err := helm.NewOptions(inputOptions) + Expect(err).Error().ShouldNot(HaveOccurred()) + Expect(opts.Chart.ChartName).Should(Equal(testChartName)) + Expect(opts.Repo.Name).Should(Equal(testRepoName)) + }) +}) diff --git a/internal/pkg/plugininstaller/helm/validate.go b/internal/pkg/plugininstaller/helm/validate.go index d3d014dca..c15e00abd 100644 --- a/internal/pkg/plugininstaller/helm/validate.go +++ b/internal/pkg/plugininstaller/helm/validate.go @@ -14,7 +14,7 @@ func Validate(options plugininstaller.RawOptions) (plugininstaller.RawOptions, e if err != nil { return nil, err } - errs := helm.DefaultsAndValidate(opts.GetHelmParam()) + errs := helm.Validate(opts.GetHelmParam()) if len(errs) > 0 { for _, e := range errs { log.Errorf("Options error: %s.", e) @@ -23,3 +23,15 @@ func Validate(options plugininstaller.RawOptions) (plugininstaller.RawOptions, e } return options, nil } + +// SetDefaultConfig will update options empty values base on import options +func SetDefaultConfig(defaultConfig *Options) plugininstaller.MutableOperation { + return func(options plugininstaller.RawOptions) (plugininstaller.RawOptions, error) { + opts, err := NewOptions(options) + if err != nil { + return nil, err + } + opts.fillDefaultValue(defaultConfig) + return opts.Encode() + } +} diff --git a/internal/pkg/plugininstaller/helm/validate_test.go b/internal/pkg/plugininstaller/helm/validate_test.go new file mode 100644 index 000000000..7fde9e41e --- /dev/null +++ b/internal/pkg/plugininstaller/helm/validate_test.go @@ -0,0 +1,107 @@ +package helm_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/devstream-io/devstream/internal/pkg/plugininstaller" + "github.com/devstream-io/devstream/internal/pkg/plugininstaller/helm" + helmCommon "github.com/devstream-io/devstream/pkg/util/helm" +) + +var _ = Describe("Validate func", func() { + var testOption plugininstaller.RawOptions + + When("options is not valid", func() { + BeforeEach(func() { + testOption = map[string]interface{}{ + "chart": map[string]string{}, + "repo": map[string]string{}, + } + }) + It("should return error", func() { + _, err := helm.Validate(testOption) + Expect(err).Error().Should(HaveOccurred()) + }) + }) + + When("options is valid", func() { + BeforeEach(func() { + testOption = map[string]interface{}{ + "chart": map[string]string{ + "chart_name": "test", + }, + "repo": map[string]string{ + "url": "http://test.com", + "name": "test", + }, + } + }) + It("should return success", func() { + opt, err := helm.Validate(testOption) + Expect(err).Error().ShouldNot(HaveOccurred()) + Expect(opt).ShouldNot(BeEmpty()) + }) + }) +}) + +var _ = Describe("SetDefaultConfig func", func() { + var ( + testChartName string + testRepoURL string + testRepoName string + testWait *bool + emptyBool *bool + defaultConfig helm.Options + testOptions plugininstaller.RawOptions + expectChart map[string]interface{} + expectRepo map[string]interface{} + ) + BeforeEach(func() { + testChartName = "test_chart" + testRepoName = "test_repo" + testRepoURL = "http://test.com" + testWait = helmCommon.GetBoolTrueAddress() + testOptions = map[string]interface{}{ + "chart": map[string]string{}, + "repo": map[string]string{}, + } + defaultConfig = helm.Options{ + Chart: helmCommon.Chart{ + ChartName: testChartName, + Wait: testWait, + UpgradeCRDs: testWait, + }, + Repo: helmCommon.Repo{ + URL: testRepoURL, + Name: testRepoName, + }, + } + expectChart = map[string]interface{}{ + "chart_name": testChartName, + "wait": testWait, + "name_space": "", + "version": "", + "release_name": "", + "values_yaml": "", + "timeout": "", + "create_namespace": emptyBool, + "upgradeCRDs": testWait, + } + expectRepo = map[string]interface{}{ + "url": testRepoURL, + "name": testRepoName, + } + }) + It("should update default value", func() { + updateFunc := helm.SetDefaultConfig(&defaultConfig) + o, err := updateFunc(testOptions) + Expect(err).Error().ShouldNot(HaveOccurred()) + oRepo, exist := o["repo"] + Expect(exist).Should(BeTrue()) + oChart, exist := o["chart"] + Expect(exist).Should(BeTrue()) + Expect(oRepo).Should(Equal(expectRepo)) + Expect(oChart).Should(Equal(expectChart)) + }) +}) diff --git a/pkg/util/helm/helm.go b/pkg/util/helm/helm.go index a3be23a63..d490d7b0e 100644 --- a/pkg/util/helm/helm.go +++ b/pkg/util/helm/helm.go @@ -43,7 +43,7 @@ func NewHelm(param *HelmParam, option ...Option) (*Helm, error) { PassCredentialsAll: false, } atomic := true - if !param.Chart.Wait { + if !*param.Chart.Wait { atomic = false } tmout, err := time.ParseDuration(param.Chart.Timeout) @@ -59,14 +59,14 @@ func NewHelm(param *HelmParam, option ...Option) (*Helm, error) { CreateNamespace: false, DisableHooks: false, Replace: true, - Wait: param.Chart.Wait, + Wait: *param.Chart.Wait, DependencyUpdate: false, Timeout: tmout, GenerateName: false, NameTemplate: "", Atomic: atomic, SkipCRDs: false, - UpgradeCRDs: param.Chart.UpgradeCRDs, + UpgradeCRDs: *param.Chart.UpgradeCRDs, SubNotes: false, Force: false, ResetValues: false, @@ -139,3 +139,13 @@ func GetAnnotationName() string { func GetLabelName() string { return "app.kubernetes.io/instance" } + +func GetBoolTrueAddress() *bool { + trueValue := true + return &trueValue +} + +func GetBoolFalseAddress() *bool { + falseValue := false + return &falseValue +} diff --git a/pkg/util/helm/helm_operation_test.go b/pkg/util/helm/helm_operation_test.go index eb3819bef..6c0e2e550 100644 --- a/pkg/util/helm/helm_operation_test.go +++ b/pkg/util/helm/helm_operation_test.go @@ -10,7 +10,7 @@ import ( func TestInstallOrUpgradeChart(t *testing.T) { atomic := true - if !helmParam.Chart.Wait { + if !*helmParam.Chart.Wait { atomic = false } tmout, err := time.ParseDuration(helmParam.Chart.Timeout) @@ -26,14 +26,14 @@ func TestInstallOrUpgradeChart(t *testing.T) { CreateNamespace: false, DisableHooks: false, Replace: true, - Wait: helmParam.Chart.Wait, + Wait: *helmParam.Chart.Wait, DependencyUpdate: false, Timeout: tmout, GenerateName: false, NameTemplate: "", Atomic: atomic, SkipCRDs: false, - UpgradeCRDs: helmParam.Chart.UpgradeCRDs, + UpgradeCRDs: *helmParam.Chart.UpgradeCRDs, SubNotes: false, Force: false, ResetValues: false, @@ -77,7 +77,7 @@ func TestAddOrUpdateChartRepo(t *testing.T) { PassCredentialsAll: false, } atomic := true - if !helmParam.Chart.Wait { + if !*helmParam.Chart.Wait { atomic = false } tmout, err := time.ParseDuration(helmParam.Chart.Timeout) @@ -93,14 +93,14 @@ func TestAddOrUpdateChartRepo(t *testing.T) { CreateNamespace: false, DisableHooks: false, Replace: true, - Wait: helmParam.Chart.Wait, + Wait: *helmParam.Chart.Wait, DependencyUpdate: false, Timeout: tmout, GenerateName: false, NameTemplate: "", Atomic: atomic, SkipCRDs: false, - UpgradeCRDs: helmParam.Chart.UpgradeCRDs, + UpgradeCRDs: *helmParam.Chart.UpgradeCRDs, SubNotes: false, Force: false, ResetValues: false, @@ -124,7 +124,7 @@ func TestAddOrUpdateChartRepo(t *testing.T) { func TestHelm_UninstallHelmChartRelease(t *testing.T) { atomic := true - if !helmParam.Chart.Wait { + if !*helmParam.Chart.Wait { atomic = false } tmout, err := time.ParseDuration(helmParam.Chart.Timeout) @@ -140,14 +140,14 @@ func TestHelm_UninstallHelmChartRelease(t *testing.T) { CreateNamespace: false, DisableHooks: false, Replace: true, - Wait: helmParam.Chart.Wait, + Wait: *helmParam.Chart.Wait, DependencyUpdate: false, Timeout: tmout, GenerateName: false, NameTemplate: "", Atomic: atomic, SkipCRDs: false, - UpgradeCRDs: helmParam.Chart.UpgradeCRDs, + UpgradeCRDs: *helmParam.Chart.UpgradeCRDs, SubNotes: false, Force: false, ResetValues: false, diff --git a/pkg/util/helm/helm_test.go b/pkg/util/helm/helm_test.go index 1d7076e22..b394d27d7 100644 --- a/pkg/util/helm/helm_test.go +++ b/pkg/util/helm/helm_test.go @@ -15,6 +15,7 @@ import ( var ( NormalError = errors.New("normal error") NotFoundError = errors.New("release name not found") + wait = true ) var mockedRelease = release.Release{Name: "test"} @@ -26,7 +27,8 @@ var helmParam = &HelmParam{ Chart{ ReleaseName: "helm:v1.0.0", Timeout: "1m", - Wait: true, + Wait: GetBoolFalseAddress(), + UpgradeCRDs: GetBoolFalseAddress(), }, } @@ -101,7 +103,7 @@ func TestNewHelmWithOption(t *testing.T) { PassCredentialsAll: false, } atomic := true - if !helmParam.Chart.Wait { + if !*helmParam.Chart.Wait { atomic = false } tmout, err := time.ParseDuration(helmParam.Chart.Timeout) @@ -117,14 +119,14 @@ func TestNewHelmWithOption(t *testing.T) { CreateNamespace: false, DisableHooks: false, Replace: true, - Wait: helmParam.Chart.Wait, + Wait: *helmParam.Chart.Wait, DependencyUpdate: false, Timeout: tmout, GenerateName: false, NameTemplate: "", Atomic: atomic, SkipCRDs: false, - UpgradeCRDs: helmParam.Chart.UpgradeCRDs, + UpgradeCRDs: *helmParam.Chart.UpgradeCRDs, SubNotes: false, Force: false, ResetValues: false, diff --git a/pkg/util/helm/param.go b/pkg/util/helm/param.go index da4b70b9a..5f9dcc44d 100644 --- a/pkg/util/helm/param.go +++ b/pkg/util/helm/param.go @@ -9,22 +9,54 @@ type HelmParam struct { // Repo is the struct containing details of a git repository. // TODO(daniel-hutao): make the Repo equals to repo.Entry type Repo struct { - Name string `validate:"required"` - URL string `validate:"required"` + Name string `validate:"required" mapstructure:"name"` + URL string `validate:"required" mapstructure:"url"` } // Chart is the struct containing details of a helm chart. // TODO(daniel-hutao): make the Chart equals to helmclient.ChartSpec type Chart struct { ChartName string `validate:"required" mapstructure:"chart_name"` - Version string + Version string `mapstructure:"version"` ReleaseName string `mapstructure:"release_name"` - Namespace string - CreateNamespace bool `mapstructure:"create_namespace"` - Wait bool - Timeout string // such as "1.5h" or "2h45m", valid time units are "s", "m", "h" - UpgradeCRDs bool `mapstructure:"upgradeCRDs"` + Namespace string `mapstructure:"name_space"` + CreateNamespace *bool `mapstructure:"create_namespace"` + Wait *bool `mapstructure:"wait"` + Timeout string `mapstructure:"timeout"` // such as "1.5h" or "2h45m", valid time units are "s", "m", "h" + UpgradeCRDs *bool `mapstructure:"upgradeCRDs"` // ValuesYaml is the values.yaml content. // use string instead of map[string]interface{} ValuesYaml string `mapstructure:"values_yaml"` } + +func (repo *Repo) FillDefaultValue(defaultRepo *Repo) { + if repo.Name == "" { + repo.Name = defaultRepo.Name + } + if repo.URL == "" { + repo.URL = defaultRepo.URL + } +} + +func (chart *Chart) FillDefaultValue(defaultChart *Chart) { + if chart.ChartName == "" { + chart.ChartName = defaultChart.ChartName + } + if chart.Timeout == "" { + chart.Timeout = defaultChart.Timeout + } + if chart.UpgradeCRDs == nil { + if defaultChart.UpgradeCRDs == nil { + chart.UpgradeCRDs = GetBoolFalseAddress() + } else { + chart.UpgradeCRDs = defaultChart.UpgradeCRDs + } + } + if chart.Wait == nil { + if defaultChart.Wait == nil { + chart.Wait = GetBoolFalseAddress() + } else { + chart.Wait = defaultChart.Wait + } + } +} diff --git a/pkg/util/helm/validation.go b/pkg/util/helm/validation.go index 77bdb535b..7da6356dc 100644 --- a/pkg/util/helm/validation.go +++ b/pkg/util/helm/validation.go @@ -2,19 +2,6 @@ package helm import "github.com/devstream-io/devstream/pkg/util/validator" -// defaults set the default value with HelmParam. -func defaults(param *HelmParam) { - if param.Chart.Timeout == "" { - // Make the timeout be same as the default value for `--timeout` with `helm install/upgrade/rollback` - param.Chart.Timeout = "5m0s" - } -} - -func validate(param *HelmParam) []error { +func Validate(param *HelmParam) []error { return validator.Struct(param) } - -func DefaultsAndValidate(param *HelmParam) []error { - defaults(param) - return validate(param) -} diff --git a/pkg/util/helm/validation_test.go b/pkg/util/helm/validation_test.go index 0a7bb8b5a..61299c405 100644 --- a/pkg/util/helm/validation_test.go +++ b/pkg/util/helm/validation_test.go @@ -1,50 +1,9 @@ package helm import ( - "reflect" "testing" ) -func Test_defaults(t *testing.T) { - // base - // timeout := 1h - tests := []struct { - name string - got HelmParam - want HelmParam - }{ - // TODO: Add test cases. - {"base", - HelmParam{Repo{"test", ""}, - Chart{ - Timeout: "", - }}, - HelmParam{ - Repo{"test", ""}, - Chart{ - Timeout: "5m0s", - }}}, - {"case timeout := 1h", - HelmParam{Repo{"test", ""}, - Chart{ - Timeout: "1h", - }}, - HelmParam{ - Repo{"test", ""}, - Chart{ - Timeout: "1h", - }}}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - defaults(&tt.got) - if !reflect.DeepEqual(tt.got, tt.want) { - t.Errorf("validate() = %v, want %v", tt.got, tt.want) - } - }) - } -} - func Test_validate(t *testing.T) { type args struct { param *HelmParam @@ -66,45 +25,10 @@ func Test_validate(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := validate(tt.args.param); len(got) != tt.want { + if got := Validate(tt.args.param); len(got) != tt.want { t.Logf("got errors' length: %d\n", len(got)) t.Errorf("validate() = %v, want %v", got, tt.want) } }) } } - -func TestDefaultsAndValidate(t *testing.T) { - type args struct { - param *HelmParam - } - type want struct { - HelmParam - errCount int - } - tests := []struct { - name string - args args - want want - }{ - // TODO: Add test cases. - {"base", args{&HelmParam{ - Repo{Name: "argo", URL: "https://argoproj.github.io/argo-helm"}, - Chart{ChartName: "argo/argo-cd"}, - }}, want{HelmParam{ - Repo{Name: "argo", URL: "https://argoproj.github.io/argo-helm"}, - Chart{ChartName: "argo/argo-cd", Timeout: "5m0s"}, - }, 0}}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := DefaultsAndValidate(tt.args.param) - if len(got) != tt.want.errCount { - t.Errorf("DefaultsAndValidate(): errorCount = %v, want %v", len(got), tt.want.errCount) - } - if !reflect.DeepEqual(*tt.args.param, tt.want.HelmParam) { - t.Errorf("DefaultsAndValidate(): HelmParam= %v, want %v", *tt.args.param, tt.want.HelmParam) - } - }) - } -} From 044c8f3e4dd8147f9db6c4d2989b06b26d428916 Mon Sep 17 00:00:00 2001 From: Meng JiaFeng Date: Wed, 3 Aug 2022 15:30:38 +0800 Subject: [PATCH 2/3] fix: repair name error Signed-off-by: Meng JiaFeng --- .../pkg/plugininstaller/helm/option_test.go | 2 +- .../pkg/plugininstaller/helm/validate_test.go | 20 +++++++------- pkg/util/helm/helm_test.go | 1 - pkg/util/helm/param.go | 27 ++++++++++--------- 4 files changed, 26 insertions(+), 24 deletions(-) diff --git a/internal/pkg/plugininstaller/helm/option_test.go b/internal/pkg/plugininstaller/helm/option_test.go index f43fb60d2..eed7d65bd 100644 --- a/internal/pkg/plugininstaller/helm/option_test.go +++ b/internal/pkg/plugininstaller/helm/option_test.go @@ -43,7 +43,7 @@ var _ = Describe("Options struct", func() { "release_name": "", "wait": emptyBool, "chart_name": "test_chart", - "name_space": "test_nameSpace", + "namespace": "test_nameSpace", "create_namespace": emptyBool, "timeout": "", "upgradeCRDs": emptyBool, diff --git a/internal/pkg/plugininstaller/helm/validate_test.go b/internal/pkg/plugininstaller/helm/validate_test.go index 7fde9e41e..3eddaedfe 100644 --- a/internal/pkg/plugininstaller/helm/validate_test.go +++ b/internal/pkg/plugininstaller/helm/validate_test.go @@ -50,8 +50,7 @@ var _ = Describe("SetDefaultConfig func", func() { testChartName string testRepoURL string testRepoName string - testWait *bool - emptyBool *bool + testBool *bool defaultConfig helm.Options testOptions plugininstaller.RawOptions expectChart map[string]interface{} @@ -61,16 +60,17 @@ var _ = Describe("SetDefaultConfig func", func() { testChartName = "test_chart" testRepoName = "test_repo" testRepoURL = "http://test.com" - testWait = helmCommon.GetBoolTrueAddress() + testBool = helmCommon.GetBoolTrueAddress() testOptions = map[string]interface{}{ "chart": map[string]string{}, "repo": map[string]string{}, } defaultConfig = helm.Options{ Chart: helmCommon.Chart{ - ChartName: testChartName, - Wait: testWait, - UpgradeCRDs: testWait, + ChartName: testChartName, + Wait: testBool, + UpgradeCRDs: testBool, + CreateNamespace: testBool, }, Repo: helmCommon.Repo{ URL: testRepoURL, @@ -79,14 +79,14 @@ var _ = Describe("SetDefaultConfig func", func() { } expectChart = map[string]interface{}{ "chart_name": testChartName, - "wait": testWait, - "name_space": "", + "wait": testBool, + "namespace": "", "version": "", "release_name": "", "values_yaml": "", "timeout": "", - "create_namespace": emptyBool, - "upgradeCRDs": testWait, + "create_namespace": testBool, + "upgradeCRDs": testBool, } expectRepo = map[string]interface{}{ "url": testRepoURL, diff --git a/pkg/util/helm/helm_test.go b/pkg/util/helm/helm_test.go index b394d27d7..b743bebdc 100644 --- a/pkg/util/helm/helm_test.go +++ b/pkg/util/helm/helm_test.go @@ -15,7 +15,6 @@ import ( var ( NormalError = errors.New("normal error") NotFoundError = errors.New("release name not found") - wait = true ) var mockedRelease = release.Release{Name: "test"} diff --git a/pkg/util/helm/param.go b/pkg/util/helm/param.go index 5f9dcc44d..2948faeac 100644 --- a/pkg/util/helm/param.go +++ b/pkg/util/helm/param.go @@ -19,7 +19,7 @@ type Chart struct { ChartName string `validate:"required" mapstructure:"chart_name"` Version string `mapstructure:"version"` ReleaseName string `mapstructure:"release_name"` - Namespace string `mapstructure:"name_space"` + Namespace string `mapstructure:"namespace"` CreateNamespace *bool `mapstructure:"create_namespace"` Wait *bool `mapstructure:"wait"` Timeout string `mapstructure:"timeout"` // such as "1.5h" or "2h45m", valid time units are "s", "m", "h" @@ -45,18 +45,21 @@ func (chart *Chart) FillDefaultValue(defaultChart *Chart) { if chart.Timeout == "" { chart.Timeout = defaultChart.Timeout } - if chart.UpgradeCRDs == nil { - if defaultChart.UpgradeCRDs == nil { - chart.UpgradeCRDs = GetBoolFalseAddress() - } else { - chart.UpgradeCRDs = defaultChart.UpgradeCRDs - } - } - if chart.Wait == nil { - if defaultChart.Wait == nil { - chart.Wait = GetBoolFalseAddress() + chart.UpgradeCRDs = getBoolValue(chart.UpgradeCRDs, defaultChart.UpgradeCRDs) + chart.Wait = getBoolValue(chart.Wait, defaultChart.Wait) + chart.CreateNamespace = getBoolValue(chart.CreateNamespace, defaultChart.CreateNamespace) +} + +func getBoolValue(field, defaultField *bool) *bool { + var boolAddress *bool + if field == nil { + if defaultField == nil { + boolAddress = GetBoolFalseAddress() } else { - chart.Wait = defaultChart.Wait + boolAddress = defaultField } + } else { + boolAddress = field } + return boolAddress } From d001f0f69c4d49f6395c78c5c42ba05a0284ba96 Mon Sep 17 00:00:00 2001 From: Meng JiaFeng Date: Wed, 3 Aug 2022 16:48:33 +0800 Subject: [PATCH 3/3] fix: ues bool type Signed-off-by: Meng JiaFeng --- internal/pkg/plugin/argocd/argocd.go | 5 +++-- .../pkg/plugininstaller/helm/validate_test.go | 3 ++- pkg/util/helm/helm.go | 10 --------- pkg/util/helm/helm_test.go | 6 +++-- pkg/util/helm/param.go | 22 ++++++++++--------- pkg/util/types/bool.go | 3 +++ 6 files changed, 24 insertions(+), 25 deletions(-) create mode 100644 pkg/util/types/bool.go diff --git a/internal/pkg/plugin/argocd/argocd.go b/internal/pkg/plugin/argocd/argocd.go index e5de17226..61104fca5 100644 --- a/internal/pkg/plugin/argocd/argocd.go +++ b/internal/pkg/plugin/argocd/argocd.go @@ -3,14 +3,15 @@ package argocd import ( "github.com/devstream-io/devstream/internal/pkg/plugininstaller/helm" helmCommon "github.com/devstream-io/devstream/pkg/util/helm" + "github.com/devstream-io/devstream/pkg/util/types" ) var defaultHelmConfig = helm.Options{ Chart: helmCommon.Chart{ ChartName: "argo/argo-cd", Timeout: "5m", - UpgradeCRDs: helmCommon.GetBoolTrueAddress(), - Wait: helmCommon.GetBoolTrueAddress(), + UpgradeCRDs: types.Bool(true), + Wait: types.Bool(true), }, Repo: helmCommon.Repo{ URL: "https://argoproj.github.io/argo-helm", diff --git a/internal/pkg/plugininstaller/helm/validate_test.go b/internal/pkg/plugininstaller/helm/validate_test.go index 3eddaedfe..0514544fa 100644 --- a/internal/pkg/plugininstaller/helm/validate_test.go +++ b/internal/pkg/plugininstaller/helm/validate_test.go @@ -7,6 +7,7 @@ import ( "github.com/devstream-io/devstream/internal/pkg/plugininstaller" "github.com/devstream-io/devstream/internal/pkg/plugininstaller/helm" helmCommon "github.com/devstream-io/devstream/pkg/util/helm" + "github.com/devstream-io/devstream/pkg/util/types" ) var _ = Describe("Validate func", func() { @@ -60,7 +61,7 @@ var _ = Describe("SetDefaultConfig func", func() { testChartName = "test_chart" testRepoName = "test_repo" testRepoURL = "http://test.com" - testBool = helmCommon.GetBoolTrueAddress() + testBool = types.Bool(true) testOptions = map[string]interface{}{ "chart": map[string]string{}, "repo": map[string]string{}, diff --git a/pkg/util/helm/helm.go b/pkg/util/helm/helm.go index d490d7b0e..a37ad8d04 100644 --- a/pkg/util/helm/helm.go +++ b/pkg/util/helm/helm.go @@ -139,13 +139,3 @@ func GetAnnotationName() string { func GetLabelName() string { return "app.kubernetes.io/instance" } - -func GetBoolTrueAddress() *bool { - trueValue := true - return &trueValue -} - -func GetBoolFalseAddress() *bool { - falseValue := false - return &falseValue -} diff --git a/pkg/util/helm/helm_test.go b/pkg/util/helm/helm_test.go index b743bebdc..e353b202f 100644 --- a/pkg/util/helm/helm_test.go +++ b/pkg/util/helm/helm_test.go @@ -10,6 +10,8 @@ import ( helmclient "github.com/mittwald/go-helm-client" "helm.sh/helm/v3/pkg/release" "helm.sh/helm/v3/pkg/repo" + + "github.com/devstream-io/devstream/pkg/util/types" ) var ( @@ -26,8 +28,8 @@ var helmParam = &HelmParam{ Chart{ ReleaseName: "helm:v1.0.0", Timeout: "1m", - Wait: GetBoolFalseAddress(), - UpgradeCRDs: GetBoolFalseAddress(), + Wait: types.Bool(false), + UpgradeCRDs: types.Bool(false), }, } diff --git a/pkg/util/helm/param.go b/pkg/util/helm/param.go index 2948faeac..e4b582814 100644 --- a/pkg/util/helm/param.go +++ b/pkg/util/helm/param.go @@ -1,5 +1,9 @@ package helm +import ( + "github.com/devstream-io/devstream/pkg/util/types" +) + // HelmParam is the struct for parameters with helm style. type HelmParam struct { Repo Repo @@ -51,15 +55,13 @@ func (chart *Chart) FillDefaultValue(defaultChart *Chart) { } func getBoolValue(field, defaultField *bool) *bool { - var boolAddress *bool - if field == nil { - if defaultField == nil { - boolAddress = GetBoolFalseAddress() - } else { - boolAddress = defaultField - } - } else { - boolAddress = field + if field != nil { + return field + } + + if defaultField != nil { + return defaultField } - return boolAddress + + return types.Bool(false) } diff --git a/pkg/util/types/bool.go b/pkg/util/types/bool.go new file mode 100644 index 000000000..027674b87 --- /dev/null +++ b/pkg/util/types/bool.go @@ -0,0 +1,3 @@ +package types + +func Bool(v bool) *bool { return &v }