Bugfixes and some code improvements

minio · Apr 8, 2024 · e59983b · e59983b
1 parent 51c69ad
commit e59983b
Show file tree

Hide file tree

Showing 7 changed files with 191 additions and 294 deletions.
diff --git a/internal/keystore/azure/client.go b/internal/keystore/azure/client.go
@@ -8,9 +8,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"math"
 	"net/http"
-	"path"
 	"time"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets"
@@ -43,17 +41,9 @@ type client struct {
 func (c *client) CreateSecret(ctx context.Context, name, value string) (status, error) {
 	_, err := c.azsecretsClient.SetSecret(ctx, name, azsecrets.SetSecretParameters{
 		Value: &value,
-	}, &azsecrets.SetSecretOptions{})
+	}, nil)
 	if err != nil {
-		azResp, ok := transportErrToResponseError(err)
-		if !ok {
-			return status{}, err
-		}
-		return status{
-			StatusCode: azResp.StatusCode,
-			ErrorCode:  azResp.ErrorCode,
-			Message:    azResp.errorResponse.Error.Message,
-		}, nil
+		return transportErrToStatus(err)
 	}
 	return status{
 		StatusCode: http.StatusOK,
@@ -77,20 +67,13 @@ func (c *client) CreateSecret(ctx context.Context, name, value string) (status,
 // if the secret is disabled, expired or should not
 // be used, yet.
 func (c *client) GetSecret(ctx context.Context, name, version string) (string, status, error) {
-	response, err := c.azsecretsClient.GetSecret(ctx, name, version, &azsecrets.GetSecretOptions{})
+	response, err := c.azsecretsClient.GetSecret(ctx, name, version, nil)
 	if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
 		return "", status{}, err
 	}
 	if err != nil {
-		azResp, ok := transportErrToResponseError(err)
-		if !ok {
-			return "", status{}, err
-		}
-		return "", status{
-			StatusCode: azResp.StatusCode,
-			ErrorCode:  azResp.ErrorCode,
-			Message:    azResp.errorResponse.Error.Message,
-		}, nil
+		stat, err := transportErrToStatus(err)
+		return "", stat, err
 	}
 	if response.Attributes.Enabled != nil && !*response.Attributes.Enabled {
 		return "", status{
@@ -135,17 +118,9 @@ func (c *client) GetSecret(ctx context.Context, name, version string) (string, s
 // even if it returns 200 OK. Instead, the secret may be in
 // a transition state from "active" to (soft) deleted.
 func (c *client) DeleteSecret(ctx context.Context, name string) (status, error) {
-	_, err := c.azsecretsClient.DeleteSecret(ctx, name, &azsecrets.DeleteSecretOptions{})
+	_, err := c.azsecretsClient.DeleteSecret(ctx, name, nil)
 	if err != nil {
-		azResp, ok := transportErrToResponseError(err)
-		if !ok {
-			return status{}, err
-		}
-		return status{
-			StatusCode: azResp.StatusCode,
-			ErrorCode:  azResp.ErrorCode,
-			Message:    azResp.errorResponse.Error.Message,
-		}, nil
+		return transportErrToStatus(err)
 	}
 	return status{
 		StatusCode: http.StatusOK,
@@ -158,17 +133,12 @@ func (c *client) DeleteSecret(ctx context.Context, name string) (status, error)
 // recovered. Therefore, deleting a KeyVault secret permanently is
 // a two-step process.
 func (c *client) PurgeSecret(ctx context.Context, name string) (status, error) {
-	_, err := c.azsecretsClient.PurgeDeletedSecret(ctx, name, &azsecrets.PurgeDeletedSecretOptions{})
+	_, err := c.azsecretsClient.PurgeDeletedSecret(ctx, name, nil)
 	if err != nil {
-		azResp, ok := transportErrToResponseError(err)
-		if !ok {
-			return status{}, err
+		stat, err := transportErrToStatus(err)
+		if stat.StatusCode != http.StatusNoContent && stat.StatusCode != http.StatusOK && stat.StatusCode != http.StatusNotFound {
+			return stat, err
 		}
-		return status{
-			StatusCode: azResp.StatusCode,
-			ErrorCode:  azResp.ErrorCode,
-			Message:    azResp.errorResponse.Error.Message,
-		}, nil
 	}
 	return status{
 		StatusCode: http.StatusOK,
@@ -183,53 +153,39 @@ func (c *client) PurgeSecret(ctx context.Context, name string) (status, error) {
 // versions of the given secret. When a secret contains more then 25
 // versions GetFirstVersions returns a status with a 422 HTTP error code.
 func (c *client) GetFirstVersion(ctx context.Context, name string) (string, status, error) {
-	pager := c.azsecretsClient.NewListSecretPropertiesVersionsPager(name, &azsecrets.ListSecretPropertiesVersionsOptions{})
+	pager := c.azsecretsClient.NewListSecretPropertiesVersionsPager(name, nil)
+	page, err := pager.NextPage(ctx)
+	if err != nil {
+		stat, err := transportErrToStatus(err)
+		return "", stat, err
+	}
 	if pager.More() {
-		page, err := pager.NextPage(ctx)
-		if err != nil {
-			azResp, ok := transportErrToResponseError(err)
-			if !ok {
-				return "", status{}, err
-			}
-			return "", status{
-				StatusCode: azResp.StatusCode,
-				ErrorCode:  azResp.ErrorCode,
-				Message:    azResp.errorResponse.Error.Message,
-			}, nil
-		}
-		if page.SecretPropertiesListResult.NextLink != nil && *page.SecretPropertiesListResult.NextLink != "" {
-			return "", status{
-				StatusCode: http.StatusUnprocessableEntity,
-				ErrorCode:  "TooManyObjectVersions",
-				Message:    fmt.Sprintf("There are too many versions of %q.", name),
-			}, nil
-		}
-		if len(page.SecretPropertiesListResult.Value) == 0 {
-			return "", status{
-				StatusCode: http.StatusNotFound,
-				ErrorCode:  "NoObjectVersions",
-				Message:    fmt.Sprintf("There are no versions of %q.", name),
-			}, nil
-		}
-		var (
-			id        string                 // most recent Secret ID
-			createdAt int64  = math.MaxInt64 // most recent createdAt UNIX timestamp
-		)
-		for _, v := range page.SecretPropertiesListResult.Value {
-			if v.Attributes != nil && v.Attributes.Created != nil && v.ID != nil {
-				if createdAt > (*v.Attributes.Created).Unix() {
-					createdAt = (*v.Attributes.Created).Unix()
-					id = v.ID.Version()
-				}
+		return "", status{
+			StatusCode: http.StatusUnprocessableEntity,
+			ErrorCode:  "TooManyObjectVersions",
+			Message:    fmt.Sprintf("There are too many versions of %q.", name),
+		}, nil
+	}
+	if len(page.SecretPropertiesListResult.Value) == 0 {
+		return "", status{
+			StatusCode: http.StatusNotFound,
+			ErrorCode:  "NoObjectVersions",
+			Message:    fmt.Sprintf("There are no versions of %q.", name),
+		}, nil
+	}
+	var (
+		version   string           // most recent Secret version
+		createdAt *time.Time = nil // most recent createdAt UNIX timestamp
+	)
+	for _, v := range page.SecretPropertiesListResult.Value {
+		if v.Attributes != nil && v.Attributes.Created != nil && v.ID != nil {
+			if createdAt == nil || createdAt.After(*v.Attributes.Created) {
+				createdAt = v.Attributes.Created
+				version = v.ID.Version()
 			}
 		}
-		return path.Base(id), status{
-			StatusCode: http.StatusOK,
-		}, nil
 	}
-	return "", status{
-		StatusCode: http.StatusNotFound,
-		ErrorCode:  "NoObjectVersions",
-		Message:    fmt.Sprintf("There are no versions of %q.", name),
+	return version, status{
+		StatusCode: http.StatusOK,
 	}, nil
 }
diff --git a/internal/keystore/azure/error.go b/internal/keystore/azure/error.go
@@ -6,9 +6,9 @@ package azure
 
 import (
 	"encoding/json"
-	"net/http"
+	"errors"
 
-	"aead.dev/mem"
+	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
 )
 
 // errorResponse is a KeyVault secrets API error response.
@@ -22,18 +22,19 @@ type errorResponse struct {
 	} `json:"error"`
 }
 
-// parseErrorResponse parses the response body as
-// KeyVault secrets API error response.
-func parseErrorResponse(resp *http.Response) (errorResponse, error) {
-	const MaxSize = 1 * mem.MiB
-	limit := mem.Size(resp.ContentLength)
-	if limit < 0 || limit > MaxSize {
-		limit = MaxSize
+// transportErrToStatus converts a transport error to a Status.
+func transportErrToStatus(err error) (status, error) {
+	var rerr *azcore.ResponseError
+	if errors.As(err, &rerr) {
+		var errorResponse errorResponse
+		if rerr.RawResponse != nil {
+			json.NewDecoder(rerr.RawResponse.Body).Decode(&errorResponse)
+		}
+		return status{
+			ErrorCode:  errorResponse.Error.Inner.Code,
+			StatusCode: rerr.StatusCode,
+			Message:    errorResponse.Error.Message,
+		}, nil
 	}
-
-	var response errorResponse
-	if err := json.NewDecoder(mem.LimitReader(resp.Body, limit)).Decode(&response); err != nil {
-		return errorResponse{}, err
-	}
-	return response, nil
+	return status{}, err
 }
diff --git a/internal/keystore/azure/key-vault-error.go b/internal/keystore/azure/key-vault-error.go