From b3bb6035a31df2385648c7cdf96843f37ea784da Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Fri, 31 May 2024 13:06:28 +1200 Subject: [PATCH 1/4] Update the add-trailing-comma hook. --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f5fcd9e8..947fcf5c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -13,7 +13,7 @@ repos: - id: end-of-file-fixer - id: trailing-whitespace - repo: https://github.com/asottile/add-trailing-comma - rev: v2.4.0 + rev: v3.1.0 hooks: - id: add-trailing-comma args: [--py36-plus] From 895200c087b80768b11bbeb8e0cc3259cc2b5c17 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Fri, 7 Jun 2024 10:45:05 +1200 Subject: [PATCH 2/4] Move the cloud spec to the model, use the ops CloudSpec and CloudCredential classes, add trustedness. --- README.md | 32 ++++++------- scenario/__init__.py | 4 -- scenario/consistency_checker.py | 6 ++- scenario/mocking.py | 11 +++-- scenario/state.py | 78 ++----------------------------- tests/test_consistency_checker.py | 8 ++-- tests/test_e2e/test_cloud_spec.py | 32 ++++++------- 7 files changed, 52 insertions(+), 119 deletions(-) diff --git a/README.md b/README.md index 21939ae7..e31ad014 100644 --- a/README.md +++ b/README.md @@ -948,30 +948,30 @@ assert out.model.name == "my-model" assert out.model.uuid == state_in.model.uuid ``` -## CloudSpec +### CloudSpec -You can set CloudSpec information in the state (only `type` and `name` are required). +You can set CloudSpec information in the model (only `type` and `name` are required). Example: ```python import scenario -state = scenario.State( - cloud_spec=scenario.CloudSpec( - type="lxd", - name="localhost", - endpoint="https://127.0.0.1:8443", - credential=scenario.CloudCredential( - auth_type="clientcertificate", - attributes={ - "client-cert": "foo", - "client-key": "bar", - "server-cert": "baz", - }, - ), +cloud_spec=ops.CloudSpec( + type="lxd", + name="localhost", + endpoint="https://127.0.0.1:8443", + credential=scenario.CloudCredential( + auth_type="clientcertificate", + attributes={ + "client-cert": "foo", + "client-key": "bar", + "server-cert": "baz", + }, ), - model=scenario.Model(name="my-vm-model", type="lxd"), +) +state = scenario.State( + model=scenario.Model(name="my-vm-model", type="lxd", cloud_spec=cloud_spec), ) ``` diff --git a/scenario/__init__.py b/scenario/__init__.py index 8b832ab0..b8424819 100644 --- a/scenario/__init__.py +++ b/scenario/__init__.py @@ -6,8 +6,6 @@ Action, Address, BindAddress, - CloudCredential, - CloudSpec, Container, DeferredEvent, Event, @@ -31,8 +29,6 @@ __all__ = [ "Action", "ActionOutput", - "CloudCredential", - "CloudSpec", "Context", "deferred", "StateValidationError", diff --git a/scenario/consistency_checker.py b/scenario/consistency_checker.py index c8aebbf9..584bad67 100644 --- a/scenario/consistency_checker.py +++ b/scenario/consistency_checker.py @@ -580,9 +580,11 @@ def check_cloudspec_consistency( errors = [] warnings = [] - if state.model.type == "kubernetes" and state.cloud_spec: + if state.model.type == "kubernetes" and state.model.cloud_spec: errors.append( - "CloudSpec is only available for machine charms, not Kubernetes charms. Tell Scenario to simulate a machine substrate with: `scenario.State(..., model=scenario.Model(type='lxd'))`.", + "CloudSpec is only available for machine charms, not Kubernetes charms. " + "Tell Scenario to simulate a machine substrate with: " + "`scenario.State(..., model=scenario.Model(type='lxd'))`.", ) return Results(errors, warnings) diff --git a/scenario/mocking.py b/scenario/mocking.py index 1081b4ac..27a4c7c6 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -632,11 +632,16 @@ def resource_get(self, resource_name: str) -> str: ) def credential_get(self) -> CloudSpec: - if not self._state.cloud_spec: + if not self._state.trusted: raise ModelError( - "ERROR cloud spec is empty, initialise it with `scenario.State(cloud_spec=scenario.CloudSpec(...))`", + "ERROR charm is not trusted, initialise State with trusted=True", ) - return self._state.cloud_spec._to_ops() + if not self._state.model.cloud_spec: + raise ModelError( + "ERROR cloud spec is empty, initialise it with " + "`State(model=Model(..., cloud_spec=ops.CloudSpec(...)))`", + ) + return self._state.model.cloud_spec class _MockPebbleClient(_TestingPebbleClient): diff --git a/scenario/state.py b/scenario/state.py index a36a7a0a..2fee58a9 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -141,77 +141,6 @@ def copy(self) -> "Self": return copy.deepcopy(self) -@dataclasses.dataclass(frozen=True) -class CloudCredential: - auth_type: str - """Authentication type.""" - - attributes: Dict[str, str] = dataclasses.field(default_factory=dict) - """A dictionary containing cloud credentials. - - For example, for AWS, it contains `access-key` and `secret-key`; - for Azure, `application-id`, `application-password` and `subscription-id` - can be found here. - """ - - redacted: List[str] = dataclasses.field(default_factory=list) - """A list of redacted generic cloud API secrets.""" - - def _to_ops(self) -> ops.CloudCredential: - return ops.CloudCredential( - auth_type=self.auth_type, - attributes=self.attributes, - redacted=self.redacted, - ) - - -@dataclasses.dataclass(frozen=True) -class CloudSpec: - type: str - """Type of the cloud.""" - - name: str = "localhost" - """Juju cloud name.""" - - region: Optional[str] = None - """Region of the cloud.""" - - endpoint: Optional[str] = None - """Endpoint of the cloud.""" - - identity_endpoint: Optional[str] = None - """Identity endpoint of the cloud.""" - - storage_endpoint: Optional[str] = None - """Storage endpoint of the cloud.""" - - credential: Optional[CloudCredential] = None - """Cloud credentials with key-value attributes.""" - - ca_certificates: List[str] = dataclasses.field(default_factory=list) - """A list of CA certificates.""" - - skip_tls_verify: bool = False - """Whether to skip TLS verfication.""" - - is_controller_cloud: bool = False - """If this is the cloud used by the controller.""" - - def _to_ops(self) -> ops.CloudSpec: - return ops.CloudSpec( - type=self.type, - name=self.name, - region=self.region, - endpoint=self.endpoint, - identity_endpoint=self.identity_endpoint, - storage_endpoint=self.storage_endpoint, - credential=self.credential._to_ops() if self.credential else None, - ca_certificates=self.ca_certificates, - skip_tls_verify=self.skip_tls_verify, - is_controller_cloud=self.is_controller_cloud, - ) - - @dataclasses.dataclass(frozen=True) class Secret(_DCBase): id: str @@ -641,6 +570,9 @@ class Model(_DCBase): # TODO: make this exhaustive. type: Literal["kubernetes", "lxd"] = "kubernetes" + cloud_spec: Optional[ops.CloudSpec] = None + """Cloud specification information (metadata) including credentials.""" + # for now, proc mock allows you to map one command to one mocked output. # todo extend: one input -> multiple outputs, at different times @@ -965,6 +897,8 @@ class State(_DCBase): """Whether this charm has leadership.""" model: Model = Model() """The model this charm lives in.""" + trusted: bool = False + """Whether ``juju-trust`` has been run for this charm.""" secrets: List[Secret] = dataclasses.field(default_factory=list) """The secrets this charm has access to (as an owner, or as a grantee). The presence of a secret in this list entails that the charm can read it. @@ -991,8 +925,6 @@ class State(_DCBase): """Status of the unit.""" workload_version: str = "" """Workload version.""" - cloud_spec: Optional[CloudSpec] = None - """Cloud specification information (metadata) including credentials.""" def __post_init__(self): for name in ["app_status", "unit_status"]: diff --git a/tests/test_consistency_checker.py b/tests/test_consistency_checker.py index d5929d98..bc1c22cb 100644 --- a/tests/test_consistency_checker.py +++ b/tests/test_consistency_checker.py @@ -1,5 +1,6 @@ import pytest from ops.charm import CharmBase +from ops.model import CloudCredential, CloudSpec from scenario import Model from scenario.consistency_checker import check_consistency @@ -7,8 +8,6 @@ from scenario.state import ( RELATION_EVENTS_SUFFIX, Action, - CloudCredential, - CloudSpec, Container, Event, Network, @@ -575,6 +574,7 @@ def test_networks_consistency(): def test_cloudspec_consistency(): cloud_spec = CloudSpec( + name="localhost", type="lxd", endpoint="https://127.0.0.1:8443", credential=CloudCredential( @@ -588,7 +588,7 @@ def test_cloudspec_consistency(): ) assert_consistent( - State(cloud_spec=cloud_spec, model=Model(name="lxd-model", type="lxd")), + State(model=Model(name="lxd-model", type="lxd", cloud_spec=cloud_spec)), Event("start"), _CharmSpec( MyCharm, @@ -597,7 +597,7 @@ def test_cloudspec_consistency(): ) assert_inconsistent( - State(cloud_spec=cloud_spec, model=Model(name="k8s-model", type="kubernetes")), + State(model=Model(name="k8s-model", type="kubernetes", cloud_spec=cloud_spec)), Event("start"), _CharmSpec( MyCharm, diff --git a/tests/test_e2e/test_cloud_spec.py b/tests/test_e2e/test_cloud_spec.py index 357061d2..fe0e999c 100644 --- a/tests/test_e2e/test_cloud_spec.py +++ b/tests/test_e2e/test_cloud_spec.py @@ -15,20 +15,7 @@ def _on_event(self, event): def test_get_cloud_spec(): - scenario_cloud_spec = scenario.CloudSpec( - type="lxd", - name="localhost", - endpoint="https://127.0.0.1:8443", - credential=scenario.CloudCredential( - auth_type="clientcertificate", - attributes={ - "client-cert": "foo", - "client-key": "bar", - "server-cert": "baz", - }, - ), - ) - expected_cloud_spec = ops.CloudSpec( + cloud_spec = ops.CloudSpec( type="lxd", name="localhost", endpoint="https://127.0.0.1:8443", @@ -43,11 +30,11 @@ def test_get_cloud_spec(): ) ctx = scenario.Context(MyCharm, meta={"name": "foo"}) state = scenario.State( - cloud_spec=scenario_cloud_spec, - model=scenario.Model(name="lxd-model", type="lxd"), + model=scenario.Model(name="lxd-model", type="lxd", cloud_spec=cloud_spec), + trusted=True, ) with ctx.manager("start", state=state) as mgr: - assert mgr.charm.model.get_cloud_spec() == expected_cloud_spec + assert mgr.charm.model.get_cloud_spec() == cloud_spec def test_get_cloud_spec_error(): @@ -56,3 +43,14 @@ def test_get_cloud_spec_error(): with ctx.manager("start", state) as mgr: with pytest.raises(ops.ModelError): mgr.charm.model.get_cloud_spec() + + +def test_get_cloud_spec_untrusted(): + cloud_spec = ops.CloudSpec(type="lxd", name="localhost") + ctx = scenario.Context(MyCharm, meta={"name": "foo"}) + state = scenario.State( + model=scenario.Model(name="lxd-model", type="lxd", cloud_spec=cloud_spec), + ) + with ctx.manager("start", state) as mgr: + with pytest.raises(ops.ModelError): + mgr.charm.model.get_cloud_spec() From c7ed120fd3f320c457639df346a27802120ebd1a Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Fri, 7 Jun 2024 14:48:01 +1200 Subject: [PATCH 3/4] Restore the Scenario CloudSpec and CloudCredential classes. --- README.md | 3 +- scenario/__init__.py | 4 ++ scenario/mocking.py | 2 +- scenario/state.py | 72 ++++++++++++++++++++++++++++++- tests/test_consistency_checker.py | 3 +- tests/test_e2e/test_cloud_spec.py | 21 +++++++-- 6 files changed, 97 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index e31ad014..6a2cf135 100644 --- a/README.md +++ b/README.md @@ -957,9 +957,8 @@ Example: ```python import scenario -cloud_spec=ops.CloudSpec( +cloud_spec=scenario.CloudSpec( type="lxd", - name="localhost", endpoint="https://127.0.0.1:8443", credential=scenario.CloudCredential( auth_type="clientcertificate", diff --git a/scenario/__init__.py b/scenario/__init__.py index b8424819..8b832ab0 100644 --- a/scenario/__init__.py +++ b/scenario/__init__.py @@ -6,6 +6,8 @@ Action, Address, BindAddress, + CloudCredential, + CloudSpec, Container, DeferredEvent, Event, @@ -29,6 +31,8 @@ __all__ = [ "Action", "ActionOutput", + "CloudCredential", + "CloudSpec", "Context", "deferred", "StateValidationError", diff --git a/scenario/mocking.py b/scenario/mocking.py index 27a4c7c6..8cd4d3c8 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -641,7 +641,7 @@ def credential_get(self) -> CloudSpec: "ERROR cloud spec is empty, initialise it with " "`State(model=Model(..., cloud_spec=ops.CloudSpec(...)))`", ) - return self._state.model.cloud_spec + return self._state.model.cloud_spec._to_ops() class _MockPebbleClient(_TestingPebbleClient): diff --git a/scenario/state.py b/scenario/state.py index 2fee58a9..93bcadbc 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -141,6 +141,76 @@ def copy(self) -> "Self": return copy.deepcopy(self) +@dataclasses.dataclass(frozen=True) +class CloudCredential: + auth_type: str + """Authentication type.""" + + attributes: Dict[str, str] = dataclasses.field(default_factory=dict) + """A dictionary containing cloud credentials. + For example, for AWS, it contains `access-key` and `secret-key`; + for Azure, `application-id`, `application-password` and `subscription-id` + can be found here. + """ + + redacted: List[str] = dataclasses.field(default_factory=list) + """A list of redacted generic cloud API secrets.""" + + def _to_ops(self) -> ops.CloudCredential: + return ops.CloudCredential( + auth_type=self.auth_type, + attributes=self.attributes, + redacted=self.redacted, + ) + + +@dataclasses.dataclass(frozen=True) +class CloudSpec: + type: str + """Type of the cloud.""" + + name: str = "localhost" + """Juju cloud name.""" + + region: Optional[str] = None + """Region of the cloud.""" + + endpoint: Optional[str] = None + """Endpoint of the cloud.""" + + identity_endpoint: Optional[str] = None + """Identity endpoint of the cloud.""" + + storage_endpoint: Optional[str] = None + """Storage endpoint of the cloud.""" + + credential: Optional[CloudCredential] = None + """Cloud credentials with key-value attributes.""" + + ca_certificates: List[str] = dataclasses.field(default_factory=list) + """A list of CA certificates.""" + + skip_tls_verify: bool = False + """Whether to skip TLS verfication.""" + + is_controller_cloud: bool = False + """If this is the cloud used by the controller.""" + + def _to_ops(self) -> ops.CloudSpec: + return ops.CloudSpec( + type=self.type, + name=self.name, + region=self.region, + endpoint=self.endpoint, + identity_endpoint=self.identity_endpoint, + storage_endpoint=self.storage_endpoint, + credential=self.credential._to_ops() if self.credential else None, + ca_certificates=self.ca_certificates, + skip_tls_verify=self.skip_tls_verify, + is_controller_cloud=self.is_controller_cloud, + ) + + @dataclasses.dataclass(frozen=True) class Secret(_DCBase): id: str @@ -570,7 +640,7 @@ class Model(_DCBase): # TODO: make this exhaustive. type: Literal["kubernetes", "lxd"] = "kubernetes" - cloud_spec: Optional[ops.CloudSpec] = None + cloud_spec: Optional[CloudSpec] = None """Cloud specification information (metadata) including credentials.""" diff --git a/tests/test_consistency_checker.py b/tests/test_consistency_checker.py index bc1c22cb..78c85102 100644 --- a/tests/test_consistency_checker.py +++ b/tests/test_consistency_checker.py @@ -1,6 +1,5 @@ import pytest from ops.charm import CharmBase -from ops.model import CloudCredential, CloudSpec from scenario import Model from scenario.consistency_checker import check_consistency @@ -8,6 +7,8 @@ from scenario.state import ( RELATION_EVENTS_SUFFIX, Action, + CloudCredential, + CloudSpec, Container, Event, Network, diff --git a/tests/test_e2e/test_cloud_spec.py b/tests/test_e2e/test_cloud_spec.py index fe0e999c..fc5fa83f 100644 --- a/tests/test_e2e/test_cloud_spec.py +++ b/tests/test_e2e/test_cloud_spec.py @@ -15,7 +15,20 @@ def _on_event(self, event): def test_get_cloud_spec(): - cloud_spec = ops.CloudSpec( + scenario_cloud_spec = scenario.CloudSpec( + type="lxd", + name="localhost", + endpoint="https://127.0.0.1:8443", + credential=scenario.CloudCredential( + auth_type="clientcertificate", + attributes={ + "client-cert": "foo", + "client-key": "bar", + "server-cert": "baz", + }, + ), + ) + expected_cloud_spec = ops.CloudSpec( type="lxd", name="localhost", endpoint="https://127.0.0.1:8443", @@ -30,11 +43,13 @@ def test_get_cloud_spec(): ) ctx = scenario.Context(MyCharm, meta={"name": "foo"}) state = scenario.State( - model=scenario.Model(name="lxd-model", type="lxd", cloud_spec=cloud_spec), + model=scenario.Model( + name="lxd-model", type="lxd", cloud_spec=scenario_cloud_spec + ), trusted=True, ) with ctx.manager("start", state=state) as mgr: - assert mgr.charm.model.get_cloud_spec() == cloud_spec + assert mgr.charm.model.get_cloud_spec() == expected_cloud_spec def test_get_cloud_spec_error(): From b895d36db021a8559b5b545b99b59338374a38ec Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Fri, 7 Jun 2024 22:56:23 +1200 Subject: [PATCH 4/4] The trust is really more context than state, per review. --- scenario/context.py | 4 ++++ scenario/mocking.py | 4 ++-- scenario/state.py | 2 -- tests/test_e2e/test_cloud_spec.py | 3 +-- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/scenario/context.py b/scenario/context.py index 9de78b1c..a3e2a8ea 100644 --- a/scenario/context.py +++ b/scenario/context.py @@ -169,6 +169,7 @@ def __init__( capture_framework_events: bool = False, app_name: Optional[str] = None, unit_id: Optional[int] = 0, + app_trusted: bool = False, ): """Represents a simulated charm's execution context. @@ -225,6 +226,8 @@ def __init__( :arg app_name: App name that this charm is deployed as. Defaults to the charm name as defined in metadata.yaml. :arg unit_id: Unit ID that this charm is deployed as. Defaults to 0. + :arg app_trusted: whether the charm has Juju trust (deployed with ``--trust`` or added with + ``juju trust``). Defaults to False :arg charm_root: virtual charm root the charm will be executed with. If the charm, say, expects a `./src/foo/bar.yaml` file present relative to the execution cwd, you need to use this. E.g.: @@ -268,6 +271,7 @@ def __init__( self._app_name = app_name self._unit_id = unit_id + self.app_trusted = app_trusted self._tmp = tempfile.TemporaryDirectory() # config for what events to be captured in emitted_events. diff --git a/scenario/mocking.py b/scenario/mocking.py index 8cd4d3c8..a55397d2 100644 --- a/scenario/mocking.py +++ b/scenario/mocking.py @@ -632,9 +632,9 @@ def resource_get(self, resource_name: str) -> str: ) def credential_get(self) -> CloudSpec: - if not self._state.trusted: + if not self._context.app_trusted: raise ModelError( - "ERROR charm is not trusted, initialise State with trusted=True", + "ERROR charm is not trusted, initialise Context with `app_trusted=True`", ) if not self._state.model.cloud_spec: raise ModelError( diff --git a/scenario/state.py b/scenario/state.py index 93bcadbc..c0d573fa 100644 --- a/scenario/state.py +++ b/scenario/state.py @@ -967,8 +967,6 @@ class State(_DCBase): """Whether this charm has leadership.""" model: Model = Model() """The model this charm lives in.""" - trusted: bool = False - """Whether ``juju-trust`` has been run for this charm.""" secrets: List[Secret] = dataclasses.field(default_factory=list) """The secrets this charm has access to (as an owner, or as a grantee). The presence of a secret in this list entails that the charm can read it. diff --git a/tests/test_e2e/test_cloud_spec.py b/tests/test_e2e/test_cloud_spec.py index fc5fa83f..8ce413f8 100644 --- a/tests/test_e2e/test_cloud_spec.py +++ b/tests/test_e2e/test_cloud_spec.py @@ -41,12 +41,11 @@ def test_get_cloud_spec(): }, ), ) - ctx = scenario.Context(MyCharm, meta={"name": "foo"}) + ctx = scenario.Context(MyCharm, meta={"name": "foo"}, app_trusted=True) state = scenario.State( model=scenario.Model( name="lxd-model", type="lxd", cloud_spec=scenario_cloud_spec ), - trusted=True, ) with ctx.manager("start", state=state) as mgr: assert mgr.charm.model.get_cloud_spec() == expected_cloud_spec