Merge pull request #1150 from james-garner-canonical/24.10/tests/vali…

…date-client-facades

#1150

#### Description

`client_facades` in `juju/client/connection.py` is manually updated. It doesn't reflect the actual facade code present, which was recently generated from the current 3.1.X and 3.3.0 schemas currently checked in. Nor did it reflect the state of the generated code with the previous set of schemas (same + 3.2.X schemas), nor the state of the actually checked in code before the last regeneration of the code (which contained some orphan facades not present in the schemas).

This PR updates `client_schemas` and adds a test to ensure that its facade versions always match the schemas present in the generated client code. The test can be run with `tox -e validate`, and is run in a separate github job as it didn't seem to fit in well as either a unit or integration test. In future we could add more tests to this group to validate other aspects of the codebase related to the code generation.

#### QA Steps

All non-quarantined tests should continue to work.

```
tox -e validate
```
Should pass currently.

#### Notes & Discussion

Does the current state of client_facades make sense? One thing that stood out to me was the addition of `Admin`. I'm wondering if any non-client facades are being added here, since facade code is generated from the full schemas still. If that's the case, then this should be deferred till after we switch to client-only schemas.

.github/workflows/test.yaml

@@ -43,6 +43,25 @@ jobs:
         run: |
           make build-test
 
+  validate:
+    name: Validate
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        python:
+          - "3.10"
+    steps:
+      - name: Check out code
+        uses: actions/checkout@v4
+      - name: Setup Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: ${{ matrix.python }}
+      - name: Install dependencies
+        run: pip install tox
+      - name: Run validation tests
+        run: tox -e validate
+
   unit-tests:
     needs: lint
     name: Unit tests

juju/client/connection.py

@@ -9,6 +9,7 @@
 import weakref
 from http.client import HTTPSConnection
 from dateutil.parser import parse
+from typing import Dict, List
 
 import macaroonbakery.bakery as bakery
 import macaroonbakery.httpbakery as httpbakery
@@ -20,86 +21,89 @@
 
 log = logging.getLogger('juju.client.connection')
 
+# Manual list of facades present in schemas + codegen which python-libjuju does not yet support
+excluded_facades: Dict[str, List[int]] = {
+    'Charms': [7],
+}
 # Please keep in alphabetical order
