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

cmd: adds env var backups to command flags #6508

Merged
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.
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
6 changes: 5 additions & 1 deletion cmd/bench.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/olekukonko/tablewriter"
"github.com/spf13/cobra"

"github.com/open-policy-agent/opa/cmd/internal/env"
"github.com/open-policy-agent/opa/compile"
"github.com/open-policy-agent/opa/internal/presentation"
"github.com/open-policy-agent/opa/metrics"
Expand Down Expand Up @@ -90,7 +91,10 @@ To run benchmarks against a running OPA server to evaluate server overhead use t
The optional "gobench" output format conforms to the Go Benchmark Data Format.
`,

PreRunE: func(_ *cobra.Command, args []string) error {
PreRunE: func(cmd *cobra.Command, args []string) error {
if err := env.CmdFlags.CheckEnvironmentVariables(cmd); err != nil {
return err
}
return validateEvalParams(&params.evalCommandParams, args)
},
Run: func(_ *cobra.Command, args []string) {
Expand Down
3 changes: 2 additions & 1 deletion cmd/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/bundle"
"github.com/open-policy-agent/opa/cmd/internal/env"
"github.com/open-policy-agent/opa/compile"
"github.com/open-policy-agent/opa/keys"
"github.com/open-policy-agent/opa/util"
Expand Down Expand Up @@ -219,7 +220,7 @@ against OPA v0.22.0:
if len(args) == 0 {
return fmt.Errorf("expected at least one path")
}
return nil
return env.CmdFlags.CheckEnvironmentVariables(Cmd)
},
Run: func(cmd *cobra.Command, args []string) {
if err := dobuild(buildParams, args); err != nil {
Expand Down
4 changes: 4 additions & 0 deletions cmd/capabilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"strings"

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/cmd/internal/env"
"github.com/spf13/cobra"
)

Expand Down Expand Up @@ -69,6 +70,9 @@ Print the capabilities of a capabilities file
}

`,
PreRunE: func(cmd *cobra.Command, args []string) error {
return env.CmdFlags.CheckEnvironmentVariables(cmd)
},
RunE: func(*cobra.Command, []string) error {
cs, err := doCapabilities(capabilitiesParams)
if err != nil {
Expand Down
5 changes: 3 additions & 2 deletions cmd/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/spf13/cobra"

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/cmd/internal/env"
pr "github.com/open-policy-agent/opa/internal/presentation"
"github.com/open-policy-agent/opa/loader"
"github.com/open-policy-agent/opa/util"
Expand Down Expand Up @@ -158,11 +159,11 @@ func init() {
is produced. If the parsing or compiling fails, 'check' will output the errors
and exit with a non-zero exit code.`,

PreRunE: func(_ *cobra.Command, args []string) error {
PreRunE: func(cmd *cobra.Command, args []string) error {
if len(args) == 0 {
return fmt.Errorf("specify at least one file")
}
return nil
return env.CmdFlags.CheckEnvironmentVariables(cmd)
},

Run: func(_ *cobra.Command, args []string) {
Expand Down
3 changes: 2 additions & 1 deletion cmd/deps.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/spf13/cobra"

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/cmd/internal/env"
"github.com/open-policy-agent/opa/loader"
"github.com/open-policy-agent/opa/util"
)
Expand Down Expand Up @@ -93,7 +94,7 @@ data.policy.is_admin.
if len(args) != 1 {
return errors.New("specify exactly one query argument")
}
return nil
return env.CmdFlags.CheckEnvironmentVariables(cmd)
},
Run: func(cmd *cobra.Command, args []string) {
if err := deps(args, params, os.Stdout); err != nil {
Expand Down
6 changes: 5 additions & 1 deletion cmd/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/ast/location"
"github.com/open-policy-agent/opa/bundle"
"github.com/open-policy-agent/opa/cmd/internal/env"
"github.com/open-policy-agent/opa/compile"
"github.com/open-policy-agent/opa/cover"
fileurl "github.com/open-policy-agent/opa/internal/file/url"
Expand Down Expand Up @@ -270,7 +271,10 @@ access.
`,

PreRunE: func(cmd *cobra.Command, args []string) error {
return validateEvalParams(&params, args)
if err := validateEvalParams(&params, args); err != nil {
return err
}
return env.CmdFlags.CheckEnvironmentVariables(cmd)
},
Run: func(cmd *cobra.Command, args []string) {

Expand Down
4 changes: 4 additions & 0 deletions cmd/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/spf13/cobra"

"github.com/open-policy-agent/opa/cmd/internal/env"
"github.com/open-policy-agent/opa/cmd/internal/exec"
"github.com/open-policy-agent/opa/internal/config"
internal_logging "github.com/open-policy-agent/opa/internal/logging"
Expand Down Expand Up @@ -52,6 +53,9 @@ specifying the --decision argument and pointing at a specific policy decision,
e.g., opa exec --decision /foo/bar/baz ...`,

Args: cobra.MinimumNArgs(1),
PreRunE: func(cmd *cobra.Command, args []string) error {
return env.CmdFlags.CheckEnvironmentVariables(cmd)
},
Run: func(cmd *cobra.Command, args []string) {
params.Paths = args
params.BundlePaths = bundlePaths.v
Expand Down
4 changes: 4 additions & 0 deletions cmd/fmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/spf13/cobra"

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/cmd/internal/env"
"github.com/open-policy-agent/opa/format"
fileurl "github.com/open-policy-agent/opa/internal/file/url"
)
Expand Down Expand Up @@ -63,6 +64,9 @@ to stdout from the 'fmt' command.

If the '--fail' option is supplied, the 'fmt' command will return a non zero exit
code if a file would be reformatted.`,
PreRunE: func(cmd *cobra.Command, args []string) error {
return env.CmdFlags.CheckEnvironmentVariables(cmd)
},
Run: func(cmd *cobra.Command, args []string) {
os.Exit(opaFmt(args))
},
Expand Down
8 changes: 6 additions & 2 deletions cmd/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/bundle"
"github.com/open-policy-agent/opa/cmd/internal/env"
ib "github.com/open-policy-agent/opa/internal/bundle/inspect"
pr "github.com/open-policy-agent/opa/internal/presentation"
iStrs "github.com/open-policy-agent/opa/internal/strings"
Expand Down Expand Up @@ -78,8 +79,11 @@ Example:
You can provide exactly one OPA bundle or path to the 'inspect' command on the command-line. If you provide a path
referring to a directory, the 'inspect' command will load that path as a bundle and summarize its structure and contents.
`,
PreRunE: func(_ *cobra.Command, args []string) error {
return validateInspectParams(&params, args)
PreRunE: func(cmd *cobra.Command, args []string) error {
if err := validateInspectParams(&params, args); err != nil {
return err
}
return env.CmdFlags.CheckEnvironmentVariables(cmd)
},
Run: func(_ *cobra.Command, args []string) {
if err := doInspect(params, args[0], os.Stdout); err != nil {
Expand Down
50 changes: 50 additions & 0 deletions cmd/internal/env/env.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package env

import (
"fmt"
"strings"

"github.com/spf13/cobra"
"github.com/spf13/pflag"
"github.com/spf13/viper"
)

type cmdFlags interface {
CheckEnvironmentVariables(command *cobra.Command) error
}

type cmdFlagsImpl struct{}

var (
CmdFlags cmdFlags = cmdFlagsImpl{}
errorMessagePrefix = "error mapping environment variables to command flags"
)

const globalPrefix = "opa"

func (cf cmdFlagsImpl) CheckEnvironmentVariables(command *cobra.Command) error {
var errs []string
v := viper.New()
v.AutomaticEnv()
if command.Name() == globalPrefix {
v.SetEnvPrefix(command.Name())
} else {
v.SetEnvPrefix(fmt.Sprintf("%s_%s", globalPrefix, command.Name()))
}
command.Flags().VisitAll(func(f *pflag.Flag) {
configName := f.Name
configName = strings.Replace(configName, "-", "_", -1)
if !f.Changed && v.IsSet(configName) {
val := v.Get(configName)
err := command.Flags().Set(f.Name, fmt.Sprintf("%v", val))
if err != nil {
errs = append(errs, err.Error())
}
}
})

if len(errs) == 0 {
return nil
}
return fmt.Errorf("%s: %s", errorMessagePrefix, strings.Join(errs, "; "))
}
175 changes: 175 additions & 0 deletions cmd/internal/env/env_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
package env
anderseknert marked this conversation as resolved.
Show resolved Hide resolved

import (
"bytes"
"fmt"
"io"
"strings"
"testing"

"github.com/spf13/cobra"
)

func mockRootCmd(writer io.Writer) *cobra.Command {
var rootArgs struct {
IntFlag int
StrFlag string
BoolFlag bool
}
cmd := cobra.Command{
Use: "opa [opts]",
Short: "test root command",
Long: `test root command`,
PreRunE: func(cmd *cobra.Command, args []string) error {
return CmdFlags.CheckEnvironmentVariables(cmd)
},
Run: func(cmd *cobra.Command, args []string) {
fmt.Fprintf(writer, "%v; %v; %v", rootArgs.IntFlag, rootArgs.StrFlag, rootArgs.BoolFlag)
},
}
cmd.Flags().IntVarP(&rootArgs.IntFlag, "int", "i", 0, "set int")
cmd.Flags().StringVarP(&rootArgs.StrFlag, "some-string", "s", "", "set string")
cmd.Flags().BoolVarP(&rootArgs.BoolFlag, "bool", "b", false, "set bool")
return &cmd
}

func mockChildCmd(writer io.Writer) *cobra.Command {
var rootArgs struct {
IntFlag int
StrFlag string
BoolFlag bool
}
cmd := cobra.Command{
Use: "child [opts]",
Short: "test child command",
Long: `test child command`,
PreRunE: func(cmd *cobra.Command, args []string) error {
return CmdFlags.CheckEnvironmentVariables(cmd)
},
Run: func(cmd *cobra.Command, args []string) {
fmt.Fprintf(writer, "%v; %v; %v", rootArgs.IntFlag, rootArgs.StrFlag, rootArgs.BoolFlag)
},
}
cmd.Flags().IntVarP(&rootArgs.IntFlag, "second-int", "i", 100, "set int")
cmd.Flags().StringVarP(&rootArgs.StrFlag, "second-string", "s", "child-string", "set string")
cmd.Flags().BoolVarP(&rootArgs.BoolFlag, "second-bool", "b", true, "set bool")
return &cmd
}

func TestCmdFlagsImpl_CheckEnvironmentVariables_NoEnvVarsSingleCommand(t *testing.T) {
rootWriter := bytes.NewBuffer([]byte{})
root := mockRootCmd(rootWriter)
if err := root.PreRunE(root, []string{}); err != nil {
t.Fatalf("unexpected error: %s", err.Error())
}
root.Run(root, []string{})
out := rootWriter.String()
expectation := "0; ; false"
if out != expectation {
t.Fatalf("expected default flag values %q, got %q", expectation, out)
}
}

func TestCmdFlagsImpl_CheckEnvironmentVariables_OneEnvVarsSingleCommand(t *testing.T) {
rootWriter := bytes.NewBuffer([]byte{})
root := mockRootCmd(rootWriter)
t.Setenv("OPA_INT", "3")
if err := root.PreRunE(root, []string{}); err != nil {
t.Fatalf("unexpected error: %s", err.Error())
}
root.Run(root, []string{})
out := rootWriter.String()
expectation := "3; ; false"
if out != expectation {
t.Fatalf("expected flag values %q, got %q", expectation, out)
}
}

func TestCmdFlagsImpl_CheckEnvironmentVariables_AllEnvVarsSingleCommand(t *testing.T) {
rootWriter := bytes.NewBuffer([]byte{})
root := mockRootCmd(rootWriter)
t.Setenv("OPA_INT", "40")
t.Setenv("OPA_SOME_STRING", "test")
t.Setenv("OPA_BOOL", "true")
if err := root.PreRunE(root, []string{}); err != nil {
t.Fatalf("unexpected error: %s", err.Error())
}
root.Run(root, []string{})
out := rootWriter.String()
expectation := "40; test; true"
if out != expectation {
t.Fatalf("expected flag values %q, got %q", expectation, out)
}
}

func TestCmdFlagsImpl_CheckEnvironmentVariables_ChildCommandAllEnvVars(t *testing.T) {
root := mockRootCmd(&bytes.Buffer{})
childWriter := bytes.NewBuffer([]byte{})
child := mockChildCmd(childWriter)
root.AddCommand(child)
t.Setenv("OPA_CHILD_SECOND_INT", "7")
t.Setenv("OPA_CHILD_SECOND_STRING", "testing child")
t.Setenv("OPA_CHILD_SECOND_BOOL", "false")
if err := child.PreRunE(child, []string{}); err != nil {
t.Fatalf("unexpected error: %s", err.Error())
}
child.Run(child, []string{})
childOut := childWriter.String()
childExpectation := "7; testing child; false"
if childOut != childExpectation {
t.Fatalf("expected child flag values %q, got %q", childExpectation, childOut)
}
}

func TestCmdFlagsImpl_CheckEnvironmentVariables_ChildCommandReturnsSingleErr(t *testing.T) {
root := mockRootCmd(&bytes.Buffer{})
child := mockChildCmd(&bytes.Buffer{})
root.AddCommand(child)
t.Setenv("OPA_CHILD_SECOND_BOOL", "7")
err := child.PreRunE(child, []string{})
if err == nil {
t.Fatalf("expected error, found none")
}
expectedString := "invalid argument \"7\""
if !strings.Contains(err.Error(), expectedString) {
t.Fatalf("expected error to include %q, instead got %q", expectedString, err.Error())
}
}

func TestCmdFlagsImpl_CheckEnvironmentVariables_ChildCommandReturnsMultipleErr(t *testing.T) {
root := mockRootCmd(&bytes.Buffer{})
child := mockChildCmd(&bytes.Buffer{})
root.AddCommand(child)
t.Setenv("OPA_CHILD_SECOND_INT", "true")
t.Setenv("OPA_CHILD_SECOND_BOOL", "7")
err := child.PreRunE(child, []string{"child"})
expectedString := "invalid argument"
if err == nil {
t.Fatalf("expected error, found none")
}
if !strings.Contains(err.Error(), expectedString) {
t.Fatalf("expected error to include %q, instead got %q", expectedString, err.Error())
}
if !strings.Contains(err.Error(), "7") {
t.Fatalf("expected error for invalid int 7 as argument for boolean flag")
}
if !strings.Contains(err.Error(), "true") {
t.Fatalf("expected error for invalid int 7 as argument for int flag")
}
}

func TestCmdFlagsImpl_CheckEnvironmentVariables_ConfirmCommandFlagPrecedence(t *testing.T) {
anderseknert marked this conversation as resolved.
Show resolved Hide resolved
rootWriter := bytes.NewBuffer([]byte{})
root := mockRootCmd(rootWriter)
t.Setenv("OPA_INT", "3")
t.Setenv("OPA_BOOL", "true")
root.SetArgs([]string{"-i", "42"})
if err := root.Execute(); err != nil {
t.Fatalf("unexpected error: %s", err.Error())
}
out := rootWriter.String()
expectation := "42; ; true"
if out != expectation {
t.Fatalf("expected flag values %q, got %q", expectation, out)
}
}