Add support for container-level immutability to Azure Blob Storage Containers #1098

renormalize · 2025-02-24T09:05:38Z

How to categorize this PR?

/area backup security
/kind enhancement
/platform azure

What this PR does / why we need it:
Mirror of gardener/gardener-extension-provider-gcp#906

Which issue(s) this PR fixes:
Part of gardener/gardener#10866

Tests performed:

Before	Before's Lock state	After	After's Lock state	Test	Output	Expectation
No immutability	Unlocked	23h immutability	Unlocked	Tests < 24h policy create rejection	Rejected	Rejected
No immutability	Unlocked	24h immutability	Unlocked	Tests policy creation	Accepted	Accepted
No immutability	Unlocked	25h immutability	Unlocked	Tests non-multiple of 24h policy create rejection	Rejected	Rejected
24h immutability	Unlocked	23h immutability	Unlocked	Tests < 24h policy update rejection	Rejected	Rejected
24h immutability	Unlocked	25h immutability	Unlocked	Tests non-multiple of 24h policy update rejection	Rejected	Rejected
24h immutability	Unlocked	48h immutability	Unlocked	Tests policy update while unlocked	Accepted	Accepted
48h immutability	Unlocked	24h immutability	Unlocked	Tests policy reduction while unlocked	Accepted	Accepted
48h immutability	Unlocked	No immutability	Unlocked	Set `immutability` to `null` Tests policy deletion while unlocked	Accepted	Accepted
48h immutability	Unlocked	No immutability	Unlocked	Remove the `immutability` section Tests policy deletion while unlocked	Accepted	Accepted
48h immutability	Unlocked	No immutability	Unlocked	Remove the `providerConfig` section Tests policy deletion while unlocked	Accepted	Accepted
24h immutability	Unlocked	24h immutability	Locked	Tests locking the policy	Accepted	Accepted
24h immutability	Locked	23h immutability	Locked	Tests reduction to an invalid value after locking	Rejected	Rejected
24h immutability	Locked	25h immutability	Locked	Tests extension to an invalid value after locking	Rejected	Rejected
24h immutability	Locked	24h immutability	Unlocked	Tests removal of lock on the policy	Rejected	Rejected
24h immutability	Locked	No immutability	-	Tests deletion of the immutability policy on a locked policy	Rejected	Rejected
24h immutability	Locked	48h immutability	Locked	Tests policy extension while locked	Accepted	Accepted
48h immutability	Locked	24h immutability	Locked	Tests reduction to a valid value after locking	Rejected	Rejected

Special notes for your reviewer:
This PR introduces a new client BlobContainersClient which is a client for ABS containers.
The reason this new client is introduced is because the storage client that was previously used did not expose methods for making the necessary API calls to Azure to modify buckets to become container-level immutable.
These changes also make the container client more uniform with the rest of the codebase.

Release note:

Azure Blob Storage Containers can now be configured to be created with container-level immutability settings.

renormalize · 2025-02-25T13:23:46Z

Rebasing since there seems to be merge conflicts with master after #1101.

* The immutability configuration required to make Azure Blob Storage containers immutable gets deserialized into the `BackupBucketConfig` `struct`. This `struct` now contains the additional field `ImmutableConfig`. * Generated code, mocks, and the api-reference.

* The new `BlobContainers` interface (with `BlobContainersClient` struct) is introduced, which extends the factory pattern used for all other resources in the repository. * The new `BlobContainersClient` uses the `github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/storage/armstorage` package, instead of `github.com/Azure/azure-sdk-for-go/sdk/storage/azblob` which is used by `BlobStorageClient`. * `BlobContainersClient` is necessary since `BlobStorageClient` does not expose APIs which can be used to change ABS Container level immutability policies. * Generate the mock client for `BlobContainers`.

…ation. * The reconciler now uses the newly introduced `BlobContainers` client for creation and deletion of the ABS containers. * The reconciler now introduces logic to handle immutability configuration passed to the `BackupBucket` resource.

