Add more tests for Azure KV. Move common test pieces to common file

README.md

@@ -81,7 +81,7 @@ To deploy it just run `kubectl apply -f secretdefinition-sample.yaml`
 | `azure-kv.tenant-id` | `""` | Azure KeyVault Tenant ID. `AZURE_TENANT_ID` environment would take precedence |
 | `azure-kv.client-id` | `""` | Azure KeyVault Cliend ID used to authenticate. `AZURE_CLIENT_ID` environment would take precedence |
 | `azure-kv.client-secret` | `""` | Azure KeyVault Client Secret used to authenticate. `AZURE_CLIENT_SECRET` environment would take precedence |
-| `azure-kv.managed-client-id` | `""` | Azure Managed Identity Resource ID used to authenticate. `AZURE_MANAGED_CLIENT_ID` environment would take precedence |
+| `azure-kv.managed-client-id` | `""` | Azure Managed Identity Client ID used to authenticate. `AZURE_MANAGED_CLIENT_ID` environment would take precedence |
 | `azure-kv.managed-resource-id` | `""` | Azure Managed Identity Resource ID used to authenticate. `AZURE_MANAGED_RESOURCE_ID` environment would take precedence |
 | `vault.url` | https://127.0.0.1:8200 | Vault address. `VAULT_ADDR` environment would take precedence. |
 | `vault.role-id` | `""` | Vault appRole `role_id`. `VAULT_ROLE_ID` environment would take precedence. |

backend/azure_kv.go

@@ -8,7 +8,6 @@ import (
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
 	"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
 	"github.com/Azure/azure-sdk-for-go/sdk/keyvault/azsecrets"
-	"github.com/Azure/go-autorest/autorest"
 	"github.com/go-logr/logr"
 	"github.com/tuenti/secrets-manager/errors"
 )
