Skip to content

fix(cli): deprecate flaghelper for new flags#3583

Merged
jakedoublev merged 2 commits into
mainfrom
fix/deprecate-flaghelper-cli
Jun 8, 2026
Merged

fix(cli): deprecate flaghelper for new flags#3583
jakedoublev merged 2 commits into
mainfrom
fix/deprecate-flaghelper-cli

Conversation

@jakedoublev

@jakedoublev jakedoublev commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Proposed Changes

Checklist

  • I have added or updated unit tests
  • I have added or updated integration tests (if appropriate)
  • I have added or updated documentation

Testing Instructions

Summary by CodeRabbit

  • Refactor
    • Improved internal CLI flag handling infrastructure across authentication, policy, and profile commands for better code maintainability. All existing functionality and user-facing behavior remain unchanged.

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b3788302-6045-4030-b4f1-011373bfa29b

📥 Commits

Reviewing files that changed from the base of the PR and between 11af44a and f22e9d0.

📒 Files selected for processing (8)
  • otdfctl/cmd/auth/login.go
  • otdfctl/cmd/auth/logout.go
  • otdfctl/cmd/common/common.go
  • otdfctl/cmd/policy/attributeValues.go
  • otdfctl/cmd/policy/kasRegistry.go
  • otdfctl/cmd/profile.go
  • otdfctl/pkg/cli/cli.go
  • otdfctl/pkg/cli/flagValues.go

📝 Walkthrough

Walkthrough

This PR modernizes the CLI flag parsing infrastructure by promoting the internal flagHelper type to a public Flags type with improved nil-safe flag registration handling. The core type definition is updated in flagValues.go, the Cli struct is refactored to use the new type, and all command modules are updated to use the new accessor.

Changes

CLI Flag Helper Type Modernization

Layer / File(s) Summary
Flag type definition and core parsing methods
otdfctl/pkg/cli/flagValues.go
Introduces exported Flags type with newFlags constructor and flag parsing methods for string, ID, int32, and bool values. Adds explicit nil-handling for unregistered flags while preserving UUID validation and error reporting.
Cli struct field and initialization update
otdfctl/pkg/cli/cli.go
Updates Cli struct to store Flags *Flags instead of prior *flagHelper and FlagHelper fields, and initializes the field via newFlags(cmd) in the New constructor.
Command-site flag accessor migrations
otdfctl/cmd/auth/login.go, otdfctl/cmd/auth/logout.go, otdfctl/cmd/common/common.go, otdfctl/cmd/policy/attributeValues.go, otdfctl/cmd/policy/kasRegistry.go, otdfctl/cmd/profile.go
Updates all command handlers to fetch flags via c.Flags instead of c.FlagHelper across login, logout, attribute values, KAS registry, profile management, and handler initialization routines.

🎯 2 (Simple) | ⏱️ ~10 minutes

🐰 The flags now hop with pride,
Exported and public, with nil-safe stride,
From helper to Flags with a modernizing glide,
All commands now use the refactored tide! 🏃‍♂️✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(cli): deprecate flaghelper for new flags' directly and clearly describes the main change: deprecating the FlagHelper in favor of a new Flags implementation across the CLI codebase.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/deprecate-flaghelper-cli

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the size/s label Jun 8, 2026
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! 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!

This pull request performs a refactor of the CLI flag handling mechanism within the project. By renaming the internal helper struct and removing redundant wrappers, the codebase achieves a cleaner and more consistent interface for accessing command-line flags.

Highlights

  • Refactoring Flag Handling: Renamed the internal 'flagHelper' struct to 'flags' and updated the CLI structure to use this new naming convention.
  • Code Cleanup: Removed the temporary 'FlagHelper' wrapper in the CLI struct, standardizing flag access through the 'Flags' field across the codebase.
New Features

🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize the 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 may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.


The flags were hidden in a helper's name, / But now the structure plays a cleaner game. / With 'flags' in place and wrappers cast aside, / The code is neat, with nowhere left to hide.

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.

@jakedoublev jakedoublev changed the title fix(otdfctl): deprecate flaghelper for new flags fix(cli): deprecate flaghelper for new flags Jun 8, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the CLI flag helper by renaming flagHelper to flags and removing the deprecated FlagHelper wrapper field from the Cli struct, updating all references across the codebase. Feedback on these changes highlights a potential nil pointer dereference in GetRequiredString if a requested flag is not registered, and suggests exporting the flags struct as Flags to avoid the Go anti-pattern of exporting a field of an unexported type.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread otdfctl/pkg/cli/flagValues.go Outdated
Comment thread otdfctl/pkg/cli/flagValues.go Outdated
Comment thread otdfctl/pkg/cli/cli.go Outdated
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 286.43765ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 120.143517ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 472.593594ms
Throughput 211.60 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 43.880182894s
Average Latency 437.182897ms
Throughput 113.95 requests/second

Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 170.920571ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 96.923922ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 435.993856ms
Throughput 229.36 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 43.395731603s
Average Latency 432.272069ms
Throughput 115.22 requests/second

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

⚠️ Govulncheck found vulnerabilities ⚠️

The following modules have known vulnerabilities:

  • examples
  • otdfctl
  • sdk
  • service
  • lib/fixtures
  • tests-bdd

See the workflow run for details.

@jakedoublev
jakedoublev marked this pull request as ready for review June 8, 2026 16:13
@jakedoublev
jakedoublev requested a review from a team as a code owner June 8, 2026 16:13
@jakedoublev
jakedoublev enabled auto-merge June 8, 2026 16:14
@jakedoublev
jakedoublev added this pull request to the merge queue Jun 8, 2026
Merged via the queue into main with commit 8f3c429 Jun 8, 2026
40 checks passed
@jakedoublev
jakedoublev deleted the fix/deprecate-flaghelper-cli branch June 8, 2026 16:31
dmihalcik-virtru pushed a commit that referenced this pull request Jun 11, 2026
🤖 I have created a release *beep* *boop*
---


##
[0.33.0](otdfctl/v0.32.0...otdfctl/v0.33.0)
(2026-06-09)


### Features

* **cli:** improve auth
([#3466](#3466))
([b244910](b244910))
* **core:** add hybrid NIST EC + ML-KEM key wrapping support
([#3276](#3276))
([1209acc](1209acc))


### Bug Fixes

* **ci:** Prefer go.work for toolchain info
([#3285](#3285))
([3c05b22](3c05b22))
* **cli:** deprecate flaghelper for new flags
([#3583](#3583))
([8f3c429](8f3c429))
* **deps:** bump github.com/opentdf/platform/lib/ocrypto from 0.10.0 to
0.11.0 in /otdfctl
([#3530](#3530))
([745a205](745a205))
* **deps:** bump github.com/opentdf/platform/lib/ocrypto from 0.11.0 to
0.12.0 in /otdfctl
([#3536](#3536))
([8d1c018](8d1c018))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.30.0 to
0.31.0 in /otdfctl
([#3499](#3499))
([fa91478](fa91478))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.31.0 to
0.32.0 in /otdfctl
([#3531](#3531))
([23946e4](23946e4))
* **deps:** bump github.com/opentdf/platform/sdk from 0.17.0 to 0.21.0
in /otdfctl ([#3549](#3549))
([1cb751e](1cb751e))
* **deps:** bump the external group across 1 directory with 4 updates
([#3494](#3494))
([5b87b2b](5b87b2b))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants