Merge pull request #1836 from benmoss/goerr113

🌱 Fix error handling for goerr113 linter

Author: k8s-ci-robot

.golangci.yml

@@ -12,7 +12,6 @@ linters:
     - gocognit
     - gomnd
     - godot
-    - goerr113
     - gofumpt
   # Run with --fast=false for more extensive checks
   fast: true

api/v1alpha3/awsmachinetemplate_webhook.go

@@ -17,9 +17,9 @@ limitations under the License.
 package v1alpha3
 
 import (
-	"errors"
 	"reflect"
 
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -60,7 +60,7 @@ func (r *AWSMachineTemplate) ValidateCreate() error {
 func (r *AWSMachineTemplate) ValidateUpdate(old runtime.Object) error {
 	oldAWSMachineTemplate := old.(*AWSMachineTemplate)
 	if !reflect.DeepEqual(r.Spec, oldAWSMachineTemplate.Spec) {
-		return errors.New("awsMachineTemplateSpec is immutable")
+		return apierrors.NewBadRequest("AWSMachineTemplate.Spec is immutable")
 	}
 
 	return nil

cmd/clusterawsadm/cloudformation/bootstrap/iam.go

@@ -47,13 +47,13 @@ func (t Template) GenerateManagedIAMPolicyDocuments(policyDocDir string) error {
 
 		pds, err := converters.IAMPolicyDocumentToJSON(*pd)
 		if err != nil {
-			return fmt.Errorf("failed to marshal policy document for ManagedIAMPolicy %q: %v", pn, err)
+			return fmt.Errorf("failed to marshal policy document for ManagedIAMPolicy %q: %w", pn, err)
 		}
 
 		fn := path.Join(policyDocDir, fmt.Sprintf("%s.json", pn))
 		err = ioutil.WriteFile(fn, []byte(pds), 0o600)
 		if err != nil {
-			return fmt.Errorf("failed to generate policy document for ManagedIAMPolicy %q: %v", pn, err)
+			return fmt.Errorf("failed to generate policy document for ManagedIAMPolicy %q: %w", pn, err)
 		}
 	}
 	return nil

cmd/clusterawsadm/cmd/alpha/bootstrap/bootstrap.go

@@ -206,7 +206,7 @@ func generateIAMPolicyDocJSON() *cobra.Command {
 			t := bootstrapTemplateFromCmdLine()
 			err := t.GenerateManagedIAMPolicyDocuments(policyDocDir)
 			if err != nil {
-				return fmt.Errorf("failed to generate PolicyDocument for all ManagedIAMPolicies: %v", err)
+				return fmt.Errorf("failed to generate PolicyDocument for all ManagedIAMPolicies: %w", err)
 			}
 
 			fmt.Printf("PolicyDocument for all ManagedIAMPolicies successfully generated in JSON at %q\n", policyDocDir)

cmd/clusterawsadm/cmd/bootstrap/credentials/credentials.go

@@ -76,6 +76,8 @@ const (
 	`
 )
 
+var errInvalidOutputFlag = errors.New("invalid output flag. Expected rawSharedConfig or base64SharedConfig")
+
 // RootCmd is the root of the `alpha bootstrap command`
 func RootCmd() *cobra.Command {
 	newCmd := &cobra.Command{
@@ -103,7 +105,7 @@ func getOutputFlag(cmd *cobra.Command) (string, error) {
 	case rawSharedConfig, base64SharedConfig:
 		return val, nil
 	default:
-		return "", errors.New("invalid output flag. Expected rawSharedConfig or base64SharedConfig")
+		return "", errInvalidOutputFlag
 	}
 }
 

cmd/clusterawsadm/cmd/bootstrap/iam/iam_doc.go

@@ -25,6 +25,8 @@ import (
 	"sigs.k8s.io/cluster-api/cmd/clusterctl/cmd"
 )
 
+var errInvalidDocumentName = fmt.Errorf("invalid document name, use one of: %+v", bootstrap.ManagedIAMPolicyNames)
+
 func printPolicyCmd() *cobra.Command {
 	newCmd := &cobra.Command{
 		Use:   "print-policy",
@@ -76,7 +78,7 @@ func printPolicyCmd() *cobra.Command {
 func getDocumentName(cmd *cobra.Command) (bootstrap.PolicyName, error) {
 	val := bootstrap.PolicyName(cmd.Flags().Lookup("document").Value.String())
 	if !val.IsValid() {
-		return "", fmt.Errorf("invalid document name, use one of: %+v", bootstrap.ManagedIAMPolicyNames)
+		return "", errInvalidDocumentName
 	}
 
 	return val, nil

cmd/clusterawsadm/configreader/configreader.go

@@ -28,23 +28,24 @@ import (
 	bootstrapschemev1 "sigs.k8s.io/cluster-api-provider-aws/cmd/clusterawsadm/api/bootstrap/v1alpha1/scheme"
 )
 
+type errEmptyBootstrapConfig string
+
+func (e errEmptyBootstrapConfig) Error() string {
+	return fmt.Sprintf("bootstrap config file %q was empty", string(e))
+}
+
 // LoadConfigFile loads a YAML file representing a bootstrapv1.AWSIAMConfiguration.
 func LoadConfigFile(name string) (*bootstrapv1.AWSIAMConfiguration, error) {
-	const errFmt = "failed to load bootstrap config file %s, error %v"
 	// compute absolute path based on current working dir
 	iamConfigFile, err := filepath.Abs(name)
 	if err != nil {
-		return nil, fmt.Errorf(errFmt, name, err)
+		return nil, fmt.Errorf("failed to convert IAM config path into absolute path %s, error: %w", name, err)
 	}
 	loader, err := newFsLoader(iamConfigFile)
 	if err != nil {
-		return nil, fmt.Errorf(errFmt, name, err)
-	}
-	kc, err := loader.Load()
-	if err != nil {
-		return nil, fmt.Errorf(errFmt, name, err)
+		return nil, fmt.Errorf("failed to initialize filesystem loader: %w", err)
 	}
-	return kc, err
+	return loader.Load()
 }
 
 // Loader loads configuration from a storage layer.
@@ -83,12 +84,12 @@ func newFsLoader(bootstrapFile string) (loader, error) {
 func (loader *fsLoader) Load() (*bootstrapv1.AWSIAMConfiguration, error) {
 	data, err := loader.ReadFile(loader.bootstrapFile)
 	if err != nil {
-		return nil, fmt.Errorf("failed to read bootstrap config file %q, error: %v", loader.bootstrapFile, err)
+		return nil, fmt.Errorf("failed to read bootstrap config file %q, error: %w", loader.bootstrapFile, err)
 	}
 
 	// no configuration is an error, some parameters are required
 	if len(data) == 0 {
-		return nil, fmt.Errorf("bootstrap config file %q was empty", loader.bootstrapFile)
+		return nil, errEmptyBootstrapConfig(loader.bootstrapFile)
 	}
 
 	kc, err := DecodeBootstrapConfiguration(loader.bootstrapCodecs, data)

cmd/clusterawsadm/credentials/credentials.go

@@ -41,6 +41,14 @@ aws_session_token = {{ .SessionToken }}
 
 const AWSDefaultRegion = "us-east-1"
 
+var ErrNoAWSRegionConfigured = errors.New("no AWS region configured. Use --region or set AWS_REGION or DEFAULT_AWS_REGION environment variable")
+
+type ErrEnvironmentVariableNotFound string
+
+func (e ErrEnvironmentVariableNotFound) Error() string {
+	return fmt.Sprintf("environment variable %q not found", string(e))
+}
+
 type AWSCredentials struct {
 	AccessKeyID     string
 	SecretAccessKey string
@@ -78,13 +86,13 @@ func ResolveRegion(explicitRegion string) (string, error) {
 	if err == nil {
 		return region, nil
 	}
-	return "", errors.New("no AWS region configured. Use --region or set AWS_REGION or DEFAULT_AWS_REGION environment variable")
+	return "", ErrNoAWSRegionConfigured
 }
 
 func getEnv(key string) (string, error) {
 	val, ok := os.LookupEnv(key)
 	if !ok {
-		return "", fmt.Errorf("environment variable %q not found", key)
+		return "", ErrEnvironmentVariableNotFound(key)
 	}
 	return val, nil
 }

controllers/awsmachine_controller.go

@@ -384,7 +384,7 @@ func (r *AWSMachineReconciler) reconcileDelete(machineScope *scope.MachineScope,
 func (r *AWSMachineReconciler) findInstance(scope *scope.MachineScope, ec2svc services.EC2MachineInterface) (*infrav1.Instance, error) {
 	// Parse the ProviderID.
 	pid, err := noderefutil.NewProviderID(scope.GetProviderID())
-	if err != nil && err != noderefutil.ErrEmptyProviderID {
+	if err != nil && !errors.Is(err, noderefutil.ErrEmptyProviderID) {
 		return nil, errors.Wrapf(err, "failed to parse Spec.ProviderID")
 	}
 

pkg/cloud/awserrors/errors.go

@@ -63,28 +63,28 @@ func Message(err error) string {
 
 // EC2Error is an error exposed to users of this library.
 type EC2Error struct {
-	err error
+	msg string
 
 	Code int
 }
 
 // Error implements the Error interface.
 func (e *EC2Error) Error() string {
-	return e.err.Error()
+	return e.msg
 }
 
-// NewNotFound returns a new error which indicates that the resource of the kind and the name was not found.
-func NewNotFound(err error) error {
+// NewNotFound returns an error which indicates that the resource of the kind and the name was not found.
+func NewNotFound(msg string) error {
 	return &EC2Error{
-		err:  err,
+		msg:  msg,
 		Code: http.StatusNotFound,
 	}
 }
 
-// NewConflict returns a new error which indicates that the request cannot be processed due to a conflict.
-func NewConflict(err error) error {
+// NewConflict returns an error which indicates that the request cannot be processed due to a conflict.
+func NewConflict(msg string) error {
 	return &EC2Error{
-		err:  err,
+		msg:  msg,
 		Code: http.StatusConflict,
 	}
 }
@@ -96,10 +96,10 @@ func IsResourceExists(err error) bool {
 	return false
 }
 
-// NewFailedDependency returns a new error which indicates that a dependency failure status
-func NewFailedDependency(err error) error {
+// NewFailedDependency returns an error which indicates that a dependency failure status
+func NewFailedDependency(msg string) error {
 	return &EC2Error{
-		err:  err,
+		msg:  msg,
 		Code: http.StatusFailedDependency,
 	}
 }