azure: encode storage account in azureblob:// URLs

Move the Azure storage account name from the AZURE_STORAGE_ACCOUNT env
var into the URL so each azureblob:// path is self-contained. New
format: azureblob://{account}/{container}/{key}.

The env var is no longer read by kops; the legacy URL form is rejected
with a migration error pointing users at the new shape.

Signed-off-by: Ciprian Hacman <ciprian@hakman.dev>

Author: hakman

cmd/kops/create_cluster_integration_test.go

@@ -51,7 +51,6 @@ func TestCreateClusterGossipAWS(t *testing.T) {
 
 // TestCreateClusterGossipAzure creates a minimal Azure gossip cluster
 func TestCreateClusterGossipAzure(t *testing.T) {
-	t.Setenv("AZURE_STORAGE_ACCOUNT", "teststorage")
 	runCreateClusterIntegrationTest(t, "../../tests/integration/create_cluster/gossip-azure", "v1alpha2")
 }
 

cmd/kops/integration_test.go

@@ -1792,7 +1792,6 @@ func (i *integrationTest) runTestTerraformGCE(t *testing.T) {
 }
 
 func (i *integrationTest) runTestTerraformAzure(t *testing.T) {
-	t.Setenv("AZURE_STORAGE_ACCOUNT", "teststorage")
 	t.Setenv("KOPS_RUN_TOO_NEW_VERSION", "1")
 
 	featureflag.ParseFlags("+Azure,+AzureTerraform")

docs/getting_started/azure.md

@@ -30,13 +30,12 @@ export KOPS_FEATURE_FLAGS="Azure"
 
 ```bash
 export AZURE_SUBSCRIPTION_ID=<subscription-id>
-export AZURE_STORAGE_ACCOUNT=<storage-account-name>
 ```
 
 ### kOps-specific
 
 ```bash
-export KOPS_STATE_STORE=azureblob://<container-name>
+export KOPS_STATE_STORE=azureblob://<storage-account-name>/<container-name>
 ```
 
 ## Creating a Single Master Cluster
@@ -90,3 +89,11 @@ kOps for Azure currently does not support the following features:
 ## Next steps
 
 Now that you have a working kOps cluster, read through the recommendations for [production setups guide](production.md) to learn more about how to configure kOps for production workloads.
+
+## Migrating from earlier alpha versions
+
+Older alpha releases used `azureblob://<container>/...` URLs and read the storage account from `AZURE_STORAGE_ACCOUNT`. To upgrade an existing cluster:
+
+1. `unset AZURE_STORAGE_ACCOUNT` and re-export `KOPS_STATE_STORE` in the new shape.
+2. `kops edit cluster` to update `spec.configStore.base` to the updated URL.
+3. `kops update cluster --yes` and `kops rolling-update cluster --yes`.

nodeup/pkg/bootstrap/install.go

@@ -127,10 +127,6 @@ func (i *Installation) buildEnvFile() *nodetasks.InstallFile {
 		envVars["OSS_REGION"] = os.Getenv("OSS_REGION")
 	}
 
-	if os.Getenv("AZURE_STORAGE_ACCOUNT") != "" {
-		envVars["AZURE_STORAGE_ACCOUNT"] = os.Getenv("AZURE_STORAGE_ACCOUNT")
-	}
-
 	if os.Getenv("SCW_PROFILE") != "" || os.Getenv("SCW_SECRET_KEY") != "" {
 		profile, err := scaleway.CreateValidScalewayProfile()
 		if err != nil {

nodeup/pkg/model/protokube.go

@@ -287,10 +287,6 @@ func (t *ProtokubeBuilder) buildEnvFile() (*nodetasks.File, error) {
 		envVars["OSS_REGION"] = os.Getenv("OSS_REGION")
 	}
 
-	if os.Getenv("AZURE_STORAGE_ACCOUNT") != "" {
-		envVars["AZURE_STORAGE_ACCOUNT"] = os.Getenv("AZURE_STORAGE_ACCOUNT")
-	}
-
 	if t.CloudProvider() == kops.CloudProviderScaleway {
 		if os.Getenv("SCW_PROFILE") != "" || os.Getenv("SCW_SECRET_KEY") != "" {
 			profile, err := scaleway.CreateValidScalewayProfile()

pkg/apis/kops/validation/validation.go

@@ -45,6 +45,7 @@ import (
 	"k8s.io/kops/pkg/model/iam"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/utils"
+	"k8s.io/kops/util/pkg/vfs"
 )
 
 func newValidateCluster(cluster *kops.Cluster, strict bool) field.ErrorList {
@@ -199,6 +200,8 @@ func validateClusterSpec(spec *kops.ClusterSpec, c *kops.Cluster, fieldPath *fie
 		}
 	}
 
+	allErrs = append(allErrs, validateAzureBlobAccountUniformity(spec, fieldPath)...)
+
 	if spec.ContainerRuntime != "" {
 		allErrs = append(allErrs, validateContainerRuntime(c, spec.ContainerRuntime, fieldPath.Child("containerRuntime"))...)
 	}
@@ -1521,6 +1524,82 @@ func validateEtcdBackupStore(specs []kops.EtcdClusterSpec, fieldPath *field.Path
 	return allErrs
 }
 
+// azureBlobAccount returns the storage account encoded in an azureblob:// URL,
+// or "" with no error if the URL is not azureblob://. Returns an error only if
+// the URL has the azureblob:// prefix but fails to parse.
+func azureBlobAccount(rawURL string) (string, error) {
+	if !strings.HasPrefix(rawURL, "azureblob://") {
+		return "", nil
+	}
+	p, err := vfs.Context.BuildVfsPath(rawURL)
+	if err != nil {
+		return "", err
+	}
+	azPath, ok := p.(*vfs.AzureBlobPath)
+	if !ok {
+		return "", fmt.Errorf("expected azureblob:// URL, got %q", rawURL)
+	}
+	return azPath.Account(), nil
+}
+
+// validateAzureBlobAccountUniformity enforces that every azureblob:// URL in
+// the cluster spec uses the same storage account as configStore.base. Any
+// azureblob:// URL elsewhere in the spec is rejected when configStore.base is
+// not itself azureblob://.
+func validateAzureBlobAccountUniformity(spec *kops.ClusterSpec, fieldPath *field.Path) field.ErrorList {
+	var allErrs field.ErrorList
+	csPath := fieldPath.Child("configStore")
+
+	canonical := ""
+	if strings.HasPrefix(spec.ConfigStore.Base, "azureblob://") {
+		account, err := azureBlobAccount(spec.ConfigStore.Base)
+		if err != nil {
+			allErrs = append(allErrs, field.Invalid(csPath.Child("base"), spec.ConfigStore.Base, err.Error()))
+			return allErrs
+		}
+		canonical = account
+	}
+
+	type entry struct {
+		path *field.Path
+		url  string
+	}
+	others := []entry{
+		{csPath.Child("keypairs"), spec.ConfigStore.Keypairs},
+		{csPath.Child("secrets"), spec.ConfigStore.Secrets},
+	}
+	for i, ec := range spec.EtcdClusters {
+		if ec.Backups != nil {
+			others = append(others, entry{
+				fieldPath.Child("etcdClusters").Index(i).Child("backups", "backupStore"),
+				ec.Backups.BackupStore,
+			})
+		}
+	}
+
+	for _, e := range others {
+		if !strings.HasPrefix(e.url, "azureblob://") {
+			continue
+		}
+		account, err := azureBlobAccount(e.url)
+		if err != nil {
+			allErrs = append(allErrs, field.Invalid(e.path, e.url, err.Error()))
+			continue
+		}
+		if canonical == "" {
+			allErrs = append(allErrs, field.Invalid(e.path, e.url,
+				"azureblob:// URL requires configStore.base to also be azureblob://"))
+			continue
+		}
+		if account != canonical {
+			allErrs = append(allErrs, field.Invalid(e.path, e.url,
+				fmt.Sprintf("storage account %q does not match configStore.base account %q", account, canonical)))
+		}
+	}
+
+	return allErrs
+}
+
 // validateEtcdStorage is responsible for checking versions are identical.
 func validateEtcdStorage(specs []kops.EtcdClusterSpec, fieldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}

pkg/apis/kops/validation/validation_test.go

@@ -1931,3 +1931,160 @@ func TestValidateNetworkingLinode(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateAzureBlobAccountUniformity(t *testing.T) {
+	t.Setenv("AZURE_STORAGE_ACCOUNT", "")
+
+	tests := []struct {
+		name     string
+		spec     kops.ClusterSpec
+		expected []*field.Error
+	}{
+		{
+			name: "all matching azureblob URLs",
+			spec: kops.ClusterSpec{
+				ConfigStore: kops.ConfigStoreSpec{
+					Base:     "azureblob://kopsstate/state/cluster.example.com",
+					Keypairs: "azureblob://kopsstate/state/cluster.example.com/pki",
+					Secrets:  "azureblob://kopsstate/state/cluster.example.com/secrets",
+				},
+				EtcdClusters: []kops.EtcdClusterSpec{{
+					Backups: &kops.EtcdBackupSpec{
+						BackupStore: "azureblob://kopsstate/state/cluster.example.com/backups/etcd/main",
+					},
+				}},
+			},
+		},
+		{
+			name: "non-azure cluster is unaffected",
+			spec: kops.ClusterSpec{
+				ConfigStore: kops.ConfigStoreSpec{
+					Base:     "s3://my-bucket/cluster.example.com",
+					Keypairs: "s3://my-bucket/cluster.example.com/pki",
+				},
+				EtcdClusters: []kops.EtcdClusterSpec{{
+					Backups: &kops.EtcdBackupSpec{
+						BackupStore: "s3://my-bucket/cluster.example.com/backups/etcd/main",
+					},
+				}},
+			},
+		},
+		{
+			name: "keypairs uses different storage account",
+			spec: kops.ClusterSpec{
+				ConfigStore: kops.ConfigStoreSpec{
+					Base:     "azureblob://kopsstate/state/cluster.example.com",
+					Keypairs: "azureblob://otheracct/state/cluster.example.com/pki",
+				},
+			},
+			expected: []*field.Error{
+				{
+					Type:  field.ErrorTypeInvalid,
+					Field: "spec.configStore.keypairs",
+				},
+			},
+		},
+		{
+			name: "secrets uses different storage account",
+			spec: kops.ClusterSpec{
+				ConfigStore: kops.ConfigStoreSpec{
+					Base:    "azureblob://kopsstate/state/cluster.example.com",
+					Secrets: "azureblob://otheracct/state/cluster.example.com/secrets",
+				},
+			},
+			expected: []*field.Error{
+				{
+					Type:  field.ErrorTypeInvalid,
+					Field: "spec.configStore.secrets",
+				},
+			},
+		},
+		{
+			name: "etcd backupStore uses different storage account",
+			spec: kops.ClusterSpec{
+				ConfigStore: kops.ConfigStoreSpec{
+					Base: "azureblob://kopsstate/state/cluster.example.com",
+				},
+				EtcdClusters: []kops.EtcdClusterSpec{{
+					Backups: &kops.EtcdBackupSpec{
+						BackupStore: "azureblob://otheracct/backups/etcd/main",
+					},
+				}},
+			},
+			expected: []*field.Error{
+				{
+					Type:  field.ErrorTypeInvalid,
+					Field: "spec.etcdClusters[0].backups.backupStore",
+				},
+			},
+		},
+		{
+			name: "azureblob backupStore with non-azure configStore.base is rejected",
+			spec: kops.ClusterSpec{
+				ConfigStore: kops.ConfigStoreSpec{
+					Base: "s3://my-bucket/cluster.example.com",
+				},
+				EtcdClusters: []kops.EtcdClusterSpec{{
+					Backups: &kops.EtcdBackupSpec{
+						BackupStore: "azureblob://kopsstate/backups/etcd/main",
+					},
+				}},
+			},
+			expected: []*field.Error{
+				{
+					Type:  field.ErrorTypeInvalid,
+					Field: "spec.etcdClusters[0].backups.backupStore",
+				},
+			},
+		},
+		{
+			name: "malformed azureblob configStore.base is rejected",
+			spec: kops.ClusterSpec{
+				ConfigStore: kops.ConfigStoreSpec{
+					Base: "azureblob://kopsstate",
+				},
+			},
+			expected: []*field.Error{
+				{
+					Type:  field.ErrorTypeInvalid,
+					Field: "spec.configStore.base",
+				},
+			},
+		},
+		{
+			name: "malformed azureblob keypairs is rejected",
+			spec: kops.ClusterSpec{
+				ConfigStore: kops.ConfigStoreSpec{
+					Base:     "azureblob://kopsstate/state/cluster.example.com",
+					Keypairs: "azureblob://kopsstate",
+				},
+			},
+			expected: []*field.Error{
+				{
+					Type:  field.ErrorTypeInvalid,
+					Field: "spec.configStore.keypairs",
+				},
+			},
+		},
+		{
+			name: "non-azure backup store with azure config base is allowed",
+			spec: kops.ClusterSpec{
+				ConfigStore: kops.ConfigStoreSpec{
+					Base: "azureblob://kopsstate/state/cluster.example.com",
+				},
+				EtcdClusters: []kops.EtcdClusterSpec{{
+					Backups: &kops.EtcdBackupSpec{
+						BackupStore: "memfs://tests/cluster.example.com/backups/etcd/main",
+					},
+				}},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			errList := validateAzureBlobAccountUniformity(&tt.spec, field.NewPath("spec"))
+			testFieldErrors(t, errList, tt.expected)
+		})
+	}
+}