From 24aa0bfaa2f485bc95332723f3f8ea2676a3bffb Mon Sep 17 00:00:00 2001 From: Judit Novak Date: Thu, 20 Jul 2023 20:25:46 +0200 Subject: [PATCH 1/6] Testing a gigantic secret --- tests/integration/test_charm.py | 44 +++++++++++++++++++++++++++++++++ tox.ini | 1 - 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index d9258a1..3f05a2a 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -185,3 +185,47 @@ async def test_set_secrets_within_the_same_action_scope_works(ops_test: OpsTest) "key4": "value4", "key5": "### DELETED ###", } + + +async def test_delete_lotta_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, "forget-all-secrets") + + for i in range(20): + 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": [f"key{i}" for i in range(20)]}) + + secrets_data = await helper_execute_action(ops_test, "get-secrets") + + # + # ISSUE!!!!! Empty dict wouldn't have made it to event results + # + assert secrets_data.get("secrets") + + print(f"Actual results: {secrets_data.get('secrets')}") + + +async def test_set_lotta_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, "forget-all-secrets") + + for i in range(20): + await helper_execute_action(ops_test, "set-secret", {"key": f"key{i}", "value": f"value{i}"}) + + await helper_execute_action(ops_test, "pseudo-delete-secrets", {"keys": [f"key{i}" for i in range(20)]}) + + secrets_data = await helper_execute_action(ops_test, "get-secrets") + + # + # ISSUE!!!!! Empty dict wouldn't have made it to event results + # + assert secrets_data.get("secrets") + + print(f"Actual results: {secrets_data.get('secrets')}") diff --git a/tox.ini b/tox.ini index 8936bc1..6fa263a 100644 --- a/tox.ini +++ b/tox.ini @@ -72,7 +72,6 @@ pass_env = CI_PACKED_CHARMS commands = pytest -v \ - -s \ --tb native \ --log-cli-level=INFO \ {posargs} \ From cf6e806b3a62775729e4ce1b012aa4dbc2b93215 Mon Sep 17 00:00:00 2001 From: Judit Novak Date: Fri, 21 Jul 2023 01:17:12 +0200 Subject: [PATCH 2/6] Verbose tests --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index 6fa263a..8936bc1 100644 --- a/tox.ini +++ b/tox.ini @@ -72,6 +72,7 @@ pass_env = CI_PACKED_CHARMS commands = pytest -v \ + -s \ --tb native \ --log-cli-level=INFO \ {posargs} \ From 07327ec9f95fcf3a5aa86c8952fc4a7564f1207a Mon Sep 17 00:00:00 2001 From: Judit Novak Date: Fri, 21 Jul 2023 01:17:52 +0200 Subject: [PATCH 3/6] Setting multiple secrets, quicker test initialization --- actions.yaml | 9 +++------ src/charm.py | 20 +++++++------------- 2 files changed, 10 insertions(+), 19 deletions(-) diff --git a/actions.yaml b/actions.yaml index 8e65150..bee16e8 100644 --- a/actions.yaml +++ b/actions.yaml @@ -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. diff --git a/src/charm.py b/src/charm.py index aebb7ea..3c2a10b 100755 --- a/src/charm.py +++ b/src/charm.py @@ -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.""" @@ -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"): @@ -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 From 4d72496f3882867c68636ec6690f934436015fa0 Mon Sep 17 00:00:00 2001 From: Judit Novak Date: Fri, 21 Jul 2023 01:18:17 +0200 Subject: [PATCH 4/6] Matrix for stable/edge --- .github/workflows/ci.yaml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index c4b0c64..d150fc9 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -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 @@ -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: @@ -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() From 7bf0f4dcce43495205fd008a06eede83009ded01 Mon Sep 17 00:00:00 2001 From: Judit Novak Date: Fri, 21 Jul 2023 01:19:06 +0200 Subject: [PATCH 5/6] Tests reoladed --- tests/integration/test_charm.py | 172 ++++++++++++++++---------------- 1 file changed, 84 insertions(+), 88 deletions(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 3f05a2a..3b833d9 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -4,6 +4,7 @@ import asyncio import logging +import json from pathlib import Path from typing import Optional @@ -50,28 +51,26 @@ async def helper_execute_action( return action.results -async def test_delete_secret(ops_test: OpsTest): - """Testing if it's possible to remove a secret from a joined secret removing one-by-one. +async def test_delete_secret_within_separate_event_scopes_always_works(ops_test: OpsTest): + """Testing if it's possible to remove all keys from a joined secret one-by-one in SEPARATE event scopes. - NOTE: This should work + NOTE: This functionality is OK """ await helper_execute_action(ops_test, "forget-all-secrets") - 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"}) + content = {f"key{i}": f"value{i}" for i in range(3)} + await helper_execute_action(ops_test, "set-secret", content) secrets_data = await helper_execute_action(ops_test, "get-secrets") assert secrets_data["secrets"] == { + "key0": "value0", "key1": "value1", "key2": "value2", - "key3": "value3", } - await helper_execute_action(ops_test, "set-secret", {"key": "key1"}) - await helper_execute_action(ops_test, "set-secret", {"key": "key2"}) - await helper_execute_action(ops_test, "set-secret", {"key": "key3"}) + for i in range(3): + await helper_execute_action(ops_test, "delete-secrets", {"keys": [f"key{i}"]}) secrets_data = await helper_execute_action(ops_test, "get-secrets") @@ -80,152 +79,149 @@ async def test_delete_secret(ops_test: OpsTest): async def test_delete_all_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 + """Testing if it's possible to remove all keys from a joined secret removing one-by-one within the same event scope. """ await helper_execute_action(ops_test, "forget-all-secrets") - 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"}) + content = {f"key{i}": f"value{i}" for i in range(3)} + await helper_execute_action(ops_test, "set-secret", content) - await helper_execute_action(ops_test, "delete-secrets", {"keys": ["key1", "key2", "key3"]}) + await helper_execute_action(ops_test, "delete-secrets", {"keys": [f"key{i}" for i in range(3)]}) secrets_data = await helper_execute_action(ops_test, "get-secrets") - # - # The issue still holds :-( :-( :-( - # Should be {} - # - assert secrets_data.get("secrets") == { - "key2": "value2", - "key3": "value3", - } + secrets = secrets_data.get("secrets") + # ISSUE!!!!! Empty dict wouldn't have made it to event results + assert secrets + + print() + print("*************************************************************************") + print("All keys should be deleted [0..2].") + print(f"Actual results: {json.dumps(secrets, sort_keys=True, indent=4)}") + print("*************************************************************************") 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. + """Testing if it's possible to remove keys from a joined secret one-by-one within the same event scope. NOTE: This should fail """ await helper_execute_action(ops_test, "forget-all-secrets") - 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, "set-secret", {"key": "key4", "value": "value4"}) - await helper_execute_action(ops_test, "set-secret", {"key": "key5", "value": "value5"}) + content = {f"key{i}": f"value{i}" for i in range(5)} + await helper_execute_action(ops_test, "set-secret", content) - await helper_execute_action(ops_test, "delete-secrets", {"keys": ["key1", "key3", "key5"]}) + await helper_execute_action(ops_test, "delete-secrets", {"keys": [f"key{i}" for i in range(0, 5, 2)]}) secrets_data = await helper_execute_action(ops_test, "get-secrets") - # - # The issue still holds :-( - # Should be {"key2": "password2", "key4": "password4"} - # - assert secrets_data.get("secrets") == { - "key2": "value2", - "key3": "value3", - "key4": "value4", - "key5": "value5", - } + secrets = secrets_data.get("secrets") + # ISSUE!!!!! Empty dict wouldn't have made it to event results + assert secrets + assert 2 < len(secrets.keys()) + + print() + print("*************************************************************************") + print("Even keys should be deleted [0..4].") + print(f"Actual results: {json.dumps(secrets, sort_keys=True, indent=4)}") + print("*************************************************************************") async def test_set_all_secrets_within_the_same_action_scope_work_fine(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. + """Testing if it's possible to set all values within a joined secret one-by-one within the same event scope. NOTE: This should fail """ await helper_execute_action(ops_test, "forget-all-secrets") - 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"}) + content = {f"key{i}": f"value{i}" for i in range(5)} + await helper_execute_action(ops_test, "set-secret", content) - await helper_execute_action(ops_test, "pseudo-delete-secrets", {"keys": ["key1", "key2", "key3"]}) + await helper_execute_action(ops_test, "pseudo-delete-secrets", {"keys": [f"key{i}" for i in range(5)]}) secrets_data = await helper_execute_action(ops_test, "get-secrets") - # - # ISSUE!!!!! "key2" should be '### DELETED ###' - # - assert secrets_data.get("secrets") == { - "key1": "### DELETED ###", - "key2": "value2", - "key3": "### DELETED ###", - } + secrets = secrets_data.get("secrets") + assert 0 < sum([secrets[key] != "### DELETED ###" for key in secrets]) + + print() + print("*************************************************************************") + print("All keys should be marked as deleted [0..4].") + print(f"Actual results: {json.dumps(secrets, sort_keys=True, indent=4)}") + print("*************************************************************************") async def test_set_secrets_within_the_same_action_scope_works(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. + """Testing if it's possible to set a some values within a joined secret one-by-one within the same event scope. NOTE: This should fail """ await helper_execute_action(ops_test, "forget-all-secrets") - 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, "set-secret", {"key": "key4", "value": "value4"}) - await helper_execute_action(ops_test, "set-secret", {"key": "key5", "value": "value5"}) + content = {f"key{i}": f"value{i}" for i in range(5)} + await helper_execute_action(ops_test, "set-secret", content) - await helper_execute_action(ops_test, "pseudo-delete-secrets", {"keys": ["key1", "key3", "key5"]}) + await helper_execute_action(ops_test, "pseudo-delete-secrets", {"keys": [f"key{i}" for i in range(0, 5, 2)]}) secrets_data = await helper_execute_action(ops_test, "get-secrets") - # # ISSUE!!!!! "key3" should be '### DELETED ###' - # - assert secrets_data.get("secrets") == { - "key1": "### DELETED ###", - "key2": "value2", - "key3": "value3", - "key4": "value4", - "key5": "### DELETED ###", - } + secrets = secrets_data.get("secrets") + assert 2 < sum([secrets[key] != "### DELETED ###" for key in secrets]) + + print() + print("*************************************************************************") + print("Even keys should be marked as deleted [0..4].") + print(f"Actual results: {json.dumps(secrets_data.get('secrets'), sort_keys=True, indent=4)}") + print("*************************************************************************") async def test_delete_lotta_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. + """Testing if it's possible to remove keys from a joined secret one-by-one within the same event scope. NOTE: This should fail """ await helper_execute_action(ops_test, "forget-all-secrets") - for i in range(20): - await helper_execute_action(ops_test, "set-secret", {"key": f"key{i}", "value": f"value{i}"}) + content = {f"key{i}": f"value{i}" for i in range(15)} + await helper_execute_action(ops_test, "set-secret", content) - await helper_execute_action(ops_test, "delete-secrets", {"keys": [f"key{i}" for i in range(20)]}) + await helper_execute_action(ops_test, "delete-secrets", {"keys": [f"key{i}" for i in range(15)]}) secrets_data = await helper_execute_action(ops_test, "get-secrets") - # + secrets = secrets_data.get("secrets") # ISSUE!!!!! Empty dict wouldn't have made it to event results - # - assert secrets_data.get("secrets") + assert secrets - print(f"Actual results: {secrets_data.get('secrets')}") + print() + print("*************************************************************************") + print("Keys ([0..14]) all deleted.") + print(f"Actual results: {json.dumps(secrets, sort_keys=True, indent=4)}") + print("*************************************************************************") async def test_set_lotta_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. + """Testing if it's possible to remove a keys from a joined secret one-by-one within the same event scope. NOTE: This should fail """ await helper_execute_action(ops_test, "forget-all-secrets") - for i in range(20): - await helper_execute_action(ops_test, "set-secret", {"key": f"key{i}", "value": f"value{i}"}) + content = {f"key{i}": f"value{i}" for i in range(15)} + await helper_execute_action(ops_test, "set-secret", content) - await helper_execute_action(ops_test, "pseudo-delete-secrets", {"keys": [f"key{i}" for i in range(20)]}) + await helper_execute_action(ops_test, "pseudo-delete-secrets", {"keys": [f"key{i}" for i in range(15)]}) secrets_data = await helper_execute_action(ops_test, "get-secrets") - # + secrets = secrets_data.get("secrets") # ISSUE!!!!! Empty dict wouldn't have made it to event results - # - assert secrets_data.get("secrets") - - print(f"Actual results: {secrets_data.get('secrets')}") + assert secrets + assert 0 < sum([f"key{i}" != "### DELETED ###" for i in range(15)]) + + print() + print("*************************************************************************") + print("Keys [0..14] were all marked as deleted.") + print(f"Actual results: {json.dumps(secrets, sort_keys=True, indent=4)}") + print("*************************************************************************") From 0de65da3518a1660625371d817f1145828f114d3 Mon Sep 17 00:00:00 2001 From: Judit Novak Date: Fri, 21 Jul 2023 02:22:33 +0200 Subject: [PATCH 6/6] README.md --- README.md | 55 ++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index c1e8207..81a2ee9 100644 --- a/README.md +++ b/README.md @@ -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.