Merge pull request #192 from hakkiai/fix/multi-error-returns

k8s-ci-robot · web-flow · commit 588b99be434a · 2025-12-01T09:38:27.000-08:00
Refactor: use errors.Join for multi-error returns
diff --git a/controller/internal/reconciler/bucketaccess.go b/controller/internal/reconciler/bucketaccess.go
@@ -306,7 +306,7 @@ func getAllBucketClaims(
 	}
 
 	if len(errs) > 0 {
-		return nil, fmt.Errorf("could not get one or more BucketClaims: %v", errs)
+		return nil, fmt.Errorf("could not get one or more BucketClaims: %w", errors.Join(errs...))
 	}
 
 	if len(claims) != len(claimAccesses) {
@@ -345,7 +345,7 @@ func markAllBucketClaimsAsAccessed(
 		}
 	}
 	if len(errs) > 0 {
-		return fmt.Errorf("failed to mark one or more BucketClaims as having a BucketAccess reference: %v", errs)
+		return fmt.Errorf("failed to mark one or more BucketClaims as having a BucketAccess reference: %w", errors.Join(errs...))
 	}
 
 	return nil
@@ -356,28 +356,28 @@ func validateAccessAgainstClass(
 	class *cosiapi.BucketAccessClassSpec,
 	access *cosiapi.BucketAccessSpec,
 ) error {
-	errs := []string{}
+	errs := []error{}
 
 	needServiceAccount := class.AuthenticationType == cosiapi.BucketAccessAuthenticationTypeServiceAccount
 	if needServiceAccount && access.ServiceAccountName == "" {
-		errs = append(errs, "serviceAccountName must be specified")
+		errs = append(errs, fmt.Errorf("serviceAccountName must be specified"))
 	}
 
 	if class.FeatureOptions.DisallowMultiBucketAccess && len(access.BucketClaims) > 1 {
-		errs = append(errs, "multi-bucket access is disallowed")
+		errs = append(errs, fmt.Errorf("multi-bucket access is disallowed"))
 	}
 
 	for _, claimRef := range access.BucketClaims {
 		if slices.Contains(class.FeatureOptions.DisallowedBucketAccessModes, claimRef.AccessMode) {
 			errs = append(errs,
-				fmt.Sprintf("accessMode %q requested for BucketClaim %q is disallowed",
+				fmt.Errorf("accessMode %q requested for BucketClaim %q is disallowed",
 					claimRef.AccessMode, claimRef.BucketClaimName),
 			)
 		}
 	}
 
 	if len(errs) > 0 {
-		return fmt.Errorf("one or more features are disallowed by the BucketAccessClass: %v", errs)
+		return fmt.Errorf("one or more features are disallowed by the BucketAccessClass: %w", errors.Join(errs...))
 	}
 	return nil
 }
diff --git a/internal/protocol/azure.go b/internal/protocol/azure.go
@@ -17,6 +17,7 @@ limitations under the License.
 package protocol
 
 import (
+	"errors"
 	"fmt"
 
 	cosiapi "sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2"
@@ -60,15 +61,15 @@ func (AzureBucketInfoTranslator) ApiToRpc(vars map[cosiapi.BucketInfoVar]string)
 func (AzureBucketInfoTranslator) Validate(
 	vars map[cosiapi.BucketInfoVar]string, _ cosiapi.BucketAccessAuthenticationType,
 ) error {
-	errs := []string{}
+	errs := []error{}
 
 	storageAccount := vars[cosiapi.BucketInfoVar_Azure_StorageAccount]
 	if storageAccount == "" {
-		errs = append(errs, "azure storage account cannot be unset")
+		errs = append(errs, fmt.Errorf("azure storage account cannot be unset"))
 	}
 
 	if len(errs) > 0 {
-		return fmt.Errorf("azure bucket info is invalid: %v", errs)
+		return fmt.Errorf("azure bucket info is invalid: %w", errors.Join(errs...))
 	}
 	return nil
 }
@@ -107,15 +108,15 @@ func (AzureCredentialTranslator) Validate(
 		return nil
 	}
 
-	errs := []string{}
+	errs := []error{}
 
 	accessToken := vars[cosiapi.CredentialVar_Azure_AccessToken]
 	if accessToken == "" {
-		errs = append(errs, "azure access token cannot be unset")
+		errs = append(errs, fmt.Errorf("azure access token cannot be unset"))
 	}
 
 	if len(errs) > 0 {
-		return fmt.Errorf("azure credential info is invalid: %v", errs)
+		return fmt.Errorf("azure credential info is invalid: %w", errors.Join(errs...))
 	}
 	return nil
 }
diff --git a/internal/protocol/gcs.go b/internal/protocol/gcs.go
@@ -17,6 +17,7 @@ limitations under the License.
 package protocol
 
 import (
+	"errors"
 	"fmt"
 
 	cosiapi "sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2"
@@ -62,20 +63,20 @@ func (GcsBucketInfoTranslator) ApiToRpc(vars map[cosiapi.BucketInfoVar]string) *
 func (GcsBucketInfoTranslator) Validate(
 	vars map[cosiapi.BucketInfoVar]string, _ cosiapi.BucketAccessAuthenticationType,
 ) error {
-	errs := []string{}
+	errs := []error{}
 
 	bucketName := vars[cosiapi.BucketInfoVar_GCS_BucketName]
 	if bucketName == "" {
-		errs = append(errs, "GCS bucket name cannot be unset")
+		errs = append(errs, fmt.Errorf("GCS bucket name cannot be unset"))
 	}
 
 	projectId := vars[cosiapi.BucketInfoVar_GCS_ProjectId]
 	if projectId == "" {
-		errs = append(errs, "GCS project ID cannot be unset")
+		errs = append(errs, fmt.Errorf("GCS project ID cannot be unset"))
 	}
 
 	if len(errs) > 0 {
-		return fmt.Errorf("GCS bucket info is invalid: %v", errs)
+		return fmt.Errorf("GCS bucket info is invalid: %w", errors.Join(errs...))
 	}
 	return nil
 }
@@ -113,34 +114,34 @@ func (GcsCredentialTranslator) ApiToRpc(vars map[cosiapi.CredentialVar]string) *
 func (GcsCredentialTranslator) Validate(
 	vars map[cosiapi.CredentialVar]string, authType cosiapi.BucketAccessAuthenticationType,
 ) error {
-	errs := []string{}
+	errs := []error{}
 
 	switch authType {
 	case cosiapi.BucketAccessAuthenticationTypeKey:
 		accessId := vars[cosiapi.CredentialVar_GCS_AccessId]
 		if accessId == "" {
-			errs = append(errs, "GCS access ID cannot be unset")
+			errs = append(errs, fmt.Errorf("GCS access ID cannot be unset"))
 		}
 
 		accessSecret := vars[cosiapi.CredentialVar_GCS_AccessSecret]
 		if accessSecret == "" {
-			errs = append(errs, "GCS access secret cannot be unset")
+			errs = append(errs, fmt.Errorf("GCS access secret cannot be unset"))
 		}
 
 	case cosiapi.BucketAccessAuthenticationTypeServiceAccount:
 		privateKeyName := vars[cosiapi.CredentialVar_GCS_PrivateKeyName]
 		if privateKeyName == "" {
-			errs = append(errs, "GCS private key name cannot be unset")
+			errs = append(errs, fmt.Errorf("GCS private key name cannot be unset"))
 		}
 
 		serviceAccount := vars[cosiapi.CredentialVar_GCS_ServiceAccount]
 		if serviceAccount == "" {
-			errs = append(errs, "GCS service account cannot be unset")
+			errs = append(errs, fmt.Errorf("GCS service account cannot be unset"))
 		}
 	}
 
 	if len(errs) > 0 {
-		return fmt.Errorf("GCS credential info is invalid: %v", errs)
+		return fmt.Errorf("GCS credential info is invalid: %w", errors.Join(errs...))
 	}
 	return nil
 }
diff --git a/internal/protocol/s3.go b/internal/protocol/s3.go
@@ -17,6 +17,7 @@ limitations under the License.
 package protocol
 
 import (
+	"errors"
 	"fmt"
 	"slices"
 
@@ -97,30 +98,30 @@ func (S3BucketInfoTranslator) ApiToRpc(vars map[cosiapi.BucketInfoVar]string) *c
 func (S3BucketInfoTranslator) Validate(
 	vars map[cosiapi.BucketInfoVar]string, _ cosiapi.BucketAccessAuthenticationType,
 ) error {
-	errs := []string{}
+	errs := []error{}
 
 	id := vars[cosiapi.BucketInfoVar_S3_BucketId]
 	if id == "" {
-		errs = append(errs, "S3 bucket ID cannot be unset")
+		errs = append(errs, fmt.Errorf("S3 bucket ID cannot be unset"))
 	}
 
 	ep := vars[cosiapi.BucketInfoVar_S3_Endpoint]
 	if ep == "" {
-		errs = append(errs, "S3 endpoint cannot be unset")
+		errs = append(errs, fmt.Errorf("S3 endpoint cannot be unset"))
 	}
 
 	rg := vars[cosiapi.BucketInfoVar_S3_Region]
 	if rg == "" {
-		errs = append(errs, "S3 region cannot be unset")
+		errs = append(errs, fmt.Errorf("S3 region cannot be unset"))
 	}
 
 	as := vars[cosiapi.BucketInfoVar_S3_AddressingStyle]
 	if !slices.Contains(validS3AddressingStyles, as) {
-		errs = append(errs, fmt.Sprintf("S3 addressing style %q must be one of %v", as, validS3AddressingStyles))
+		errs = append(errs, fmt.Errorf("S3 addressing style %q must be one of %v", as, validS3AddressingStyles))
 	}
 
 	if len(errs) > 0 {
-		return fmt.Errorf("S3 bucket info is invalid: %v", errs)
+		return fmt.Errorf("S3 bucket info is invalid: %w", errors.Join(errs...))
 	}
 	return nil
 }
@@ -159,20 +160,20 @@ func (S3CredentialTranslator) Validate(
 		return nil
 	}
 
-	errs := []string{}
+	errs := []error{}
 
 	accessKeyId := vars[cosiapi.CredentialVar_S3_AccessKeyId]
 	if accessKeyId == "" {
-		errs = append(errs, "S3 access key ID cannot be unset")
+		errs = append(errs, fmt.Errorf("S3 access key ID cannot be unset"))
 	}
 
 	accessSecretKey := vars[cosiapi.CredentialVar_S3_AccessSecretKey]
 	if accessSecretKey == "" {
-		errs = append(errs, "S3 access secret key cannot be unset")
+		errs = append(errs, fmt.Errorf("S3 access secret key cannot be unset"))
 	}
 
 	if len(errs) > 0 {
-		return fmt.Errorf("S3 credential info is invalid: %v", errs)
+		return fmt.Errorf("S3 credential info is invalid: %w", errors.Join(errs...))
 	}
 	return nil
 }
diff --git a/sidecar/internal/reconciler/bucket.go b/sidecar/internal/reconciler/bucket.go
@@ -317,13 +317,13 @@ func parseProtocolBucketInfo(pbi *cosiproto.ObjectProtocolAndBucketInfo) (
 
 // convert an API proto list into an RPC proto message list
 func objectProtocolListFromApiList(apiList []cosiapi.ObjectProtocol) ([]*cosiproto.ObjectProtocol, error) {
-	errs := []string{}
+	errs := []error{}
 	out := []*cosiproto.ObjectProtocol{}
 
 	for _, apiProto := range apiList {
 		rpcProto, err := protocol.ObjectProtocolTranslator{}.ApiToRpc(apiProto)
 		if err != nil {
-			errs = append(errs, err.Error())
+			errs = append(errs, err)
 			continue
 		}
 		out = append(out, &cosiproto.ObjectProtocol{
@@ -332,7 +332,7 @@ func objectProtocolListFromApiList(apiList []cosiapi.ObjectProtocol) ([]*cosipro
 	}
 
 	if len(errs) > 0 {
-		return nil, fmt.Errorf("failed to parse protocol list: %v", errs)
+		return nil, fmt.Errorf("failed to parse protocol list: %w", errors.Join(errs...))
 	}
 	return out, nil
 }
@@ -362,7 +362,7 @@ func validateBucketSupportsProtocols(supported, required []cosiapi.ObjectProtoco
 		}
 	}
 	if len(unsupported) > 0 {
-		return fmt.Errorf("required protocols are not supported: %v", required)
+		return fmt.Errorf("required protocols are not supported: %v", unsupported)
 	}
 	return nil
 }

Original file line number	Diff line number	Diff line change
`@@ -306,7 +306,7 @@ func getAllBucketClaims(`
`306`	`306`	`}`
`307`	`307`
`308`	`308`	`if len(errs) > 0 {`
`309`		`- return nil, fmt.Errorf("could not get one or more BucketClaims: %v", errs)`
	`309`	`+ return nil, fmt.Errorf("could not get one or more BucketClaims: %w", errors.Join(errs...))`
`310`	`310`	`}`
`311`	`311`
`312`	`312`	`if len(claims) != len(claimAccesses) {`
`@@ -345,7 +345,7 @@ func markAllBucketClaimsAsAccessed(`
`345`	`345`	`}`
`346`	`346`	`}`
`347`	`347`	`if len(errs) > 0 {`
`348`		`- return fmt.Errorf("failed to mark one or more BucketClaims as having a BucketAccess reference: %v", errs)`
	`348`	`+ return fmt.Errorf("failed to mark one or more BucketClaims as having a BucketAccess reference: %w", errors.Join(errs...))`
`349`	`349`	`}`
`350`	`350`
`351`	`351`	`return nil`
`@@ -356,28 +356,28 @@ func validateAccessAgainstClass(`
`356`	`356`	`class *cosiapi.BucketAccessClassSpec,`
`357`	`357`	`access *cosiapi.BucketAccessSpec,`
`358`	`358`	`) error {`
`359`		`- errs := []string{}`
	`359`	`+ errs := []error{}`
`360`	`360`
`361`	`361`	`needServiceAccount := class.AuthenticationType == cosiapi.BucketAccessAuthenticationTypeServiceAccount`
`362`	`362`	`if needServiceAccount && access.ServiceAccountName == "" {`
`363`		`- errs = append(errs, "serviceAccountName must be specified")`
	`363`	`+ errs = append(errs, fmt.Errorf("serviceAccountName must be specified"))`
`364`	`364`	`}`
`365`	`365`
`366`	`366`	`if class.FeatureOptions.DisallowMultiBucketAccess && len(access.BucketClaims) > 1 {`
`367`		`- errs = append(errs, "multi-bucket access is disallowed")`
	`367`	`+ errs = append(errs, fmt.Errorf("multi-bucket access is disallowed"))`
`368`	`368`	`}`
`369`	`369`
`370`	`370`	`for _, claimRef := range access.BucketClaims {`
`371`	`371`	`if slices.Contains(class.FeatureOptions.DisallowedBucketAccessModes, claimRef.AccessMode) {`
`372`	`372`	`errs = append(errs,`
`373`		`- fmt.Sprintf("accessMode %q requested for BucketClaim %q is disallowed",`
	`373`	`+ fmt.Errorf("accessMode %q requested for BucketClaim %q is disallowed",`
`374`	`374`	`claimRef.AccessMode, claimRef.BucketClaimName),`
`375`	`375`	`)`
`376`	`376`	`}`
`377`	`377`	`}`
`378`	`378`
`379`	`379`	`if len(errs) > 0 {`
`380`		`- return fmt.Errorf("one or more features are disallowed by the BucketAccessClass: %v", errs)`
	`380`	`+ return fmt.Errorf("one or more features are disallowed by the BucketAccessClass: %w", errors.Join(errs...))`
`381`	`381`	`}`
`382`	`382`	`return nil`
`383`	`383`	`}`
Original file line number	Diff line number	Diff line change
`@@ -17,6 +17,7 @@ limitations under the License.`
`17`	`17`	`package protocol`
`18`	`18`
`19`	`19`	`import (`
	`20`	`+ "errors"`
`20`	`21`	`"fmt"`
`21`	`22`
`22`	`23`	`cosiapi "sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2"`
`@@ -60,15 +61,15 @@ func (AzureBucketInfoTranslator) ApiToRpc(vars map[cosiapi.BucketInfoVar]string)`
`60`	`61`	`func (AzureBucketInfoTranslator) Validate(`
`61`	`62`	`vars map[cosiapi.BucketInfoVar]string, _ cosiapi.BucketAccessAuthenticationType,`
`62`	`63`	`) error {`
`63`		`- errs := []string{}`
	`64`	`+ errs := []error{}`
`64`	`65`
`65`	`66`	`storageAccount := vars[cosiapi.BucketInfoVar_Azure_StorageAccount]`
`66`	`67`	`if storageAccount == "" {`
`67`		`- errs = append(errs, "azure storage account cannot be unset")`
	`68`	`+ errs = append(errs, fmt.Errorf("azure storage account cannot be unset"))`
`68`	`69`	`}`
`69`	`70`
`70`	`71`	`if len(errs) > 0 {`
`71`		`- return fmt.Errorf("azure bucket info is invalid: %v", errs)`
	`72`	`+ return fmt.Errorf("azure bucket info is invalid: %w", errors.Join(errs...))`
`72`	`73`	`}`
`73`	`74`	`return nil`
`74`	`75`	`}`
`@@ -107,15 +108,15 @@ func (AzureCredentialTranslator) Validate(`
`107`	`108`	`return nil`
`108`	`109`	`}`
`109`	`110`
`110`		`- errs := []string{}`
	`111`	`+ errs := []error{}`
`111`	`112`
`112`	`113`	`accessToken := vars[cosiapi.CredentialVar_Azure_AccessToken]`
`113`	`114`	`if accessToken == "" {`
`114`		`- errs = append(errs, "azure access token cannot be unset")`
	`115`	`+ errs = append(errs, fmt.Errorf("azure access token cannot be unset"))`
`115`	`116`	`}`
`116`	`117`
`117`	`118`	`if len(errs) > 0 {`
`118`		`- return fmt.Errorf("azure credential info is invalid: %v", errs)`
	`119`	`+ return fmt.Errorf("azure credential info is invalid: %w", errors.Join(errs...))`
`119`	`120`	`}`
`120`	`121`	`return nil`
`121`	`122`	`}`
Original file line number	Diff line number	Diff line change
`@@ -317,13 +317,13 @@ func parseProtocolBucketInfo(pbi *cosiproto.ObjectProtocolAndBucketInfo) (`
`317`	`317`
`318`	`318`	`// convert an API proto list into an RPC proto message list`
`319`	`319`	`func objectProtocolListFromApiList(apiList []cosiapi.ObjectProtocol) ([]*cosiproto.ObjectProtocol, error) {`
`320`		`- errs := []string{}`
	`320`	`+ errs := []error{}`
`321`	`321`	`out := []*cosiproto.ObjectProtocol{}`
`322`	`322`
`323`	`323`	`for _, apiProto := range apiList {`
`324`	`324`	`rpcProto, err := protocol.ObjectProtocolTranslator{}.ApiToRpc(apiProto)`
`325`	`325`	`if err != nil {`
`326`		`- errs = append(errs, err.Error())`
	`326`	`+ errs = append(errs, err)`
`327`	`327`	`continue`
`328`	`328`	`}`
`329`	`329`	`out = append(out, &cosiproto.ObjectProtocol{`
`@@ -332,7 +332,7 @@ func objectProtocolListFromApiList(apiList []cosiapi.ObjectProtocol) ([]*cosipro`
`332`	`332`	`}`
`333`	`333`
`334`	`334`	`if len(errs) > 0 {`
`335`		`- return nil, fmt.Errorf("failed to parse protocol list: %v", errs)`
	`335`	`+ return nil, fmt.Errorf("failed to parse protocol list: %w", errors.Join(errs...))`
`336`	`336`	`}`
`337`	`337`	`return out, nil`
`338`	`338`	`}`
`@@ -362,7 +362,7 @@ func validateBucketSupportsProtocols(supported, required []cosiapi.ObjectProtoco`
`362`	`362`	`}`
`363`	`363`	`}`
`364`	`364`	`if len(unsupported) > 0 {`
`365`		`- return fmt.Errorf("required protocols are not supported: %v", required)`
	`365`	`+ return fmt.Errorf("required protocols are not supported: %v", unsupported)`
`366`	`366`	`}`
`367`	`367`	`return nil`
`368`	`368`	`}`