Sitelet https://github.com/kubernetes/kops/commit/ccae9a612d097d1ca5d3d2656a08afbfaacc051a
Skip to content

Commit ccae9a6

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #18320 from hakman/automated-cherry-pick-of-#18260-upstream-release-1.35
Automated cherry pick of #18260: azure: encode storage account in azureblob:// URLs
2 parents d958cfc + a3972af commit ccae9a6

18 files changed

Lines changed: 391 additions & 58 deletions

File tree

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ UPLOAD_CMD=$(KOPS_ROOT)/hack/upload ${UPLOAD_ARGS}
5050
unexport AWS_ACCESS_KEY_ID AWS_REGION AWS_SECRET_ACCESS_KEY AWS_SESSION_TOKEN CNI_VERSION_URL DNS_IGNORE_NS_CHECK DNSCONTROLLER_IMAGE DO_ACCESS_TOKEN GOOGLE_APPLICATION_CREDENTIALS
5151
unexport KOPS_BASE_URL KOPS_CLUSTER_NAME KOPS_RUN_OBSOLETE_VERSION KOPS_STATE_STORE KOPS_STATE_S3_ACL KUBE_API_VERSIONS NODEUP_URL OPENSTACK_CREDENTIAL_FILE SKIP_PACKAGE_UPDATE
5252
unexport SKIP_REGION_CHECK S3_ACCESS_KEY_ID S3_ENDPOINT S3_REGION S3_SECRET_ACCESS_KEY HCLOUD_TOKEN SCW_ACCESS_KEY SCW_SECRET_KEY SCW_DEFAULT_PROJECT_ID SCW_PROFILE
53-
unexport AZURE_CLIENT_ID AZURE_CLIENT_SECRET AZURE_STORAGE_ACCOUNT AZURE_SUBSCRIPTION_ID AZURE_TENANT_ID
53+
unexport AZURE_CLIENT_ID AZURE_CLIENT_SECRET AZURE_SUBSCRIPTION_ID AZURE_TENANT_ID
5454

5555

5656
VERSION=$(shell tools/get_version.sh | grep VERSION | awk '{print $$2}')

‎docs/getting_started/azure.md‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,12 @@ export KOPS_FEATURE_FLAGS="Azure"
2929

3030
```bash
3131
export AZURE_SUBSCRIPTION_ID=<subscription-id>
32-
export AZURE_STORAGE_ACCOUNT=<storage-account-name>
3332
```
3433

3534
### kOps-specific
3635

3736
```bash
38-
export KOPS_STATE_STORE=azureblob://<container-name>
37+
export KOPS_STATE_STORE=azureblob://<storage-account-name>/<container-name>
3938
```
4039

4140
## Creating a Single Master Cluster
@@ -90,3 +89,11 @@ kOps for Azure currently does not support the following features:
9089
## Next steps
9190

9291
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.
92+
93+
## Migrating from earlier alpha versions
94+
95+
Older alpha releases used `azureblob://<container>/...` URLs and read the storage account from `AZURE_STORAGE_ACCOUNT`. To upgrade an existing cluster:
96+
97+
1. `unset AZURE_STORAGE_ACCOUNT` and re-export `KOPS_STATE_STORE` in the new shape.
98+
2. `kops edit cluster` to update `spec.configStore.base` to the updated URL.
99+
3. `kops update cluster --yes` and `kops rolling-update cluster --yes`.

‎hack/update-expected.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ unset AWS_ACCESS_KEY_ID AWS_REGION AWS_SECRET_ACCESS_KEY AWS_SESSION_TOKEN CNI_V
3131
unset KOPS_CLUSTER_NAME KOPS_RUN_OBSOLETE_VERSION KOPS_STATE_STORE KOPS_STATE_S3_ACL KUBE_API_VERSIONS NODEUP_URL OPENSTACK_CREDENTIAL_FILE PROTOKUBE_IMAGE SKIP_PACKAGE_UPDATE
3232
unset SKIP_REGION_CHECK S3_ACCESS_KEY_ID S3_ENDPOINT S3_REGION S3_SECRET_ACCESS_KEY
3333
unset SCW_ACCESS_KEY SCW_SECRET_KEY SCW_DEFAULT_PROJECT_ID SCW_PROFILE
34-
unset AZURE_CLIENT_ID AZURE_CLIENT_SECRET AZURE_STORAGE_ACCOUNT AZURE_SUBSCRIPTION_ID AZURE_TENANT_ID
34+
unset AZURE_CLIENT_ID AZURE_CLIENT_SECRET AZURE_SUBSCRIPTION_ID AZURE_TENANT_ID
3535
unset DIGITALOCEAN_ACCESS_TOKEN
3636

