PR feedback

api/v1alpha1/storageaccount_types.go

@@ -39,7 +39,8 @@ type StorageAccountSpec struct {
 // StorageAccountSku the SKU of the storage account.
 type StorageAccountSku struct {
 	// Name - The SKU name. Required for account creation; optional for update.
-	// Possible values include: 'Standard_LRS', 'Standard_GRS', 'Standard_RAGRS', 'Standard_ZRS', 'Premium_LRS', 'Premium_ZRS', 'Standard_GZRS', 'Standard_RAGZRS'
+	// Possible values include: 'Standard_LRS', 'Standard_GRS', 'Standard_RAGRS', 'Standard_ZRS', 'Premium_LRS', 'Premium_ZRS', 'Standard_GZRS', 'Standard_RAGZRS'.
+	// For the full list of allowed options, see: https://docs.microsoft.com/en-us/rest/api/storagerp/storageaccounts/create#skuname
 	Name StorageAccountSkuName `json:"name,omitempty"`
 }
 

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

@@ -23,4 +23,4 @@ data:
   {{- end }}
   {{- if .Values.azureSecretNamingVersion }}
   AZURE_SECRET_NAMING_VERSION: {{ .Values.azureSecretNamingVersion | b64enc | quote }}
-  {{- end }}
+  {{- end }}

controllers/azuresqlaction_controller_test.go

@@ -30,12 +30,7 @@ func RunSQLActionHappy(t *testing.T, server string) {
 	//Get SQL credentials to compare after rollover
 	secret := &v1.Secret{}
 	assert.Eventually(func() bool {
-		var secretName string
-		if tc.secretClient.GetSecretNamingVersion() == secrets.SecretNamingV1 {
-			secretName = server
-		} else {
-			secretName = "azuresqlserver-" + server
-		}
+		secretName := getSecretName(server)
 
 		err := tc.k8sClient.Get(ctx, types.NamespacedName{Name: secretName, Namespace: "default"}, secret)
 		if err != nil {
@@ -82,3 +77,13 @@ func RunSQLActionHappy(t *testing.T, server string) {
 
 	EnsureDelete(ctx, t, tc, sqlActionInstance)
 }
+
+func getSecretName(server string) string {
+	var secretName string
+	if tc.secretClient.GetSecretNamingVersion() == secrets.SecretNamingV1 {
+		secretName = server
+	} else {
+		secretName = "azuresqlserver-" + server
+	}
+	return secretName
+}

controllers/suite_test.go

@@ -90,10 +90,8 @@ func setup() error {
 	// the purpose of these tests
 	envy.Set("POD_NAMESPACE", "azureoperator-system")
 
-	// Force the secret naming version to 2 for these tests
-	// Note: This can be removed to run the tests in the old v1 naming scheme (they
-	// should still pass)
-	envy.Set("AZURE_SECRET_NAMING_VERSION", "2")
+	// Uncomment the below to run the tests in the old v1 naming mode
+	// envy.Set("AZURE_SECRET_NAMING_VERSION", "1")
 
 	err := resourcemanagerconfig.ParseEnvironment()
 	if err != nil {

docs/howto/secrets.md

@@ -35,7 +35,7 @@ Some things to note about the Key Vault you use with the operator:
 ## Secret naming
 
 There are two versions of secret naming used by the Azure Service Operator. The secret naming version is controlled by the `AZURE_SECRET_NAMING_VERSION` field of the `azureoperatorsettings` secret.
-Valid values are `"1"` and `"2"`. Version `2` is the default when installing the operator via Helm. If no value is specified for `AZURE_SECRET_NAMING_VERSION` the default is version `1` for backwards compatibility.
+Valid values are `"1"` and `"2"`. Version `2` is the default.
 
 **We strongly recommend that you use version `2` as it is more consistent in how secrets are named and does a better job of avoiding naming conflicts.**
 

pkg/resourcemanager/config/env.go

@@ -72,9 +72,9 @@ func ParseEnvironment() error {
 	}
 
 	// If this isn't set, default to the oldest support version
-	if secretNamingVersionInt == 0 || secretNamingVersionInt == 1 {
+	if secretNamingVersionInt == 1 {
 		secretNamingVersion = secrets.SecretNamingV1
-	} else if secretNamingVersionInt == 2 {
+	} else if secretNamingVersionInt == 0 || secretNamingVersionInt == 2 {
 		secretNamingVersion = secrets.SecretNamingV2
 	} else {
 		return errors.Errorf("secret naming version must be between 0 and 2 but was %d", secretNamingVersionInt)