chore: parse the disabled secret error

Signed-off-by: Shahram Kalantari <[email protected]>
shahramk64 · Nov 12, 2024 · 864b18f · 864b18f
1 parent cbabb9c
commit 864b18f
Show file tree

Hide file tree

Showing 2 changed files with 109 additions and 21 deletions.
diff --git a/pkg/keymanagementprovider/azurekeyvault/provider.go b/pkg/keymanagementprovider/azurekeyvault/provider.go
@@ -26,6 +26,8 @@ import (
 	"encoding/pem"
 	"errors"
 	"fmt"
+	"io"
+	"net/http"
 	"strconv"
 	"strings"
 	"time"
@@ -189,14 +191,9 @@ func (s *akvKMProvider) GetCertificates(ctx context.Context) (map[keymanagementp
 		startTime := time.Now()
 		secretResponse, err := s.secretKVClient.GetSecret(ctx, keyVaultCert.Name, keyVaultCert.Version)
 		if err != nil {
-			// I am aware that there are so many logs here and inside isSecretDisabledError, but I am trying to understand the structure of the error
-			// I'll make sure to remove them before merging
-			logger.GetLogger(ctx, logOpt).Infof("s.secretKVClient.GetSecret errored:, err %v", err)
 			if isSecretDisabledError(err) {
 				// if secret is disabled, get the version of the certificate for status
-				logger.GetLogger(ctx, logOpt).Infof("calling s.certificateKVClient.GetCertificate:, keyVaultCert.Name %v, keyVaultCert.Version %v", keyVaultCert.Name, keyVaultCert.Version)
 				certResponse, err := s.certificateKVClient.GetCertificate(ctx, keyVaultCert.Name, keyVaultCert.Version)
-				logger.GetLogger(ctx, logOpt).Infof("s.certificateKVClient.GetCertificate was called,checking for possible errors")
 				if err != nil {
 					return nil, nil, fmt.Errorf("failed to get certificate objectName:%s, objectVersion:%s, error: %w", keyVaultCert.Name, keyVaultCert.Version, err)
 				}
@@ -205,7 +202,6 @@ func (s *akvKMProvider) GetCertificates(ctx context.Context) (map[keymanagementp
 				isEnabled := *certBundle.Attributes.Enabled
 				lastRefreshed := startTime.Format(time.RFC3339)
 				certProperty := getStatusProperty(keyVaultCert.Name, keyVaultCert.Version, lastRefreshed, isEnabled)
-				logger.GetLogger(ctx, logOpt).Infof("certProperty %v", certProperty)
 				certsStatus = append(certsStatus, certProperty)
 				mapKey := keymanagementprovider.KMPMapKey{Name: keyVaultCert.Name, Version: keyVaultCert.Version, Enabled: isEnabled}
 				keymanagementprovider.DeleteCertificateFromMap(s.resource, mapKey)
@@ -448,17 +444,38 @@ func getObjectVersion(id string) string {
 }
 
 func isSecretDisabledError(err error) bool {
-	logger.GetLogger(context.Background(), logOpt).Infof("Inside isSecretDisabledError ------------------")
-	var responseError *azcore.ResponseError
-	if errors.As(err, &responseError) {
-		// Is there a better way to check if the error is a secret disabled error?
-		// if responseError.ErrorCode == "Forbidden" && strings.Contains(responseError.Error(), "SecretDisabled") {
-		if strings.Contains(responseError.Error(), "Forbidden") && strings.Contains(responseError.Error(), "SecretDisabled") {
-			logger.GetLogger(context.Background(), logOpt).Infof("Leaving isSecretDisabledError returnning true ------------------")
-			return true
+	// AzureError defines the structure of the error response from Azure Key Vault
+	type AzureError struct {
+		Error struct {
+			Code       string `json:"code"`
+			Message    string `json:"message"`
+			InnerError struct {
+				Code string `json:"code"`
+			} `json:"innererror"`
+		} `json:"error"`
+	}
+
+	// Parse err and make sure it is a secretDisabled error and return true
+	const ErrorCodeForbidden = "Forbidden"
+	const SecretDisabledCode = "SecretDisabled"
+	var httpErr *azcore.ResponseError
+	if errors.As(err, &httpErr) {
+		if httpErr.StatusCode == http.StatusForbidden {
+			var azureError AzureError
+			errorResponseBody, readErr := io.ReadAll(httpErr.RawResponse.Body)
+			if readErr != nil {
+				return false
+			}
+			jsonErr := json.Unmarshal(errorResponseBody, &azureError)
+			if jsonErr == nil {
+				if azureError.Error.Code == ErrorCodeForbidden && azureError.Error.InnerError.Code == SecretDisabledCode {
+					return true
+				}
+			}
 		}
 	}
-	logger.GetLogger(context.Background(), logOpt).Infof("Leaving isSecretDisabledError returnning falses ------------------")
+
+	// Return false if it's not a secretDisabled error
 	return false
 }
 

diff --git a/pkg/keymanagementprovider/azurekeyvault/provider_test.go b/pkg/keymanagementprovider/azurekeyvault/provider_test.go
@@ -21,6 +21,9 @@ import (
 	"context"
 	"crypto"
 	"errors"
+	"io"
+	"net/http"
+	"strings"
 	"testing"
 	"time"
 
@@ -251,10 +254,23 @@ func TestGetCertificates(t *testing.T) {
 			},
 			mockSecretKVClient: &MockSecretKVClient{
 				GetSecretFunc: func(_ context.Context, _ string, _ string) (azsecrets.GetSecretResponse, error) {
-					err := azcore.ResponseError{
-						ErrorCode: "Forbidden, SecretDisabled",
+					rawResponse := `{
+						"error": {
+							"code": "Forbidden",
+							"message": "Operation get is not allowed on a disabled secret.",
+							"innererror": {
+								"code": "SecretDisabled"
+							}
+						}
+					}`
+
+					httpErr := &azcore.ResponseError{
+						StatusCode: http.StatusForbidden,
+						RawResponse: &http.Response{
+							Body: io.NopCloser(strings.NewReader(rawResponse)),
+						},
 					}
-					return azsecrets.GetSecretResponse{}, &err
+					return azsecrets.GetSecretResponse{}, httpErr
 				},
 			},
 			expectedErr: false,
@@ -268,10 +284,23 @@ func TestGetCertificates(t *testing.T) {
 			},
 			mockSecretKVClient: &MockSecretKVClient{
 				GetSecretFunc: func(_ context.Context, _ string, _ string) (azsecrets.GetSecretResponse, error) {
-					err := azcore.ResponseError{
-						ErrorCode: "SecretDisabled",
+					rawResponse := `{
+						"error": {
+							"code": "Forbidden",
+							"message": "Operation get is not allowed on a disabled secret.",
+							"innererror": {
+								"code": "SecretDisabled"
+							}
+						}
+					}`
+
+					httpErr := &azcore.ResponseError{
+						StatusCode: http.StatusForbidden,
+						RawResponse: &http.Response{
+							Body: io.NopCloser(strings.NewReader(rawResponse)),
+						},
 					}
-					return azsecrets.GetSecretResponse{}, &err
+					return azsecrets.GetSecretResponse{}, httpErr
 				},
 			},
 			expectedErr: true,
@@ -992,3 +1021,45 @@ func TestInitializeKvClient_FailureInAzCertificatesClient(t *testing.T) {
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "failed to create workload identity credential")
 }
+func TestIsSecretDisabledError(t *testing.T) {
+	rawResponse := `{
+		"error": {
+			"code": "Forbidden",
+			"message": "Operation get is not allowed on a disabled secret.",
+			"innererror": {
+				"code": "SecretDisabled"
+			}
+		}
+	}`
+
+	httpErr := &azcore.ResponseError{
+		StatusCode: http.StatusForbidden,
+		RawResponse: &http.Response{
+			Body: io.NopCloser(strings.NewReader(rawResponse)),
+		},
+	}
+
+	testCases := []struct {
+		name        string
+		err         error
+		expectedRes bool
+	}{
+		{
+			name:        "SecretDisabledError",
+			err:         httpErr,
+			expectedRes: true,
+		},
+		{
+			name:        "NonSecretDisabledError",
+			err:         errors.New("some other error"),
+			expectedRes: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			res := isSecretDisabledError(tc.err)
+			assert.Equal(t, tc.expectedRes, res)
+		})
+	}
+}