test(azure): mock credentials and service APIs in all Azure tests

internal/secrets/azure_credential_guard_test.go

@@ -0,0 +1,35 @@
+package secrets
+
+import (
+	"os"
+	"testing"
+)
+
+// TestMain pins the Azure credential environment for every test in this
+// package so no test can walk the live DefaultAzureCredential fallback chain
+// (IMDS probe, `az account get-access-token`, `pwsh Get-AzAccessToken`). The
+// pwsh leg reads the MSAL token cache from the macOS login keychain, which
+// pops an interactive password prompt on developer machines whenever
+// `go test ./...` runs.
+//
+// AZURE_TOKEN_CREDENTIALS restricts the chain to EnvironmentCredential only
+// (supported since azidentity v1.10), and the dummy client-secret variables
+// let that credential construct without contacting anything. A test that
+// accidentally acquires a real token fails fast against the dummy tenant
+// instead of harvesting local developer credentials.
+func TestMain(m *testing.M) {
+	os.Setenv("AZURE_TOKEN_CREDENTIALS", "EnvironmentCredential")
+	os.Setenv("AZURE_TENANT_ID", "00000000-0000-0000-0000-000000000000")
+	os.Setenv("AZURE_CLIENT_ID", "00000000-0000-0000-0000-000000000000")
+	os.Setenv("AZURE_CLIENT_SECRET", "dummy-test-client-secret")
+	os.Exit(m.Run())
+}
+
+// TestAzureCredentialChainRestricted is the regression guard for the keychain
+// prompt incident: it fails if TestMain stops pinning the restricted
+// credential chain.
+func TestAzureCredentialChainRestricted(t *testing.T) {
+	if got := os.Getenv("AZURE_TOKEN_CREDENTIALS"); got != "EnvironmentCredential" {
+		t.Fatalf("AZURE_TOKEN_CREDENTIALS = %q; TestMain must pin it to EnvironmentCredential so tests never invoke the az/pwsh/IMDS credential legs", got)
+	}
+}

internal/secrets/azure_resolver_coverage_test.go