…he first reconciliation

…-setup testing.

…rateStorageAccountName`.

… type * `RetentionType` holds the immutability type that is to be used with the bucket. * Ran `make generate`

…on`.

* Add the webhook which validates the backup configuration of the seed. * Add corresponding unit tests.

pkg/admission/validator/seed.go

pkg/controller/worker/machines.go

pkg/controller/backupbucket/actuator.go

…ilityPolicy`. * Rename `updateBackupBucketIfNeeded` to `ensureBackupBucketImmutabilityPolicy`. * Move all bucket immutability handling into `ensureBackupBucketImmutabilityPolicy`. * Enhance `CreateOrUpdateImmutabilityPolicy` to return the etag of the immutability policy that is created, which will then be used for locking the policy if configured. An unnecessary cal to `GetImmutabilityPolicy` is now avoided.

* Move all `BackupBucket` validation code to `pkg/apis/validation`. * Add unit tests for `ValidateBackupBucketConfigUpdate`

renormalize · 2025-02-27T11:21:15Z

It was noticed that control plane migrations of shoots scheduled on seeds with immutable buckets would fail since the BackupEntrys are deleted forcefully during migrations; see this and this. A fix was planned and the corresponding PR was raised: gardener/gardener#11388.

However, after more deliberation and research, it was concluded that the proposed changes in the gardener/gardener PR are not the optimal way to solve this problem, as hinted at by the review comments in the gardener/gardener PR.

The workaround solution that I propose is to use data lifecycle management policies provided by Azure.
The gist of this solution is that we set a lifecycle policy on the bucket, which deletes objects which meet certain conditions.
The policy will act on objects which are tagged with certain tags, which are added to the objects during deletion of the BackupEntry.

This solution is a delayed-delete, which is performed by the infrastructure provider completely by itself.

With this approach, we satisfy the immutability properties of objects in the bucket, and avoid making changes to gardener/gardener to make it work with buckets with certain configurations.

renormalize · 2025-02-27T11:23:41Z

This solution was fully agreed upon by the etcd-druid maintainers yesterday, therefore I was not able to include these changes in the initial commits with which I raised this PR.

However, the changes will be quite simple. I estimate that I will be able to push them sometime tomorrow.

ishan16696

few nits:

ishan16696 · 2025-03-03T04:46:00Z

docs/usage/usage.md

@@ -701,3 +701,48 @@ Some key facts about VMSS Flex based clusters:
 - In case the configuration of the VMSS will change (e.g. amount of fault domains in a region change; configured in the CloudProfile) all machines of the worker pool need to be rolled
 - It is not possible to migrate an existing primary AvailabilitySet based Shoot cluster to VMSS Flex based Shoot cluster and vice versa
 - VMSS Flex based clusters are using `Standard` SKU LoadBalancers instead of `Basic` SKU LoadBalancers for AvailabilitySet based Shoot clusters
+
+## `BackupBucketConfig`


Suggested change

## `BackupBucketConfig`

## BackupBucketConfig

ishan16696 · 2025-03-03T05:11:55Z

docs/usage/usage.md

+
+`BackupBucketConfig` describes the configuration that needs to be passed over for creation of the backup bucket infrastructure. Configuration like immutability (WORM, i.e. write-once-read-many) that can be set on the bucket are specified here. Objects in the bucket will inherit the immutability duration which is set on the bucket, and they can not be modified or deleted for that duration.
+
+This extension supports creating (and migrating already existing buckets) to use [container-level WORM policies](https://learn.microsoft.com/en-us/azure/storage/blobs/immutable-container-level-worm-policies).


Suggested change

This extension supports creating (and migrating already existing buckets) to use [container-level WORM policies](https://learn.microsoft.com/en-us/azure/storage/blobs/immutable-container-level-worm-policies).

This extension supports creating (and migrating already existing buckets if enabled) to use [container-level WORM policies](https://learn.microsoft.com/en-us/azure/storage/blobs/immutable-container-level-worm-policies).

ishan16696 · 2025-03-03T05:13:17Z

docs/usage/usage.md

+```yaml
+apiVersion: azure.provider.extensions.gardener.cloud/v1alpha1
+kind: BackupBucketConfig
+immutability:
+  retentionType: "bucket"
+  retentionPeriod: 24h
+  locked: false
+```


Suggested change

```yaml

