Skip to content

Commit 004c2f1

Browse files
sontekJohn Anderson
andauthored
fix: Make URL and Prefix mutually exclusive (#77)
### what Make URL and Prefix mutually exclusive. Don't return exit code zero if things failed. ### why If you passed both at the same time `prefix` would override the URL, so for example if you said `--url=console.dev.stacklet.dev` but then also said `--prefix=dev` it would ignore url and send you to `console.dev.stacklet.io` which is confusing. ### testing Added many tests ### docs The CLI has `--help` --------- Co-authored-by: John Anderson <john@stacklet.io>
1 parent 16909fe commit 004c2f1

2 files changed

Lines changed: 279 additions & 29 deletions

File tree

‎stacklet/client/platform/cli.py‎

Lines changed: 38 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,11 @@ def configure(
128128
@cli.command()
129129
@click.option(
130130
"--url",
131+
help="A Stacklet console URL",
132+
)
133+
@click.option(
131134
"--prefix",
132-
required=True,
133-
help="A Stacklet console URL or deployment prefix",
135+
help="A deployment prefix (assumes hosting at .stacklet.io)",
134136
)
135137
@click.option(
136138
"--idp",
@@ -142,7 +144,7 @@ def configure(
142144
type=click.Path(path_type=pathlib.Path, dir_okay=False),
143145
show_default=True,
144146
)
145-
def auto_configure(url, idp, location):
147+
def auto_configure(url, prefix, idp, location):
146148
"""Automatically configure the stacklet-admin CLI
147149
148150
Fetch configuration details from a live Stacklet instance and use it
@@ -163,23 +165,36 @@ def auto_configure(url, idp, location):
163165
# Using a deployment prefix (assumes hosting at .stacklet.io)
164166
> stacklet-admin auto-configure --prefix myorg
165167
"""
166-
parts = urlsplit(url, scheme="https", allow_fragments=False)
167-
host = parts.netloc or parts.path
168+
# Validate mutual exclusion
169+
if url and prefix:
170+
raise click.ClickException(
171+
"Cannot specify both --url and --prefix. Please provide one or the other."
172+
)
173+
174+
if not url and not prefix:
175+
raise click.ClickException("Must specify either --url or --prefix.")
168176

169-
if "." not in host:
170-
# Assume that we have a prefix for a Stacklet instance
171-
# hosted under stacklet.io
172-
host = f"console.{host}.stacklet.io"
173-
if not host.startswith("console"):
174-
# Be forgiving if we get a base URL like customer.stacklet.io
175-
host = f"console.{host}"
177+
# Determine the host based on input type
178+
if prefix:
179+
# Handle prefix: always generate .stacklet.io URL
180+
host = f"console.{prefix}.stacklet.io"
181+
scheme = "https"
182+
else:
183+
# Handle URL: parse and potentially add console prefix
184+
parts = urlsplit(url, scheme="https", allow_fragments=False)
185+
host = parts.netloc or parts.path
186+
scheme = parts.scheme
187+
188+
if not host.startswith("console"):
189+
# Be forgiving if we get a base URL like customer.stacklet.io
190+
host = f"console.{host}"
176191

177192
config = {}
178193
try:
179194
for config_path in ("config/cognito.json", "config/cubejs.json"):
180195
config_url = urlunsplit(
181196
(
182-
parts.scheme,
197+
scheme,
183198
host,
184199
config_path,
185200
None,
@@ -190,17 +205,15 @@ def auto_configure(url, idp, location):
190205
response.raise_for_status()
191206
config.update(response.json())
192207
except requests.exceptions.ConnectionError as err:
193-
click.echo(f"Unable to connect to {config_url}")
194-
click.echo(err)
195-
return
208+
raise click.ClickException(f"Unable to connect to {config_url}\n{err}")
196209
except requests.exceptions.HTTPError as err:
197-
click.echo(f"Unable to retrieve configuration details from {config_url}")
198-
click.echo(err)
199-
return
210+
raise click.ClickException(
211+
f"Unable to retrieve configuration details from {config_url}\n{err}"
212+
)
200213
except requests.exceptions.JSONDecodeError as err:
201-
click.echo(f"Unable to parse configuration details from {config_url}")
202-
click.echo(err)
203-
return
214+
raise click.ClickException(
215+
f"Unable to parse configuration details from {config_url}\n{err}"
216+
)
204217

205218
try:
206219
auth_url = f"https://{config['cognito_install']}"
@@ -223,23 +236,19 @@ def auto_configure(url, idp, location):
223236
_, idp_id = name_to_id.popitem()
224237
else:
225238
if not idp:
226-
click.echo(
239+
raise click.ClickException(
227240
"Multiple identity providers available, specify one with --idp: "
228241
+ ", ".join(sorted(name_to_id))
229242
)
230-
return
231243
idp_id = name_to_id.get(idp)
232244
if not idp_id:
233-
click.echo(
245+
raise click.ClickException(
234246
f"Unknown identity provider '{idp}', known names: "
235247
+ ", ".join(sorted(name_to_id))
236248
)
237-
return
238249
formatted_config["idp_id"] = idp_id
239250
except KeyError as err:
240-
click.echo("The configuration details are missing a required key")
241-
click.echo(err)
242-
return
251+
raise click.ClickException(f"The configuration details are missing a required key: {err}")
243252

244253
config_file = location.expanduser()
245254
if not config_file.exists():

‎tests/test_cli.py‎

Lines changed: 241 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
import textwrap
77
from tempfile import NamedTemporaryFile
88

9+
import requests
10+
import requests_mock
11+
912
from .utils import BaseCliTest
1013

1114

@@ -67,3 +70,241 @@ def test_admin_save_config_and_show(self):
6770
)
6871

6972
os.unlink(file_location.name)
73+
74+
def test_auto_configure_url_only(self):
75+
"""Test auto-configure with --url parameter"""
76+
file_location = NamedTemporaryFile(mode="w+", delete=False)
77+
78+
# Mock the HTTP responses for config endpoints
79+
mock_config = {
80+
"cognito_install": "auth.console.dev.stacklet.dev",
81+
"cognito_user_pool_region": "us-east-2",
82+
"cognito_user_pool_id": "us-east-2_test123",
83+
"cognito_user_pool_client_id": "test-client-id",
84+
"cubejs_domain": "cubejs.dev.stacklet.dev",
85+
"saml_providers": [{"name": "TestIDP", "idp_id": "test-idp-id"}],
86+
}
87+
88+
with requests_mock.Mocker() as m:
89+
# Mock both config endpoints
90+
m.get("https://console.dev.stacklet.dev/config/cognito.json", json=mock_config)
91+
m.get("https://console.dev.stacklet.dev/config/cubejs.json", json={})
92+
93+
res = self.runner.invoke(
94+
self.cli,
95+
[
96+
"auto-configure",
97+
"--url=console.dev.stacklet.dev",
98+
f"--location={file_location.name}",
99+
],
100+
)
101+
102+
self.assertEqual(res.exit_code, 0)
103+
104+
# Verify the config was saved correctly
105+
with open(file_location.name, "r") as f:
106+
config = json.load(f)
107+
108+
self.assertEqual(config["api"], "https://api.dev.stacklet.dev")
109+
self.assertEqual(config["auth_url"], "https://auth.console.dev.stacklet.dev")
110+
self.assertEqual(config["cognito_client_id"], "test-client-id")
111+
self.assertEqual(config["cognito_user_pool_id"], "us-east-2_test123")
112+
self.assertEqual(config["region"], "us-east-2")
113+
self.assertEqual(config["cubejs"], "https://cubejs.dev.stacklet.dev")
114+
self.assertEqual(config["idp_id"], "test-idp-id")
115+
116+
os.unlink(file_location.name)
117+
118+
def test_auto_configure_prefix_only(self):
119+
"""Test auto-configure with --prefix parameter"""
120+
file_location = NamedTemporaryFile(mode="w+", delete=False)
121+
122+
mock_config = {
123+
"cognito_install": "auth.console.myorg.stacklet.io",
124+
"cognito_user_pool_region": "us-west-2",
125+
"cognito_user_pool_id": "us-west-2_test456",
126+
"cognito_user_pool_client_id": "test-client-id-2",
127+
"cubejs_domain": "cubejs.myorg.stacklet.io",
128+
"saml_providers": [{"name": "OrgIDP", "idp_id": "org-idp-id"}],
129+
}
130+
131+
with requests_mock.Mocker() as m:
132+
m.get("https://console.myorg.stacklet.io/config/cognito.json", json=mock_config)
133+
m.get("https://console.myorg.stacklet.io/config/cubejs.json", json={})
134+
135+
res = self.runner.invoke(
136+
self.cli,
137+
[
138+
"auto-configure",
139+
"--prefix=myorg",
140+
f"--location={file_location.name}",
141+
],
142+
)
143+
144+
self.assertEqual(res.exit_code, 0)
145+
146+
with open(file_location.name, "r") as f:
147+
config = json.load(f)
148+
149+
self.assertEqual(config["api"], "https://api.myorg.stacklet.io")
150+
self.assertEqual(config["auth_url"], "https://auth.console.myorg.stacklet.io")
151+
self.assertEqual(config["cubejs"], "https://cubejs.myorg.stacklet.io")
152+
153+
os.unlink(file_location.name)
154+
155+
def test_auto_configure_url_without_console_prefix(self):
156+
"""Test auto-configure with URL that needs console prefix added"""
157+
file_location = NamedTemporaryFile(mode="w+", delete=False)
158+
159+
mock_config = {
160+
"cognito_install": "auth.console.example.stacklet.io",
161+
"cognito_user_pool_region": "eu-west-1",
162+
"cognito_user_pool_id": "eu-west-1_test789",
163+
"cognito_user_pool_client_id": "test-client-id-3",
164+
"cubejs_domain": "cubejs.example.stacklet.io",
165+
"saml_providers": [{"name": "ExampleIDP", "idp_id": "example-idp-id"}],
166+
}
167+
168+
with requests_mock.Mocker() as m:
169+
m.get("https://console.example.stacklet.io/config/cognito.json", json=mock_config)
170+
m.get("https://console.example.stacklet.io/config/cubejs.json", json={})
171+
172+
res = self.runner.invoke(
173+
self.cli,
174+
[
175+
"auto-configure",
176+
"--url=example.stacklet.io", # Missing console prefix
177+
f"--location={file_location.name}",
178+
],
179+
)
180+
181+
self.assertEqual(res.exit_code, 0)
182+
183+
# Verify it added the console prefix and connected correctly
184+
with open(file_location.name, "r") as f:
185+
config = json.load(f)
186+
187+
self.assertEqual(config["region"], "eu-west-1")
188+
self.assertEqual(config["cognito_user_pool_id"], "eu-west-1_test789")
189+
190+
os.unlink(file_location.name)
191+
192+
def test_auto_configure_both_url_and_prefix_error(self):
193+
"""Test that providing both --url and --prefix results in an error"""
194+
res = self.runner.invoke(
195+
self.cli,
196+
[
197+
"auto-configure",
198+
"--url=console.dev.stacklet.dev",
199+
"--prefix=dev",
200+
],
201+
)
202+
203+
self.assertEqual(res.exit_code, 1) # Command should exit with error code
204+
self.assertIn("Cannot specify both --url and --prefix", res.output)
205+
206+
def test_auto_configure_neither_url_nor_prefix_error(self):
207+
"""Test that providing neither --url nor --prefix results in an error"""
208+
res = self.runner.invoke(
209+
self.cli,
210+
[
211+
"auto-configure",
212+
],
213+
)
214+
215+
self.assertEqual(res.exit_code, 1) # Command should exit with error code
216+
self.assertIn("Must specify either --url or --prefix", res.output)
217+
218+
def test_auto_configure_connection_error(self):
219+
"""Test auto-configure behavior when connection fails"""
220+
file_location = NamedTemporaryFile(mode="w+", delete=False)
221+
222+
with requests_mock.Mocker() as m:
223+
# Mock connection error
224+
m.get(
225+
"https://console.nonexistent.stacklet.dev/config/cognito.json",
226+
exc=requests.exceptions.ConnectionError,
227+
)
228+
229+
res = self.runner.invoke(
230+
self.cli,
231+
[
232+
"auto-configure",
233+
"--url=console.nonexistent.stacklet.dev",
234+
f"--location={file_location.name}",
235+
],
236+
)
237+
238+
self.assertEqual(res.exit_code, 1) # Command should exit with error code
239+
self.assertIn("Unable to connect to", res.output)
240+
241+
os.unlink(file_location.name)
242+
243+
def test_auto_configure_multiple_idps_with_selection(self):
244+
"""Test auto-configure with multiple IDPs and explicit selection"""
245+
file_location = NamedTemporaryFile(mode="w+", delete=False)
246+
247+
mock_config = {
248+
"cognito_install": "auth.console.multi.stacklet.dev",
249+
"cognito_user_pool_region": "us-east-1",
250+
"cognito_user_pool_id": "us-east-1_multi123",
251+
"cognito_user_pool_client_id": "multi-client-id",
252+
"cubejs_domain": "cubejs.multi.stacklet.dev",
253+
"saml_providers": [
254+
{"name": "IDP1", "idp_id": "idp-1"},
255+
{"name": "IDP2", "idp_id": "idp-2"},
256+
],
257+
}
258+
259+
with requests_mock.Mocker() as m:
260+
m.get("https://console.multi.stacklet.dev/config/cognito.json", json=mock_config)
261+
m.get("https://console.multi.stacklet.dev/config/cubejs.json", json={})
262+
263+
res = self.runner.invoke(
264+
self.cli,
265+
[
266+
"auto-configure",
267+
"--url=console.multi.stacklet.dev",
268+
"--idp=IDP2",
269+
f"--location={file_location.name}",
270+
],
271+
)
272+
273+
self.assertEqual(res.exit_code, 0)
274+
275+
with open(file_location.name, "r") as f:
276+
config = json.load(f)
277+
278+
self.assertEqual(config["idp_id"], "idp-2") # Should use the selected IDP
279+
280+
os.unlink(file_location.name)
281+
282+
def test_auto_configure_multiple_idps_no_selection_error(self):
283+
"""Test auto-configure with multiple IDPs but no selection results in error"""
284+
mock_config = {
285+
"cognito_install": "auth.console.multi.stacklet.dev",
286+
"cognito_user_pool_region": "us-east-1",
287+
"cognito_user_pool_id": "us-east-1_multi123",
288+
"cognito_user_pool_client_id": "multi-client-id",
289+
"cubejs_domain": "cubejs.multi.stacklet.dev",
290+
"saml_providers": [
291+
{"name": "IDP1", "idp_id": "idp-1"},
292+
{"name": "IDP2", "idp_id": "idp-2"},
293+
],
294+
}
295+
296+
with requests_mock.Mocker() as m:
297+
m.get("https://console.multi.stacklet.dev/config/cognito.json", json=mock_config)
298+
m.get("https://console.multi.stacklet.dev/config/cubejs.json", json={})
299+
300+
res = self.runner.invoke(
301+
self.cli,
302+
[
303+
"auto-configure",
304+
"--url=console.multi.stacklet.dev",
305+
# No --idp specified
306+
],
307+
)
308+
309+
self.assertEqual(res.exit_code, 1) # Command should exit with error code
310+
self.assertIn("Multiple identity providers available", res.output)

0 commit comments

Comments
 (0)