Skip to content
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.

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

feat!: config management and error handling #155

Merged
merged 16 commits into from
Aug 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 4 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,21 @@ build
.env
.DS_Store
.idea/
.vscode/
proxies/*.yaml
proxies/*.yml
.temp
temp
shield
!proto/*
.shield.yaml
/config.yaml
dist
coverage.txt
coverage.out
rules/test.yaml
*.pprof
ignore/
vendor/
buf.lock
buf.yaml
/resources_config
/rules
30 changes: 0 additions & 30 deletions .shield.sample.yaml

This file was deleted.

9 changes: 7 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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..."
Expand All @@ -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}'
Expand Down
1 change: 0 additions & 1 deletion buf.gen.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#!/usr/bin/env -S buf generate buf.build/odpf/proton:1497165f2f48facb3ec6f5c5556ccd44f0a7119f --path odpf/shield --template
---
version: "v1"
plugins:
Expand Down
61 changes: 26 additions & 35 deletions cmd/action.go
Original file line number Diff line number Diff line change
@@ -1,20 +1,17 @@
package cmd

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"},
Expand All @@ -29,19 +26,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{
Expand All @@ -59,7 +59,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
}

Expand All @@ -68,16 +68,13 @@ 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.Context(), cliConfig.Host)
if err != nil {
return err
}
defer cancel()

ctx = setCtxHeader(ctx, header)

ctx := setCtxHeader(cmd.Context(), header)
res, err := client.CreateAction(ctx, &shieldv1beta1.CreateActionRequest{
Body: &reqBody,
})
Expand All @@ -86,7 +83,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
},
}
Expand All @@ -99,7 +96,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{
Expand All @@ -117,7 +114,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
}

Expand All @@ -126,16 +123,14 @@ 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.Context(), cliConfig.Host)
if err != nil {
return err
}
defer cancel()

actionID := args[0]
_, err = client.UpdateAction(ctx, &shieldv1beta1.UpdateActionRequest{
_, err = client.UpdateAction(cmd.Context(), &shieldv1beta1.UpdateActionRequest{
Id: actionID,
Body: &reqBody,
})
Expand All @@ -144,7 +139,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
},
}
Expand All @@ -155,7 +150,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",
Expand All @@ -170,16 +165,14 @@ 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.Context(), cliConfig.Host)
if err != nil {
return err
}
defer cancel()

actionID := args[0]
res, err := client.GetAction(ctx, &shieldv1beta1.GetActionRequest{
res, err := client.GetAction(cmd.Context(), &shieldv1beta1.GetActionRequest{
Id: actionID,
})
if err != nil {
Expand Down Expand Up @@ -207,7 +200,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",
Expand All @@ -222,15 +215,13 @@ 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.Context(), cliConfig.Host)
if err != nil {
return err
}
defer cancel()

res, err := client.ListActions(ctx, &shieldv1beta1.ListActionsRequest{})
res, err := client.ListActions(cmd.Context(), &shieldv1beta1.ListActionsRequest{})
if err != nil {
return err
}
Expand Down
120 changes: 120 additions & 0 deletions cmd/action_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
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) {
tt.cliConfig = &cmd.Config{}
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)
})
}
})
}
Loading