From 16117eea2f199ee346ee3e19b8c79d50d9c9e1c5 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Tue, 14 Jan 2020 10:32:20 -0800 Subject: [PATCH 1/8] Put the Config in a new env package [#67] I'd like to be able to make calls like NewUpgrade(cfg) rather than Upgrade{...}.Prepare, but I wouldn't be able to define a NewUpgrade function while Config is in the helm package; there would be a circular import when Plan tried to import run. --- cmd/drone-helm/main.go | 3 +- internal/{helm => env}/config.go | 2 +- internal/{helm => env}/config_test.go | 2 +- internal/helm/plan.go | 21 +++++++------ internal/helm/plan_test.go | 45 ++++++++++++++------------- 5 files changed, 38 insertions(+), 35 deletions(-) rename internal/{helm => env}/config.go (99%) rename internal/{helm => env}/config_test.go (99%) diff --git a/cmd/drone-helm/main.go b/cmd/drone-helm/main.go index 1d6c2b4..7c6e3ff 100644 --- a/cmd/drone-helm/main.go +++ b/cmd/drone-helm/main.go @@ -5,11 +5,12 @@ import ( "os" _ "github.com/joho/godotenv/autoload" + "github.com/pelotech/drone-helm3/internal/env" "github.com/pelotech/drone-helm3/internal/helm" ) func main() { - cfg, err := helm.NewConfig(os.Stdout, os.Stderr) + cfg, err := env.NewConfig(os.Stdout, os.Stderr) if err != nil { fmt.Fprintf(os.Stderr, "%s\n", err.Error()) diff --git a/internal/helm/config.go b/internal/env/config.go similarity index 99% rename from internal/helm/config.go rename to internal/env/config.go index b633439..aa853c0 100644 --- a/internal/helm/config.go +++ b/internal/env/config.go @@ -1,4 +1,4 @@ -package helm +package env import ( "fmt" diff --git a/internal/helm/config_test.go b/internal/env/config_test.go similarity index 99% rename from internal/helm/config_test.go rename to internal/env/config_test.go index 13bf22a..40123dd 100644 --- a/internal/helm/config_test.go +++ b/internal/env/config_test.go @@ -1,4 +1,4 @@ -package helm +package env import ( "fmt" diff --git a/internal/helm/plan.go b/internal/helm/plan.go index 48f3f5f..7797f20 100644 --- a/internal/helm/plan.go +++ b/internal/helm/plan.go @@ -2,6 +2,7 @@ package helm import ( "fmt" + "github.com/pelotech/drone-helm3/internal/env" "github.com/pelotech/drone-helm3/internal/run" "os" ) @@ -20,12 +21,12 @@ type Step interface { // A Plan is a series of steps to perform. type Plan struct { steps []Step - cfg Config + cfg env.Config runCfg run.Config } // NewPlan makes a plan for running a helm operation. -func NewPlan(cfg Config) (*Plan, error) { +func NewPlan(cfg env.Config) (*Plan, error) { p := Plan{ cfg: cfg, runCfg: run.Config{ @@ -54,7 +55,7 @@ func NewPlan(cfg Config) (*Plan, error) { // determineSteps is primarily for the tests' convenience: it allows testing the "which stuff should // we do" logic without building a config that meets all the steps' requirements. -func determineSteps(cfg Config) *func(Config) []Step { +func determineSteps(cfg env.Config) *func(env.Config) []Step { switch cfg.Command { case "upgrade": return &upgrade @@ -91,7 +92,7 @@ func (p *Plan) Execute() error { return nil } -var upgrade = func(cfg Config) []Step { +var upgrade = func(cfg env.Config) []Step { steps := initKube(cfg) steps = append(steps, addRepos(cfg)...) if cfg.UpdateDependencies { @@ -116,7 +117,7 @@ var upgrade = func(cfg Config) []Step { return steps } -var uninstall = func(cfg Config) []Step { +var uninstall = func(cfg env.Config) []Step { steps := initKube(cfg) if cfg.UpdateDependencies { steps = append(steps, depUpdate(cfg)...) @@ -130,7 +131,7 @@ var uninstall = func(cfg Config) []Step { return steps } -var lint = func(cfg Config) []Step { +var lint = func(cfg env.Config) []Step { steps := addRepos(cfg) if cfg.UpdateDependencies { steps = append(steps, depUpdate(cfg)...) @@ -146,14 +147,14 @@ var lint = func(cfg Config) []Step { return steps } -var help = func(cfg Config) []Step { +var help = func(cfg env.Config) []Step { help := &run.Help{ HelmCommand: cfg.Command, } return []Step{help} } -func initKube(cfg Config) []Step { +func initKube(cfg env.Config) []Step { return []Step{ &run.InitKube{ SkipTLSVerify: cfg.SkipTLSVerify, @@ -167,7 +168,7 @@ func initKube(cfg Config) []Step { } } -func addRepos(cfg Config) []Step { +func addRepos(cfg env.Config) []Step { steps := make([]Step, 0) for _, repo := range cfg.AddRepos { steps = append(steps, &run.AddRepo{ @@ -178,7 +179,7 @@ func addRepos(cfg Config) []Step { return steps } -func depUpdate(cfg Config) []Step { +func depUpdate(cfg env.Config) []Step { return []Step{ &run.DepUpdate{ Chart: cfg.Chart, diff --git a/internal/helm/plan_test.go b/internal/helm/plan_test.go index 1bc3e11..24f84a1 100644 --- a/internal/helm/plan_test.go +++ b/internal/helm/plan_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "github.com/pelotech/drone-helm3/internal/env" "github.com/pelotech/drone-helm3/internal/run" ) @@ -25,14 +26,14 @@ func (suite *PlanTestSuite) TestNewPlan() { stepTwo := NewMockStep(ctrl) origHelp := help - help = func(cfg Config) []Step { + help = func(cfg env.Config) []Step { return []Step{stepOne, stepTwo} } defer func() { help = origHelp }() stdout := strings.Builder{} stderr := strings.Builder{} - cfg := Config{ + cfg := env.Config{ Command: "help", Debug: false, Namespace: "outer", @@ -65,12 +66,12 @@ func (suite *PlanTestSuite) TestNewPlanAbortsOnError() { stepTwo := NewMockStep(ctrl) origHelp := help - help = func(cfg Config) []Step { + help = func(cfg env.Config) []Step { return []Step{stepOne, stepTwo} } defer func() { help = origHelp }() - cfg := Config{ + cfg := env.Config{ Command: "help", } @@ -129,7 +130,7 @@ func (suite *PlanTestSuite) TestExecuteAbortsOnError() { } func (suite *PlanTestSuite) TestUpgrade() { - cfg := Config{ + cfg := env.Config{ ChartVersion: "seventeen", DryRun: true, Wait: true, @@ -172,7 +173,7 @@ func (suite *PlanTestSuite) TestUpgrade() { } func (suite *PlanTestSuite) TestUpgradeWithUpdateDependencies() { - cfg := Config{ + cfg := env.Config{ UpdateDependencies: true, } steps := upgrade(cfg) @@ -182,7 +183,7 @@ func (suite *PlanTestSuite) TestUpgradeWithUpdateDependencies() { } func (suite *PlanTestSuite) TestUpgradeWithAddRepos() { - cfg := Config{ + cfg := env.Config{ AddRepos: []string{ "machine=https://github.com/harold_finch/themachine", }, @@ -193,7 +194,7 @@ func (suite *PlanTestSuite) TestUpgradeWithAddRepos() { } func (suite *PlanTestSuite) TestUninstall() { - cfg := Config{ + cfg := env.Config{ KubeToken: "b2YgbXkgYWZmZWN0aW9u", SkipTLSVerify: true, Certificate: "cHJvY2xhaW1zIHdvbmRlcmZ1bCBmcmllbmRzaGlw", @@ -233,7 +234,7 @@ func (suite *PlanTestSuite) TestUninstall() { } func (suite *PlanTestSuite) TestUninstallWithUpdateDependencies() { - cfg := Config{ + cfg := env.Config{ UpdateDependencies: true, } steps := uninstall(cfg) @@ -243,7 +244,7 @@ func (suite *PlanTestSuite) TestUninstallWithUpdateDependencies() { } func (suite *PlanTestSuite) TestInitKube() { - cfg := Config{ + cfg := env.Config{ KubeToken: "cXVlZXIgY2hhcmFjdGVyCg==", SkipTLSVerify: true, Certificate: "b2Ygd29rZW5lc3MK", @@ -269,7 +270,7 @@ func (suite *PlanTestSuite) TestInitKube() { } func (suite *PlanTestSuite) TestDepUpdate() { - cfg := Config{ + cfg := env.Config{ UpdateDependencies: true, Chart: "scatterplot", } @@ -286,7 +287,7 @@ func (suite *PlanTestSuite) TestDepUpdate() { } func (suite *PlanTestSuite) TestAddRepos() { - cfg := Config{ + cfg := env.Config{ AddRepos: []string{ "first=https://add.repos/one", "second=https://add.repos/two", @@ -304,7 +305,7 @@ func (suite *PlanTestSuite) TestAddRepos() { } func (suite *PlanTestSuite) TestLint() { - cfg := Config{ + cfg := env.Config{ Chart: "./flow", Values: "steadfastness,forthrightness", StringValues: "tensile_strength,flexibility", @@ -326,7 +327,7 @@ func (suite *PlanTestSuite) TestLint() { } func (suite *PlanTestSuite) TestLintWithUpdateDependencies() { - cfg := Config{ + cfg := env.Config{ UpdateDependencies: true, } steps := lint(cfg) @@ -335,7 +336,7 @@ func (suite *PlanTestSuite) TestLintWithUpdateDependencies() { } func (suite *PlanTestSuite) TestLintWithAddRepos() { - cfg := Config{ + cfg := env.Config{ AddRepos: []string{"friendczar=https://github.com/logan_pierce/friendczar"}, } steps := lint(cfg) @@ -344,7 +345,7 @@ func (suite *PlanTestSuite) TestLintWithAddRepos() { } func (suite *PlanTestSuite) TestDeterminePlanUpgradeCommand() { - cfg := Config{ + cfg := env.Config{ Command: "upgrade", } stepsMaker := determineSteps(cfg) @@ -352,7 +353,7 @@ func (suite *PlanTestSuite) TestDeterminePlanUpgradeCommand() { } func (suite *PlanTestSuite) TestDeterminePlanUpgradeFromDroneEvent() { - cfg := Config{} + cfg := env.Config{} upgradeEvents := []string{"push", "tag", "deployment", "pull_request", "promote", "rollback"} for _, event := range upgradeEvents { @@ -363,7 +364,7 @@ func (suite *PlanTestSuite) TestDeterminePlanUpgradeFromDroneEvent() { } func (suite *PlanTestSuite) TestDeterminePlanUninstallCommand() { - cfg := Config{ + cfg := env.Config{ Command: "uninstall", } stepsMaker := determineSteps(cfg) @@ -372,7 +373,7 @@ func (suite *PlanTestSuite) TestDeterminePlanUninstallCommand() { // helm_command = delete is provided as an alias for backward-compatibility with drone-helm func (suite *PlanTestSuite) TestDeterminePlanDeleteCommand() { - cfg := Config{ + cfg := env.Config{ Command: "delete", } stepsMaker := determineSteps(cfg) @@ -380,7 +381,7 @@ func (suite *PlanTestSuite) TestDeterminePlanDeleteCommand() { } func (suite *PlanTestSuite) TestDeterminePlanDeleteFromDroneEvent() { - cfg := Config{ + cfg := env.Config{ DroneEvent: "delete", } stepsMaker := determineSteps(cfg) @@ -388,7 +389,7 @@ func (suite *PlanTestSuite) TestDeterminePlanDeleteFromDroneEvent() { } func (suite *PlanTestSuite) TestDeterminePlanLintCommand() { - cfg := Config{ + cfg := env.Config{ Command: "lint", } @@ -397,7 +398,7 @@ func (suite *PlanTestSuite) TestDeterminePlanLintCommand() { } func (suite *PlanTestSuite) TestDeterminePlanHelpCommand() { - cfg := Config{ + cfg := env.Config{ Command: "help", } From 588c7cb9f76467d415b484c9fc9448ce6fb8ce22 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Thu, 16 Jan 2020 13:50:04 -0800 Subject: [PATCH 2/8] Initialize Steps with a NewSTEPNAME function [#67] This seems to be be a more natural separation of concerns--the knowledge of which config fields map to which parts of a Step belong to the Step, not to the Plan. --- internal/helm/plan.go | 58 ++------------ internal/helm/plan_test.go | 142 ++++----------------------------- internal/run/addrepo.go | 7 ++ internal/run/addrepo_test.go | 6 ++ internal/run/depupdate.go | 8 ++ internal/run/depupdate_test.go | 9 +++ internal/run/help.go | 8 ++ internal/run/help_test.go | 10 +++ internal/run/initkube.go | 14 ++++ internal/run/initkube_test.go | 22 +++++ internal/run/lint.go | 12 +++ internal/run/lint_test.go | 20 +++++ internal/run/uninstall.go | 10 +++ internal/run/uninstall_test.go | 15 ++++ internal/run/upgrade.go | 20 +++++ internal/run/upgrade_test.go | 36 +++++++++ 16 files changed, 218 insertions(+), 179 deletions(-) diff --git a/internal/helm/plan.go b/internal/helm/plan.go index 7797f20..391d220 100644 --- a/internal/helm/plan.go +++ b/internal/helm/plan.go @@ -98,21 +98,7 @@ var upgrade = func(cfg env.Config) []Step { if cfg.UpdateDependencies { steps = append(steps, depUpdate(cfg)...) } - steps = append(steps, &run.Upgrade{ - Chart: cfg.Chart, - Release: cfg.Release, - ChartVersion: cfg.ChartVersion, - DryRun: cfg.DryRun, - Wait: cfg.Wait, - Values: cfg.Values, - StringValues: cfg.StringValues, - ValuesFiles: cfg.ValuesFiles, - ReuseValues: cfg.ReuseValues, - Timeout: cfg.Timeout, - Force: cfg.Force, - Atomic: cfg.AtomicUpgrade, - CleanupOnFail: cfg.CleanupOnFail, - }) + steps = append(steps, run.NewUpgrade(cfg)) return steps } @@ -122,11 +108,7 @@ var uninstall = func(cfg env.Config) []Step { if cfg.UpdateDependencies { steps = append(steps, depUpdate(cfg)...) } - steps = append(steps, &run.Uninstall{ - Release: cfg.Release, - DryRun: cfg.DryRun, - KeepHistory: cfg.KeepHistory, - }) + steps = append(steps, run.NewUninstall(cfg)) return steps } @@ -136,53 +118,27 @@ var lint = func(cfg env.Config) []Step { if cfg.UpdateDependencies { steps = append(steps, depUpdate(cfg)...) } - steps = append(steps, &run.Lint{ - Chart: cfg.Chart, - Values: cfg.Values, - StringValues: cfg.StringValues, - ValuesFiles: cfg.ValuesFiles, - Strict: cfg.LintStrictly, - }) - + steps = append(steps, run.NewLint(cfg)) return steps } var help = func(cfg env.Config) []Step { - help := &run.Help{ - HelmCommand: cfg.Command, - } - return []Step{help} + return []Step{run.NewHelp(cfg)} } func initKube(cfg env.Config) []Step { - return []Step{ - &run.InitKube{ - SkipTLSVerify: cfg.SkipTLSVerify, - Certificate: cfg.Certificate, - APIServer: cfg.APIServer, - ServiceAccount: cfg.ServiceAccount, - Token: cfg.KubeToken, - TemplateFile: kubeConfigTemplate, - ConfigFile: kubeConfigFile, - }, - } + return []Step{run.NewInitKube(cfg, kubeConfigTemplate, kubeConfigFile)} } func addRepos(cfg env.Config) []Step { steps := make([]Step, 0) for _, repo := range cfg.AddRepos { - steps = append(steps, &run.AddRepo{ - Repo: repo, - }) + steps = append(steps, run.NewAddRepo(repo)) } return steps } func depUpdate(cfg env.Config) []Step { - return []Step{ - &run.DepUpdate{ - Chart: cfg.Chart, - }, - } + return []Step{run.NewDepUpdate(cfg)} } diff --git a/internal/helm/plan_test.go b/internal/helm/plan_test.go index 24f84a1..f922010 100644 --- a/internal/helm/plan_test.go +++ b/internal/helm/plan_test.go @@ -130,46 +130,10 @@ func (suite *PlanTestSuite) TestExecuteAbortsOnError() { } func (suite *PlanTestSuite) TestUpgrade() { - cfg := env.Config{ - ChartVersion: "seventeen", - DryRun: true, - Wait: true, - Values: "steadfastness,forthrightness", - StringValues: "tensile_strength,flexibility", - ValuesFiles: []string{"/root/price_inventory.yml"}, - ReuseValues: true, - Timeout: "go sit in the corner", - Chart: "billboard_top_100", - Release: "post_malone_circles", - Force: true, - AtomicUpgrade: true, - CleanupOnFail: true, - } - - steps := upgrade(cfg) + steps := upgrade(env.Config{}) suite.Require().Equal(2, len(steps), "upgrade should return 2 steps") - suite.Require().IsType(&run.InitKube{}, steps[0]) - - suite.Require().IsType(&run.Upgrade{}, steps[1]) - upgrade, _ := steps[1].(*run.Upgrade) - - expected := &run.Upgrade{ - Chart: cfg.Chart, - Release: cfg.Release, - ChartVersion: cfg.ChartVersion, - DryRun: true, - Wait: cfg.Wait, - Values: "steadfastness,forthrightness", - StringValues: "tensile_strength,flexibility", - ValuesFiles: []string{"/root/price_inventory.yml"}, - ReuseValues: cfg.ReuseValues, - Timeout: cfg.Timeout, - Force: cfg.Force, - Atomic: true, - CleanupOnFail: true, - } - - suite.Equal(expected, upgrade) + suite.IsType(&run.InitKube{}, steps[0]) + suite.IsType(&run.Upgrade{}, steps[1]) } func (suite *PlanTestSuite) TestUpgradeWithUpdateDependencies() { @@ -194,43 +158,11 @@ func (suite *PlanTestSuite) TestUpgradeWithAddRepos() { } func (suite *PlanTestSuite) TestUninstall() { - cfg := env.Config{ - KubeToken: "b2YgbXkgYWZmZWN0aW9u", - SkipTLSVerify: true, - Certificate: "cHJvY2xhaW1zIHdvbmRlcmZ1bCBmcmllbmRzaGlw", - APIServer: "98.765.43.21", - ServiceAccount: "greathelm", - DryRun: true, - Timeout: "think about what you did", - Release: "jetta_id_love_to_change_the_world", - KeepHistory: true, - } - - steps := uninstall(cfg) + steps := uninstall(env.Config{}) suite.Require().Equal(2, len(steps), "uninstall should return 2 steps") - suite.Require().IsType(&run.InitKube{}, steps[0]) - init, _ := steps[0].(*run.InitKube) - var expected Step = &run.InitKube{ - SkipTLSVerify: true, - Certificate: "cHJvY2xhaW1zIHdvbmRlcmZ1bCBmcmllbmRzaGlw", - APIServer: "98.765.43.21", - ServiceAccount: "greathelm", - Token: "b2YgbXkgYWZmZWN0aW9u", - TemplateFile: kubeConfigTemplate, - ConfigFile: kubeConfigFile, - } - - suite.Equal(expected, init) - - suite.Require().IsType(&run.Uninstall{}, steps[1]) - actual, _ := steps[1].(*run.Uninstall) - expected = &run.Uninstall{ - Release: "jetta_id_love_to_change_the_world", - DryRun: true, - KeepHistory: true, - } - suite.Equal(expected, actual) + suite.IsType(&run.InitKube{}, steps[0]) + suite.IsType(&run.Uninstall{}, steps[1]) } func (suite *PlanTestSuite) TestUninstallWithUpdateDependencies() { @@ -244,46 +176,21 @@ func (suite *PlanTestSuite) TestUninstallWithUpdateDependencies() { } func (suite *PlanTestSuite) TestInitKube() { - cfg := env.Config{ - KubeToken: "cXVlZXIgY2hhcmFjdGVyCg==", - SkipTLSVerify: true, - Certificate: "b2Ygd29rZW5lc3MK", - APIServer: "123.456.78.9", - ServiceAccount: "helmet", - } + cfg := env.Config{} steps := initKube(cfg) suite.Require().Equal(1, len(steps), "initKube should return one step") - suite.Require().IsType(&run.InitKube{}, steps[0]) - init, _ := steps[0].(*run.InitKube) - - expected := &run.InitKube{ - SkipTLSVerify: true, - Certificate: "b2Ygd29rZW5lc3MK", - APIServer: "123.456.78.9", - ServiceAccount: "helmet", - Token: "cXVlZXIgY2hhcmFjdGVyCg==", - TemplateFile: kubeConfigTemplate, - ConfigFile: kubeConfigFile, - } - suite.Equal(expected, init) + suite.IsType(&run.InitKube{}, steps[0]) } func (suite *PlanTestSuite) TestDepUpdate() { cfg := env.Config{ UpdateDependencies: true, - Chart: "scatterplot", } steps := depUpdate(cfg) suite.Require().Equal(1, len(steps), "depUpdate should return one step") - suite.Require().IsType(&run.DepUpdate{}, steps[0]) - update, _ := steps[0].(*run.DepUpdate) - - expected := &run.DepUpdate{ - Chart: "scatterplot", - } - suite.Equal(expected, update) + suite.IsType(&run.DepUpdate{}, steps[0]) } func (suite *PlanTestSuite) TestAddRepos() { @@ -295,35 +202,14 @@ func (suite *PlanTestSuite) TestAddRepos() { } steps := addRepos(cfg) suite.Require().Equal(2, len(steps), "addRepos should add one step per repo") - suite.Require().IsType(&run.AddRepo{}, steps[0]) - suite.Require().IsType(&run.AddRepo{}, steps[1]) - first := steps[0].(*run.AddRepo) - second := steps[1].(*run.AddRepo) - - suite.Equal(first.Repo, "first=https://add.repos/one") - suite.Equal(second.Repo, "second=https://add.repos/two") + suite.IsType(&run.AddRepo{}, steps[0]) + suite.IsType(&run.AddRepo{}, steps[1]) } func (suite *PlanTestSuite) TestLint() { - cfg := env.Config{ - Chart: "./flow", - Values: "steadfastness,forthrightness", - StringValues: "tensile_strength,flexibility", - ValuesFiles: []string{"/root/price_inventory.yml"}, - LintStrictly: true, - } - - steps := lint(cfg) - suite.Equal(1, len(steps)) - - want := &run.Lint{ - Chart: "./flow", - Values: "steadfastness,forthrightness", - StringValues: "tensile_strength,flexibility", - ValuesFiles: []string{"/root/price_inventory.yml"}, - Strict: true, - } - suite.Equal(want, steps[0]) + steps := lint(env.Config{}) + suite.Require().Equal(1, len(steps)) + suite.IsType(&run.Lint{}, steps[0]) } func (suite *PlanTestSuite) TestLintWithUpdateDependencies() { diff --git a/internal/run/addrepo.go b/internal/run/addrepo.go index 3382957..8cc7552 100644 --- a/internal/run/addrepo.go +++ b/internal/run/addrepo.go @@ -11,6 +11,13 @@ type AddRepo struct { cmd cmd } +// NewAddRepo creates an AddRepo for the given repo-spec. No validation is performed at this time. +func NewAddRepo(repo string) *AddRepo { + return &AddRepo{ + Repo: repo, + } +} + // Execute executes the `helm repo add` command. func (a *AddRepo) Execute(_ Config) error { return a.cmd.Run() diff --git a/internal/run/addrepo_test.go b/internal/run/addrepo_test.go index ad42d06..51c31a0 100644 --- a/internal/run/addrepo_test.go +++ b/internal/run/addrepo_test.go @@ -38,6 +38,12 @@ func TestAddRepoTestSuite(t *testing.T) { suite.Run(t, new(AddRepoTestSuite)) } +func (suite *AddRepoTestSuite) TestNewAddRepo() { + repo := NewAddRepo("picompress=https://github.com/caleb_phipps/picompress") + suite.Require().NotNil(repo) + suite.Equal("picompress=https://github.com/caleb_phipps/picompress", repo.Repo) +} + func (suite *AddRepoTestSuite) TestPrepareAndExecute() { stdout := strings.Builder{} stderr := strings.Builder{} diff --git a/internal/run/depupdate.go b/internal/run/depupdate.go index a9b6c91..c34c0ee 100644 --- a/internal/run/depupdate.go +++ b/internal/run/depupdate.go @@ -2,6 +2,7 @@ package run import ( "fmt" + "github.com/pelotech/drone-helm3/internal/env" ) // DepUpdate is an execution step that calls `helm dependency update` when executed. @@ -10,6 +11,13 @@ type DepUpdate struct { cmd cmd } +// NewDepUpdate creates a DepUpdate using fields from the given Config. No validation is performed at this time. +func NewDepUpdate(cfg env.Config) *DepUpdate { + return &DepUpdate{ + Chart: cfg.Chart, + } +} + // Execute executes the `helm upgrade` command. func (d *DepUpdate) Execute(_ Config) error { return d.cmd.Run() diff --git a/internal/run/depupdate_test.go b/internal/run/depupdate_test.go index 315b351..94b0198 100644 --- a/internal/run/depupdate_test.go +++ b/internal/run/depupdate_test.go @@ -3,6 +3,7 @@ package run import ( "fmt" "github.com/golang/mock/gomock" + "github.com/pelotech/drone-helm3/internal/env" "github.com/stretchr/testify/suite" "strings" "testing" @@ -31,6 +32,14 @@ func TestDepUpdateTestSuite(t *testing.T) { suite.Run(t, new(DepUpdateTestSuite)) } +func (suite *DepUpdateTestSuite) TestNewDepUpdate() { + cfg := env.Config{ + Chart: "scatterplot", + } + d := NewDepUpdate(cfg) + suite.Equal("scatterplot", d.Chart) +} + func (suite *DepUpdateTestSuite) TestPrepareAndExecute() { defer suite.ctrl.Finish() diff --git a/internal/run/help.go b/internal/run/help.go index f2d6c59..40e0f86 100644 --- a/internal/run/help.go +++ b/internal/run/help.go @@ -2,6 +2,7 @@ package run import ( "fmt" + "github.com/pelotech/drone-helm3/internal/env" ) // Help is a step in a helm Plan that calls `helm help`. @@ -10,6 +11,13 @@ type Help struct { cmd cmd } +// NewHelp creates a Help using fields from the given Config. No validation is performed at this time. +func NewHelp(cfg env.Config) *Help { + return &Help{ + HelmCommand: cfg.Command, + } +} + // Execute executes the `helm help` command. func (h *Help) Execute(cfg Config) error { if err := h.cmd.Run(); err != nil { diff --git a/internal/run/help_test.go b/internal/run/help_test.go index 19c49d2..0cf5927 100644 --- a/internal/run/help_test.go +++ b/internal/run/help_test.go @@ -3,6 +3,7 @@ package run import ( "fmt" "github.com/golang/mock/gomock" + "github.com/pelotech/drone-helm3/internal/env" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" "strings" @@ -17,6 +18,15 @@ func TestHelpTestSuite(t *testing.T) { suite.Run(t, new(HelpTestSuite)) } +func (suite *HelpTestSuite) TestNewHelp() { + cfg := env.Config{ + Command: "everybody dance NOW!!", + } + help := NewHelp(cfg) + suite.Require().NotNil(help) + suite.Equal("everybody dance NOW!!", help.HelmCommand) +} + func (suite *HelpTestSuite) TestPrepare() { ctrl := gomock.NewController(suite.T()) defer ctrl.Finish() diff --git a/internal/run/initkube.go b/internal/run/initkube.go index fc0fb11..f15ba75 100644 --- a/internal/run/initkube.go +++ b/internal/run/initkube.go @@ -3,6 +3,7 @@ package run import ( "errors" "fmt" + "github.com/pelotech/drone-helm3/internal/env" "io" "os" "text/template" @@ -32,6 +33,19 @@ type kubeValues struct { Token string } +// NewInitKube creates a InitKube using the given Config and filepaths. No validation is performed at this time. +func NewInitKube(cfg env.Config, templateFile, configFile string) *InitKube { + return &InitKube{ + SkipTLSVerify: cfg.SkipTLSVerify, + Certificate: cfg.Certificate, + APIServer: cfg.APIServer, + ServiceAccount: cfg.ServiceAccount, + Token: cfg.KubeToken, + TemplateFile: templateFile, + ConfigFile: configFile, + } +} + // Execute generates a kubernetes config file from drone-helm3's template. func (i *InitKube) Execute(cfg Config) error { if cfg.Debug { diff --git a/internal/run/initkube_test.go b/internal/run/initkube_test.go index 72452a8..df6d531 100644 --- a/internal/run/initkube_test.go +++ b/internal/run/initkube_test.go @@ -1,6 +1,7 @@ package run import ( + "github.com/pelotech/drone-helm3/internal/env" "github.com/stretchr/testify/suite" yaml "gopkg.in/yaml.v2" "io/ioutil" @@ -17,6 +18,27 @@ func TestInitKubeTestSuite(t *testing.T) { suite.Run(t, new(InitKubeTestSuite)) } +func (suite *InitKubeTestSuite) TestNewInitKube() { + cfg := env.Config{ + SkipTLSVerify: true, + Certificate: "cHJvY2xhaW1zIHdvbmRlcmZ1bCBmcmllbmRzaGlw", + APIServer: "98.765.43.21", + ServiceAccount: "greathelm", + KubeToken: "b2YgbXkgYWZmZWN0aW9u", + } + + init := NewInitKube(cfg, "conf.tpl", "conf.yml") + suite.Equal(&InitKube{ + SkipTLSVerify: true, + Certificate: "cHJvY2xhaW1zIHdvbmRlcmZ1bCBmcmllbmRzaGlw", + APIServer: "98.765.43.21", + ServiceAccount: "greathelm", + Token: "b2YgbXkgYWZmZWN0aW9u", + TemplateFile: "conf.tpl", + ConfigFile: "conf.yml", + }, init) +} + func (suite *InitKubeTestSuite) TestPrepareExecute() { templateFile, err := tempfile("kubeconfig********.yml.tpl", ` certificate: {{ .Certificate }} diff --git a/internal/run/lint.go b/internal/run/lint.go index db4e13b..03912fd 100644 --- a/internal/run/lint.go +++ b/internal/run/lint.go @@ -2,6 +2,7 @@ package run import ( "fmt" + "github.com/pelotech/drone-helm3/internal/env" ) // Lint is an execution step that calls `helm lint` when executed. @@ -14,6 +15,17 @@ type Lint struct { cmd cmd } +// NewLint creates a Lint using fields from the given Config. No validation is performed at this time. +func NewLint(cfg env.Config) *Lint { + return &Lint{ + Chart: cfg.Chart, + Values: cfg.Values, + StringValues: cfg.StringValues, + ValuesFiles: cfg.ValuesFiles, + Strict: cfg.LintStrictly, + } +} + // Execute executes the `helm lint` command. func (l *Lint) Execute(_ Config) error { return l.cmd.Run() diff --git a/internal/run/lint_test.go b/internal/run/lint_test.go index f46ad63..3323ccb 100644 --- a/internal/run/lint_test.go +++ b/internal/run/lint_test.go @@ -3,6 +3,7 @@ package run import ( "fmt" "github.com/golang/mock/gomock" + "github.com/pelotech/drone-helm3/internal/env" "github.com/stretchr/testify/suite" "strings" "testing" @@ -31,6 +32,25 @@ func TestLintTestSuite(t *testing.T) { suite.Run(t, new(LintTestSuite)) } +func (suite *LintTestSuite) TestNewLint() { + cfg := env.Config{ + Chart: "./flow", + Values: "steadfastness,forthrightness", + StringValues: "tensile_strength,flexibility", + ValuesFiles: []string{"/root/price_inventory.yml"}, + LintStrictly: true, + } + lint := NewLint(cfg) + suite.Require().NotNil(lint) + suite.Equal(&Lint{ + Chart: "./flow", + Values: "steadfastness,forthrightness", + StringValues: "tensile_strength,flexibility", + ValuesFiles: []string{"/root/price_inventory.yml"}, + Strict: true, + }, lint) +} + func (suite *LintTestSuite) TestPrepareAndExecute() { defer suite.ctrl.Finish() diff --git a/internal/run/uninstall.go b/internal/run/uninstall.go index 5c5c654..790e9c6 100644 --- a/internal/run/uninstall.go +++ b/internal/run/uninstall.go @@ -2,6 +2,7 @@ package run import ( "fmt" + "github.com/pelotech/drone-helm3/internal/env" ) // Uninstall is an execution step that calls `helm uninstall` when executed. @@ -12,6 +13,15 @@ type Uninstall struct { cmd cmd } +// NewUninstall creates an Uninstall using fields from the given Config. No validation is performed at this time. +func NewUninstall(cfg env.Config) *Uninstall { + return &Uninstall{ + Release: cfg.Release, + DryRun: cfg.DryRun, + KeepHistory: cfg.KeepHistory, + } +} + // Execute executes the `helm uninstall` command. func (u *Uninstall) Execute(_ Config) error { return u.cmd.Run() diff --git a/internal/run/uninstall_test.go b/internal/run/uninstall_test.go index 6ac7cc9..0dafe33 100644 --- a/internal/run/uninstall_test.go +++ b/internal/run/uninstall_test.go @@ -3,6 +3,7 @@ package run import ( "fmt" "github.com/golang/mock/gomock" + "github.com/pelotech/drone-helm3/internal/env" "github.com/stretchr/testify/suite" "strings" "testing" @@ -35,6 +36,20 @@ func TestUninstallTestSuite(t *testing.T) { suite.Run(t, new(UninstallTestSuite)) } +func (suite *UninstallTestSuite) TestNewUninstall() { + cfg := env.Config{ + DryRun: true, + Release: "jetta_id_love_to_change_the_world", + KeepHistory: true, + } + u := NewUninstall(cfg) + suite.Equal(&Uninstall{ + Release: "jetta_id_love_to_change_the_world", + DryRun: true, + KeepHistory: true, + }, u) +} + func (suite *UninstallTestSuite) TestPrepareAndExecute() { defer suite.ctrl.Finish() diff --git a/internal/run/upgrade.go b/internal/run/upgrade.go index c239807..b85b431 100644 --- a/internal/run/upgrade.go +++ b/internal/run/upgrade.go @@ -2,6 +2,7 @@ package run import ( "fmt" + "github.com/pelotech/drone-helm3/internal/env" ) // Upgrade is an execution step that calls `helm upgrade` when executed. @@ -24,6 +25,25 @@ type Upgrade struct { cmd cmd } +// NewUpgrade creates an Upgrade using fields from the given Config. No validation is performed at this time. +func NewUpgrade(cfg env.Config) *Upgrade { + return &Upgrade{ + Chart: cfg.Chart, + Release: cfg.Release, + ChartVersion: cfg.ChartVersion, + DryRun: cfg.DryRun, + Wait: cfg.Wait, + Values: cfg.Values, + StringValues: cfg.StringValues, + ValuesFiles: cfg.ValuesFiles, + ReuseValues: cfg.ReuseValues, + Timeout: cfg.Timeout, + Force: cfg.Force, + Atomic: cfg.AtomicUpgrade, + CleanupOnFail: cfg.CleanupOnFail, + } +} + // Execute executes the `helm upgrade` command. func (u *Upgrade) Execute(_ Config) error { return u.cmd.Run() diff --git a/internal/run/upgrade_test.go b/internal/run/upgrade_test.go index 886fb3b..46fb7c5 100644 --- a/internal/run/upgrade_test.go +++ b/internal/run/upgrade_test.go @@ -3,6 +3,7 @@ package run import ( "fmt" "github.com/golang/mock/gomock" + "github.com/pelotech/drone-helm3/internal/env" "github.com/stretchr/testify/suite" "strings" "testing" @@ -31,6 +32,41 @@ func TestUpgradeTestSuite(t *testing.T) { suite.Run(t, new(UpgradeTestSuite)) } +func (suite *UpgradeTestSuite) TestNewUpgrade() { + cfg := env.Config{ + ChartVersion: "seventeen", + DryRun: true, + Wait: true, + Values: "steadfastness,forthrightness", + StringValues: "tensile_strength,flexibility", + ValuesFiles: []string{"/root/price_inventory.yml"}, + ReuseValues: true, + Timeout: "go sit in the corner", + Chart: "billboard_top_100", + Release: "post_malone_circles", + Force: true, + AtomicUpgrade: true, + CleanupOnFail: true, + } + + up := NewUpgrade(cfg) + suite.Equal(&Upgrade{ + Chart: cfg.Chart, + Release: cfg.Release, + ChartVersion: cfg.ChartVersion, + DryRun: true, + Wait: cfg.Wait, + Values: "steadfastness,forthrightness", + StringValues: "tensile_strength,flexibility", + ValuesFiles: []string{"/root/price_inventory.yml"}, + ReuseValues: cfg.ReuseValues, + Timeout: cfg.Timeout, + Force: cfg.Force, + Atomic: true, + CleanupOnFail: true, + }, up) +} + func (suite *UpgradeTestSuite) TestPrepareAndExecute() { defer suite.ctrl.Finish() From 21b9d3232950491790af972f6ea0bbde74ae49c3 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Thu, 16 Jan 2020 13:57:28 -0800 Subject: [PATCH 3/8] Remove the tiny helper functions from plan.go [#67] Now that InitKube, AddRepo, and UpdateDependencies are initialized with NewSTEPNAME functions, the helper functions in plan.go are unnecessary--they do too little to be a useful abstraction, and they aren't complex or frequently-used enough to be worth extracting. --- internal/helm/plan.go | 38 ++++++++++++++------------------------ internal/helm/plan_test.go | 31 ------------------------------- 2 files changed, 14 insertions(+), 55 deletions(-) diff --git a/internal/helm/plan.go b/internal/helm/plan.go index 391d220..bb1e901 100644 --- a/internal/helm/plan.go +++ b/internal/helm/plan.go @@ -93,10 +93,13 @@ func (p *Plan) Execute() error { } var upgrade = func(cfg env.Config) []Step { - steps := initKube(cfg) - steps = append(steps, addRepos(cfg)...) + var steps []Step + steps = append(steps, run.NewInitKube(cfg, kubeConfigTemplate, kubeConfigFile)) + for _, repo := range cfg.AddRepos { + steps = append(steps, run.NewAddRepo(repo)) + } if cfg.UpdateDependencies { - steps = append(steps, depUpdate(cfg)...) + steps = append(steps, run.NewDepUpdate(cfg)) } steps = append(steps, run.NewUpgrade(cfg)) @@ -104,9 +107,10 @@ var upgrade = func(cfg env.Config) []Step { } var uninstall = func(cfg env.Config) []Step { - steps := initKube(cfg) + var steps []Step + steps = append(steps, run.NewInitKube(cfg, kubeConfigTemplate, kubeConfigFile)) if cfg.UpdateDependencies { - steps = append(steps, depUpdate(cfg)...) + steps = append(steps, run.NewDepUpdate(cfg)) } steps = append(steps, run.NewUninstall(cfg)) @@ -114,9 +118,12 @@ var uninstall = func(cfg env.Config) []Step { } var lint = func(cfg env.Config) []Step { - steps := addRepos(cfg) + var steps []Step + for _, repo := range cfg.AddRepos { + steps = append(steps, run.NewAddRepo(repo)) + } if cfg.UpdateDependencies { - steps = append(steps, depUpdate(cfg)...) + steps = append(steps, run.NewDepUpdate(cfg)) } steps = append(steps, run.NewLint(cfg)) return steps @@ -125,20 +132,3 @@ var lint = func(cfg env.Config) []Step { var help = func(cfg env.Config) []Step { return []Step{run.NewHelp(cfg)} } - -func initKube(cfg env.Config) []Step { - return []Step{run.NewInitKube(cfg, kubeConfigTemplate, kubeConfigFile)} -} - -func addRepos(cfg env.Config) []Step { - steps := make([]Step, 0) - for _, repo := range cfg.AddRepos { - steps = append(steps, run.NewAddRepo(repo)) - } - - return steps -} - -func depUpdate(cfg env.Config) []Step { - return []Step{run.NewDepUpdate(cfg)} -} diff --git a/internal/helm/plan_test.go b/internal/helm/plan_test.go index f922010..a792440 100644 --- a/internal/helm/plan_test.go +++ b/internal/helm/plan_test.go @@ -175,37 +175,6 @@ func (suite *PlanTestSuite) TestUninstallWithUpdateDependencies() { suite.IsType(&run.DepUpdate{}, steps[1]) } -func (suite *PlanTestSuite) TestInitKube() { - cfg := env.Config{} - - steps := initKube(cfg) - suite.Require().Equal(1, len(steps), "initKube should return one step") - suite.IsType(&run.InitKube{}, steps[0]) -} - -func (suite *PlanTestSuite) TestDepUpdate() { - cfg := env.Config{ - UpdateDependencies: true, - } - - steps := depUpdate(cfg) - suite.Require().Equal(1, len(steps), "depUpdate should return one step") - suite.IsType(&run.DepUpdate{}, steps[0]) -} - -func (suite *PlanTestSuite) TestAddRepos() { - cfg := env.Config{ - AddRepos: []string{ - "first=https://add.repos/one", - "second=https://add.repos/two", - }, - } - steps := addRepos(cfg) - suite.Require().Equal(2, len(steps), "addRepos should add one step per repo") - suite.IsType(&run.AddRepo{}, steps[0]) - suite.IsType(&run.AddRepo{}, steps[1]) -} - func (suite *PlanTestSuite) TestLint() { steps := lint(env.Config{}) suite.Require().Equal(1, len(steps)) From 88bb8085b0f55281f93cae3b16c2a2c6b2ca2708 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Thu, 16 Jan 2020 15:11:42 -0800 Subject: [PATCH 4/8] Deduplicate the kubeValues data in InitKube [#67] Now that the InitKube initialization happens inside its own package, the private .values field can be populated at the same time, rather than having to wait for Prepare(). Also clarified the config/template filename fields (configFile vs. ConfigFile was particularly ambiguous). --- internal/run/initkube.go | 62 +++++++----------- internal/run/initkube_test.go | 120 +++++++++++++++++++--------------- 2 files changed, 92 insertions(+), 90 deletions(-) diff --git a/internal/run/initkube.go b/internal/run/initkube.go index f15ba75..cee9d28 100644 --- a/internal/run/initkube.go +++ b/internal/run/initkube.go @@ -11,17 +11,11 @@ import ( // InitKube is a step in a helm Plan that initializes the kubernetes config file. type InitKube struct { - SkipTLSVerify bool - Certificate string - APIServer string - ServiceAccount string - Token string - TemplateFile string - ConfigFile string - - template *template.Template - configFile io.WriteCloser - values kubeValues + templateFilename string + configFilename string + template *template.Template + configFile io.WriteCloser + values kubeValues } type kubeValues struct { @@ -36,20 +30,23 @@ type kubeValues struct { // NewInitKube creates a InitKube using the given Config and filepaths. No validation is performed at this time. func NewInitKube(cfg env.Config, templateFile, configFile string) *InitKube { return &InitKube{ - SkipTLSVerify: cfg.SkipTLSVerify, - Certificate: cfg.Certificate, - APIServer: cfg.APIServer, - ServiceAccount: cfg.ServiceAccount, - Token: cfg.KubeToken, - TemplateFile: templateFile, - ConfigFile: configFile, + values: kubeValues{ + SkipTLSVerify: cfg.SkipTLSVerify, + Certificate: cfg.Certificate, + APIServer: cfg.APIServer, + Namespace: cfg.Namespace, + ServiceAccount: cfg.ServiceAccount, + Token: cfg.KubeToken, + }, + templateFilename: templateFile, + configFilename: configFile, } } // Execute generates a kubernetes config file from drone-helm3's template. func (i *InitKube) Execute(cfg Config) error { if cfg.Debug { - fmt.Fprintf(cfg.Stderr, "writing kubeconfig file to %s\n", i.ConfigFile) + fmt.Fprintf(cfg.Stderr, "writing kubeconfig file to %s\n", i.configFilename) } defer i.configFile.Close() return i.template.Execute(i.configFile, i.values) @@ -59,45 +56,36 @@ func (i *InitKube) Execute(cfg Config) error { func (i *InitKube) Prepare(cfg Config) error { var err error - if i.APIServer == "" { + if i.values.APIServer == "" { return errors.New("an API Server is needed to deploy") } - if i.Token == "" { + if i.values.Token == "" { return errors.New("token is needed to deploy") } - if i.ServiceAccount == "" { - i.ServiceAccount = "helm" + if i.values.ServiceAccount == "" { + i.values.ServiceAccount = "helm" } if cfg.Debug { - fmt.Fprintf(cfg.Stderr, "loading kubeconfig template from %s\n", i.TemplateFile) + fmt.Fprintf(cfg.Stderr, "loading kubeconfig template from %s\n", i.templateFilename) } - i.template, err = template.ParseFiles(i.TemplateFile) + i.template, err = template.ParseFiles(i.templateFilename) if err != nil { return fmt.Errorf("could not load kubeconfig template: %w", err) } - i.values = kubeValues{ - SkipTLSVerify: i.SkipTLSVerify, - Certificate: i.Certificate, - APIServer: i.APIServer, - ServiceAccount: i.ServiceAccount, - Token: i.Token, - Namespace: cfg.Namespace, - } - if cfg.Debug { - if _, err := os.Stat(i.ConfigFile); err != nil { + if _, err := os.Stat(i.configFilename); err != nil { // non-nil err here isn't an actual error state; the kubeconfig just doesn't exist fmt.Fprint(cfg.Stderr, "creating ") } else { fmt.Fprint(cfg.Stderr, "truncating ") } - fmt.Fprintf(cfg.Stderr, "kubeconfig file at %s\n", i.ConfigFile) + fmt.Fprintf(cfg.Stderr, "kubeconfig file at %s\n", i.configFilename) } - i.configFile, err = os.Create(i.ConfigFile) + i.configFile, err = os.Create(i.configFilename) if err != nil { return fmt.Errorf("could not open kubeconfig file for writing: %w", err) } diff --git a/internal/run/initkube_test.go b/internal/run/initkube_test.go index df6d531..87e5398 100644 --- a/internal/run/initkube_test.go +++ b/internal/run/initkube_test.go @@ -29,13 +29,15 @@ func (suite *InitKubeTestSuite) TestNewInitKube() { init := NewInitKube(cfg, "conf.tpl", "conf.yml") suite.Equal(&InitKube{ - SkipTLSVerify: true, - Certificate: "cHJvY2xhaW1zIHdvbmRlcmZ1bCBmcmllbmRzaGlw", - APIServer: "98.765.43.21", - ServiceAccount: "greathelm", - Token: "b2YgbXkgYWZmZWN0aW9u", - TemplateFile: "conf.tpl", - ConfigFile: "conf.yml", + values: kubeValues{ + SkipTLSVerify: true, + Certificate: "cHJvY2xhaW1zIHdvbmRlcmZ1bCBmcmllbmRzaGlw", + APIServer: "98.765.43.21", + ServiceAccount: "greathelm", + Token: "b2YgbXkgYWZmZWN0aW9u", + }, + templateFilename: "conf.tpl", + configFilename: "conf.yml", }, init) } @@ -52,15 +54,16 @@ namespace: {{ .Namespace }} suite.Require().Nil(err) init := InitKube{ - APIServer: "Sysadmin", - Certificate: "CCNA", - Token: "Aspire virtual currency", - TemplateFile: templateFile.Name(), - ConfigFile: configFile.Name(), - } - cfg := Config{ - Namespace: "Cisco", + values: kubeValues{ + APIServer: "Sysadmin", + Certificate: "CCNA", + Token: "Aspire virtual currency", + Namespace: "Cisco", + }, + templateFilename: templateFile.Name(), + configFilename: configFile.Name(), } + cfg := Config{} err = init.Prepare(cfg) suite.Require().Nil(err) @@ -85,16 +88,17 @@ func (suite *InitKubeTestSuite) TestExecuteGeneratesConfig() { defer os.Remove(configFile.Name()) suite.Require().NoError(err) - cfg := Config{ - Namespace: "marshmallow", - } + cfg := Config{} init := InitKube{ - ConfigFile: configFile.Name(), - TemplateFile: "../../assets/kubeconfig.tpl", // the actual kubeconfig template - APIServer: "https://kube.cluster/peanut", - ServiceAccount: "chef", - Token: "eWVhaCB3ZSB0b2tpbic=", - Certificate: "d293LCB5b3UgYXJlIHNvIGNvb2wgZm9yIHNtb2tpbmcgd2VlZCDwn5mE", + configFilename: configFile.Name(), + templateFilename: "../../assets/kubeconfig.tpl", // the actual kubeconfig template + values: kubeValues{ + APIServer: "https://kube.cluster/peanut", + ServiceAccount: "chef", + Token: "eWVhaCB3ZSB0b2tpbic=", + Certificate: "d293LCB5b3UgYXJlIHNvIGNvb2wgZm9yIHNtb2tpbmcgd2VlZCDwn5mE", + Namespace: "marshmallow", + }, } suite.Require().NoError(init.Prepare(cfg)) suite.Require().NoError(init.Execute(cfg)) @@ -120,8 +124,8 @@ func (suite *InitKubeTestSuite) TestExecuteGeneratesConfig() { suite.NoError(yaml.UnmarshalStrict(contents, &conf)) // test the other branch of the certificate/SkipTLSVerify conditional - init.SkipTLSVerify = true - init.Certificate = "" + init.values.SkipTLSVerify = true + init.values.Certificate = "" suite.Require().NoError(init.Prepare(cfg)) suite.Require().NoError(init.Execute(cfg)) @@ -139,10 +143,12 @@ func (suite *InitKubeTestSuite) TestPrepareParseError() { suite.Require().Nil(err) init := InitKube{ - APIServer: "Sysadmin", - Certificate: "CCNA", - Token: "Aspire virtual currency", - TemplateFile: templateFile.Name(), + values: kubeValues{ + APIServer: "Sysadmin", + Certificate: "CCNA", + Token: "Aspire virtual currency", + }, + templateFilename: templateFile.Name(), } err = init.Prepare(Config{}) suite.Error(err) @@ -151,10 +157,12 @@ func (suite *InitKubeTestSuite) TestPrepareParseError() { func (suite *InitKubeTestSuite) TestPrepareNonexistentTemplateFile() { init := InitKube{ - APIServer: "Sysadmin", - Certificate: "CCNA", - Token: "Aspire virtual currency", - TemplateFile: "/usr/foreign/exclude/kubeprofig.tpl", + values: kubeValues{ + APIServer: "Sysadmin", + Certificate: "CCNA", + Token: "Aspire virtual currency", + }, + templateFilename: "/usr/foreign/exclude/kubeprofig.tpl", } err := init.Prepare(Config{}) suite.Error(err) @@ -166,11 +174,13 @@ func (suite *InitKubeTestSuite) TestPrepareCannotOpenDestinationFile() { defer os.Remove(templateFile.Name()) suite.Require().Nil(err) init := InitKube{ - APIServer: "Sysadmin", - Certificate: "CCNA", - Token: "Aspire virtual currency", - TemplateFile: templateFile.Name(), - ConfigFile: "/usr/foreign/exclude/kubeprofig", + values: kubeValues{ + APIServer: "Sysadmin", + Certificate: "CCNA", + Token: "Aspire virtual currency", + }, + templateFilename: templateFile.Name(), + configFilename: "/usr/foreign/exclude/kubeprofig", } cfg := Config{} @@ -190,22 +200,24 @@ func (suite *InitKubeTestSuite) TestPrepareRequiredConfig() { // initial config with all required fields present init := InitKube{ - APIServer: "Sysadmin", - Certificate: "CCNA", - Token: "Aspire virtual currency", - TemplateFile: templateFile.Name(), - ConfigFile: configFile.Name(), + values: kubeValues{ + APIServer: "Sysadmin", + Certificate: "CCNA", + Token: "Aspire virtual currency", + }, + templateFilename: templateFile.Name(), + configFilename: configFile.Name(), } cfg := Config{} suite.NoError(init.Prepare(cfg)) // consistency check; we should be starting in a happy state - init.APIServer = "" + init.values.APIServer = "" suite.Error(init.Prepare(cfg), "APIServer should be required.") - init.APIServer = "Sysadmin" - init.Token = "" + init.values.APIServer = "Sysadmin" + init.values.Token = "" suite.Error(init.Prepare(cfg), "Token should be required.") } @@ -219,17 +231,19 @@ func (suite *InitKubeTestSuite) TestPrepareDefaultsServiceAccount() { suite.Require().Nil(err) init := InitKube{ - APIServer: "Sysadmin", - Certificate: "CCNA", - Token: "Aspire virtual currency", - TemplateFile: templateFile.Name(), - ConfigFile: configFile.Name(), + values: kubeValues{ + APIServer: "Sysadmin", + Certificate: "CCNA", + Token: "Aspire virtual currency", + }, + templateFilename: templateFile.Name(), + configFilename: configFile.Name(), } cfg := Config{} init.Prepare(cfg) - suite.Equal("helm", init.ServiceAccount) + suite.Equal("helm", init.values.ServiceAccount) } func tempfile(name, contents string) (*os.File, error) { From 231138563ced30d51c67e7a17b75a439f68fe005 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Thu, 16 Jan 2020 15:30:21 -0800 Subject: [PATCH 5/8] Remove the cfg argument from Step.Execute [#67] This is the first step toward removing run.Config entirely. InitKube was the only Step that even used cfg in its Execute function; the rest just discarded it. --- internal/helm/mock_step_test.go | 8 ++++---- internal/helm/plan.go | 4 ++-- internal/helm/plan_test.go | 6 +++--- internal/run/addrepo.go | 2 +- internal/run/addrepo_test.go | 2 +- internal/run/depupdate.go | 2 +- internal/run/depupdate_test.go | 2 +- internal/run/help.go | 2 +- internal/run/help_test.go | 5 ++--- internal/run/initkube.go | 22 +++++++++++++--------- internal/run/initkube_test.go | 11 ++++++++--- internal/run/lint.go | 2 +- internal/run/lint_test.go | 2 +- internal/run/uninstall.go | 2 +- internal/run/uninstall_test.go | 2 +- internal/run/upgrade.go | 2 +- internal/run/upgrade_test.go | 2 +- 17 files changed, 43 insertions(+), 35 deletions(-) diff --git a/internal/helm/mock_step_test.go b/internal/helm/mock_step_test.go index 5387162..ca8e90c 100644 --- a/internal/helm/mock_step_test.go +++ b/internal/helm/mock_step_test.go @@ -48,15 +48,15 @@ func (mr *MockStepMockRecorder) Prepare(arg0 interface{}) *gomock.Call { } // Execute mocks base method -func (m *MockStep) Execute(arg0 run.Config) error { +func (m *MockStep) Execute() error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Execute", arg0) + ret := m.ctrl.Call(m, "Execute") ret0, _ := ret[0].(error) return ret0 } // Execute indicates an expected call of Execute -func (mr *MockStepMockRecorder) Execute(arg0 interface{}) *gomock.Call { +func (mr *MockStepMockRecorder) Execute() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Execute", reflect.TypeOf((*MockStep)(nil).Execute), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Execute", reflect.TypeOf((*MockStep)(nil).Execute)) } diff --git a/internal/helm/plan.go b/internal/helm/plan.go index bb1e901..5d28b10 100644 --- a/internal/helm/plan.go +++ b/internal/helm/plan.go @@ -15,7 +15,7 @@ const ( // A Step is one step in the plan. type Step interface { Prepare(run.Config) error - Execute(run.Config) error + Execute() error } // A Plan is a series of steps to perform. @@ -84,7 +84,7 @@ func (p *Plan) Execute() error { fmt.Fprintf(p.cfg.Stderr, "calling %T.Execute (step %d)\n", step, i) } - if err := step.Execute(p.runCfg); err != nil { + if err := step.Execute(); err != nil { return fmt.Errorf("while executing %T step: %w", step, err) } } diff --git a/internal/helm/plan_test.go b/internal/helm/plan_test.go index a792440..2dea619 100644 --- a/internal/helm/plan_test.go +++ b/internal/helm/plan_test.go @@ -98,10 +98,10 @@ func (suite *PlanTestSuite) TestExecute() { } stepOne.EXPECT(). - Execute(runCfg). + Execute(). Times(1) stepTwo.EXPECT(). - Execute(runCfg). + Execute(). Times(1) suite.NoError(plan.Execute()) @@ -121,7 +121,7 @@ func (suite *PlanTestSuite) TestExecuteAbortsOnError() { } stepOne.EXPECT(). - Execute(runCfg). + Execute(). Times(1). Return(fmt.Errorf("oh, he'll gnaw")) diff --git a/internal/run/addrepo.go b/internal/run/addrepo.go index 8cc7552..6a065bf 100644 --- a/internal/run/addrepo.go +++ b/internal/run/addrepo.go @@ -19,7 +19,7 @@ func NewAddRepo(repo string) *AddRepo { } // Execute executes the `helm repo add` command. -func (a *AddRepo) Execute(_ Config) error { +func (a *AddRepo) Execute() error { return a.cmd.Run() } diff --git a/internal/run/addrepo_test.go b/internal/run/addrepo_test.go index 51c31a0..5e4d0bc 100644 --- a/internal/run/addrepo_test.go +++ b/internal/run/addrepo_test.go @@ -70,7 +70,7 @@ func (suite *AddRepoTestSuite) TestPrepareAndExecute() { Run(). Times(1) - suite.Require().NoError(a.Execute(cfg)) + suite.Require().NoError(a.Execute()) } diff --git a/internal/run/depupdate.go b/internal/run/depupdate.go index c34c0ee..5f3f189 100644 --- a/internal/run/depupdate.go +++ b/internal/run/depupdate.go @@ -19,7 +19,7 @@ func NewDepUpdate(cfg env.Config) *DepUpdate { } // Execute executes the `helm upgrade` command. -func (d *DepUpdate) Execute(_ Config) error { +func (d *DepUpdate) Execute() error { return d.cmd.Run() } diff --git a/internal/run/depupdate_test.go b/internal/run/depupdate_test.go index 94b0198..f0a7e1a 100644 --- a/internal/run/depupdate_test.go +++ b/internal/run/depupdate_test.go @@ -69,7 +69,7 @@ func (suite *DepUpdateTestSuite) TestPrepareAndExecute() { } suite.Require().NoError(d.Prepare(cfg)) - suite.NoError(d.Execute(cfg)) + suite.NoError(d.Execute()) } func (suite *DepUpdateTestSuite) TestPrepareNamespaceFlag() { diff --git a/internal/run/help.go b/internal/run/help.go index 40e0f86..69666d3 100644 --- a/internal/run/help.go +++ b/internal/run/help.go @@ -19,7 +19,7 @@ func NewHelp(cfg env.Config) *Help { } // Execute executes the `helm help` command. -func (h *Help) Execute(cfg Config) error { +func (h *Help) Execute() error { if err := h.cmd.Run(); err != nil { return fmt.Errorf("while running '%s': %w", h.cmd.String(), err) } diff --git a/internal/run/help_test.go b/internal/run/help_test.go index 0cf5927..1bad0bf 100644 --- a/internal/run/help_test.go +++ b/internal/run/help_test.go @@ -73,15 +73,14 @@ func (suite *HelpTestSuite) TestExecute() { Run(). Times(2) - cfg := Config{} help := Help{ HelmCommand: "help", cmd: mCmd, } - suite.NoError(help.Execute(cfg)) + suite.NoError(help.Execute()) help.HelmCommand = "get down on friday" - suite.EqualError(help.Execute(cfg), "unknown command 'get down on friday'") + suite.EqualError(help.Execute(), "unknown command 'get down on friday'") } func (suite *HelpTestSuite) TestPrepareDebugFlag() { diff --git a/internal/run/initkube.go b/internal/run/initkube.go index cee9d28..b6c0440 100644 --- a/internal/run/initkube.go +++ b/internal/run/initkube.go @@ -13,6 +13,8 @@ import ( type InitKube struct { templateFilename string configFilename string + debug bool + stderr io.Writer template *template.Template configFile io.WriteCloser values kubeValues @@ -40,13 +42,15 @@ func NewInitKube(cfg env.Config, templateFile, configFile string) *InitKube { }, templateFilename: templateFile, configFilename: configFile, + debug: cfg.Debug, + stderr: cfg.Stderr, } } // Execute generates a kubernetes config file from drone-helm3's template. -func (i *InitKube) Execute(cfg Config) error { - if cfg.Debug { - fmt.Fprintf(cfg.Stderr, "writing kubeconfig file to %s\n", i.configFilename) +func (i *InitKube) Execute() error { + if i.debug { + fmt.Fprintf(i.stderr, "writing kubeconfig file to %s\n", i.configFilename) } defer i.configFile.Close() return i.template.Execute(i.configFile, i.values) @@ -67,22 +71,22 @@ func (i *InitKube) Prepare(cfg Config) error { i.values.ServiceAccount = "helm" } - if cfg.Debug { - fmt.Fprintf(cfg.Stderr, "loading kubeconfig template from %s\n", i.templateFilename) + if i.debug { + fmt.Fprintf(i.stderr, "loading kubeconfig template from %s\n", i.templateFilename) } i.template, err = template.ParseFiles(i.templateFilename) if err != nil { return fmt.Errorf("could not load kubeconfig template: %w", err) } - if cfg.Debug { + if i.debug { if _, err := os.Stat(i.configFilename); err != nil { // non-nil err here isn't an actual error state; the kubeconfig just doesn't exist - fmt.Fprint(cfg.Stderr, "creating ") + fmt.Fprint(i.stderr, "creating ") } else { - fmt.Fprint(cfg.Stderr, "truncating ") + fmt.Fprint(i.stderr, "truncating ") } - fmt.Fprintf(cfg.Stderr, "kubeconfig file at %s\n", i.configFilename) + fmt.Fprintf(i.stderr, "kubeconfig file at %s\n", i.configFilename) } i.configFile, err = os.Create(i.configFilename) diff --git a/internal/run/initkube_test.go b/internal/run/initkube_test.go index 87e5398..567944e 100644 --- a/internal/run/initkube_test.go +++ b/internal/run/initkube_test.go @@ -6,6 +6,7 @@ import ( yaml "gopkg.in/yaml.v2" "io/ioutil" "os" + "strings" "testing" "text/template" ) @@ -25,6 +26,8 @@ func (suite *InitKubeTestSuite) TestNewInitKube() { APIServer: "98.765.43.21", ServiceAccount: "greathelm", KubeToken: "b2YgbXkgYWZmZWN0aW9u", + Stderr: &strings.Builder{}, + Debug: true, } init := NewInitKube(cfg, "conf.tpl", "conf.yml") @@ -38,6 +41,8 @@ func (suite *InitKubeTestSuite) TestNewInitKube() { }, templateFilename: "conf.tpl", configFilename: "conf.yml", + debug: true, + stderr: cfg.Stderr, }, init) } @@ -70,7 +75,7 @@ namespace: {{ .Namespace }} suite.IsType(&template.Template{}, init.template) suite.NotNil(init.configFile) - err = init.Execute(cfg) + err = init.Execute() suite.Require().Nil(err) conf, err := ioutil.ReadFile(configFile.Name()) @@ -101,7 +106,7 @@ func (suite *InitKubeTestSuite) TestExecuteGeneratesConfig() { }, } suite.Require().NoError(init.Prepare(cfg)) - suite.Require().NoError(init.Execute(cfg)) + suite.Require().NoError(init.Execute()) contents, err := ioutil.ReadFile(configFile.Name()) suite.Require().NoError(err) @@ -128,7 +133,7 @@ func (suite *InitKubeTestSuite) TestExecuteGeneratesConfig() { init.values.Certificate = "" suite.Require().NoError(init.Prepare(cfg)) - suite.Require().NoError(init.Execute(cfg)) + suite.Require().NoError(init.Execute()) contents, err = ioutil.ReadFile(configFile.Name()) suite.Require().NoError(err) suite.Contains(string(contents), "insecure-skip-tls-verify: true") diff --git a/internal/run/lint.go b/internal/run/lint.go index 03912fd..e6b550c 100644 --- a/internal/run/lint.go +++ b/internal/run/lint.go @@ -27,7 +27,7 @@ func NewLint(cfg env.Config) *Lint { } // Execute executes the `helm lint` command. -func (l *Lint) Execute(_ Config) error { +func (l *Lint) Execute() error { return l.cmd.Run() } diff --git a/internal/run/lint_test.go b/internal/run/lint_test.go index 3323ccb..e7469de 100644 --- a/internal/run/lint_test.go +++ b/internal/run/lint_test.go @@ -82,7 +82,7 @@ func (suite *LintTestSuite) TestPrepareAndExecute() { err := l.Prepare(cfg) suite.Require().Nil(err) - l.Execute(cfg) + l.Execute() } func (suite *LintTestSuite) TestPrepareRequiresChart() { diff --git a/internal/run/uninstall.go b/internal/run/uninstall.go index 790e9c6..07fd820 100644 --- a/internal/run/uninstall.go +++ b/internal/run/uninstall.go @@ -23,7 +23,7 @@ func NewUninstall(cfg env.Config) *Uninstall { } // Execute executes the `helm uninstall` command. -func (u *Uninstall) Execute(_ Config) error { +func (u *Uninstall) Execute() error { return u.cmd.Run() } diff --git a/internal/run/uninstall_test.go b/internal/run/uninstall_test.go index 0dafe33..6bac55c 100644 --- a/internal/run/uninstall_test.go +++ b/internal/run/uninstall_test.go @@ -78,7 +78,7 @@ func (suite *UninstallTestSuite) TestPrepareAndExecute() { expected := []string{"uninstall", "zayde_wølf_king"} suite.Equal(expected, actual) - u.Execute(cfg) + u.Execute() } func (suite *UninstallTestSuite) TestPrepareDryRunFlag() { diff --git a/internal/run/upgrade.go b/internal/run/upgrade.go index b85b431..317f065 100644 --- a/internal/run/upgrade.go +++ b/internal/run/upgrade.go @@ -45,7 +45,7 @@ func NewUpgrade(cfg env.Config) *Upgrade { } // Execute executes the `helm upgrade` command. -func (u *Upgrade) Execute(_ Config) error { +func (u *Upgrade) Execute() error { return u.cmd.Run() } diff --git a/internal/run/upgrade_test.go b/internal/run/upgrade_test.go index 46fb7c5..71b92cb 100644 --- a/internal/run/upgrade_test.go +++ b/internal/run/upgrade_test.go @@ -93,7 +93,7 @@ func (suite *UpgradeTestSuite) TestPrepareAndExecute() { cfg := Config{} err := u.Prepare(cfg) suite.Require().Nil(err) - u.Execute(cfg) + u.Execute() } func (suite *UpgradeTestSuite) TestPrepareNamespaceFlag() { From d8ddb79ef4b1f9018282a73b3423a03024aa552e Mon Sep 17 00:00:00 2001 From: Erin Call Date: Thu, 16 Jan 2020 15:32:40 -0800 Subject: [PATCH 6/8] Test InitKube's use of the Debug flag [#67] (Just something I happened across while writing the previous commit) --- internal/run/initkube_test.go | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/internal/run/initkube_test.go b/internal/run/initkube_test.go index 567944e..c34723f 100644 --- a/internal/run/initkube_test.go +++ b/internal/run/initkube_test.go @@ -1,6 +1,7 @@ package run import ( + "fmt" "github.com/pelotech/drone-helm3/internal/env" "github.com/stretchr/testify/suite" yaml "gopkg.in/yaml.v2" @@ -251,6 +252,40 @@ func (suite *InitKubeTestSuite) TestPrepareDefaultsServiceAccount() { suite.Equal("helm", init.values.ServiceAccount) } +func (suite *InitKubeTestSuite) TestDebugOutput() { + templateFile, err := tempfile("kubeconfig********.yml.tpl", "hurgity burgity") + defer os.Remove(templateFile.Name()) + suite.Require().Nil(err) + + configFile, err := tempfile("kubeconfig********.yml", "") + defer os.Remove(configFile.Name()) + suite.Require().Nil(err) + + stdout := &strings.Builder{} + stderr := &strings.Builder{} + cfg := env.Config{ + APIServer: "http://my.kube.server/", + KubeToken: "QSBzaW5nbGUgcm9zZQ==", + Debug: true, + Stdout: stdout, + Stderr: stderr, + } + runCfg := Config{ + Debug: true, + Stdout: stdout, + Stderr: stderr, + } + + init := NewInitKube(cfg, templateFile.Name(), configFile.Name()) + suite.NoError(init.Prepare(runCfg)) + + suite.Contains(stderr.String(), fmt.Sprintf("loading kubeconfig template from %s\n", templateFile.Name())) + suite.Contains(stderr.String(), fmt.Sprintf("truncating kubeconfig file at %s\n", configFile.Name())) + + suite.NoError(init.Execute()) + suite.Contains(stderr.String(), fmt.Sprintf("writing kubeconfig file to %s\n", configFile.Name())) +} + func tempfile(name, contents string) (*os.File, error) { file, err := ioutil.TempFile("", name) if err != nil { From a21848484bc6dc1e5d45cb1b69d843c521686ab8 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Fri, 17 Jan 2020 10:13:53 -0800 Subject: [PATCH 7/8] Initialize run.Configs in the NewSTEP functions [#67] This fixes the run package's leaky abstraction; other packages no longer need to know or care that run.Config even exists. Note that since the various Steps now depend on having a non-nil pointer to a run.Config, it's unsafe (or at least risky) to initialize them directly. They should be created with their NewSTEPNAME functions. All their fields are now private, to reflect this. --- internal/helm/mock_step_test.go | 9 +- internal/helm/plan.go | 19 ++-- internal/helm/plan_test.go | 24 ++--- internal/run/addrepo.go | 31 ++++--- internal/run/addrepo_test.go | 47 ++++------ internal/run/config.go | 21 +++-- internal/run/config_test.go | 35 ++++++++ internal/run/depupdate.go | 26 +++--- internal/run/depupdate_test.go | 33 ++++--- internal/run/help.go | 22 ++--- internal/run/help_test.go | 28 +++--- internal/run/initkube.go | 8 +- internal/run/initkube_test.go | 153 ++++++++++++-------------------- internal/run/lint.go | 54 +++++------ internal/run/lint_test.go | 59 ++++++------ internal/run/uninstall.go | 38 ++++---- internal/run/uninstall_test.go | 53 ++++++----- internal/run/upgrade.go | 106 +++++++++++----------- internal/run/upgrade_test.go | 89 +++++++++---------- 19 files changed, 408 insertions(+), 447 deletions(-) create mode 100644 internal/run/config_test.go diff --git a/internal/helm/mock_step_test.go b/internal/helm/mock_step_test.go index ca8e90c..fa9c629 100644 --- a/internal/helm/mock_step_test.go +++ b/internal/helm/mock_step_test.go @@ -6,7 +6,6 @@ package helm import ( gomock "github.com/golang/mock/gomock" - run "github.com/pelotech/drone-helm3/internal/run" reflect "reflect" ) @@ -34,17 +33,17 @@ func (m *MockStep) EXPECT() *MockStepMockRecorder { } // Prepare mocks base method -func (m *MockStep) Prepare(arg0 run.Config) error { +func (m *MockStep) Prepare() error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Prepare", arg0) + ret := m.ctrl.Call(m, "Prepare") ret0, _ := ret[0].(error) return ret0 } // Prepare indicates an expected call of Prepare -func (mr *MockStepMockRecorder) Prepare(arg0 interface{}) *gomock.Call { +func (mr *MockStepMockRecorder) Prepare() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Prepare", reflect.TypeOf((*MockStep)(nil).Prepare), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Prepare", reflect.TypeOf((*MockStep)(nil).Prepare)) } // Execute mocks base method diff --git a/internal/helm/plan.go b/internal/helm/plan.go index 5d28b10..c09bdc8 100644 --- a/internal/helm/plan.go +++ b/internal/helm/plan.go @@ -14,27 +14,20 @@ const ( // A Step is one step in the plan. type Step interface { - Prepare(run.Config) error + Prepare() error Execute() error } // A Plan is a series of steps to perform. type Plan struct { - steps []Step - cfg env.Config - runCfg run.Config + steps []Step + cfg env.Config } // NewPlan makes a plan for running a helm operation. func NewPlan(cfg env.Config) (*Plan, error) { p := Plan{ cfg: cfg, - runCfg: run.Config{ - Debug: cfg.Debug, - Namespace: cfg.Namespace, - Stdout: cfg.Stdout, - Stderr: cfg.Stderr, - }, } p.steps = (*determineSteps(cfg))(cfg) @@ -44,7 +37,7 @@ func NewPlan(cfg env.Config) (*Plan, error) { fmt.Fprintf(os.Stderr, "calling %T.Prepare (step %d)\n", step, i) } - if err := step.Prepare(p.runCfg); err != nil { + if err := step.Prepare(); err != nil { err = fmt.Errorf("while preparing %T step: %w", step, err) return nil, err } @@ -96,7 +89,7 @@ var upgrade = func(cfg env.Config) []Step { var steps []Step steps = append(steps, run.NewInitKube(cfg, kubeConfigTemplate, kubeConfigFile)) for _, repo := range cfg.AddRepos { - steps = append(steps, run.NewAddRepo(repo)) + steps = append(steps, run.NewAddRepo(cfg, repo)) } if cfg.UpdateDependencies { steps = append(steps, run.NewDepUpdate(cfg)) @@ -120,7 +113,7 @@ var uninstall = func(cfg env.Config) []Step { var lint = func(cfg env.Config) []Step { var steps []Step for _, repo := range cfg.AddRepos { - steps = append(steps, run.NewAddRepo(repo)) + steps = append(steps, run.NewAddRepo(cfg, repo)) } if cfg.UpdateDependencies { steps = append(steps, run.NewDepUpdate(cfg)) diff --git a/internal/helm/plan_test.go b/internal/helm/plan_test.go index 2dea619..77e913c 100644 --- a/internal/helm/plan_test.go +++ b/internal/helm/plan_test.go @@ -41,22 +41,14 @@ func (suite *PlanTestSuite) TestNewPlan() { Stderr: &stderr, } - runCfg := run.Config{ - Debug: false, - Namespace: "outer", - Stdout: &stdout, - Stderr: &stderr, - } - stepOne.EXPECT(). - Prepare(runCfg) + Prepare() stepTwo.EXPECT(). - Prepare(runCfg) + Prepare() plan, err := NewPlan(cfg) suite.Require().Nil(err) suite.Equal(cfg, plan.cfg) - suite.Equal(runCfg, plan.runCfg) } func (suite *PlanTestSuite) TestNewPlanAbortsOnError() { @@ -76,7 +68,7 @@ func (suite *PlanTestSuite) TestNewPlanAbortsOnError() { } stepOne.EXPECT(). - Prepare(gomock.Any()). + Prepare(). Return(fmt.Errorf("I'm starry Dave, aye, cat blew that")) _, err := NewPlan(cfg) @@ -90,11 +82,8 @@ func (suite *PlanTestSuite) TestExecute() { stepOne := NewMockStep(ctrl) stepTwo := NewMockStep(ctrl) - runCfg := run.Config{} - plan := Plan{ - steps: []Step{stepOne, stepTwo}, - runCfg: runCfg, + steps: []Step{stepOne, stepTwo}, } stepOne.EXPECT(). @@ -113,11 +102,8 @@ func (suite *PlanTestSuite) TestExecuteAbortsOnError() { stepOne := NewMockStep(ctrl) stepTwo := NewMockStep(ctrl) - runCfg := run.Config{} - plan := Plan{ - steps: []Step{stepOne, stepTwo}, - runCfg: runCfg, + steps: []Step{stepOne, stepTwo}, } stepOne.EXPECT(). diff --git a/internal/run/addrepo.go b/internal/run/addrepo.go index 6a065bf..05d5f3b 100644 --- a/internal/run/addrepo.go +++ b/internal/run/addrepo.go @@ -2,19 +2,22 @@ package run import ( "fmt" + "github.com/pelotech/drone-helm3/internal/env" "strings" ) // AddRepo is an execution step that calls `helm repo add` when executed. type AddRepo struct { - Repo string + *config + repo string cmd cmd } // NewAddRepo creates an AddRepo for the given repo-spec. No validation is performed at this time. -func NewAddRepo(repo string) *AddRepo { +func NewAddRepo(cfg env.Config, repo string) *AddRepo { return &AddRepo{ - Repo: repo, + config: newConfig(cfg), + repo: repo, } } @@ -24,13 +27,13 @@ func (a *AddRepo) Execute() error { } // Prepare gets the AddRepo ready to execute. -func (a *AddRepo) Prepare(cfg Config) error { - if a.Repo == "" { +func (a *AddRepo) Prepare() error { + if a.repo == "" { return fmt.Errorf("repo is required") } - split := strings.SplitN(a.Repo, "=", 2) + split := strings.SplitN(a.repo, "=", 2) if len(split) != 2 { - return fmt.Errorf("bad repo spec '%s'", a.Repo) + return fmt.Errorf("bad repo spec '%s'", a.repo) } name := split[0] @@ -38,21 +41,21 @@ func (a *AddRepo) Prepare(cfg Config) error { args := make([]string, 0) - if cfg.Namespace != "" { - args = append(args, "--namespace", cfg.Namespace) + if a.namespace != "" { + args = append(args, "--namespace", a.namespace) } - if cfg.Debug { + if a.debug { args = append(args, "--debug") } args = append(args, "repo", "add", name, url) a.cmd = command(helmBin, args...) - a.cmd.Stdout(cfg.Stdout) - a.cmd.Stderr(cfg.Stderr) + a.cmd.Stdout(a.stdout) + a.cmd.Stderr(a.stderr) - if cfg.Debug { - fmt.Fprintf(cfg.Stderr, "Generated command: '%s'\n", a.cmd.String()) + if a.debug { + fmt.Fprintf(a.stderr, "Generated command: '%s'\n", a.cmd.String()) } return nil diff --git a/internal/run/addrepo_test.go b/internal/run/addrepo_test.go index 5e4d0bc..86c4ad1 100644 --- a/internal/run/addrepo_test.go +++ b/internal/run/addrepo_test.go @@ -3,6 +3,7 @@ package run import ( "fmt" "github.com/golang/mock/gomock" + "github.com/pelotech/drone-helm3/internal/env" "github.com/stretchr/testify/suite" "strings" "testing" @@ -39,21 +40,20 @@ func TestAddRepoTestSuite(t *testing.T) { } func (suite *AddRepoTestSuite) TestNewAddRepo() { - repo := NewAddRepo("picompress=https://github.com/caleb_phipps/picompress") + repo := NewAddRepo(env.Config{}, "picompress=https://github.com/caleb_phipps/picompress") suite.Require().NotNil(repo) - suite.Equal("picompress=https://github.com/caleb_phipps/picompress", repo.Repo) + suite.Equal("picompress=https://github.com/caleb_phipps/picompress", repo.repo) + suite.NotNil(repo.config) } func (suite *AddRepoTestSuite) TestPrepareAndExecute() { stdout := strings.Builder{} stderr := strings.Builder{} - cfg := Config{ + cfg := env.Config{ Stdout: &stdout, Stderr: &stderr, } - a := AddRepo{ - Repo: "edeath=https://github.com/n_marks/e-death", - } + a := NewAddRepo(cfg, "edeath=https://github.com/n_marks/e-death") suite.mockCmd.EXPECT(). Stdout(&stdout). @@ -62,7 +62,7 @@ func (suite *AddRepoTestSuite) TestPrepareAndExecute() { Stderr(&stderr). Times(1) - suite.Require().NoError(a.Prepare(cfg)) + suite.Require().NoError(a.Prepare()) suite.Equal(helmBin, suite.commandPath) suite.Equal([]string{"repo", "add", "edeath", "https://github.com/n_marks/e-death"}, suite.commandArgs) @@ -78,42 +78,35 @@ func (suite *AddRepoTestSuite) TestPrepareRepoIsRequired() { // These aren't really expected, but allowing them gives clearer test-failure messages suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() - cfg := Config{} - a := AddRepo{} + a := NewAddRepo(env.Config{}, "") - err := a.Prepare(cfg) + err := a.Prepare() suite.EqualError(err, "repo is required") } func (suite *AddRepoTestSuite) TestPrepareMalformedRepo() { - a := AddRepo{ - Repo: "dwim", - } - err := a.Prepare(Config{}) + a := NewAddRepo(env.Config{}, "dwim") + err := a.Prepare() suite.EqualError(err, "bad repo spec 'dwim'") } func (suite *AddRepoTestSuite) TestPrepareWithEqualSignInURL() { suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() - a := AddRepo{ - Repo: "samaritan=https://github.com/arthur_claypool/samaritan?version=2.1", - } - suite.NoError(a.Prepare(Config{})) + a := NewAddRepo(env.Config{}, "samaritan=https://github.com/arthur_claypool/samaritan?version=2.1") + suite.NoError(a.Prepare()) suite.Contains(suite.commandArgs, "https://github.com/arthur_claypool/samaritan?version=2.1") } func (suite *AddRepoTestSuite) TestNamespaceFlag() { suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() - cfg := Config{ + cfg := env.Config{ Namespace: "alliteration", } - a := AddRepo{ - Repo: "edeath=https://github.com/theater_guy/e-death", - } + a := NewAddRepo(cfg, "edeath=https://github.com/theater_guy/e-death") - suite.NoError(a.Prepare(cfg)) + suite.NoError(a.Prepare()) suite.Equal(suite.commandPath, helmBin) suite.Equal(suite.commandArgs, []string{"--namespace", "alliteration", "repo", "add", "edeath", "https://github.com/theater_guy/e-death"}) @@ -133,15 +126,13 @@ func (suite *AddRepoTestSuite) TestDebugFlag() { return suite.mockCmd } - cfg := Config{ + cfg := env.Config{ Debug: true, Stderr: &stderr, } - a := AddRepo{ - Repo: "edeath=https://github.com/the_bug/e-death", - } + a := NewAddRepo(cfg, "edeath=https://github.com/the_bug/e-death") - suite.Require().NoError(a.Prepare(cfg)) + suite.Require().NoError(a.Prepare()) suite.Equal(fmt.Sprintf("Generated command: '%s --debug "+ "repo add edeath https://github.com/the_bug/e-death'\n", helmBin), stderr.String()) } diff --git a/internal/run/config.go b/internal/run/config.go index b237f18..7a20862 100644 --- a/internal/run/config.go +++ b/internal/run/config.go @@ -1,13 +1,22 @@ package run import ( + "github.com/pelotech/drone-helm3/internal/env" "io" ) -// Config contains configuration applicable to all helm commands -type Config struct { - Debug bool - Namespace string - Stdout io.Writer - Stderr io.Writer +type config struct { + debug bool + namespace string + stdout io.Writer + stderr io.Writer +} + +func newConfig(cfg env.Config) *config { + return &config{ + debug: cfg.Debug, + namespace: cfg.Namespace, + stdout: cfg.Stdout, + stderr: cfg.Stderr, + } } diff --git a/internal/run/config_test.go b/internal/run/config_test.go new file mode 100644 index 0000000..fd781bd --- /dev/null +++ b/internal/run/config_test.go @@ -0,0 +1,35 @@ +package run + +import ( + "github.com/pelotech/drone-helm3/internal/env" + "github.com/stretchr/testify/suite" + "strings" + "testing" +) + +type ConfigTestSuite struct { + suite.Suite +} + +func TestConfigTestSuite(t *testing.T) { + suite.Run(t, new(ConfigTestSuite)) +} + +func (suite *ConfigTestSuite) TestNewConfig() { + stdout := &strings.Builder{} + stderr := &strings.Builder{} + envCfg := env.Config{ + Namespace: "private", + Debug: true, + Stdout: stdout, + Stderr: stderr, + } + cfg := newConfig(envCfg) + suite.Require().NotNil(cfg) + suite.Equal(&config{ + namespace: "private", + debug: true, + stdout: stdout, + stderr: stderr, + }, cfg) +} diff --git a/internal/run/depupdate.go b/internal/run/depupdate.go index 5f3f189..667a429 100644 --- a/internal/run/depupdate.go +++ b/internal/run/depupdate.go @@ -7,14 +7,16 @@ import ( // DepUpdate is an execution step that calls `helm dependency update` when executed. type DepUpdate struct { - Chart string + *config + chart string cmd cmd } // NewDepUpdate creates a DepUpdate using fields from the given Config. No validation is performed at this time. func NewDepUpdate(cfg env.Config) *DepUpdate { return &DepUpdate{ - Chart: cfg.Chart, + config: newConfig(cfg), + chart: cfg.Chart, } } @@ -24,28 +26,28 @@ func (d *DepUpdate) Execute() error { } // Prepare gets the DepUpdate ready to execute. -func (d *DepUpdate) Prepare(cfg Config) error { - if d.Chart == "" { +func (d *DepUpdate) Prepare() error { + if d.chart == "" { return fmt.Errorf("chart is required") } args := make([]string, 0) - if cfg.Namespace != "" { - args = append(args, "--namespace", cfg.Namespace) + if d.namespace != "" { + args = append(args, "--namespace", d.namespace) } - if cfg.Debug { + if d.debug { args = append(args, "--debug") } - args = append(args, "dependency", "update", d.Chart) + args = append(args, "dependency", "update", d.chart) d.cmd = command(helmBin, args...) - d.cmd.Stdout(cfg.Stdout) - d.cmd.Stderr(cfg.Stderr) + d.cmd.Stdout(d.stdout) + d.cmd.Stderr(d.stderr) - if cfg.Debug { - fmt.Fprintf(cfg.Stderr, "Generated command: '%s'\n", d.cmd.String()) + if d.debug { + fmt.Fprintf(d.stderr, "Generated command: '%s'\n", d.cmd.String()) } return nil diff --git a/internal/run/depupdate_test.go b/internal/run/depupdate_test.go index f0a7e1a..73c3563 100644 --- a/internal/run/depupdate_test.go +++ b/internal/run/depupdate_test.go @@ -37,7 +37,7 @@ func (suite *DepUpdateTestSuite) TestNewDepUpdate() { Chart: "scatterplot", } d := NewDepUpdate(cfg) - suite.Equal("scatterplot", d.Chart) + suite.Equal("scatterplot", d.chart) } func (suite *DepUpdateTestSuite) TestPrepareAndExecute() { @@ -45,7 +45,8 @@ func (suite *DepUpdateTestSuite) TestPrepareAndExecute() { stdout := strings.Builder{} stderr := strings.Builder{} - cfg := Config{ + cfg := env.Config{ + Chart: "your_top_songs_2019", Stdout: &stdout, Stderr: &stderr, } @@ -64,19 +65,18 @@ func (suite *DepUpdateTestSuite) TestPrepareAndExecute() { Run(). Times(1) - d := DepUpdate{ - Chart: "your_top_songs_2019", - } + d := NewDepUpdate(cfg) - suite.Require().NoError(d.Prepare(cfg)) + suite.Require().NoError(d.Prepare()) suite.NoError(d.Execute()) } func (suite *DepUpdateTestSuite) TestPrepareNamespaceFlag() { defer suite.ctrl.Finish() - cfg := Config{ + cfg := env.Config{ Namespace: "spotify", + Chart: "your_top_songs_2019", } command = func(path string, args ...string) cmd { @@ -87,11 +87,9 @@ func (suite *DepUpdateTestSuite) TestPrepareNamespaceFlag() { suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() - d := DepUpdate{ - Chart: "your_top_songs_2019", - } + d := NewDepUpdate(cfg) - suite.Require().NoError(d.Prepare(cfg)) + suite.Require().NoError(d.Prepare()) } func (suite *DepUpdateTestSuite) TestPrepareDebugFlag() { @@ -99,7 +97,8 @@ func (suite *DepUpdateTestSuite) TestPrepareDebugFlag() { stdout := strings.Builder{} stderr := strings.Builder{} - cfg := Config{ + cfg := env.Config{ + Chart: "your_top_songs_2019", Debug: true, Stdout: &stdout, Stderr: &stderr, @@ -115,11 +114,9 @@ func (suite *DepUpdateTestSuite) TestPrepareDebugFlag() { suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() - d := DepUpdate{ - Chart: "your_top_songs_2019", - } + d := NewDepUpdate(cfg) - suite.Require().NoError(d.Prepare(cfg)) + suite.Require().NoError(d.Prepare()) want := fmt.Sprintf("Generated command: '%s --debug dependency update your_top_songs_2019'\n", helmBin) suite.Equal(want, stderr.String()) @@ -127,11 +124,11 @@ func (suite *DepUpdateTestSuite) TestPrepareDebugFlag() { } func (suite *DepUpdateTestSuite) TestPrepareChartRequired() { - d := DepUpdate{} + d := NewDepUpdate(env.Config{}) suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() - err := d.Prepare(Config{}) + err := d.Prepare() suite.EqualError(err, "chart is required") } diff --git a/internal/run/help.go b/internal/run/help.go index 69666d3..79f4101 100644 --- a/internal/run/help.go +++ b/internal/run/help.go @@ -7,14 +7,16 @@ import ( // Help is a step in a helm Plan that calls `helm help`. type Help struct { - HelmCommand string + *config + helmCommand string cmd cmd } // NewHelp creates a Help using fields from the given Config. No validation is performed at this time. func NewHelp(cfg env.Config) *Help { return &Help{ - HelmCommand: cfg.Command, + config: newConfig(cfg), + helmCommand: cfg.Command, } } @@ -24,25 +26,25 @@ func (h *Help) Execute() error { return fmt.Errorf("while running '%s': %w", h.cmd.String(), err) } - if h.HelmCommand == "help" { + if h.helmCommand == "help" { return nil } - return fmt.Errorf("unknown command '%s'", h.HelmCommand) + return fmt.Errorf("unknown command '%s'", h.helmCommand) } // Prepare gets the Help ready to execute. -func (h *Help) Prepare(cfg Config) error { +func (h *Help) Prepare() error { args := []string{"help"} - if cfg.Debug { + if h.debug { args = append([]string{"--debug"}, args...) } h.cmd = command(helmBin, args...) - h.cmd.Stdout(cfg.Stdout) - h.cmd.Stderr(cfg.Stderr) + h.cmd.Stdout(h.stdout) + h.cmd.Stderr(h.stderr) - if cfg.Debug { - fmt.Fprintf(cfg.Stderr, "Generated command: '%s'\n", h.cmd.String()) + if h.debug { + fmt.Fprintf(h.stderr, "Generated command: '%s'\n", h.cmd.String()) } return nil diff --git a/internal/run/help_test.go b/internal/run/help_test.go index 1bad0bf..bdc9dbe 100644 --- a/internal/run/help_test.go +++ b/internal/run/help_test.go @@ -24,7 +24,7 @@ func (suite *HelpTestSuite) TestNewHelp() { } help := NewHelp(cfg) suite.Require().NotNil(help) - suite.Equal("everybody dance NOW!!", help.HelmCommand) + suite.Equal("everybody dance NOW!!", help.helmCommand) } func (suite *HelpTestSuite) TestPrepare() { @@ -49,13 +49,13 @@ func (suite *HelpTestSuite) TestPrepare() { mCmd.EXPECT(). Stderr(&stderr) - cfg := Config{ + cfg := env.Config{ Stdout: &stdout, Stderr: &stderr, } - h := Help{} - err := h.Prepare(cfg) + h := NewHelp(cfg) + err := h.Prepare() suite.NoError(err) } @@ -63,38 +63,30 @@ func (suite *HelpTestSuite) TestExecute() { ctrl := gomock.NewController(suite.T()) defer ctrl.Finish() mCmd := NewMockcmd(ctrl) - originalCommand := command - command = func(_ string, _ ...string) cmd { - return mCmd - } - defer func() { command = originalCommand }() mCmd.EXPECT(). Run(). Times(2) - help := Help{ - HelmCommand: "help", - cmd: mCmd, - } + help := NewHelp(env.Config{Command: "help"}) + help.cmd = mCmd suite.NoError(help.Execute()) - help.HelmCommand = "get down on friday" + help.helmCommand = "get down on friday" suite.EqualError(help.Execute(), "unknown command 'get down on friday'") } func (suite *HelpTestSuite) TestPrepareDebugFlag() { - help := Help{} - stdout := strings.Builder{} stderr := strings.Builder{} - cfg := Config{ + cfg := env.Config{ Debug: true, Stdout: &stdout, Stderr: &stderr, } - help.Prepare(cfg) + help := NewHelp(cfg) + help.Prepare() want := fmt.Sprintf("Generated command: '%s --debug help'\n", helmBin) suite.Equal(want, stderr.String()) diff --git a/internal/run/initkube.go b/internal/run/initkube.go index b6c0440..bf4de3f 100644 --- a/internal/run/initkube.go +++ b/internal/run/initkube.go @@ -11,10 +11,9 @@ import ( // InitKube is a step in a helm Plan that initializes the kubernetes config file. type InitKube struct { + *config templateFilename string configFilename string - debug bool - stderr io.Writer template *template.Template configFile io.WriteCloser values kubeValues @@ -32,6 +31,7 @@ type kubeValues struct { // NewInitKube creates a InitKube using the given Config and filepaths. No validation is performed at this time. func NewInitKube(cfg env.Config, templateFile, configFile string) *InitKube { return &InitKube{ + config: newConfig(cfg), values: kubeValues{ SkipTLSVerify: cfg.SkipTLSVerify, Certificate: cfg.Certificate, @@ -42,8 +42,6 @@ func NewInitKube(cfg env.Config, templateFile, configFile string) *InitKube { }, templateFilename: templateFile, configFilename: configFile, - debug: cfg.Debug, - stderr: cfg.Stderr, } } @@ -57,7 +55,7 @@ func (i *InitKube) Execute() error { } // Prepare ensures all required configuration is present and that the config file is writable. -func (i *InitKube) Prepare(cfg Config) error { +func (i *InitKube) Prepare() error { var err error if i.values.APIServer == "" { diff --git a/internal/run/initkube_test.go b/internal/run/initkube_test.go index c34723f..8e13552 100644 --- a/internal/run/initkube_test.go +++ b/internal/run/initkube_test.go @@ -32,19 +32,16 @@ func (suite *InitKubeTestSuite) TestNewInitKube() { } init := NewInitKube(cfg, "conf.tpl", "conf.yml") - suite.Equal(&InitKube{ - values: kubeValues{ - SkipTLSVerify: true, - Certificate: "cHJvY2xhaW1zIHdvbmRlcmZ1bCBmcmllbmRzaGlw", - APIServer: "98.765.43.21", - ServiceAccount: "greathelm", - Token: "b2YgbXkgYWZmZWN0aW9u", - }, - templateFilename: "conf.tpl", - configFilename: "conf.yml", - debug: true, - stderr: cfg.Stderr, - }, init) + suite.Equal(kubeValues{ + SkipTLSVerify: true, + Certificate: "cHJvY2xhaW1zIHdvbmRlcmZ1bCBmcmllbmRzaGlw", + APIServer: "98.765.43.21", + ServiceAccount: "greathelm", + Token: "b2YgbXkgYWZmZWN0aW9u", + }, init.values) + suite.Equal("conf.tpl", init.templateFilename) + suite.Equal("conf.yml", init.configFilename) + suite.NotNil(init.config) } func (suite *InitKubeTestSuite) TestPrepareExecute() { @@ -59,18 +56,14 @@ namespace: {{ .Namespace }} defer os.Remove(configFile.Name()) suite.Require().Nil(err) - init := InitKube{ - values: kubeValues{ - APIServer: "Sysadmin", - Certificate: "CCNA", - Token: "Aspire virtual currency", - Namespace: "Cisco", - }, - templateFilename: templateFile.Name(), - configFilename: configFile.Name(), + cfg := env.Config{ + APIServer: "Sysadmin", + Certificate: "CCNA", + KubeToken: "Aspire virtual currency", + Namespace: "Cisco", } - cfg := Config{} - err = init.Prepare(cfg) + init := NewInitKube(cfg, templateFile.Name(), configFile.Name()) + err = init.Prepare() suite.Require().Nil(err) suite.IsType(&template.Template{}, init.template) @@ -94,19 +87,15 @@ func (suite *InitKubeTestSuite) TestExecuteGeneratesConfig() { defer os.Remove(configFile.Name()) suite.Require().NoError(err) - cfg := Config{} - init := InitKube{ - configFilename: configFile.Name(), - templateFilename: "../../assets/kubeconfig.tpl", // the actual kubeconfig template - values: kubeValues{ - APIServer: "https://kube.cluster/peanut", - ServiceAccount: "chef", - Token: "eWVhaCB3ZSB0b2tpbic=", - Certificate: "d293LCB5b3UgYXJlIHNvIGNvb2wgZm9yIHNtb2tpbmcgd2VlZCDwn5mE", - Namespace: "marshmallow", - }, + cfg := env.Config{ + APIServer: "https://kube.cluster/peanut", + ServiceAccount: "chef", + KubeToken: "eWVhaCB3ZSB0b2tpbic=", + Certificate: "d293LCB5b3UgYXJlIHNvIGNvb2wgZm9yIHNtb2tpbmcgd2VlZCDwn5mE", + Namespace: "marshmallow", } - suite.Require().NoError(init.Prepare(cfg)) + init := NewInitKube(cfg, "../../assets/kubeconfig.tpl", configFile.Name()) // the actual kubeconfig template + suite.Require().NoError(init.Prepare()) suite.Require().NoError(init.Execute()) contents, err := ioutil.ReadFile(configFile.Name()) @@ -133,7 +122,7 @@ func (suite *InitKubeTestSuite) TestExecuteGeneratesConfig() { init.values.SkipTLSVerify = true init.values.Certificate = "" - suite.Require().NoError(init.Prepare(cfg)) + suite.Require().NoError(init.Prepare()) suite.Require().NoError(init.Execute()) contents, err = ioutil.ReadFile(configFile.Name()) suite.Require().NoError(err) @@ -148,29 +137,25 @@ func (suite *InitKubeTestSuite) TestPrepareParseError() { defer os.Remove(templateFile.Name()) suite.Require().Nil(err) - init := InitKube{ - values: kubeValues{ - APIServer: "Sysadmin", - Certificate: "CCNA", - Token: "Aspire virtual currency", - }, - templateFilename: templateFile.Name(), + cfg := env.Config{ + APIServer: "Sysadmin", + Certificate: "CCNA", + KubeToken: "Aspire virtual currency", } - err = init.Prepare(Config{}) + init := NewInitKube(cfg, templateFile.Name(), "") + err = init.Prepare() suite.Error(err) suite.Regexp("could not load kubeconfig .* function .* not defined", err) } func (suite *InitKubeTestSuite) TestPrepareNonexistentTemplateFile() { - init := InitKube{ - values: kubeValues{ - APIServer: "Sysadmin", - Certificate: "CCNA", - Token: "Aspire virtual currency", - }, - templateFilename: "/usr/foreign/exclude/kubeprofig.tpl", + cfg := env.Config{ + APIServer: "Sysadmin", + Certificate: "CCNA", + KubeToken: "Aspire virtual currency", } - err := init.Prepare(Config{}) + init := NewInitKube(cfg, "/usr/foreign/exclude/kubeprofig.tpl", "") + err := init.Prepare() suite.Error(err) suite.Regexp("could not load kubeconfig .* no such file or directory", err) } @@ -179,18 +164,14 @@ func (suite *InitKubeTestSuite) TestPrepareCannotOpenDestinationFile() { templateFile, err := tempfile("kubeconfig********.yml.tpl", "hurgity burgity") defer os.Remove(templateFile.Name()) suite.Require().Nil(err) - init := InitKube{ - values: kubeValues{ - APIServer: "Sysadmin", - Certificate: "CCNA", - Token: "Aspire virtual currency", - }, - templateFilename: templateFile.Name(), - configFilename: "/usr/foreign/exclude/kubeprofig", + cfg := env.Config{ + APIServer: "Sysadmin", + Certificate: "CCNA", + KubeToken: "Aspire virtual currency", } + init := NewInitKube(cfg, templateFile.Name(), "/usr/foreign/exclude/kubeprofig") - cfg := Config{} - err = init.Prepare(cfg) + err = init.Prepare() suite.Error(err) suite.Regexp("could not open .* for writing: .* no such file or directory", err) } @@ -205,26 +186,21 @@ func (suite *InitKubeTestSuite) TestPrepareRequiredConfig() { suite.Require().Nil(err) // initial config with all required fields present - init := InitKube{ - values: kubeValues{ - APIServer: "Sysadmin", - Certificate: "CCNA", - Token: "Aspire virtual currency", - }, - templateFilename: templateFile.Name(), - configFilename: configFile.Name(), + cfg := env.Config{ + APIServer: "Sysadmin", + Certificate: "CCNA", + KubeToken: "Aspire virtual currency", } - cfg := Config{} - - suite.NoError(init.Prepare(cfg)) // consistency check; we should be starting in a happy state + init := NewInitKube(cfg, templateFile.Name(), configFile.Name()) + suite.NoError(init.Prepare()) // consistency check; we should be starting in a happy state init.values.APIServer = "" - suite.Error(init.Prepare(cfg), "APIServer should be required.") + suite.Error(init.Prepare(), "APIServer should be required.") init.values.APIServer = "Sysadmin" init.values.Token = "" - suite.Error(init.Prepare(cfg), "Token should be required.") + suite.Error(init.Prepare(), "Token should be required.") } func (suite *InitKubeTestSuite) TestPrepareDefaultsServiceAccount() { @@ -236,19 +212,14 @@ func (suite *InitKubeTestSuite) TestPrepareDefaultsServiceAccount() { defer os.Remove(configFile.Name()) suite.Require().Nil(err) - init := InitKube{ - values: kubeValues{ - APIServer: "Sysadmin", - Certificate: "CCNA", - Token: "Aspire virtual currency", - }, - templateFilename: templateFile.Name(), - configFilename: configFile.Name(), + cfg := env.Config{ + APIServer: "Sysadmin", + Certificate: "CCNA", + KubeToken: "Aspire virtual currency", } + init := NewInitKube(cfg, templateFile.Name(), configFile.Name()) - cfg := Config{} - - init.Prepare(cfg) + init.Prepare() suite.Equal("helm", init.values.ServiceAccount) } @@ -270,14 +241,8 @@ func (suite *InitKubeTestSuite) TestDebugOutput() { Stdout: stdout, Stderr: stderr, } - runCfg := Config{ - Debug: true, - Stdout: stdout, - Stderr: stderr, - } - init := NewInitKube(cfg, templateFile.Name(), configFile.Name()) - suite.NoError(init.Prepare(runCfg)) + suite.NoError(init.Prepare()) suite.Contains(stderr.String(), fmt.Sprintf("loading kubeconfig template from %s\n", templateFile.Name())) suite.Contains(stderr.String(), fmt.Sprintf("truncating kubeconfig file at %s\n", configFile.Name())) diff --git a/internal/run/lint.go b/internal/run/lint.go index e6b550c..242e113 100644 --- a/internal/run/lint.go +++ b/internal/run/lint.go @@ -7,22 +7,24 @@ import ( // Lint is an execution step that calls `helm lint` when executed. type Lint struct { - Chart string - Values string - StringValues string - ValuesFiles []string - Strict bool + *config + chart string + values string + stringValues string + valuesFiles []string + strict bool cmd cmd } // NewLint creates a Lint using fields from the given Config. No validation is performed at this time. func NewLint(cfg env.Config) *Lint { return &Lint{ - Chart: cfg.Chart, - Values: cfg.Values, - StringValues: cfg.StringValues, - ValuesFiles: cfg.ValuesFiles, - Strict: cfg.LintStrictly, + config: newConfig(cfg), + chart: cfg.Chart, + values: cfg.Values, + stringValues: cfg.StringValues, + valuesFiles: cfg.ValuesFiles, + strict: cfg.LintStrictly, } } @@ -32,43 +34,43 @@ func (l *Lint) Execute() error { } // Prepare gets the Lint ready to execute. -func (l *Lint) Prepare(cfg Config) error { - if l.Chart == "" { +func (l *Lint) Prepare() error { + if l.chart == "" { return fmt.Errorf("chart is required") } args := make([]string, 0) - if cfg.Namespace != "" { - args = append(args, "--namespace", cfg.Namespace) + if l.namespace != "" { + args = append(args, "--namespace", l.namespace) } - if cfg.Debug { + if l.debug { args = append(args, "--debug") } args = append(args, "lint") - if l.Values != "" { - args = append(args, "--set", l.Values) + if l.values != "" { + args = append(args, "--set", l.values) } - if l.StringValues != "" { - args = append(args, "--set-string", l.StringValues) + if l.stringValues != "" { + args = append(args, "--set-string", l.stringValues) } - for _, vFile := range l.ValuesFiles { + for _, vFile := range l.valuesFiles { args = append(args, "--values", vFile) } - if l.Strict { + if l.strict { args = append(args, "--strict") } - args = append(args, l.Chart) + args = append(args, l.chart) l.cmd = command(helmBin, args...) - l.cmd.Stdout(cfg.Stdout) - l.cmd.Stderr(cfg.Stderr) + l.cmd.Stdout(l.stdout) + l.cmd.Stderr(l.stderr) - if cfg.Debug { - fmt.Fprintf(cfg.Stderr, "Generated command: '%s'\n", l.cmd.String()) + if l.debug { + fmt.Fprintf(l.stderr, "Generated command: '%s'\n", l.cmd.String()) } return nil diff --git a/internal/run/lint_test.go b/internal/run/lint_test.go index e7469de..1241dfd 100644 --- a/internal/run/lint_test.go +++ b/internal/run/lint_test.go @@ -42,13 +42,12 @@ func (suite *LintTestSuite) TestNewLint() { } lint := NewLint(cfg) suite.Require().NotNil(lint) - suite.Equal(&Lint{ - Chart: "./flow", - Values: "steadfastness,forthrightness", - StringValues: "tensile_strength,flexibility", - ValuesFiles: []string{"/root/price_inventory.yml"}, - Strict: true, - }, lint) + suite.Equal("./flow", lint.chart) + suite.Equal("steadfastness,forthrightness", lint.values) + suite.Equal("tensile_strength,flexibility", lint.stringValues) + suite.Equal([]string{"/root/price_inventory.yml"}, lint.valuesFiles) + suite.Equal(true, lint.strict) + suite.NotNil(lint.config) } func (suite *LintTestSuite) TestPrepareAndExecute() { @@ -57,13 +56,12 @@ func (suite *LintTestSuite) TestPrepareAndExecute() { stdout := strings.Builder{} stderr := strings.Builder{} - l := Lint{ - Chart: "./epic/mychart", - } - cfg := Config{ + cfg := env.Config{ + Chart: "./epic/mychart", Stdout: &stdout, Stderr: &stderr, } + l := NewLint(cfg) command = func(path string, args ...string) cmd { suite.Equal(helmBin, path) @@ -72,6 +70,7 @@ func (suite *LintTestSuite) TestPrepareAndExecute() { return suite.mockCmd } + suite.mockCmd.EXPECT().String().AnyTimes() suite.mockCmd.EXPECT(). Stdout(&stdout) suite.mockCmd.EXPECT(). @@ -80,7 +79,7 @@ func (suite *LintTestSuite) TestPrepareAndExecute() { Run(). Times(1) - err := l.Prepare(cfg) + err := l.Prepare() suite.Require().Nil(err) l.Execute() } @@ -90,25 +89,22 @@ func (suite *LintTestSuite) TestPrepareRequiresChart() { suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() - cfg := Config{} - l := Lint{} - - err := l.Prepare(cfg) + l := NewLint(env.Config{}) + err := l.Prepare() suite.EqualError(err, "chart is required", "Chart should be mandatory") } func (suite *LintTestSuite) TestPrepareWithLintFlags() { defer suite.ctrl.Finish() - cfg := Config{} - - l := Lint{ + cfg := env.Config{ Chart: "./uk/top_40", Values: "width=5", StringValues: "version=2.0", ValuesFiles: []string{"/usr/local/underrides", "/usr/local/overrides"}, - Strict: true, + LintStrictly: true, } + l := NewLint(cfg) command = func(path string, args ...string) cmd { suite.Equal(helmBin, path) @@ -125,8 +121,9 @@ func (suite *LintTestSuite) TestPrepareWithLintFlags() { suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() + suite.mockCmd.EXPECT().String().AnyTimes() - err := l.Prepare(cfg) + err := l.Prepare() suite.Require().Nil(err) } @@ -135,14 +132,12 @@ func (suite *LintTestSuite) TestPrepareWithDebugFlag() { stderr := strings.Builder{} - cfg := Config{ + cfg := env.Config{ Debug: true, Stderr: &stderr, + Chart: "./scotland/top_40", } - - l := Lint{ - Chart: "./scotland/top_40", - } + l := NewLint(cfg) command = func(path string, args ...string) cmd { suite.mockCmd.EXPECT(). @@ -155,7 +150,7 @@ func (suite *LintTestSuite) TestPrepareWithDebugFlag() { suite.mockCmd.EXPECT().Stdout(gomock.Any()) suite.mockCmd.EXPECT().Stderr(&stderr) - err := l.Prepare(cfg) + err := l.Prepare() suite.Require().Nil(err) want := fmt.Sprintf("Generated command: '%s --debug lint ./scotland/top_40'\n", helmBin) @@ -165,13 +160,11 @@ func (suite *LintTestSuite) TestPrepareWithDebugFlag() { func (suite *LintTestSuite) TestPrepareWithNamespaceFlag() { defer suite.ctrl.Finish() - cfg := Config{ + cfg := env.Config{ Namespace: "table-service", + Chart: "./wales/top_40", } - - l := Lint{ - Chart: "./wales/top_40", - } + l := NewLint(cfg) actual := []string{} command = func(path string, args ...string) cmd { @@ -182,7 +175,7 @@ func (suite *LintTestSuite) TestPrepareWithNamespaceFlag() { suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() - err := l.Prepare(cfg) + err := l.Prepare() suite.Require().Nil(err) expected := []string{"--namespace", "table-service", "lint", "./wales/top_40"} diff --git a/internal/run/uninstall.go b/internal/run/uninstall.go index 07fd820..186cb16 100644 --- a/internal/run/uninstall.go +++ b/internal/run/uninstall.go @@ -7,18 +7,20 @@ import ( // Uninstall is an execution step that calls `helm uninstall` when executed. type Uninstall struct { - Release string - DryRun bool - KeepHistory bool + *config + release string + dryRun bool + keepHistory bool cmd cmd } // NewUninstall creates an Uninstall using fields from the given Config. No validation is performed at this time. func NewUninstall(cfg env.Config) *Uninstall { return &Uninstall{ - Release: cfg.Release, - DryRun: cfg.DryRun, - KeepHistory: cfg.KeepHistory, + config: newConfig(cfg), + release: cfg.Release, + dryRun: cfg.DryRun, + keepHistory: cfg.KeepHistory, } } @@ -28,37 +30,37 @@ func (u *Uninstall) Execute() error { } // Prepare gets the Uninstall ready to execute. -func (u *Uninstall) Prepare(cfg Config) error { - if u.Release == "" { +func (u *Uninstall) Prepare() error { + if u.release == "" { return fmt.Errorf("release is required") } args := make([]string, 0) - if cfg.Namespace != "" { - args = append(args, "--namespace", cfg.Namespace) + if u.namespace != "" { + args = append(args, "--namespace", u.namespace) } - if cfg.Debug { + if u.debug { args = append(args, "--debug") } args = append(args, "uninstall") - if u.DryRun { + if u.dryRun { args = append(args, "--dry-run") } - if u.KeepHistory { + if u.keepHistory { args = append(args, "--keep-history") } - args = append(args, u.Release) + args = append(args, u.release) u.cmd = command(helmBin, args...) - u.cmd.Stdout(cfg.Stdout) - u.cmd.Stderr(cfg.Stderr) + u.cmd.Stdout(u.stdout) + u.cmd.Stderr(u.stderr) - if cfg.Debug { - fmt.Fprintf(cfg.Stderr, "Generated command: '%s'\n", u.cmd.String()) + if u.debug { + fmt.Fprintf(u.stderr, "Generated command: '%s'\n", u.cmd.String()) } return nil diff --git a/internal/run/uninstall_test.go b/internal/run/uninstall_test.go index 6bac55c..69bde6e 100644 --- a/internal/run/uninstall_test.go +++ b/internal/run/uninstall_test.go @@ -43,19 +43,19 @@ func (suite *UninstallTestSuite) TestNewUninstall() { KeepHistory: true, } u := NewUninstall(cfg) - suite.Equal(&Uninstall{ - Release: "jetta_id_love_to_change_the_world", - DryRun: true, - KeepHistory: true, - }, u) + suite.Equal("jetta_id_love_to_change_the_world", u.release) + suite.Equal(true, u.dryRun) + suite.Equal(true, u.keepHistory) + suite.NotNil(u.config) } func (suite *UninstallTestSuite) TestPrepareAndExecute() { defer suite.ctrl.Finish() - u := Uninstall{ + cfg := env.Config{ Release: "zayde_wølf_king", } + u := NewUninstall(cfg) actual := []string{} command = func(path string, args ...string) cmd { @@ -73,8 +73,7 @@ func (suite *UninstallTestSuite) TestPrepareAndExecute() { Run(). Times(1) - cfg := Config{} - suite.NoError(u.Prepare(cfg)) + suite.NoError(u.Prepare()) expected := []string{"uninstall", "zayde_wølf_king"} suite.Equal(expected, actual) @@ -82,60 +81,58 @@ func (suite *UninstallTestSuite) TestPrepareAndExecute() { } func (suite *UninstallTestSuite) TestPrepareDryRunFlag() { - u := Uninstall{ + cfg := env.Config{ Release: "firefox_ak_wildfire", DryRun: true, } - cfg := Config{} + u := NewUninstall(cfg) suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() - suite.NoError(u.Prepare(cfg)) + suite.NoError(u.Prepare()) expected := []string{"uninstall", "--dry-run", "firefox_ak_wildfire"} suite.Equal(expected, suite.actualArgs) } func (suite *UninstallTestSuite) TestPrepareKeepHistoryFlag() { - u := Uninstall{ + cfg := env.Config{ Release: "perturbator_sentient", KeepHistory: true, } - cfg := Config{} + u := NewUninstall(cfg) suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() - suite.NoError(u.Prepare(cfg)) + suite.NoError(u.Prepare()) expected := []string{"uninstall", "--keep-history", "perturbator_sentient"} suite.Equal(expected, suite.actualArgs) } func (suite *UninstallTestSuite) TestPrepareNamespaceFlag() { - u := Uninstall{ - Release: "carly_simon_run_away_with_me", - } - cfg := Config{ + cfg := env.Config{ + Release: "carly_simon_run_away_with_me", Namespace: "emotion", } + u := NewUninstall(cfg) suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() - suite.NoError(u.Prepare(cfg)) + suite.NoError(u.Prepare()) expected := []string{"--namespace", "emotion", "uninstall", "carly_simon_run_away_with_me"} suite.Equal(expected, suite.actualArgs) } func (suite *UninstallTestSuite) TestPrepareDebugFlag() { - u := Uninstall{ - Release: "just_a_band_huff_and_puff", - } stderr := strings.Builder{} - cfg := Config{ - Debug: true, - Stderr: &stderr, + cfg := env.Config{ + Release: "just_a_band_huff_and_puff", + Debug: true, + Stderr: &stderr, } + u := NewUninstall(cfg) command = func(path string, args ...string) cmd { suite.mockCmd.EXPECT(). @@ -148,7 +145,7 @@ func (suite *UninstallTestSuite) TestPrepareDebugFlag() { suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() suite.mockCmd.EXPECT().Stderr(&stderr).AnyTimes() - suite.NoError(u.Prepare(cfg)) + suite.NoError(u.Prepare()) suite.Equal(fmt.Sprintf("Generated command: '%s --debug "+ "uninstall just_a_band_huff_and_puff'\n", helmBin), stderr.String()) } @@ -158,7 +155,7 @@ func (suite *UninstallTestSuite) TestPrepareRequiresRelease() { suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() - u := Uninstall{} - err := u.Prepare(Config{}) + u := NewUninstall(env.Config{}) + err := u.Prepare() suite.EqualError(err, "release is required", "Uninstall.Release should be mandatory") } diff --git a/internal/run/upgrade.go b/internal/run/upgrade.go index 317f065..59af6a6 100644 --- a/internal/run/upgrade.go +++ b/internal/run/upgrade.go @@ -7,20 +7,21 @@ import ( // Upgrade is an execution step that calls `helm upgrade` when executed. type Upgrade struct { - Chart string - Release string + *config + chart string + release string - ChartVersion string - DryRun bool - Wait bool - Values string - StringValues string - ValuesFiles []string - ReuseValues bool - Timeout string - Force bool - Atomic bool - CleanupOnFail bool + chartVersion string + dryRun bool + wait bool + values string + stringValues string + valuesFiles []string + reuseValues bool + timeout string + force bool + atomic bool + cleanupOnFail bool cmd cmd } @@ -28,19 +29,20 @@ type Upgrade struct { // NewUpgrade creates an Upgrade using fields from the given Config. No validation is performed at this time. func NewUpgrade(cfg env.Config) *Upgrade { return &Upgrade{ - Chart: cfg.Chart, - Release: cfg.Release, - ChartVersion: cfg.ChartVersion, - DryRun: cfg.DryRun, - Wait: cfg.Wait, - Values: cfg.Values, - StringValues: cfg.StringValues, - ValuesFiles: cfg.ValuesFiles, - ReuseValues: cfg.ReuseValues, - Timeout: cfg.Timeout, - Force: cfg.Force, - Atomic: cfg.AtomicUpgrade, - CleanupOnFail: cfg.CleanupOnFail, + config: newConfig(cfg), + chart: cfg.Chart, + release: cfg.Release, + chartVersion: cfg.ChartVersion, + dryRun: cfg.DryRun, + wait: cfg.Wait, + values: cfg.Values, + stringValues: cfg.StringValues, + valuesFiles: cfg.ValuesFiles, + reuseValues: cfg.ReuseValues, + timeout: cfg.Timeout, + force: cfg.Force, + atomic: cfg.AtomicUpgrade, + cleanupOnFail: cfg.CleanupOnFail, } } @@ -50,66 +52,66 @@ func (u *Upgrade) Execute() error { } // Prepare gets the Upgrade ready to execute. -func (u *Upgrade) Prepare(cfg Config) error { - if u.Chart == "" { +func (u *Upgrade) Prepare() error { + if u.chart == "" { return fmt.Errorf("chart is required") } - if u.Release == "" { + if u.release == "" { return fmt.Errorf("release is required") } args := make([]string, 0) - if cfg.Namespace != "" { - args = append(args, "--namespace", cfg.Namespace) + if u.namespace != "" { + args = append(args, "--namespace", u.namespace) } - if cfg.Debug { + if u.debug { args = append(args, "--debug") } args = append(args, "upgrade", "--install") - if u.ChartVersion != "" { - args = append(args, "--version", u.ChartVersion) + if u.chartVersion != "" { + args = append(args, "--version", u.chartVersion) } - if u.DryRun { + if u.dryRun { args = append(args, "--dry-run") } - if u.Wait { + if u.wait { args = append(args, "--wait") } - if u.ReuseValues { + if u.reuseValues { args = append(args, "--reuse-values") } - if u.Timeout != "" { - args = append(args, "--timeout", u.Timeout) + if u.timeout != "" { + args = append(args, "--timeout", u.timeout) } - if u.Force { + if u.force { args = append(args, "--force") } - if u.Atomic { + if u.atomic { args = append(args, "--atomic") } - if u.CleanupOnFail { + if u.cleanupOnFail { args = append(args, "--cleanup-on-fail") } - if u.Values != "" { - args = append(args, "--set", u.Values) + if u.values != "" { + args = append(args, "--set", u.values) } - if u.StringValues != "" { - args = append(args, "--set-string", u.StringValues) + if u.stringValues != "" { + args = append(args, "--set-string", u.stringValues) } - for _, vFile := range u.ValuesFiles { + for _, vFile := range u.valuesFiles { args = append(args, "--values", vFile) } - args = append(args, u.Release, u.Chart) + args = append(args, u.release, u.chart) u.cmd = command(helmBin, args...) - u.cmd.Stdout(cfg.Stdout) - u.cmd.Stderr(cfg.Stderr) + u.cmd.Stdout(u.stdout) + u.cmd.Stderr(u.stderr) - if cfg.Debug { - fmt.Fprintf(cfg.Stderr, "Generated command: '%s'\n", u.cmd.String()) + if u.debug { + fmt.Fprintf(u.stderr, "Generated command: '%s'\n", u.cmd.String()) } return nil diff --git a/internal/run/upgrade_test.go b/internal/run/upgrade_test.go index 71b92cb..a73b062 100644 --- a/internal/run/upgrade_test.go +++ b/internal/run/upgrade_test.go @@ -50,30 +50,30 @@ func (suite *UpgradeTestSuite) TestNewUpgrade() { } up := NewUpgrade(cfg) - suite.Equal(&Upgrade{ - Chart: cfg.Chart, - Release: cfg.Release, - ChartVersion: cfg.ChartVersion, - DryRun: true, - Wait: cfg.Wait, - Values: "steadfastness,forthrightness", - StringValues: "tensile_strength,flexibility", - ValuesFiles: []string{"/root/price_inventory.yml"}, - ReuseValues: cfg.ReuseValues, - Timeout: cfg.Timeout, - Force: cfg.Force, - Atomic: true, - CleanupOnFail: true, - }, up) + suite.Equal(cfg.Chart, up.chart) + suite.Equal(cfg.Release, up.release) + suite.Equal(cfg.ChartVersion, up.chartVersion) + suite.Equal(true, up.dryRun) + suite.Equal(cfg.Wait, up.wait) + suite.Equal("steadfastness,forthrightness", up.values) + suite.Equal("tensile_strength,flexibility", up.stringValues) + suite.Equal([]string{"/root/price_inventory.yml"}, up.valuesFiles) + suite.Equal(cfg.ReuseValues, up.reuseValues) + suite.Equal(cfg.Timeout, up.timeout) + suite.Equal(cfg.Force, up.force) + suite.Equal(true, up.atomic) + suite.Equal(true, up.cleanupOnFail) + suite.NotNil(up.config) } func (suite *UpgradeTestSuite) TestPrepareAndExecute() { defer suite.ctrl.Finish() - u := Upgrade{ + cfg := env.Config{ Chart: "at40", Release: "jonas_brothers_only_human", } + u := NewUpgrade(cfg) command = func(path string, args ...string) cmd { suite.Equal(helmBin, path) @@ -90,8 +90,7 @@ func (suite *UpgradeTestSuite) TestPrepareAndExecute() { Run(). Times(1) - cfg := Config{} - err := u.Prepare(cfg) + err := u.Prepare() suite.Require().Nil(err) u.Execute() } @@ -99,10 +98,12 @@ func (suite *UpgradeTestSuite) TestPrepareAndExecute() { func (suite *UpgradeTestSuite) TestPrepareNamespaceFlag() { defer suite.ctrl.Finish() - u := Upgrade{ - Chart: "at40", - Release: "shaed_trampoline", + cfg := env.Config{ + Namespace: "melt", + Chart: "at40", + Release: "shaed_trampoline", } + u := NewUpgrade(cfg) command = func(path string, args ...string) cmd { suite.Equal(helmBin, path) @@ -114,17 +115,14 @@ func (suite *UpgradeTestSuite) TestPrepareNamespaceFlag() { suite.mockCmd.EXPECT().Stdout(gomock.Any()) suite.mockCmd.EXPECT().Stderr(gomock.Any()) - cfg := Config{ - Namespace: "melt", - } - err := u.Prepare(cfg) + err := u.Prepare() suite.Require().Nil(err) } func (suite *UpgradeTestSuite) TestPrepareWithUpgradeFlags() { defer suite.ctrl.Finish() - u := Upgrade{ + cfg := env.Config{ Chart: "hot_ac", Release: "maroon_5_memories", ChartVersion: "radio_edit", @@ -136,11 +134,10 @@ func (suite *UpgradeTestSuite) TestPrepareWithUpgradeFlags() { ReuseValues: true, Timeout: "sit_in_the_corner", Force: true, - Atomic: true, + AtomicUpgrade: true, CleanupOnFail: true, } - - cfg := Config{} + u := NewUpgrade(cfg) command = func(path string, args ...string) cmd { suite.Equal(helmBin, path) @@ -165,7 +162,7 @@ func (suite *UpgradeTestSuite) TestPrepareWithUpgradeFlags() { suite.mockCmd.EXPECT().Stdout(gomock.Any()) suite.mockCmd.EXPECT().Stderr(gomock.Any()) - err := u.Prepare(cfg) + err := u.Prepare() suite.Require().Nil(err) } @@ -174,34 +171,30 @@ func (suite *UpgradeTestSuite) TestRequiresChartAndRelease() { suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() - u := Upgrade{ - Release: "seth_everman_unskippable_cutscene", - } + u := NewUpgrade(env.Config{}) + u.release = "seth_everman_unskippable_cutscene" - err := u.Prepare(Config{}) + err := u.Prepare() suite.EqualError(err, "chart is required", "Chart should be mandatory") - u = Upgrade{ - Chart: "billboard_top_zero", - } + u.release = "" + u.chart = "billboard_top_zero" - err = u.Prepare(Config{}) + err = u.Prepare() suite.EqualError(err, "release is required", "Release should be mandatory") } func (suite *UpgradeTestSuite) TestPrepareDebugFlag() { - u := Upgrade{ - Chart: "at40", - Release: "lewis_capaldi_someone_you_loved", - } - stdout := strings.Builder{} stderr := strings.Builder{} - cfg := Config{ - Debug: true, - Stdout: &stdout, - Stderr: &stderr, + cfg := env.Config{ + Chart: "at40", + Release: "lewis_capaldi_someone_you_loved", + Debug: true, + Stdout: &stdout, + Stderr: &stderr, } + u := NewUpgrade(cfg) command = func(path string, args ...string) cmd { suite.mockCmd.EXPECT(). @@ -216,7 +209,7 @@ func (suite *UpgradeTestSuite) TestPrepareDebugFlag() { suite.mockCmd.EXPECT(). Stderr(&stderr) - u.Prepare(cfg) + u.Prepare() want := fmt.Sprintf("Generated command: '%s --debug upgrade "+ "--install lewis_capaldi_someone_you_loved at40'\n", helmBin) From 79532e76353e2977845d071f891a426fa2968bcc Mon Sep 17 00:00:00 2001 From: Erin Call Date: Fri, 17 Jan 2020 11:12:53 -0800 Subject: [PATCH 8/8] Extract the debug/namespace flags into run.Config [#67] This is a general-purpose cleanup commit; every step except InitKube had the same six "add the --debug and --namespace flags if applicable" code. --- internal/run/addrepo.go | 10 +----- internal/run/addrepo_test.go | 40 ------------------------ internal/run/config.go | 11 +++++++ internal/run/config_test.go | 13 ++++++++ internal/run/depupdate.go | 10 +----- internal/run/depupdate_test.go | 53 -------------------------------- internal/run/help.go | 6 ++-- internal/run/help_test.go | 18 ----------- internal/run/lint.go | 10 +----- internal/run/lint_test.go | 56 ---------------------------------- internal/run/uninstall.go | 10 +----- internal/run/uninstall_test.go | 42 ------------------------- internal/run/upgrade.go | 10 +----- 13 files changed, 31 insertions(+), 258 deletions(-) diff --git a/internal/run/addrepo.go b/internal/run/addrepo.go index 05d5f3b..5ff7a6e 100644 --- a/internal/run/addrepo.go +++ b/internal/run/addrepo.go @@ -39,15 +39,7 @@ func (a *AddRepo) Prepare() error { name := split[0] url := split[1] - args := make([]string, 0) - - if a.namespace != "" { - args = append(args, "--namespace", a.namespace) - } - if a.debug { - args = append(args, "--debug") - } - + args := a.globalFlags() args = append(args, "repo", "add", name, url) a.cmd = command(helmBin, args...) diff --git a/internal/run/addrepo_test.go b/internal/run/addrepo_test.go index 86c4ad1..4760d45 100644 --- a/internal/run/addrepo_test.go +++ b/internal/run/addrepo_test.go @@ -1,7 +1,6 @@ package run import ( - "fmt" "github.com/golang/mock/gomock" "github.com/pelotech/drone-helm3/internal/env" "github.com/stretchr/testify/suite" @@ -97,42 +96,3 @@ func (suite *AddRepoTestSuite) TestPrepareWithEqualSignInURL() { suite.NoError(a.Prepare()) suite.Contains(suite.commandArgs, "https://github.com/arthur_claypool/samaritan?version=2.1") } - -func (suite *AddRepoTestSuite) TestNamespaceFlag() { - suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() - suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() - cfg := env.Config{ - Namespace: "alliteration", - } - a := NewAddRepo(cfg, "edeath=https://github.com/theater_guy/e-death") - - suite.NoError(a.Prepare()) - suite.Equal(suite.commandPath, helmBin) - suite.Equal(suite.commandArgs, []string{"--namespace", "alliteration", - "repo", "add", "edeath", "https://github.com/theater_guy/e-death"}) -} - -func (suite *AddRepoTestSuite) TestDebugFlag() { - suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() - suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() - - stderr := strings.Builder{} - - command = func(path string, args ...string) cmd { - suite.mockCmd.EXPECT(). - String(). - Return(fmt.Sprintf("%s %s", path, strings.Join(args, " "))) - - return suite.mockCmd - } - - cfg := env.Config{ - Debug: true, - Stderr: &stderr, - } - a := NewAddRepo(cfg, "edeath=https://github.com/the_bug/e-death") - - suite.Require().NoError(a.Prepare()) - suite.Equal(fmt.Sprintf("Generated command: '%s --debug "+ - "repo add edeath https://github.com/the_bug/e-death'\n", helmBin), stderr.String()) -} diff --git a/internal/run/config.go b/internal/run/config.go index 7a20862..268482c 100644 --- a/internal/run/config.go +++ b/internal/run/config.go @@ -20,3 +20,14 @@ func newConfig(cfg env.Config) *config { stderr: cfg.Stderr, } } + +func (cfg *config) globalFlags() []string { + flags := []string{} + if cfg.debug { + flags = append(flags, "--debug") + } + if cfg.namespace != "" { + flags = append(flags, "--namespace", cfg.namespace) + } + return flags +} diff --git a/internal/run/config_test.go b/internal/run/config_test.go index fd781bd..88dbfc4 100644 --- a/internal/run/config_test.go +++ b/internal/run/config_test.go @@ -33,3 +33,16 @@ func (suite *ConfigTestSuite) TestNewConfig() { stderr: stderr, }, cfg) } + +func (suite *ConfigTestSuite) TestGlobalFlags() { + cfg := config{ + debug: true, + namespace: "public", + } + flags := cfg.globalFlags() + suite.Equal([]string{"--debug", "--namespace", "public"}, flags) + + cfg = config{} + flags = cfg.globalFlags() + suite.Equal([]string{}, flags) +} diff --git a/internal/run/depupdate.go b/internal/run/depupdate.go index 667a429..551e7e7 100644 --- a/internal/run/depupdate.go +++ b/internal/run/depupdate.go @@ -31,15 +31,7 @@ func (d *DepUpdate) Prepare() error { return fmt.Errorf("chart is required") } - args := make([]string, 0) - - if d.namespace != "" { - args = append(args, "--namespace", d.namespace) - } - if d.debug { - args = append(args, "--debug") - } - + args := d.globalFlags() args = append(args, "dependency", "update", d.chart) d.cmd = command(helmBin, args...) diff --git a/internal/run/depupdate_test.go b/internal/run/depupdate_test.go index 73c3563..ef553a5 100644 --- a/internal/run/depupdate_test.go +++ b/internal/run/depupdate_test.go @@ -1,7 +1,6 @@ package run import ( - "fmt" "github.com/golang/mock/gomock" "github.com/pelotech/drone-helm3/internal/env" "github.com/stretchr/testify/suite" @@ -71,58 +70,6 @@ func (suite *DepUpdateTestSuite) TestPrepareAndExecute() { suite.NoError(d.Execute()) } -func (suite *DepUpdateTestSuite) TestPrepareNamespaceFlag() { - defer suite.ctrl.Finish() - - cfg := env.Config{ - Namespace: "spotify", - Chart: "your_top_songs_2019", - } - - command = func(path string, args ...string) cmd { - suite.Equal([]string{"--namespace", "spotify", "dependency", "update", "your_top_songs_2019"}, args) - - return suite.mockCmd - } - suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() - suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() - - d := NewDepUpdate(cfg) - - suite.Require().NoError(d.Prepare()) -} - -func (suite *DepUpdateTestSuite) TestPrepareDebugFlag() { - defer suite.ctrl.Finish() - - stdout := strings.Builder{} - stderr := strings.Builder{} - cfg := env.Config{ - Chart: "your_top_songs_2019", - Debug: true, - Stdout: &stdout, - Stderr: &stderr, - } - - command = func(path string, args ...string) cmd { - suite.mockCmd.EXPECT(). - String(). - Return(fmt.Sprintf("%s %s", path, strings.Join(args, " "))) - - return suite.mockCmd - } - suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() - suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() - - d := NewDepUpdate(cfg) - - suite.Require().NoError(d.Prepare()) - - want := fmt.Sprintf("Generated command: '%s --debug dependency update your_top_songs_2019'\n", helmBin) - suite.Equal(want, stderr.String()) - suite.Equal("", stdout.String()) -} - func (suite *DepUpdateTestSuite) TestPrepareChartRequired() { d := NewDepUpdate(env.Config{}) diff --git a/internal/run/help.go b/internal/run/help.go index 79f4101..f5609cd 100644 --- a/internal/run/help.go +++ b/internal/run/help.go @@ -34,10 +34,8 @@ func (h *Help) Execute() error { // Prepare gets the Help ready to execute. func (h *Help) Prepare() error { - args := []string{"help"} - if h.debug { - args = append([]string{"--debug"}, args...) - } + args := h.globalFlags() + args = append(args, "help") h.cmd = command(helmBin, args...) h.cmd.Stdout(h.stdout) diff --git a/internal/run/help_test.go b/internal/run/help_test.go index bdc9dbe..d85cc35 100644 --- a/internal/run/help_test.go +++ b/internal/run/help_test.go @@ -1,7 +1,6 @@ package run import ( - "fmt" "github.com/golang/mock/gomock" "github.com/pelotech/drone-helm3/internal/env" "github.com/stretchr/testify/assert" @@ -75,20 +74,3 @@ func (suite *HelpTestSuite) TestExecute() { help.helmCommand = "get down on friday" suite.EqualError(help.Execute(), "unknown command 'get down on friday'") } - -func (suite *HelpTestSuite) TestPrepareDebugFlag() { - stdout := strings.Builder{} - stderr := strings.Builder{} - cfg := env.Config{ - Debug: true, - Stdout: &stdout, - Stderr: &stderr, - } - - help := NewHelp(cfg) - help.Prepare() - - want := fmt.Sprintf("Generated command: '%s --debug help'\n", helmBin) - suite.Equal(want, stderr.String()) - suite.Equal("", stdout.String()) -} diff --git a/internal/run/lint.go b/internal/run/lint.go index 242e113..7752f8d 100644 --- a/internal/run/lint.go +++ b/internal/run/lint.go @@ -39,15 +39,7 @@ func (l *Lint) Prepare() error { return fmt.Errorf("chart is required") } - args := make([]string, 0) - - if l.namespace != "" { - args = append(args, "--namespace", l.namespace) - } - if l.debug { - args = append(args, "--debug") - } - + args := l.globalFlags() args = append(args, "lint") if l.values != "" { diff --git a/internal/run/lint_test.go b/internal/run/lint_test.go index 1241dfd..42fd923 100644 --- a/internal/run/lint_test.go +++ b/internal/run/lint_test.go @@ -1,7 +1,6 @@ package run import ( - "fmt" "github.com/golang/mock/gomock" "github.com/pelotech/drone-helm3/internal/env" "github.com/stretchr/testify/suite" @@ -126,58 +125,3 @@ func (suite *LintTestSuite) TestPrepareWithLintFlags() { err := l.Prepare() suite.Require().Nil(err) } - -func (suite *LintTestSuite) TestPrepareWithDebugFlag() { - defer suite.ctrl.Finish() - - stderr := strings.Builder{} - - cfg := env.Config{ - Debug: true, - Stderr: &stderr, - Chart: "./scotland/top_40", - } - l := NewLint(cfg) - - command = func(path string, args ...string) cmd { - suite.mockCmd.EXPECT(). - String(). - Return(fmt.Sprintf("%s %s", path, strings.Join(args, " "))) - - return suite.mockCmd - } - - suite.mockCmd.EXPECT().Stdout(gomock.Any()) - suite.mockCmd.EXPECT().Stderr(&stderr) - - err := l.Prepare() - suite.Require().Nil(err) - - want := fmt.Sprintf("Generated command: '%s --debug lint ./scotland/top_40'\n", helmBin) - suite.Equal(want, stderr.String()) -} - -func (suite *LintTestSuite) TestPrepareWithNamespaceFlag() { - defer suite.ctrl.Finish() - - cfg := env.Config{ - Namespace: "table-service", - Chart: "./wales/top_40", - } - l := NewLint(cfg) - - actual := []string{} - command = func(path string, args ...string) cmd { - actual = args - return suite.mockCmd - } - - suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() - suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() - - err := l.Prepare() - suite.Require().Nil(err) - - expected := []string{"--namespace", "table-service", "lint", "./wales/top_40"} - suite.Equal(expected, actual) -} diff --git a/internal/run/uninstall.go b/internal/run/uninstall.go index 186cb16..d0cce60 100644 --- a/internal/run/uninstall.go +++ b/internal/run/uninstall.go @@ -35,15 +35,7 @@ func (u *Uninstall) Prepare() error { return fmt.Errorf("release is required") } - args := make([]string, 0) - - if u.namespace != "" { - args = append(args, "--namespace", u.namespace) - } - if u.debug { - args = append(args, "--debug") - } - + args := u.globalFlags() args = append(args, "uninstall") if u.dryRun { diff --git a/internal/run/uninstall_test.go b/internal/run/uninstall_test.go index 69bde6e..b52ebfd 100644 --- a/internal/run/uninstall_test.go +++ b/internal/run/uninstall_test.go @@ -1,11 +1,9 @@ package run import ( - "fmt" "github.com/golang/mock/gomock" "github.com/pelotech/drone-helm3/internal/env" "github.com/stretchr/testify/suite" - "strings" "testing" ) @@ -110,46 +108,6 @@ func (suite *UninstallTestSuite) TestPrepareKeepHistoryFlag() { suite.Equal(expected, suite.actualArgs) } -func (suite *UninstallTestSuite) TestPrepareNamespaceFlag() { - cfg := env.Config{ - Release: "carly_simon_run_away_with_me", - Namespace: "emotion", - } - u := NewUninstall(cfg) - - suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() - suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() - - suite.NoError(u.Prepare()) - expected := []string{"--namespace", "emotion", "uninstall", "carly_simon_run_away_with_me"} - suite.Equal(expected, suite.actualArgs) -} - -func (suite *UninstallTestSuite) TestPrepareDebugFlag() { - stderr := strings.Builder{} - cfg := env.Config{ - Release: "just_a_band_huff_and_puff", - Debug: true, - Stderr: &stderr, - } - u := NewUninstall(cfg) - - command = func(path string, args ...string) cmd { - suite.mockCmd.EXPECT(). - String(). - Return(fmt.Sprintf("%s %s", path, strings.Join(args, " "))) - - return suite.mockCmd - } - - suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() - suite.mockCmd.EXPECT().Stderr(&stderr).AnyTimes() - - suite.NoError(u.Prepare()) - suite.Equal(fmt.Sprintf("Generated command: '%s --debug "+ - "uninstall just_a_band_huff_and_puff'\n", helmBin), stderr.String()) -} - func (suite *UninstallTestSuite) TestPrepareRequiresRelease() { // These aren't really expected, but allowing them gives clearer test-failure messages suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() diff --git a/internal/run/upgrade.go b/internal/run/upgrade.go index 59af6a6..5995200 100644 --- a/internal/run/upgrade.go +++ b/internal/run/upgrade.go @@ -60,15 +60,7 @@ func (u *Upgrade) Prepare() error { return fmt.Errorf("release is required") } - args := make([]string, 0) - - if u.namespace != "" { - args = append(args, "--namespace", u.namespace) - } - if u.debug { - args = append(args, "--debug") - } - + args := u.globalFlags() args = append(args, "upgrade", "--install") if u.chartVersion != "" {