Skip to content

Commit

Permalink
Merge pull request #9 from juditnovak/big_test
Browse files Browse the repository at this point in the history
More extended and verbose tests
  • Loading branch information
juditnovak authored Jul 21, 2023
2 parents 8ebda89 + 0de65da commit 90c154b
Show file tree
Hide file tree
Showing 5 changed files with 169 additions and 106 deletions.
9 changes: 8 additions & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ jobs:
runs-on: ubuntu-latest
needs:
- build
strategy:
matrix:
include:
- snap: "3.1/stable"
- snap: "3.1/edge"
steps:
- name: Checkout
uses: actions/checkout@v3
Expand All @@ -32,7 +37,7 @@ jobs:
# This is needed until https://bugs.launchpad.net/juju/+bug/1977582 is fixed
bootstrap-options: "--agent-version 3.1.5"
bootstrap-constraints: "cores=2 mem=2G"
juju-channel: "3.1/stable"
juju-channel: ${{ matrix.snap }}
- name: Download packed charm(s)
uses: actions/download-artifact@v3
with:
Expand All @@ -41,6 +46,8 @@ jobs:
run: tox run -e integration -- --model testing
env:
CI_PACKED_CHARMS: ${{ needs.build.outputs.charms }}
- name: Print logs
run: juju switch testing; juju debug-log --replay --no-tail
- name: Dump logs
uses: canonical/charm-logdump-action@main
if: failure()
Expand Down
55 changes: 40 additions & 15 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,50 @@ Use links instead.

Small demonstration of the unexpected beahvior observed when multiple changes (deletion) are applied on a multi-pack secret of a charm within the smae event context.

## Issue

Demonstration of the issue is visible in the correpsonding [Integration Test](tests/integration/test_charm.py) -- run successfully by the corresponding pipeline.
## Description

Demonstration of the issue is visible in the correpsonding [Integration Test](tests/integration/test_charm.py).

NOTE: These tests are representing BROKEN behavior.

Running green means that the feature is still broken.


## How to use

Pipelines are equipped to run both on Juju 3.1. `edge` and `stable` releases.

Whenever a new Juju 3.1. `stable`/`edge` release may be out, you just have to re-run the pipelines to verify if previous issues still may hold.

NOTE: You would want to see ALL tests to FAIL (except the first one marked in the docstring as functional). That means that issues listed here are fully fixed.


## Pipelines

The demo is equipped with verbose pipelines, printing expected results vs. actual ones.

Furthermore, all `juju debug-logs` are also listed after each test run (in case curiousity about the details).

An example of how the demo is showing broken behavior:

```
for i in range(3):
await helper_execute_action(ops_test, "set-secret", {"key": f"key{i}", "value": f"value{i}"})
await helper_execute_action(ops_test, "delete-secrets", {"keys": ["key{i}" for i in range(3)]})
secrets_data = await helper_execute_action(ops_test, "get-secrets")
# ISSUE: This is NOT the intuitively expected behavior
assert secrets_data.get("secrets") == {"key2": "value2", "key3": "value3"}
```
async def test_delete_secrets_within_the_same_action_scope(ops_test: OpsTest):
"""Testing if it's possible to remove a secret from a joined secret removing one-by-one within the same event scope.

NOTE: This should fail
"""
await helper_execute_action(ops_test, "set-secret", {"key": "key1", "value": "value1"})
await helper_execute_action(ops_test, "set-secret", {"key": "key2", "value": "value2"})
await helper_execute_action(ops_test, "set-secret", {"key": "key3", "value": "value3"})

await helper_execute_action(ops_test, "delete-secrets", {"keys": ["key1", "key2", "key3"]})
## Juju Secret issues

At the time of creating the demo, major issues are typically as such:
multiple manipulations within the same event scope applied on a multi-value secret object are not working as expected.

secrets_data = await helper_execute_action(ops_test, "get-secrets")
Typically for example when there's an attempt to remove multiple keys from a joined secret -- it's only the first action taking effect.
(Occasionally it's the first and the last action taking effect.)

#
# ISSUE: This is NOT the intuitively expected behavior
#
assert secrets_data.get("secrets") == {"key2": "value2", "key3": "value3"}
Similar problems were observed on setting a secret value within a joined secret.
9 changes: 3 additions & 6 deletions actions.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,9 @@ forget-all-secrets:

set-secret:
description: Test action to set a secret key/value pair.
key:
type: string
description: The key for the secret
value:
type: string
description: The value for the secret
content:
type: dict
description: The content for the secret

get-secrets:
description: Retrieve all the secrets stored in juju storage.
Expand Down
20 changes: 7 additions & 13 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ def _on_start(self, event) -> None:
self.unit.status = ActiveStatus()

def _on_set_secret_action(self, event: ActionEvent):
key, value = event.params.get("key"), event.params.get("value")
event.set_results({"secret-id": self.set_secret(key, value)})
content = event.params
event.set_results({"secret-id": self.set_secret(content)})

def _on_get_secrets_action(self, event: ActionEvent):
"""Return the secrets stored in juju secrets backend."""
Expand All @@ -58,7 +58,7 @@ def _on_delete_secrets_action(self, event: ActionEvent):
def _on_pseudo_delete_secrets_action(self, event: ActionEvent):
keys = event.params.get("keys")
for key in keys:
self.set_secret(key, "### DELETED ###")
self.set_secret({key: "### DELETED ###"})

def _on_forget_all_secrets_action(self, event: ActionEvent):
if self.app_peer_data.get("secret-id"):
Expand Down Expand Up @@ -92,26 +92,20 @@ def get_secrets(self) -> dict[str, str]:
logger.info(f"Retrieved secret {secret_id} with content {content}")
return content

def set_secret(self, key: str, value: str) -> None:
def set_secret(self, new_content: dict) -> None:
"""Set the secret in the juju secret storage."""
if not value:
return self.delete_secret(key)

secret_id = self.app_peer_data.get("secret-id")

if secret_id:
secret = self.model.get_secret(id=secret_id)
content = secret.get_content()
content.update({key: value})
content.update(new_content)
logger.info(f"Setting secret {secret.id} to {content}")
secret.set_content(content)
else:
content = {
key: value,
}
secret = self.app.add_secret(content)
secret = self.app.add_secret(new_content)
self.app_peer_data["secret-id"] = secret.id
logger.info(f"Added secret {secret.id} to {content}")
logger.info(f"Added secret {secret.id} to {new_content}")

return secret.id

Expand Down
Loading

0 comments on commit 90c154b

Please sign in to comment.