3737
# Run the tests in "autofix mode"

‎nodeup/pkg/bootstrap/install.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,10 +127,6 @@ func (i *Installation) buildEnvFile() *nodetasks.InstallFile {
127127
envVars["OSS_REGION"] = os.Getenv("OSS_REGION")
128128
}
129129

130-
if os.Getenv("AZURE_STORAGE_ACCOUNT") != "" {
131-
envVars["AZURE_STORAGE_ACCOUNT"] = os.Getenv("AZURE_STORAGE_ACCOUNT")
132-
}
133-
134130
if os.Getenv("SCW_PROFILE") != "" || os.Getenv("SCW_SECRET_KEY") != "" {
135131
profile, err := scaleway.CreateValidScalewayProfile()
136132
if err != nil {

‎nodeup/pkg/model/protokube.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -287,10 +287,6 @@ func (t *ProtokubeBuilder) buildEnvFile() (*nodetasks.File, error) {
287287
envVars["OSS_REGION"] = os.Getenv("OSS_REGION")
288288
}
289289

290-
if os.Getenv("AZURE_STORAGE_ACCOUNT") != "" {
291-
envVars["AZURE_STORAGE_ACCOUNT"] = os.Getenv("AZURE_STORAGE_ACCOUNT")
292-
}
293-
294290
if t.CloudProvider() == kops.CloudProviderScaleway {
295291
if os.Getenv("SCW_PROFILE") != "" || os.Getenv("SCW_SECRET_KEY") != "" {
296292
profile, err := scaleway.CreateValidScalewayProfile()

‎pkg/apis/kops/validation/validation.go‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import (
4545
"k8s.io/kops/pkg/model/iam"
4646
"k8s.io/kops/upup/pkg/fi"
4747
"k8s.io/kops/upup/pkg/fi/utils"
48+
"k8s.io/kops/util/pkg/vfs"
4849
)
4950

5051
func newValidateCluster(cluster *kops.Cluster, strict bool) field.ErrorList {
@@ -199,6 +200,8 @@ func validateClusterSpec(spec *kops.ClusterSpec, c *kops.Cluster, fieldPath *fie
199200
}
200201
}
201202

203+
allErrs = append(allErrs, validateAzureBlobAccountUniformity(spec, fieldPath)...)
204+
202205
if spec.ContainerRuntime != "" {
203206
allErrs = append(allErrs, validateContainerRuntime(c, spec.ContainerRuntime, fieldPath.Child("containerRuntime"))...)
204207
}
@@ -1441,6 +1444,82 @@ func validateEtcdBackupStore(specs []kops.EtcdClusterSpec, fieldPath *field.Path
14411444
return allErrs
14421445
}
14431446

1447+
// azureBlobAccount returns the storage account encoded in an azureblob:// URL,
1448+
// or "" with no error if the URL is not azureblob://. Returns an error only if
1449+
// the URL has the azureblob:// prefix but fails to parse.
1450+
func azureBlobAccount(rawURL string) (string, error) {
1451+
if !strings.HasPrefix(rawURL, "azureblob://") {
1452+
return "", nil
1453+
}
1454+
p, err := vfs.Context.BuildVfsPath(rawURL)
1455+
if err != nil {
1456+
return "", err
1457+
}
1458+
azPath, ok := p.(*vfs.AzureBlobPath)
1459+
if !ok {
1460+
return "", fmt.Errorf("expected azureblob:// URL, got %q", rawURL)
1461+
}
1462+
return azPath.Account(), nil
1463+
}
1464+
1465+
// validateAzureBlobAccountUniformity enforces that every azureblob:// URL in
1466+
// the cluster spec uses the same storage account as configStore.base. Any
1467+
// azureblob:// URL elsewhere in the spec is rejected when configStore.base is
1468+
// not itself azureblob://.
1469+
func validateAzureBlobAccountUniformity(spec *kops.ClusterSpec, fieldPath *field.Path) field.ErrorList {
1470+
var allErrs field.ErrorList
1471+
csPath := fieldPath.Child("configStore")
1472+
1473+
canonical := ""
1474+
if strings.HasPrefix(spec.ConfigStore.Base, "azureblob://") {
1475+
account, err := azureBlobAccount(spec.ConfigStore.Base)
1476+
if err != nil {
1477+
allErrs = append(allErrs, field.Invalid(csPath.Child("base"), spec.ConfigStore.Base, err.Error()))
1478+
return allErrs
1479+
}
1480+
canonical = account
1481+
}
1482+
1483+
type entry struct {
1484+
path *field.Path
1485+
url string
1486+
}
1487+
others := []entry{
1488+
{csPath.Child("keypairs"), spec.ConfigStore.Keypairs},
1489+
{csPath.Child("secrets"), spec.ConfigStore.Secrets},
1490+
}
1491+
for i, ec := range spec.EtcdClusters {
1492+
if ec.Backups != nil {
1493+
others = append(others, entry{
1494+
fieldPath.Child("etcdClusters").Index(i).Child("backups", "backupStore"),
1495+
ec.Backups.BackupStore,
1496+
})
1497+
}
1498+
}
1499+
1500+
for _, e := range others {
1501+
if !strings.HasPrefix(e.url, "azureblob://") {
1502+
continue
1503+
}
1504+
account, err := azureBlobAccount(e.url)
1505+
if err != nil {
1506+
allErrs = append(allErrs, field.Invalid(e.path, e.url, err.Error()))
1507+
continue
1508+
}
1509+
if canonical == "" {
1510+
allErrs = append(allErrs, field.Invalid(e.path, e.url,
1511+
"azureblob:// URL requires configStore.base to also be azureblob://"))
1512+
continue
1513+
}
1514+
if account != canonical {
1515+
allErrs = append(allErrs, field.Invalid(e.path, e.url,
1516+
fmt.Sprintf("storage account %q does not match configStore.base account %q", account, canonical)))
1517+
}
1518+
}
1519+
1520+
return allErrs
1521+
}
1522+
14441523
// validateEtcdStorage is responsible for checking versions are identical.
14451524
func validateEtcdStorage(specs []kops.EtcdClusterSpec, fieldPath *field.Path) field.ErrorList {
14461525
allErrs := field.ErrorList{}

‎pkg/model/components/etcdmanager/model.go‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,46 @@ import (
4646
"k8s.io/kops/upup/pkg/fi/fitasks"
4747
"k8s.io/kops/util/pkg/env"
4848
"k8s.io/kops/util/pkg/exec"
49+
"k8s.io/kops/util/pkg/vfs"
4950
)
5051

52+
// resolveAzureBackupStore rewrites azureblob://<account>/<container>/<key> into
53+
// the legacy azureblob://<container>/<key> shape understood by the pinned
54+
// etcd-manager image, returning the storage account derived from
55+
// configStoreBase (the single source of truth for the cluster) for
56+
// AZURE_STORAGE_ACCOUNT injection. Non-azureblob backup stores pass through
57+
// unchanged. Errors if a backup store is azureblob:// but configStoreBase is
58+
// not, since validation already enforces account uniformity.
59+
//
60+
// TODO: remove once etcd-manager is bumped to a release whose vendored VFS
61+
// understands azureblob://<account>/<container>/<key>.
62+
func resolveAzureBackupStore(configStoreBase, backupStore string) (legacyURL string, storageAccount string, err error) {
63+
if !strings.HasPrefix(backupStore, "azureblob://") {
64+
return backupStore, "", nil
65+
}
66+
bp, err := vfs.Context.BuildVfsPath(backupStore)
67+
if err != nil {
68+
return "", "", fmt.Errorf("parsing etcd backup-store %q: %w", backupStore, err)
69+
}
70+
bpAzure, ok := bp.(*vfs.AzureBlobPath)
71+
if !ok {
72+
return "", "", fmt.Errorf("expected azureblob:// backup-store, got %q", backupStore)
73+
}
74+
csp, err := vfs.Context.BuildVfsPath(configStoreBase)
75+
if err != nil {
76+
return "", "", fmt.Errorf("parsing configStore.base %q: %w", configStoreBase, err)
77+
}
78+
csAzure, ok := csp.(*vfs.AzureBlobPath)
79+
if !ok {
80+
return "", "", fmt.Errorf("backup-store %q is azureblob:// but configStore.base %q is not", backupStore, configStoreBase)
81+
}
82+
legacy := "azureblob://" + bpAzure.Container()
83+
if bpAzure.Key() != "" {
84+
legacy += "/" + bpAzure.Key()
85+
}
86+
return legacy, csAzure.Account(), nil
87+
}
88+
5189
// EtcdManagerBuilder builds the manifest for the etcd-manager
5290
type EtcdManagerBuilder struct {
5391
*model.KopsModelContext
@@ -437,6 +475,14 @@ func (b *EtcdManagerBuilder) buildPod(etcdCluster kops.EtcdClusterSpec, instance
437475
DNSSuffix: dnsInternalSuffix,
438476
}
439477

478+
// Rewrite to the legacy URL shape for the pinned etcd-manager image; see
479+
// resolveAzureBackupStore.
480+
legacyBackupStore, azureStorageAccount, err := resolveAzureBackupStore(b.Cluster.Spec.ConfigStore.Base, backupStore)
481+
if err != nil {
482+
return nil, err
483+
}
484+
config.BackupStore = legacyBackupStore
485+
440486
config.LogLevel = 6
441487

442488
if etcdCluster.Manager != nil && etcdCluster.Manager.LogLevel != nil {
@@ -619,6 +665,14 @@ func (b *EtcdManagerBuilder) buildPod(etcdCluster kops.EtcdClusterSpec, instance
619665

620666
container.Env = envMap.ToEnvVars()
621667

668+
// Required by the pinned etcd-manager's legacy VFS; see resolveAzureBackupStore.
669+
if azureStorageAccount != "" {
670+
container.Env = append(container.Env, v1.EnvVar{
671+
Name: "AZURE_STORAGE_ACCOUNT",
672+
Value: azureStorageAccount,
673+
})
674+
}
675+
622676
if etcdCluster.Manager != nil {
623677
if etcdCluster.Manager.BackupRetentionDays != nil {
624678
envVar := v1.EnvVar{

‎pkg/model/components/etcdmanager/model_test.go‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,3 +88,83 @@ func LoadKopsModelContext(basedir string) (*model.KopsModelContext, error) {
8888

8989
return kopsContext, nil
9090
}
91+
92+
func Test_resolveAzureBackupStore(t *testing.T) {
93+
tests := []struct {
94+
name string
95+
configStoreBase string
96+
backupStore string
97+
wantURL string
98+
wantAccount string
99+
wantErr bool
100+
}{
101+
{
102+
name: "non-azure backup store passes through",
103+
configStoreBase: "memfs://tests/cluster",
104+
backupStore: "memfs://tests/cluster/backups/etcd/main",
105+
wantURL: "memfs://tests/cluster/backups/etcd/main",
106+
wantAccount: "",
107+
},
108+
{
109+
name: "non-azure backup store with azure config base passes through",
110+
configStoreBase: "azureblob://kopsstate/state/cluster",
111+
backupStore: "s3://my-bucket/cluster/backups/etcd/main",
112+
wantURL: "s3://my-bucket/cluster/backups/etcd/main",
113+
wantAccount: "",
114+
},
115+
{
116+
name: "azureblob with multi-segment key",
117+
configStoreBase: "azureblob://kopsstate/state/cluster",
118+
backupStore: "azureblob://kopsstate/state/cluster.example.com/backups/etcd/main",
119+
wantURL: "azureblob://state/cluster.example.com/backups/etcd/main",
120+
wantAccount: "kopsstate",
121+
},
122+
{
123+
name: "azureblob with empty key",
124+
configStoreBase: "azureblob://kopsstate/state",
125+
backupStore: "azureblob://kopsstate/state",
126+
wantURL: "azureblob://state",
127+
wantAccount: "kopsstate",
128+
},
129+
{
130+
name: "account taken from configStore.base, not backup store",
131+
configStoreBase: "azureblob://canonicalacct/state/cluster",
132+
backupStore: "azureblob://canonicalacct/backups/etcd/main",
133+
wantURL: "azureblob://backups/etcd/main",
134+
wantAccount: "canonicalacct",
135+
},
136+
{
137+
name: "azureblob backup store with non-azure configStore.base is rejected",
138+
configStoreBase: "s3://my-bucket/state",
139+
backupStore: "azureblob://kopsstate/state/cluster/backups/etcd/main",
140+
wantErr: true,
141+
},
142+
{
143+
name: "azureblob missing container is rejected",
144+
configStoreBase: "azureblob://kopsstate/state",
145+
backupStore: "azureblob://kopsstate",
146+
wantErr: true,
147+
},
148+
}
149+
150+
for _, tc := range tests {
151+
t.Run(tc.name, func(t *testing.T) {
152+
gotURL, gotAccount, err := resolveAzureBackupStore(tc.configStoreBase, tc.backupStore)
153+
if tc.wantErr {
154+
if err == nil {
155+
t.Fatalf("expected error, got URL=%q account=%q", gotURL, gotAccount)
156+
}
157+
return
158+
}
159+
if err != nil {
160+
t.Fatalf("unexpected error: %v", err)
161+
}
162+
if gotURL != tc.wantURL {
163+
t.Errorf("URL: got %q, want %q", gotURL, tc.wantURL)
164+
}
165+
if gotAccount != tc.wantAccount {
166+
t.Errorf("Account: got %q, want %q", gotAccount, tc.wantAccount)
167+
}
168+
})
169+
}
170+
}

‎pkg/model/resources/nodeup.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -433,10 +433,9 @@ func buildEnvironmentVariables(cluster *kops.Cluster, ig *kops.InstanceGroup) (m
433433
}
434434

435435
if cluster.GetCloudProvider() == kops.CloudProviderAzure {
436-
env["AZURE_STORAGE_ACCOUNT"] = os.Getenv("AZURE_STORAGE_ACCOUNT")
437436
azureEnv := os.Getenv("AZURE_ENVIRONMENT")
438437
if azureEnv != "" {
439-
env["AZURE_ENVIRONMENT"] = os.Getenv("AZURE_ENVIRONMENT")
438+
env["AZURE_ENVIRONMENT"] = azureEnv
440439
}
441440
}
442441

‎tests/e2e/kubetest2-kops/deployer/common.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ func (d *deployer) env() []string {
250250
vars = append(vars, fmt.Sprintf("KUBE_SSH_KEY_PATH=%v", d.SSHPrivateKeyPath))
251251
case "azure":
252252
// Pass through some env vars if set
253-
for _, k := range []string{"AZURE_TENANT_ID", "AZURE_SUBSCRIPTION_ID", "AZURE_CLIENT_ID", "AZURE_FEDERATED_TOKEN_FILE", "AZURE_STORAGE_ACCOUNT"} {
253+
for _, k := range []string{"AZURE_TENANT_ID", "AZURE_SUBSCRIPTION_ID", "AZURE_CLIENT_ID", "AZURE_FEDERATED_TOKEN_FILE"} {
254254
v := os.Getenv(k)
255255
if v != "" {
256256
vars = append(vars, k+"="+v)
@@ -409,7 +409,7 @@ func (d *deployer) stateStore() string {
409409
ss = "s3://" + bucketName
410410
case "azure":
411411
// TODO: Use dynamic container name
412-
ss = "azureblob://cluster-state"
412+
ss = "azureblob://stkopsstatestore/cluster-state"
413413
case "gce":
414414
d.createBucket = true
415415
ss = "gs://" + gce.GCSBucketName(d.GCPProject, "state")

0 commit comments

Comments
 (0)