Skip to content

Commit d6932f3

Browse files
jrschumacherjrschumacher
authored
fix: refactor to support varying print output (#350)
- reduce burden of multiple cli objects - c.Flags rather than FlagHelper - add various print options (c.Print, c.Printf, c.Println) - add debug print options (c.Debug, c.Debugf, c.Debugln) - add `--json` support - add `--debug` support
1 parent 30b823a commit d6932f3

31 files changed

+677
-410
lines changed

‎adr/0000-use-adr-in-directory.md‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
---
2+
status: accepted
3+
date: 2024-08-29
4+
decision: Use ADRs in the `adr` directory of the repo to document architectural decisions
5+
author: '@jakedoublev'
6+
deciders: ['@ryanulit', '@jrschumacher']
7+
---
8+
9+
# Use a ADR storage format that make diffs easier to read
10+
11+
## Context and Problem Statement
12+
13+
We've been using Github Issues to document ADR decisions, but it's hard to read the diffs when changes are made. We need a better way to store and manage ADRs. ADRs sometimes get updated and it's hard to track the changes and decision using the edit history dropdown or the comments section.
14+
15+
## Decision Drivers
16+
17+
- **Low barrier of entry**: A primary goal of our ADR process is to ensure decisions are captured.
18+
- **Ease of management**: Make it easy to manage the ADRs.
19+
- **Ensure appropriate tracking and review**: Make it easy to track and review the changes in the ADRs.
20+
21+
## Considered Options
22+
23+
1. Use Github Issues
24+
2. Use Github Discussions
25+
3. Use a shared ADR repository
26+
4. Use an `adr` directory in the repo
27+
28+
## Decision Outcome
29+
30+
It was decided to use an `adr` directory in the repo to store ADRs. This approach provides a low barrier of entry for developers to document decisions and ensures that the decisions are tracked and reviewed appropriately.
31+
32+
Additionally, this change does not impact other teams or repositories, and it is easy to manage and maintain. We can experiment with this decision and if it works promote it to other repositories.
33+
34+
### Consequences
35+
36+
- **Positive**:
37+
- Low barrier of entry for developers to document decisions.
38+
- Easy to manage and maintain.
39+
- Ensures appropriate tracking and review of decisions via git history and code review.
40+
- **Negative**:
41+
- Requires developers to be aware of the ADR process and where to find the ADRs.
42+
- May require additional tooling to manage and maintain the ADRs.
43+
- May require additional training for developers to understand the ADR process and how to use it effectively.

‎adr/0001-printing-with-json.md‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
---
2+
status: accepted
3+
date: 2024-08-29
4+
decision: Encapsulate printing to ensure consistent output format
5+
author: '@jrschumacher'
6+
deciders: ['@jakedoublev', '@ryanulit', '@suchak1']
7+
---
8+
9+
# Consistent output format for printing JSON and pretty-print
10+
11+
## Context and Problem Statement
12+
13+
We need to develop a printer that can globally determine when to print in pretty-print format versus JSON format. This decision is crucial to ensure consistent and appropriate output formatting across different use cases and environments.
14+
15+
## Decision Drivers
16+
17+
- **Consistency**: Ensure uniform output format across the application.
18+
- **Flexibility**: Ability to switch between pretty-print and JSON formats based on context.
19+
- **Ease of Implementation**: Simplicity in implementing and maintaining the solution.
20+
21+
## Considered Options
22+
23+
1. Keep existing code as is
24+
2. Move the printing into a global function that has context about the CLI flags to drive output format
25+
26+
## Decision Outcome
27+
28+
It was decided to encapsulate printing to ensure there is consistent output format. This function will have context about the CLI flags to drive the output format. This approach provides the flexibility to switch between pretty-print and JSON formats based on the context.
29+
30+
### Consequences
31+
32+
- **Positive**:
33+
- Provides flexibility to switch formats without changing the code.
34+
- Ensures consistent output format across different environments.
35+
- Simplifies the implementation and maintenance process.
36+
37+
- **Negative**:
38+
- Requires careful management of configuration settings.
39+
- Potential for misconfiguration leading to incorrect output format when developers use `fmt` directly.

‎cmd/auth-clientCredentials.go‎

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package cmd
22

33
import (
4-
"fmt"
5-
64
"github.com/opentdf/otdfctl/pkg/auth"
75
"github.com/opentdf/otdfctl/pkg/cli"
86
"github.com/opentdf/otdfctl/pkg/man"
@@ -16,9 +14,8 @@ var clientCredentialsCmd = man.Docs.GetCommand("auth/client-credentials",
1614
)
1715

1816
func auth_clientCredentials(cmd *cobra.Command, args []string) {
19-
cp := InitProfile(cmd, false)
20-
21-
p := cli.NewPrinter(true)
17+
c := cli.New(cmd, args)
18+
cp := InitProfile(c, false)
2219

2320
var clientId string
2421
var clientSecret string
@@ -45,20 +42,20 @@ func auth_clientCredentials(cmd *cobra.Command, args []string) {
4542
})
4643

4744
// Validate the client credentials
48-
p.Printf("Validating client credentials for %s... ", cp.GetEndpoint())
45+
c.Printf("Validating client credentials for %s... ", cp.GetEndpoint())
4946
if err := auth.ValidateProfileAuthCredentials(cmd.Context(), cp); err != nil {
50-
fmt.Println("failed")
51-
cli.ExitWithError("An error occurred during login. Please check your credentials and try again", err)
47+
c.Println("failed")
48+
c.ExitWithError("An error occurred during login. Please check your credentials and try again", err)
5249
}
53-
p.Println("ok")
50+
c.Println("ok")
5451

5552
// Save the client credentials
56-
p.Print("Storing client ID and secret in keyring... ")
53+
c.Print("Storing client ID and secret in keyring... ")
5754
if err := cp.Save(); err != nil {
58-
p.Println("failed")
59-
cli.ExitWithError("An error occurred while storing client credentials", err)
55+
c.Println("failed")
56+
c.ExitWithError("An error occurred while storing client credentials", err)
6057
}
61-
p.Println("ok")
58+
c.Println("ok")
6259
}
6360

6461
func init() {

‎cmd/auth-login.go‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,21 @@ import (
99
)
1010

1111
func auth_codeLogin(cmd *cobra.Command, args []string) {
12-
fh := cli.NewFlagHelper(cmd)
13-
clientID := fh.GetOptionalString("client-id")
14-
tlsNoVerify := fh.GetOptionalBool("tls-no-verify")
15-
16-
cp := InitProfile(cmd, false)
17-
printer := cli.NewPrinter(true)
18-
19-
printer.Println("Initiating login...")
20-
tok, publicClientID, err := auth.LoginWithPKCE(cmd.Context(), cp.GetEndpoint(), clientID, tlsNoVerify)
12+
c := cli.New(cmd, args)
13+
cp := InitProfile(c, false)
14+
15+
c.Print("Initiating login...")
16+
tok, publicClientID, err := auth.LoginWithPKCE(
17+
cmd.Context(),
18+
cp.GetEndpoint(),
19+
c.FlagHelper.GetOptionalString("client-id"),
20+
c.FlagHelper.GetOptionalBool("tls-no-verify"),
21+
)
2122
if err != nil {
22-
cli.ExitWithError("could not authenticate", err)
23+
c.Println("failed")
24+
c.ExitWithError("could not authenticate", err)
2325
}
24-
printer.Println("ok")
26+
c.Println("ok")
2527

2628
// Set the auth credentials to profile
2729
if err := cp.SetAuthCredentials(profiles.AuthCredentials{
@@ -33,15 +35,15 @@ func auth_codeLogin(cmd *cobra.Command, args []string) {
3335
RefreshToken: tok.RefreshToken,
3436
},
3537
}); err != nil {
36-
cli.ExitWithError("failed to set auth credentials", err)
38+
c.ExitWithError("failed to set auth credentials", err)
3739
}
3840

39-
printer.Println("Storing credentials to profile in keyring...")
41+
c.Print("Storing credentials to profile in keyring...")
4042
if err := cp.Save(); err != nil {
41-
printer.Println("failed")
42-
cli.ExitWithError("An error occurred while storing authentication credentials", err)
43+
c.Println("failed")
44+
c.ExitWithError("An error occurred while storing authentication credentials", err)
4345
}
44-
printer.Println("ok")
46+
c.Println("ok")
4547
}
4648

4749
var codeLoginCmd *man.Doc

‎cmd/auth-logout.go‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,33 +9,31 @@ import (
99
)
1010

1111
func auth_logout(cmd *cobra.Command, args []string) {
12-
fh := cli.NewFlagHelper(cmd)
13-
tlsNoVerify := fh.GetOptionalBool("tls-no-verify")
14-
cp := InitProfile(cmd, false)
15-
printer := cli.NewPrinter(true)
16-
printer.Println("Initiating logout...")
12+
c := cli.New(cmd, args)
13+
cp := InitProfile(c, false)
14+
c.Println("Initiating logout...")
1715

1816
// we can only revoke access tokens stored for the code login flow, not client credentials
1917
creds := cp.GetAuthCredentials()
2018
if creds.AuthType == profiles.PROFILE_AUTH_TYPE_ACCESS_TOKEN {
21-
printer.Println("Revoking access token...")
19+
c.Println("Revoking access token...")
2220
if err := auth.RevokeAccessToken(
2321
cmd.Context(),
2422
cp.GetEndpoint(),
2523
creds.AccessToken.PublicClientID,
2624
creds.AccessToken.RefreshToken,
27-
tlsNoVerify,
25+
c.FlagHelper.GetOptionalBool("tls-no-verify"),
2826
); err != nil {
29-
printer.Println("failed")
30-
cli.ExitWithError("An error occurred while revoking the access token", err)
27+
c.Println("failed")
28+
c.ExitWithError("An error occurred while revoking the access token", err)
3129
}
3230
}
3331

3432
if err := cp.SetAuthCredentials(profiles.AuthCredentials{}); err != nil {
35-
printer.Println("failed")
36-
cli.ExitWithError("An error occurred while logging out", err)
33+
c.Println("failed")
34+
c.ExitWithError("An error occurred while logging out", err)
3735
}
38-
printer.Println("ok")
36+
c.Println("ok")
3937
}
4038

4139
var codeLogoutCmd *man.Doc

‎cmd/auth-printAccessToken.go‎

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
package cmd
22

33
import (
4-
"encoding/json"
5-
"fmt"
6-
74
"github.com/opentdf/otdfctl/pkg/auth"
85
"github.com/opentdf/otdfctl/pkg/cli"
96
"github.com/opentdf/otdfctl/pkg/man"
@@ -15,40 +12,27 @@ var auth_printAccessTokenCmd = man.Docs.GetCommand("auth/print-access-token",
1512
man.WithRun(auth_printAccessToken))
1613

1714
func auth_printAccessToken(cmd *cobra.Command, args []string) {
18-
flagHelper := cli.NewFlagHelper(cmd)
19-
jsonOut := flagHelper.GetOptionalBool("json")
20-
21-
cp := InitProfile(cmd, false)
22-
23-
printEnabled := !jsonOut
24-
p := cli.NewPrinter(printEnabled)
15+
c := cli.New(cmd, args)
16+
cp := InitProfile(c, false)
2517

2618
ac := cp.GetAuthCredentials()
2719
switch ac.AuthType {
2820
case profiles.PROFILE_AUTH_TYPE_CLIENT_CREDENTIALS:
29-
p.Printf("Getting access token for %s... ", ac.ClientId)
21+
c.Printf("Getting access token for %s... ", ac.ClientId)
3022
case profiles.PROFILE_AUTH_TYPE_ACCESS_TOKEN:
31-
p.Printf("Getting profile's stored access token... ")
23+
c.Printf("Getting profile's stored access token... ")
3224
default:
33-
cli.ExitWithError("Invalid auth type", nil)
25+
c.ExitWithError("Invalid auth type", nil)
3426
}
3527
tok, err := auth.GetTokenWithProfile(cmd.Context(), cp)
3628
if err != nil {
37-
p.Println("failed")
29+
c.Println("failed")
3830
cli.ExitWithError("Failed to get token", err)
3931
}
40-
p.Println("ok")
41-
p.Printf("Access Token: %s\n", tok.AccessToken)
32+
c.Println("ok")
33+
c.Printf("Access Token: %s\n", tok.AccessToken)
4234

43-
if jsonOut {
44-
d, err := json.MarshalIndent(tok, "", " ")
45-
if err != nil {
46-
cli.ExitWithError("Failed to marshal token to json", err)
47-
}
48-
49-
fmt.Println(string(d))
50-
return
51-
}
35+
c.PrintIfJson(tok)
5236
}
5337

5438
func init() {

‎cmd/config.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@ import (
1010
)
1111

1212
func config_updateOutput(cmd *cobra.Command, args []string) {
13-
h := NewHandler(cmd)
13+
c := cli.New(cmd, args)
14+
h := NewHandler(c)
1415
defer h.Close()
1516

16-
flagHelper := cli.NewFlagHelper(cmd)
17-
format := flagHelper.GetRequiredString("format")
17+
format := c.Flags.GetRequiredString("format")
1818

1919
config.UpdateOutputFormat(cfgKey, format)
2020
fmt.Println(cli.SuccessMessage(fmt.Sprintf("Output format updated to %s", format)))

‎cmd/dev-selectors.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,16 @@ import (
1515
var (
1616
selectors []string
1717

18-
dev_selectorsCmd *cobra.Command
18+
// dev_selectorsCmd *cobra.Command
1919
)
2020

2121
func dev_selectorsGen(cmd *cobra.Command, args []string) {
22-
h := NewHandler(cmd)
22+
c := cli.New(cmd, args)
23+
h := NewHandler(c)
2324
defer h.Close()
2425

25-
flagHelper := cli.NewFlagHelper(cmd)
26-
subject := flagHelper.GetRequiredString("subject")
27-
contextType := flagHelper.GetRequiredString("type")
26+
subject := c.Flags.GetRequiredString("subject")
27+
contextType := c.Flags.GetRequiredString("type")
2828

2929
var value any
3030
if contextType == "json" || contextType == "" {
@@ -62,13 +62,13 @@ func dev_selectorsGen(cmd *cobra.Command, args []string) {
6262
}
6363

6464
func dev_selectorsTest(cmd *cobra.Command, args []string) {
65-
h := NewHandler(cmd)
65+
c := cli.New(cmd, args)
66+
h := NewHandler(c)
6667
defer h.Close()
6768

68-
flagHelper := cli.NewFlagHelper(cmd)
69-
subject := flagHelper.GetRequiredString("subject")
70-
contextType := flagHelper.GetRequiredString("type")
71-
selectors := flagHelper.GetStringSlice("selectors", selectors, cli.FlagHelperStringSliceOptions{Min: 1})
69+
subject := c.Flags.GetRequiredString("subject")
70+
contextType := c.Flags.GetRequiredString("type")
71+
selectors := c.Flags.GetStringSlice("selectors", selectors, cli.FlagsStringSliceOptions{Min: 1})
7272

7373
var value any
7474
if contextType == "json" || contextType == "" {

‎cmd/dev.go‎

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package cmd
22

33
import (
4-
"encoding/json"
54
"fmt"
65
"io"
76
"os"
@@ -72,16 +71,17 @@ func getMetadataRows(m *common.Metadata) [][]string {
7271
return nil
7372
}
7473

75-
func unMarshalMetadata(m string) *common.MetadataMutable {
76-
if m != "" {
77-
metadata := &common.MetadataMutable{}
78-
if err := json.Unmarshal([]byte(m), metadata); err != nil {
79-
cli.ExitWithError("Failed to unmarshal metadata", err)
80-
}
81-
return metadata
82-
}
83-
return nil
84-
}
74+
// TODO can we use it or remove it?
75+
// func unMarshalMetadata(m string) *common.MetadataMutable {
76+
// if m != "" {
77+
// metadata := &common.MetadataMutable{}
78+
// if err := json.Unmarshal([]byte(m), metadata); err != nil {
79+
// cli.ExitWithError("Failed to unmarshal metadata", err)
80+
// }
81+
// return metadata
82+
// }
83+
// return nil
84+
// }
8585

8686
func getMetadataMutable(labels []string) *common.MetadataMutable {
8787
metadata := common.MetadataMutable{}
@@ -108,13 +108,9 @@ func getMetadataUpdateBehavior() common.MetadataUpdateEnum {
108108

109109
// HandleSuccess prints a success message according to the configured format (styled table or JSON)
110110
func HandleSuccess(command *cobra.Command, id string, t table.Model, policyObject interface{}) {
111+
c := cli.New(command, []string{})
111112
if OtdfctlCfg.Output.Format == config.OutputJSON || configFlagOverrides.OutputFormatJSON {
112-
if output, err := json.MarshalIndent(policyObject, "", " "); err != nil {
113-
cli.ExitWithError("Error marshalling policy object", err)
114-
} else {
115-
fmt.Println(string(output))
116-
}
117-
return
113+
c.ExitWithJson(policyObject)
118114
}
119115
cli.PrintSuccessTable(command, id, t)
120116
}

0 commit comments

Comments
 (0)