@@ -69,6 +68,7 @@ func azureKeyVaultClient(ctx context.Context, l logr.Logger, cfg Config) (*azure
 	if err != nil {
 		logger.Error(err, "Error occured while creating Azure KV client")
 		akvMetrics.updateLoginErrorsTotalMetric()
+		return nil, err
 	}
 
 	logger.Info("successfully logged into Azure KeyVault")
@@ -91,11 +91,12 @@ func (c *azureKVClient) ReadSecret(path string, key string) (string, error) {
 
 	if err != nil {
 		errorType := errors.UnknownErrorType
-		if v, ok := err.(autorest.DetailedError); ok {
-			if v.StatusCode == 404 {
+		var responseError *azcore.ResponseError
+		if goerrors.As(err, &responseError) {
+			if responseError.StatusCode == 404 {
 				errorType = errors.BackendSecretNotFoundErrorType
 			}
-			if v.StatusCode == 403 {
+			if responseError.StatusCode == 403 {
 				errorType = errors.BackendSecretForbiddenErrorType
 			}
 		}

backend/azure_kv_metrics_test.go

@@ -8,11 +8,6 @@ import (
 	"github.com/tuenti/secrets-manager/errors"
 )
 
-const (
-	fakeKeyVaultName   = "azure-keyvault-fake-name"
-	fakeKeyVaultTenant = "01234567-0123-0123-0123-0123456789ab"
-)
-
 func TestAzureKVUpdateLoginErrorsTotal(t *testing.T) {
 	metrics := newAzureKVMetrics(fakeKeyVaultName, fakeKeyVaultTenant)
 	azureKVLoginErrorsTotal.Reset()

backend/azure_kv_test.go

@@ -2,13 +2,108 @@ package backend
 
 import (
 	"context"
+	"encoding/json"
+	"fmt"
+	"net/http"
 	"testing"
+	"time"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
+	"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
 	"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
+	"github.com/Azure/azure-sdk-for-go/sdk/keyvault/azsecrets"
+	"github.com/gorilla/mux"
+
 	"github.com/stretchr/testify/assert"
 )
 
+const (
+	fakeKeyVaultName   = "azure-keyvault-fake-name"
+	fakeKeyVaultTenant = "01234567-0123-0123-0123-0123456789ab"
+	fakeKeyVaultSecret = "fake-secret"
+)
+
+var akvSecrets = map[string]struct {
+	value  string
+	access bool
+}{
+	fakeKeyVaultSecret: {value: "some-fake-value", access: true},
+	"exists":           {value: "yes", access: true},
+	"internal-error":   {value: "\"bad-scaped", access: true},
+	"forbidden":        {value: "yes", access: false},
+}
+
+func akvGetSecret(w http.ResponseWriter, r *http.Request) {
+	// Info about what keyvault responses should look like extracted from
+	// https://github.com/Azure/azure-sdk-for-go/blob/c73b114ded83c0a9c2685336b8b90836c1530cb3/sdk/keyvault/azsecrets/testdata/recordings/TestSetGetSecret.json
+	vars := mux.Vars(r)
+	jsonData := "{}"
+	if v, ok := akvSecrets[vars["secretName"]]; ok {
+		if v.access {
+			jsonData = fmt.Sprintf(`
+			{
+				"value": "%s",
+				"id": "https://%s.vault.azure.net/secrets/%s/3f3b11064811494a8a8b27edf4f0985b",
+        "attributes": {
+          "enabled": true,
+          "created": 1643130727,
+          "updated": 1643130727,
+          "recoveryLevel": "CustomizedRecoverable\u002BPurgeable",
+          "recoverableDays": 7
+        }
+			}`, v.value, fakeKeyVaultName, vars["secretName"])
+		} else {
+			w.WriteHeader(http.StatusForbidden)
+		}
+	} else {
+		jsonData = fmt.Sprintf(`
+		"error": {
+          "code": "SecretNotFound",
+          "message": "Secret not found: %s"
+        }
+		`, vars["secretName"])
+		w.WriteHeader(http.StatusNotFound)
+	}
+	var response interface{}
+
+	if err := json.Unmarshal([]byte(jsonData), &response); err != nil {
+		fmt.Printf("unable to unmarshal json %v", err)
+		w.WriteHeader(http.StatusInternalServerError)
+	}
+	w.Header().Set("Content-Type", "application/json")
+	w.Header().Set("x-ms-keyvault-network-info", "conn_type=Ipv4;addr=72.49.29.93;act_addr_fam=InterNetwork;")
+	w.Header().Set("x-ms-keyvault-region", "westus2")
+	w.Header().Set("x-ms-keyvault-service-version", "1.9.264.2")
+	w.Header().Set("x-ms-request-id", "868ba1d2-efe7-4930-b3ad-d010cf499778")
+	w.Header().Set("X-Powered-By", "ASP.NET")
+	// Trick Azure client to make it think everything is legit
+	w.Header().Set(
+		"WWW-Authenticate",
+		"Bearer authorization=\u0022https://login.windows.net/72f988bf-86f1-41af-91ab-2d7cd011db47\u0022, resource=\u0022https://vault.azure.net\u0022",
+	)
+	json.NewEncoder(w).Encode(response)
+}
+
+// Copied from https://github.com/Azure/azure-sdk-for-go/blob/35fb64f82ef3b3308f55b1da37c1fec36bdd4166/sdk/keyvault/azsecrets/utils_test.go
+type FakeCredential struct {
+	accountName string
+	accountKey  string
+}
+
+func NewFakeCredential(accountName, accountKey string) *FakeCredential {
+	return &FakeCredential{
+		accountName: accountName,
+		accountKey:  accountKey,
+	}
+}
+
+func (f *FakeCredential) GetToken(ctx context.Context, options policy.TokenRequestOptions) (*azcore.AccessToken, error) {
+	return &azcore.AccessToken{
+		Token:     "faketoken",
+		ExpiresOn: time.Date(2040, time.January, 1, 1, 1, 1, 1, time.UTC),
+	}, nil
+}
+
 func TestGetAzureCredential(t *testing.T) {
 	cases := []struct {
 		cfg Config
@@ -106,5 +201,67 @@ func TestAzureKeyVaultClient(t *testing.T) {
 	cfg := Config{}
 	client, err := azureKeyVaultClient(context.TODO(), logger, cfg)
 	assert.NotNilf(t, err, "Empty config should generate an error")
-	assert.Nil(t, client, "Empty configu should not generate any client")
+	assert.Nilf(t, client, "Empty config should not generate any client")
+
+	// Managed Identity auth is performed at client call, so the client generated is "valid"
+	cfg = Config{
+		AzureKVTenantID: fakeKeyVaultTenant,
+		AzureKVClientID: "fake-client-id",
+	}
+	client, err = azureKeyVaultClient(context.TODO(), logger, cfg)
+	assert.NotNilf(t, err, "Invalid Service Principal Authentication should generate an error")
+	assert.Nilf(t, client, "Invalid Service Principal Authentication should not generate any client")
+
+	// Authentication is performed at client call, so the client generated is "valid"
+	// This happens for both service principal and managed identity
+	cfg = Config{
+		AzureKVTenantID:     fakeKeyVaultTenant,
+		AzureKVClientID:     "fake-client-id",
+		AzureKVClientSecret: "fake-client-secret",
+	}
+	client, err = azureKeyVaultClient(context.TODO(), logger, cfg)
+	assert.Nilf(t, err, "Service Principal Authentication should not generate error")
+	assert.NotNilf(t, client, "Service Principal Authentication should generate a client")
+
+	cfg = Config{AzureKVManagedClientID: "fake-client-id"}
+	client, err = azureKeyVaultClient(context.TODO(), logger, cfg)
+	assert.Nilf(t, err, "Managed Identity Authentication should not generate error")
+	assert.NotNilf(t, client, "Managed Identity Authentication should generate a client")
+}
+
+func TestAzureKVClientReadSecret(t *testing.T) {
+	akvMetrics = newAzureKVMetrics(fakeKeyVaultName, fakeKeyVaultTenant)
+	azClient, _ := azsecrets.NewClient(
+		testingCfg.VaultURL, // Is a mock server, valid for both cases
+		NewFakeCredential("fake", "fake"),
+		nil,
+	)
+	client := azureKVClient{
+		client:       azClient,
+		keyvaultName: "fakekvurl",
+		context:      context.TODO(),
+		logger:       logger,
+	}
+
+	value, err := client.ReadSecret("exists", "")
+	assert.Nil(t, err)
+	assert.Equal(t, akvSecrets["exists"].value, value)
+
+	value, err = client.ReadSecret(fakeKeyVaultSecret, "")
+	assert.Nil(t, err)
+	assert.Equal(t, akvSecrets[fakeKeyVaultSecret].value, value)
+
+	value, err = client.ReadSecret("forbidden", "")
+	assert.NotNil(t, err)
+	assert.Equal(t, "", value)
+	assert.IsType(t, new(azcore.ResponseError), err)
+
+	value, err = client.ReadSecret("not-found", "")
+	assert.NotNil(t, err)
+	assert.Equal(t, "", value)
+	assert.IsType(t, new(azcore.ResponseError), err)
+
+	value, err = client.ReadSecret("internal-error", "")
+	assert.NotNil(t, err)
+	assert.Equal(t, "", value)
 }

backend/backend_test.go

@@ -3,10 +3,23 @@ package backend
 import (
 	"context"
 	"fmt"
+	"net/http/httptest"
+	"os"
+	"sync"
 	"testing"
 
+	"github.com/go-logr/logr"
+	"github.com/gorilla/mux"
 	"github.com/stretchr/testify/assert"
 	"github.com/tuenti/secrets-manager/errors"
+	"sigs.k8s.io/controller-runtime/pkg/log/zap"
+)
+
+var (
+	testingCfg Config
+	server     *httptest.Server
+	mutex      sync.Mutex
+	logger     logr.Logger
 )
 
 func TestNotImplementedBackend(t *testing.T) {
@@ -17,3 +30,45 @@ func TestNotImplementedBackend(t *testing.T) {
 	_, err := NewBackendClient(ctx, backend, nil, cfg)
 	assert.EqualError(t, err, fmt.Sprintf("[%s] backend %s not supported", errors.BackendNotImplementedErrorType, backend))
 }
+
+func TestMain(m *testing.M) {
+	r := mux.NewRouter()
+	v1SysHandler := r.PathPrefix(fmt.Sprintf("/%s/sys", vaultAPIVersion)).Subrouter()
+	v1AuthHandler := r.PathPrefix(fmt.Sprintf("/%s/auth", vaultAPIVersion)).Subrouter()
+	v1SecretHandler := r.PathPrefix(fmt.Sprintf("/%s/secret", vaultAPIVersion)).Subrouter()
+	akvSecretsHandler := r.PathPrefix("/secrets").Subrouter()
+
+	v1SysHandler.HandleFunc("/health", v1SysHealth).Methods("GET")
+	v1AuthHandler.HandleFunc("/token/lookup-self", v1AuthTokenLookupSelf).Methods("GET")
+	v1AuthHandler.HandleFunc("/token/renew-self", v1AuthTokenRenewSelf).Methods("PUT")
+	v1AuthHandler.HandleFunc("/approle/login", v1AuthAppRoleLogin).Methods("PUT")
+	v1AuthHandler.HandleFunc("/kubernetes/login", v1AuthKubernetesLogin).Methods("PUT")
+	v1SecretHandler.HandleFunc("/data/test", v1SecretTestKv2).Methods("GET")
+	v1SecretHandler.HandleFunc("/test", v1SecretTestKv1).Methods("GET")
+
+	akvSecretsHandler.PathPrefix("/{secretName}").HandlerFunc(akvGetSecret).Methods("GET")
+
+	server = httptest.NewServer(r)
+	defer server.Close()
+
+	testingCfg = Config{
+		VaultURL:                string(server.URL),
+		VaultRoleID:             vaultFakeRoleID,
+		VaultSecretID:           vaultFakeSecretID,
+		VaultTokenPollingPeriod: 1,
+		VaultEngine:             "kv2",
+		VaultApprolePath:        vaultAppRolePath,
+	}
+
+	vaultTestCfg = &testConfig{
+		tokenRenewable:  defaultTokenRenewable,
+		tokenTTL:        defaultTokenTTL,
+		tokenRevoked:    defaultRevokedToken,
+		invalidRoleID:   defaultInvalidAppRole,
+		invalidSecretID: defaultInvalidAppRole,
+	}
+
+	logger = zap.New(zap.UseDevMode(true))
+
+	os.Exit(m.Run())
+}