+# in future this will likely be generated automatically (perhaps at runtime)
 client_facades = {
-    'Action': {'versions': [2, 6, 7]},
+    'Action': {'versions': [7]},
     'ActionPruner': {'versions': [1]},
-    'Agent': {'versions': [2, 3]},
+    'Admin': {'versions': [3]},
+    'Agent': {'versions': [3]},
     'AgentLifeFlag': {'versions': [1]},
     'AgentTools': {'versions': [1]},
-    'AllModelWatcher': {'versions': [2, 3, 4]},
-    'AllWatcher': {'versions': [1, 2, 3, 4]},
+    'AllModelWatcher': {'versions': [4]},
+    'AllWatcher': {'versions': [3]},
     'Annotations': {'versions': [2]},
-    'Application': {'versions': [14, 15, 16, 17, 19]},
-    'ApplicationOffers': {'versions': [1, 2, 4]},
+    'Application': {'versions': [17, 18, 19]},
+    'ApplicationOffers': {'versions': [4]},
     'ApplicationScaler': {'versions': [1]},
-    'Backups': {'versions': [1, 2, 3]},
+    'Backups': {'versions': [3]},
     'Block': {'versions': [2]},
-    'Bundle': {'versions': [5, 6]},
-    'CharmHub': {'versions': [1]},
-    'CharmRevisionUpdater': {'versions': [2]},
-    'CharmDownloader': {'versions': [1]},
-    'Charms': {'versions': [5, 6]},
-    'Cleaner': {'versions': [2]},
-    'Client': {'versions': [5, 6]},
-    'Cloud': {'versions': [1, 2, 3, 4, 5, 7]},
-    'Controller': {'versions': [9, 11]},
-    'CrossModelRelations': {'versions': [1, 2]},
-    'CrossModelSecrets': {'versions': [1]},
-    'CrossController': {'versions': [1]},
-    'CredentialManager': {'versions': [1]},
-    'CredentialValidator': {'versions': [1, 2]},
+    'Bundle': {'versions': [6]},
     'CAASAdmission': {'versions': [1]},
-    'CAASAgent': {'versions': [1, 2]},
+    'CAASAgent': {'versions': [2]},
     'CAASApplication': {'versions': [1]},
     'CAASApplicationProvisioner': {'versions': [1]},
     'CAASFirewaller': {'versions': [1]},
-    'CAASFirewallerEmbedded': {'versions': [1]},
     'CAASFirewallerSidecar': {'versions': [1]},
-    'CAASModelOperator': {'versions': [1]},
     'CAASModelConfigManager': {'versions': [1]},
+    'CAASModelOperator': {'versions': [1]},
     'CAASOperator': {'versions': [1]},
     'CAASOperatorProvisioner': {'versions': [1]},
     'CAASOperatorUpgrader': {'versions': [1]},
-    'CAASUnitProvisioner': {'versions': [1, 2]},
+    'CAASUnitProvisioner': {'versions': [2]},
+    'CharmDownloader': {'versions': [1]},
+    'CharmRevisionUpdater': {'versions': [2]},
+    'Charms': {'versions': [6]},
+    'Cleaner': {'versions': [2]},
+    'Client': {'versions': [6, 7]},
+    'Cloud': {'versions': [7]},
+    'Controller': {'versions': [11]},
+    'CredentialManager': {'versions': [1]},
+    'CredentialValidator': {'versions': [2]},
+    'CrossController': {'versions': [1]},
+    'CrossModelRelations': {'versions': [2]},
+    'CrossModelSecrets': {'versions': [1]},
     'Deployer': {'versions': [1]},
     'DiskManager': {'versions': [2]},
     'EntityWatcher': {'versions': [2]},
     'EnvironUpgrader': {'versions': [1]},
     'ExternalControllerUpdater': {'versions': [1]},
     'FanConfigurer': {'versions': [1]},
     'FilesystemAttachmentsWatcher': {'versions': [2]},
-    'Firewaller': {'versions': [3, 4, 5, 7]},
     'FirewallRules': {'versions': [1]},
+    'Firewaller': {'versions': [7]},
     'HighAvailability': {'versions': [2]},
     'HostKeyReporter': {'versions': [1]},
-    'ImageManager': {'versions': [2]},
     'ImageMetadata': {'versions': [3]},
     'ImageMetadataManager': {'versions': [1]},
-    'InstanceMutater': {'versions': [2, 3]},
-    'InstancePoller': {'versions': [3, 4]},
+    'InstanceMutater': {'versions': [3]},
+    'InstancePoller': {'versions': [4]},
     'KeyManager': {'versions': [1]},
     'KeyUpdater': {'versions': [1]},
     'LeadershipService': {'versions': [2]},
     'LifeFlag': {'versions': [1]},
-    'Logger': {'versions': [1]},
     'LogForwarding': {'versions': [1]},
-    'Machiner': {'versions': [1, 2, 5]},
+    'Logger': {'versions': [1]},
     'MachineActions': {'versions': [1]},
-    'MachineManager': {'versions': [9, 10]},
+    'MachineManager': {'versions': [10]},
     'MachineUndertaker': {'versions': [1]},
-    'MeterStatus': {'versions': [1, 2]},
+    'Machiner': {'versions': [5]},
+    'MeterStatus': {'versions': [2]},
     'MetricsAdder': {'versions': [2]},
     'MetricsDebug': {'versions': [2]},
     'MetricsManager': {'versions': [1]},
     'MigrationFlag': {'versions': [1]},
-    'MigrationMaster': {'versions': [1, 3]},
+    'MigrationMaster': {'versions': [3]},
     'MigrationMinion': {'versions': [1]},
     'MigrationStatusWatcher': {'versions': [1]},
-    'MigrationTarget': {'versions': [1]},
-    'ModelConfig': {'versions': [1, 2, 3]},
-    'ModelGeneration': {'versions': [1, 2, 4]},
-    'ModelManager': {'versions': [2, 3, 4, 5, 9]},
+    'MigrationTarget': {'versions': [1, 2]},
+    'ModelConfig': {'versions': [3]},
+    'ModelGeneration': {'versions': [4]},
+    'ModelManager': {'versions': [9]},
     'ModelSummaryWatcher': {'versions': [1]},
     'ModelUpgrader': {'versions': [1]},
     'NotifyWatcher': {'versions': [1]},
@@ -108,45 +112,43 @@
     'PayloadsHookContext': {'versions': [1]},
     'Pinger': {'versions': [1]},
     'Provisioner': {'versions': [11]},
-    'ProxyUpdater': {'versions': [1, 2]},
-    'RaftLease': {'versions': [1, 2]},
+    'ProxyUpdater': {'versions': [2]},
+    'RaftLease': {'versions': [2]},
     'Reboot': {'versions': [2]},
     'RelationStatusWatcher': {'versions': [1]},
     'RelationUnitsWatcher': {'versions': [1]},
-    'RemoteRelations': {'versions': [1, 2]},
     'RemoteRelationWatcher': {'versions': [1]},
-    'Resources': {'versions': [1, 2, 3]},
+    'RemoteRelations': {'versions': [2]},
+    'Resources': {'versions': [3]},
     'ResourcesHookContext': {'versions': [1]},
-    'Resumer': {'versions': [2]},
     'RetryStrategy': {'versions': [1]},
-    'Secrets': {'versions': [1, 2]},
-    'SecretsManager': {'versions': [1, 2]},
+    'SSHClient': {'versions': [4]},
     'SecretBackends': {'versions': [1]},
     'SecretBackendsManager': {'versions': [1]},
     'SecretBackendsRotateWatcher': {'versions': [1]},
+    'Secrets': {'versions': [1, 2]},
     'SecretsDrain': {'versions': [1]},
+    'SecretsManager': {'versions': [1, 2]},
     'SecretsRevisionWatcher': {'versions': [1]},
-    'SecretsRotationWatcher': {'versions': [1]},
     'SecretsTriggerWatcher': {'versions': [1]},
     'Singular': {'versions': [2]},
     'Spaces': {'versions': [6]},
     'StatusHistory': {'versions': [2]},
-    'Storage': {'versions': [3, 4, 6]},
-    'StorageProvisioner': {'versions': [3, 4]},
+    'Storage': {'versions': [6]},
+    'StorageProvisioner': {'versions': [4]},
     'StringsWatcher': {'versions': [1]},
-    'Subnets': {'versions': [2, 4, 5]},
-    'SSHClient': {'versions': [1, 2, 3, 4]},
+    'Subnets': {'versions': [5]},
     'Undertaker': {'versions': [1]},
     'UnitAssigner': {'versions': [1]},
-    'Uniter': {'versions': [18]},
+    'Uniter': {'versions': [18, 19]},
+    'UpgradeSeries': {'versions': [3]},
+    'UpgradeSteps': {'versions': [2]},
     'Upgrader': {'versions': [1]},
-    'UpgradeSeries': {'versions': [1, 3]},
-    'UpgradeSteps': {'versions': [1, 2]},
-    'UserManager': {'versions': [1, 2, 3]},
+    'UserManager': {'versions': [3]},
     'UserSecretsDrain': {'versions': [1]},
     'UserSecretsManager': {'versions': [1]},
-    'VolumeAttachmentsWatcher': {'versions': [2]},
     'VolumeAttachmentPlansWatcher': {'versions': [1]},
+    'VolumeAttachmentsWatcher': {'versions': [2]},
 }
 
 

