feat!: config management and error handling

.gitignore

@@ -2,6 +2,7 @@ build
 .env
 .DS_Store
 .idea/
+.vscode/
 proxies/*.yaml
 proxies/*.yml
 .temp
@@ -18,3 +19,5 @@ ignore/
 vendor/
 buf.lock
 buf.yaml
+resources_config
+rules

Makefile

@@ -2,6 +2,7 @@ GOVERSION := $(shell go version | cut -d ' ' -f 3 | cut -d '.' -f 2)
 
 .PHONY: build check fmt lint test test-race vet test-cover-html help install proto
 .DEFAULT_GOAL := build
+PROTON_COMMIT := "1497165f2f48facb3ec6f5c5556ccd44f0a7119f"
 
 install:
 	@echo "Clean up imports..."
@@ -26,8 +27,12 @@ coverage: ## print code coverage
 clean :
 	rm -rf dist
 
-proto:
-	./buf.gen.yaml && cp -R proto/odpf/shield/* proto/ && rm -Rf proto/odpf
+proto: ## Generate the protobuf files
+	@echo " > generating protobuf from odpf/proton"
+	@echo " > [info] make sure correct version of dependencies are installed using 'make install'"
+	@buf generate https://github.com/odpf/proton/archive/${PROTON_COMMIT}.zip#strip_components=1 --template buf.gen.yaml --path odpf/shield
+	@cp -R proto/odpf/shield/* proto/ && rm -Rf proto/odpf
+	@echo " > protobuf compilation finished"
 
 help:
 	@grep -E '^[a-zA-Z0-9_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'

buf.gen.yaml

@@ -1,4 +1,3 @@
-#!/usr/bin/env -S buf generate buf.build/odpf/proton:1497165f2f48facb3ec6f5c5556ccd44f0a7119f --path odpf/shield --template
 ---
 version: "v1"
 plugins:

cmd/action.go

@@ -4,17 +4,15 @@ import (
 	"context"
 	"fmt"
 	"os"
-	"strconv"
 
 	"github.com/MakeNowJust/heredoc"
-	"github.com/odpf/salt/log"
 	"github.com/odpf/salt/printer"
-	"github.com/odpf/shield/config"
+	"github.com/odpf/shield/pkg/file"
 	shieldv1beta1 "github.com/odpf/shield/proto/v1beta1"
 	cli "github.com/spf13/cobra"
 )
 
-func ActionCommand(logger log.Logger, appConfig *config.Shield) *cli.Command {
+func ActionCommand(cliConfig *Config) *cli.Command {
 	cmd := &cli.Command{
 		Use:     "action",
 		Aliases: []string{"actions"},
@@ -29,19 +27,22 @@ func ActionCommand(logger log.Logger, appConfig *config.Shield) *cli.Command {
 			$ shield action list
 		`),
 		Annotations: map[string]string{
-			"action:core": "true",
+			"group:core": "true",
+			"client":     "true",
 		},
 	}
 
-	cmd.AddCommand(createActionCommand(logger, appConfig))
-	cmd.AddCommand(editActionCommand(logger, appConfig))
-	cmd.AddCommand(viewActionCommand(logger, appConfig))
-	cmd.AddCommand(listActionCommand(logger, appConfig))
+	cmd.AddCommand(createActionCommand(cliConfig))
+	cmd.AddCommand(editActionCommand(cliConfig))
+	cmd.AddCommand(viewActionCommand(cliConfig))
+	cmd.AddCommand(listActionCommand(cliConfig))
+
+	bindFlagsFromClientConfig(cmd)
 
 	return cmd
 }
 
-func createActionCommand(logger log.Logger, appConfig *config.Shield) *cli.Command {
+func createActionCommand(cliConfig *Config) *cli.Command {
 	var filePath, header string
 
 	cmd := &cli.Command{
@@ -59,7 +60,7 @@ func createActionCommand(logger log.Logger, appConfig *config.Shield) *cli.Comma
 			defer spinner.Stop()
 
 			var reqBody shieldv1beta1.ActionRequestBody
-			if err := parseFile(filePath, &reqBody); err != nil {
+			if err := file.Parse(filePath, &reqBody); err != nil {
 				return err
 			}
 
@@ -68,9 +69,8 @@ func createActionCommand(logger log.Logger, appConfig *config.Shield) *cli.Comma
 				return err
 			}
 
-			host := appConfig.App.Host + ":" + strconv.Itoa(appConfig.App.Port)
 			ctx := context.Background()
-			client, cancel, err := createClient(ctx, host)
+			client, cancel, err := createClient(cmd)
 			if err != nil {
 				return err
 			}
@@ -86,7 +86,7 @@ func createActionCommand(logger log.Logger, appConfig *config.Shield) *cli.Comma
 			}
 
 			spinner.Stop()
-			logger.Info(fmt.Sprintf("successfully created action %s with id %s", res.GetAction().GetName(), res.GetAction().GetId()))
+			fmt.Printf("successfully created action %s with id %s\n", res.GetAction().GetName(), res.GetAction().GetId())
 			return nil
 		},
 	}
@@ -99,7 +99,7 @@ func createActionCommand(logger log.Logger, appConfig *config.Shield) *cli.Comma
 	return cmd
 }
 
-func editActionCommand(logger log.Logger, appConfig *config.Shield) *cli.Command {
+func editActionCommand(cliConfig *Config) *cli.Command {
 	var filePath string
 
 	cmd := &cli.Command{
@@ -117,7 +117,7 @@ func editActionCommand(logger log.Logger, appConfig *config.Shield) *cli.Command
 			defer spinner.Stop()
 
 			var reqBody shieldv1beta1.ActionRequestBody
-			if err := parseFile(filePath, &reqBody); err != nil {
+			if err := file.Parse(filePath, &reqBody); err != nil {
 				return err
 			}
 
@@ -126,9 +126,8 @@ func editActionCommand(logger log.Logger, appConfig *config.Shield) *cli.Command
 				return err
 			}
 
-			host := appConfig.App.Host + ":" + strconv.Itoa(appConfig.App.Port)
 			ctx := context.Background()
-			client, cancel, err := createClient(ctx, host)
+			client, cancel, err := createClient(cmd)
 			if err != nil {
 				return err
 			}
@@ -144,7 +143,7 @@ func editActionCommand(logger log.Logger, appConfig *config.Shield) *cli.Command
 			}
 
 			spinner.Stop()
-			logger.Info(fmt.Sprintf("successfully edited action with id %s", actionID))
+			fmt.Printf("successfully edited action with id %s\n", actionID)
 			return nil
 		},
 	}
@@ -155,7 +154,7 @@ func editActionCommand(logger log.Logger, appConfig *config.Shield) *cli.Command
 	return cmd
 }
 
-func viewActionCommand(logger log.Logger, appConfig *config.Shield) *cli.Command {
+func viewActionCommand(cliConfig *Config) *cli.Command {
 	cmd := &cli.Command{
 		Use:   "view",
 		Short: "View an action",
@@ -170,9 +169,8 @@ func viewActionCommand(logger log.Logger, appConfig *config.Shield) *cli.Command
 			spinner := printer.Spin("")
 			defer spinner.Stop()
 
-			host := appConfig.App.Host + ":" + strconv.Itoa(appConfig.App.Port)
 			ctx := context.Background()
-			client, cancel, err := createClient(ctx, host)
+			client, cancel, err := createClient(cmd)
 			if err != nil {
 				return err
 			}
@@ -207,7 +205,7 @@ func viewActionCommand(logger log.Logger, appConfig *config.Shield) *cli.Command
 	return cmd
 }
 
-func listActionCommand(logger log.Logger, appConfig *config.Shield) *cli.Command {
+func listActionCommand(cliConfig *Config) *cli.Command {
 	cmd := &cli.Command{
 		Use:   "list",
 		Short: "List all actions",
@@ -222,9 +220,8 @@ func listActionCommand(logger log.Logger, appConfig *config.Shield) *cli.Command
 			spinner := printer.Spin("")
 			defer spinner.Stop()
 
-			host := appConfig.App.Host + ":" + strconv.Itoa(appConfig.App.Port)
 			ctx := context.Background()
-			client, cancel, err := createClient(ctx, host)
+			client, cancel, err := createClient(cmd)
 			if err != nil {
 				return err
 			}

cmd/action_test.go

@@ -0,0 +1,119 @@
+package cmd_test
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/MakeNowJust/heredoc"
+	"github.com/odpf/shield/cmd"
+	"github.com/stretchr/testify/assert"
+)
+
+var expectedActionUsageHelp = heredoc.Doc(`
+
+USAGE
+  shield action [flags]
+
+CORE COMMANDS
+  create      Create an action
+  edit        Edit an action
+  list        List all actions
+  view        View an action
+
+FLAGS
+  -h, --host string   Shield API service to connect to
+
+INHERITED FLAGS
+  --help   Show help for command
+
+EXAMPLES
+  $ shield action create
+  $ shield action edit
+  $ shield action view
+  $ shield action list
+
+`)
+
+func TestClientAction(t *testing.T) {
+	t.Run("without config file", func(t *testing.T) {
+		tests := []struct {
+			name        string
+			cliConfig   *cmd.Config
+			subCommands []string
+			want        string
+			err         error
+		}{
+			{
+				name: "`action` only should show usage help",
+				want: expectedActionUsageHelp,
+				err:  nil,
+			},
+			{
+				name:        "`action` list only should throw error host not found",
+				want:        "",
+				subCommands: []string{"list"},
+				err:         cmd.ErrClientConfigHostNotFound,
+			},
+			{
+				name:        "`action` list with host flag should pass",
+				want:        "",
+				subCommands: []string{"list", "-h", "test"},
+				err:         context.DeadlineExceeded,
+			},
+			{
+				name:        "`action` create only should throw error host not found",
+				want:        "",
+				subCommands: []string{"create"},
+				err:         cmd.ErrClientConfigHostNotFound,
+			},
+			{
+				name:        "`action` create with host flag should throw error missing required flag",
+				want:        "",
+				subCommands: []string{"create", "-h", "test"},
+				err:         errors.New("required flag(s) \"file\", \"header\" not set"),
+			},
+			{
+				name:        "`action` edit without host should throw error host not found",
+				want:        "",
+				subCommands: []string{"edit", "123"},
+				err:         cmd.ErrClientConfigHostNotFound,
+			},
+			{
+				name:        "`action` edit with host flag should throw error missing required flag",
+				want:        "",
+				subCommands: []string{"edit", "123", "-h", "test"},
+				err:         errors.New("required flag(s) \"file\" not set"),
+			},
+			{
+				name:        "`action` view without host should throw error host not found",
+				want:        "",
+				subCommands: []string{"view", "123"},
+				err:         cmd.ErrClientConfigHostNotFound,
+			},
+			{
+				name:        "`action` view with host flag should pass",
+				want:        "",
+				subCommands: []string{"view", "123", "-h", "test"},
+				err:         context.DeadlineExceeded,
+			},
+		}
+		for _, tt := range tests {
+			t.Run(tt.name, func(t *testing.T) {
+				cli := cmd.New(tt.cliConfig)
+
+				buf := new(bytes.Buffer)
+				cli.SetOutput(buf)
+				args := append([]string{"action"}, tt.subCommands...)
+				cli.SetArgs(args)
+
+				err := cli.Execute()
+				got := buf.String()
+
+				assert.Equal(t, tt.err, err)
+				assert.Equal(t, tt.want, got)
+			})
+		}
+	})
+}

cmd/client.go

@@ -5,20 +5,27 @@ import (
 	"time"
 
 	shieldv1beta1 "github.com/odpf/shield/proto/v1beta1"
+	"github.com/spf13/cobra"
 	"google.golang.org/grpc"
+	"google.golang.org/grpc/credentials/insecure"
 )
 
 func createConnection(ctx context.Context, host string) (*grpc.ClientConn, error) {
 	opts := []grpc.DialOption{
-		grpc.WithInsecure(),
+		grpc.WithTransportCredentials(insecure.NewCredentials()),
 		grpc.WithBlock(),
 	}
 
 	return grpc.DialContext(ctx, host, opts...)
 }
 
-func createClient(ctx context.Context, host string) (shieldv1beta1.ShieldServiceClient, func(), error) {
-	dialTimeoutCtx, dialCancel := context.WithTimeout(ctx, time.Second*2)
+func createClient(cmd *cobra.Command) (shieldv1beta1.ShieldServiceClient, func(), error) {
+	dialTimeoutCtx, dialCancel := context.WithTimeout(cmd.Context(), time.Second*2)
+	host, err := cmd.Flags().GetString("host")
+	if err != nil {
+		dialCancel()
+		return nil, nil, err
+	}
 	conn, err := createConnection(dialTimeoutCtx, host)
 	if err != nil {
 		dialCancel()
@@ -33,3 +40,23 @@ func createClient(ctx context.Context, host string) (shieldv1beta1.ShieldService
 	client := shieldv1beta1.NewShieldServiceClient(conn)
 	return client, cancel, nil
 }
+
+func isClientCLI(cmd *cobra.Command) bool {
+	for c := cmd; c.Parent() != nil; c = c.Parent() {
+		if c.Annotations != nil && c.Annotations["client"] == "true" {
+			return true
+		}
+	}
+	return false
+}
+
+func clientConfigHostExist(cmd *cobra.Command) bool {
+	host, err := cmd.Flags().GetString("host")
+	if err != nil {
+		return false
+	}
+	if host != "" {
+		return true
+	}
+	return false
+}