@@ -2,23 +2,33 @@ package secrets
 
 import (
 	"context"
+	"encoding/json"
+	"net/http"
+	"sync/atomic"
 	"testing"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-// TestAzureResolver_DirectMethods tests the actual AzureResolver methods
-// These tests exercise the real code paths but may skip if Azure credentials are unavailable.
+// notFoundHandler simulates the Key Vault response for a missing secret.
+func notFoundHandler(w http.ResponseWriter, _ *http.Request) {
+	w.Header().Set("Content-Type", "application/json")
+	w.WriteHeader(http.StatusNotFound)
+	_ = json.NewEncoder(w).Encode(map[string]any{
+		"error": map[string]any{"code": "SecretNotFound", "message": "Secret not found"},
+	})
+}
+
+// TestAzureResolver_DirectMethods tests the real constructor wiring. It makes
+// no requests, and TestMain pins the credential chain to a dummy
+// EnvironmentCredential, so construction is deterministic and offline.
 func TestAzureResolver_DirectMethods(t *testing.T) {
 	ctx := context.Background()
 
-	// Try to create an Azure resolver
 	resolver, err := NewAzureResolver(ctx, "https://test-vault.vault.azure.net/")
-	if err != nil {
-		t.Skipf("Skipping test: Azure config not available: %v", err)
-	}
+	require.NoError(t, err)
 	defer resolver.Close()
 
 	// Test that the resolver is properly configured
@@ -28,89 +38,67 @@ func TestAzureResolver_DirectMethods(t *testing.T) {
 
 // TestAzureResolver_GetSecret_NonExistent tests getting a non-existent secret.
 func TestAzureResolver_GetSecret_NonExistent(t *testing.T) {
-	ctx := context.Background()
+	resolver, server := newTestAzureResolver(t, notFoundHandler)
+	defer server.Close()
 
-	resolver, err := NewAzureResolver(ctx, "https://test-vault.vault.azure.net/")
-	if err != nil {
-		t.Skipf("Skipping test: Azure config not available: %v", err)
-	}
-	defer resolver.Close()
+	_, err := resolver.GetSecret(context.Background(), "cudly-test-nonexistent-secret-12345-xyz")
 
-	// Try to get a secret that definitely doesn't exist
-	_, err = resolver.GetSecret(ctx, "cudly-test-nonexistent-secret-12345-xyz")
-
-	// Should get an error (either not found or permission denied)
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "failed to get secret")
 }
 
 // TestAzureResolver_GetSecretJSON_NonExistent tests getting a non-existent JSON secret.
 func TestAzureResolver_GetSecretJSON_NonExistent(t *testing.T) {
-	ctx := context.Background()
-
-	resolver, err := NewAzureResolver(ctx, "https://test-vault.vault.azure.net/")
-	if err != nil {
-		t.Skipf("Skipping test: Azure config not available: %v", err)
-	}
-	defer resolver.Close()
+	resolver, server := newTestAzureResolver(t, notFoundHandler)
+	defer server.Close()
 
-	// Try to get a JSON secret that doesn't exist
-	result, err := resolver.GetSecretJSON(ctx, "cudly-test-nonexistent-json-secret-12345-xyz")
+	result, err := resolver.GetSecretJSON(context.Background(), "cudly-test-nonexistent-json-secret-12345-xyz")
 
-	// Should get an error
 	assert.Error(t, err)
 	assert.Nil(t, result)
 }
 
 // TestAzureResolver_ListSecrets tests listing secrets.
 func TestAzureResolver_ListSecrets(t *testing.T) {
-	ctx := context.Background()
-
-	resolver, err := NewAzureResolver(ctx, "https://test-vault.vault.azure.net/")
-	if err != nil {
-		t.Skipf("Skipping test: Azure config not available: %v", err)
-	}
-	defer resolver.Close()
+	resolver, server := newTestAzureResolver(t, func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_ = json.NewEncoder(w).Encode(map[string]any{"value": []map[string]any{}})
+	})
+	defer server.Close()
 
-	// List without filter
-	result, err := resolver.ListSecrets(ctx, "")
+	result, err := resolver.ListSecrets(context.Background(), "")
 
-	// Either succeeds or fails with permission error
-	if err == nil {
-		assert.NotNil(t, result)
-	} else {
-		assert.Contains(t, err.Error(), "failed to list secrets")
-	}
+	require.NoError(t, err)
+	assert.NotNil(t, result)
 }
 
-// TestAzureResolver_ListSecrets_WithFilter_Coverage_Direct tests listing secrets with a filter using direct resolver.
+// TestAzureResolver_ListSecrets_WithFilter_Coverage_Direct tests that the
+// client-side prefix filter is applied while listing.
 func TestAzureResolver_ListSecrets_WithFilter_Coverage_Direct(t *testing.T) {
-	ctx := context.Background()
-
-	resolver, err := NewAzureResolver(ctx, "https://test-vault.vault.azure.net/")
-	if err != nil {
-		t.Skipf("Skipping test: Azure config not available: %v", err)
-	}
-	defer resolver.Close()
+	resolver, server := newTestAzureResolver(t, func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_ = json.NewEncoder(w).Encode(map[string]any{
+			"value": []map[string]any{
+				{"id": "https://myvault.vault.azure.net/secrets/prod-db-creds"},
+				{"id": "https://myvault.vault.azure.net/secrets/dev-api-key"},
+			},
+		})
+	})
+	defer server.Close()
 
-	// List with filter
-	result, err := resolver.ListSecrets(ctx, "prod")
+	result, err := resolver.ListSecrets(context.Background(), "prod")
 
-	// Either succeeds or fails with permission error
-	if err == nil {
-		// Filter is applied client-side in Azure resolver
-		assert.NotNil(t, result)
-	}
+	require.NoError(t, err)
+	assert.Equal(t, []string{"prod-db-creds"}, result)
 }
 
