fix: updates approach for discovering mod/submods (#1301)

Co-authored-by: Awais Malik <[email protected]>

cli/Makefile

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

cli/bpmetadata/repo.go

@@ -2,7 +2,6 @@ package bpmetadata
 
 import (
 	"fmt"
-	"regexp"
 	"strings"
 
 	"github.com/GoogleCloudPlatform/cloud-foundation-toolkit/cli/util"
@@ -20,25 +19,20 @@ type repoSource struct {
 }
 
 const (
-	bpUrlRegEx   = "(.*terraform-google-[^/]+)/?(modules/.*)?"
-	nestedBpPath = "modules/"
+	nestedBpPath = "/modules"
 )
 
 // 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, "/")
-	repoPath, err := getBpPathForRepoName(bpPath)
-	if err != nil {
-		return nil, fmt.Errorf("error getting the repo path from the provided blueprint path: %w", err)
-	}
-
-	repoName, err := util.GetRepoName(repoPath)
+	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)
 	}
 
-	repoUrl, err := util.GetRepoUrl(repoPath)
+	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)
 	}
@@ -48,34 +42,19 @@ func getRepoDetailsByPath(bpPath string) (*repoDetail, error) {
 		Source: &repoSource{
 			Path:       repoUrl.String(),
 			SourceType: "git",
-			RootPath:   repoPath,
+			RootPath:   rootRepoPath,
 		},
 	}, nil
 }
 
-// getBpPathForRepoName verifies if the blueprint follows blueprint
-// naming conventions and returns the local path for the repo root
-func getBpPathForRepoName(bpPath string) (string, error) {
-	r := regexp.MustCompile(bpUrlRegEx)
-	matches := r.FindStringSubmatch(bpPath)
-
-	// not a valid blueprint path if there is no match
-	if matches == nil {
-		return "", fmt.Errorf("provided blueprint path is not valid: %s", bpPath)
-	}
-
-	// if matched, matches should haveexactly 3 items,
-	// [0] for the match and [1] for root repo path and [2]
-	// for the nested blueprint name
-	if len(matches) != 3 {
-		return "", fmt.Errorf("provided nested blueprint path is not valid: %s. It should be under the %s directory", bpPath, nestedBpPath)
-	}
+// 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 {
 
-	// check if the path has a nested blueprint
-	// and under the right directory
-	if len(bpPath) != len(matches[1]) && matches[2] == "" {
-		return "", fmt.Errorf("provided nested blueprint path is not valid: %s. It should be under the %s directory", bpPath, nestedBpPath)
+	if strings.Contains(bpPath, nestedBpPath) {
+		i := strings.Index(bpPath, nestedBpPath)
+		bpPath = bpPath[0:i]
 	}
 
-	return matches[1], nil
+	return bpPath
 }

cli/bpmetadata/repo_test.go

@@ -4,7 +4,7 @@ import (
 	"testing"
 )
 
-func TestGetBpRepoPath(t *testing.T) {
+func TestGetBpRootPath(t *testing.T) {
 	tests := []struct {
 		name    string
 		path    string
@@ -30,25 +30,23 @@ func TestGetBpRepoPath(t *testing.T) {
 			wantErr: false,
 		},
 		{
-			name:    "invalid top level",
-			path:    "testdata/bpmetadata/erraform-google-bp01",
-			wantErr: true,
+			name:    "docker workspace root",
+			path:    "workspace",
+			want:    "workspace",
+			wantErr: false,
 		},
 		{
-			name:    "invalid nested",
-			path:    "testdata/bpmetadata/terraform-google-bp01/test/bp01-01",
-			wantErr: true,
+			name:    "docker workspace submodule",
+			path:    "workspace/modules/bp-01",
+			want:    "workspace",
+			wantErr: false,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := getBpPathForRepoName(tt.path)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("getBpPathForRepoName() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
+			got := getBpRootPath(tt.path)
 			if got != tt.want {
-				t.Errorf("getBpPathForRepoName() = %v, want %v", got, tt.want)
+				t.Errorf("getBpRootPath() = %v, want %v", got, tt.want)
 			}
 		})
 	}

cli/util/git.go

@@ -30,7 +30,8 @@ func GetRepoName(dir string) (string, error) {
 
 // getRepoName finds upstream repo name from a given repo directory
 func GetRepoUrl(dir string) (*url.URL, error) {
-	r, err := git.PlainOpen(dir)
+	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)
 	}

cli/util/git_test.go

@@ -1,6 +1,7 @@
 package util
 
 import (
+	"os"
 	"path"
 	"testing"
 
@@ -74,9 +75,13 @@ func tempGitRepoWithRemote(t *testing.T, repoURL, remote string, subDir string)
 	t.Helper()
 	dir := t.TempDir()
 	if subDir != "" {
-		dir = path.Join(dir, subDir)
+		err := os.MkdirAll(path.Join(dir, subDir), 0755)
+		if err != nil {
+			t.Fatalf("Error sub dir for temp git repo: %v", err)
+		}
 	}
-	r, err := git.PlainInit(dir, true)
+
+	r, err := git.PlainInit(dir, false)
 	if err != nil {
 		t.Fatalf("Error creating git repo in tempdir: %v", err)
 	}
@@ -87,5 +92,6 @@ func tempGitRepoWithRemote(t *testing.T, repoURL, remote string, subDir string)
 	if err != nil {
 		t.Fatalf("Error creating remote in tempdir repo: %v", err)
 	}
-	return dir
+
+	return path.Join(dir, subDir)
 }

infra/build/developer-tools/build/scripts/task_helper_functions.sh

@@ -305,6 +305,25 @@ function generate_docs() {
   done < <(find_files . -name '*.tf' -print0 \
     | compat_xargs -0 -n1 dirname \
     | sort -u)
+
+  generate_metadata
+}
+
+function generate_metadata() {
+  if [[ "${DISABLE_BPMETADATA:-}" ]]; then
+    echo "DISABLE_BPMETADATA set. Skipping metadata generation."
+    return 0
+  fi
+  echo "Generating blueprint metadata"
+  cft blueprint metadata
+  if [ $? -eq 0 ]; then
+    echo "Success!"
+  else
+    echo "Warning! Unable to generate metadata."
+  fi
+  # add headers since comments are not preserved with metadata generation
+  # TODO: b/260869608
+  fix_headers
 }
 
 function check_tflint() {
@@ -357,18 +376,18 @@ function check_documentation() {
   rsync -axh \
     --exclude '*/.terraform' \
     --exclude '*/.kitchen' \
-    --exclude '*/.git' \
     --exclude 'autogen' \
     --exclude '*/.tfvars' \
     /workspace "${tempdir}" >/dev/null 2>/dev/null
-  cd "${tempdir}"
+  cd "${tempdir}/workspace"
   generate_docs >/dev/null 2>/dev/null
+  # TODO: (b/261241276) preserve verion no. for release PR
   diff -r \
     --exclude=".terraform" \
     --exclude=".kitchen" \
-    --exclude=".git" \
     --exclude="autogen" \
     --exclude="*.tfvars" \
+    --exclude="*metadata.yaml" \
     /workspace "${tempdir}/workspace"
   rc=$?
   if [[ "${rc}" -ne 0 ]]; then