Merge ~troyanov/maas:backport-1f79650-3.3 into maas:3.3

Proposed by Anton Troyanov
Status: Merged
Approved by: Anton Troyanov
Approved revision: 8778774187326d4433f17fcb06dc18d8fc56661e
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~troyanov/maas:backport-1f79650-3.3
Merge into: maas:3.3
Diff against target: 174 lines (+36/-16)
5 files modified
src/maascli/cli.py (+7/-3)
src/maascli/snap.py (+3/-1)
src/maascli/tests/test_snap.py (+5/-4)
src/provisioningserver/certificates.py (+0/-8)
utilities/check-imports (+21/-0)
Reviewer Review Type Date Requested Status
Anton Troyanov Approve
MAAS Lander Approve
Review via email: mp+439282@code.launchpad.net

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

(cherry picked from commit 1f7965043fb2c1586f7d4ba17d3645ffa5a0f8e2)

To post a comment you must log in.
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b backport-1f79650-3.3 lp:~troyanov/maas/+git/maas into -b 3.3 lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 8778774187326d4433f17fcb06dc18d8fc56661e

review: Approve
Revision history for this message
Anton Troyanov (troyanov) wrote :

Self approve backport to 3.3

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maascli/cli.py b/src/maascli/cli.py
2index 29e63f8..47a62c8 100644
3--- a/src/maascli/cli.py
4+++ b/src/maascli/cli.py
5@@ -11,6 +11,8 @@ import pkgutil
6 import sys
7 from textwrap import fill
8
9+from OpenSSL import crypto
10+
11 from apiclient.creds import convert_tuple_to_string
12 from maascli.api import fetch_api_description
13 from maascli.auth import (
14@@ -18,7 +20,7 @@ from maascli.auth import (
15 obtain_credentials,
16 UnexpectedResponse,
17 )
18-from maascli.command import Command
19+from maascli.command import Command, CommandError
20 from maascli.config import ProfileConfig
21 from maascli.init import (
22 add_candid_options,
23@@ -27,7 +29,6 @@ from maascli.init import (
24 init_maas,
25 )
26 from maascli.utils import api_url, parse_docstring, safe_name
27-from provisioningserver.certificates import check_certificate
28
29 CERTS_DIR = Path("~/.maascli.certs").expanduser()
30
31@@ -96,7 +97,10 @@ class cmd_login(Command):
32 cacerts_path = None
33 if options.cacerts is not None:
34 cacerts = options.cacerts.read()
35- check_certificate(cacerts)
36+ try:
37+ crypto.load_certificate(crypto.FILETYPE_PEM, cacerts)
38+ except crypto.Error:
39+ raise CommandError("Invalid PEM material")
40
41 if not CERTS_DIR.exists():
42 CERTS_DIR.mkdir()
43diff --git a/src/maascli/snap.py b/src/maascli/snap.py
44index a8b8068..58a6747 100644
45--- a/src/maascli/snap.py
46+++ b/src/maascli/snap.py
47@@ -31,7 +31,6 @@ from maascli.init import (
48 prompt_for_choices,
49 read_input,
50 )
51-from maasserver.vault import prepare_wrapped_approle, VaultError
52
53 ARGUMENTS = OrderedDict(
54 [
55@@ -568,6 +567,8 @@ def get_vault_settings(options) -> dict:
56 if not options.vault_uri:
57 return {}
58
59+ from maasserver.vault import prepare_wrapped_approle, VaultError
60+
61 required_arguments = [
62 "vault-approle-id",
63 "vault-wrapped-token",
64@@ -582,6 +583,7 @@ def get_vault_settings(options) -> dict:
65 raise CommandError(
66 f"Missing required vault arguments: {', '.join(missing_arguments)}"
67 )
68+
69 try:
70 secret_id = prepare_wrapped_approle(
71 url=options.vault_uri,
72diff --git a/src/maascli/tests/test_snap.py b/src/maascli/tests/test_snap.py
73index 00f5262..7bbb167 100644
74--- a/src/maascli/tests/test_snap.py
75+++ b/src/maascli/tests/test_snap.py
76@@ -19,6 +19,7 @@ import pytest
77 from maascli import snap
78 from maascli.command import CommandError
79 from maascli.parser import ArgumentParser
80+import maasserver.vault
81 from maasserver.vault import VaultError
82 from maastesting.factory import factory
83 from maastesting.testcase import MAASTestCase
84@@ -577,7 +578,7 @@ class TestCmdInit(MAASTestCase):
85 ]
86 )
87
88- prepare_mock = self.patch(snap, "prepare_wrapped_approle")
89+ prepare_mock = self.patch(maasserver.vault, "prepare_wrapped_approle")
90 prepare_mock.return_value = secret_id
91
92 assert snap.get_vault_settings(options) == {
93@@ -620,7 +621,7 @@ class TestCmdInit(MAASTestCase):
94 ]
95 )
96
97- prepare_mock = self.patch(snap, "prepare_wrapped_approle")
98+ prepare_mock = self.patch(maasserver.vault, "prepare_wrapped_approle")
99 prepare_mock.return_value = secret_id
100
101 assert snap.get_vault_settings(options) == {
102@@ -659,7 +660,7 @@ class TestCmdInit(MAASTestCase):
103 ]
104 )
105
106- prepare_mock = self.patch(snap, "prepare_wrapped_approle")
107+ prepare_mock = self.patch(maasserver.vault, "prepare_wrapped_approle")
108 prepare_mock.side_effect = [VaultError()]
109
110 self.assertRaises(CommandError, snap.get_vault_settings, options)
111@@ -685,7 +686,7 @@ class TestCmdInit(MAASTestCase):
112 ]
113 )
114
115- prepare_mock = self.patch(snap, "prepare_wrapped_approle")
116+ prepare_mock = self.patch(maasserver.vault, "prepare_wrapped_approle")
117 exc = factory.make_exception()
118 prepare_mock.side_effect = [exc]
119 self.assertRaises(type(exc), snap.get_vault_settings, options)
120diff --git a/src/provisioningserver/certificates.py b/src/provisioningserver/certificates.py
121index 8cbf6e3..74dd330 100644
122--- a/src/provisioningserver/certificates.py
123+++ b/src/provisioningserver/certificates.py
124@@ -203,11 +203,3 @@ def get_maas_cert_tuple():
125 if not private_key.exists() or not certificate.exists():
126 return None
127 return str(certificate), str(private_key)
128-
129-
130-def check_certificate(material):
131- """Check if certificate is a valid PEM format certificate"""
132- try:
133- crypto.load_certificate(crypto.FILETYPE_PEM, material)
134- except crypto.Error:
135- raise CertificateError("Invalid PEM material")
136diff --git a/utilities/check-imports b/utilities/check-imports
137index fb59c92..401e068 100755
138--- a/utilities/check-imports
139+++ b/utilities/check-imports
140@@ -180,6 +180,8 @@ def files(*patterns):
141
142
143 APIClient = files("src/apiclient/**/*.py")
144+MAASCLI = files("src/maascli/**/*.py")
145+
146
147 PerfTestHarness = files("src/maastesting/perftest.py")
148 PerfTestMigrations = files("src/maastesting/migrations/**/*.py")
149@@ -387,6 +389,25 @@ checks = [
150 ),
151 ),
152 #
153+ # MAAS CLI
154+ #
155+ (
156+ MAASCLI - Tests,
157+ Rule(
158+ Allow("maascli|maascli.**"),
159+ Allow("apiclient.creds.*"),
160+ Allow("apiclient.**"),
161+ Allow("httplib2"),
162+ Allow("macaroonbakery|macaroonbakery.**"),
163+ Allow("netifaces|netifaces.*"),
164+ Allow("OpenSSL|OpenSSL.**"),
165+ Allow("psycopg2|psycopg2.**"),
166+ Allow("tempita"),
167+ Allow("yaml"),
168+ Allow(StandardLibraries),
169+ ),
170+ ),
171+ #
172 # PERF TEST HARNESS
173 #
174 (

Subscribers

People subscribed via source and target branches