tests/validate/test_facades.py

@@ -0,0 +1,60 @@
+# Copyright 2023 Canonical Ltd.
+# Licensed under the Apache V2, see LICENCE file for details.
+
+import importlib
+from collections import defaultdict
+from pathlib import Path
+from typing import Dict, List, TypedDict
+
+import pytest
+
+from juju.client.connection import client_facades, excluded_facades
+
+
+class Versions(TypedDict, total=True):
+    versions: List[int]
+
+
+ClientFacades = Dict[str, Versions]
+
+
+@pytest.fixture
+def project_root(pytestconfig: pytest.Config) -> Path:
+    return pytestconfig.rootpath
+
+
+@pytest.fixture
+def generated_code_facades(project_root: Path) -> ClientFacades:
+    """Return a client_facades dictionary from generated code under project_root.
+
+    Iterates through all the generated files matching juju/client/_client*.py,
+    extracting facade types (those that have .name and .version properties).
+    Excludes facades in juju.client.connection.excluded_facades, as these are
+    manually marked as incompatible with the current version of python-libjuju.
+    """
+    facades: Dict[str, List[int]] = defaultdict(list)
+    for file in project_root.glob('juju/client/_client*.py'):
+        module = importlib.import_module(f'juju.client.{file.stem}')
+        for cls_name in dir(module):
+            cls = getattr(module, cls_name)
+            try:  # duck typing check for facade types
+                cls.name
+                cls.version
+            except AttributeError:
+                continue
+            if cls.version in excluded_facades.get(cls.name, []):
+                continue
+            facades[cls.name].append(cls.version)
+    return {name: {'versions': sorted(facades[name])} for name in sorted(facades)}
+
+
+def test_client_facades(project_root: Path, generated_code_facades: ClientFacades) -> None:
+    """Ensure that juju.client.connection.client_facades matches expected facades.
+
+    See generated_code_facades for how expected facades are computed.
+    """
+    assert {
+        k: v['versions'] for k, v in client_facades.items()
+    } == {
+        k: v['versions'] for k, v in generated_code_facades.items()
+    }

tox.ini

@@ -103,6 +103,10 @@ commands =
     pip install pylxd
     python -m pytest --tb native -ra -v -s {posargs:-m 'serial'}
 
+[testenv:validate]
+envdir = {toxworkdir}/validate
+commands = python -m pytest --tb native -ra -vv tests/validate
+
 [testenv:example]
 envdir = {toxworkdir}/py3
 commands = python {posargs}