sts-sponsors team mailing list archive

[Anton Troyanov <mp+439218@xxxxxxxxxxxxxxxxxx>]

Anton Troyanov has proposed merging ~troyanov/maas:fix-cli-imports into maas:master.

Commit message:
fix(cli): follow maascli import boundaries

Remove provisioningserver related import
Move maasserver.vault to a local import
Update utilities/check-imports for maascli

Resolves LP:1986590

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~troyanov/maas/+git/maas/+merge/439218
-- 
Your team MAAS Maintainers is requested to review the proposed merge of ~troyanov/maas:fix-cli-imports into maas:master.

diff --git a/src/maascli/cli.py b/src/maascli/cli.py
index 29e63f8..47a62c8 100644
--- a/src/maascli/cli.py
+++ b/src/maascli/cli.py
@@ -11,6 +11,8 @@ import pkgutil
 import sys
 from textwrap import fill
 
+from OpenSSL import crypto
+
 from apiclient.creds import convert_tuple_to_string
 from maascli.api import fetch_api_description
 from maascli.auth import (
@@ -18,7 +20,7 @@ from maascli.auth import (
     obtain_credentials,
     UnexpectedResponse,
 )
-from maascli.command import Command
+from maascli.command import Command, CommandError
 from maascli.config import ProfileConfig
 from maascli.init import (
     add_candid_options,
@@ -27,7 +29,6 @@ from maascli.init import (
     init_maas,
 )
 from maascli.utils import api_url, parse_docstring, safe_name
-from provisioningserver.certificates import check_certificate
 
 CERTS_DIR = Path("~/.maascli.certs").expanduser()
 
@@ -96,7 +97,10 @@ class cmd_login(Command):
         cacerts_path = None
         if options.cacerts is not None:
             cacerts = options.cacerts.read()
-            check_certificate(cacerts)
+            try:
+                crypto.load_certificate(crypto.FILETYPE_PEM, cacerts)
+            except crypto.Error:
+                raise CommandError("Invalid PEM material")
 
             if not CERTS_DIR.exists():
                 CERTS_DIR.mkdir()
diff --git a/src/maascli/snap.py b/src/maascli/snap.py
index a8b8068..58a6747 100644
--- a/src/maascli/snap.py
+++ b/src/maascli/snap.py
@@ -31,7 +31,6 @@ from maascli.init import (
     prompt_for_choices,
     read_input,
 )
-from maasserver.vault import prepare_wrapped_approle, VaultError
 
 ARGUMENTS = OrderedDict(
     [
@@ -568,6 +567,8 @@ def get_vault_settings(options) -> dict:
     if not options.vault_uri:
         return {}
 
+    from maasserver.vault import prepare_wrapped_approle, VaultError
+
     required_arguments = [
         "vault-approle-id",
         "vault-wrapped-token",
@@ -582,6 +583,7 @@ def get_vault_settings(options) -> dict:
         raise CommandError(
             f"Missing required vault arguments: {', '.join(missing_arguments)}"
         )
+
     try:
         secret_id = prepare_wrapped_approle(
             url=options.vault_uri,
diff --git a/src/maascli/tests/test_snap.py b/src/maascli/tests/test_snap.py
index a4fad9e..b3679f3 100644
--- a/src/maascli/tests/test_snap.py
+++ b/src/maascli/tests/test_snap.py
@@ -19,6 +19,7 @@ import pytest
 from maascli import snap
 from maascli.command import CommandError
 from maascli.parser import ArgumentParser
+import maasserver.vault
 from maasserver.vault import VaultError
 from maastesting.factory import factory
 from maastesting.testcase import MAASTestCase
@@ -576,7 +577,7 @@ class TestCmdInit(MAASTestCase):
             ]
         )
 
-        prepare_mock = self.patch(snap, "prepare_wrapped_approle")
+        prepare_mock = self.patch(maasserver.vault, "prepare_wrapped_approle")
         prepare_mock.return_value = secret_id
 
         assert snap.get_vault_settings(options) == {
@@ -619,7 +620,7 @@ class TestCmdInit(MAASTestCase):
             ]
         )
 
-        prepare_mock = self.patch(snap, "prepare_wrapped_approle")
+        prepare_mock = self.patch(maasserver.vault, "prepare_wrapped_approle")
         prepare_mock.return_value = secret_id
 
         assert snap.get_vault_settings(options) == {
@@ -658,7 +659,7 @@ class TestCmdInit(MAASTestCase):
             ]
         )
 
-        prepare_mock = self.patch(snap, "prepare_wrapped_approle")
+        prepare_mock = self.patch(maasserver.vault, "prepare_wrapped_approle")
         prepare_mock.side_effect = [VaultError()]
 
         self.assertRaises(CommandError, snap.get_vault_settings, options)
@@ -684,7 +685,7 @@ class TestCmdInit(MAASTestCase):
             ]
         )
 
-        prepare_mock = self.patch(snap, "prepare_wrapped_approle")
+        prepare_mock = self.patch(maasserver.vault, "prepare_wrapped_approle")
         exc = factory.make_exception()
         prepare_mock.side_effect = [exc]
         self.assertRaises(type(exc), snap.get_vault_settings, options)
diff --git a/src/provisioningserver/certificates.py b/src/provisioningserver/certificates.py
index 8cbf6e3..74dd330 100644
--- a/src/provisioningserver/certificates.py
+++ b/src/provisioningserver/certificates.py
@@ -203,11 +203,3 @@ def get_maas_cert_tuple():
     if not private_key.exists() or not certificate.exists():
         return None
     return str(certificate), str(private_key)
-
-
-def check_certificate(material):
-    """Check if certificate is a valid PEM format certificate"""
-    try:
-        crypto.load_certificate(crypto.FILETYPE_PEM, material)
-    except crypto.Error:
-        raise CertificateError("Invalid PEM material")
diff --git a/utilities/check-imports b/utilities/check-imports
index 5884de4..1b70608 100755
--- a/utilities/check-imports
+++ b/utilities/check-imports
@@ -180,6 +180,8 @@ def files(*patterns):
 
 
 APIClient = files("src/apiclient/**/*.py")
+MAASCLI = files("src/maascli/**/*.py")
+
 
 PerfTestHarness = files("src/maastesting/perftest.py")
 PerfTestMigrations = files("src/maastesting/migrations/**/*.py")
@@ -387,6 +389,25 @@ checks = [
         ),
     ),
     #
+    # MAAS CLI
+    #
+    (
+        MAASCLI - Tests,
+        Rule(
+            Allow("maascli|maascli.**"),
+            Allow("apiclient.creds.*"),
+            Allow("apiclient.**"),
+            Allow("httplib2"),
+            Allow("macaroonbakery|macaroonbakery.**"),
+            Allow("netifaces|netifaces.*"),
+            Allow("OpenSSL|OpenSSL.**"),
+            Allow("psycopg2|psycopg2.**"),
+            Allow("tempita"),
+            Allow("yaml"),
+            Allow(StandardLibraries),
+        ),
+    ),
+    #
     # PERF TEST HARNESS
     #
     (