Add new options to support KeyVault soft delete (#1717)

* Update KeyVault SecretClient to recover soft delete

   * Includes a test ensuring that this works
   * Add Azure SQL Combined test to ensure create+delete+recreate works
   * Update CI to not fail on stderr

azure-pipelines.yml

@@ -305,7 +305,6 @@ jobs:
           # Set tags to not available for the selected cluster so it doesn't get used in another run
           az resource tag --tags 'freeforpipeline=false' -g $(AKS_CLUSTER_RG) -n $clustername --resource-type Microsoft.ContainerService/managedClusters
         workingDirectory: '$(System.DefaultWorkingDirectory)'
-        failOnStandardError: true
       displayName: Deploy to AKS - Find available AKS cluster and connect to it
       condition: or(eq(variables['check_changes.SOURCE_CODE_CHANGED'], 'true'), eq(variables['Build.SourceBranch'], 'refs/heads/master'))
 
@@ -384,7 +383,6 @@ jobs:
         # Turn off this check until our aad-pod-identity dep is updated
         # so that it's not trying to install v1beta1
         # ClusterRoleBindings.
-        failOnStandardError: false
 
 
     - task: Docker@2

charts/azure-service-operator/templates/secret.yaml

@@ -25,4 +25,10 @@ data:
   {{- if .Values.azureSecretNamingVersion }}
   AZURE_SECRET_NAMING_VERSION: {{ .Values.azureSecretNamingVersion | b64enc | quote }}
   {{- end }}
+  {{- if .Values.purgeDeletedKeyVaultSecrets }}
+  PURGE_DELETED_KEYVAULT_SECRETS: {{ .Values.purgeDeletedKeyVaultSecrets | b64enc | quote }}
+  {{- end }}
+  {{- if .Values.recoverSoftDeletedKeyVaultSecrets }}
+  RECOVER_SOFT_DELETED_KEYVAULT_SECRETS: {{ .Values.recoverSoftDeletedKeyVaultSecrets | b64enc | quote }}
+  {{- end }}
 {{- end }}

charts/azure-service-operator/values.yaml

@@ -25,6 +25,18 @@ azureUseMI: False
 # azureSecretNamingVersion allows choosing the algorithm used to derive secret names. Version 2 is recommended.
 azureSecretNamingVersion: "2"
 
+# purgeDeletedKeyVaultSecrets determines if the operator should issue a secret Purge request in addition
+# to Delete when deleting secrets in Azure Key Vault. This only applies to secrets that are stored in Azure Key Vault.
+# It does nothing if the secret is stored in Kubernetes.
+purgeDeletedKeyVaultSecrets: False
+
+# recoverSoftDeletedKeyVaultSecrets determines if the operator should issue a secret Recover request when it
+# encounters an "ObjectIsDeletedButRecoverable" error from Azure Key Vault during secret creation. This error
+# can occur when a Key Vault has soft delete enabled and an ASO resource was deleted and recreated with the same name.
+# This only applies to secrets that are stored in Azure Key Vault.
+# It does nothing if the secret is stored in Kubernetes.
+recoverSoftDeletedKeyVaultSecrets: True
+
 # image defines the container image the ASO pod should run
 # Note: This should use the latest released tag number explicitly. If
 # it's ':latest' and someone deploys the chart after a new version has

config/default/manager_image_patch.yaml

@@ -61,6 +61,18 @@ spec:
                 name: azureoperatorsettings
                 key: AZURE_SECRET_NAMING_VERSION
                 optional: true
+          - name: PURGE_DELETED_KEYVAULT_SECRETS
+            valueFrom:
+              secretKeyRef:
+                name: azureoperatorsettings
+                key: PURGE_DELETED_KEYVAULT_SECRETS
+                optional: true
+          - name: RECOVER_SOFT_DELETED_KEYVAULT_SECRETS
+            valueFrom:
+              secretKeyRef:
+                name: azureoperatorsettings
+                key: RECOVER_SOFT_DELETED_KEYVAULT_SECRETS
+                optional: true
           - name: AZURE_TARGET_NAMESPACES
             valueFrom:
               secretKeyRef:

controllers/async_controller.go

@@ -78,7 +78,12 @@ func (r *AsyncReconciler) Reconcile(ctx context.Context, req ctrl.Request, obj c
 	keyVaultName := keyvaultsecretlib.GetKeyVaultName(obj)
 	if len(keyVaultName) != 0 {
 		// Instantiate the KeyVault Secret Client
-		keyvaultSecretClient = keyvaultsecretlib.New(keyVaultName, config.GlobalCredentials(), config.SecretNamingVersion())
+		keyvaultSecretClient = keyvaultsecretlib.New(
+			keyVaultName,
+			config.GlobalCredentials(),
+			config.SecretNamingVersion(),
+			config.PurgeDeletedKeyVaultSecrets(),
+			config.RecoverSoftDeletedKeyVaultSecrets())
 	}
 
 	// Check to see if the skipreconcile annotation is on

controllers/azuresql_combined_test.go

@@ -11,7 +11,7 @@ import (
 	"strings"
 	"testing"
 
-	sql "github.com/Azure/azure-sdk-for-go/services/preview/sql/mgmt/v3.0/sql"
+	"github.com/Azure/azure-sdk-for-go/services/preview/sql/mgmt/v3.0/sql"
 	"github.com/stretchr/testify/require"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -21,7 +21,9 @@ import (
 	"github.com/Azure/azure-service-operator/pkg/errhelp"
 	"github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlshared"
 	"github.com/Azure/azure-service-operator/pkg/resourcemanager/config"
+	resourcemanagerkeyvaults "github.com/Azure/azure-service-operator/pkg/resourcemanager/keyvaults"
 	"github.com/Azure/azure-service-operator/pkg/secrets"
+	testcommon "github.com/Azure/azure-service-operator/test/common"
 )
 
 func TestAzureSqlServerCombinedHappyPath(t *testing.T) {
@@ -338,3 +340,47 @@ func TestAzureSqlServerCombinedHappyPath(t *testing.T) {
 	})
 
 }
+
+func TestAzureSqlServer_KeyVaultSoftDelete_CreateDeleteCreateAgain(t *testing.T) {
+	t.Parallel()
+	defer PanicRecover(t)
+	ctx := context.Background()
+	require := require.New(t)
+
+	rgLocation := "westus2"
+
+	// Create a KeyVault with soft delete enabled that we can use to perform our tests
+	keyVaultName := GenerateAlphaNumTestResourceNameWithRandom("kvsoftdel", 5)
+	objID, err := resourcemanagerkeyvaults.GetObjectID(
+		context.Background(),
+		config.GlobalCredentials(),
+		config.GlobalCredentials().TenantID(),
+		config.GlobalCredentials().ClientID())
+	require.NoError(err)
+
+	err = testcommon.CreateKeyVaultSoftDeleteEnabled(
+		context.Background(),
+		config.GlobalCredentials(),
+		tc.resourceGroupName,
+		keyVaultName,
+		rgLocation,
+		objID)
+	require.NoError(err)
+
+	sqlServerName := GenerateTestResourceNameWithRandom("sqlserver", 10)
+	sqlServerNamespacedName := types.NamespacedName{Name: sqlServerName, Namespace: "default"}
+	sqlServerInstance := v1beta1.NewAzureSQLServer(sqlServerNamespacedName, tc.resourceGroupName, rgLocation)
+	sqlServerInstance.Spec.KeyVaultToStoreSecrets = keyVaultName
+
+	// create and wait
+	RequireInstance(ctx, t, tc, sqlServerInstance)
+
+	EnsureDelete(ctx, t, tc, sqlServerInstance)
+
+	// Recreate with the same name
+	sqlServerInstance = v1beta1.NewAzureSQLServer(sqlServerNamespacedName, tc.resourceGroupName, rgLocation)
+	sqlServerInstance.Spec.KeyVaultToStoreSecrets = keyVaultName
+	RequireInstance(ctx, t, tc, sqlServerInstance)
+
+	EnsureDelete(ctx, t, tc, sqlServerInstance)
+}

controllers/eventhub_storageaccount_controller_test.go

@@ -10,6 +10,7 @@ import (
 	"testing"
 
 	s "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-04-01/storage"
+
 	azurev1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1"
 	"github.com/Azure/azure-service-operator/api/v1alpha2"
 	"github.com/Azure/azure-service-operator/pkg/secrets"
@@ -173,7 +174,12 @@ func TestEventHubControllerCreateAndDeleteCustomKeyVault(t *testing.T) {
 	EnsureInstance(ctx, t, tc, eventhubInstance)
 
 	// Check that the secret is added to KeyVault
-	keyvaultSecretClient := kvsecrets.New(keyVaultNameForSecrets, config.GlobalCredentials(), config.SecretNamingVersion())
+	keyvaultSecretClient := kvsecrets.New(
+		keyVaultNameForSecrets,
+		config.GlobalCredentials(),
+		config.SecretNamingVersion(),
+		config.PurgeDeletedKeyVaultSecrets(),
+		config.RecoverSoftDeletedKeyVaultSecrets())
 	key := secrets.SecretKey{Name: eventhubInstance.Name, Namespace: eventhubInstance.Namespace, Kind: "EventHub"}
 
 	EnsureSecrets(ctx, t, tc, eventhubInstance, keyvaultSecretClient, key)

controllers/helpers.go

@@ -13,11 +13,7 @@ import (
 	"testing"
 	"time"
 
-	"github.com/Azure/azure-sdk-for-go/services/keyvault/mgmt/2018-02-14/keyvault"
-	"github.com/Azure/go-autorest/autorest/to"
 	"github.com/go-logr/logr"
-	"github.com/gofrs/uuid"
-	"github.com/pkg/errors"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/crypto/ssh"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -35,7 +31,6 @@ import (
 	resourcemanagersqlfirewallrule "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlfirewallrule"
 	resourcemanagersqlserver "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlserver"
 	resourcemanagersqluser "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqluser"
-	"github.com/Azure/azure-service-operator/pkg/resourcemanager/config"
 	resourcemanagerconfig "github.com/Azure/azure-service-operator/pkg/resourcemanager/config"
 	resourcemanagereventhub "github.com/Azure/azure-service-operator/pkg/resourcemanager/eventhubs"
 	resourcemanagerkeyvaults "github.com/Azure/azure-service-operator/pkg/resourcemanager/keyvaults"
@@ -442,61 +437,3 @@ func GenerateRandomSshPublicKeyString() string {
 	sshPublicKeyData := string(ssh.MarshalAuthorizedKey(publicRsaKey))
 	return sshPublicKeyData
 }
-
-//CreateVaultWithAccessPolicies creates a new key vault and provides access policies to the specified user - used in test
-func CreateVaultWithAccessPolicies(ctx context.Context, creds config.Credentials, groupName string, vaultName string, location string, clientID string) error {
-	vaultsClient, err := resourcemanagerkeyvaults.GetKeyVaultClient(creds)
-	if err != nil {
-		return errors.Wrapf(err, "couldn't get vaults client")
-	}
-	id, err := uuid.FromString(creds.TenantID())
-	if err != nil {
-		return errors.Wrapf(err, "couldn't convert tenantID to UUID")
-	}
-
-	apList := []keyvault.AccessPolicyEntry{}
-	ap := keyvault.AccessPolicyEntry{
-		TenantID: &id,
-		Permissions: &keyvault.Permissions{
-			Keys: &[]keyvault.KeyPermissions{
-				keyvault.KeyPermissionsCreate,
-			},
-			Secrets: &[]keyvault.SecretPermissions{
-				keyvault.SecretPermissionsSet,
-				keyvault.SecretPermissionsGet,
-				keyvault.SecretPermissionsDelete,
-				keyvault.SecretPermissionsList,
-			},
-		},
-	}
-	if clientID != "" {
-		objID, err := resourcemanagerkeyvaults.GetObjectID(ctx, creds, creds.TenantID(), clientID)
-		if err != nil {
-			return err
-		}
-		if objID != nil {
-			ap.ObjectID = objID
-			apList = append(apList, ap)
-		}
-
-	}
-
-	params := keyvault.VaultCreateOrUpdateParameters{
-		Properties: &keyvault.VaultProperties{
-			TenantID:       &id,
-			AccessPolicies: &apList,
-			Sku: &keyvault.Sku{
-				Family: to.StringPtr("A"),
-				Name:   keyvault.Standard,
-			},
-		},
-		Location: to.StringPtr(location),
-	}
-
-	future, err := vaultsClient.CreateOrUpdate(ctx, groupName, vaultName, params)
-	if err != nil {
-		return err
-	}
-
-	return future.WaitForCompletionRef(ctx, vaultsClient.Client)
-}

controllers/keyvault_controller_test.go

@@ -12,7 +12,7 @@ import (
 	"testing"
 	"time"
 
-	uuid "github.com/gofrs/uuid"
+	"github.com/gofrs/uuid"
 
 	azurev1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1"
 	"github.com/Azure/azure-service-operator/pkg/errhelp"
@@ -126,7 +126,12 @@ func TestKeyvaultControllerWithAccessPolicies(t *testing.T) {
 
 	//Add code to set secret and get secret from this keyvault using secretclient
 
-	keyvaultSecretClient := kvsecrets.New(keyVaultName, config.GlobalCredentials(), config.SecretNamingVersion())
+	keyvaultSecretClient := kvsecrets.New(
+		keyVaultName,
+		config.GlobalCredentials(),
+		config.SecretNamingVersion(),
+		config.PurgeDeletedKeyVaultSecrets(),
+		config.RecoverSoftDeletedKeyVaultSecrets())
 	secretName := "test-key"
 	key := secrets.SecretKey{Name: secretName, Namespace: "default", Kind: "test"}
 	datanew := map[string][]byte{
@@ -186,7 +191,12 @@ func TestKeyvaultControllerWithLimitedAccessPoliciesAndUpdate(t *testing.T) {
 	}, tc.timeout, tc.retry, "wait for keyVaultInstance to be ready in azure")
 	//Add code to set secret and get secret from this keyvault using secretclient
 
-	keyvaultSecretClient := kvsecrets.New(keyVaultName, config.GlobalCredentials(), config.SecretNamingVersion())
+	keyvaultSecretClient := kvsecrets.New(
+		keyVaultName,
+		config.GlobalCredentials(),
+		config.SecretNamingVersion(),
+		config.PurgeDeletedKeyVaultSecrets(),
+		config.RecoverSoftDeletedKeyVaultSecrets())
 	key := secrets.SecretKey{Name: "test-key", Namespace: "default", Kind: "test"}
 	datanew := map[string][]byte{
 		"test1": []byte("test2"),
@@ -333,7 +343,12 @@ func TestKeyvaultControllerWithVirtualNetworkRulesAndUpdate(t *testing.T) {
 		return result.Response.StatusCode == http.StatusOK
 	}, tc.timeout, tc.retry, "wait for keyVaultInstance to be ready in azure")
 
-	keyvaultSecretClient := kvsecrets.New(keyVaultName, config.GlobalCredentials(), config.SecretNamingVersion())
+	keyvaultSecretClient := kvsecrets.New(
+		keyVaultName,
+		config.GlobalCredentials(),
+		config.SecretNamingVersion(),
+		config.PurgeDeletedKeyVaultSecrets(),
+		config.RecoverSoftDeletedKeyVaultSecrets())
 	secretName := "test-key"
 	key := secrets.SecretKey{Name: secretName, Namespace: "default", Kind: "test"}
 	datanew := map[string][]byte{

controllers/secret_naming_version_test.go

@@ -155,7 +155,12 @@ func assertSQLServerAdminSecretCreated(ctx context.Context, t *testing.T, sqlSer
 		}, tc.timeoutFast, tc.retry, "wait for server to have secret")
 	} else {
 		// Check that the user's secret is in the keyvault
-		keyVaultSecretClient := kvsecrets.New(sqlServerInstance.Spec.KeyVaultToStoreSecrets, config.GlobalCredentials(), config.SecretNamingVersion())
+		keyVaultSecretClient := kvsecrets.New(
+			sqlServerInstance.Spec.KeyVaultToStoreSecrets,
+			config.GlobalCredentials(),
+			config.SecretNamingVersion(),
+			config.PurgeDeletedKeyVaultSecrets(),
+			config.RecoverSoftDeletedKeyVaultSecrets())
 
 		assert.Eventually(t, func() bool {
 			expectedSecretName := makeSQLServerKeyVaultSecretName(sqlServerInstance)
@@ -294,7 +299,12 @@ func TestAzureSqlServerAndUser_SecretNamedCorrectly(t *testing.T) {
 			EnsureInstance(ctx, t, tc, kvSqlUser1)
 
 			// Check that the user's secret is in the keyvault
-			keyVaultSecretClient := kvsecrets.New(tc.keyvaultName, config.GlobalCredentials(), config.SecretNamingVersion())
+			keyVaultSecretClient := kvsecrets.New(
+				tc.keyvaultName,
+				config.GlobalCredentials(),
+				config.SecretNamingVersion(),
+				config.PurgeDeletedKeyVaultSecrets(),
+				config.RecoverSoftDeletedKeyVaultSecrets())
 
 			assert.Eventually(func() bool {
 				key := makeSQLUserSecretKey(keyVaultSecretClient, kvSqlUser1)
@@ -333,7 +343,12 @@ func TestAzureSqlServerAndUser_SecretNamedCorrectly(t *testing.T) {
 			EnsureInstance(ctx, t, tc, kvSqlUser2)
 
 			// Check that the user's secret is in the keyvault
-			keyVaultSecretClient := kvsecrets.New(tc.keyvaultName, config.GlobalCredentials(), config.SecretNamingVersion())
+			keyVaultSecretClient := kvsecrets.New(
+				tc.keyvaultName,
+				config.GlobalCredentials(),
+				config.SecretNamingVersion(),
+				config.PurgeDeletedKeyVaultSecrets(),
+				config.RecoverSoftDeletedKeyVaultSecrets())
 
 			assert.Eventually(func() bool {
 				key := makeSQLUserSecretKey(keyVaultSecretClient, kvSqlUser2)
@@ -350,7 +365,12 @@ func TestAzureSqlServerAndUser_SecretNamedCorrectly(t *testing.T) {
 
 	t.Run("deploy sql action and roll user credentials", func(t *testing.T) {
 		keyVaultName := tc.keyvaultName
-		keyVaultSecretClient := kvsecrets.New(keyVaultName, config.GlobalCredentials(), config.SecretNamingVersion())
+		keyVaultSecretClient := kvsecrets.New(
+			keyVaultName,
+			config.GlobalCredentials(),
+			config.SecretNamingVersion(),
+			config.PurgeDeletedKeyVaultSecrets(),
+			config.RecoverSoftDeletedKeyVaultSecrets())
 
 		key := makeSQLUserSecretKey(keyVaultSecretClient, kvSqlUser1)
 		oldSecret, err := keyVaultSecretClient.Get(ctx, key)
@@ -395,7 +415,12 @@ func TestAzureSqlServerAndUser_SecretNamedCorrectly(t *testing.T) {
 		EnsureDelete(ctx, t, tc, kvSqlUser2)
 
 		// Check that the user's secret is in the keyvault
-		keyVaultSecretClient := kvsecrets.New(tc.keyvaultName, config.GlobalCredentials(), config.SecretNamingVersion())
+		keyVaultSecretClient := kvsecrets.New(
+			tc.keyvaultName,
+			config.GlobalCredentials(),
+			config.SecretNamingVersion(),
+			config.PurgeDeletedKeyVaultSecrets(),
+			config.RecoverSoftDeletedKeyVaultSecrets())
 
 		assert.Eventually(func() bool {
 			key := secrets.SecretKey{Name: sqlUser.ObjectMeta.Name, Namespace: sqlUser.ObjectMeta.Namespace, Kind: "azuresqluser"}
@@ -519,7 +544,12 @@ func TestAzureSqlServerKVSecretAndUser_SecretNamedCorrectly(t *testing.T) {
 			EnsureInstance(ctx, t, tc, kvSqlUser1)
 
 			// Check that the user's secret is in the keyvault
-			keyVaultSecretClient := kvsecrets.New(tc.keyvaultName, config.GlobalCredentials(), config.SecretNamingVersion())
+			keyVaultSecretClient := kvsecrets.New(
+				tc.keyvaultName,
+				config.GlobalCredentials(),
+				config.SecretNamingVersion(),
+				config.PurgeDeletedKeyVaultSecrets(),
+				config.RecoverSoftDeletedKeyVaultSecrets())
 
 			assert.Eventually(func() bool {
 				key := makeSQLUserSecretKey(keyVaultSecretClient, kvSqlUser1)