Merge pull request #660 from jetstack/linter_fix_part2

Linter fixes (part 2)

Author: inteon

.golangci.yaml

@@ -6,21 +6,12 @@ linters:
     presets: [comments, common-false-positives, legacy, std-error-handling]
     rules:
       - linters:
-          - bodyclose
-          - errcheck
           - errchkjson
           - forbidigo
-          - gocritic
           - gosec
-          - govet
           - musttag
           - nilerr
-          - noctx
-          - predeclared
-          - staticcheck
-          - unconvert
           - unparam
-          - usestdlibvars
         text: .*
     paths: [third_party$, builtin$, examples$]
     warn-unused: true

api/common.go

@@ -26,5 +26,5 @@ func (t Time) MarshalJSON() ([]byte, error) {
 	if err != nil {
 		return nil, err
 	}
-	return []byte(jsonStr), nil
+	return jsonStr, nil
 }

cmd/root.go

@@ -67,7 +67,7 @@ func setFlagsFromEnv(prefix string, fs *pflag.FlagSet) {
 		}
 		// remove trailing _ to reduce common errors with the prefix, i.e. people setting it to MY_PROG_
 		cleanPrefix := strings.TrimSuffix(prefix, "_")
-		name := fmt.Sprintf("%s_%s", cleanPrefix, strings.Replace(strings.ToUpper(f.Name), "-", "_", -1))
+		name := fmt.Sprintf("%s_%s", cleanPrefix, strings.ReplaceAll(strings.ToUpper(f.Name), "-", "_"))
 		if e, ok := os.LookupEnv(name); ok {
 			_ = f.Value.Set(e)
 		}

pkg/agent/config.go

@@ -232,7 +232,9 @@ func InitAgentCmdFlags(c *cobra.Command, cfg *AgentCmdFlags) {
 		false,
 		fmt.Sprintf("Turns on the %s mode. The flag --credentials-file must also be passed.", JetstackSecureOAuth),
 	)
