Fix arg parsing when providing long short flags (#390)

* update flag parsing lib

* test unmanaged arg parsing

* small self review fixes

cli_test.go

@@ -177,3 +177,126 @@ func TestNewApplicationWithOptionsAndAliases(t *testing.T) {
 		})
 	}
 }
+
+func TestApplicationUnmanagedArgs(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name              string   // The name of the test to find the failing one
+		args              []string // The args passed to the Application
+		expectedUnmanaged []string // What we expect to be marked as unmanaged
+	}{
+		{
+			"tgf leave every unmanaged arg intact after its own args",
+			[]string{"--temp-location", "host", "-var", "region=us-west-2", "-auto-approve"},
+			[]string{"-var", "region=us-west-2", "-auto-approve"},
+		},
+		{
+			"tgf leave every unmanaged arg intact when they're before its own args",
+			[]string{"-var", "region=us-west-2", "-auto-approve", "--temp-location", "host"},
+			[]string{"-var", "region=us-west-2", "-auto-approve"},
+		},
+		{
+			// This test is a safety to catch if someone adds a -v flag to not break
+			// Terraform "-var" single dash flag. It will crash trying to parse -ar
+			// because the underlying flag parser will consider -var as equivalent to
+			// writing "-v -a -r" and so may try to continue reading "-a" and "-r" or
+			// "ar" as a value
+			"[case 1] tgf leaves every argument unmanaged",
+			[]string{"-v", "-a", "-r", "-var", "region=us-west-2", "-auto-approve"},
+			[]string{"-v", "-a", "-r", "-var", "region=us-west-2", "-auto-approve"},
+		},
+		{
+			"[case 2] tgf leaves every argument unmanaged",
+			[]string{"apply", "-auto-approve", "-var", "region=us-west-2"},
+			[]string{"apply", "-auto-approve", "-var", "region=us-west-2"},
+		},
+		{
+			"[case 3] tgf leaves every argument unmanaged",
+			[]string{"plan", "plan-all", "apply", "apply-all"},
+			[]string{"plan", "plan-all", "apply", "apply-all"},
+		},
+		{
+			"[case 4] tgf leaves every argument unmanaged",
+			[]string{"output-all", "destroy-all", "-profile", "aprofile"},
+			[]string{"output-all", "destroy-all", "-profile", "aprofile"},
+		},
+		{
+			"tgf catches its own --profile flag",
+			[]string{"output-all", "destroy-all", "--profile", "aprofile"},
+			[]string{"output-all", "destroy-all"},
+		},
+		{
+			"tgf catches its own -P (short flag for --profile)",
+			[]string{"output-all", "destroy-all", "-P", "aprofile"},
+			[]string{"output-all", "destroy-all"},
+		},
+		{
+			"tgf leaves Terragrunt options unmanaged",
+			[]string{
+				// One in two purposely have double dash just to play with parsing
+				// to make sure they always get parsed correctly
+				"plan",
+				"-terragrunt-config", "somevalue",
+				"--terragrunt-tfpath", "somevalue",
+				"-terragrunt-non-interactive", "somevalue",
+				"--terragrunt-working-dir", "somevalue",
+				"-terragrunt-source", "somevalue",
+				"--terragrunt-source-update", "somevalue",
+				"-terragrunt-ignore-dependency-errors", "somevalue",
+				"--terragrunt-logging-level", "somevalue",
+				"-terragrunt-logging-file-dir", "somevalue",
+				"--terragrunt-logging-file-level", "somevalue",
+				"-terragrunt-approval", "somevalue",
+				"--terragrunt-flush-delay", "somevalue",
+				"-terragrunt-workers", "somevalue",
+				"--terragrunt-include-empty-folders", "somevalue",
+			},
+			[]string{
+				"plan",
+				"-terragrunt-config", "somevalue",
+				"--terragrunt-tfpath", "somevalue",
+				"-terragrunt-non-interactive", "somevalue",
+				"--terragrunt-working-dir", "somevalue",
+				"-terragrunt-source", "somevalue",
+				"--terragrunt-source-update", "somevalue",
+				"-terragrunt-ignore-dependency-errors", "somevalue",
+				"--terragrunt-logging-level", "somevalue",
+				"-terragrunt-logging-file-dir", "somevalue",
+				"--terragrunt-logging-file-level", "somevalue",
+				"-terragrunt-approval", "somevalue",
+				"--terragrunt-flush-delay", "somevalue",
+				"-terragrunt-workers", "somevalue",
+				"--terragrunt-include-empty-folders", "somevalue",
+			},
+		},
+		{
+			"tgf leaves Terraform commands and options unmanaged",
+			// Testing only a subset and mostly those we use and haven't tested yet
+			// and those which have dashes
+			[]string{
+				"destroy",
+				"force-unlock",
+				"-chdir=DIR",
+				"-help",
+				"-version",
+			},
+			[]string{
+				"destroy",
+				"force-unlock",
+				"-chdir=DIR",
+				"-help",
+				"-version",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.NotPanics(t, func() {
+				app := NewTestApplication(tt.args, false)
+				assert.Equal(t, tt.expectedUnmanaged, app.Unmanaged, "Unmanaged args are not equal")
+			})
+		})
+	}
+}

go.mod

@@ -14,7 +14,7 @@ require (
 	github.com/blang/semver/v4 v4.0.0
 	github.com/coveooss/gotemplate/v3 v3.7.3
 	github.com/coveooss/multilogger v0.5.2
-	github.com/coveord/kingpin/v2 v2.4.0
+	github.com/coveord/kingpin/v2 v2.4.1
 	github.com/docker/docker v20.10.21+incompatible
 	github.com/fatih/color v1.13.0
 	github.com/hashicorp/go-getter v1.6.2

go.sum

@@ -461,8 +461,9 @@ github.com/coveooss/gotemplate/v3 v3.7.3 h1:Hj+dfLmXFRUNfveUJyqegkxQlCru7gUQG6Tf
 github.com/coveooss/gotemplate/v3 v3.7.3/go.mod h1:9yJD1GLPopmgNfE3Nra6G04yKX8WmQh5HAWE+FC3dQ0=
 github.com/coveooss/multilogger v0.5.2 h1:HmuMT1RTD0d6tkqrrO9FlUBbp2p7QYbcAR6guszlifc=
 github.com/coveooss/multilogger v0.5.2/go.mod h1:Pw3jXW2Y4KEd+j5vFTTGSAFRX4fIOvJZzDPnvf9dr7U=
-github.com/coveord/kingpin/v2 v2.4.0 h1:wU/bzvMIzN9JlN8kxpAUmTKc5MPSVN6mxqReTRNCaw8=
 github.com/coveord/kingpin/v2 v2.4.0/go.mod h1:Qw3FnQuB068XW8vcP58GH4c03MYzCmiK00/yLDfqQA8=
+github.com/coveord/kingpin/v2 v2.4.1 h1:nqqSel2A5VoQp7TR6Q/vGqvwGs0fE7Qwg/RdBmdHSk0=
+github.com/coveord/kingpin/v2 v2.4.1/go.mod h1:Qw3FnQuB068XW8vcP58GH4c03MYzCmiK00/yLDfqQA8=
 github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7DoTY=
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
 github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=