apiVersion: azure.provider.extensions.gardener.cloud/v1alpha1

kind: BackupBucketConfig

immutability:

retentionType: "bucket"

retentionPeriod: 24h

locked: false

```

apiVersion: azure.provider.extensions.gardener.cloud/v1alpha1

kind: BackupBucketConfig

immutability:

retentionType: "bucket"

retentionPeriod: 24h

lockedPolicy: false

Can we use lockedPolicy instead of just locked as to me lockedPolicy name makes more sense ?

cc @seshachalam-yv @kon-angelo

I don't have a strong opinion here. For a boolean value locked is okay.
It will diverge from the GCP spec but let's decide on the correct name and it can be retrofitted if needed.

ishan16696 · 2025-03-03T05:23:56Z

docs/usage/usage.md

+```yaml
+apiVersion: extensions.gardener.cloud/v1alpha1
+kind: BackupBucket
+metadata:
+  name: my-backup-bucket
+spec:
+  provider:
+    region: westeurope
+  secretRef:
+    name: my-azure-secret
+    namespace: my-namespace
+  providerConfig:
+    apiVersion: azure.provider.extensions.gardener.cloud/v1alpha1
+    kind: BackupBucketConfig
+    immutability:
+      retentionType: "bucket"
+      retentionPeriod: 24h
+      locked: true
+```


I checked the BackupBucket resource on dev landscape, I didn't find .spec.provider.region. I find this .spec.region, can you please check this again and update this doc accordingly ?

apiVersion: extensions.gardener.cloud/v1alpha1 kind: BackupBucket metadata: ... spec: region: westeurope secretRef: name: .. namespace: garden type: azure

ishan16696 · 2025-03-03T05:26:32Z

pkg/admission/validator/seed_test.go

+		DescribeTable("Invalid update scenarios",
+			func(oldSeed, newSeed *core.Seed, expectedError string) {
+				err := seedValidator.Validate(context.Background(), newSeed, oldSeed)
+				fmt.Printf("%#v\n", err)


may be you forgot to remove this ?

Will remove in a future commit where newer changes will be introduced. Thanks for noticing.

renormalize requested review from a team as code owners February 24, 2025 09:05

gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 24, 2025

gardener-robot added the size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) label Feb 24, 2025

gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 24, 2025

renormalize force-pushed the immutable-backups branch from 4a2552a to 8f67e1f Compare February 24, 2025 09:48

gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 24, 2025

renormalize added 13 commits February 25, 2025 19:02

Removed a tautology.

ca986b3

Fix a bug where storageAccountName ends up being empty in all but t…

6cb8450

…he first reconciliation

Simply updateBackupBucketIfNeeded, and iron out bugs from extension…

64742b6

…-setup testing.

Rename the function which generates the storage account name to `Gene…

7541107

…rateStorageAccountName`.

Generate mocks for StorageAccount.

b53654e

Add unit tests for the backupbucket controller.

ff3513e

Introduce the new type RetentionType which is to be used as an enum…

9aa0395

… type * `RetentionType` holds the immutability type that is to be used with the bucket. * Ran `make generate`

Introduce BackupBucket validation logic in `pkg/apis/azure/validati…

e2b820e