-	c.PersistentFlags().MarkHidden("venafi-cloud")
+	if err := c.PersistentFlags().MarkHidden("venafi-cloud"); err != nil {
+		panic(err)
+	}
 	c.PersistentFlags().StringVarP(
 		&cfg.ClientID,
 		"client-id",
@@ -247,7 +249,7 @@ func InitAgentCmdFlags(c *cobra.Command, cfg *AgentCmdFlags) {
 		"private-key-path",
 		"",
 		"",
-		fmt.Sprintf("To be used in conjunction with --client-id. The path to the private key file for the service account."),
+		"To be used in conjunction with --client-id. The path to the private key file for the service account.",
 	)
 	c.PersistentFlags().BoolVarP(
 		&cfg.OneShot,
@@ -334,7 +336,9 @@ func InitAgentCmdFlags(c *cobra.Command, cfg *AgentCmdFlags) {
 		false,
 		"Deprecated. No longer has an effect.",
 	)
-	c.PersistentFlags().MarkDeprecated("disable-compression", "no longer has an effect")
+	if err := c.PersistentFlags().MarkDeprecated("disable-compression", "no longer has an effect"); err != nil {
+		panic(err)
+	}
 
 	// This is a hidden feature flag we use to build the "Machine Hub" feature
 	// gradually without impacting customers. Once the feature is GA, we will
@@ -345,7 +349,9 @@ func InitAgentCmdFlags(c *cobra.Command, cfg *AgentCmdFlags) {
 		false,
 		"Enables the MachineHub mode. The agent will push data to CyberArk MachineHub.",
 	)
-	c.PersistentFlags().MarkHidden("machine-hub")
+	if err := c.PersistentFlags().MarkHidden("machine-hub"); err != nil {
+		panic(err)
+	}
 
 }
 
@@ -531,8 +537,8 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags)
 	// Validation of `venafi-cloud.upload_path`.
 	{
 		var uploadPath string
-		switch {
-		case res.TLSPKMode == VenafiCloudKeypair:
+		switch res.TLSPKMode { // nolint:exhaustive
+		case VenafiCloudKeypair:
 			if cfg.VenafiCloud == nil || cfg.VenafiCloud.UploadPath == "" {
 				errs = multierror.Append(errs, fmt.Errorf("the venafi-cloud.upload_path field is required when using the %s mode", res.TLSPKMode))
 				break // Skip to the end of the switch statement.
@@ -544,7 +550,7 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags)
 			}
 
 			uploadPath = cfg.VenafiCloud.UploadPath
-		case res.TLSPKMode == VenafiCloudVenafiConnection:
+		case VenafiCloudVenafiConnection:
 			// The venafi-cloud.upload_path was initially meant to let users
 			// configure HTTP proxies, but it has never been used since HTTP
 			// proxies don't rewrite paths. Thus, we've disabled the ability to
@@ -577,18 +583,18 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags)
 	if res.TLSPKMode != Off {
 		var clusterID string
 		var organizationID string // Only used by the old jetstack-secure mode.
-		switch {
-		case res.TLSPKMode == VenafiCloudKeypair:
+		switch res.TLSPKMode {    // nolint:exhaustive
+		case VenafiCloudKeypair:
 			if cfg.ClusterID == "" {
 				errs = multierror.Append(errs, fmt.Errorf("cluster_id is required in %s mode", res.TLSPKMode))
 			}
 			clusterID = cfg.ClusterID
-		case res.TLSPKMode == VenafiCloudVenafiConnection:
+		case VenafiCloudVenafiConnection:
 			if cfg.ClusterID == "" {
 				errs = multierror.Append(errs, fmt.Errorf("cluster_id is required in %s mode", res.TLSPKMode))
 			}
 			clusterID = cfg.ClusterID
-		case res.TLSPKMode == JetstackSecureOAuth || res.TLSPKMode == JetstackSecureAPIToken:
+		case JetstackSecureOAuth, JetstackSecureAPIToken:
 			if cfg.OrganizationID == "" {
 				errs = multierror.Append(errs, fmt.Errorf("organization_id is required"))
 			}
@@ -637,7 +643,7 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags)
 	}
 
 	// Validation of --install-namespace.
-	var installNS string = flags.InstallNS
+	installNS := flags.InstallNS
 	if flags.InstallNS == "" {
 		var err error
 		installNS, err = getInClusterNamespace()
@@ -650,7 +656,7 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags)
 	// Validation of --venafi-connection and --venafi-connection-namespace.
 	if res.TLSPKMode == VenafiCloudVenafiConnection {
 		res.VenConnName = flags.VenConnName
-		var venConnNS string = flags.VenConnNS
+		venConnNS := flags.VenConnNS
 		if flags.VenConnNS == "" {
 			venConnNS = installNS
 		}
@@ -714,8 +720,8 @@ func validateCredsAndCreateClient(log logr.Logger, flagCredentialsPath, flagClie
 
 	var preflightClient client.Client
 	metadata := &api.AgentMetadata{Version: version.PreflightVersion, ClusterID: cfg.ClusterID}
-	switch {
-	case cfg.TLSPKMode == JetstackSecureOAuth:
+	switch cfg.TLSPKMode {
+	case JetstackSecureOAuth:
 		// Note that there are no command line flags to configure the
 		// JetstackSecureOAuth mode.
 		credsBytes, err := readCredentialsFile(flagCredentialsPath)
@@ -734,7 +740,7 @@ func validateCredsAndCreateClient(log logr.Logger, flagCredentialsPath, flagClie
 		if err != nil {
 			errs = multierror.Append(errs, err)
 		}
-	case cfg.TLSPKMode == VenafiCloudKeypair:
+	case VenafiCloudKeypair:
 		var creds client.Credentials
 
 		if flagClientID != "" && flagCredentialsPath != "" {
@@ -750,14 +756,15 @@ func validateCredsAndCreateClient(log logr.Logger, flagCredentialsPath, flagClie
 			break
 		}
 
-		if flagClientID != "" && flagPrivateKeyPath != "" {
+		switch {
+		case flagClientID != "" && flagPrivateKeyPath != "":
 			// If --client-id and --private-key-path are passed, then
 			// --credentials-file is ignored.
 			creds = &client.VenafiSvcAccountCredentials{
 				ClientID:       flagClientID,
 				PrivateKeyFile: flagPrivateKeyPath,
 			}
-		} else if flagCredentialsPath != "" {
+		case flagCredentialsPath != "":
 			credsBytes, err := readCredentialsFile(flagCredentialsPath)
 			if err != nil {
 				errs = multierror.Append(errs, multierror.Prefix(err, "credentials file:"))
@@ -768,7 +775,7 @@ func validateCredsAndCreateClient(log logr.Logger, flagCredentialsPath, flagClie
 				errs = multierror.Append(errs, multierror.Prefix(err, "credentials file:"))
 				break // Don't continue with the client since creds is invalid.
 			}
-		} else {
+		default:
 			return nil, fmt.Errorf("programmer mistake: --client-id and --private-key-path or --credentials-file must have been provided")
 		}
 
@@ -777,7 +784,7 @@ func validateCredsAndCreateClient(log logr.Logger, flagCredentialsPath, flagClie
 		if err != nil {
 			errs = multierror.Append(errs, err)
 		}
-	case cfg.TLSPKMode == VenafiCloudVenafiConnection:
+	case VenafiCloudVenafiConnection:
 		var restCfg *rest.Config
 		restCfg, err := kubeconfig.LoadRESTConfig("")
 		if err != nil {
@@ -789,13 +796,13 @@ func validateCredsAndCreateClient(log logr.Logger, flagCredentialsPath, flagClie
 		if err != nil {
 			errs = multierror.Append(errs, err)
 		}
-	case cfg.TLSPKMode == JetstackSecureAPIToken:
+	case JetstackSecureAPIToken:
 		var err error
 		preflightClient, err = client.NewAPITokenClient(metadata, flagAPIToken, cfg.Server)
 		if err != nil {
 			errs = multierror.Append(errs, err)
 		}
-	case cfg.TLSPKMode == Off:
+	case Off:
 		// No client needed in this mode.
 	default:
 		panic(fmt.Errorf("programmer mistake: auth mode not implemented: %s", cfg.TLSPKMode))

pkg/client/client_oauth.go

@@ -186,7 +186,7 @@ func (c *OAuthClient) renewAccessToken(ctx context.Context) error {
 	payload.Set("audience", audience)
 	payload.Set("username", c.credentials.UserID)
 	payload.Set("password", c.credentials.UserSecret)
-	req, err := http.NewRequestWithContext(ctx, "POST", tokenURL, strings.NewReader(payload.Encode()))
+	req, err := http.NewRequestWithContext(ctx, http.MethodPost, tokenURL, strings.NewReader(payload.Encode()))
 	if err != nil {
 		return errors.WithStack(err)
 	}

pkg/client/client_venafi_cloud.go

@@ -71,7 +71,7 @@ type (
 	}
 
 	accessTokenInformation struct {
-		AccessToken string `json:"access_token"` //base 64 encoded token
+		AccessToken string `json:"access_token"` // base 64 encoded token
 		Type        string `json:"token_type"`   // always be “bearer” for now
 		ExpiresIn   int64  `json:"expires_in"`   // number of seconds after which the access token will expire
 	}

pkg/datagatherer/k8s/cache.go

@@ -56,14 +56,14 @@ func onAdd(log logr.Logger, obj interface{}, dgCache *cache.Cache) {
 // onUpdate handles the informer update events, replacing the old object with the new one
 // if it's present in the data gatherer's cache, (if the object isn't present, it gets added).
 // The cache key is the uid of the object
-func onUpdate(log logr.Logger, old, new interface{}, dgCache *cache.Cache) {
-	item, ok := old.(cacheResource)
+func onUpdate(log logr.Logger, oldObj, newObj interface{}, dgCache *cache.Cache) {
+	item, ok := oldObj.(cacheResource)
 	if ok {
-		cacheObject := updateCacheGatheredResource(string(item.GetUID()), new, dgCache)
+		cacheObject := updateCacheGatheredResource(string(item.GetUID()), newObj, dgCache)
 		dgCache.Set(string(item.GetUID()), cacheObject, cache.DefaultExpiration)
 		return
 	}
-	logCacheUpdateFailure(log, old, "update")
+	logCacheUpdateFailure(log, oldObj, "update")
 }
 
 // onDelete handles the informer deletion events, updating the object's properties with the deletion

pkg/datagatherer/k8s/cache_test.go

@@ -52,7 +52,9 @@ func TestOnAddCache(t *testing.T) {
 				getObject("v1", "Service", "testservice", "testns", false),
 				getObject("foobar/v1", "NotFoo", "notfoo", "testns", false),
 			},
-			eventFunc: func(log logr.Logger, old, new interface{}, dgCache *cache.Cache) { onDelete(log, old, dgCache) },
+			eventFunc: func(log logr.Logger, oldObj, newObj interface{}, dgCache *cache.Cache) {
+				onDelete(log, oldObj, dgCache)
+			},
 			expected: []*api.GatheredResource{
 				makeGatheredResource(
 					getObject("foobar/v1", "Foo", "testfoo", "testns", false),

pkg/datagatherer/k8s/dynamic.go

@@ -76,27 +76,27 @@ func (c *ConfigDynamic) UnmarshalYAML(unmarshal func(interface{}) error) error {
 
 // validate validates the configuration.
 func (c *ConfigDynamic) validate() error {
-	var errors []string
+	var errs []string
 	if len(c.ExcludeNamespaces) > 0 && len(c.IncludeNamespaces) > 0 {
-		errors = append(errors, "cannot set excluded and included namespaces")
+		errs = append(errs, "cannot set excluded and included namespaces")
 	}
 
 	if c.GroupVersionResource.Resource == "" {
-		errors = append(errors, "invalid configuration: GroupVersionResource.Resource cannot be empty")
+		errs = append(errs, "invalid configuration: GroupVersionResource.Resource cannot be empty")
 	}
 
 	for i, selectorString := range c.FieldSelectors {
 		if selectorString == "" {
-			errors = append(errors, fmt.Sprintf("invalid field selector %d: must not be empty", i))
+			errs = append(errs, fmt.Sprintf("invalid field selector %d: must not be empty", i))
 		}
 		_, err := fields.ParseSelector(selectorString)
 		if err != nil {
-			errors = append(errors, fmt.Sprintf("invalid field selector %d: %s", i, err))
+			errs = append(errs, fmt.Sprintf("invalid field selector %d: %s", i, err))
 		}
 	}
 
-	if len(errors) > 0 {
-		return fmt.Errorf(strings.Join(errors, ", "))
+	if len(errs) > 0 {
+		return errors.New(strings.Join(errs, ", "))
 	}
 
 	return nil
@@ -221,8 +221,8 @@ func (c *ConfigDynamic) newDataGathererWithClient(ctx context.Context, cl dynami
 		AddFunc: func(obj interface{}) {
 			onAdd(log, obj, dgCache)
 		},
-		UpdateFunc: func(old, new interface{}) {
-			onUpdate(log, old, new, dgCache)
+		UpdateFunc: func(oldObj, newObj interface{}) {
+			onUpdate(log, oldObj, newObj, dgCache)
 		},
 		DeleteFunc: func(obj interface{}) {
 			onDelete(log, obj, dgCache)
@@ -329,7 +329,7 @@ func (g *DataGathererDynamic) Fetch() (interface{}, int, error) {
 		fetchNamespaces = []string{metav1.NamespaceAll}
 	}
 
-	//delete expired items from the cache
+	// delete expired items from the cache
 	g.cache.DeleteExpired()
 	for _, item := range g.cache.Items() {
 		// filter cache items by namespace
@@ -371,11 +371,15 @@ func redactList(list []*api.GatheredResource, excludeAnnotKeys, excludeLabelKeys
 			for _, gvk := range gvks {
 				// secret object
 				if gvk.Kind == "Secret" && (gvk.Group == "core" || gvk.Group == "") {
-					Select(SecretSelectedFields, resource)
+					if err := Select(SecretSelectedFields, resource); err != nil {
+						return err
+					}
 
 					// route object
 				} else if gvk.Kind == "Route" && gvk.Group == "route.openshift.io" {
-					Select(RouteSelectedFields, resource)
+					if err := Select(RouteSelectedFields, resource); err != nil {
+						return err
+					}
 				}
 			}