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

Fix pre command flags #13995

Merged
merged 2 commits into from
Apr 21, 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
2 changes: 1 addition & 1 deletion cmd/minikube/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ func setFlags(parse bool) {

// setLastStartFlags sets the log_file flag to lastStart.txt if start command and user doesn't specify log_file or log_dir flags.
func setLastStartFlags() {
if len(os.Args) < 2 || os.Args[1] != "start" {
if pflag.Arg(0) != "start" {
return
}
if pflag.CommandLine.Changed("log_file") || pflag.CommandLine.Changed("log_dir") {
Expand Down
23 changes: 6 additions & 17 deletions pkg/minikube/audit/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"strings"
"time"

"github.com/spf13/pflag"
"github.com/spf13/viper"
"k8s.io/klog/v2"
"k8s.io/minikube/pkg/minikube/config"
Expand Down Expand Up @@ -52,10 +53,10 @@ func args() string {

// Log details about the executed command.
func Log(startTime time.Time) {
if len(os.Args) < 2 || !shouldLog() {
if !shouldLog() {
return
}
r := newRow(os.Args[1], args(), userName(), version.GetVersion(), startTime, time.Now())
r := newRow(pflag.Arg(0), args(), userName(), version.GetVersion(), startTime, time.Now())
if err := appendToLog(r); err != nil {
klog.Warning(err)
}
Expand All @@ -64,7 +65,7 @@ func Log(startTime time.Time) {
// shouldLog returns if the command should be logged.
func shouldLog() bool {
// in rare chance we get here without a command, don't log
if len(os.Args) < 2 {
if pflag.NArg() == 0 {
return false
}

Expand All @@ -74,7 +75,7 @@ func shouldLog() bool {

// commands that should not be logged.
no := []string{"status", "version"}
a := os.Args[1]
a := pflag.Arg(0)
for _, c := range no {
if a == c {
return false
Expand All @@ -85,17 +86,5 @@ func shouldLog() bool {

// isDeletePurge return true if command is delete with purge flag.
func isDeletePurge() bool {
args := os.Args
if len(args) < 2 {
return false
}
if args[1] != "delete" {
return false
}
for _, a := range args {
if a == "--purge" {
return true
}
}
return false
return pflag.Arg(0) == "delete" && viper.GetBool("purge")
}
45 changes: 37 additions & 8 deletions pkg/minikube/audit/audit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"testing"
"time"

"github.com/spf13/pflag"
"github.com/spf13/viper"
"k8s.io/minikube/pkg/minikube/config"
)
Expand Down Expand Up @@ -88,8 +89,11 @@ func TestAudit(t *testing.T) {
})

t.Run("shouldLog", func(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()
oldCommandLine := pflag.CommandLine
defer func() {
pflag.CommandLine = oldCommandLine
pflag.Parse()
}()

tests := []struct {
args []string
Expand Down Expand Up @@ -122,19 +126,22 @@ func TestAudit(t *testing.T) {
}

for _, test := range tests {
os.Args = test.args
mockArgs(t, test.args)

got := shouldLog()

if got != test.want {
t.Errorf("os.Args = %q; shouldLog() = %t; want %t", os.Args, got, test.want)
t.Errorf("test.args = %q; shouldLog() = %t; want %t", test.args, got, test.want)
}
}
})

t.Run("isDeletePurge", func(t *testing.T) {
oldArgs := os.Args
defer func() { os.Args = oldArgs }()
oldCommandLine := pflag.CommandLine
defer func() {
pflag.CommandLine = oldCommandLine
pflag.Parse()
}()

tests := []struct {
args []string
Expand All @@ -159,12 +166,12 @@ func TestAudit(t *testing.T) {
}

for _, test := range tests {
os.Args = test.args
mockArgs(t, test.args)

got := isDeletePurge()

if got != test.want {
t.Errorf("os.Args = %q; isDeletePurge() = %t; want %t", os.Args, got, test.want)
t.Errorf("test.args = %q; isDeletePurge() = %t; want %t", test.args, got, test.want)
}
}
})
Expand All @@ -175,6 +182,28 @@ func TestAudit(t *testing.T) {
defer func() { os.Args = oldArgs }()
os.Args = []string{"minikube"}

oldCommandLine := pflag.CommandLine
defer func() {
pflag.CommandLine = oldCommandLine
pflag.Parse()
}()
mockArgs(t, os.Args)

Log(time.Now())
})
}

func mockArgs(t *testing.T, args []string) {
if len(args) == 0 {
t.Fatalf("cannot pass an empty slice to mockArgs")
}
fs := pflag.NewFlagSet(args[0], pflag.ExitOnError)
fs.Bool("purge", false, "")
if err := fs.Parse(args[1:]); err != nil {
t.Fatal(err)
}
pflag.CommandLine = fs
if err := viper.BindPFlags(pflag.CommandLine); err != nil {
t.Fatal(err)
}
}