From 49027f415142f8daeb8c4fb72e70ae4bebd7628e Mon Sep 17 00:00:00 2001 From: Ian Howell Date: Wed, 8 Jan 2020 13:54:13 -0600 Subject: [PATCH] Tighten the restrictions of the linter This change causes the linter to be a bit more complainy. The hope is that this will cut down on some of the more pedantic issues being caught in code reviews, and thus reduce the overall time a change spends in the review process. This change includes various changes to the codebase to bring it up to the new standards. Change-Id: I570d304bca5554404354f972d8a2743279a0171b --- .golangci.yaml | 122 +++++++++++++----- cmd/bootstrap/bootstrap.go | 4 +- cmd/bootstrap/bootstrap_remotedirect.go | 1 - cmd/config/config_test.go | 1 - cmd/config/get_cluster.go | 1 - cmd/config/get_context.go | 2 - cmd/config/get_context_test.go | 1 - cmd/config/set_cluster.go | 16 ++- cmd/config/set_cluster_test.go | 2 - cmd/config/set_context.go | 2 - cmd/config/set_context_test.go | 4 - .../missing.golden | 2 +- .../missing.golden | 2 +- cmd/root.go | 2 - pkg/bootstrap/cloudinit/cloud-init_test.go | 1 - pkg/bootstrap/isogen/command.go | 5 +- pkg/bootstrap/isogen/command_test.go | 1 - pkg/config/cmds_test.go | 1 - pkg/config/config.go | 21 +-- pkg/config/config_test.go | 1 - pkg/config/test_utils.go | 9 +- pkg/container/container.go | 2 - pkg/container/container_docker_test.go | 13 +- pkg/document/bundle.go | 34 ++--- pkg/document/bundle_test.go | 18 --- pkg/document/document.go | 7 +- pkg/document/document_test.go | 11 -- pkg/document/repo/repo.go | 1 - pkg/document/repo/repo_test.go | 20 ++- pkg/errors/common.go | 3 +- pkg/remote/redfish/redfish.go | 7 +- pkg/remote/redfish/redfish_test.go | 7 - pkg/remote/redfish/utils.go | 17 +-- pkg/remote/redfish/utils_test.go | 19 +-- pkg/remote/remote_direct.go | 2 - pkg/remote/remote_direct_test.go | 5 - pkg/util/writefiles.go | 1 - pkg/util/yaml/writer.go | 1 - pkg/util/yaml/writer_test.go | 2 - 39 files changed, 170 insertions(+), 201 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index ccc481806..a65292326 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,5 +1,5 @@ # This file contains all available configuration options -# with their default values. +# with their documentation # options for analysis running run: @@ -61,39 +61,92 @@ output: # all available settings of specific linters linters-settings: + + dupl: + # tokens count to trigger issue, 150 by default if not set here + threshold: 150 + errcheck: # report about not checking of errors in type assetions: `a := b.(MyStruct)`; # default is false: such cases aren't reported by default. - check-type-assertions: false + check-type-assertions: true # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`; # default is false: such cases aren't reported by default. - check-blank: false + check-blank: true # path to a file containing a list of functions to exclude from checking # see https://github.com/kisielk/errcheck#excluding-functions for details # exclude: /path/to/file.txt - govet: - # report about shadowed variables - check-shadowing: true + + goconst: + # minimal length of string constant, 3 by default + min-len: 4 + # minimal occurrences count to trigger, 3 by default + min-occurrences: 5 + + gocritic: + # Which checks should be enabled; can't be combined with 'disabled-checks'; + # See https://go-critic.github.io/overview#checks-overview + # To check which checks are enabled run `GL_DEBUG=gocritic golangci-lint run` + # By default list of stable checks is used. + enabled-checks: + - appendAssign + - appendCombine + - assignOp + - captLocal + - caseOrder + - commentedOutCode + - commentedOutImport + - defaultCaseOrder + - dupArg + - dupBranchBody + - dupCase + - dupSubExpr + - elseif + - equalFold + - flagDeref + - ifElseChain + - regexpMust + - singleCaseSwitch + - sloppyLen + - switchTrue + - typeAssertChain + - typeSwitchVar + - underef + - unlambda + - unslice + + # Which checks should be disabled; can't be combined with 'enabled-checks'; default is empty + # disabled-checks: + # - regexpMust + + # Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint run` to see all tags and checks. + # Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags". + # enabled-tags: + # - performance + + settings: # settings passed to gocritic + captLocal: + paramsOnly: false + + gocyclo: + # minimal code complexity to report, 30 by default (but we recommend 10-20) + min-complexity: 15 + gofmt: # simplify code: gofmt with `-s` option, true by default simplify: true + goimports: # put imports beginning with prefix after 3rd-party packages; # it's a comma-separated list of prefixes local-prefixes: opendev.org/airship/airshipctl - gocyclo: - # minimal code complexity to report, 30 by default (but we recommend 10-20) - min-complexity: 15 - dupl: - # tokens count to trigger issue, 150 by default - threshold: 100 - goconst: - # minimal length of string constant, 3 by default - min-len: 3 - # minimal occurrences count to trigger, 3 by default - min-occurrences: 3 + + govet: + # report about shadowed variables + check-shadowing: true + misspell: # Correct spellings using locale preferences for US or UK. # Default is to use a neutral variety of English. @@ -101,47 +154,45 @@ linters-settings: locale: US # ignore-words: # - someword + + nakedret: + # make an issue if func has more lines of code than this setting and it has naked returns; default is 30 + max-func-lines: 10 + lll: # max line length, lines longer will be reported. Default is 120. # '\t' is counted as 1 character by default, and can be changed with the tab-width option line-length: 120 # tab width in spaces. Default to 1. tab-width: 1 - unused: - # treat code as a program (not a library) and report unused exported identifiers; default is false. - # XXX: if you enable this setting, unused will report a lot of false-positives in text editors: - # if it's called for subdir of a project it can't find funcs usages. All text editor integrations - # with golangci-lint call it on a directory with the changed file. - check-exported: false + unparam: # Inspect exported functions, default is false. Set to true if no external program/library imports your code. # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors: # if it's called for subdir of a project it can't find external interfaces. All text editor integrations # with golangci-lint call it on a directory with the changed file. check-exported: false - nakedret: - # make an issue if func has more lines of code than this setting and it has naked returns; default is 30 - max-func-lines: 10 - prealloc: - # XXX: we don't recommend using this linter before doing performance profiling. - # For most programs usage of prealloc will be a premature optimization. - # Report preallocation suggestions only on simple loops that have no returns/breaks/continues/gotos in them. - # True by default. - simple: true - range-loops: true # Report preallocation suggestions on range loops, true by default - for-loops: false # Report preallocation suggestions on for loops, false by default + unused: + # treat code as a program (not a library) and report unused exported identifiers; default is false. + # XXX: if you enable this setting, unused will report a lot of false-positives in text editors: + # if it's called for subdir of a project it can't find funcs usages. All text editor integrations + # with golangci-lint call it on a directory with the changed file. + check-exported: false linters: disable-all: true enable: - dupl # Tool for code clone detection + - errcheck # Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases - goconst # Finds repeated strings that could be replaced by a constant - gocritic # The most opinionated Go source code linter - gocyclo # Computes and checks the cyclomatic complexity of functions - gofmt # Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification - goimports # Goimports does everything that gofmt does. Additionally it checks unused imports - gosec # Inspects source code for security problems + - govet # Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string + - ineffassign # Detects when assignments to existing variables are not used - interfacer # Linter that suggests narrower interface types - lll # Reports long lines - misspell # Finds commonly misspelled English words in comments @@ -150,3 +201,6 @@ linters: - scopelint # Scopelint checks for unpinned variables in go programs - unconvert # Remove unnecessary type conversions - unparam # Reports unused function parameters + - unused # Checks Go code for unused constants, variables, functions and types + - varcheck # Finds unused global variables and constants + - whitespace # Tool for detection of leading and trailing whitespace NOTE(howell): This linter does _not_ check for trailing whitespace in multiline strings diff --git a/cmd/bootstrap/bootstrap.go b/cmd/bootstrap/bootstrap.go index ce39fd51e..709d42e19 100644 --- a/cmd/bootstrap/bootstrap.go +++ b/cmd/bootstrap/bootstrap.go @@ -13,8 +13,8 @@ func NewBootstrapCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Co Short: "Bootstrap ephemeral Kubernetes cluster", } - ISOGenCmd := NewISOGenCommand(bootstrapRootCmd, rootSettings) - bootstrapRootCmd.AddCommand(ISOGenCmd) + isoGenCmd := NewISOGenCommand(bootstrapRootCmd, rootSettings) + bootstrapRootCmd.AddCommand(isoGenCmd) remoteDirectCmd := NewRemoteDirectCommand(rootSettings) bootstrapRootCmd.AddCommand(remoteDirectCmd) diff --git a/cmd/bootstrap/bootstrap_remotedirect.go b/cmd/bootstrap/bootstrap_remotedirect.go index 149dec3a1..67fb28fc9 100644 --- a/cmd/bootstrap/bootstrap_remotedirect.go +++ b/cmd/bootstrap/bootstrap_remotedirect.go @@ -59,7 +59,6 @@ func NewRemoteDirectCommand(rootSettings *environment.AirshipCTLSettings) *cobra Use: "remotedirect", Short: "Bootstrap ephemeral node", RunE: func(cmd *cobra.Command, args []string) error { - /* TODO: Get config from settings.GetCurrentContext() and remove cli arguments */ /* Trigger remotedirect based on remote type */ diff --git a/cmd/config/config_test.go b/cmd/config/config_test.go index c6531a1e3..50fdec507 100644 --- a/cmd/config/config_test.go +++ b/cmd/config/config_test.go @@ -27,7 +27,6 @@ const ( ) func TestConfig(t *testing.T) { - cmdTests := []*testutil.CmdTest{ { Name: "config-cmd-with-defaults", diff --git a/cmd/config/get_cluster.go b/cmd/config/get_cluster.go index a0c7e0954..5e00d6443 100644 --- a/cmd/config/get_cluster.go +++ b/cmd/config/get_cluster.go @@ -39,7 +39,6 @@ airshipctl config get-cluster e2e --%v=ephemeral`, config.FlagClusterType) // NewCmdConfigGetCluster returns a Command instance for 'config -Cluster' sub command func NewCmdConfigGetCluster(rootSettings *environment.AirshipCTLSettings) *cobra.Command { - theCluster := &config.ClusterOptions{} getclustercmd := &cobra.Command{ Use: "get-cluster NAME", diff --git a/cmd/config/get_context.go b/cmd/config/get_context.go index b77048457..a4abdda9e 100644 --- a/cmd/config/get_context.go +++ b/cmd/config/get_context.go @@ -46,7 +46,6 @@ airshipctl config get-context e2e`, // NewCmdConfigGetContext returns a Command instance for 'config -Context' sub command func NewCmdConfigGetContext(rootSettings *environment.AirshipCTLSettings) *cobra.Command { - theContext := &config.ContextOptions{} getcontextcmd := &cobra.Command{ Use: "get-context NAME", @@ -68,7 +67,6 @@ func NewCmdConfigGetContext(rootSettings *environment.AirshipCTLSettings) *cobra func gctxInitFlags(o *config.ContextOptions, getcontextcmd *cobra.Command) { getcontextcmd.Flags().BoolVar(&o.CurrentContext, config.FlagCurrentContext, false, config.FlagCurrentContext+" to retrieve the current context entry in airshipctl config") - } // runGetContext performs the execution of 'config get-Context' sub command diff --git a/cmd/config/get_context_test.go b/cmd/config/get_context_test.go index 9f85ec801..7db2d1230 100644 --- a/cmd/config/get_context_test.go +++ b/cmd/config/get_context_test.go @@ -99,7 +99,6 @@ func TestNoContextsGetContextCmd(t *testing.T) { } func getNamedTestContext(contextName string) *config.Context { - kContext := &kubeconfig.Context{ Namespace: "dummy_namespace", AuthInfo: "dummy_user", diff --git a/cmd/config/set_cluster.go b/cmd/config/set_cluster.go index 6a042e197..c730b5346 100644 --- a/cmd/config/set_cluster.go +++ b/cmd/config/set_cluster.go @@ -17,12 +17,14 @@ limitations under the License. package config import ( + "errors" "fmt" "github.com/spf13/cobra" "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/environment" + conferrors "opendev.org/airship/airshipctl/pkg/errors" "opendev.org/airship/airshipctl/pkg/log" ) @@ -87,7 +89,6 @@ func NewCmdConfigSetCluster(rootSettings *environment.AirshipCTLSettings) *cobra } func scInitFlags(o *config.ClusterOptions, setclustercmd *cobra.Command) { - setclustercmd.Flags().StringVar(&o.Server, config.FlagAPIServer, o.Server, config.FlagAPIServer+" for the cluster entry in airshipctl config") @@ -106,11 +107,9 @@ func scInitFlags(o *config.ClusterOptions, setclustercmd *cobra.Command) { setclustercmd.Flags().BoolVar(&o.EmbedCAData, config.FlagEmbedCerts, false, config.FlagEmbedCerts+" for the cluster entry in airshipctl config") - } func runSetCluster(o *config.ClusterOptions, rootSettings *environment.AirshipCTLSettings) (bool, error) { - clusterWasModified := false err := o.Validate() if err != nil { @@ -119,9 +118,14 @@ func runSetCluster(o *config.ClusterOptions, rootSettings *environment.AirshipCT airconfig := rootSettings.Config() cluster, err := airconfig.GetCluster(o.Name, o.ClusterType) - // Safe to ignore the error. Simple means I didnt find the cluster - if cluster == nil { - // New Cluster + if err != nil { + var cerr conferrors.ErrMissingConfig + if !errors.As(err, &cerr) { + // An error occurred, but it wasn't a "missing" config error. + return clusterWasModified, err + } + + // Cluster didn't exist, create it _, err := airconfig.AddCluster(o) if err != nil { return clusterWasModified, err diff --git a/cmd/config/set_cluster_test.go b/cmd/config/set_cluster_test.go index 12e7af1f7..62efb3035 100644 --- a/cmd/config/set_cluster_test.go +++ b/cmd/config/set_cluster_test.go @@ -116,7 +116,6 @@ func TestSetClusterWithCAFileData(t *testing.T) { } func TestSetCluster(t *testing.T) { - conf := config.InitConfig(t) tname := testCluster @@ -188,7 +187,6 @@ func TestModifyCluster(t *testing.T) { } func (test setClusterTest) run(t *testing.T) { - // Get the Environment settings := &environment.AirshipCTLSettings{} settings.SetConfig(test.config) diff --git a/cmd/config/set_context.go b/cmd/config/set_context.go index abaa157dc..d4f3c8661 100644 --- a/cmd/config/set_context.go +++ b/cmd/config/set_context.go @@ -77,7 +77,6 @@ func NewCmdConfigSetContext(rootSettings *environment.AirshipCTLSettings) *cobra } func sctxInitFlags(o *config.ContextOptions, setcontextcmd *cobra.Command) { - setcontextcmd.Flags().BoolVar(&o.CurrentContext, config.FlagCurrentContext, false, config.FlagCurrentContext+" for the context entry in airshipctl config") @@ -95,7 +94,6 @@ func sctxInitFlags(o *config.ContextOptions, setcontextcmd *cobra.Command) { setcontextcmd.Flags().StringVar(&o.ClusterType, config.FlagClusterType, "", config.FlagClusterType+" for the context entry in airshipctl config") - } func runSetContext(o *config.ContextOptions, airconfig *config.Config) (bool, error) { diff --git a/cmd/config/set_context_test.go b/cmd/config/set_context_test.go index 38bc13a8b..012e314d3 100644 --- a/cmd/config/set_context_test.go +++ b/cmd/config/set_context_test.go @@ -44,7 +44,6 @@ type setContextTest struct { } func TestConfigSetContext(t *testing.T) { - cmdTests := []*testutil.CmdTest{ { Name: "config-cmd-set-context-with-help", @@ -59,7 +58,6 @@ func TestConfigSetContext(t *testing.T) { } func TestSetContext(t *testing.T) { - conf := config.InitConfig(t) tname := "dummycontext" @@ -149,7 +147,6 @@ func TestModifyContext(t *testing.T) { } func (test setContextTest) run(t *testing.T) { - // Get the Environment settings := &environment.AirshipCTLSettings{} settings.SetConfig(test.config) @@ -186,5 +183,4 @@ func (test setContextTest) run(t *testing.T) { if len(test.expected) != 0 { assert.EqualValuesf(t, buf.String(), test.expected, "expected %v, but got %v", test.expected, buf.String()) } - } diff --git a/cmd/config/testdata/TestGetClusterCmdGoldenOutput/missing.golden b/cmd/config/testdata/TestGetClusterCmdGoldenOutput/missing.golden index 9881e5c9e..9a3253af2 100644 --- a/cmd/config/testdata/TestGetClusterCmdGoldenOutput/missing.golden +++ b/cmd/config/testdata/TestGetClusterCmdGoldenOutput/missing.golden @@ -1,4 +1,4 @@ -Error: Cluster clusterMissing information was not found in the configuration. +Error: Missing configuration: Cluster with name 'clusterMissing' of type 'target' Usage: get-cluster NAME [flags] diff --git a/cmd/config/testdata/TestGetContextCmdGoldenOutput/missing.golden b/cmd/config/testdata/TestGetContextCmdGoldenOutput/missing.golden index 8862d63a0..8a7ba8c89 100644 --- a/cmd/config/testdata/TestGetContextCmdGoldenOutput/missing.golden +++ b/cmd/config/testdata/TestGetContextCmdGoldenOutput/missing.golden @@ -1,4 +1,4 @@ -Error: Missing configuration +Error: Missing configuration: Context with name 'contextMissing' Usage: get-context NAME [flags] diff --git a/cmd/root.go b/cmd/root.go index 847da6e91..ef846170a 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -38,7 +38,6 @@ func NewRootCmd(out io.Writer) (*cobra.Command, *environment.AirshipCTLSettings, SilenceUsage: true, PersistentPreRun: func(cmd *cobra.Command, args []string) { log.Init(settings.Debug, cmd.OutOrStderr()) - }, } rootCmd.SetOutput(out) @@ -54,7 +53,6 @@ func NewRootCmd(out io.Writer) (*cobra.Command, *environment.AirshipCTLSettings, // AddDefaultAirshipCTLCommands is a convenience function for adding all of the // default commands to airshipctl func AddDefaultAirshipCTLCommands(cmd *cobra.Command, settings *environment.AirshipCTLSettings) *cobra.Command { - //cmd.AddCommand(argo.NewCommand()) cmd.AddCommand(bootstrap.NewBootstrapCommand(settings)) cmd.AddCommand(cluster.NewClusterCommand(settings)) cmd.AddCommand(completion.NewCompletionCommand()) diff --git a/pkg/bootstrap/cloudinit/cloud-init_test.go b/pkg/bootstrap/cloudinit/cloud-init_test.go index e7b76634f..689ede885 100644 --- a/pkg/bootstrap/cloudinit/cloud-init_test.go +++ b/pkg/bootstrap/cloudinit/cloud-init_test.go @@ -11,7 +11,6 @@ import ( ) func TestGetCloudData(t *testing.T) { - fSys := testutil.SetupTestFs(t, "testdata") bundle, err := document.NewBundle(fSys, "/", "/") require.NoError(t, err, "Building Bundle Failed") diff --git a/pkg/bootstrap/isogen/command.go b/pkg/bootstrap/isogen/command.go index f9d159b13..c32ac41cf 100644 --- a/pkg/bootstrap/isogen/command.go +++ b/pkg/bootstrap/isogen/command.go @@ -58,7 +58,6 @@ func GenerateBootstrapIso(settings *Settings, args []string) error { } log.Print("Checking artifacts") return verifyArtifacts(cfg) - } func verifyInputs(cfg *Config, args []string) error { @@ -124,6 +123,10 @@ func generateBootstrapIso( var fls map[string][]byte fls, err = getContainerCfg(cfg, userData, netConf) + if err != nil { + return err + } + if err = util.WriteFiles(fls, 0600); err != nil { return err } diff --git a/pkg/bootstrap/isogen/command_test.go b/pkg/bootstrap/isogen/command_test.go index a7af6a802..550cbdca4 100644 --- a/pkg/bootstrap/isogen/command_test.go +++ b/pkg/bootstrap/isogen/command_test.go @@ -179,5 +179,4 @@ func TestVerifyInputs(t *testing.T) { actualErr := verifyInputs(tt.cfg, tt.args) assert.Equal(t, tt.expectedErr, actualErr) } - } diff --git a/pkg/config/cmds_test.go b/pkg/config/cmds_test.go index cae23e8b8..b5a9ef9d9 100644 --- a/pkg/config/cmds_test.go +++ b/pkg/config/cmds_test.go @@ -67,5 +67,4 @@ func TestValidateContext(t *testing.T) { // Valid Data case err := co.Validate() assert.NoError(t, err) - } diff --git a/pkg/config/config.go b/pkg/config/config.go index 74288ec82..3c571e209 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -240,7 +240,6 @@ func (c *Config) reconcileAuthInfos() { if c.AuthInfos[key] == nil && authinfo != nil { // Add the reference c.AuthInfos[key] = NewAuthInfo() - } } // Checking if there is any AuthInfo reference in airship config that does not match @@ -392,14 +391,12 @@ func (c *Config) ContextNames() []string { func (c *Config) GetCluster(cName, cType string) (*Cluster, error) { _, exists := c.Clusters[cName] if !exists { - return nil, errors.New("Cluster " + cName + - " information was not found in the configuration.") + return nil, conferrors.ErrMissingConfig{What: fmt.Sprintf("Cluster with name '%s' of type '%s'", cName, cType)} } // Alternative to this would be enhance Cluster.String() to embedd the appropriate kubeconfig cluster information cluster, exists := c.Clusters[cName].ClusterTypes[cType] if !exists { - return nil, errors.New("Cluster " + cName + " of type " + cType + - " information was not found in the configuration.") + return nil, conferrors.ErrMissingConfig{What: fmt.Sprintf("Cluster with name '%s' of type '%s'", cName, cType)} } return cluster, nil } @@ -426,7 +423,6 @@ func (c *Config) AddCluster(theCluster *ClusterOptions) (*Cluster, error) { // Ok , I have initialized structs for the Cluster information // We can use Modify to populate the correct information return c.ModifyCluster(nCluster, theCluster) - } func (c *Config) ModifyCluster(cluster *Cluster, theCluster *ClusterOptions) (*Cluster, error) { @@ -470,8 +466,8 @@ func (c *Config) ModifyCluster(cluster *Cluster, theCluster *ClusterOptions) (*C } } return cluster, nil - } + func (c *Config) GetClusters() ([]*Cluster, error) { clusters := []*Cluster{} for _, cName := range c.ClusterNames() { @@ -493,7 +489,7 @@ func (c *Config) GetClusters() ([]*Cluster, error) { func (c *Config) GetContext(cName string) (*Context, error) { context, exists := c.Contexts[cName] if !exists { - return nil, conferrors.ErrMissingConfig{} + return nil, conferrors.ErrMissingConfig{What: fmt.Sprintf("Context with name '%s'", cName)} } return context, nil } @@ -528,7 +524,6 @@ func (c *Config) AddContext(theContext *ContextOptions) *Context { // We can use Modify to populate the correct information c.ModifyContext(nContext, theContext) return nContext - } func (c *Config) ModifyContext(context *Context, theContext *ContextOptions) { @@ -596,12 +591,7 @@ func (c *Config) CurrentContextManifest() (*Manifest, error) { // Purge removes the config file func (c *Config) Purge() error { - //configFile := c.ConfigFile() - err := os.Remove(c.loadedConfigPath) - if err != nil { - return err - } - return nil + return os.Remove(c.loadedConfigPath) } func (c *Config) Equal(d *Config) bool { @@ -699,7 +689,6 @@ func (c *Context) ClusterType() string { clusterName := NewClusterComplexName() clusterName.FromName(c.NameInKubeconf) return clusterName.ClusterType() - } // AuthInfo functions diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 1a33bc45e..e6b839c43 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -395,5 +395,4 @@ func TestGetContext(t *testing.T) { // Test Wrong Cluster _, err = conf.GetContext("unknown") assert.Error(t, err) - } diff --git a/pkg/config/test_utils.go b/pkg/config/test_utils.go index 86773c1d7..a0dbe5b5b 100644 --- a/pkg/config/test_utils.go +++ b/pkg/config/test_utils.go @@ -93,6 +93,8 @@ func DummyManifest() *Manifest { } func DummyRepository() *Repository { + // TODO(howell): handle this error + //nolint: errcheck url, _ := url.Parse("http://dummy.url.com") return &Repository{ Url: url, @@ -121,12 +123,15 @@ func DummyClusterPurpose() *ClusterPurpose { func InitConfig(t *testing.T) *Config { t.Helper() testDir, err := ioutil.TempDir("", "airship-test") + require.NoError(t, err) configPath := filepath.Join(testDir, "config") - ioutil.WriteFile(configPath, []byte(testConfigYAML), 0666) + err = ioutil.WriteFile(configPath, []byte(testConfigYAML), 0666) + require.NoError(t, err) kubeConfigPath := filepath.Join(testDir, "kubeconfig") - ioutil.WriteFile(kubeConfigPath, []byte(testKubeConfigYAML), 0666) + err = ioutil.WriteFile(kubeConfigPath, []byte(testKubeConfigYAML), 0666) + require.NoError(t, err) conf := NewConfig() diff --git a/pkg/container/container.go b/pkg/container/container.go index 8e6c91d4e..95bf032b5 100644 --- a/pkg/container/container.go +++ b/pkg/container/container.go @@ -30,8 +30,6 @@ func NewContainer(ctx *context.Context, driver string, url string) (Container, e } return NewDockerContainer(ctx, url, cli) default: - return nil, ErrContainerDrvNotSupported{Driver: driver} } - } diff --git a/pkg/container/container_docker_test.go b/pkg/container/container_docker_test.go index 785f5268a..34d0d62e0 100644 --- a/pkg/container/container_docker_test.go +++ b/pkg/container/container_docker_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" @@ -139,7 +140,6 @@ func TestGetCmd(t *testing.T) { Cmd: []string{"testCmd"}, }, }, nil, nil - }, }, expectedResult: []string{"testCmd"}, @@ -166,9 +166,7 @@ func TestGetCmd(t *testing.T) { assert.Equal(t, tt.expectedErr, actualErr) assert.Equal(t, tt.expectedResult, actualRes) - } - } func TestGetImageId(t *testing.T) { @@ -242,7 +240,8 @@ func TestImagePull(t *testing.T) { func TestGetId(t *testing.T) { cnt := getDockerContainerMock(mockDockerClient{}) - cnt.RunCommand([]string{"testCmd"}, nil, nil, []string{}, false) + err := cnt.RunCommand([]string{"testCmd"}, nil, nil, []string{}, false) + require.NoError(t, err) actialId := cnt.GetId() assert.Equal(t, "testID", actialId) @@ -333,7 +332,6 @@ func TestRunCommand(t *testing.T) { debug: false, mockDockerClient: mockDockerClient{ containerWait: func() (<-chan container.ContainerWaitOKBody, <-chan error) { - errC := make(chan error) go func() { errC <- containerWaitError @@ -351,7 +349,6 @@ func TestRunCommand(t *testing.T) { debug: false, mockDockerClient: mockDockerClient{ containerWait: func() (<-chan container.ContainerWaitOKBody, <-chan error) { - resC := make(chan container.ContainerWaitOKBody) go func() { resC <- container.ContainerWaitOKBody{StatusCode: 1} @@ -412,7 +409,9 @@ func TestRunCommandOutput(t *testing.T) { var actualResBytes []byte if actualRes != nil { - actualResBytes, _ = ioutil.ReadAll(actualRes) + var err error + actualResBytes, err = ioutil.ReadAll(actualRes) + require.NoError(t, err) } else { actualResBytes = []byte{} } diff --git a/pkg/document/bundle.go b/pkg/document/bundle.go index 45165156a..7b906f842 100644 --- a/pkg/document/bundle.go +++ b/pkg/document/bundle.go @@ -54,8 +54,7 @@ type Bundle interface { // NewBundle is a convenience function to create a new bundle // Over time, it will evolve to support allowing more control // for kustomize plugins -func NewBundle(fSys fs.FileSystem, kustomizePath string, outputPath string) (Bundle, error) { - +func NewBundle(fSys fs.FileSystem, kustomizePath string, outputPath string) (bundle Bundle, err error) { var options = KustomizeBuildOptions{ KustomizationPath: kustomizePath, OutputPath: outputPath, @@ -64,11 +63,15 @@ func NewBundle(fSys fs.FileSystem, kustomizePath string, outputPath string) (Bun } // init an empty bundle factory - var bundle Bundle = &BundleFactory{} + bundle = &BundleFactory{} // set the fs and build options we will use - bundle.SetFileSystem(fSys) - bundle.SetKustomizeBuildOptions(options) + if err = bundle.SetFileSystem(fSys); err != nil { + return nil, err + } + if err = bundle.SetKustomizeBuildOptions(options); err != nil { + return nil, err + } // boiler plate to allow us to run Kustomize build uf := kunstruct.NewKunstructuredFactoryImpl() @@ -84,7 +87,11 @@ func NewBundle(fSys fs.FileSystem, kustomizePath string, outputPath string) (Bun if err != nil { return bundle, err } - defer ldr.Cleanup() + + defer func() { + err = ldr.Cleanup() + }() + kt, err := target.NewKustTarget(ldr, rf, pf, pl) if err != nil { return bundle, err @@ -92,13 +99,12 @@ func NewBundle(fSys fs.FileSystem, kustomizePath string, outputPath string) (Bun // build a resource map of kustomize rendered objects m, err := kt.MakeCustomizedResMap() - bundle.SetKustomizeResourceMap(m) if err != nil { return bundle, err } + err = bundle.SetKustomizeResourceMap(m) - return bundle, nil - + return bundle, err } // GetKustomizeResourceMap returns a Kustomize Resource Map for this bundle @@ -175,7 +181,6 @@ func (b *BundleFactory) GetByName(name string) (Document, error) { // Select offers a direct interface to pass a Kustomize Selector to the bundle // returning Documents that match the criteria func (b *BundleFactory) Select(selector types.Selector) ([]Document, error) { - // use the kustomize select method resources, err := b.ResMap.Select(selector) if err != nil { @@ -185,7 +190,8 @@ func (b *BundleFactory) Select(selector types.Selector) ([]Document, error) { // Construct Bundle document for each resource returned docSet := []Document{} for _, res := range resources { - doc, err := NewDocument(res) + var doc Document + doc, err = NewDocument(res) if err != nil { return docSet, err } @@ -196,36 +202,30 @@ func (b *BundleFactory) Select(selector types.Selector) ([]Document, error) { // GetByAnnotation is a convenience method to get documents for a particular annotation func (b *BundleFactory) GetByAnnotation(annotation string) ([]Document, error) { - // Construct kustomize annotation selector selector := types.Selector{AnnotationSelector: annotation} // pass it to the selector return b.Select(selector) - } // GetByLabel is a convenience method to get documents for a particular label func (b *BundleFactory) GetByLabel(label string) ([]Document, error) { - // Construct kustomize annotation selector selector := types.Selector{LabelSelector: label} // pass it to the selector return b.Select(selector) - } // GetByGvk is a convenience method to get documents for a particular Gvk tuple func (b *BundleFactory) GetByGvk(group, version, kind string) ([]Document, error) { - // Construct kustomize gvk object g := gvk.Gvk{Group: group, Version: version, Kind: kind} // pass it to the selector selector := types.Selector{Gvk: g} return b.Select(selector) - } // Write will write out the entire bundle resource map diff --git a/pkg/document/bundle_test.go b/pkg/document/bundle_test.go index 5bf0cbf87..bd4c2cce9 100644 --- a/pkg/document/bundle_test.go +++ b/pkg/document/bundle_test.go @@ -17,7 +17,6 @@ func TestNewBundle(t *testing.T) { bundle := testutil.NewTestBundle(t, "testdata") require.NotNil(bundle) - } func TestBundleDocumentFiltering(t *testing.T) { @@ -27,43 +26,34 @@ func TestBundleDocumentFiltering(t *testing.T) { bundle := testutil.NewTestBundle(t, "testdata") t.Run("GetKustomizeResourceMap", func(t *testing.T) { - r := bundle.GetKustomizeResourceMap() // ensure it is populated assert.NotZero(r.Size()) - }) t.Run("GetByGvk", func(t *testing.T) { - docs, err := bundle.GetByGvk("apps", "v1", "Deployment") require.NoError(err) assert.Len(docs, 3, "GetGvk returned the wrong number of resources") - }) t.Run("GetByAnnotation", func(t *testing.T) { - docs, err := bundle.GetByAnnotation("airshipit.org/clustertype=ephemeral") require.NoError(err, "Error trying to GetByAnnotation") assert.Len(docs, 4, "GetByAnnotation returned wrong number of resources") - }) t.Run("GetByLabel", func(t *testing.T) { - docs, err := bundle.GetByLabel("app=workflow-controller") require.NoError(err, "Error trying to GetByLabel") assert.Len(docs, 1, "GetByLabel returned wrong number of resources") - }) t.Run("SelectGvk", func(t *testing.T) { - // Select* tests test the Kustomize selector, which requires we build Kustomize // selector objects which is useful for advanced searches that // need to stack filters @@ -73,22 +63,18 @@ func TestBundleDocumentFiltering(t *testing.T) { require.NoError(err, "Error trying to select resources") assert.Len(docs, 3, "SelectGvk returned wrong number of resources") - }) t.Run("SelectAnnotations", func(t *testing.T) { - // Find documents with a particular annotation, namely airshipit.org/clustertype=ephemeral selector := types.Selector{AnnotationSelector: "airshipit.org/clustertype=ephemeral"} docs, err := bundle.Select(selector) require.NoError(err, "Error trying to select annotated resources") assert.Len(docs, 4, "SelectAnnotations returned wrong number of resources") - }) t.Run("SelectLabels", func(t *testing.T) { - // Find documents with a particular label, namely app=workflow-controller // note how this will only find resources labeled at the top most level (metadata.labels) // and not spec templates with this label (spec.template.metadata.labels) @@ -96,11 +82,9 @@ func TestBundleDocumentFiltering(t *testing.T) { docs, err := bundle.Select(selector) require.NoError(err, "Error trying to select labeled resources") assert.Len(docs, 1, "SelectLabels returned wrong number of resources") - }) t.Run("Write", func(t *testing.T) { - // Ensure we can write out a bundle // // alanmeadows(TODO) improve validation what we write looks correct @@ -113,7 +97,5 @@ func TestBundleDocumentFiltering(t *testing.T) { // so we for now we just search for an expected string // obviously, this should be improved assert.Contains(b.String(), "workflow-controller") - }) - } diff --git a/pkg/document/document.go b/pkg/document/document.go index 834a9c1c7..73354c713 100644 --- a/pkg/document/document.go +++ b/pkg/document/document.go @@ -30,9 +30,8 @@ type Document interface { // GetNamespace returns the namespace the resource thinks it's in. func (d *DocumentFactory) GetNamespace() string { - namespace, _ := d.GetString("metadata.namespace") - // if err, namespace is empty, so no need to check. - return namespace + r := d.GetKustomizeResource() + return r.GetNamespace() } // GetString returns the string value at path. @@ -125,9 +124,7 @@ func (d *DocumentFactory) SetKustomizeResource(r *resource.Resource) error { // documents - e.g. in the future all documents require an airship // annotation X func NewDocument(r *resource.Resource) (Document, error) { - var doc Document = &DocumentFactory{} err := doc.SetKustomizeResource(r) return doc, err - } diff --git a/pkg/document/document_test.go b/pkg/document/document_test.go index 7a764e1f2..83960cb25 100644 --- a/pkg/document/document_test.go +++ b/pkg/document/document_test.go @@ -27,7 +27,6 @@ func TestDocument(t *testing.T) { require.NotNil(bundle) t.Run("GetName", func(t *testing.T) { - docs, err := bundle.GetAllDocuments() require.NoError(err, "Unexpected error trying to GetAllDocuments") @@ -38,11 +37,9 @@ func TestDocument(t *testing.T) { } assert.Contains(nameList, "tiller-deploy", "Could not find expected name") - }) t.Run("AsYAML", func(t *testing.T) { - doc, err := bundle.GetByName("some-random-deployment-we-will-filter") require.NoError(err, "Unexpected error trying to GetByName for AsYAML Test") @@ -70,11 +67,9 @@ func TestDocument(t *testing.T) { // increase the chance of a match by removing any \n suffix on both actual // and expected assert.Equal(strings.TrimRight(s, "\n"), strings.TrimRight(string(fileData), "\n")) - }) t.Run("GetString", func(t *testing.T) { - doc, err := bundle.GetByName("some-random-deployment-we-will-filter") require.NoError(err, "Unexpected error trying to GetByName") @@ -82,20 +77,16 @@ func TestDocument(t *testing.T) { require.NoError(err, "Unexpected error trying to GetString from document") assert.Equal(appLabelMatch, "some-random-deployment-we-will-filter") - }) t.Run("GetNamespace", func(t *testing.T) { - doc, err := bundle.GetByName("some-random-deployment-we-will-filter") require.NoError(err, "Unexpected error trying to GetByName") assert.Equal("foobar", doc.GetNamespace()) - }) t.Run("GetStringSlice", func(t *testing.T) { - doc, err := bundle.GetByName("some-random-deployment-we-will-filter") require.NoError(err, "Unexpected error trying to GetByName") s := []string{"foobar"} @@ -104,7 +95,5 @@ func TestDocument(t *testing.T) { require.NoError(err, "Unexpected error trying to GetStringSlice") assert.Equal(s, gotSlice) - }) - } diff --git a/pkg/document/repo/repo.go b/pkg/document/repo/repo.go index 6129f8de3..13b5c77b8 100644 --- a/pkg/document/repo/repo.go +++ b/pkg/document/repo/repo.go @@ -50,7 +50,6 @@ type Repository struct { // NewRepository create repository object, with real filesystem on disk // basePath is used to calculate final path where to clone/open the repository func NewRepository(basePath string, builder OptionsBuilder) (*Repository, error) { - dirName := nameFromURL(builder.URL()) if dirName == "" { return nil, fmt.Errorf("URL: %s, original error: %w", builder.URL(), ErrCantParseUrl) diff --git a/pkg/document/repo/repo_test.go b/pkg/document/repo/repo_test.go index 42d6dbe49..1a67c54ab 100644 --- a/pkg/document/repo/repo_test.go +++ b/pkg/document/repo/repo_test.go @@ -46,8 +46,8 @@ func TestNewRepositoryNegative(t *testing.T) { assert.Error(t, err) assert.Nil(t, repo) } -func TestDownload(t *testing.T) { +func TestDownload(t *testing.T) { err := fixtures.Init() require.NoError(t, err) fx := fixtures.Basic().One() @@ -134,7 +134,7 @@ func TestUpdate(t *testing.T) { err = repo.Checkout(true) assert.NoError(t, err) // update repository - err = repo.Update(true) + require.NoError(t, repo.Update(true)) currentHash, err := repo.Driver.Head() assert.NoError(t, err) @@ -144,7 +144,6 @@ func TestUpdate(t *testing.T) { repo.Driver.Close() updateError := repo.Update(true) assert.Error(t, updateError) - } func TestOpen(t *testing.T) { @@ -161,11 +160,10 @@ func TestOpen(t *testing.T) { repo, err := NewRepository(".", builder) require.NoError(t, err) - driver := &GitDriver{ + repo.Driver = &GitDriver{ Filesystem: memfs.New(), Storer: memory.NewStorage(), } - repo.Driver = driver err = repo.Clone() assert.NotNil(t, repo.Driver) @@ -173,7 +171,17 @@ func TestOpen(t *testing.T) { // This should open the repo repoOpen, err := NewRepository(".", builder) - err = repoOpen.Open() + require.NoError(t, err) + + storer := memory.NewStorage() + err = storer.SetReference(plumbing.NewReferenceFromStrings("HEAD", "")) + require.NoError(t, err) + repoOpen.Driver = &GitDriver{ + Filesystem: memfs.New(), + Storer: storer, + } + + require.NoError(t, repoOpen.Open()) ref, err := repo.Driver.Head() assert.NoError(t, err) assert.NotNil(t, ref.String()) diff --git a/pkg/errors/common.go b/pkg/errors/common.go index 80353eaab..b1a725092 100644 --- a/pkg/errors/common.go +++ b/pkg/errors/common.go @@ -31,10 +31,11 @@ func (e ErrWrongConfig) Error() string { // ErrMissingConfig returned in case of missing configuration type ErrMissingConfig struct { + What string } func (e ErrMissingConfig) Error() string { - return "Missing configuration" + return "Missing configuration: " + e.What } // ErrConfigFailed returned in case of failure during configuration diff --git a/pkg/remote/redfish/redfish.go b/pkg/remote/redfish/redfish.go index 425f27e2c..e5deb0c4b 100644 --- a/pkg/remote/redfish/redfish.go +++ b/pkg/remote/redfish/redfish.go @@ -30,7 +30,6 @@ type RedfishRemoteDirect struct { // Top level function to handle Redfish remote direct func (cfg RedfishRemoteDirect) DoRemoteDirect() error { - alog.Debugf("Using Redfish Endpoint: '%s'", cfg.RemoteURL.String()) /* TODO: Add Authentication when redfish library supports it. */ @@ -44,10 +43,7 @@ func (cfg RedfishRemoteDirect) DoRemoteDirect() error { alog.Debugf("Ephemeral Node System ID: '%s'", systemId) /* get manager for system */ - managerId, err := GetResourceIDFromURL(system.Links.ManagedBy[0].OdataId) - if err != nil { - return err - } + managerId := GetResourceIDFromURL(system.Links.ManagedBy[0].OdataId) alog.Debugf("Ephemeral node managerId: '%s'", managerId) /* Get manager's Cd or DVD virtual media ID */ @@ -86,7 +82,6 @@ func NewRedfishRemoteDirectClient(ctx context.Context, ephNodeID string, isoPath string, ) (RedfishRemoteDirect, error) { - if remoteURL == "" { return RedfishRemoteDirect{}, NewRedfishConfigErrorf("redfish remote url empty") diff --git a/pkg/remote/redfish/redfish_test.go b/pkg/remote/redfish/redfish_test.go index 47da1e827..15c303c5d 100644 --- a/pkg/remote/redfish/redfish_test.go +++ b/pkg/remote/redfish/redfish_test.go @@ -22,7 +22,6 @@ const ( ) func TestRedfishRemoteDirectNormal(t *testing.T) { - m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) @@ -60,7 +59,6 @@ func TestRedfishRemoteDirectNormal(t *testing.T) { } func TestRedfishRemoteDirectInvalidSystemId(t *testing.T) { - m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) @@ -81,7 +79,6 @@ func TestRedfishRemoteDirectInvalidSystemId(t *testing.T) { } func TestRedfishRemoteDirectGetSystemNetworkError(t *testing.T) { - m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) @@ -103,7 +100,6 @@ func TestRedfishRemoteDirectGetSystemNetworkError(t *testing.T) { } func TestRedfishRemoteDirectInvalidIsoPath(t *testing.T) { - m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) @@ -131,7 +127,6 @@ func TestRedfishRemoteDirectInvalidIsoPath(t *testing.T) { } func TestRedfishRemoteDirectCdDvdNotAvailableInBootSources(t *testing.T) { - m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) @@ -157,7 +152,6 @@ func TestRedfishRemoteDirectCdDvdNotAvailableInBootSources(t *testing.T) { } func TestRedfishRemoteDirectSetSystemBootSourceFailed(t *testing.T) { - m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) @@ -185,7 +179,6 @@ func TestRedfishRemoteDirectSetSystemBootSourceFailed(t *testing.T) { } func TestRedfishRemoteDirectSystemRebootFailed(t *testing.T) { - m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) diff --git a/pkg/remote/redfish/utils.go b/pkg/remote/redfish/utils.go index e3c36393f..9824a1969 100644 --- a/pkg/remote/redfish/utils.go +++ b/pkg/remote/redfish/utils.go @@ -8,6 +8,8 @@ import ( redfishApi "github.com/Nordix/go-redfish/api" redfishClient "github.com/Nordix/go-redfish/client" + + "opendev.org/airship/airshipctl/pkg/log" ) const ( @@ -17,24 +19,23 @@ const ( // Redfish Id ref is a URI which contains resource Id // as the last part. This function extracts resource // ID from ID ref -func GetResourceIDFromURL(refURL string) (string, error) { - +func GetResourceIDFromURL(refURL string) string { u, err := url.Parse(refURL) - if err != nil { - return "", err + log.Fatal(err) } + url := strings.TrimSuffix(u.Path, "/") elems := strings.Split(url, "/") id := elems[len(elems)-1] - return id, nil + return id } // Checks whether an ID exists in Redfish IDref collection func IsIDInList(idRefList []redfishClient.IdRef, id string) bool { for _, r := range idRefList { - rId, _ := GetResourceIDFromURL(r.OdataId) + rId := GetResourceIDFromURL(r.OdataId) if rId == id { return true } @@ -48,7 +49,6 @@ func GetVirtualMediaId(ctx context.Context, api redfishApi.RedfishAPI, managerId string, ) (string, string, error) { - // TODO: Sushy emulator has a bug which sends 'virtualMedia.inserted' field as // string instead of a boolean which causes the redfish client to fail // while unmarshalling this field. @@ -62,7 +62,6 @@ func SetSystemBootSourceForMediaType(ctx context.Context, api redfishApi.RedfishAPI, systemId string, mediaType string) error { - /* Check available boot sources for system */ system, _, err := api.GetSystem(ctx, systemId) if err != nil { @@ -85,7 +84,6 @@ func SetSystemBootSourceForMediaType(ctx context.Context, // Reboots a system by force shutoff and turning on. func RebootSystem(ctx context.Context, api redfishApi.RedfishAPI, systemId string) error { - resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF _, httpResp, err := api.ResetSystem(ctx, systemId, resetReq) @@ -109,7 +107,6 @@ func SetVirtualMedia(ctx context.Context, managerId string, vMediaId string, isoPath string) error { - vMediaReq := redfishClient.InsertMediaRequestBody{} vMediaReq.Image = isoPath vMediaReq.Inserted = true diff --git a/pkg/remote/redfish/utils_test.go b/pkg/remote/redfish/utils_test.go index 715134fe6..db016e975 100644 --- a/pkg/remote/redfish/utils_test.go +++ b/pkg/remote/redfish/utils_test.go @@ -16,13 +16,11 @@ const ( ) func TestRedfishErrorNoError(t *testing.T) { - err := ScreenRedfishError(nil, nil) assert.NoError(t, err) } func TestRedfishErrorNonNilErrorWithoutHttpResp(t *testing.T) { - realErr := fmt.Errorf("Sample error") err := ScreenRedfishError(nil, realErr) assert.Error(t, err) @@ -31,7 +29,6 @@ func TestRedfishErrorNonNilErrorWithoutHttpResp(t *testing.T) { } func TestRedfishErrorNonNilErrorWithHttpRespError(t *testing.T) { - realErr := fmt.Errorf("Sample error") httpResp := &http.Response{StatusCode: 408} @@ -48,7 +45,6 @@ func TestRedfishErrorNonNilErrorWithHttpRespError(t *testing.T) { } func TestRedfishErrorNonNilErrorWithHttpRespOK(t *testing.T) { - realErr := fmt.Errorf("Sample error") httpResp := &http.Response{StatusCode: 204} @@ -65,28 +61,23 @@ func TestRedfishErrorNonNilErrorWithHttpRespOK(t *testing.T) { } func TestRedfishUtilGetResIDFromURL(t *testing.T) { - // simple case url := "api/user/123" - id, err := GetResourceIDFromURL(url) - assert.NoError(t, err) + id := GetResourceIDFromURL(url) assert.Equal(t, id, "123") // FQDN url = "http://www.abc.com/api/user/123" - id, err = GetResourceIDFromURL(url) - assert.NoError(t, err) + id = GetResourceIDFromURL(url) assert.Equal(t, id, "123") //Trailing slash url = "api/user/123/" - id, err = GetResourceIDFromURL(url) - assert.NoError(t, err) + id = GetResourceIDFromURL(url) assert.Equal(t, id, "123") } func TestRedfishUtilIsIdInList(t *testing.T) { - idList := []redfishClient.IdRef{ {OdataId: "/path/to/id/1"}, {OdataId: "/path/to/id/2"}, @@ -106,7 +97,6 @@ func TestRedfishUtilIsIdInList(t *testing.T) { } func TestRedfishUtilRebootSystemOK(t *testing.T) { - m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) @@ -127,7 +117,6 @@ func TestRedfishUtilRebootSystemOK(t *testing.T) { } func TestRedfishUtilRebootSystemForceOffError2(t *testing.T) { - m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) @@ -148,7 +137,6 @@ func TestRedfishUtilRebootSystemForceOffError2(t *testing.T) { } func TestRedfishUtilRebootSystemForceOffError(t *testing.T) { - m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) @@ -168,7 +156,6 @@ func TestRedfishUtilRebootSystemForceOffError(t *testing.T) { } func TestRedfishUtilRebootSystemTurningOnError(t *testing.T) { - m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) diff --git a/pkg/remote/remote_direct.go b/pkg/remote/remote_direct.go index cd8779e3d..cbf155ecb 100644 --- a/pkg/remote/remote_direct.go +++ b/pkg/remote/remote_direct.go @@ -41,7 +41,6 @@ type RemoteDirectClient interface { // Get remotedirect client based on config func getRemoteDirectClient(remoteConfig RemoteDirectConfig) (RemoteDirectClient, error) { - var client RemoteDirectClient var err error @@ -68,7 +67,6 @@ func getRemoteDirectClient(remoteConfig RemoteDirectConfig) (RemoteDirectClient, // Top level function to execute remote direct based on remote type func DoRemoteDirect(remoteConfig RemoteDirectConfig) error { - client, err := getRemoteDirectClient(remoteConfig) if err != nil { return err diff --git a/pkg/remote/remote_direct_test.go b/pkg/remote/remote_direct_test.go index d51f36f66..3e5ea137d 100644 --- a/pkg/remote/remote_direct_test.go +++ b/pkg/remote/remote_direct_test.go @@ -9,7 +9,6 @@ import ( ) func TestUnknownRemoteType(t *testing.T) { - rdCfg := RemoteDirectConfig{ RemoteType: "new-remote", RemoteURL: "http://localhost:8000", @@ -24,7 +23,6 @@ func TestUnknownRemoteType(t *testing.T) { } func TestRedfishRemoteDirectWithBogusConfig(t *testing.T) { - rdCfg := RemoteDirectConfig{ RemoteType: "redfish", RemoteURL: "http://nolocalhost:8888", @@ -39,7 +37,6 @@ func TestRedfishRemoteDirectWithBogusConfig(t *testing.T) { } func TestRedfishRemoteDirectWithEmptyURL(t *testing.T) { - rdCfg := RemoteDirectConfig{ RemoteType: "redfish", RemoteURL: "", @@ -54,7 +51,6 @@ func TestRedfishRemoteDirectWithEmptyURL(t *testing.T) { } func TestRedfishRemoteDirectWithEmptyNodeID(t *testing.T) { - rdCfg := RemoteDirectConfig{ RemoteType: "redfish", RemoteURL: "http://nolocalhost:8888", @@ -69,7 +65,6 @@ func TestRedfishRemoteDirectWithEmptyNodeID(t *testing.T) { } func TestRedfishRemoteDirectWithEmptyIsoPath(t *testing.T) { - rdCfg := RemoteDirectConfig{ RemoteType: "redfish", RemoteURL: "http://nolocalhost:8888", diff --git a/pkg/util/writefiles.go b/pkg/util/writefiles.go index 1ec393815..7bb2c5823 100644 --- a/pkg/util/writefiles.go +++ b/pkg/util/writefiles.go @@ -13,5 +13,4 @@ func WriteFiles(fls map[string][]byte, mode os.FileMode) error { } } return nil - } diff --git a/pkg/util/yaml/writer.go b/pkg/util/yaml/writer.go index 7a1b749c5..934bb1d4b 100644 --- a/pkg/util/yaml/writer.go +++ b/pkg/util/yaml/writer.go @@ -16,7 +16,6 @@ const ( // WriteOut dumps any yaml competible document to writer, adding yaml separator `---` // at the beginning of the document, and `...` at the end func WriteOut(dst io.Writer, src interface{}) error { - yamlOut, err := yaml.Marshal(src) if err != nil { return err diff --git a/pkg/util/yaml/writer_test.go b/pkg/util/yaml/writer_test.go index f8debd1fc..504673fc4 100644 --- a/pkg/util/yaml/writer_test.go +++ b/pkg/util/yaml/writer_test.go @@ -34,7 +34,6 @@ func (f *FakeErrorDashWriter) Write(b []byte) (int, error) { } func TestWriteOut(t *testing.T) { - // Create some object, that can be marshaled into yaml ob := &metav1.ObjectMeta{ Name: "RandomName", @@ -93,6 +92,5 @@ func TestWriteOutErrorsErrorWriter(t *testing.T) { for _, str := range strings { fakeWriter.Match = str assert.Error(t, utilyaml.WriteOut(fakeWriter, fakeYaml)) - } }