From f557841e54d75e610faf45c21ff1243f8b3465c9 Mon Sep 17 00:00:00 2001 From: Oleg Atamanenko Date: Thu, 14 Jun 2018 19:52:30 -0400 Subject: [PATCH] Enable unparam linter and fix issues --- bin/pre-commit.sh | 1 + pkg/commands/addpatch.go | 7 +++--- pkg/commands/addpatch_test.go | 11 +++------ pkg/commands/addresource.go | 7 +++--- pkg/commands/addresource_test.go | 11 +++------ pkg/commands/build.go | 6 ++--- pkg/commands/build_test.go | 2 +- pkg/commands/commands.go | 24 +++++++++---------- pkg/commands/configmap.go | 3 +-- pkg/commands/configmap_test.go | 2 +- pkg/commands/diff.go | 4 ++-- pkg/commands/kustomizationfile.go | 2 +- pkg/commands/set_name_prefix.go | 7 +++--- pkg/commands/set_name_prefix_test.go | 8 ++----- .../configmap_secret_test.go | 4 ++-- pkg/configmapandsecret/util/util.go | 2 +- pkg/diff/rundiff.go | 10 +++++--- pkg/transformers/namehash.go | 10 ++++++-- 18 files changed, 57 insertions(+), 64 deletions(-) diff --git a/bin/pre-commit.sh b/bin/pre-commit.sh index 95f6e89a88..6234d7253f 100755 --- a/bin/pre-commit.sh +++ b/bin/pre-commit.sh @@ -49,6 +49,7 @@ function testGoMetalinter { --enable=goimports \ --enable=varcheck \ --enable=goconst \ + --enable=unparam \ --enable=ineffassign \ --enable=nakedret \ --enable=interfacer \ diff --git a/pkg/commands/addpatch.go b/pkg/commands/addpatch.go index 60ed0161ba..3eaaed9e51 100644 --- a/pkg/commands/addpatch.go +++ b/pkg/commands/addpatch.go @@ -19,7 +19,6 @@ package commands import ( "errors" "fmt" - "io" "github.com/spf13/cobra" @@ -32,7 +31,7 @@ type addPatchOptions struct { } // newCmdAddPatch adds the name of a file containing a patch to the kustomization file. -func newCmdAddPatch(out, errOut io.Writer, fsys fs.FileSystem) *cobra.Command { +func newCmdAddPatch(fsys fs.FileSystem) *cobra.Command { var o addPatchOptions cmd := &cobra.Command{ @@ -49,7 +48,7 @@ func newCmdAddPatch(out, errOut io.Writer, fsys fs.FileSystem) *cobra.Command { if err != nil { return err } - return o.RunAddPatch(out, errOut, fsys) + return o.RunAddPatch(fsys) }, } return cmd @@ -70,7 +69,7 @@ func (o *addPatchOptions) Complete(cmd *cobra.Command, args []string) error { } // RunAddPatch runs addPatch command (do real work). -func (o *addPatchOptions) RunAddPatch(out, errOut io.Writer, fsys fs.FileSystem) error { +func (o *addPatchOptions) RunAddPatch(fsys fs.FileSystem) error { _, err := fsys.Stat(o.patchFilePath) if err != nil { return err diff --git a/pkg/commands/addpatch_test.go b/pkg/commands/addpatch_test.go index 42cf0b447e..1705ebb0a8 100644 --- a/pkg/commands/addpatch_test.go +++ b/pkg/commands/addpatch_test.go @@ -17,8 +17,6 @@ limitations under the License. package commands import ( - "bytes" - "os" "testing" "strings" @@ -36,12 +34,11 @@ sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. ) func TestAddPatchHappyPath(t *testing.T) { - buf := bytes.NewBuffer([]byte{}) fakeFS := fs.MakeFakeFS() fakeFS.WriteFile(patchFileName, []byte(patchFileContent)) fakeFS.WriteFile(constants.KustomizationFileName, []byte(kustomizationContent)) - cmd := newCmdAddPatch(buf, os.Stderr, fakeFS) + cmd := newCmdAddPatch(fakeFS) args := []string{patchFileName} err := cmd.RunE(cmd, args) if err != nil { @@ -57,12 +54,11 @@ func TestAddPatchHappyPath(t *testing.T) { } func TestAddPatchAlreadyThere(t *testing.T) { - buf := bytes.NewBuffer([]byte{}) fakeFS := fs.MakeFakeFS() fakeFS.WriteFile(patchFileName, []byte(patchFileContent)) fakeFS.WriteFile(constants.KustomizationFileName, []byte(kustomizationContent)) - cmd := newCmdAddPatch(buf, os.Stderr, fakeFS) + cmd := newCmdAddPatch(fakeFS) args := []string{patchFileName} err := cmd.RunE(cmd, args) if err != nil { @@ -80,10 +76,9 @@ func TestAddPatchAlreadyThere(t *testing.T) { } func TestAddPatchNoArgs(t *testing.T) { - buf := bytes.NewBuffer([]byte{}) fakeFS := fs.MakeFakeFS() - cmd := newCmdAddPatch(buf, os.Stderr, fakeFS) + cmd := newCmdAddPatch(fakeFS) err := cmd.Execute() if err == nil { t.Errorf("expected error: %v", err) diff --git a/pkg/commands/addresource.go b/pkg/commands/addresource.go index 5b2896b9a5..254aa7af17 100644 --- a/pkg/commands/addresource.go +++ b/pkg/commands/addresource.go @@ -19,7 +19,6 @@ package commands import ( "errors" "fmt" - "io" "github.com/spf13/cobra" @@ -32,7 +31,7 @@ type addResourceOptions struct { } // newCmdAddResource adds the name of a file containing a resource to the kustomization file. -func newCmdAddResource(out, errOut io.Writer, fsys fs.FileSystem) *cobra.Command { +func newCmdAddResource(fsys fs.FileSystem) *cobra.Command { var o addResourceOptions cmd := &cobra.Command{ @@ -49,7 +48,7 @@ func newCmdAddResource(out, errOut io.Writer, fsys fs.FileSystem) *cobra.Command if err != nil { return err } - return o.RunAddResource(out, errOut, fsys) + return o.RunAddResource(fsys) }, } return cmd @@ -70,7 +69,7 @@ func (o *addResourceOptions) Complete(cmd *cobra.Command, args []string) error { } // RunAddResource runs addResource command (do real work). -func (o *addResourceOptions) RunAddResource(out, errOut io.Writer, fsys fs.FileSystem) error { +func (o *addResourceOptions) RunAddResource(fsys fs.FileSystem) error { _, err := fsys.Stat(o.resourceFilePath) if err != nil { return err diff --git a/pkg/commands/addresource_test.go b/pkg/commands/addresource_test.go index edab657ff3..6b8890d5d3 100644 --- a/pkg/commands/addresource_test.go +++ b/pkg/commands/addresource_test.go @@ -17,8 +17,6 @@ limitations under the License. package commands import ( - "bytes" - "os" "testing" "strings" @@ -52,12 +50,11 @@ secretGenerator: [] ) func TestAddResourceHappyPath(t *testing.T) { - buf := bytes.NewBuffer([]byte{}) fakeFS := fs.MakeFakeFS() fakeFS.WriteFile(resourceFileName, []byte(resourceFileContent)) fakeFS.WriteFile(constants.KustomizationFileName, []byte(kustomizationContent)) - cmd := newCmdAddResource(buf, os.Stderr, fakeFS) + cmd := newCmdAddResource(fakeFS) args := []string{resourceFileName} err := cmd.RunE(cmd, args) if err != nil { @@ -73,12 +70,11 @@ func TestAddResourceHappyPath(t *testing.T) { } func TestAddResourceAlreadyThere(t *testing.T) { - buf := bytes.NewBuffer([]byte{}) fakeFS := fs.MakeFakeFS() fakeFS.WriteFile(resourceFileName, []byte(resourceFileContent)) fakeFS.WriteFile(constants.KustomizationFileName, []byte(kustomizationContent)) - cmd := newCmdAddResource(buf, os.Stderr, fakeFS) + cmd := newCmdAddResource(fakeFS) args := []string{resourceFileName} err := cmd.RunE(cmd, args) if err != nil { @@ -96,10 +92,9 @@ func TestAddResourceAlreadyThere(t *testing.T) { } func TestAddResourceNoArgs(t *testing.T) { - buf := bytes.NewBuffer([]byte{}) fakeFS := fs.MakeFakeFS() - cmd := newCmdAddResource(buf, os.Stderr, fakeFS) + cmd := newCmdAddResource(fakeFS) err := cmd.Execute() if err == nil { t.Errorf("expected error: %v", err) diff --git a/pkg/commands/build.go b/pkg/commands/build.go index bedfcb6a63..4e76b9e989 100644 --- a/pkg/commands/build.go +++ b/pkg/commands/build.go @@ -35,7 +35,7 @@ type buildOptions struct { } // newCmdBuild creates a new build command. -func newCmdBuild(out, errOut io.Writer, fs fs.FileSystem) *cobra.Command { +func newCmdBuild(out io.Writer, fs fs.FileSystem) *cobra.Command { var o buildOptions cmd := &cobra.Command{ @@ -48,7 +48,7 @@ func newCmdBuild(out, errOut io.Writer, fs fs.FileSystem) *cobra.Command { if err != nil { return err } - return o.RunBuild(out, errOut, fs) + return o.RunBuild(out, fs) }, } return cmd @@ -68,7 +68,7 @@ func (o *buildOptions) Validate(args []string) error { } // RunBuild runs build command. -func (o *buildOptions) RunBuild(out, errOut io.Writer, fs fs.FileSystem) error { +func (o *buildOptions) RunBuild(out io.Writer, fs fs.FileSystem) error { l := loader.Init([]loader.SchemeLoader{loader.NewFileLoader(fs)}) absPath, err := filepath.Abs(o.kustomizationPath) diff --git a/pkg/commands/build_test.go b/pkg/commands/build_test.go index e15cebb3e8..2adad1d752 100644 --- a/pkg/commands/build_test.go +++ b/pkg/commands/build_test.go @@ -125,7 +125,7 @@ func runBuildTestCase(t *testing.T, testcaseName string, updateKustomizeExpected kustomizationPath: testcase.Filename, } buf := bytes.NewBuffer([]byte{}) - err = ops.RunBuild(buf, os.Stderr, fs) + err = ops.RunBuild(buf, fs) switch { case err != nil && len(testcase.ExpectedError) == 0: t.Errorf("unexpected error: %v", err) diff --git a/pkg/commands/commands.go b/pkg/commands/commands.go index f93a018744..1405e668e2 100644 --- a/pkg/commands/commands.go +++ b/pkg/commands/commands.go @@ -19,7 +19,6 @@ package commands import ( "flag" - "io" "os" "github.com/kubernetes-sigs/kustomize/pkg/fs" @@ -42,9 +41,10 @@ See https://github.com/kubernetes-sigs/kustomize } c.AddCommand( - newCmdBuild(stdOut, stdErr, fsys), + // TODO: Make consistent API for newCmd* functions. + newCmdBuild(stdOut, fsys), newCmdDiff(stdOut, stdErr, fsys), - newCmdEdit(stdOut, stdErr, fsys), + newCmdEdit(fsys), newCmdVersion(stdOut), ) c.PersistentFlags().AddGoFlagSet(flag.CommandLine) @@ -56,7 +56,7 @@ See https://github.com/kubernetes-sigs/kustomize } // newCmdEdit returns an instance of 'edit' subcommand. -func newCmdEdit(stdOut, stdErr io.Writer, fsys fs.FileSystem) *cobra.Command { +func newCmdEdit(fsys fs.FileSystem) *cobra.Command { c := &cobra.Command{ Use: "edit", Short: "Edits a kustomization file", @@ -71,14 +71,14 @@ func newCmdEdit(stdOut, stdErr io.Writer, fsys fs.FileSystem) *cobra.Command { Args: cobra.MinimumNArgs(1), } c.AddCommand( - newCmdAdd(stdOut, stdErr, fsys), - newCmdSet(stdOut, stdErr, fsys), + newCmdAdd(fsys), + newCmdSet(fsys), ) return c } // newAddCommand returns an instance of 'add' subcommand. -func newCmdAdd(stdOut, stdErr io.Writer, fsys fs.FileSystem) *cobra.Command { +func newCmdAdd(fsys fs.FileSystem) *cobra.Command { c := &cobra.Command{ Use: "add", Short: "Adds configmap/resource/patch to the kustomization file.", @@ -96,15 +96,15 @@ func newCmdAdd(stdOut, stdErr io.Writer, fsys fs.FileSystem) *cobra.Command { Args: cobra.MinimumNArgs(1), } c.AddCommand( - newCmdAddResource(stdOut, stdErr, fsys), - newCmdAddPatch(stdOut, stdErr, fsys), - newCmdAddConfigMap(stdErr, fsys), + newCmdAddResource(fsys), + newCmdAddPatch(fsys), + newCmdAddConfigMap(fsys), ) return c } // newSetCommand returns an instance of 'set' subcommand. -func newCmdSet(stdOut, stdErr io.Writer, fsys fs.FileSystem) *cobra.Command { +func newCmdSet(fsys fs.FileSystem) *cobra.Command { c := &cobra.Command{ Use: "set", Short: "Sets the value of different fields in kustomization file.", @@ -117,7 +117,7 @@ func newCmdSet(stdOut, stdErr io.Writer, fsys fs.FileSystem) *cobra.Command { } c.AddCommand( - newCmdSetNamePrefix(stdOut, stdErr, fsys), + newCmdSetNamePrefix(fsys), ) return c } diff --git a/pkg/commands/configmap.go b/pkg/commands/configmap.go index 9570d7e482..b161ee6842 100644 --- a/pkg/commands/configmap.go +++ b/pkg/commands/configmap.go @@ -18,7 +18,6 @@ package commands import ( "fmt" - "io" "github.com/spf13/cobra" @@ -28,7 +27,7 @@ import ( "github.com/kubernetes-sigs/kustomize/pkg/types" ) -func newCmdAddConfigMap(errOut io.Writer, fsys fs.FileSystem) *cobra.Command { +func newCmdAddConfigMap(fsys fs.FileSystem) *cobra.Command { var config dataConfig cmd := &cobra.Command{ Use: "configmap NAME [--from-file=[key=]source] [--from-literal=key1=value1]", diff --git a/pkg/commands/configmap_test.go b/pkg/commands/configmap_test.go index a16ec2037a..100ea7db5d 100644 --- a/pkg/commands/configmap_test.go +++ b/pkg/commands/configmap_test.go @@ -24,7 +24,7 @@ import ( ) func TestNewAddConfigMapIsNotNil(t *testing.T) { - if newCmdAddConfigMap(nil, fs.MakeFakeFS()) == nil { + if newCmdAddConfigMap(fs.MakeFakeFS()) == nil { t.Fatal("newCmdAddConfigMap shouldn't be nil") } } diff --git a/pkg/commands/diff.go b/pkg/commands/diff.go index 8c41fc8966..e46e9a0b44 100644 --- a/pkg/commands/diff.go +++ b/pkg/commands/diff.go @@ -42,7 +42,7 @@ func newCmdDiff(out, errOut io.Writer, fs fs.FileSystem) *cobra.Command { Use: "diff [path]", Short: "diff between customized resources and uncustomized resources", RunE: func(cmd *cobra.Command, args []string) error { - err := o.Validate(cmd, args) + err := o.Validate(args) if err != nil { return err } @@ -53,7 +53,7 @@ func newCmdDiff(out, errOut io.Writer, fs fs.FileSystem) *cobra.Command { } // Validate validates diff command. -func (o *diffOptions) Validate(cmd *cobra.Command, args []string) error { +func (o *diffOptions) Validate(args []string) error { if len(args) > 1 { return errors.New("specify one path to " + constants.KustomizationFileName) } diff --git a/pkg/commands/kustomizationfile.go b/pkg/commands/kustomizationfile.go index a2a6211916..fbd595c10a 100644 --- a/pkg/commands/kustomizationfile.go +++ b/pkg/commands/kustomizationfile.go @@ -35,7 +35,7 @@ type kustomizationFile struct { fsys fs.FileSystem } -func newKustomizationFile(mPath string, fsys fs.FileSystem) (*kustomizationFile, error) { +func newKustomizationFile(mPath string, fsys fs.FileSystem) (*kustomizationFile, error) { // nolint mf := &kustomizationFile{path: mPath, fsys: fsys} err := mf.validate() if err != nil { diff --git a/pkg/commands/set_name_prefix.go b/pkg/commands/set_name_prefix.go index 9aebe799f7..d5a425a16b 100644 --- a/pkg/commands/set_name_prefix.go +++ b/pkg/commands/set_name_prefix.go @@ -18,7 +18,6 @@ package commands import ( "errors" - "io" "github.com/spf13/cobra" @@ -31,7 +30,7 @@ type setNamePrefixOptions struct { } // newCmdSetNamePrefix sets the value of the namePrefix field in the kustomization. -func newCmdSetNamePrefix(out, errOut io.Writer, fsys fs.FileSystem) *cobra.Command { +func newCmdSetNamePrefix(fsys fs.FileSystem) *cobra.Command { var o setNamePrefixOptions cmd := &cobra.Command{ @@ -52,7 +51,7 @@ and overwrite the value with "acme-" if the field does exist. if err != nil { return err } - return o.RunSetNamePrefix(out, errOut, fsys) + return o.RunSetNamePrefix(fsys) }, } return cmd @@ -74,7 +73,7 @@ func (o *setNamePrefixOptions) Complete(cmd *cobra.Command, args []string) error } // RunSetNamePrefix runs setNamePrefix command (does real work). -func (o *setNamePrefixOptions) RunSetNamePrefix(out, errOut io.Writer, fsys fs.FileSystem) error { +func (o *setNamePrefixOptions) RunSetNamePrefix(fsys fs.FileSystem) error { mf, err := newKustomizationFile(constants.KustomizationFileName, fsys) if err != nil { return err diff --git a/pkg/commands/set_name_prefix_test.go b/pkg/commands/set_name_prefix_test.go index c97bd522b8..fee1907579 100644 --- a/pkg/commands/set_name_prefix_test.go +++ b/pkg/commands/set_name_prefix_test.go @@ -17,8 +17,6 @@ limitations under the License. package commands import ( - "bytes" - "os" "testing" "strings" @@ -32,11 +30,10 @@ const ( ) func TestSetNamePrefixHappyPath(t *testing.T) { - buf := bytes.NewBuffer([]byte{}) fakeFS := fs.MakeFakeFS() fakeFS.WriteFile(constants.KustomizationFileName, []byte(kustomizationContent)) - cmd := newCmdSetNamePrefix(buf, os.Stderr, fakeFS) + cmd := newCmdSetNamePrefix(fakeFS) args := []string{goodPrefixValue} err := cmd.RunE(cmd, args) if err != nil { @@ -52,10 +49,9 @@ func TestSetNamePrefixHappyPath(t *testing.T) { } func TestSetNamePrefixNoArgs(t *testing.T) { - buf := bytes.NewBuffer([]byte{}) fakeFS := fs.MakeFakeFS() - cmd := newCmdSetNamePrefix(buf, os.Stderr, fakeFS) + cmd := newCmdSetNamePrefix(fakeFS) err := cmd.Execute() if err == nil { t.Errorf("expected error: %v", err) diff --git a/pkg/configmapandsecret/configmap_secret_test.go b/pkg/configmapandsecret/configmap_secret_test.go index 03be6b31eb..e4de510f2b 100644 --- a/pkg/configmapandsecret/configmap_secret_test.go +++ b/pkg/configmapandsecret/configmap_secret_test.go @@ -234,10 +234,10 @@ func TestObjectConvertToUnstructured(t *testing.T) { for _, tc := range testCases { actual, err := objectToUnstructured(tc.input) if err != nil { - t.Fatalf("unexpected error: %v", err) + t.Fatalf("%s: unexpected error: %v", tc.description, err) } if !reflect.DeepEqual(actual, tc.expected) { - t.Fatalf("%#v\ndoesn't match expected\n%#v\n", actual, tc.expected) + t.Fatalf("%s: %#v\ndoesn't match expected\n%#v\n", tc.description, actual, tc.expected) } } } diff --git a/pkg/configmapandsecret/util/util.go b/pkg/configmapandsecret/util/util.go index 7d0bd72880..62723b8466 100644 --- a/pkg/configmapandsecret/util/util.go +++ b/pkg/configmapandsecret/util/util.go @@ -30,7 +30,7 @@ import ( ) // ParseRFC3339 parses an RFC3339 date in either RFC3339Nano or RFC3339 format. -func ParseRFC3339(s string, nowFn func() metav1.Time) (metav1.Time, error) { +func ParseRFC3339(s string) (metav1.Time, error) { if t, timeErr := time.Parse(time.RFC3339Nano, s); timeErr == nil { return metav1.Time{Time: t}, nil } diff --git a/pkg/diff/rundiff.go b/pkg/diff/rundiff.go index 1c04fb0c17..5b3475f709 100644 --- a/pkg/diff/rundiff.go +++ b/pkg/diff/rundiff.go @@ -26,18 +26,22 @@ import ( ) // RunDiff runs system diff program to compare two Maps. -func RunDiff(raw, transformed resmap.ResMap, out, errOut io.Writer) error { +func RunDiff(raw, transformed resmap.ResMap, out, errOut io.Writer) (err error) { transformedDir, err := writeYamlToNewDir(transformed, "transformed") if err != nil { return err } - defer transformedDir.delete() + defer func() { + err = transformedDir.delete() + }() noopDir, err := writeYamlToNewDir(raw, "noop") if err != nil { return err } - defer noopDir.delete() + defer func() { + err = noopDir.delete() + }() return newProgram(out, errOut).run(noopDir.name(), transformedDir.name()) } diff --git a/pkg/transformers/namehash.go b/pkg/transformers/namehash.go index 52e72f51a7..732ebae14e 100644 --- a/pkg/transformers/namehash.go +++ b/pkg/transformers/namehash.go @@ -43,10 +43,16 @@ func (o *nameHashTransformer) Transform(m resmap.ResMap) error { for id, res := range m { switch { case selectByGVK(id.Gvk(), &schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"}): - appendHashForConfigMap(res) + err := appendHashForConfigMap(res) + if err != nil { + return err + } case selectByGVK(id.Gvk(), &schema.GroupVersionKind{Version: "v1", Kind: "Secret"}): - appendHashForSecret(res) + err := appendHashForSecret(res) + if err != nil { + return err + } } } return nil