fix: refactor to support varying print output

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

@@ -0,0 +1,43 @@
+---
+status: accepted
+date: 2024-08-29
+decision: Use ADRs in the `adr` directory of the repo to document architectural decisions
+author: '@jakedoublev'
+deciders: ['@ryanulit', '@jrschumacher']
+---
+
+# Use a ADR storage format that make diffs easier to read
+
+## Context and Problem Statement
+
+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.
+
+## Decision Drivers
+
+- **Low barrier of entry**: A primary goal of our ADR process is to ensure decisions are captured.
+- **Ease of management**: Make it easy to manage the ADRs.
+- **Ensure appropriate tracking and review**: Make it easy to track and review the changes in the ADRs.
+
+## Considered Options
+
+1. Use Github Issues
+2. Use Github Discussions
+3. Use a shared ADR repository
+4. Use an `adr` directory in the repo
+
+## Decision Outcome
+
+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.
+
+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.
+
+### Consequences
+
+- **Positive**:
+  - Low barrier of entry for developers to document decisions.
+  - Easy to manage and maintain.
+  - Ensures appropriate tracking and review of decisions via git history and code review.
+- **Negative**:
+  - Requires developers to be aware of the ADR process and where to find the ADRs.
+  - May require additional tooling to manage and maintain the ADRs.
+  - May require additional training for developers to understand the ADR process and how to use it effectively.

adr/0001-printing-with-json.md

@@ -0,0 +1,39 @@
+---
+status: accepted
+date: 2024-08-29
+decision: Encapsulate printing to ensure consistent output format
+author: '@jrschumacher'
+deciders: ['@jakedoublev', '@ryanulit', '@suchak1']
+---
+
+# Consistent output format for printing JSON and pretty-print
+
+## Context and Problem Statement
+
+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.
+
+## Decision Drivers
+
+- **Consistency**: Ensure uniform output format across the application.
+- **Flexibility**: Ability to switch between pretty-print and JSON formats based on context.
+- **Ease of Implementation**: Simplicity in implementing and maintaining the solution.
+
+## Considered Options
+
+1. Keep existing code as is
+2. Move the printing into a global function that has context about the CLI flags to drive output format
+
+## Decision Outcome
+
+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.
+
+### Consequences
+
+- **Positive**:
+  - Provides flexibility to switch formats without changing the code.
+  - Ensures consistent output format across different environments.
+  - Simplifies the implementation and maintenance process.
+
+- **Negative**:
+  - Requires careful management of configuration settings.
+  - Potential for misconfiguration leading to incorrect output format when developers use `fmt` directly.

cmd/auth-clientCredentials.go

