From 34b9ec1c4c9ab2bb016800fa19a96cfd60d0168a Mon Sep 17 00:00:00 2001 From: Erin Call Date: Thu, 26 Dec 2019 10:47:42 -0800 Subject: [PATCH 01/12] Run the Help step by default [#15] --- internal/helm/plan.go | 2 +- internal/helm/plan_test.go | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/internal/helm/plan.go b/internal/helm/plan.go index 1d4ced9..d5c8835 100644 --- a/internal/helm/plan.go +++ b/internal/helm/plan.go @@ -72,7 +72,7 @@ func determineSteps(cfg Config) *func(Config) []Step { case "delete": return &uninstall default: - panic("not implemented") + return &help } } } diff --git a/internal/helm/plan_test.go b/internal/helm/plan_test.go index e5a1a96..eebf51b 100644 --- a/internal/helm/plan_test.go +++ b/internal/helm/plan_test.go @@ -255,3 +255,10 @@ func (suite *PlanTestSuite) TestDeterminePlanHelpCommand() { stepsMaker := determineSteps(cfg) suite.Same(&help, stepsMaker) } + +func (suite *PlanTestSuite) TestDeterminePlanHelpOnUnknown() { + cfg := Config{} + + stepsMaker := determineSteps(cfg) + suite.Same(&help, stepsMaker) +} From 6d28b7b28ad52596e953594a42bfbf329ba05640 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Thu, 26 Dec 2019 11:29:33 -0800 Subject: [PATCH 02/12] Return an error on unknown commands [#15] I'm probably overthinking this--explicitly calling help is a strange and unusual case--but it doesn't really hurt, so I'm going for it. --- internal/helm/plan.go | 1 + internal/helm/plan_test.go | 8 +------- internal/run/config.go | 1 + internal/run/help.go | 11 +++++++++-- internal/run/help_test.go | 32 +++++++++++++++++++++++++++----- 5 files changed, 39 insertions(+), 14 deletions(-) diff --git a/internal/helm/plan.go b/internal/helm/plan.go index d5c8835..0f32c1e 100644 --- a/internal/helm/plan.go +++ b/internal/helm/plan.go @@ -26,6 +26,7 @@ func NewPlan(cfg Config) (*Plan, error) { p := Plan{ cfg: cfg, runCfg: run.Config{ + HelmCommand: string(cfg.Command), Debug: cfg.Debug, KubeConfig: cfg.KubeConfig, Values: cfg.Values, diff --git a/internal/helm/plan_test.go b/internal/helm/plan_test.go index eebf51b..cf3871d 100644 --- a/internal/helm/plan_test.go +++ b/internal/helm/plan_test.go @@ -40,6 +40,7 @@ func (suite *PlanTestSuite) TestNewPlan() { } runCfg := run.Config{ + HelmCommand: "help", Debug: false, KubeConfig: "/branch/.sfere/profig", Values: "steadfastness,forthrightness", @@ -255,10 +256,3 @@ func (suite *PlanTestSuite) TestDeterminePlanHelpCommand() { stepsMaker := determineSteps(cfg) suite.Same(&help, stepsMaker) } - -func (suite *PlanTestSuite) TestDeterminePlanHelpOnUnknown() { - cfg := Config{} - - stepsMaker := determineSteps(cfg) - suite.Same(&help, stepsMaker) -} diff --git a/internal/run/config.go b/internal/run/config.go index 09b9642..3d5b3f9 100644 --- a/internal/run/config.go +++ b/internal/run/config.go @@ -6,6 +6,7 @@ import ( // Config contains configuration applicable to all helm commands type Config struct { + HelmCommand string Debug bool KubeConfig string Values string diff --git a/internal/run/help.go b/internal/run/help.go index 8597815..a4a116e 100644 --- a/internal/run/help.go +++ b/internal/run/help.go @@ -10,8 +10,15 @@ type Help struct { } // Execute executes the `helm help` command. -func (h *Help) Execute(_ Config) error { - return h.cmd.Run() +func (h *Help) Execute(cfg Config) error { + if err := h.cmd.Run(); err != nil { + return fmt.Errorf("while running '%s': %w", h.cmd.String(), err) + } + + if cfg.HelmCommand == "help" { + return nil + } + return fmt.Errorf("unknown command '%s'", cfg.HelmCommand) } // Prepare gets the Help ready to execute. diff --git a/internal/run/help_test.go b/internal/run/help_test.go index 1824578..ecca2bb 100644 --- a/internal/run/help_test.go +++ b/internal/run/help_test.go @@ -38,9 +38,6 @@ func (suite *HelpTestSuite) TestPrepare() { Stdout(&stdout) mCmd.EXPECT(). Stderr(&stderr) - mCmd.EXPECT(). - Run(). - Times(1) cfg := Config{ Stdout: &stdout, @@ -49,8 +46,33 @@ func (suite *HelpTestSuite) TestPrepare() { h := Help{} err := h.Prepare(cfg) - suite.Require().Nil(err) - h.Execute(cfg) + suite.NoError(err) +} + +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) + + cfg := Config{ + HelmCommand: "help", + } + help := Help{ + cmd: mCmd, + } + suite.NoError(help.Execute(cfg)) + + cfg.HelmCommand = "get down on friday" + suite.EqualError(help.Execute(cfg), "unknown command 'get down on friday'") } func (suite *HelpTestSuite) TestPrepareDebugFlag() { From 41e9e42239ac7aa5785ab80859eddf2d0a13812d Mon Sep 17 00:00:00 2001 From: Erin Call Date: Thu, 26 Dec 2019 11:31:45 -0800 Subject: [PATCH 03/12] Emit a trailing newline on execution error [#15] Just something I noticed while testing the help command's error case. --- cmd/drone-helm/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/drone-helm/main.go b/cmd/drone-helm/main.go index 61673b2..3ca31dd 100644 --- a/cmd/drone-helm/main.go +++ b/cmd/drone-helm/main.go @@ -28,7 +28,7 @@ func main() { // Expect the plan to go off the rails if err != nil { - fmt.Fprintf(os.Stderr, err.Error()) + fmt.Fprintf(os.Stderr, "%s\n", err.Error()) // Throw away the plan os.Exit(1) } From 167b53691b9eed6f1446c643fe2217aa0f0617f5 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Thu, 26 Dec 2019 12:23:56 -0800 Subject: [PATCH 04/12] Put HelmCommand in Help, not run.Config [#15] --- internal/helm/plan.go | 5 +++-- internal/helm/plan_test.go | 1 - internal/run/config.go | 1 - internal/run/help.go | 7 ++++--- internal/run/help_test.go | 9 ++++----- 5 files changed, 11 insertions(+), 12 deletions(-) diff --git a/internal/helm/plan.go b/internal/helm/plan.go index 72fd48a..a82ffea 100644 --- a/internal/helm/plan.go +++ b/internal/helm/plan.go @@ -29,7 +29,6 @@ func NewPlan(cfg Config) (*Plan, error) { p := Plan{ cfg: cfg, runCfg: run.Config{ - HelmCommand: string(cfg.Command), Debug: cfg.Debug, Values: cfg.Values, StringValues: cfg.StringValues, @@ -132,7 +131,9 @@ var lint = func(cfg Config) []Step { } var help = func(cfg Config) []Step { - help := &run.Help{} + help := &run.Help{ + HelmCommand: cfg.Command, + } return []Step{help} } diff --git a/internal/helm/plan_test.go b/internal/helm/plan_test.go index fc70ec9..ce5311a 100644 --- a/internal/helm/plan_test.go +++ b/internal/helm/plan_test.go @@ -43,7 +43,6 @@ func (suite *PlanTestSuite) TestNewPlan() { } runCfg := run.Config{ - HelmCommand: "help", Debug: false, Values: "steadfastness,forthrightness", StringValues: "tensile_strength,flexibility", diff --git a/internal/run/config.go b/internal/run/config.go index 25fcd6f..4f9b99a 100644 --- a/internal/run/config.go +++ b/internal/run/config.go @@ -6,7 +6,6 @@ import ( // Config contains configuration applicable to all helm commands type Config struct { - HelmCommand string Debug bool Values string StringValues string diff --git a/internal/run/help.go b/internal/run/help.go index a4a116e..f2d6c59 100644 --- a/internal/run/help.go +++ b/internal/run/help.go @@ -6,7 +6,8 @@ import ( // Help is a step in a helm Plan that calls `helm help`. type Help struct { - cmd cmd + HelmCommand string + cmd cmd } // Execute executes the `helm help` command. @@ -15,10 +16,10 @@ func (h *Help) Execute(cfg Config) error { return fmt.Errorf("while running '%s': %w", h.cmd.String(), err) } - if cfg.HelmCommand == "help" { + if h.HelmCommand == "help" { return nil } - return fmt.Errorf("unknown command '%s'", cfg.HelmCommand) + return fmt.Errorf("unknown command '%s'", h.HelmCommand) } // Prepare gets the Help ready to execute. diff --git a/internal/run/help_test.go b/internal/run/help_test.go index ecca2bb..19c49d2 100644 --- a/internal/run/help_test.go +++ b/internal/run/help_test.go @@ -63,15 +63,14 @@ func (suite *HelpTestSuite) TestExecute() { Run(). Times(2) - cfg := Config{ - HelmCommand: "help", - } + cfg := Config{} help := Help{ - cmd: mCmd, + HelmCommand: "help", + cmd: mCmd, } suite.NoError(help.Execute(cfg)) - cfg.HelmCommand = "get down on friday" + help.HelmCommand = "get down on friday" suite.EqualError(help.Execute(cfg), "unknown command 'get down on friday'") } From 354dce2e126437219d13b4a83d93e4ccbd041dd8 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Fri, 27 Dec 2019 11:18:13 -0800 Subject: [PATCH 05/12] Use the apache 2.0 license [#23] --- LICENSE | 202 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 202 insertions(+) create mode 100644 LICENSE diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..d645695 --- /dev/null +++ b/LICENSE @@ -0,0 +1,202 @@ + + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright [yyyy] [name of copyright owner] + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. From 181165cc51eab5c815dd8e5938d76a7f6359583a Mon Sep 17 00:00:00 2001 From: Erin Call Date: Fri, 27 Dec 2019 15:06:32 -0800 Subject: [PATCH 06/12] Call `helm dependency update` when so instructed [#25] As with Lint, I have no idea whether the --namespace flag actually matters here. I don't think it will hurt, though! --- docs/parameter_reference.md | 2 +- internal/helm/plan.go | 25 +++++-- internal/helm/plan_test.go | 48 ++++++++++++- internal/run/depupdate.go | 44 ++++++++++++ internal/run/depupdate_test.go | 128 +++++++++++++++++++++++++++++++++ 5 files changed, 241 insertions(+), 6 deletions(-) create mode 100644 internal/run/depupdate.go create mode 100644 internal/run/depupdate_test.go diff --git a/docs/parameter_reference.md b/docs/parameter_reference.md index 07d8107..a5e02d6 100644 --- a/docs/parameter_reference.md +++ b/docs/parameter_reference.md @@ -4,7 +4,7 @@ | Param name | Type | Purpose | |---------------------|-----------------|---------| | helm_command | string | Indicates the operation to perform. Recommended, but not required. Valid options are `upgrade`, `uninstall`, `lint`, and `help`. | -| update_dependencies | boolean | Calls `helm dependency update` before running the main command. **Not currently implemented**; see [#25](https://github.com/pelotech/drone-helm3/issues/25).| +| update_dependencies | boolean | Calls `helm dependency update` before running the main command.| | helm_repos | list\ | Calls `helm repo add $repo` before running the main command. Each string should be formatted as `repo_name=https://repo.url/`. **Not currently implemented**; see [#26](https://github.com/pelotech/drone-helm3/issues/26). | | namespace | string | Kubernetes namespace to use for this operation. | | prefix | string | Expect environment variables to be prefixed with the given string. For more details, see "Using the prefix setting" below. **Not currently implemented**; see [#19](https://github.com/pelotech/drone-helm3/issues/19). | diff --git a/internal/helm/plan.go b/internal/helm/plan.go index e6c8721..ad2e5ae 100644 --- a/internal/helm/plan.go +++ b/internal/helm/plan.go @@ -96,7 +96,9 @@ func (p *Plan) Execute() error { var upgrade = func(cfg Config) []Step { steps := initKube(cfg) - + if cfg.UpdateDependencies { + steps = append(steps, depUpdate(cfg)...) + } steps = append(steps, &run.Upgrade{ Chart: cfg.Chart, Release: cfg.Release, @@ -113,6 +115,9 @@ var upgrade = func(cfg Config) []Step { var uninstall = func(cfg Config) []Step { steps := initKube(cfg) + if cfg.UpdateDependencies { + steps = append(steps, depUpdate(cfg)...) + } steps = append(steps, &run.Uninstall{ Release: cfg.Release, DryRun: cfg.DryRun, @@ -122,11 +127,15 @@ var uninstall = func(cfg Config) []Step { } var lint = func(cfg Config) []Step { - lint := &run.Lint{ - Chart: cfg.Chart, + steps := make([]Step, 0) + if cfg.UpdateDependencies { + steps = append(steps, depUpdate(cfg)...) } + steps = append(steps, &run.Lint{ + Chart: cfg.Chart, + }) - return []Step{lint} + return steps } var help = func(cfg Config) []Step { @@ -147,3 +156,11 @@ func initKube(cfg Config) []Step { }, } } + +func depUpdate(cfg Config) []Step { + return []Step{ + &run.DepUpdate{ + Chart: cfg.Chart, + }, + } +} diff --git a/internal/helm/plan_test.go b/internal/helm/plan_test.go index 4808aef..2cdde5c 100644 --- a/internal/helm/plan_test.go +++ b/internal/helm/plan_test.go @@ -167,7 +167,17 @@ func (suite *PlanTestSuite) TestUpgrade() { suite.Equal(expected, upgrade) } -func (suite *PlanTestSuite) TestDel() { +func (suite *PlanTestSuite) TestUpgradeWithUpdateDependencies() { + cfg := Config{ + UpdateDependencies: true, + } + steps := upgrade(cfg) + suite.Require().Equal(3, len(steps), "upgrade should have a third step when DepUpdate is true") + suite.IsType(&run.InitKube{}, steps[0]) + suite.IsType(&run.DepUpdate{}, steps[1]) +} + +func (suite *PlanTestSuite) TestUninstall() { cfg := Config{ KubeToken: "b2YgbXkgYWZmZWN0aW9u", SkipTLSVerify: true, @@ -205,6 +215,16 @@ func (suite *PlanTestSuite) TestDel() { suite.Equal(expected, actual) } +func (suite *PlanTestSuite) TestUninstallWithUpdateDependencies() { + cfg := Config{ + UpdateDependencies: true, + } + steps := uninstall(cfg) + suite.Require().Equal(3, len(steps), "uninstall should have a third step when DepUpdate is true") + suite.IsType(&run.InitKube{}, steps[0]) + suite.IsType(&run.DepUpdate{}, steps[1]) +} + func (suite *PlanTestSuite) TestInitKube() { cfg := Config{ KubeToken: "cXVlZXIgY2hhcmFjdGVyCg==", @@ -231,6 +251,23 @@ func (suite *PlanTestSuite) TestInitKube() { suite.Equal(expected, init) } +func (suite *PlanTestSuite) TestDepUpdate() { + cfg := 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) +} + func (suite *PlanTestSuite) TestLint() { cfg := Config{ Chart: "./flow", @@ -245,6 +282,15 @@ func (suite *PlanTestSuite) TestLint() { suite.Equal(want, steps[0]) } +func (suite *PlanTestSuite) TestLintWithUpdateDependencies() { + cfg := Config{ + UpdateDependencies: true, + } + steps := lint(cfg) + suite.Require().Equal(2, len(steps), "lint should have a second step when DepUpdate is true") + suite.IsType(&run.DepUpdate{}, steps[0]) +} + func (suite *PlanTestSuite) TestDeterminePlanUpgradeCommand() { cfg := Config{ Command: "upgrade", diff --git a/internal/run/depupdate.go b/internal/run/depupdate.go new file mode 100644 index 0000000..a9b6c91 --- /dev/null +++ b/internal/run/depupdate.go @@ -0,0 +1,44 @@ +package run + +import ( + "fmt" +) + +// DepUpdate is an execution step that calls `helm dependency update` when executed. +type DepUpdate struct { + Chart string + cmd cmd +} + +// Execute executes the `helm upgrade` command. +func (d *DepUpdate) Execute(_ Config) error { + return d.cmd.Run() +} + +// Prepare gets the DepUpdate ready to execute. +func (d *DepUpdate) Prepare(cfg Config) error { + if d.Chart == "" { + return fmt.Errorf("chart is required") + } + + args := make([]string, 0) + + if cfg.Namespace != "" { + args = append(args, "--namespace", cfg.Namespace) + } + if cfg.Debug { + args = append(args, "--debug") + } + + args = append(args, "dependency", "update", d.Chart) + + d.cmd = command(helmBin, args...) + d.cmd.Stdout(cfg.Stdout) + d.cmd.Stderr(cfg.Stderr) + + if cfg.Debug { + fmt.Fprintf(cfg.Stderr, "Generated command: '%s'\n", d.cmd.String()) + } + + return nil +} diff --git a/internal/run/depupdate_test.go b/internal/run/depupdate_test.go new file mode 100644 index 0000000..315b351 --- /dev/null +++ b/internal/run/depupdate_test.go @@ -0,0 +1,128 @@ +package run + +import ( + "fmt" + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/suite" + "strings" + "testing" +) + +type DepUpdateTestSuite struct { + suite.Suite + ctrl *gomock.Controller + mockCmd *Mockcmd + originalCommand func(string, ...string) cmd +} + +func (suite *DepUpdateTestSuite) BeforeTest(_, _ string) { + suite.ctrl = gomock.NewController(suite.T()) + suite.mockCmd = NewMockcmd(suite.ctrl) + + suite.originalCommand = command + command = func(path string, args ...string) cmd { return suite.mockCmd } +} + +func (suite *DepUpdateTestSuite) AfterTest(_, _ string) { + command = suite.originalCommand +} + +func TestDepUpdateTestSuite(t *testing.T) { + suite.Run(t, new(DepUpdateTestSuite)) +} + +func (suite *DepUpdateTestSuite) TestPrepareAndExecute() { + defer suite.ctrl.Finish() + + stdout := strings.Builder{} + stderr := strings.Builder{} + cfg := Config{ + Stdout: &stdout, + Stderr: &stderr, + } + + command = func(path string, args ...string) cmd { + suite.Equal(helmBin, path) + suite.Equal([]string{"dependency", "update", "your_top_songs_2019"}, args) + + return suite.mockCmd + } + suite.mockCmd.EXPECT(). + Stdout(&stdout) + suite.mockCmd.EXPECT(). + Stderr(&stderr) + suite.mockCmd.EXPECT(). + Run(). + Times(1) + + d := DepUpdate{ + Chart: "your_top_songs_2019", + } + + suite.Require().NoError(d.Prepare(cfg)) + suite.NoError(d.Execute(cfg)) +} + +func (suite *DepUpdateTestSuite) TestPrepareNamespaceFlag() { + defer suite.ctrl.Finish() + + cfg := Config{ + Namespace: "spotify", + } + + 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 := DepUpdate{ + Chart: "your_top_songs_2019", + } + + suite.Require().NoError(d.Prepare(cfg)) +} + +func (suite *DepUpdateTestSuite) TestPrepareDebugFlag() { + defer suite.ctrl.Finish() + + stdout := strings.Builder{} + stderr := strings.Builder{} + cfg := Config{ + 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 := DepUpdate{ + Chart: "your_top_songs_2019", + } + + suite.Require().NoError(d.Prepare(cfg)) + + 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 := DepUpdate{} + + suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() + suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() + + err := d.Prepare(Config{}) + suite.EqualError(err, "chart is required") +} From 89ec9425b0938fd5b49344d245f05e63d84296af Mon Sep 17 00:00:00 2001 From: Erin Call Date: Fri, 27 Dec 2019 15:44:09 -0800 Subject: [PATCH 07/12] Mention the chart param for uninstalls [#25] --- docs/parameter_reference.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/parameter_reference.md b/docs/parameter_reference.md index a5e02d6..0073ddf 100644 --- a/docs/parameter_reference.md +++ b/docs/parameter_reference.md @@ -58,6 +58,7 @@ Uninstallations are triggered when the `helm_command` setting is "uninstall" or | dry_run | boolean | | Pass `--dry-run` to `helm uninstall`. | | timeout | duration | | Timeout for any *individual* Kubernetes operation. The uninstallation's full runtime may exceed this duration. | | skip_tls_verify | boolean | | Connect to the Kubernetes cluster without checking for a valid TLS certificate. Not recommended in production. | +| chart | string | | Required when the global `update_dependencies` parameter is true. No effect otherwise. | ### Where to put settings From d5a59590a129514340007bc06214ad297949e895 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Fri, 27 Dec 2019 16:18:10 -0800 Subject: [PATCH 08/12] Shim bare numbers into duration strings [#39] Helm2's --timeout took a number of seconds, rather than the ParseDuration-compatible string that helm3 uses. For backward- compatibility, update a bare number into a duration string. --- docs/parameter_reference.md | 1 + internal/helm/config.go | 7 +++++++ internal/helm/config_test.go | 7 +++++++ 3 files changed, 15 insertions(+) diff --git a/docs/parameter_reference.md b/docs/parameter_reference.md index 07d8107..c11aac5 100644 --- a/docs/parameter_reference.md +++ b/docs/parameter_reference.md @@ -67,6 +67,7 @@ Any setting (with the exception of `prefix`; [see below](#user-content-using-the * Booleans can be yaml's `true` and `false` literals or the strings `"true"` and `"false"`. * Durations are strings formatted with the syntax accepted by [golang's ParseDuration function](https://golang.org/pkg/time/#ParseDuration) (e.g. 5m30s) + * For backward-compatibility with drone-helm, a duration can also be an integer, in which case it will be interpreted to mean seconds. * List\s can be a yaml sequence or a comma-separated string. All of the following are equivalent: diff --git a/internal/helm/config.go b/internal/helm/config.go index 987b6de..cf351ae 100644 --- a/internal/helm/config.go +++ b/internal/helm/config.go @@ -4,8 +4,11 @@ import ( "fmt" "github.com/kelseyhightower/envconfig" "io" + "regexp" ) +var justNumbers = regexp.MustCompile(`^\d+$`) + // The Config struct captures the `settings` and `environment` blocks in the application's drone // config. Configuration in drone's `settings` block arrives as uppercase env vars matching the // config key, prefixed with `PLUGIN_`. Config from the `environment` block is uppercased, but does @@ -62,6 +65,10 @@ func NewConfig(stdout, stderr io.Writer) (*Config, error) { } } + if justNumbers.MatchString(cfg.Timeout) { + cfg.Timeout = fmt.Sprintf("%ss", cfg.Timeout) + } + if cfg.Debug && cfg.Stderr != nil { cfg.logDebug() } diff --git a/internal/helm/config_test.go b/internal/helm/config_test.go index 3ca2f91..f39dd0c 100644 --- a/internal/helm/config_test.go +++ b/internal/helm/config_test.go @@ -106,6 +106,13 @@ func (suite *ConfigTestSuite) TestNewConfigWithConflictingVariables() { suite.Equal("2m30s", cfg.Timeout) } +func (suite *ConfigTestSuite) TestNewConfigInfersNumbersAreSeconds() { + suite.setenv("PLUGIN_TIMEOUT", "42") + cfg, err := NewConfig(&strings.Builder{}, &strings.Builder{}) + suite.Require().NoError(err) + suite.Equal("42s", cfg.Timeout) +} + func (suite *ConfigTestSuite) TestNewConfigSetsWriters() { stdout := &strings.Builder{} stderr := &strings.Builder{} From 75c99683b5fffe3cf7f0b8bd4c2710228066bed6 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Mon, 30 Dec 2019 09:52:00 -0800 Subject: [PATCH 09/12] AddRepo step that calls `helm repo add` [#26] As with some of the other commands, I'm not sure `--namespace` is relevant here. Just rolling with the "at worst it doesn't hurt" theory. --- docs/parameter_reference.md | 2 +- internal/run/addrepo.go | 48 +++++++++++++ internal/run/addrepo_test.go | 134 +++++++++++++++++++++++++++++++++++ 3 files changed, 183 insertions(+), 1 deletion(-) create mode 100644 internal/run/addrepo.go create mode 100644 internal/run/addrepo_test.go diff --git a/docs/parameter_reference.md b/docs/parameter_reference.md index 07d8107..426dda9 100644 --- a/docs/parameter_reference.md +++ b/docs/parameter_reference.md @@ -5,7 +5,7 @@ |---------------------|-----------------|---------| | helm_command | string | Indicates the operation to perform. Recommended, but not required. Valid options are `upgrade`, `uninstall`, `lint`, and `help`. | | update_dependencies | boolean | Calls `helm dependency update` before running the main command. **Not currently implemented**; see [#25](https://github.com/pelotech/drone-helm3/issues/25).| -| helm_repos | list\ | Calls `helm repo add $repo` before running the main command. Each string should be formatted as `repo_name=https://repo.url/`. **Not currently implemented**; see [#26](https://github.com/pelotech/drone-helm3/issues/26). | +| helm_repos | list\ | Calls `helm repo add $repo` before running the main command. Each string should be formatted as `repo_name=https://repo.url/`. | | namespace | string | Kubernetes namespace to use for this operation. | | prefix | string | Expect environment variables to be prefixed with the given string. For more details, see "Using the prefix setting" below. **Not currently implemented**; see [#19](https://github.com/pelotech/drone-helm3/issues/19). | | debug | boolean | Generate debug output within drone-helm3 and pass `--debug` to all helm commands. Use with care, since the debug output may include secrets. | diff --git a/internal/run/addrepo.go b/internal/run/addrepo.go new file mode 100644 index 0000000..4dc326a --- /dev/null +++ b/internal/run/addrepo.go @@ -0,0 +1,48 @@ +package run + +import ( + "fmt" +) + +// AddRepo is an execution step that calls `helm repo add` when executed. +type AddRepo struct { + Name string + URL string + cmd cmd +} + +// Execute executes the `helm repo add` command. +func (a *AddRepo) Execute(_ Config) error { + return a.cmd.Run() +} + +// Prepare gets the AddRepo ready to execute. +func (a *AddRepo) Prepare(cfg Config) error { + if a.Name == "" { + return fmt.Errorf("repo name is required") + } + if a.URL == "" { + return fmt.Errorf("repo URL is required") + } + + args := make([]string, 0) + + if cfg.Namespace != "" { + args = append(args, "--namespace", cfg.Namespace) + } + if cfg.Debug { + args = append(args, "--debug") + } + + args = append(args, "repo", "add", a.Name, a.URL) + + a.cmd = command(helmBin, args...) + a.cmd.Stdout(cfg.Stdout) + a.cmd.Stderr(cfg.Stderr) + + if cfg.Debug { + fmt.Fprintf(cfg.Stderr, "Generated command: '%s'\n", a.cmd.String()) + } + + return nil +} diff --git a/internal/run/addrepo_test.go b/internal/run/addrepo_test.go new file mode 100644 index 0000000..867c039 --- /dev/null +++ b/internal/run/addrepo_test.go @@ -0,0 +1,134 @@ +package run + +import ( + "fmt" + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/suite" + "strings" + "testing" +) + +type AddRepoTestSuite struct { + suite.Suite + ctrl *gomock.Controller + mockCmd *Mockcmd + originalCommand func(string, ...string) cmd + commandPath string + commandArgs []string +} + +func (suite *AddRepoTestSuite) BeforeTest(_, _ string) { + suite.ctrl = gomock.NewController(suite.T()) + suite.mockCmd = NewMockcmd(suite.ctrl) + + suite.originalCommand = command + command = func(path string, args ...string) cmd { + suite.commandPath = path + suite.commandArgs = args + return suite.mockCmd + } +} + +func (suite *AddRepoTestSuite) AfterTest(_, _ string) { + suite.ctrl.Finish() + command = suite.originalCommand +} + +func TestAddRepoTestSuite(t *testing.T) { + suite.Run(t, new(AddRepoTestSuite)) +} + +func (suite *AddRepoTestSuite) TestPrepareAndExecute() { + stdout := strings.Builder{} + stderr := strings.Builder{} + cfg := Config{ + Stdout: &stdout, + Stderr: &stderr, + } + a := AddRepo{ + Name: "edeath", + URL: "https://github.com/n_marks/e-death", + } + + suite.mockCmd.EXPECT(). + Stdout(&stdout). + Times(1) + suite.mockCmd.EXPECT(). + Stderr(&stderr). + Times(1) + + suite.Require().NoError(a.Prepare(cfg)) + suite.Equal(suite.commandPath, helmBin) + suite.Equal(suite.commandArgs, []string{"repo", "add", "edeath", "https://github.com/n_marks/e-death"}) + + suite.mockCmd.EXPECT(). + Run(). + Times(1) + + suite.Require().NoError(a.Execute(cfg)) + +} + +func (suite *AddRepoTestSuite) TestRequiredFields() { + // 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{ + Name: "idgen", + } + + err := a.Prepare(cfg) + suite.EqualError(err, "repo URL is required") + + a.Name = "" + a.URL = "https://github.com/n_marks/idgen" + + err = a.Prepare(cfg) + suite.EqualError(err, "repo name is required") +} + +func (suite *AddRepoTestSuite) TestNamespaceFlag() { + suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() + suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() + cfg := Config{ + Namespace: "alliteration", + } + a := AddRepo{ + Name: "edeath", + URL: "https://github.com/theater_guy/e-death", + } + + suite.NoError(a.Prepare(cfg)) + 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 := Config{ + Debug: true, + Stderr: &stderr, + } + a := AddRepo{ + Name: "edeath", + URL: "https://github.com/the_bug/e-death", + } + + suite.Require().NoError(a.Prepare(cfg)) + suite.Equal(fmt.Sprintf("Generated command: '%s --debug "+ + "repo add edeath https://github.com/the_bug/e-death'\n", helmBin), stderr.String()) +} From 22e30fea5667e8f26d7378bcb39171487e33e8e4 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Mon, 30 Dec 2019 09:56:47 -0800 Subject: [PATCH 10/12] The prefix setting is implemented [#19,#9] Just something I noticed while resolving a merge conflict. The "write some docs" and "implement prefix" branches happened concurrently and didn't get re-coordinated. --- docs/parameter_reference.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/parameter_reference.md b/docs/parameter_reference.md index 703dd5e..52f3dd7 100644 --- a/docs/parameter_reference.md +++ b/docs/parameter_reference.md @@ -7,7 +7,7 @@ | update_dependencies | boolean | Calls `helm dependency update` before running the main command.| | helm_repos | list\ | Calls `helm repo add $repo` before running the main command. Each string should be formatted as `repo_name=https://repo.url/`. | | namespace | string | Kubernetes namespace to use for this operation. | -| prefix | string | Expect environment variables to be prefixed with the given string. For more details, see "Using the prefix setting" below. **Not currently implemented**; see [#19](https://github.com/pelotech/drone-helm3/issues/19). | +| prefix | string | Expect environment variables to be prefixed with the given string. For more details, see "Using the prefix setting" below. | | debug | boolean | Generate debug output within drone-helm3 and pass `--debug` to all helm commands. Use with care, since the debug output may include secrets. | ## Linting From 48b6b3f5b30f38d6c3800921eca74552617af2f1 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Mon, 30 Dec 2019 11:57:19 -0800 Subject: [PATCH 11/12] Create AddRepo steps when there are repos to add [#26] --- internal/helm/config.go | 2 +- internal/helm/plan.go | 21 ++++++++++- internal/helm/plan_test.go | 75 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 2 deletions(-) diff --git a/internal/helm/config.go b/internal/helm/config.go index cf351ae..a4d1914 100644 --- a/internal/helm/config.go +++ b/internal/helm/config.go @@ -18,7 +18,7 @@ type Config struct { Command string `envconfig:"HELM_COMMAND"` // Helm command to run DroneEvent string `envconfig:"DRONE_BUILD_EVENT"` // Drone event that invoked this plugin. UpdateDependencies bool `split_words:"true"` // Call `helm dependency update` before the main command - Repos []string `envconfig:"HELM_REPOS"` // Call `helm repo add` before the main command + AddRepos []string `envconfig:"HELM_REPOS"` // Call `helm repo add` before the main command Prefix string `` // Prefix to use when looking up secret env vars Debug bool `` // Generate debug output and pass --debug to all helm commands Values string `` // Argument to pass to --set in applicable helm commands diff --git a/internal/helm/plan.go b/internal/helm/plan.go index ad2e5ae..303aa7c 100644 --- a/internal/helm/plan.go +++ b/internal/helm/plan.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/pelotech/drone-helm3/internal/run" "os" + "strings" ) const ( @@ -96,6 +97,7 @@ func (p *Plan) Execute() error { var upgrade = func(cfg Config) []Step { steps := initKube(cfg) + steps = append(steps, addRepos(cfg)...) if cfg.UpdateDependencies { steps = append(steps, depUpdate(cfg)...) } @@ -127,7 +129,7 @@ var uninstall = func(cfg Config) []Step { } var lint = func(cfg Config) []Step { - steps := make([]Step, 0) + steps := addRepos(cfg) if cfg.UpdateDependencies { steps = append(steps, depUpdate(cfg)...) } @@ -157,6 +159,23 @@ func initKube(cfg Config) []Step { } } +func addRepos(cfg Config) []Step { + steps := make([]Step, 0) + for _, repo := range cfg.AddRepos { + split := strings.SplitN(repo, "=", 2) + if len(split) != 2 { + fmt.Fprintf(cfg.Stderr, "Warning: skipping bad repo spec '%s'.\n", repo) + continue + } + steps = append(steps, &run.AddRepo{ + Name: split[0], + URL: split[1], + }) + } + + return steps +} + func depUpdate(cfg Config) []Step { return []Step{ &run.DepUpdate{ diff --git a/internal/helm/plan_test.go b/internal/helm/plan_test.go index 2cdde5c..f8db1bd 100644 --- a/internal/helm/plan_test.go +++ b/internal/helm/plan_test.go @@ -177,6 +177,17 @@ func (suite *PlanTestSuite) TestUpgradeWithUpdateDependencies() { suite.IsType(&run.DepUpdate{}, steps[1]) } +func (suite *PlanTestSuite) TestUpgradeWithAddRepos() { + cfg := Config{ + AddRepos: []string{ + "machine=https://github.com/harold_finch/themachine", + }, + } + steps := upgrade(cfg) + suite.Require().True(len(steps) > 1, "upgrade should generate at least two steps") + suite.IsType(&run.AddRepo{}, steps[1]) +} + func (suite *PlanTestSuite) TestUninstall() { cfg := Config{ KubeToken: "b2YgbXkgYWZmZWN0aW9u", @@ -268,6 +279,61 @@ func (suite *PlanTestSuite) TestDepUpdate() { suite.Equal(expected, update) } +func (suite *PlanTestSuite) TestAddRepos() { + cfg := 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.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.Name, "first") + suite.Equal(first.URL, "https://add.repos/one") + suite.Equal(second.Name, "second") + suite.Equal(second.URL, "https://add.repos/two") +} + +func (suite *PlanTestSuite) TestAddNoRepos() { + cfg := Config{ + AddRepos: []string{}, + } + steps := addRepos(cfg) + suite.Equal(0, len(steps), "adding no repos should take zero steps") +} + +func (suite *PlanTestSuite) TestAddReposWithMalformedRepoSpec() { + stderr := strings.Builder{} + cfg := Config{ + AddRepos: []string{ + "dwim", + }, + Stderr: &stderr, + } + steps := addRepos(cfg) + suite.Equal(len(steps), 0) + suite.Equal("Warning: skipping bad repo spec 'dwim'.\n", stderr.String()) +} + +func (suite *PlanTestSuite) TestAddReposWithEqualsInURL() { + cfg := Config{ + AddRepos: []string{ + "samaritan=https://github.com/arthur_claypool/samaritan?version=2.1", + }, + Stderr: &strings.Builder{}, + } + steps := addRepos(cfg) + suite.Require().Equal(1, len(steps)) + suite.Require().IsType(&run.AddRepo{}, steps[0]) + addRepo := steps[0].(*run.AddRepo) + suite.Equal("https://github.com/arthur_claypool/samaritan?version=2.1", addRepo.URL) +} + func (suite *PlanTestSuite) TestLint() { cfg := Config{ Chart: "./flow", @@ -291,6 +357,15 @@ func (suite *PlanTestSuite) TestLintWithUpdateDependencies() { suite.IsType(&run.DepUpdate{}, steps[0]) } +func (suite *PlanTestSuite) TestLintWithAddRepos() { + cfg := Config{ + AddRepos: []string{"friendczar=https://github.com/logan_pierce/friendczar"}, + } + steps := lint(cfg) + suite.Require().True(len(steps) > 0, "lint should return at least one step") + suite.IsType(&run.AddRepo{}, steps[0]) +} + func (suite *PlanTestSuite) TestDeterminePlanUpgradeCommand() { cfg := Config{ Command: "upgrade", From 499ab6877fca3df6f1426a766ad020cd28527eea Mon Sep 17 00:00:00 2001 From: Erin Call Date: Mon, 30 Dec 2019 13:24:57 -0800 Subject: [PATCH 12/12] Do repo error-checking in AddRepo.Prepare [#26] --- internal/helm/plan.go | 9 +------- internal/helm/plan_test.go | 41 ++---------------------------------- internal/run/addrepo.go | 18 ++++++++++------ internal/run/addrepo_test.go | 41 +++++++++++++++++++++--------------- 4 files changed, 38 insertions(+), 71 deletions(-) diff --git a/internal/helm/plan.go b/internal/helm/plan.go index 303aa7c..0a20969 100644 --- a/internal/helm/plan.go +++ b/internal/helm/plan.go @@ -4,7 +4,6 @@ import ( "fmt" "github.com/pelotech/drone-helm3/internal/run" "os" - "strings" ) const ( @@ -162,14 +161,8 @@ func initKube(cfg Config) []Step { func addRepos(cfg Config) []Step { steps := make([]Step, 0) for _, repo := range cfg.AddRepos { - split := strings.SplitN(repo, "=", 2) - if len(split) != 2 { - fmt.Fprintf(cfg.Stderr, "Warning: skipping bad repo spec '%s'.\n", repo) - continue - } steps = append(steps, &run.AddRepo{ - Name: split[0], - URL: split[1], + Repo: repo, }) } diff --git a/internal/helm/plan_test.go b/internal/helm/plan_test.go index f8db1bd..d4e4e82 100644 --- a/internal/helm/plan_test.go +++ b/internal/helm/plan_test.go @@ -293,45 +293,8 @@ func (suite *PlanTestSuite) TestAddRepos() { first := steps[0].(*run.AddRepo) second := steps[1].(*run.AddRepo) - suite.Equal(first.Name, "first") - suite.Equal(first.URL, "https://add.repos/one") - suite.Equal(second.Name, "second") - suite.Equal(second.URL, "https://add.repos/two") -} - -func (suite *PlanTestSuite) TestAddNoRepos() { - cfg := Config{ - AddRepos: []string{}, - } - steps := addRepos(cfg) - suite.Equal(0, len(steps), "adding no repos should take zero steps") -} - -func (suite *PlanTestSuite) TestAddReposWithMalformedRepoSpec() { - stderr := strings.Builder{} - cfg := Config{ - AddRepos: []string{ - "dwim", - }, - Stderr: &stderr, - } - steps := addRepos(cfg) - suite.Equal(len(steps), 0) - suite.Equal("Warning: skipping bad repo spec 'dwim'.\n", stderr.String()) -} - -func (suite *PlanTestSuite) TestAddReposWithEqualsInURL() { - cfg := Config{ - AddRepos: []string{ - "samaritan=https://github.com/arthur_claypool/samaritan?version=2.1", - }, - Stderr: &strings.Builder{}, - } - steps := addRepos(cfg) - suite.Require().Equal(1, len(steps)) - suite.Require().IsType(&run.AddRepo{}, steps[0]) - addRepo := steps[0].(*run.AddRepo) - suite.Equal("https://github.com/arthur_claypool/samaritan?version=2.1", addRepo.URL) + suite.Equal(first.Repo, "first=https://add.repos/one") + suite.Equal(second.Repo, "second=https://add.repos/two") } func (suite *PlanTestSuite) TestLint() { diff --git a/internal/run/addrepo.go b/internal/run/addrepo.go index 4dc326a..3382957 100644 --- a/internal/run/addrepo.go +++ b/internal/run/addrepo.go @@ -2,12 +2,12 @@ package run import ( "fmt" + "strings" ) // AddRepo is an execution step that calls `helm repo add` when executed. type AddRepo struct { - Name string - URL string + Repo string cmd cmd } @@ -18,13 +18,17 @@ func (a *AddRepo) Execute(_ Config) error { // Prepare gets the AddRepo ready to execute. func (a *AddRepo) Prepare(cfg Config) error { - if a.Name == "" { - return fmt.Errorf("repo name is required") + if a.Repo == "" { + return fmt.Errorf("repo is required") } - if a.URL == "" { - return fmt.Errorf("repo URL is required") + split := strings.SplitN(a.Repo, "=", 2) + if len(split) != 2 { + return fmt.Errorf("bad repo spec '%s'", a.Repo) } + name := split[0] + url := split[1] + args := make([]string, 0) if cfg.Namespace != "" { @@ -34,7 +38,7 @@ func (a *AddRepo) Prepare(cfg Config) error { args = append(args, "--debug") } - args = append(args, "repo", "add", a.Name, a.URL) + args = append(args, "repo", "add", name, url) a.cmd = command(helmBin, args...) a.cmd.Stdout(cfg.Stdout) diff --git a/internal/run/addrepo_test.go b/internal/run/addrepo_test.go index 867c039..ad42d06 100644 --- a/internal/run/addrepo_test.go +++ b/internal/run/addrepo_test.go @@ -46,8 +46,7 @@ func (suite *AddRepoTestSuite) TestPrepareAndExecute() { Stderr: &stderr, } a := AddRepo{ - Name: "edeath", - URL: "https://github.com/n_marks/e-death", + Repo: "edeath=https://github.com/n_marks/e-death", } suite.mockCmd.EXPECT(). @@ -58,8 +57,8 @@ func (suite *AddRepoTestSuite) TestPrepareAndExecute() { Times(1) suite.Require().NoError(a.Prepare(cfg)) - suite.Equal(suite.commandPath, helmBin) - suite.Equal(suite.commandArgs, []string{"repo", "add", "edeath", "https://github.com/n_marks/e-death"}) + suite.Equal(helmBin, suite.commandPath) + suite.Equal([]string{"repo", "add", "edeath", "https://github.com/n_marks/e-death"}, suite.commandArgs) suite.mockCmd.EXPECT(). Run(). @@ -69,23 +68,33 @@ func (suite *AddRepoTestSuite) TestPrepareAndExecute() { } -func (suite *AddRepoTestSuite) TestRequiredFields() { +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{ - Name: "idgen", - } + a := AddRepo{} err := a.Prepare(cfg) - suite.EqualError(err, "repo URL is required") + suite.EqualError(err, "repo is required") +} - a.Name = "" - a.URL = "https://github.com/n_marks/idgen" +func (suite *AddRepoTestSuite) TestPrepareMalformedRepo() { + a := AddRepo{ + Repo: "dwim", + } + err := a.Prepare(Config{}) + suite.EqualError(err, "bad repo spec 'dwim'") +} - err = a.Prepare(cfg) - suite.EqualError(err, "repo name is required") +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{})) + suite.Contains(suite.commandArgs, "https://github.com/arthur_claypool/samaritan?version=2.1") } func (suite *AddRepoTestSuite) TestNamespaceFlag() { @@ -95,8 +104,7 @@ func (suite *AddRepoTestSuite) TestNamespaceFlag() { Namespace: "alliteration", } a := AddRepo{ - Name: "edeath", - URL: "https://github.com/theater_guy/e-death", + Repo: "edeath=https://github.com/theater_guy/e-death", } suite.NoError(a.Prepare(cfg)) @@ -124,8 +132,7 @@ func (suite *AddRepoTestSuite) TestDebugFlag() { Stderr: &stderr, } a := AddRepo{ - Name: "edeath", - URL: "https://github.com/the_bug/e-death", + Repo: "edeath=https://github.com/the_bug/e-death", } suite.Require().NoError(a.Prepare(cfg))