Change command style for easier use & update docs

Author: psalajova

hack/gcp-secret-manager/commands/create.py

@@ -16,8 +16,9 @@
     get_secrets_from_index,
     update_index_secret,
     validate_collection,
-    validate_secret_name,
+    validate_path,
     validate_secret_source,
+    ensure_authentication,
 )
 
 # Metadata keys used when creating secrets:
@@ -41,14 +42,7 @@
     type=str,
     callback=validate_collection,
 )
-@click.option(
-    "-s",
-    "--secret",
-    required=True,
-    help="Name of the secret.",
-    type=str,
-    callback=validate_secret_name,
-)
+@click.argument("path", required=True, callback=validate_path)
 @click.option(
     "-f",
     "--from-file",
@@ -59,9 +53,16 @@
 @click.option(
     "-l", "--from-literal", default="", help="Secret data as string input.", type=str
 )
-def create(collection: str, secret: str, from_file: str, from_literal: str):
-    """Create a new secret in the specified collection."""
+def create(collection: str, path: str, from_file: str, from_literal: str):
+    """Create a new secret in the specified collection.
+
+    The secret PATH should be in the format 'group/field' where:
+    - group: Organizes related secrets (can be hierarchical: 'aws/prod')
+    - field: The specific secret name (e.g., 'username', 'password')
 
+    Example: secret-manager create -c my-collection aws/password -l "secret value"
+    """
+    ensure_authentication()
     validate_secret_source(from_file, from_literal)
 
     if not check_if_collection_exists(collection):
@@ -71,15 +72,20 @@ def create(collection: str, secret: str, from_file: str, from_literal: str):
             "See: https://docs.ci.openshift.org/docs/how-tos/adding-a-new-secret-to-ci/"
         )
     client = secretmanager.SecretManagerServiceClient()
-    secret_name = get_secret_name(collection, secret)
 
-    # Check if secret exists in either index or GCP
     index_secrets = get_secrets_from_index(client, collection)
-    if secret in index_secrets:
-        raise click.ClickException(f"Secret named '{secret}' already exists.")
+    path_normalized = path.replace("/", "__")
+    if path_normalized in index_secrets:
+        raise click.ClickException(
+            f"Secret '{path}' already exists."
+        )
+
+    secret_id_normalized = get_secret_name(collection, path)
     try:
-        client.get_secret(name=client.secret_path(PROJECT_ID, secret_name))
-        raise click.ClickException(f"Secret named '{secret}' already exists.")
+        client.get_secret(name=client.secret_path(PROJECT_ID, secret_id_normalized))
+        raise click.ClickException(
+            f"Secret '{path}' already exists."
+        )
     except NotFound:
         pass  # Secret doesn't exist in GCP - this is good
 
@@ -95,7 +101,7 @@ def create(collection: str, secret: str, from_file: str, from_literal: str):
         gcp_secret = client.create_secret(
             request={
                 "parent": f"projects/{PROJECT_ID}",
-                "secret_id": secret_name,
+                "secret_id": secret_id_normalized,
                 "secret": {
                     "replication": {"automatic": {}},
                     "labels": labels,
@@ -107,12 +113,12 @@ def create(collection: str, secret: str, from_file: str, from_literal: str):
             parent=gcp_secret.name,
             payload=SecretPayload(data=create_payload(from_file, from_literal)),
         )
-        update_index_secret(client, collection, index_secrets + [secret])
-        click.echo(f"Secret '{secret}' created")
+        update_index_secret(client, collection, index_secrets + [path_normalized])
+        click.echo(f"Secret '{path}' created successfully.")
     except Exception as e:
         raise click.ClickException(
-            f"Failed to create secret '{secret}': {e}. "
-            f"If the secret is in an inconsistent state, run 'delete -c {collection} -s {secret}', then try again."
+            f"Failed to create secret '{path}': {e}. "
+            f"If the secret is in an inconsistent state, run 'delete -c {collection} {path}', then try again."
         ) from e
 
 

hack/gcp-secret-manager/commands/delete.py

@@ -11,7 +11,7 @@
     get_secrets_from_index,
     update_index_secret,
     validate_collection,
-    validate_secret_name,
+    validate_path,
 )
 
 
@@ -24,40 +24,34 @@
     type=str,
     callback=validate_collection,
 )
