fix: honor repo url when manually authored (#1400)

Co-authored-by: Awais Malik <awmalik@google.com>

cli/Makefile

@@ -1,7 +1,7 @@
 SHELL := /bin/bash
 
 # Changing this value will trigger a new release
-VERSION=v0.5.2
+VERSION=v0.5.3
 BINARY=bin/cft
 GITHUB_REPO=github.com/GoogleCloudPlatform/cloud-foundation-toolkit
 PLATFORMS := linux windows darwin

cli/bpmetadata/cmd.go

@@ -115,7 +115,7 @@ func generateMetadataForBpPath(bpPath string) error {
 
 func CreateBlueprintMetadata(bpPath string, bpMetadataObj *BlueprintMetadata) (*BlueprintMetadata, error) {
 	// verfiy that the blueprint path is valid & get repo details
-	repoDetails, err := getRepoDetailsByPath(bpPath)
+	repoDetails, err := getRepoDetailsByPath(bpPath, bpMetadataObj.Spec.Info.Source)
 	if err != nil {
 		return nil, err
 	}
@@ -142,47 +142,44 @@ func CreateBlueprintMetadata(bpPath string, bpMetadataObj *BlueprintMetadata) (*
 		return nil, fmt.Errorf("error reading blueprint readme markdown: %w", err)
 	}
 
-	info, err := createInfo(bpPath, readmeContent)
+	// create blueprint info
+	err = bpMetadataObj.Spec.Info.create(bpPath, readmeContent)
 	if err != nil {
 		return nil, fmt.Errorf("error creating blueprint info: %w", err)
 	}
 
-	interfaces, err := createInterfaces(bpPath, &bpMetadataObj.Spec.Interfaces)
+	// create blueprint interfaces i.e. variables & outputs
+	err = bpMetadataObj.Spec.Interfaces.create(bpPath)
 	if err != nil {
 		return nil, fmt.Errorf("error creating blueprint interfaces: %w", err)
 	}
 
+	// get blueprint requirements
 	rolesCfgPath := path.Join(repoDetails.Source.RootPath, tfRolesFileName)
 	svcsCfgPath := path.Join(repoDetails.Source.RootPath, tfServicesFileName)
 	requirements, err := getBlueprintRequirements(rolesCfgPath, svcsCfgPath)
 	if err != nil {
 		return nil, fmt.Errorf("error creating blueprint requirements: %w", err)
 	}
 
-	content := createContent(bpPath, repoDetails.Source.RootPath, readmeContent, &bpMetadataObj.Spec.Content)
+	bpMetadataObj.Spec.Requirements = *requirements
 
-	bpMetadataObj.Spec = BlueprintMetadataSpec{
-		Info:         *info,
-		Content:      *content,
-		Interfaces:   *interfaces,
-		Requirements: *requirements,
-	}
+	// create blueprint content i.e. documentation, icons, etc.
+	bpMetadataObj.Spec.Content.create(bpPath, repoDetails.Source.RootPath, readmeContent)
 
 	return bpMetadataObj, nil
 }
 
-func createInfo(bpPath string, readmeContent []byte) (*BlueprintInfo, error) {
-	i := &BlueprintInfo{}
+func (i *BlueprintInfo) create(bpPath string, readmeContent []byte) error {
 	title, err := getMdContent(readmeContent, 1, 1, "", false)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	i.Title = title.literal
-
-	repoDetails, err := getRepoDetailsByPath(bpPath)
+	repoDetails, err := getRepoDetailsByPath(bpPath, i.Source)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	i.Source = &BlueprintRepoDetail{
@@ -233,23 +230,22 @@ func createInfo(bpPath string, readmeContent []byte) (*BlueprintInfo, error) {
 		i.Icon = iconFilePath
 	}
 
-	return i, nil
+	return nil
 }
 
-func createInterfaces(bpPath string, interfaces *BlueprintInterface) (*BlueprintInterface, error) {
-	i, err := getBlueprintInterfaces(bpPath)
+func (i *BlueprintInterface) create(bpPath string) error {
+	interfaces, err := getBlueprintInterfaces(bpPath)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
-	if interfaces.VariableGroups != nil {
-		i.VariableGroups = interfaces.VariableGroups
-	}
+	i.Variables = interfaces.Variables
+	i.Outputs = interfaces.Outputs
 
-	return i, nil
+	return nil
 }
 
-func createContent(bpPath string, rootPath string, readmeContent []byte, content *BlueprintContent) *BlueprintContent {
+func (c *BlueprintContent) create(bpPath string, rootPath string, readmeContent []byte) {
 	var docListToSet []BlueprintListContent
 	documentation, err := getMdContent(readmeContent, -1, -1, "Documentation", true)
 	if err == nil {
@@ -262,24 +258,22 @@ func createContent(bpPath string, rootPath string, readmeContent []byte, content
 			docListToSet = append(docListToSet, doc)
 		}
 
-		content.Documentation = docListToSet
+		c.Documentation = docListToSet
 	}
 
 	// create sub-blueprints
 	modPath := path.Join(bpPath, modulesPath)
 	modContent, err := getModules(modPath)
 	if err == nil {
-		content.SubBlueprints = modContent
+		c.SubBlueprints = modContent
 	}
 
 	// create examples
 	exPath := path.Join(rootPath, examplesPath)
 	exContent, err := getExamples(exPath)
 	if err == nil {
-		content.Examples = exContent
+		c.Examples = exContent
 	}
-
-	return content
 }
 
 func WriteMetadata(obj *BlueprintMetadata, bpPath string) error {

cli/bpmetadata/repo.go

@@ -24,23 +24,29 @@ const (
 
 // getRepoDetailsByPath takes a local path for a blueprint and tries
 // to get repo details that include its name, path and type
-func getRepoDetailsByPath(bpPath string) (*repoDetail, error) {
-	bpPath = strings.TrimSuffix(bpPath, "/")
+func getRepoDetailsByPath(bpPath string, sourceUrl *BlueprintRepoDetail) (*repoDetail, error) {
 	rootRepoPath := getBpRootPath(bpPath)
-	repoName, err := util.GetRepoName(rootRepoPath)
-	if err != nil {
-		return nil, fmt.Errorf("error getting the repo name from the provided local repo path: %w", err)
+	if sourceUrl == nil {
+		bpPath = strings.TrimSuffix(bpPath, "/")
+		repoUrl, err := util.GetRepoUrl(bpPath)
+		if err != nil {
+			return nil, fmt.Errorf("error getting the repo URL from the provided local repo path: %w", err)
+		}
+
+		sourceUrl = &BlueprintRepoDetail{
+			Repo: repoUrl,
+		}
 	}
 
-	repoUrl, err := util.GetRepoUrl(bpPath)
+	repoName, err := util.GetRepoName(sourceUrl.Repo)
 	if err != nil {
-		return nil, fmt.Errorf("error getting the repo URL from the provided local repo path: %w", err)
+		return nil, fmt.Errorf("error getting the repo name from the provided local repo path: %w", err)
 	}
 
 	return &repoDetail{
 		Name: repoName,
 		Source: &repoSource{
-			Path:       repoUrl.String(),
+			Path:       sourceUrl.Repo,
 			SourceType: "git",
 			RootPath:   rootRepoPath,
 		},
@@ -50,7 +56,6 @@ func getRepoDetailsByPath(bpPath string) (*repoDetail, error) {
 // getBpRootPath determines if the provided bpPath is for a submodule
 // and resolves it to the root module path if necessary
 func getBpRootPath(bpPath string) string {
-
 	if strings.Contains(bpPath, nestedBpPath) {
 		i := strings.Index(bpPath, nestedBpPath)
 		bpPath = bpPath[0:i]

cli/bpmetadata/tfconfig.go

@@ -217,7 +217,7 @@ func getBlueprintInterfaces(configPath string) (*BlueprintInterface, error) {
 	}, nil
 }
 
-//build variable
+// build variable
 func getBlueprintVariable(modVar *tfconfig.Variable) BlueprintVariable {
 	return BlueprintVariable{
 		Name:        modVar.Name,
@@ -228,7 +228,7 @@ func getBlueprintVariable(modVar *tfconfig.Variable) BlueprintVariable {
 	}
 }
 
-//build output
+// build output
 func getBlueprintOutput(modOut *tfconfig.Output) BlueprintOutput {
 	return BlueprintOutput{
 		Name:        modOut.Name,

cli/util/git.go

@@ -11,13 +11,13 @@ import (
 const defaultRemote = "origin"
 
 // getRepoName finds upstream repo name from a given repo directory
-func GetRepoName(dir string) (string, error) {
-	remoteUrl, err := GetRepoUrl(dir)
+func GetRepoName(repoUrl string) (string, error) {
+	u, err := url.Parse(repoUrl)
 	if err != nil {
-		return "", fmt.Errorf("error getting remote URL: %w", err)
+		return "", fmt.Errorf("malformed repo URL: %w", err)
 	}
 
-	trimmedRemotePath := strings.TrimSuffix(remoteUrl.Path, "/")
+	trimmedRemotePath := strings.TrimSuffix(u.Path, "/")
 	splitRemotePath := strings.Split(trimmedRemotePath, "/")
 	// expect path to be /owner/repo
 	if len(splitRemotePath) != 3 {
@@ -29,22 +29,22 @@ func GetRepoName(dir string) (string, error) {
 }
 
 // getRepoName finds upstream repo name from a given repo directory
-func GetRepoUrl(dir string) (*url.URL, error) {
+func GetRepoUrl(dir string) (string, error) {
 	opt := &git.PlainOpenOptions{DetectDotGit: true}
 	r, err := git.PlainOpenWithOptions(dir, opt)
 	if err != nil {
-		return nil, fmt.Errorf("error opening git dir %s: %w", dir, err)
+		return "", fmt.Errorf("error opening git dir %s: %w", dir, err)
 	}
 	rm, err := r.Remote(defaultRemote)
 	if err != nil {
-		return nil, fmt.Errorf("error finding remote %s in git dir %s: %w", defaultRemote, dir, err)
+		return "", fmt.Errorf("error finding remote %s in git dir %s: %w", defaultRemote, dir, err)
 	}
 
 	// validate remote URL
 	remoteURL, err := url.Parse(rm.Config().URLs[0])
 	if err != nil {
-		return nil, fmt.Errorf("error parsing remote URL: %w", err)
+		return "", fmt.Errorf("error parsing remote URL: %w", err)
 	}
 
-	return remoteURL, nil
+	return remoteURL.String(), nil
 }

cli/util/git_test.go

@@ -9,7 +9,7 @@ import (
 	"github.com/go-git/go-git/v5/config"
 )
 
-func TestGetRepoName(t *testing.T) {
+func TestGetRepoUrl(t *testing.T) {
 	tests := []struct {
 		name    string
 		repo    string
@@ -22,25 +22,19 @@ func TestGetRepoName(t *testing.T) {
 			name:   "simple",
 			repo:   "https://github.com/foo/bar",
 			remote: defaultRemote,
-			want:   "bar",
+			want:   "https://github.com/foo/bar",
 		},
 		{
 			name:   "simple trailing",
 			repo:   "https://gitlab.com/foo/bar/",
 			remote: defaultRemote,
-			want:   "bar",
+			want:   "https://gitlab.com/foo/bar/",
 		},
 		{
 			name:   "no scheme",
 			repo:   "github.com/foo/bar",
 			remote: defaultRemote,
-			want:   "bar",
-		},
-		{
-			name:    "invalid path",
-			repo:    "github.com/foo/bar/baz",
-			remote:  defaultRemote,
-			wantErr: true,
+			want:   "github.com/foo/bar",
 		},
 		{
 			name:    "invalid remote",
@@ -53,13 +47,51 @@ func TestGetRepoName(t *testing.T) {
 			repo:   "https://github.com/foo/bar",
 			subDir: "modules/bp1",
 			remote: defaultRemote,
-			want:   "bar",
+			want:   "https://github.com/foo/bar",
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			dir := tempGitRepoWithRemote(t, tt.repo, tt.remote, tt.subDir)
-			got, err := GetRepoName(dir)
+			got, err := GetRepoUrl(dir)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("GetRepoUrl() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+
+			if got != tt.want {
+				t.Errorf("GetRepoUrl() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestGetRepoNameFromUrl(t *testing.T) {
+	tests := []struct {
+		name    string
+		repoUrl string
+		want    string
+		wantErr bool
+	}{
+		{
+			name:    "simple",
+			repoUrl: "https://github.com/foo/bar",
+			want:    "bar",
+		},
+		{
+			name:    "no scheme",
+			repoUrl: "github.com/foo/bar",
+			want:    "bar",
+		},
+		{
+			name:    "invalid path",
+			repoUrl: "github.com/foo/bar/baz",
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := GetRepoName(tt.repoUrl)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("getRepoName() error = %v, wantErr %v", err, tt.wantErr)
 				return