Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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' }}
Comment thread
mvlassis marked this conversation as resolved.
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:
Expand Down
48 changes: 28 additions & 20 deletions s3/src/events/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Comment thread
mvlassis marked this conversation as resolved.
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:
Expand Down
82 changes: 82 additions & 0 deletions s3/tests/unit/test_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down