ModuleSourceRemote String now considers presence of query string (has…

…hicorp#31250)
fatahfattah · Aug 25, 2022 · cd85b5c · cd85b5c
1 parent 2aff678
commit cd85b5c
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 2 deletions.
diff --git a/internal/addrs/module_source.go b/internal/addrs/module_source.go
@@ -316,10 +316,17 @@ func parseModuleSourceRemote(raw string) (ModuleSourceRemote, error) {
 func (s ModuleSourceRemote) moduleSource() {}
 
 func (s ModuleSourceRemote) String() string {
+	base := s.Package.String()
+
 	if s.Subdir != "" {
-		return s.Package.String() + "//" + s.Subdir
+		// Address contains query string
+		if strings.Contains(base, "?") {
+			parts := strings.SplitN(base, "?", 2)
+			return parts[0] + "//" + s.Subdir + "?" + parts[1]
+		}
+		return base + "//" + s.Subdir
 	}
-	return s.Package.String()
+	return base
 }
 
 func (s ModuleSourceRemote) ForDisplay() string {

diff --git a/internal/addrs/module_source_test.go b/internal/addrs/module_source_test.go
@@ -154,6 +154,13 @@ func TestParseModuleSource(t *testing.T) {
 				Subdir:  "bleep/bloop",
 			},
 		},
+		"git over HTTPS, URL-style, subdir, query parameters": {
+			input: "git::https://example.com/code/baz.git//bleep/bloop?otherthing=blah",
+			want: ModuleSourceRemote{
+				Package: ModulePackage("git::https://example.com/code/baz.git?otherthing=blah"),
+				Subdir:  "bleep/bloop",
+			},
+		},
 		"git over SSH, URL-style": {
 			input: "git::ssh://git@example.com/code/baz.git",
 			want: ModuleSourceRemote{
@@ -401,6 +408,56 @@ func TestModuleSourceRemoteFromRegistry(t *testing.T) {
 	})
 }
 
+func TestParseModuleSourceRemote(t *testing.T) {
+
+	tests := map[string]struct {
+		input          string
+		wantString     string
+		wantForDisplay string
+		wantErr        string
+	}{
+		"git over HTTPS, URL-style, query parameters": {
+			// Query parameters should be correctly appended after the Package and Subdir
+			input:          `git::https://example.com/code/baz.git?otherthing=blah`,
+			wantString:     `git::https://example.com/code/baz.git?otherthing=blah`,
+			wantForDisplay: `git::https://example.com/code/baz.git?otherthing=blah`,
+		},
+		"git over HTTPS, URL-style, subdir, query parameters": {
+			// Query parameters should be correctly appended after the Package and Subdir
+			input:          `git::https://example.com/code/baz.git//bleep/bloop?otherthing=blah`,
+			wantString:     `git::https://example.com/code/baz.git//bleep/bloop?otherthing=blah`,
+			wantForDisplay: `git::https://example.com/code/baz.git//bleep/bloop?otherthing=blah`,
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			remote, err := parseModuleSourceRemote(test.input)
+
+			if test.wantErr != "" {
+				switch {
+				case err == nil:
+					t.Errorf("unexpected success\nwant error: %s", test.wantErr)
+				case err.Error() != test.wantErr:
+					t.Errorf("wrong error messages\ngot:  %s\nwant: %s", err.Error(), test.wantErr)
+				}
+				return
+			}
+
+			if err != nil {
+				t.Fatalf("unexpected error: %s", err.Error())
+			}
+
+			if got, want := remote.String(), test.wantString; got != want {
+				t.Errorf("wrong String() result\ngot:  %s\nwant: %s", got, want)
+			}
+			if got, want := remote.ForDisplay(), test.wantForDisplay; got != want {
+				t.Errorf("wrong ForDisplay() result\ngot:  %s\nwant: %s", got, want)
+			}
+		})
+	}
+}
+
 func TestParseModuleSourceRegistry(t *testing.T) {
 	// We test parseModuleSourceRegistry alone here, in addition to testing
 	// it indirectly as part of TestParseModuleSource, because general