From 4ecb23a21933860336d20459f877387463285479 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vykintas=20Baltru=C5=A1aitis?= Date: Wed, 28 Aug 2024 18:14:21 +0300 Subject: [PATCH] Fix allowed_operations stability issue --- CHANGELOG.md | 3 ++ b2/resource_b2_bucket.go | 3 +- b2/resource_b2_bucket_test.go | 15 +++++++-- python-bindings/GNUmakefile | 2 +- python-bindings/b2_terraform/provider_tool.py | 33 ++++++++++++++++--- 5 files changed, 46 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a317365..0c567ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +* Fixed `allowed_operations` stability issue + ### Infrastructure * Replace removed macOS 11 Big Sur in favour of macOS 12 Monterey in CI/CD diff --git a/b2/resource_b2_bucket.go b/b2/resource_b2_bucket.go index e8cb1a7..74406ad 100644 --- a/b2/resource_b2_bucket.go +++ b/b2/resource_b2_bucket.go @@ -155,7 +155,8 @@ func resourceB2BucketRead(ctx context.Context, d *schema.ResourceData, meta inte const op = RESOURCE_READ input := map[string]interface{}{ - "bucket_id": d.Id(), + "bucket_id": d.Id(), + "cors_rules": d.Get("cors_rules"), } output, err := client.apply(name, op, input) diff --git a/b2/resource_b2_bucket_test.go b/b2/resource_b2_bucket_test.go index 84e8635..89e5fb1 100644 --- a/b2/resource_b2_bucket_test.go +++ b/b2/resource_b2_bucket_test.go @@ -79,9 +79,12 @@ func TestAccResourceB2Bucket_all(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "cors_rules.0.cors_rule_name", "downloadFromAnyOrigin"), resource.TestCheckResourceAttr(resourceName, "cors_rules.0.allowed_origins.#", "1"), resource.TestCheckResourceAttr(resourceName, "cors_rules.0.allowed_origins.0", "https"), - resource.TestCheckResourceAttr(resourceName, "cors_rules.0.allowed_operations.#", "2"), + resource.TestCheckResourceAttr(resourceName, "cors_rules.0.allowed_operations.#", "5"), resource.TestCheckResourceAttr(resourceName, "cors_rules.0.allowed_operations.0", "b2_download_file_by_id"), resource.TestCheckResourceAttr(resourceName, "cors_rules.0.allowed_operations.1", "b2_download_file_by_name"), + resource.TestCheckResourceAttr(resourceName, "cors_rules.0.allowed_operations.2", "s3_put"), + resource.TestCheckResourceAttr(resourceName, "cors_rules.0.allowed_operations.3", "s3_head"), + resource.TestCheckResourceAttr(resourceName, "cors_rules.0.allowed_operations.4", "s3_get"), resource.TestCheckResourceAttr(resourceName, "cors_rules.0.expose_headers.#", "1"), resource.TestCheckResourceAttr(resourceName, "cors_rules.0.expose_headers.0", "x-bz-content-sha1"), resource.TestCheckResourceAttr(resourceName, "cors_rules.0.allowed_headers.#", "1"), @@ -170,9 +173,12 @@ func TestAccResourceB2Bucket_update(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "cors_rules.0.cors_rule_name", "downloadFromAnyOrigin"), resource.TestCheckResourceAttr(resourceName, "cors_rules.0.allowed_origins.#", "1"), resource.TestCheckResourceAttr(resourceName, "cors_rules.0.allowed_origins.0", "https"), - resource.TestCheckResourceAttr(resourceName, "cors_rules.0.allowed_operations.#", "2"), + resource.TestCheckResourceAttr(resourceName, "cors_rules.0.allowed_operations.#", "5"), resource.TestCheckResourceAttr(resourceName, "cors_rules.0.allowed_operations.0", "b2_download_file_by_id"), resource.TestCheckResourceAttr(resourceName, "cors_rules.0.allowed_operations.1", "b2_download_file_by_name"), + resource.TestCheckResourceAttr(resourceName, "cors_rules.0.allowed_operations.2", "s3_put"), + resource.TestCheckResourceAttr(resourceName, "cors_rules.0.allowed_operations.3", "s3_head"), + resource.TestCheckResourceAttr(resourceName, "cors_rules.0.allowed_operations.4", "s3_get"), resource.TestCheckResourceAttr(resourceName, "cors_rules.0.expose_headers.#", "1"), resource.TestCheckResourceAttr(resourceName, "cors_rules.0.expose_headers.0", "x-bz-content-sha1"), resource.TestCheckResourceAttr(resourceName, "cors_rules.0.allowed_headers.#", "1"), @@ -304,7 +310,10 @@ resource "b2_bucket" "test" { ] allowed_operations = [ "b2_download_file_by_id", - "b2_download_file_by_name" + "b2_download_file_by_name", + "s3_put", + "s3_head", + "s3_get", ] expose_headers = ["x-bz-content-sha1"] allowed_headers = ["range"] diff --git a/python-bindings/GNUmakefile b/python-bindings/GNUmakefile index 0d486be..66219df 100644 --- a/python-bindings/GNUmakefile +++ b/python-bindings/GNUmakefile @@ -16,7 +16,7 @@ format: lint: @black --check -Sl 100 ${DIR} - @flake8 --ignore=E501 ${DIR} + @flake8 --ignore=E501,W503 ${DIR} @python ../scripts/check-headers.py '**/*.py' testacc: build diff --git a/python-bindings/b2_terraform/provider_tool.py b/python-bindings/b2_terraform/provider_tool.py index 4bf5dbd..16f5f20 100644 --- a/python-bindings/b2_terraform/provider_tool.py +++ b/python-bindings/b2_terraform/provider_tool.py @@ -202,8 +202,9 @@ class Bucket(Command): tf_keys = BUCKET_KEYS def data_source_read(self, *, bucket_name, **kwargs): + config_cors_rules = kwargs.get('cors_rules') bucket = self.api.get_bucket_by_name(bucket_name) - return self._postprocess(bucket) + return self._postprocess(bucket, config_cors_rules=config_cors_rules) def resource_create( self, @@ -244,14 +245,14 @@ def resource_create( self.api.delete_bucket(bucket) raise - return self._postprocess(bucket) + return self._postprocess(bucket, config_cors_rules=cors_rules) def resource_read(self, *, bucket_id, **kwargs): try: bucket = self.api.get_bucket_by_id(bucket_id) except BucketIdNotFound: return None # no bucket has been found - return self._postprocess(bucket) + return self._postprocess(bucket, config_cors_rules=kwargs.get('cors_rules')) def resource_update( self, @@ -278,7 +279,7 @@ def resource_update( params.pop('is_file_lock_enabled', None) # this can only be set during bucket creation self.api.session.update_bucket(**params) bucket = self.api.get_bucket_by_id(bucket_id) - return self._postprocess(bucket) + return self._postprocess(bucket, config_cors_rules=cors_rules) def resource_delete(self, *, bucket_id, **kwargs): bucket = self.api.get_bucket_by_id(bucket_id) @@ -352,7 +353,7 @@ def _preprocess(self, **kwargs): } return result - def _postprocess(self, obj, **kwargs): + def _postprocess(self, obj, config_cors_rules=None, **kwargs): kwargs.update(obj.as_dict()) file_lock_configuration = kwargs['fileLockConfiguration'] = {} for key in ('isFileLockEnabled', 'defaultRetention'): @@ -361,8 +362,30 @@ def _postprocess(self, obj, **kwargs): file_lock_configuration[key] = value result = convert_json_to_go(kwargs, self.tf_keys) + self._order_allowed_operations(result.get('cors_rules', []), config_cors_rules or []) return result + def _order_allowed_operations(self, cors_rules, config_cors_rules): + # B2 does not necessarily return allowed_operations in the same order as they were set. + # This can cause unnecessary diffs in the Terraform state. + # In order to avoid this, we sort the allowed_operations in the same order as they were set. + for cors_rules_item, config_cors_rules_item in zip(cors_rules, config_cors_rules): + allowed_operations = cors_rules_item.get('allowed_operations', []) + config_allowed_operations = ( + config_cors_rules_item.get('allowedOperations') + or config_cors_rules_item.get('allowed_operations') + or [] + ) + if allowed_operations: + + def sort_key(allowed_operation): + try: + return config_allowed_operations.index(allowed_operation) + except ValueError: + return -1 + + allowed_operations.sort(key=sort_key) + @B2Provider.register_subcommand class BucketFileVersion(Command):