diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 7cbbb534..554b6ad1 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -102,6 +102,31 @@ jobs: permissions: contents: read + release: + strategy: + fail-fast: false + matrix: + charm: + - path: azure_storage/ + track: 1 + - path: s3/ + track: 2 + - path: gcs/ + track: 1 + name: Release charm to Charmhub branch | ${{ matrix.charm.path }} + if: ${{ github.event_name == 'pull_request' }} + needs: + - build + uses: canonical/data-platform-workflows/.github/workflows/release_charm_pr.yaml@v50.1.0 + with: + track: ${{ matrix.charm.track }} + artifact-prefix: ${{ needs.build.outputs.artifact-prefix }} + path-to-charm-directory: ${{ matrix.charm.path }} + secrets: + charmhub-token: ${{ secrets.CHARMHUB_TOKEN }} + permissions: + contents: read + integration-test: name: Integration test charm needs: diff --git a/s3/src/events/provider.py b/s3/src/events/provider.py index aeef1d9e..5d9a70e7 100644 --- a/s3/src/events/provider.py +++ b/s3/src/events/provider.py @@ -172,33 +172,41 @@ def get_statuses(self, scope: Scope, recompute: bool = False) -> list[StatusObje s3_manager = S3Manager(self.state.s3) + config_bucket = self.state.s3.get("bucket") + config_path = self.state.s3.get("path", "") + # When a config bucket is set, it takes precedence and overwrites any bucket + # requested by requirers, so only the config bucket is evaluated here. + if config_bucket: + if not s3_manager.get_bucket(bucket_name=config_bucket, path=config_path): + status_list.append(BucketStatuses.bucket_unavailable(bucket_names=[config_bucket])) + return status_list + status_list.append(CharmStatuses.ACTIVE_IDLE.value) + return status_list + requested_bucket_paths = [ (request.get("bucket", ""), request.get("path", "")) for request in self.get_requested_relation_buckets().values() if request.get("bucket", "") ] - config_bucket = self.state.s3.get("bucket") - config_path = self.state.s3.get("path", "") - if config_bucket and not s3_manager.get_bucket( - bucket_name=config_bucket, path=config_path - ): - status_list.append(BucketStatuses.bucket_unavailable(bucket_names=[config_bucket])) - return status_list - - invalid_buckets = [ - bucket_name - for bucket_name, _ in requested_bucket_paths - if not re.match(BUCKET_REGEX, bucket_name) - ] - missing_buckets = [ - bucket_name - for bucket_name, bucket_path in requested_bucket_paths - if not s3_manager.get_bucket( - bucket_name=bucket_name, path=(config_path or bucket_path or "") + invalid_buckets = list( + dict.fromkeys( + bucket_name + for bucket_name, _ in requested_bucket_paths + if not re.match(BUCKET_REGEX, bucket_name) ) - and bucket_name not in invalid_buckets - ] + ) + # For the status message, skip duplicates and invalid buckets + missing_buckets = list( + dict.fromkeys( + bucket_name + for bucket_name, bucket_path in requested_bucket_paths + if not s3_manager.get_bucket( + bucket_name=bucket_name, path=(config_path or bucket_path or "") + ) + and bucket_name not in invalid_buckets + ) + ) if missing_buckets: status_list.append(BucketStatuses.bucket_unavailable(bucket_names=missing_buckets)) if invalid_buckets: diff --git a/s3/tests/unit/test_provider.py b/s3/tests/unit/test_provider.py index 99a574e9..59332169 100644 --- a/s3/tests/unit/test_provider.py +++ b/s3/tests/unit/test_provider.py @@ -222,6 +222,88 @@ def test_provider_config_bucket_takes_priority_over_relation_bucket( assert provider_data["tls-ca-chain"] == json.dumps(parse_ca_chain(valid_ca_chain.decode())) +@patch("utils.secrets.decode_secret_key_with_retry", decode_secret_key) +@patch("managers.s3.S3Manager.get_bucket", return_value=True) +def test_recompute_statuses_config_bucket_ignores_relation_buckets( + mock_get_bucket, + charm_configuration: dict, + base_state: State, +) -> None: + """When a config bucket is set, recompute must ignore relation-requested buckets.""" + # Given + credentials_secret = Secret( + tracked_content={"access-key": "my-access-key", "secret-key": "my-secret-key"} + ) + charm_configuration["options"]["bucket"]["default"] = "config-bucket" + charm_configuration["options"]["credentials"]["default"] = credentials_secret.id + ctx = Context( + S3IntegratorCharm, meta=METADATA, config=charm_configuration, actions=ACTIONS, unit_id=0 + ) + + s3_provider_relation = Relation( + endpoint="s3-credentials", + remote_app_data={"bucket": "relation-bucket", "requested-secrets": '["foobar"]'}, + ) + relations = list(base_state.relations) + [s3_provider_relation] + state_in = dataclasses.replace(base_state, secrets=[credentials_secret], relations=relations) + + # When: the config bucket is available, but the relation bucket would be unavailable. + with ctx(ctx.on.update_status(), state_in) as manager: + manager.run() + charm = manager.charm + + # get_bucket returns True (config bucket available) -> ACTIVE, and the + # relation-requested bucket is never evaluated. + def only_config_bucket(bucket_name, path=""): + return bucket_name == "config-bucket" + + mock_get_bucket.side_effect = only_config_bucket + statuses = charm.s3_provider_events.get_statuses(scope="app", recompute=True) + + # Then + assert all(s.status == "active" for s in statuses) + assert not any("relation-bucket" in (s.message or "") for s in statuses) + + +@patch("utils.secrets.decode_secret_key_with_retry", decode_secret_key) +def test_recompute_statuses_deduplicates_missing_buckets( + charm_configuration: dict, + base_state: State, +) -> None: + """Recompute should deduplicate buckets.""" + # Given + credentials_secret = Secret( + tracked_content={"access-key": "my-access-key", "secret-key": "my-secret-key"} + ) + charm_configuration["options"]["credentials"]["default"] = credentials_secret.id + ctx = Context( + S3IntegratorCharm, meta=METADATA, config=charm_configuration, actions=ACTIONS, unit_id=0 + ) + + first_relation = Relation( + endpoint="s3-credentials", + remote_app_data={"bucket": "mlpipeline", "requested-secrets": '["foobar"]'}, + ) + second_relation = Relation( + endpoint="s3-credentials", + remote_app_data={"bucket": "mlpipeline", "requested-secrets": '["foobar"]'}, + ) + relations = list(base_state.relations) + [first_relation, second_relation] + state_in = dataclasses.replace(base_state, secrets=[credentials_secret], relations=relations) + + # When: no config bucket set and the requested bucket is unavailable. + with patch("managers.s3.S3Manager.get_bucket", return_value=None): + with ctx(ctx.on.update_status(), state_in) as manager: + manager.run() + charm = manager.charm + statuses = charm.s3_provider_events.get_statuses(scope="app", recompute=True) + + # Then + unavailable = [s for s in statuses if "Could not ensure bucket(s)" in (s.message or "")] + assert len(unavailable) == 1 + assert unavailable[0].message == "Could not ensure bucket(s): 'mlpipeline'" + + @patch("utils.secrets.decode_secret_key_with_retry", decode_secret_key) @patch("managers.s3.S3Manager.get_bucket", return_value=True) def test_provider_compatibility_with_requirer_v0(