Skip to content

Commit 8f3c429

Browse files
authored
fix(cli): deprecate flaghelper for new flags (#3583)
### 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
1 parent 11af44a commit 8f3c429

8 files changed

Lines changed: 41 additions & 53 deletions

File tree

otdfctl/cmd/auth/login.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@ import (
1414
func codeLogin(cmd *cobra.Command, args []string) {
1515
c := cli.New(cmd, args)
1616
cp := common.InitProfile(c)
17-
clientID := c.FlagHelper.GetRequiredString("client-id")
18-
port := c.FlagHelper.GetOptionalString("port")
17+
clientID := c.Flags.GetRequiredString("client-id")
18+
port := c.Flags.GetOptionalString("port")
1919
tok, err := auth.LoginWithPKCE(
2020
cmd.Context(),
2121
cp.GetEndpoint(),
2222
clientID,
23-
c.FlagHelper.GetOptionalBool("tls-no-verify"),
23+
c.Flags.GetOptionalBool("tls-no-verify"),
2424
port,
2525
)
2626
if err != nil {

otdfctl/cmd/auth/logout.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ func logout(cmd *cobra.Command, args []string) {
2323
cp.GetEndpoint(),
2424
creds.AccessToken.ClientID,
2525
creds.AccessToken.RefreshToken,
26-
c.FlagHelper.GetOptionalBool("tls-no-verify"),
26+
c.Flags.GetOptionalBool("tls-no-verify"),
2727
); err != nil {
2828
c.ExitWithError("An error occurred while revoking the access token", err)
2929
}

otdfctl/cmd/common/common.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func applyOutputFormatPreference(c *cli.Cli, store *profiles.OtdfctlProfileStore
3939
// returns the profile and the current profile store
4040
func InitProfile(c *cli.Cli) *profiles.OtdfctlProfileStore {
4141
var err error
42-
profileName := c.FlagHelper.GetOptionalString("profile")
42+
profileName := c.Flags.GetOptionalString("profile")
4343

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

9393
// Non-profile flags
94-
host := c.FlagHelper.GetOptionalString("host")
95-
tlsNoVerify := c.FlagHelper.GetOptionalBool("tls-no-verify")
96-
withClientCreds := c.FlagHelper.GetOptionalString("with-client-creds")
97-
withClientCredsFile := c.FlagHelper.GetOptionalString("with-client-creds-file")
98-
withAccessToken := c.FlagHelper.GetOptionalString("with-access-token")
94+
host := c.Flags.GetOptionalString("host")
95+
tlsNoVerify := c.Flags.GetOptionalBool("tls-no-verify")
96+
withClientCreds := c.Flags.GetOptionalString("with-client-creds")
97+
withClientCredsFile := c.Flags.GetOptionalString("with-client-creds-file")
98+
withAccessToken := c.Flags.GetOptionalString("with-access-token")
9999
var inMemoryProfile bool
100100

101101
authFlags := []string{"--with-access-token", "--with-client-creds", "--with-client-creds-file"}

otdfctl/cmd/policy/attributeValues.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@ func createAttributeValue(cmd *cobra.Command, args []string) {
2222
defer h.Close()
2323

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

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

47-
id := c.FlagHelper.GetRequiredID("id")
47+
id := c.Flags.GetRequiredID("id")
4848

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

108-
attrID := c.FlagHelper.GetRequiredID("attribute-id")
108+
attrID := c.Flags.GetRequiredID("attribute-id")
109109
state := cli.GetState(cmd)
110110
limit := c.Flags.GetRequiredInt32("limit")
111111
offset := c.Flags.GetRequiredInt32("offset")

otdfctl/cmd/policy/kasRegistry.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ func getKeyAccessRegistry(cmd *cobra.Command, args []string) {
1919
h := common.NewHandler(c)
2020
defer h.Close()
2121

22-
id := c.FlagHelper.GetRequiredID("id")
22+
id := c.Flags.GetRequiredID("id")
2323

2424
kas, err := h.GetKasRegistryEntry(cmd.Context(), handlers.KasIdentifier{
2525
ID: id,

otdfctl/cmd/profile.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ func newProfilerFromCLI(c *cli.Cli) *osprofiles.Profiler {
5959

6060
func getDriverTypeFromUser(c *cli.Cli) profiles.ProfileDriver {
6161
driverTypeStr := string(profiles.ProfileDriverDefault)
62-
store := c.FlagHelper.GetOptionalString("store")
62+
store := c.Flags.GetOptionalString("store")
6363
if len(store) > 0 {
6464
driverTypeStr = store
6565
}
@@ -90,9 +90,9 @@ var profileCreateCmd = &cobra.Command{
9090
profileName := args[0]
9191
endpoint := args[1]
9292

93-
setDefault := c.FlagHelper.GetOptionalBool("set-default")
94-
tlsNoVerify := c.FlagHelper.GetOptionalBool("tls-no-verify")
95-
outputFormat := c.FlagHelper.GetOptionalString("output-format")
93+
setDefault := c.Flags.GetOptionalBool("set-default")
94+
tlsNoVerify := c.Flags.GetOptionalBool("tls-no-verify")
95+
outputFormat := c.Flags.GetOptionalString("output-format")
9696
if !profiles.IsValidOutputFormat(outputFormat) {
9797
c.ExitWithError("Output format must be either 'styled' or 'json'", nil)
9898
}

otdfctl/pkg/cli/cli.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,8 @@ type Cli struct {
1111
args []string
1212

1313
// Helpers
14-
Flags *flagHelper
15-
FlagHelper *flagHelper
16-
printer *Printer
14+
Flags *Flags
15+
printer *Printer
1716
}
1817

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

37-
cli.Flags = newFlagHelper(cmd)
38-
// Temp wrapper for FlagHelper until we can remove it
39-
cli.FlagHelper = cli.Flags
40-
36+
cli.Flags = newFlags(cmd)
4137
cli.printer = newPrinter(opts.printerJSON || cli.Flags.GetOptionalBool("json"))
4238

4339
return cli

otdfctl/pkg/cli/flagValues.go

Lines changed: 18 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -15,23 +15,27 @@ type FlagsStringSliceOptions struct {
1515
Max int
1616
}
1717

18-
type flagHelper struct {
18+
type Flags struct {
1919
cmd *cobra.Command
2020
}
2121

22-
func newFlagHelper(cmd *cobra.Command) *flagHelper {
23-
return &flagHelper{cmd: cmd}
22+
func newFlags(cmd *cobra.Command) *Flags {
23+
return &Flags{cmd: cmd}
2424
}
2525

26-
func (f flagHelper) GetRequiredString(flag string) string {
27-
v := f.cmd.Flag(flag).Value.String()
26+
func (f Flags) GetRequiredString(flag string) string {
27+
p := f.cmd.Flag(flag)
28+
if p == nil {
29+
ExitWithError("Flag '--"+flag+"' is not registered", nil)
30+
}
31+
v := p.Value.String()
2832
if v == "" {
2933
ExitWithError("Flag '--"+flag+"' is required", nil)
3034
}
3135
return v
3236
}
3337

34-
func (f flagHelper) GetRequiredID(idFlag string) string {
38+
func (f Flags) GetRequiredID(idFlag string) string {
3539
v := f.GetRequiredString(idFlag)
3640
id, err := uuid.Parse(v)
3741
if err != nil {
@@ -40,7 +44,7 @@ func (f flagHelper) GetRequiredID(idFlag string) string {
4044
return id.String()
4145
}
4246

43-
func (f flagHelper) GetOptionalID(idFlag string) string {
47+
func (f Flags) GetOptionalID(idFlag string) string {
4448
p := f.GetOptionalString(idFlag)
4549
if p == "" {
4650
return ""
@@ -52,15 +56,15 @@ func (f flagHelper) GetOptionalID(idFlag string) string {
5256
return id.String()
5357
}
5458

55-
func (f flagHelper) GetOptionalString(flag string) string {
59+
func (f Flags) GetOptionalString(flag string) string {
5660
p := f.cmd.Flag(flag)
5761
if p == nil {
5862
return ""
5963
}
6064
return p.Value.String()
6165
}
6266

63-
func (f flagHelper) GetStringSlice(flag string, v []string, opts FlagsStringSliceOptions) []string {
67+
func (f Flags) GetStringSlice(flag string, v []string, opts FlagsStringSliceOptions) []string {
6468
if len(v) < opts.Min {
6569
ExitWithError(fmt.Sprintf("Flag '--%s' must have at least %d non-empty values", flag, opts.Min), nil)
6670
}
@@ -70,7 +74,7 @@ func (f flagHelper) GetStringSlice(flag string, v []string, opts FlagsStringSlic
7074
return v
7175
}
7276

73-
func (f flagHelper) GetRequiredInt32(flag string) int32 {
77+
func (f Flags) GetRequiredInt32(flag string) int32 {
7478
v, e := f.cmd.Flags().GetInt32(flag)
7579
if e != nil {
7680
ExitWithError("Flag '--"+flag+"' is required", nil)
@@ -82,26 +86,26 @@ func (f flagHelper) GetRequiredInt32(flag string) int32 {
8286
return v
8387
}
8488

85-
func (f flagHelper) GetOptionalInt32(flag string) int32 {
89+
func (f Flags) GetOptionalInt32(flag string) int32 {
8690
v, _ := f.cmd.Flags().GetInt32(flag)
8791
return v
8892
}
8993

90-
func (f flagHelper) GetOptionalBool(flag string) bool {
94+
func (f Flags) GetOptionalBool(flag string) bool {
9195
v, _ := f.cmd.Flags().GetBool(flag)
9296
return v
9397
}
9498

9599
// Returns nil when the flag is not explicitly set.
96-
func (f flagHelper) GetOptionalBoolWrapper(flag string) *wrapperspb.BoolValue {
100+
func (f Flags) GetOptionalBoolWrapper(flag string) *wrapperspb.BoolValue {
97101
if !f.cmd.Flags().Changed(flag) {
98102
return nil
99103
}
100104
v, _ := f.cmd.Flags().GetBool(flag)
101105
return wrapperspb.Bool(v)
102106
}
103107

104-
func (f flagHelper) GetRequiredBool(flag string) bool {
108+
func (f Flags) GetRequiredBool(flag string) bool {
105109
v, e := f.cmd.Flags().GetBool(flag)
106110
if e != nil {
107111
ExitWithError("Flag '--"+flag+"' is required", nil)
@@ -124,18 +128,6 @@ func GetState(cmd *cobra.Command) common.ActiveStateEnum {
124128
return state
125129
}
126130

127-
// func (f flagHelper) GetStructSlice(flag string, v []StructFlag[T], opts flagHelperStringSliceOptions) ([]StructFlag[T], err) {
128-
// if len(v) < opts.Min {
129-
// fmt.Println(ErrorMessage(fmt.Sprintf("Flag %s must have at least %d non-empty values", flag, opts.Min), nil))
130-
// os.Exit(1)
131-
// }
132-
// if opts.Max > 0 && len(v) > opts.Max {
133-
// fmt.Println(ErrorMessage(fmt.Sprintf("Flag %s must have at most %d non-empty values", flag, opts.Max), nil))
134-
// os.Exit(1)
135-
// }
136-
// return v
137-
// }
138-
139131
// type StructFlag[T any] struct {
140132
// Val T
141133
// }

0 commit comments

Comments
 (0)