-@click.option(
-    "-s",
-    "--secret",
-    required=True,
-    help="Name of the secret.",
-    type=str,
-    callback=validate_secret_name,
-)
-def delete(collection: str, secret: str):
-    """Delete a secret from the specified collection."""
+@click.argument("path", required=True, callback=validate_path)
+def delete(collection: str, path: str):
+    """Delete a secret from the specified collection.
+
+    The secret PATH format is 'group/field' (e.g., 'aws/password').
+    """
 
     ensure_authentication()
     client = secretmanager.SecretManagerServiceClient()
 
     index_secrets = get_secrets_from_index(client, collection)
-
-    # Remove from index if present (if missing from index, don't fail)
-    if secret in index_secrets:
-        index_secrets.remove(secret)
+    secret_id_normalized = path.replace("/", "__")
+    if secret_id_normalized in index_secrets:
+        index_secrets.remove(secret_id_normalized)
         update_index_secret(client, collection, index_secrets)
 
-    # Remove from GSM if present (don't fail if missing)
     try:
-        click.echo(f"Deleting secret '{secret}'...")
+        click.echo(f"Deleting secret '{path}'...")
         client.delete_secret(
-            name=client.secret_path(PROJECT_ID, get_secret_name(collection, secret))
+            name=client.secret_path(PROJECT_ID, get_secret_name(collection, path))
         )
     except NotFound:
         raise click.ClickException(
-            f"Secret '{secret}' does not exist within the collection."
+            f"Secret '{path}' does not exist within the collection."
         )
     except Exception as e:
         raise click.ClickException(
-            f"Failed to delete secret '{secret}': {e}. Please retry the delete operation."
+            f"Failed to delete secret '{path}': {e}. Please retry the delete operation."
         )
 
-    click.echo(f"Secret '{secret}' successfully deleted.")
+    click.echo(f"Secret '{path}' successfully deleted.")

hack/gcp-secret-manager/commands/list.py

@@ -30,19 +30,21 @@
     callback=validate_collection,
 )
 @click.option(
-    "-g",
-    "--group",
+    "--rover-group",
     default="",
-    help="Use this option to list all secret collections for a group.",
+    help="Use this option to list all secret collections for a rover group.",
 )
-def list_secrets(output: str, collection: str, group: str):
+def list_secrets(output: str, collection: str, rover_group: str):
     """
-    List secrets from the specified collection.
-    If no collection is provided, lists all secret collections.
+    List secrets.
+
+    Without options: Lists all secret collections that exist.
+    With -c {COLLECTION}: Lists all secrets in group/field format.
+    With --rover-group {GROUP}: Lists collections accessible to that rover group.
     """
-    if collection != "" and group != "":
+    if collection != "" and rover_group != "":
         raise click.ClickException(
-            "--collection and --group cannot both be set at the same time"
+            "--collection and --rover-group cannot both be set at the same time"
         )
 
     if collection != "":
@@ -51,8 +53,8 @@ def list_secrets(output: str, collection: str, group: str):
         return
 
     collections_dict = get_group_collections()
-    if group != "":
-        list_collections_for_group(collections_dict, group, output)
+    if rover_group != "":
+        list_collections_for_rover_group(collections_dict, rover_group, output)
     else:
         list_all_collections(collections_dict, output)
 
@@ -67,26 +69,43 @@ def list_all_collections(collections_dict: Dict, output: str):
                 click.echo(f"- {collection}")
 
 
-def list_collections_for_group(
-    collections_dict: Dict[str, List[str]], group: str, output: str
+def list_collections_for_rover_group(
+    collections_dict: Dict[str, List[str]], rover_group: str, output: str
 ):
-    if group and group not in collections_dict:
-        click.echo(f"Group '{group}' has no secret collections")
+    """Lists collections for a specified rover group."""
+
+    if rover_group and rover_group not in collections_dict:
+        click.echo(f"Rover group '{rover_group}' has no secret collections")
         return
 
     if output == "json":
-        click.echo(json.dumps(collections_dict[group], indent=2))
+        click.echo(json.dumps(collections_dict[rover_group], indent=2))
     else:
-        for collection in collections_dict[group]:
+        for collection in collections_dict[rover_group]:
             click.echo(f"{collection}")
 
 
 def list_secrets_for_collection(collection: str, output: str):
     secret_list = get_secrets_from_index(
         secretmanager.SecretManagerServiceClient(), collection
     )
+
+    paths = [secret.replace("__", "/") for secret in secret_list]
+    paths.sort()
+
     if output == "json":
-        click.echo(json.dumps(secret_list, indent=2))
+        click.echo(json.dumps(paths, indent=2))
     else:
-        for secret in secret_list:
-            click.echo(secret)
+        if not paths:
+            click.echo("(no secrets)")
+            return
+
+        last_top_group = None
+        for path in paths:
+            top_group = path.split("/")[0] if "/" in path else ""
+
+            if last_top_group is not None and top_group != last_top_group:
+                click.echo()
+
+            click.echo(path)
+            last_top_group = top_group

hack/gcp-secret-manager/commands/update.py

@@ -11,8 +11,8 @@
     get_secret_name,
     get_secrets_from_index,
     validate_collection,
-    validate_secret_name,
-    validate_secret_source,
+    validate_path,
+    validate_secret_source, ensure_authentication,
 )
 
 