@@ -1,8 +1,6 @@
 package cmd
 
 import (
-	"fmt"
-
 	"github.com/opentdf/otdfctl/pkg/auth"
 	"github.com/opentdf/otdfctl/pkg/cli"
 	"github.com/opentdf/otdfctl/pkg/man"
@@ -16,9 +14,8 @@ var clientCredentialsCmd = man.Docs.GetCommand("auth/client-credentials",
 )
 
 func auth_clientCredentials(cmd *cobra.Command, args []string) {
-	cp := InitProfile(cmd, false)
-
-	p := cli.NewPrinter(true)
+	c := cli.New(cmd, args)
+	cp := InitProfile(c, false)
 
 	var clientId string
 	var clientSecret string
@@ -45,20 +42,20 @@ func auth_clientCredentials(cmd *cobra.Command, args []string) {
 	})
 
 	// Validate the client credentials
-	p.Printf("Validating client credentials for %s... ", cp.GetEndpoint())
+	c.Printf("Validating client credentials for %s... ", cp.GetEndpoint())
 	if err := auth.ValidateProfileAuthCredentials(cmd.Context(), cp); err != nil {
-		fmt.Println("failed")
-		cli.ExitWithError("An error occurred during login. Please check your credentials and try again", err)
+		c.Println("failed")
+		c.ExitWithError("An error occurred during login. Please check your credentials and try again", err)
 	}
-	p.Println("ok")
+	c.Println("ok")
 
 	// Save the client credentials
-	p.Print("Storing client ID and secret in keyring... ")
+	c.Print("Storing client ID and secret in keyring... ")
 	if err := cp.Save(); err != nil {
-		p.Println("failed")
-		cli.ExitWithError("An error occurred while storing client credentials", err)
+		c.Println("failed")
+		c.ExitWithError("An error occurred while storing client credentials", err)
 	}
-	p.Println("ok")
+	c.Println("ok")
 }
 
 func init() {

cmd/auth-login.go

@@ -9,19 +9,21 @@ import (
 )
 
 func auth_codeLogin(cmd *cobra.Command, args []string) {
-	fh := cli.NewFlagHelper(cmd)
-	clientID := fh.GetOptionalString("client-id")
-	tlsNoVerify := fh.GetOptionalBool("tls-no-verify")
-
-	cp := InitProfile(cmd, false)
-	printer := cli.NewPrinter(true)
-
-	printer.Println("Initiating login...")
-	tok, publicClientID, err := auth.LoginWithPKCE(cmd.Context(), cp.GetEndpoint(), clientID, tlsNoVerify)
+	c := cli.New(cmd, args)
+	cp := InitProfile(c, false)
+
+	c.Print("Initiating login...")
+	tok, publicClientID, err := auth.LoginWithPKCE(
+		cmd.Context(),
+		cp.GetEndpoint(),
+		c.FlagHelper.GetOptionalString("client-id"),
+		c.FlagHelper.GetOptionalBool("tls-no-verify"),
+	)
 	if err != nil {
-		cli.ExitWithError("could not authenticate", err)
+		c.Println("failed")
+		c.ExitWithError("could not authenticate", err)
 	}
-	printer.Println("ok")
+	c.Println("ok")
 
 	// Set the auth credentials to profile
 	if err := cp.SetAuthCredentials(profiles.AuthCredentials{
@@ -33,15 +35,15 @@ func auth_codeLogin(cmd *cobra.Command, args []string) {
 			RefreshToken:   tok.RefreshToken,
 		},
 	}); err != nil {
-		cli.ExitWithError("failed to set auth credentials", err)
+		c.ExitWithError("failed to set auth credentials", err)
 	}
 
-	printer.Println("Storing credentials to profile in keyring...")
+	c.Print("Storing credentials to profile in keyring...")
 	if err := cp.Save(); err != nil {
-		printer.Println("failed")
-		cli.ExitWithError("An error occurred while storing authentication credentials", err)
+		c.Println("failed")
+		c.ExitWithError("An error occurred while storing authentication credentials", err)
 	}
-	printer.Println("ok")
+	c.Println("ok")
 }
 
 var codeLoginCmd *man.Doc

cmd/auth-logout.go

@@ -9,33 +9,31 @@ import (
 )
 
 func auth_logout(cmd *cobra.Command, args []string) {
-	fh := cli.NewFlagHelper(cmd)
-	tlsNoVerify := fh.GetOptionalBool("tls-no-verify")
-	cp := InitProfile(cmd, false)
-	printer := cli.NewPrinter(true)
-	printer.Println("Initiating logout...")
+	c := cli.New(cmd, args)
+	cp := InitProfile(c, false)
+	c.Println("Initiating logout...")
 
 	// we can only revoke access tokens stored for the code login flow, not client credentials
 	creds := cp.GetAuthCredentials()
 	if creds.AuthType == profiles.PROFILE_AUTH_TYPE_ACCESS_TOKEN {
-		printer.Println("Revoking access token...")
+		c.Println("Revoking access token...")
 		if err := auth.RevokeAccessToken(
 			cmd.Context(),
 			cp.GetEndpoint(),
 			creds.AccessToken.PublicClientID,
 			creds.AccessToken.RefreshToken,
-			tlsNoVerify,
+			c.FlagHelper.GetOptionalBool("tls-no-verify"),
 		); err != nil {
-			printer.Println("failed")
-			cli.ExitWithError("An error occurred while revoking the access token", err)
+			c.Println("failed")
+			c.ExitWithError("An error occurred while revoking the access token", err)
 		}
 	}
 
 	if err := cp.SetAuthCredentials(profiles.AuthCredentials{}); err != nil {
-		printer.Println("failed")
-		cli.ExitWithError("An error occurred while logging out", err)
+		c.Println("failed")
+		c.ExitWithError("An error occurred while logging out", err)
 	}
-	printer.Println("ok")
+	c.Println("ok")
 }
 
 var codeLogoutCmd *man.Doc

cmd/auth-printAccessToken.go

@@ -1,9 +1,6 @@
 package cmd
 
 import (
-	"encoding/json"
-	"fmt"
-
 	"github.com/opentdf/otdfctl/pkg/auth"
 	"github.com/opentdf/otdfctl/pkg/cli"
 	"github.com/opentdf/otdfctl/pkg/man"
@@ -15,40 +12,27 @@ var auth_printAccessTokenCmd = man.Docs.GetCommand("auth/print-access-token",
 	man.WithRun(auth_printAccessToken))
 
 func auth_printAccessToken(cmd *cobra.Command, args []string) {
-	flagHelper := cli.NewFlagHelper(cmd)
-	jsonOut := flagHelper.GetOptionalBool("json")
-
-	cp := InitProfile(cmd, false)
-
-	printEnabled := !jsonOut
-	p := cli.NewPrinter(printEnabled)
+	c := cli.New(cmd, args)
+	cp := InitProfile(c, false)
 
 	ac := cp.GetAuthCredentials()
 	switch ac.AuthType {
 	case profiles.PROFILE_AUTH_TYPE_CLIENT_CREDENTIALS:
-		p.Printf("Getting access token for %s... ", ac.ClientId)
+		c.Printf("Getting access token for %s... ", ac.ClientId)
 	case profiles.PROFILE_AUTH_TYPE_ACCESS_TOKEN:
-		p.Printf("Getting profile's stored access token... ")
+		c.Printf("Getting profile's stored access token... ")
 	default:
-		cli.ExitWithError("Invalid auth type", nil)
+		c.ExitWithError("Invalid auth type", nil)
 	}
 	tok, err := auth.GetTokenWithProfile(cmd.Context(), cp)
 	if err != nil {
-		p.Println("failed")
+		c.Println("failed")
 		cli.ExitWithError("Failed to get token", err)
 	}
-	p.Println("ok")
-	p.Printf("Access Token: %s\n", tok.AccessToken)
+	c.Println("ok")
+	c.Printf("Access Token: %s\n", tok.AccessToken)
 
-	if jsonOut {
-		d, err := json.MarshalIndent(tok, "", "  ")
-		if err != nil {
-			cli.ExitWithError("Failed to marshal token to json", err)
-		}
-
-		fmt.Println(string(d))
-		return
-	}
+	c.PrintIfJson(tok)
 }
 
 func init() {

cmd/config.go

@@ -10,11 +10,11 @@ import (
 )
 
 func config_updateOutput(cmd *cobra.Command, args []string) {
-	h := NewHandler(cmd)
+	c := cli.New(cmd, args)
+	h := NewHandler(c)
 	defer h.Close()
 
-	flagHelper := cli.NewFlagHelper(cmd)
-	format := flagHelper.GetRequiredString("format")
+	format := c.Flags.GetRequiredString("format")
 
 	config.UpdateOutputFormat(cfgKey, format)
 	fmt.Println(cli.SuccessMessage(fmt.Sprintf("Output format updated to %s", format)))

cmd/dev-selectors.go

@@ -15,16 +15,16 @@ import (
 var (
 	selectors []string
 
-	dev_selectorsCmd *cobra.Command
+	// dev_selectorsCmd *cobra.Command
 )
 
 func dev_selectorsGen(cmd *cobra.Command, args []string) {
-	h := NewHandler(cmd)
+	c := cli.New(cmd, args)
+	h := NewHandler(c)
 	defer h.Close()
 
-	flagHelper := cli.NewFlagHelper(cmd)
-	subject := flagHelper.GetRequiredString("subject")
-	contextType := flagHelper.GetRequiredString("type")
+	subject := c.Flags.GetRequiredString("subject")
+	contextType := c.Flags.GetRequiredString("type")
 
 	var value any
 	if contextType == "json" || contextType == "" {
@@ -62,13 +62,13 @@ func dev_selectorsGen(cmd *cobra.Command, args []string) {
 }
 
 func dev_selectorsTest(cmd *cobra.Command, args []string) {
-	h := NewHandler(cmd)
+	c := cli.New(cmd, args)
+	h := NewHandler(c)
 	defer h.Close()
 
-	flagHelper := cli.NewFlagHelper(cmd)
-	subject := flagHelper.GetRequiredString("subject")
-	contextType := flagHelper.GetRequiredString("type")
-	selectors := flagHelper.GetStringSlice("selectors", selectors, cli.FlagHelperStringSliceOptions{Min: 1})
+	subject := c.Flags.GetRequiredString("subject")
+	contextType := c.Flags.GetRequiredString("type")
+	selectors := c.Flags.GetStringSlice("selectors", selectors, cli.FlagsStringSliceOptions{Min: 1})
 
 	var value any
 	if contextType == "json" || contextType == "" {

cmd/dev.go

@@ -1,7 +1,6 @@
 package cmd
 
 import (
-	"encoding/json"
 	"fmt"
 	"io"
 	"os"
@@ -72,16 +71,17 @@ func getMetadataRows(m *common.Metadata) [][]string {
 	return nil
 }
 
-func unMarshalMetadata(m string) *common.MetadataMutable {
-	if m != "" {
-		metadata := &common.MetadataMutable{}
-		if err := json.Unmarshal([]byte(m), metadata); err != nil {
-			cli.ExitWithError("Failed to unmarshal metadata", err)
-		}
-		return metadata
-	}
-	return nil
-}
+// TODO can we use it or remove it?
+// func unMarshalMetadata(m string) *common.MetadataMutable {
+// 	if m != "" {
+// 		metadata := &common.MetadataMutable{}
+// 		if err := json.Unmarshal([]byte(m), metadata); err != nil {
+// 			cli.ExitWithError("Failed to unmarshal metadata", err)
+// 		}
+// 		return metadata
+// 	}
+// 	return nil
+// }
 
 func getMetadataMutable(labels []string) *common.MetadataMutable {
 	metadata := common.MetadataMutable{}
@@ -108,12 +108,9 @@ func getMetadataUpdateBehavior() common.MetadataUpdateEnum {
 
 // HandleSuccess prints a success message according to the configured format (styled table or JSON)
 func HandleSuccess(command *cobra.Command, id string, t table.Model, policyObject interface{}) {
+	c := cli.New(command, []string{})
 	if OtdfctlCfg.Output.Format == config.OutputJSON || configFlagOverrides.OutputFormatJSON {
-		if output, err := json.MarshalIndent(policyObject, "", "  "); err != nil {
-			cli.ExitWithError("Error marshalling policy object", err)
-		} else {
-			fmt.Println(string(output))
-		}
+		c.PrintJson(policyObject)
 		return
 	}
 	cli.PrintSuccessTable(command, id, t)