Skip to content

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

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.

Sign up for GitHub

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

Merged
jakedoublev merged 2 commits into from
Jun 8, 2026
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d9956bf
fix(otdfctl): deprecate flaghelper for new flags
jakedoublev Jun 8, 2026
f22e9d0
fix(cli): address flags review feedback
jakedoublev Jun 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions otdfctl/cmd/auth/login.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ import (
func codeLogin(cmd *cobra.Command, args []string) {
c := cli.New(cmd, args)
cp := common.InitProfile(c)
clientID := c.FlagHelper.GetRequiredString("client-id")
port := c.FlagHelper.GetOptionalString("port")
clientID := c.Flags.GetRequiredString("client-id")
port := c.Flags.GetOptionalString("port")
tok, err := auth.LoginWithPKCE(
cmd.Context(),
cp.GetEndpoint(),
clientID,
c.FlagHelper.GetOptionalBool("tls-no-verify"),
c.Flags.GetOptionalBool("tls-no-verify"),
port,
)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion otdfctl/cmd/auth/logout.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func logout(cmd *cobra.Command, args []string) {
cp.GetEndpoint(),
creds.AccessToken.ClientID,
creds.AccessToken.RefreshToken,
c.FlagHelper.GetOptionalBool("tls-no-verify"),
c.Flags.GetOptionalBool("tls-no-verify"),
); err != nil {
c.ExitWithError("An error occurred while revoking the access token", err)
}
Expand Down
12 changes: 6 additions & 6 deletions otdfctl/cmd/common/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func applyOutputFormatPreference(c *cli.Cli, store *profiles.OtdfctlProfileStore
// returns the profile and the current profile store
func InitProfile(c *cli.Cli) *profiles.OtdfctlProfileStore {
var err error
profileName := c.FlagHelper.GetOptionalString("profile")
profileName := c.Flags.GetOptionalString("profile")

hasKeyringStore, err := osprofiles.HasGlobalStore(config.AppName, osprofiles.WithKeyringStore())
if err != nil {
Expand Down Expand Up @@ -91,11 +91,11 @@ func NewHandler(c *cli.Cli) handlers.Handler {
var cp *profiles.OtdfctlProfileStore

// Non-profile flags
host := c.FlagHelper.GetOptionalString("host")
tlsNoVerify := c.FlagHelper.GetOptionalBool("tls-no-verify")
withClientCreds := c.FlagHelper.GetOptionalString("with-client-creds")
withClientCredsFile := c.FlagHelper.GetOptionalString("with-client-creds-file")
withAccessToken := c.FlagHelper.GetOptionalString("with-access-token")
host := c.Flags.GetOptionalString("host")
tlsNoVerify := c.Flags.GetOptionalBool("tls-no-verify")
withClientCreds := c.Flags.GetOptionalString("with-client-creds")
withClientCredsFile := c.Flags.GetOptionalString("with-client-creds-file")
withAccessToken := c.Flags.GetOptionalString("with-access-token")
var inMemoryProfile bool

authFlags := []string{"--with-access-token", "--with-client-creds", "--with-client-creds-file"}
Expand Down
10 changes: 5 additions & 5 deletions otdfctl/cmd/policy/attributeValues.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ func createAttributeValue(cmd *cobra.Command, args []string) {
defer h.Close()

ctx := cmd.Context()
attrID := c.FlagHelper.GetRequiredID("attribute-id")
value := c.FlagHelper.GetRequiredString("value")
metadataLabels = c.FlagHelper.GetStringSlice("label", metadataLabels, cli.FlagsStringSliceOptions{Min: 0})
attrID := c.Flags.GetRequiredID("attribute-id")
value := c.Flags.GetRequiredString("value")
metadataLabels = c.Flags.GetStringSlice("label", metadataLabels, cli.FlagsStringSliceOptions{Min: 0})

attr, err := h.GetAttribute(ctx, attrID)
if err != nil {
Expand All @@ -44,7 +44,7 @@ func getAttributeValue(cmd *cobra.Command, args []string) {
h := common.NewHandler(c)
defer h.Close()

id := c.FlagHelper.GetRequiredID("id")
id := c.Flags.GetRequiredID("id")

v, err := h.GetAttributeValue(cmd.Context(), id)
if err != nil {
Expand Down Expand Up @@ -105,7 +105,7 @@ func listAttributeValue(cmd *cobra.Command, args []string) {
h := common.NewHandler(c)
defer h.Close()

attrID := c.FlagHelper.GetRequiredID("attribute-id")
attrID := c.Flags.GetRequiredID("attribute-id")
state := cli.GetState(cmd)
limit := c.Flags.GetRequiredInt32("limit")
offset := c.Flags.GetRequiredInt32("offset")
Expand Down
2 changes: 1 addition & 1 deletion otdfctl/cmd/policy/kasRegistry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func getKeyAccessRegistry(cmd *cobra.Command, args []string) {
h := common.NewHandler(c)
defer h.Close()

id := c.FlagHelper.GetRequiredID("id")
id := c.Flags.GetRequiredID("id")

kas, err := h.GetKasRegistryEntry(cmd.Context(), handlers.KasIdentifier{
ID: id,
Expand Down
8 changes: 4 additions & 4 deletions otdfctl/cmd/profile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func newProfilerFromCLI(c *cli.Cli) *osprofiles.Profiler {

func getDriverTypeFromUser(c *cli.Cli) profiles.ProfileDriver {
driverTypeStr := string(profiles.ProfileDriverDefault)
store := c.FlagHelper.GetOptionalString("store")
store := c.Flags.GetOptionalString("store")
if len(store) > 0 {
driverTypeStr = store
}
Expand Down Expand Up @@ -90,9 +90,9 @@ var profileCreateCmd = &cobra.Command{
profileName := args[0]
endpoint := args[1]

setDefault := c.FlagHelper.GetOptionalBool("set-default")
tlsNoVerify := c.FlagHelper.GetOptionalBool("tls-no-verify")
outputFormat := c.FlagHelper.GetOptionalString("output-format")
setDefault := c.Flags.GetOptionalBool("set-default")
tlsNoVerify := c.Flags.GetOptionalBool("tls-no-verify")
outputFormat := c.Flags.GetOptionalString("output-format")
if !profiles.IsValidOutputFormat(outputFormat) {
c.ExitWithError("Output format must be either 'styled' or 'json'", nil)
}
Expand Down
10 changes: 3 additions & 7 deletions otdfctl/pkg/cli/cli.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@ type Cli struct {
args []string

// Helpers
Flags *flagHelper
FlagHelper *flagHelper
printer *Printer
Flags *flags
Comment thread
jakedoublev marked this conversation as resolved.
Outdated
printer *Printer
}

// New creates a new Cli object
Expand All @@ -34,10 +33,7 @@ func New(cmd *cobra.Command, args []string, options ...cliVariadicOption) *Cli {
ExitWithError("cli expects a command", ErrPrinterExpectsCommand)
}

cli.Flags = newFlagHelper(cmd)
// Temp wrapper for FlagHelper until we can remove it
cli.FlagHelper = cli.Flags

cli.Flags = newFlags(cmd)
cli.printer = newPrinter(opts.printerJSON || cli.Flags.GetOptionalBool("json"))

return cli
Expand Down
38 changes: 13 additions & 25 deletions otdfctl/pkg/cli/flagValues.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,23 @@ type FlagsStringSliceOptions struct {
Max int
}

type flagHelper struct {
type flags struct {
cmd *cobra.Command
}

func newFlagHelper(cmd *cobra.Command) *flagHelper {
return &flagHelper{cmd: cmd}
func newFlags(cmd *cobra.Command) *flags {
return &flags{cmd: cmd}
}
Comment thread
jakedoublev marked this conversation as resolved.
Outdated

func (f flagHelper) GetRequiredString(flag string) string {
func (f flags) GetRequiredString(flag string) string {
v := f.cmd.Flag(flag).Value.String()
if v == "" {
ExitWithError("Flag '--"+flag+"' is required", nil)
}
return v
}
Comment thread
jakedoublev marked this conversation as resolved.
Outdated

func (f flagHelper) GetRequiredID(idFlag string) string {
func (f flags) GetRequiredID(idFlag string) string {
v := f.GetRequiredString(idFlag)
id, err := uuid.Parse(v)
if err != nil {
Expand All @@ -40,7 +40,7 @@ func (f flagHelper) GetRequiredID(idFlag string) string {
return id.String()
}

func (f flagHelper) GetOptionalID(idFlag string) string {
func (f flags) GetOptionalID(idFlag string) string {
p := f.GetOptionalString(idFlag)
if p == "" {
return ""
Expand All @@ -52,15 +52,15 @@ func (f flagHelper) GetOptionalID(idFlag string) string {
return id.String()
}

func (f flagHelper) GetOptionalString(flag string) string {
func (f flags) GetOptionalString(flag string) string {
p := f.cmd.Flag(flag)
if p == nil {
return ""
}
return p.Value.String()
}

func (f flagHelper) GetStringSlice(flag string, v []string, opts FlagsStringSliceOptions) []string {
func (f flags) GetStringSlice(flag string, v []string, opts FlagsStringSliceOptions) []string {
if len(v) < opts.Min {
ExitWithError(fmt.Sprintf("Flag '--%s' must have at least %d non-empty values", flag, opts.Min), nil)
}
Expand All @@ -70,7 +70,7 @@ func (f flagHelper) GetStringSlice(flag string, v []string, opts FlagsStringSlic
return v
}

func (f flagHelper) GetRequiredInt32(flag string) int32 {
func (f flags) GetRequiredInt32(flag string) int32 {
v, e := f.cmd.Flags().GetInt32(flag)
if e != nil {
ExitWithError("Flag '--"+flag+"' is required", nil)
Expand All @@ -82,26 +82,26 @@ func (f flagHelper) GetRequiredInt32(flag string) int32 {
return v
}

func (f flagHelper) GetOptionalInt32(flag string) int32 {
func (f flags) GetOptionalInt32(flag string) int32 {
v, _ := f.cmd.Flags().GetInt32(flag)
return v
}

func (f flagHelper) GetOptionalBool(flag string) bool {
func (f flags) GetOptionalBool(flag string) bool {
v, _ := f.cmd.Flags().GetBool(flag)
return v
}

// Returns nil when the flag is not explicitly set.
func (f flagHelper) GetOptionalBoolWrapper(flag string) *wrapperspb.BoolValue {
func (f flags) GetOptionalBoolWrapper(flag string) *wrapperspb.BoolValue {
if !f.cmd.Flags().Changed(flag) {
return nil
}
v, _ := f.cmd.Flags().GetBool(flag)
return wrapperspb.Bool(v)
}

func (f flagHelper) GetRequiredBool(flag string) bool {
func (f flags) GetRequiredBool(flag string) bool {
v, e := f.cmd.Flags().GetBool(flag)
if e != nil {
ExitWithError("Flag '--"+flag+"' is required", nil)
Expand All @@ -124,18 +124,6 @@ func GetState(cmd *cobra.Command) common.ActiveStateEnum {
return state
}

// func (f flagHelper) GetStructSlice(flag string, v []StructFlag[T], opts flagHelperStringSliceOptions) ([]StructFlag[T], err) {
// if len(v) < opts.Min {
// fmt.Println(ErrorMessage(fmt.Sprintf("Flag %s must have at least %d non-empty values", flag, opts.Min), nil))
// os.Exit(1)
// }
// if opts.Max > 0 && len(v) > opts.Max {
// fmt.Println(ErrorMessage(fmt.Sprintf("Flag %s must have at most %d non-empty values", flag, opts.Max), nil))
// os.Exit(1)
// }
// return v
// }

// type StructFlag[T any] struct {
// Val T
// }
Expand Down
Loading