@@ -25,14 +25,7 @@
     type=str,
     callback=validate_collection,
 )
-@click.option(
-    "-s",
-    "--secret",
-    required=True,
-    help="Name of the secret.",
-    type=str,
-    callback=validate_secret_name,
-)
+@click.argument("path", required=True, callback=validate_path)
 @click.option(
     "-f",
     "--from-file",
@@ -43,19 +36,21 @@
 @click.option(
     "-l", "--from-literal", default="", help="Secret data as string input.", type=str
 )
-def update(collection: str, secret: str, from_file: str, from_literal: str):
-    """Update an existing secret."""
+def update(collection: str, path: str, from_file: str, from_literal: str):
+    """Update an existing secret.
 
-    validate_secret_source(from_file, from_literal)
+    The secret PATH format is 'group/field' (e.g., 'aws/password').
+    """
 
+    ensure_authentication()
+    validate_secret_source(from_file, from_literal)
     client = secretmanager.SecretManagerServiceClient()
-    secret_name = get_secret_name(collection, secret)
-    full_secret_path = client.secret_path(PROJECT_ID, secret_name)
 
     # Check if secret exists in both index and GSM
+    path_normalized = path.replace("/", "__")
     index_secrets = get_secrets_from_index(client, collection)
-    secret_in_index = secret in index_secrets
-
+    secret_in_index = path_normalized in index_secrets
+    full_secret_path = client.secret_path(PROJECT_ID, get_secret_name(collection, path))
     try:
         client.get_secret(request={"name": full_secret_path})
         secret_in_gsm = True
@@ -65,31 +60,33 @@ def update(collection: str, secret: str, from_file: str, from_literal: str):
     # Simple existence checks - tell user to delete/recreate if inconsistent
     if not secret_in_index and not secret_in_gsm:
         raise click.ClickException(
-            f"Secret '{secret}' does not exist in collection '{collection}'."
+            f"Secret '{path}' does not exist in collection '{collection}'."
         )
 
     if secret_in_index and not secret_in_gsm:
         raise click.ClickException(
-            f"Secret '{secret}' is in inconsistent state. "
-            f"Run 'delete -c {collection} -s {secret}', then 'create' to fix."
+            f"Secret '{path}' is in inconsistent state. "
+            f"Run 'delete -c {collection} {path}', then 'create' to fix."
         )
 
     if not secret_in_index and secret_in_gsm:
         raise click.ClickException(
-            f"Secret '{secret}' is in inconsistent state (GSM only). "
-            f"Run 'delete -c {collection} -s {secret}', then 'create' to fix."
+            f"Secret '{path}' is in inconsistent state (GSM only). "
+            f"Run 'delete -c {collection} {path}', then 'create' to fix."
         )
 
-    # Secret exists in both places - proceed with update
+    # Secret exists in both places - proceed with the update
     try:
         client.add_secret_version(
             parent=full_secret_path,
             payload=SecretPayload(data=create_payload(from_file, from_literal)),
         )
-        click.echo(f"Secret '{secret}' updated successfully.")
+        click.echo(f"Secret '{path}' updated successfully.")
     except PermissionDenied:
         raise click.ClickException(
             f"You don't have permission to update secrets in collection '{collection}'"
         )
     except Exception as e:
-        raise click.ClickException(f"Failed to update secret '{secret}': {e}.")
+        raise click.ClickException(
+            f"Failed to update secret '{path}': {e}."
+        )