Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GCS/GCP Subscription ID Management #921

Merged
merged 9 commits into from
Jan 4, 2024
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@

### Enhancements

* `[sc-28587] <https://app.shortcut.com/globus/story/28587>`_
Added a new command `globus gcs endpoint set-subscription-id` which allows
subscription managers and endpoint admins to modify the subscription ID for a
GCS endpoint.

* `[sc-28587] <https://app.shortcut.com/globus/story/28587>`_
Added a new command `globus gcp set-subscription-id` which allows subscription
managers and collection admins to modify the subscription ID for a GCP collection.
4 changes: 4 additions & 0 deletions src/globus_cli/commands/gcp/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
lazy_subcommands={
"create": (".create", "create_command"),
"update": (".update", "update_command"),
"set-subscription-id": (
"endpoint.set_subscription_id",
"set_endpoint_subscription_id",
),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with this as a short-term position, but I'd like us to write a new command for this, rather than reusing the existing one.

My reason is primarily helptext and parameter names. If you look at the existing command, you'll see it states

Unlike the '--managed' flag for 'globus endpoint update' ...

Which is not something we want to retain into the future.

I think it's also worth having the new command check to confirm that it's operating on a GCP_mapped_collection rather than a GCP_guest_collection. That way, we more clearly explain to users that it's only possible to use the relevant command on a GCP host, not the shares underneath that host.

I'm perfectly happy for us to pursue that change as a fast-follow to this PR though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially did write a new command for GCP but erased it when it ended up being identical to this one.


If we do add a duplicate code path, it's worth noting that introduces the risk of drift (one path gets updated but not the other).

If it's just a helptext concern, I don't think that's worth a copy/paste. If it's adding additional validation of mapped vs guest collection types; I could buy it. It's a fairly small copy/paste.


I'll include that in a new rev of this PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not that concerned about the paths being duplicated, since I think they are actually different. The question IMO is only how much it's worth working hard to avoid code duplication.

I want the following:

  • GCP-specific checks in the GCP command
  • legacy-specific deprecations in the legacy command
  • different helptext for the two

The worry about drift would be more pronounced if this didn't take the form of a deprecation. However, I don't think we want to keep globus endpoint (the whole tree), so we should be making moves to deprecate its various pieces.

},
)
def gcp_command() -> None:
Expand Down
5 changes: 4 additions & 1 deletion src/globus_cli/commands/gcs/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@
"gcs",
lazy_subcommands={
"collection": ("collection", "collection_command"),
# Note: endpoint is not an alias for the root 'endpoint' group as that group is
# broken up into slightly different subcommand structures here.
"endpoint": (".endpoint", "endpoint_command"),
"storage-gateway": ("endpoint.storage_gateway", "storage_gateway_command"),
"user-credential": ("endpoint.user_credential", "user_credential_command"),
},
)
def gcs_command() -> None:
"""Manage Globus Connect Server endpoints"""
"""Manage Globus Connect Server (GCS) resources"""
11 changes: 11 additions & 0 deletions src/globus_cli/commands/gcs/endpoint/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
from globus_cli.parsing import group


@group(
"endpoint",
lazy_subcommands={
"set-subscription-id": (".set_subscription_id", "set_subscription_id_command"),
},
)
def endpoint_command() -> None:
"""Manage Globus Connect Server (GCS) endpoints"""
73 changes: 73 additions & 0 deletions src/globus_cli/commands/gcs/endpoint/set_subscription_id.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
from __future__ import annotations

import typing as t
import uuid

import click

from globus_cli.constants import EXPLICIT_NULL, ExplicitNullType
from globus_cli.login_manager import LoginManager
from globus_cli.parsing import command, endpoint_id_arg
from globus_cli.termio import TextMode, display


class GCSSubscriptionIdType(click.ParamType):
def get_type_annotation(self, _: click.Parameter) -> type:
return t.cast(type, uuid.UUID | t.Literal["DEFAULT"] | ExplicitNullType)

def convert(
self, value: str, param: click.Parameter | None, ctx: click.Context | None
) -> t.Any:
if ctx and ctx.resilient_parsing:
return None

if value.lower() == "null":
return EXPLICIT_NULL
elif value.lower() == "default":
return "DEFAULT"
try:
uuid.UUID(value)
return value
except ValueError:
msg = (
f"{value} is invalid. Expected either a UUID or the special "
'values "DEFAULT" or "null"'
)
self.fail(msg, param, ctx)


@command("set-subscription-id", short_help="Update an endpoint's subscription")
derek-globus marked this conversation as resolved.
Show resolved Hide resolved
@endpoint_id_arg
@click.argument("SUBSCRIPTION_ID", type=GCSSubscriptionIdType())
@LoginManager.requires_login("transfer")
def set_subscription_id_command(
login_manager: LoginManager,
*,
endpoint_id: uuid.UUID,
subscription_id: uuid.UUID | t.Literal["DEFAULT"] | ExplicitNullType,
) -> None:
"""
Update an endpoint's subscription.

SUBSCRIPTION_ID must be one of: (1) A valid subscription ID (UUID), (2) the value
"DEFAULT" (requires that you manage exactly one subscription & assigns the endpoint
to that subscription), or (3) the value "null" (clears the endpoint's subscription)

Setting a subscription requires that you are a subscription manager for the
subscription being assigned.

Removing a subscription requires that you are either (1) a subscription manager for
the current assigned subscription group or (2) an admin of the endpoint.
"""
gcs_client = login_manager.get_gcs_client(endpoint_id=endpoint_id)

subscription_id_val = None if subscription_id is EXPLICIT_NULL else subscription_id
res = gcs_client.put(
"/endpoint/subscription_id",
data={
"DATA_TYPE": "endpoint_subscription#1.0.0",
"subscription_id": subscription_id_val,
},
)

display(res, text_mode=TextMode.text_raw, response_key="message")
Empty file.
Empty file.
64 changes: 64 additions & 0 deletions tests/functional/gcs/endpoint/test_set_subscription_id.py
derek-globus marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import uuid

import pytest
import responses
from globus_sdk._testing import load_response_set


@pytest.mark.parametrize("subscription_id", (str(uuid.uuid4()), "DEFAULT", "null"))
def test_gcs_endpoint_set_subscription_id(subscription_id, run_line, add_gcs_login):
meta = load_response_set("cli.collection_operations").metadata
endpoint_id = meta["endpoint_id"]
gcs_hostname = meta["gcs_hostname"]
add_gcs_login(endpoint_id)

responses.put(
f"https://{gcs_hostname}/api/endpoint/subscription_id",
json={
"DATA_TYPE": "result#1.0.0",
"code": "success",
"detail": "success",
"has_next_page": False,
"http_response_code": 200,
"message": f"Updated Endpoint {endpoint_id}",
},
)

result = run_line(
f"globus gcs endpoint set-subscription-id {endpoint_id} {subscription_id}"
)

assert f"Updated Endpoint {endpoint_id}" in result.stdout


def test_gcs_endpoint_set_subscription_id__when_not_subscription_manager(
kurtmckee marked this conversation as resolved.
Show resolved Hide resolved
run_line, add_gcs_login
):
meta = load_response_set("cli.collection_operations").metadata
endpoint_id = meta["endpoint_id"]
gcs_hostname = meta["gcs_hostname"]
add_gcs_login(endpoint_id)

error_message = (
"Unable to use DEFAULT subscription. Your identity does not manage any"
"subscriptions"
)
responses.put(
f"https://{gcs_hostname}/api/endpoint/subscription_id",
status=400,
json={
"DATA_TYPE": "result#1.0.0",
"code": "bad_request",
"detail": "bad_request",
"has_next_page": False,
"http_response_code": 400,
"message": error_message,
},
)

result = run_line(
f"globus gcs endpoint set-subscription-id {endpoint_id} DEFAULT",
assert_exit_code=1,
)

assert error_message in result.stderr