…on`.

Introduce the seed validating webhook in pkg/admission/validator.

8271587

* Add the webhook which validates the backup configuration of the seed. * Add corresponding unit tests.

Add/update comments for exported functions.

181a96b

renormalize added 2 commits February 25, 2025 19:04

Add license headers to new files.

f175afb

Add documentation.

c176986

renormalize force-pushed the immutable-backups branch from 8f67e1f to c176986 Compare February 25, 2025 13:41

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 25, 2025

kon-angelo requested changes Feb 26, 2025

View reviewed changes

gardener-robot added the needs/changes Needs (more) changes label Feb 26, 2025

renormalize added 5 commits February 27, 2025 13:29

Generate mocks for the modified CreateOrUpdateImmutabilityPolicy.

4bf88f8

Adapt unit tests to the modified CreateOrUpdateImmutabilityPolicy.

ae1cf7b

Enhance BlobContainersClient doc comments.

1afcd34

Move all BackupBucket validation code to pkg/apis/validation.

ff6ad6e

* Move all `BackupBucket` validation code to `pkg/apis/validation`. * Add unit tests for `ValidateBackupBucketConfigUpdate`

gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 27, 2025

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 27, 2025

renormalize requested a review from kon-angelo February 27, 2025 10:45

renormalize mentioned this pull request Feb 27, 2025

Adapt shoot flow to not get stuck on seeds with BackupBuckets configured to have immutability settings (write-once-read-many) gardener/gardener#11388

Open

ishan16696 suggested changes Mar 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for container-level immutability to Azure Blob Storage Containers #1098

Add support for container-level immutability to Azure Blob Storage Containers #1098

renormalize commented Feb 24, 2025 •

edited

Loading

renormalize commented Feb 25, 2025 •

edited

Loading

renormalize commented Feb 27, 2025 •

edited

Loading

renormalize commented Feb 27, 2025

ishan16696 left a comment

ishan16696 Mar 3, 2025

ishan16696 Mar 3, 2025

ishan16696 Mar 3, 2025 •

edited

Loading

kon-angelo Mar 3, 2025

ishan16696 Mar 3, 2025 •

edited

Loading

ishan16696 Mar 3, 2025

renormalize Mar 3, 2025


		`BackupBucketConfig` describes the configuration that needs to be passed over for creation of the backup bucket infrastructure. Configuration like immutability (WORM, i.e. write-once-read-many) that can be set on the bucket are specified here. Objects in the bucket will inherit the immutability duration which is set on the bucket, and they can not be modified or deleted for that duration.

		This extension supports creating (and migrating already existing buckets) to use [container-level WORM policies](https://learn.microsoft.com/en-us/azure/storage/blobs/immutable-container-level-worm-policies).

Add support for container-level immutability to Azure Blob Storage Containers #1098

Are you sure you want to change the base?

Add support for container-level immutability to Azure Blob Storage Containers #1098

Conversation

renormalize commented Feb 24, 2025 • edited Loading

renormalize commented Feb 25, 2025 • edited Loading

renormalize commented Feb 27, 2025 • edited Loading

renormalize commented Feb 27, 2025

ishan16696 left a comment

Choose a reason for hiding this comment

ishan16696 Mar 3, 2025

Choose a reason for hiding this comment

ishan16696 Mar 3, 2025

Choose a reason for hiding this comment

ishan16696 Mar 3, 2025 • edited Loading

Choose a reason for hiding this comment

kon-angelo Mar 3, 2025

Choose a reason for hiding this comment

ishan16696 Mar 3, 2025 • edited Loading

Choose a reason for hiding this comment

ishan16696 Mar 3, 2025

Choose a reason for hiding this comment

renormalize Mar 3, 2025

Choose a reason for hiding this comment

renormalize commented Feb 24, 2025 •

edited

Loading

renormalize commented Feb 25, 2025 •

edited

Loading

renormalize commented Feb 27, 2025 •

edited

Loading

ishan16696 Mar 3, 2025 •

edited

Loading

ishan16696 Mar 3, 2025 •

edited

Loading