-// TestAzureResolver_Close_Idempotent tests that Close can be called multiple times.
+// TestAzureResolver_Close_Idempotent tests that Close can be called multiple
+// times. Constructor-only: no requests are made.
 func TestAzureResolver_Close_Idempotent(t *testing.T) {
 	ctx := context.Background()
 
 	resolver, err := NewAzureResolver(ctx, "https://test-vault.vault.azure.net/")
-	if err != nil {
-		t.Skipf("Skipping test: Azure config not available: %v", err)
-	}
+	require.NoError(t, err)
 
 	// Close multiple times should not panic
 	err1 := resolver.Close()
@@ -133,9 +121,7 @@ func TestAzureResolver_DifferentVaultURLs(t *testing.T) {
 	for _, vaultURL := range vaultURLs {
 		t.Run(vaultURL, func(t *testing.T) {
 			resolver, err := NewAzureResolver(ctx, vaultURL)
-			if err != nil {
-				t.Skipf("Skipping test: Azure config not available for vault %s: %v", vaultURL, err)
-			}
+			require.NoError(t, err)
 			defer resolver.Close()
 
 			assert.Equal(t, vaultURL, resolver.vaultURL)
@@ -146,48 +132,35 @@ func TestAzureResolver_DifferentVaultURLs(t *testing.T) {
 
 // TestAzureResolver_ContextHandling tests context handling in Azure resolver.
 func TestAzureResolver_ContextHandling(t *testing.T) {
-	ctx := context.Background()
-
-	resolver, err := NewAzureResolver(ctx, "https://test-vault.vault.azure.net/")
-	if err != nil {
-		t.Skipf("Skipping test: Azure config not available: %v", err)
-	}
-	defer resolver.Close()
+	var handlerCalls atomic.Int32
+	resolver, server := newTestAzureResolver(t, func(w http.ResponseWriter, r *http.Request) {
+		handlerCalls.Add(1)
+		notFoundHandler(w, r)
+	})
+	defer server.Close()
 
-	// Test with canceled context
 	cancelledCtx, cancel := context.WithCancel(context.Background())
 	cancel()
 
-	// GetSecret with canceled context
-	_, err = resolver.GetSecret(cancelledCtx, "test-secret")
-	// Should fail
-	assert.Error(t, err)
+	// GetSecret with canceled context should fail before reaching the server.
+	_, err := resolver.GetSecret(cancelledCtx, "test-secret")
+	require.Error(t, err)
+	assert.Zero(t, handlerCalls.Load(), "canceled request must not reach the HTTP handler")
 }
 
 // TestAzureResolver_EmptySecretID tests getting a secret with empty ID.
 func TestAzureResolver_EmptySecretID(t *testing.T) {
-	ctx := context.Background()
-
-	resolver, err := NewAzureResolver(ctx, "https://test-vault.vault.azure.net/")
-	if err != nil {
-		t.Skipf("Skipping test: Azure config not available: %v", err)
-	}
-	defer resolver.Close()
+	resolver, server := newTestAzureResolver(t, notFoundHandler)
+	defer server.Close()
 
-	// Empty secret ID
-	_, err = resolver.GetSecret(ctx, "")
+	_, err := resolver.GetSecret(context.Background(), "")
 	assert.Error(t, err)
 }
 
 // TestAzureResolver_SpecialCharactersInSecretID tests secret IDs with special characters.
 func TestAzureResolver_SpecialCharactersInSecretID(t *testing.T) {
-	ctx := context.Background()
-
-	resolver, err := NewAzureResolver(ctx, "https://test-vault.vault.azure.net/")
-	if err != nil {
-		t.Skipf("Skipping test: Azure config not available: %v", err)
-	}
-	defer resolver.Close()
+	resolver, server := newTestAzureResolver(t, notFoundHandler)
+	defer server.Close()
 
 	// Azure Key Vault secret names can contain alphanumeric and dashes
 	testIDs := []string{
@@ -198,25 +171,19 @@ func TestAzureResolver_SpecialCharactersInSecretID(t *testing.T) {
 
 	for _, id := range testIDs {
 		t.Run(id, func(t *testing.T) {
-			// These will fail since secrets don't exist
-			_, err := resolver.GetSecret(ctx, id)
+			// These fail because the mock vault returns 404 for every secret.
+			_, err := resolver.GetSecret(context.Background(), id)
 			assert.Error(t, err)
 		})
 	}
 }
 
 // TestAzureResolver_GetSecretJSON_RealMethod tests the GetSecretJSON error propagation.
 func TestAzureResolver_GetSecretJSON_RealMethod(t *testing.T) {
-	ctx := context.Background()
-
-	resolver, err := NewAzureResolver(ctx, "https://test-vault.vault.azure.net/")
-	if err != nil {
-		t.Skipf("Skipping test: Azure config not available: %v", err)
-	}
-	defer resolver.Close()
+	resolver, server := newTestAzureResolver(t, notFoundHandler)
+	defer server.Close()
 
-	// GetSecretJSON should fail because the secret doesn't exist
-	result, err := resolver.GetSecretJSON(ctx, "cudly-test-nonexistent-json-secret-xyz")
+	result, err := resolver.GetSecretJSON(context.Background(), "cudly-test-nonexistent-json-secret-xyz")
 
 	require.Error(t, err)
 	assert.Nil(t, result)

providers/azure/recommendations_test.go

@@ -196,27 +196,73 @@ func TestRecommendationsClientAdapter_Fields(t *testing.T) {
 	assert.Nil(t, adapter.cred)
 }
 
+// overrideServiceClientFns swaps every per-service client constructor for the
+// given fakes and restores the originals on cleanup, so adapter tests never
+// build real ARM clients (which would issue live requests to
+// management.azure.com).
+func overrideServiceClientFns(t *testing.T, compute, database, cache, cosmos, sp serviceRecsGetter) {
+	t.Helper()
+	origCompute := newComputeClientFn
+	origDatabase := newDatabaseClientFn
+	origCache := newCacheClientFn
+	origCosmos := newCosmosDBClientFn
+	origSP := newSavingsPlansClientFn
+	t.Cleanup(func() {
+		newComputeClientFn = origCompute
+		newDatabaseClientFn = origDatabase
+		newCacheClientFn = origCache
+		newCosmosDBClientFn = origCosmos
+		newSavingsPlansClientFn = origSP
+	})
+	newComputeClientFn = newFakeFn(compute)
+	newDatabaseClientFn = newFakeFn(database)
+	newCacheClientFn = newFakeFn(cache)
+	newCosmosDBClientFn = newFakeFn(cosmos)
+	newSavingsPlansClientFn = newFakeFn(sp)
+}
+
 func TestRecommendationsClientAdapter_GetRecommendationsForService(t *testing.T) {
+	computeRec := common.Recommendation{Provider: common.ProviderAzure, Service: common.ServiceCompute}
+	overrideServiceClientFns(t,
+		&fakeServiceClient{recs: []common.Recommendation{computeRec}},
+		&fakeServiceClient{}, &fakeServiceClient{}, &fakeServiceClient{}, &fakeServiceClient{})
+
 	adapter := &RecommendationsClientAdapter{
-		cred:           &mockAzureTokenCredential{},
-		subscriptionID: "test-subscription",
+		cred:             &mockAzureTokenCredential{},
+		subscriptionID:   "test-subscription",
+		getAdvisorRecsFn: noopAdvisorFn,
 	}
 
-	// This will try to make API calls which will fail without real credentials,
-	// but we test the wiring is correct. Error is expected since we don't have
-	// real Azure credentials - the important thing is the function is wired correctly.
-	_, _ = adapter.GetRecommendationsForService(context.Background(), common.ServiceCompute)
+	recs, err := adapter.GetRecommendationsForService(context.Background(), common.ServiceCompute)
+
+	require.NoError(t, err)
+	assert.Equal(t, []common.Recommendation{computeRec}, recs)
 }
 
 func TestRecommendationsClientAdapter_GetAllRecommendations(t *testing.T) {
+	rec := func(svc common.ServiceType) common.Recommendation {
+		return common.Recommendation{Provider: common.ProviderAzure, Service: svc}
+	}
+	overrideServiceClientFns(t,
+		&fakeServiceClient{recs: []common.Recommendation{rec(common.ServiceCompute)}},
+		&fakeServiceClient{recs: []common.Recommendation{rec(common.ServiceRelationalDB)}},
+		&fakeServiceClient{recs: []common.Recommendation{rec(common.ServiceCache)}},
+		&fakeServiceClient{recs: []common.Recommendation{rec(common.ServiceNoSQL)}},
+		&fakeServiceClient{})
+
 	adapter := &RecommendationsClientAdapter{
-		cred:           &mockAzureTokenCredential{},
-		subscriptionID: "test-subscription",
+		cred:             &mockAzureTokenCredential{},
+		subscriptionID:   "test-subscription",
+		getAdvisorRecsFn: noopAdvisorFn,
 	}
 
-	// This will try to make API calls which will fail without real credentials,
-	// but we test the wiring is correct. Error is expected - we just verify the function is callable.
-	_, _ = adapter.GetAllRecommendations(context.Background())
+	recs, err := adapter.GetAllRecommendations(context.Background())
+
+	require.NoError(t, err)
+	assert.ElementsMatch(t, []common.Recommendation{
+		rec(common.ServiceCompute), rec(common.ServiceRelationalDB),
+		rec(common.ServiceCache), rec(common.ServiceNoSQL),
+	}, recs)
 }
 
 // TestGetRecommendations_SavingsPlansServiceIncluded pins that shouldIncludeService