Repository: kubernetes/minikube
Pull request #13995 (2022-04-20 23:42:02)

Fix pre command flags
Fixes https://github.com/kubernetes/minikube/issues/13994

**Problem:**
Having a flag before the command (ex. `minikube --alsologtostderr start`) would break instances that were coded to look for the command using `os.Args[1]`. This would result in issues such as not creating a `lastStart.txt` and not properly detecting if a purge delete was occurring.

**Solution:**
Replaced instances of `os.Args[1]` with `pflag.Arg(0)`. `pflag.Arg` returns all args from `os.Args[1:]` that aren't flags, resulting in only true args (ex. `minikube --alsologtostderr start --driver docker` -> `pflag.Args() = ["start"]`). `pflag.Arg()` will also return an empty string if trying to access a non-existing arg index.


Patch:
diff --git a/cmd/minikube/main.go b/cmd/minikube/main.go
index 6f12f844b..b7a56d375 100644
--- a/cmd/minikube/main.go
+++ b/cmd/minikube/main.go
@@ -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") {
diff --git a/pkg/minikube/audit/audit.go b/pkg/minikube/audit/audit.go
index 7deb87972..eb4b0ead7 100644
--- a/pkg/minikube/audit/audit.go
+++ b/pkg/minikube/audit/audit.go
@@ -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"
@@ -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)
 	}
@@ -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
 	}
 
@@ -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
@@ -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")
 }


Test patch:
diff --git a/pkg/minikube/audit/audit_test.go b/pkg/minikube/audit/audit_test.go
index 7c36ae655..e91f7d972 100644
--- a/pkg/minikube/audit/audit_test.go
+++ b/pkg/minikube/audit/audit_test.go
@@ -22,6 +22,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/spf13/pflag"
 	"github.com/spf13/viper"
 	"k8s.io/minikube/pkg/minikube/config"
 )
@@ -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
@@ -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
@@ -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)
 			}
 		}
 	})
@@ -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)
+	}
+}