fix: update key mgmt flags to consistent format

[strantalis]

No description provided.

[gemini-code-assist]

Hello @strantalis, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

As Gemini, your code-assist, I've reviewed this pull request which aims to standardize the command-line flags used for managing KAS keys within the otdfctl CLI tool. The primary goal is to provide a more consistent and intuitive user experience by consolidating multiple flags for identifying keys and KAS instances into single, versatile flags (--key and --kas) and adopting a hyphenated format for other key-related parameters. This involves updates to the CLI parsing logic, the introduction of a utility function to classify identifier types, and corresponding updates to documentation and end-to-end tests.

Highlights

• Flag Standardization: Replaced disparate flags like --keyId, --kasId, --kasName, and --kasUri with standardized --key and --kas flags across relevant commands (create, get, list, base set).
• Intelligent KAS Identification: The new --kas flag now intelligently accepts a KAS identifier as either a UUID, a URI, or a Name, resolving it internally to the KAS ID needed for API calls.
• Consistent Flag Naming: Renamed other key-related flags, such as --wrappingKeyId, --providerConfigId, --pubPem, and --privatePem, to use a consistent hyphenated format (--wrapping-key-id, --provider-config-id, --public-key-pem, --private-key-pem).
• Utility Function for Classification: Introduced a new utility function utils.ClassifyString to determine if an input string is a UUID, URI, or generic string, used for parsing the --kas flag.
• Documentation and Test Updates: Updated the command documentation (man pages) and end-to-end tests to reflect the new flag names and usage patterns.

Changelog

Click here to see the changelog

• cmd/base-keys.go
  • Updated getKasKeyIdentifier to use --key and --kas flags.
  • Implemented logic to classify the --kas input using utils.ClassifyString.
• cmd/kas-keys.go
  • Updated policyCreateKasKey and policyListKasKeys to use the new flag format (--key, --kas, --wrapping-key-id, --provider-config-id, --public-key-pem, --private-key-pem, --algorithm).
  • Added resolveKasIdentifier function to handle KAS identifier resolution from the --kas flag.
• docs/man/policy/kas-registry/key/_index.md
  • Updated description of the key command group.
• docs/man/policy/kas-registry/key/base/_index.md
  • Updated title and description.
  • Removed redundant alias.
• docs/man/policy/kas-registry/key/base/get.md
  • Improved command description and added an example.
• docs/man/policy/kas-registry/key/base/set.md
  • Replaced multiple KAS/key identification flags with --key and --kas.
  • Updated flag descriptions and examples.
• docs/man/policy/kas-registry/key/create.md
  • Renamed flags to hyphenated format (--key-id, --algorithm, --wrapping-key-id, --wrapping-key, --private-key-pem, --provider-config-id, --public-key-pem).
  • Consolidated KAS identification flags into --kas.
  • Updated flag descriptions and examples.
• docs/man/policy/kas-registry/key/get.md
  • Replaced multiple KAS/key identification flags with --key and --kas.
  • Updated flag descriptions and examples.
• docs/man/policy/kas-registry/key/list.md
  • Replaced multiple KAS identification flags with --kas.
  • Renamed --alg to --algorithm.
  • Updated flag descriptions and examples.
• docs/man/policy/kas-registry/key/update.md
  • Updated title and description.
  • Improved flag descriptions and added an example.
• e2e/base-key.bats
  • Updated test commands and setup calls to use the new flag names (--key, --kas, --key-id, --algorithm, --wrapping-key-id).
  • Updated test names to reflect new flag usage.
  • Fixed shell pipe syntax.
• e2e/kas-keys.bats
  • Updated test commands and setup calls to use the new flag names (--kas, --key-id, --algorithm, --public-key-pem, --provider-config-id, --wrapping-key-id, --private-key-pem).
  • Updated test names to reflect new flag usage.
  • Fixed shell pipe syntax.
  • Updated assertions to match new flag names and logic.
• pkg/utils/identifier.go
  • Added new package utils with IdentifierStringType enum and ClassifyString function.
• pkg/utils/identifier_test.go
  • Added unit tests for the ClassifyString function.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

---

Flags aligned just right,
Key and kas, clear and bright,
Docs now make sense.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request makes good progress towards standardizing key management flags, which will improve CLI consistency. The new identifier.go utility is a positive step.

However, there are several critical and high-severity issues related to how flags are defined, registered in init() functions, and then used within the command logic. These inconsistencies are also reflected in the e2e tests and their expected outcomes. Resolving these is crucial for the CLI to function correctly and predictably.

The documentation updates are generally good and align with the intended new flag structure. Once the code-level inconsistencies are fixed, these documentation changes will be accurate.

Summary of Findings

• Critical Flag Inconsistencies: There are critical inconsistencies between how flags are registered in init() functions (often using old flag names and rules) and how they are consumed in the command logic (which expects new, consolidated flags like --kas and --key). This affects cmd/kas-keys.go (create, get, list) and cmd/base-keys.go (set) and is a primary source of issues seen in e2e tests.
• Incorrect Flag Name for Provider Config: In cmd/kas-keys.go, provider-config-pem is read for KEY_MODE_REMOTE, but provider-config-id is likely intended as per documentation and variable naming.
• Algorithm Flag Naming Standardization: The algorithm flag is inconsistently named (alg vs. algorithm) across commands, their init functions, and documentation. This should be standardized.
• E2E Test Typos and Misalignments: E2E tests contain typos (e.g., algorithmorithm) and expect behaviors based on the inconsistent flag setups, which need correction after the core Go code issues are resolved.
• Unreachable Code: A return "", nil statement in resolveKasIdentifier in cmd/kas-keys.go appears to be unreachable.

Merge Readiness

This pull request has the right goal of standardizing CLI flags for key management. The new identifier.go utility is a positive step.

However, due to the critical and high severity issues identified concerning inconsistent flag definitions, registration, and usage across different commands and their init blocks, I recommend that these issues be addressed before merging. These inconsistencies can lead to incorrect CLI behavior, confusing error messages for users, and make maintenance harder.

Once these core flag handling issues are resolved, the e2e tests should also be updated to reflect the correct and consistent flag usage. After these changes, the PR will be in a much better state for merging.

As an AI, I am not authorized to approve pull requests. Please ensure these changes are reviewed and approved by the appropriate maintainers.