diff --git a/.github/.OwlBot.lock.yaml b/.github/.OwlBot.lock.yaml index c07f148f0..2567653c0 100644 --- a/.github/.OwlBot.lock.yaml +++ b/.github/.OwlBot.lock.yaml @@ -1,3 +1,3 @@ docker: - image: gcr.io/repo-automation-bots/owlbot-python:latest - digest: sha256:0ffe3bdd6c7159692df5f7744da74e5ef19966288a6bf76023e8e04e0c424d7d + image: gcr.io/cloud-devrel-public-resources/owlbot-python:latest + digest: sha256:87eee22d276554e4e52863ec9b1cb6a7245815dfae20439712bf644348215a5a diff --git a/.github/.OwlBot.yaml b/.github/.OwlBot.yaml index c2e0f4b92..27096f674 100644 --- a/.github/.OwlBot.yaml +++ b/.github/.OwlBot.yaml @@ -13,7 +13,7 @@ # limitations under the License. docker: - image: gcr.io/repo-automation-bots/owlbot-python:latest + image: gcr.io/cloud-devrel-public-resources/owlbot-python:latest begin-after-commit-hash: 6acf4a0a797f1082027985c55c4b14b60f673dd7 diff --git a/CHANGELOG.md b/CHANGELOG.md index e88e144f7..b09bdd679 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,15 @@ [1]: https://pypi.org/project/google-cloud-storage/#history +### [1.42.3](https://www.github.com/googleapis/python-storage/compare/v1.42.2...v1.42.3) (2021-09-30) + + +### Bug Fixes + +* changeover unspecified to inherited ([#603](https://www.github.com/googleapis/python-storage/issues/603)) ([283a419](https://www.github.com/googleapis/python-storage/commit/283a4196865d9b5275e87f54737d1faee40cc946)) +* check response code in batch.finish ([#609](https://www.github.com/googleapis/python-storage/issues/609)) ([318a286](https://www.github.com/googleapis/python-storage/commit/318a286d709427bfe9f3a37e933c255ac51b3033)) +* skip tests that use unspecified pap until we get the change in ([#600](https://www.github.com/googleapis/python-storage/issues/600)) ([38b9b55](https://www.github.com/googleapis/python-storage/commit/38b9b5582e2c6bbd1acab2b49410084170466fad)) + ### [1.42.2](https://www.github.com/googleapis/python-storage/compare/v1.42.1...v1.42.2) (2021-09-16) diff --git a/google/cloud/storage/batch.py b/google/cloud/storage/batch.py index 732439f14..a60df80f8 100644 --- a/google/cloud/storage/batch.py +++ b/google/cloud/storage/batch.py @@ -276,6 +276,11 @@ def finish(self): response = self._client._base_connection._make_request( "POST", url, data=body, headers=headers, timeout=timeout ) + + # Raise exception if the top-level batch request fails + if not 200 <= response.status_code < 300: + raise exceptions.from_http_response(response) + responses = list(_unpack_batch_response(response)) self._finish_futures(responses) return responses diff --git a/google/cloud/storage/bucket.py b/google/cloud/storage/bucket.py index 8da6e09a8..77e87515b 100644 --- a/google/cloud/storage/bucket.py +++ b/google/cloud/storage/bucket.py @@ -51,7 +51,7 @@ from google.cloud.storage.constants import MULTI_REGIONAL_LEGACY_STORAGE_CLASS from google.cloud.storage.constants import MULTI_REGION_LOCATION_TYPE from google.cloud.storage.constants import NEARLINE_STORAGE_CLASS -from google.cloud.storage.constants import PUBLIC_ACCESS_PREVENTION_UNSPECIFIED +from google.cloud.storage.constants import PUBLIC_ACCESS_PREVENTION_INHERITED from google.cloud.storage.constants import REGIONAL_LEGACY_STORAGE_CLASS from google.cloud.storage.constants import REGION_LOCATION_TYPE from google.cloud.storage.constants import STANDARD_STORAGE_CLASS @@ -387,8 +387,7 @@ class IAMConfiguration(dict): :type public_access_prevention: str :params public_access_prevention: - (Optional) Whether the public access prevention policy is 'unspecified' (default) or 'enforced' - See: https://cloud.google.com/storage/docs/public-access-prevention + (Optional) Whether the public access prevention policy is 'inherited' (default) or 'enforced' See: https://cloud.google.com/storage/docs/public-access-prevention :type uniform_bucket_level_access_enabled: bool @@ -438,7 +437,7 @@ def __init__( uniform_bucket_level_access_enabled = False if public_access_prevention is _default: - public_access_prevention = PUBLIC_ACCESS_PREVENTION_UNSPECIFIED + public_access_prevention = PUBLIC_ACCESS_PREVENTION_INHERITED data = { "uniformBucketLevelAccess": { @@ -481,11 +480,12 @@ def bucket(self): @property def public_access_prevention(self): - """Setting for public access prevention policy. Options are 'unspecified' (default) or 'enforced'. - More information can be found at https://cloud.google.com/storage/docs/public-access-prevention + """Setting for public access prevention policy. Options are 'inherited' (default) or 'enforced'. + + See: https://cloud.google.com/storage/docs/public-access-prevention :rtype: string - :returns: the public access prevention status, either 'enforced' or 'unspecified'. + :returns: the public access prevention status, either 'enforced' or 'inherited'. """ return self["publicAccessPrevention"] diff --git a/google/cloud/storage/constants.py b/google/cloud/storage/constants.py index d0c13f633..2e1c1dd2a 100644 --- a/google/cloud/storage/constants.py +++ b/google/cloud/storage/constants.py @@ -107,5 +107,13 @@ PUBLIC_ACCESS_PREVENTION_UNSPECIFIED = "unspecified" """Unspecified public access prevention value. +DEPRECATED: Use 'PUBLIC_ACCESS_PREVENTION_INHERITED' instead. + +See: https://cloud.google.com/storage/docs/public-access-prevention +""" + +PUBLIC_ACCESS_PREVENTION_INHERITED = "inherited" +"""Inherited public access prevention value. + See: https://cloud.google.com/storage/docs/public-access-prevention """ diff --git a/google/cloud/storage/version.py b/google/cloud/storage/version.py index ccb2fa8f6..8b0acef0b 100644 --- a/google/cloud/storage/version.py +++ b/google/cloud/storage/version.py @@ -12,4 +12,4 @@ # See the License for the specific language governing permissions and # limitations under the License. -__version__ = "1.42.2" +__version__ = "1.42.3" diff --git a/owlbot.py b/owlbot.py index 2b5e9246b..73d931454 100644 --- a/owlbot.py +++ b/owlbot.py @@ -44,6 +44,7 @@ "noxfile.py", "renovate.json", # do not bundle reports "CONTRIBUTING.rst", + ".github/CODEOWNERS", ], ) diff --git a/tests/conformance/retry_conformance_testing.md b/tests/conformance/retry_conformance_testing.md new file mode 100644 index 000000000..22b7c1952 --- /dev/null +++ b/tests/conformance/retry_conformance_testing.md @@ -0,0 +1,29 @@ +# Retry Strategy Conformance Testing + +Calls fail for a host of transient reasons. In many cases the failures are ones that should be abstracted from customers, e.g. retryable issues. Retry strategies are included in the client library when it's safe to do so. These retry strategies are complex and need automated tests to ensure they work and continue to work in the future. + +The Retry Strategy Conformance tests will ensure that retries are aligned across languages and operations are retried as specified. + +## Test Suite Overview + +The Retry Strategy Conformance tests leverage the conformance tests defined in [googleapis/conformance-tests](https://github.com/googleapis/conformance-tests/blob/master/storage/v1/retry_tests.json) to ensure adherence to expected behaviors. + +The test suite uses the [storage-testbench](https://github.com/googleapis/storage-testbench) +to configure and generate tests cases which use fault injection to ensure conformance. + +## Running the Conformance Test Suite + +#### Prerequisites +1. Python 3.8 +2. Nox +3. Docker + +The Retry Strategy Conformance test suite is included in [`noxfile.py`](https://github.com/googleapis/python-storage/blob/main/noxfile.py) and run automatically as part of the Kokoro presubmits: +1. Running the testbench server via docker +2. Setup, validation, cleanup of individual test cases with the testbench +3. Test logs included in Kokoro build + +To run the test suite locally: +```bash +nox -s conftest_retry-3.8 +``` diff --git a/tests/conformance/retry_strategy_test_data.json b/tests/conformance/retry_strategy_test_data.json index 9ed6cbdcd..b807c6a72 100644 --- a/tests/conformance/retry_strategy_test_data.json +++ b/tests/conformance/retry_strategy_test_data.json @@ -1,5 +1,5 @@ { - "retryStrategyTests": [ + "retryTests": [ { "id": 1, "description": "always_idempotent", @@ -127,6 +127,118 @@ ], "preconditionProvided": false, "expectSuccess": false + }, + { + "id": 5, + "description": "non_retryable_errors", + "cases": [ + { + "instructions": ["return-400"] + }, + { + "instructions": ["return-401"] + } + ], + "methods": [ + {"name": "storage.bucket_acl.delete", "resources": ["BUCKET"]}, + {"name": "storage.bucket_acl.get", "resources": ["BUCKET"]}, + {"name": "storage.bucket_acl.insert", "resources": ["BUCKET"]}, + {"name": "storage.bucket_acl.list", "resources": ["BUCKET"]}, + {"name": "storage.bucket_acl.patch", "resources": ["BUCKET"]}, + {"name": "storage.bucket_acl.update", "resources": ["BUCKET"]}, + {"name": "storage.buckets.delete", "resources": ["BUCKET"]}, + {"name": "storage.buckets.get", "resources": ["BUCKET"]}, + {"name": "storage.buckets.getIamPolicy", "resources": ["BUCKET"]}, + {"name": "storage.buckets.insert", "resources": ["BUCKET"]}, + {"name": "storage.buckets.list", "resources": ["BUCKET"]}, + {"name": "storage.buckets.lockRetentionPolicy", "resources": ["BUCKET"]}, + {"name": "storage.buckets.patch", "resources": ["BUCKET"]}, + {"name": "storage.buckets.setIamPolicy", "resources": ["BUCKET"]}, + {"name": "storage.buckets.testIamPermissions", "resources": ["BUCKET"]}, + {"name": "storage.buckets.update", "resources": ["BUCKET"]}, + {"name": "storage.default_object_acl.delete", "resources": ["BUCKET"]}, + {"name": "storage.default_object_acl.get", "resources": ["BUCKET"]}, + {"name": "storage.default_object_acl.insert", "resources": ["BUCKET"]}, + {"name": "storage.default_object_acl.list", "resources": ["BUCKET"]}, + {"name": "storage.default_object_acl.patch", "resources": ["BUCKET"]}, + {"name": "storage.default_object_acl.update", "resources": ["BUCKET"]}, + {"name": "storage.hmacKey.create", "resources": []}, + {"name": "storage.hmacKey.delete", "resources": ["HMAC_KEY"]}, + {"name": "storage.hmacKey.get", "resources": ["HMAC_KEY"]}, + {"name": "storage.hmacKey.list", "resources": ["HMAC_KEY"]}, + {"name": "storage.hmacKey.update", "resources": ["HMAC_KEY"]}, + {"name": "storage.notifications.delete", "resources": ["BUCKET", "NOTIFICATION"]}, + {"name": "storage.notifications.get", "resources": ["BUCKET", "NOTIFICATION"]}, + {"name": "storage.notifications.insert", "resources": ["BUCKET", "NOTIFICATION"]}, + {"name": "storage.notifications.list", "resources": ["BUCKET", "NOTIFICATION"]}, + {"name": "storage.object_acl.delete", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.object_acl.get", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.object_acl.insert", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.object_acl.list", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.object_acl.patch", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.object_acl.update", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.compose", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.copy", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.delete", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.get", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.insert", "resources": ["BUCKET"]}, + {"name": "storage.objects.list", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.patch", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.rewrite", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.update", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.serviceaccount.get", "resources": []} + ], + "preconditionProvided": false, + "expectSuccess": false + }, + { + "id": 6, + "description": "mix_retryable_non_retryable_errors", + "cases": [ + { + "instructions": ["return-503", "return-400"] + }, + { + "instructions": ["return-reset-connection", "return-401"] + } + ], + "methods": [ + {"name": "storage.bucket_acl.get", "resources": ["BUCKET"]}, + {"name": "storage.bucket_acl.list", "resources": ["BUCKET"]}, + {"name": "storage.buckets.delete", "resources": ["BUCKET"]}, + {"name": "storage.buckets.get", "resources": ["BUCKET"]}, + {"name": "storage.buckets.getIamPolicy", "resources": ["BUCKET"]}, + {"name": "storage.buckets.insert", "resources": []}, + {"name": "storage.buckets.list", "resources": ["BUCKET"]}, + {"name": "storage.buckets.lockRetentionPolicy", "resources": ["BUCKET"]}, + {"name": "storage.buckets.patch", "resources": ["BUCKET"]}, + {"name": "storage.buckets.setIamPolicy", "resources": ["BUCKET"]}, + {"name": "storage.buckets.testIamPermissions", "resources": ["BUCKET"]}, + {"name": "storage.buckets.update", "resources": ["BUCKET"]}, + {"name": "storage.default_object_acl.get", "resources": ["BUCKET"]}, + {"name": "storage.default_object_acl.list", "resources": ["BUCKET"]}, + {"name": "storage.hmacKey.delete", "resources": ["HMAC_KEY"]}, + {"name": "storage.hmacKey.get", "resources": ["HMAC_KEY"]}, + {"name": "storage.hmacKey.list", "resources": ["HMAC_KEY"]}, + {"name": "storage.hmacKey.update", "resources": ["HMAC_KEY"]}, + {"name": "storage.notifications.delete", "resources": ["BUCKET", "NOTIFICATION"]}, + {"name": "storage.notifications.get", "resources": ["BUCKET", "NOTIFICATION"]}, + {"name": "storage.notifications.list", "resources": ["BUCKET", "NOTIFICATION"]}, + {"name": "storage.object_acl.get", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.object_acl.list", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.compose", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.copy", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.delete", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.get", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.list", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.insert", "resources": ["BUCKET"]}, + {"name": "storage.objects.patch", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.rewrite", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.update", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.serviceaccount.get", "resources": []} + ], + "preconditionProvided": true, + "expectSuccess": false } ] } \ No newline at end of file diff --git a/tests/conformance/test_conformance.py b/tests/conformance/test_conformance.py index 35b788795..b85f0eaa7 100644 --- a/tests/conformance/test_conformance.py +++ b/tests/conformance/test_conformance.py @@ -14,15 +14,16 @@ """Conformance tests for retry. Verifies correct behavior around retryable errors, idempotency and preconditions.""" +import functools +import logging import os -import requests +import subprocess import tempfile +import time import uuid -import logging -import functools + import pytest -import subprocess -import time +import requests from six.moves.urllib import parse as urlparse @@ -34,9 +35,7 @@ from . import _read_local_json -_CONFORMANCE_TESTS = _read_local_json("retry_strategy_test_data.json")[ - "retryStrategyTests" -] +_CONFORMANCE_TESTS = _read_local_json("retry_strategy_test_data.json")["retryTests"] """Environment variable or default host for Storage testbench emulator.""" _HOST = os.environ.get("STORAGE_EMULATOR_HOST", "http://localhost:9000") diff --git a/tests/system/test_bucket.py b/tests/system/test_bucket.py index 9a5c76931..a9d638efb 100644 --- a/tests/system/test_bucket.py +++ b/tests/system/test_bucket.py @@ -806,21 +806,22 @@ def test_ubla_set_unset_preserves_acls( assert blob_acl_before == blob_acl_after -def test_new_bucket_created_w_unspecified_pap( +def test_new_bucket_created_w_inherited_pap( storage_client, buckets_to_delete, blobs_to_delete, ): from google.cloud.storage import constants - bucket_name = _helpers.unique_name("new-w-pap-unspecified") + bucket_name = _helpers.unique_name("new-w-pap-inherited") bucket = storage_client.bucket(bucket_name) bucket.iam_configuration.uniform_bucket_level_access_enabled = True bucket.create() buckets_to_delete.append(bucket) - assert ( - bucket.iam_configuration.public_access_prevention - == constants.PUBLIC_ACCESS_PREVENTION_UNSPECIFIED - ) + # TODO: Remove unspecified after changeover is complete + assert bucket.iam_configuration.public_access_prevention in [ + constants.PUBLIC_ACCESS_PREVENTION_UNSPECIFIED, + constants.PUBLIC_ACCESS_PREVENTION_INHERITED, + ] bucket.iam_configuration.public_access_prevention = ( constants.PUBLIC_ACCESS_PREVENTION_ENFORCED @@ -855,6 +856,7 @@ def test_new_bucket_created_w_unspecified_pap( blob.make_public() +@pytest.mark.skip(reason="Unspecified PAP is changing to inherited") def test_new_bucket_created_w_enforced_pap( storage_client, buckets_to_delete, blobs_to_delete, ): @@ -874,12 +876,13 @@ def test_new_bucket_created_w_enforced_pap( ) bucket.iam_configuration.public_access_prevention = ( - constants.PUBLIC_ACCESS_PREVENTION_UNSPECIFIED + constants.PUBLIC_ACCESS_PREVENTION_INHERITED ) bucket.patch() - assert ( - bucket.iam_configuration.public_access_prevention - == constants.PUBLIC_ACCESS_PREVENTION_UNSPECIFIED - ) + # TODO: Remove unspecified after changeover is complete + assert bucket.iam_configuration.public_access_prevention in [ + constants.PUBLIC_ACCESS_PREVENTION_UNSPECIFIED, + constants.PUBLIC_ACCESS_PREVENTION_INHERITED, + ] assert not bucket.iam_configuration.uniform_bucket_level_access_enabled diff --git a/tests/unit/test_batch.py b/tests/unit/test_batch.py index d43f27e8e..f877448f8 100644 --- a/tests/unit/test_batch.py +++ b/tests/unit/test_batch.py @@ -447,6 +447,24 @@ def test_finish_nonempty_non_multipart_response(self): with self.assertRaises(ValueError): batch.finish() + def test_finish_multipart_response_with_status_failure(self): + from google.cloud.exceptions import ServiceUnavailable + + url = "http://api.example.com/other_api" + expected_response = _make_response( + status=http_client.SERVICE_UNAVAILABLE, + headers={"content-type": 'multipart/mixed; boundary="DEADBEEF="'}, + ) + http = _make_requests_session([expected_response]) + connection = _Connection(http=http) + client = _Client(connection) + batch = self._make_one(client) + batch.API_BASE_URL = "http://api.example.com" + batch._requests.append(("POST", url, {}, {"foo": 1, "bar": 2}, None)) + + with self.assertRaises(ServiceUnavailable): + batch.finish() + def test_as_context_mgr_wo_error(self): from google.cloud.storage.client import Client diff --git a/tests/unit/test_bucket.py b/tests/unit/test_bucket.py index a63b7fca3..321de717c 100644 --- a/tests/unit/test_bucket.py +++ b/tests/unit/test_bucket.py @@ -23,6 +23,7 @@ from google.cloud.storage.retry import DEFAULT_RETRY_IF_GENERATION_SPECIFIED from google.cloud.storage.retry import DEFAULT_RETRY_IF_METAGENERATION_SPECIFIED from google.cloud.storage.constants import PUBLIC_ACCESS_PREVENTION_ENFORCED +from google.cloud.storage.constants import PUBLIC_ACCESS_PREVENTION_INHERITED from google.cloud.storage.constants import PUBLIC_ACCESS_PREVENTION_UNSPECIFIED @@ -358,8 +359,10 @@ def test_ctor_defaults(self): self.assertIs(config.bucket, bucket) self.assertFalse(config.uniform_bucket_level_access_enabled) self.assertIsNone(config.uniform_bucket_level_access_locked_time) - self.assertEqual( - config.public_access_prevention, PUBLIC_ACCESS_PREVENTION_UNSPECIFIED + # TODO: Remove unspecified after changeover is complete + self.assertIn( + config.public_access_prevention, + [PUBLIC_ACCESS_PREVENTION_UNSPECIFIED, PUBLIC_ACCESS_PREVENTION_INHERITED], ) self.assertFalse(config.bucket_policy_only_enabled) self.assertIsNone(config.bucket_policy_only_locked_time) @@ -396,9 +399,11 @@ def test_ctor_explicit_pap(self): config.public_access_prevention, PUBLIC_ACCESS_PREVENTION_ENFORCED ) - config.public_access_prevention = PUBLIC_ACCESS_PREVENTION_UNSPECIFIED - self.assertEqual( - config.public_access_prevention, PUBLIC_ACCESS_PREVENTION_UNSPECIFIED + config.public_access_prevention = PUBLIC_ACCESS_PREVENTION_INHERITED + # TODO: Remove unspecified after changeover is complete + self.assertIn( + config.public_access_prevention, + [PUBLIC_ACCESS_PREVENTION_UNSPECIFIED, PUBLIC_ACCESS_PREVENTION_INHERITED], ) def test_ctor_explicit_bpo(self):