Enabled automatic label adding for MM PRs (GoogleCloudPlatform#9402)

* Enabled automatic label adding

* Corrected path

* Run add-labels for both TPG and TPGB

* Added / fixed logging

* Explicitly pass in GH token

* changed impacted resources and removed duplicate logging

* Reverted resource changes

* Added add-labels calls to generate_comment.go

.ci/magician/cmd/generate_comment.go

@@ -136,12 +136,58 @@ func execGenerateComment(buildID, projectID, buildStep, commit, pr, githubToken
 		diffs += repoDiffs
 	}
 
-	breakingChanges, err := detectBreakingChanges(mmLocalPath, tpgLocalPath, tpgbLocalPath, oldBranch, newBranch, r)
+	// TPG diff processor
+	showBreakingChangesFailed := false
+	err = buildDiffProcessor(mmLocalPath, tpgLocalPath, oldBranch, newBranch, r)
 	if err != nil {
-		fmt.Println("Error setting up breaking change detector: ", err)
+		fmt.Println(err)
+		os.Exit(1)
+	}
+	tpgBreaking, err := computeBreakingChanges(mmLocalPath, r)
+	if err != nil {
+		fmt.Println("Error computing TPG breaking changes: ", err)
+		showBreakingChangesFailed = true
+	}
+	err = addLabels(mmLocalPath, githubToken, pr, r)
+	if err != nil {
+		fmt.Println("Error adding TPG labels to PR: ", err)
+	}
+	err = cleanDiffProcessor(mmLocalPath, r)
+	if err != nil {
+		fmt.Println("Error cleaning up diff processor: ", err)
 		os.Exit(1)
 	}
 
+	// TPGB diff processor
+	err = buildDiffProcessor(mmLocalPath, tpgbLocalPath, oldBranch, newBranch, r)
+	if err != nil {
+		fmt.Println(err)
+		os.Exit(1)
+	}
+	tpgbBreaking, err := computeBreakingChanges(mmLocalPath, r)
+	if err != nil {
+		fmt.Println("Error computing TPGB breaking changes: ", err)
+		showBreakingChangesFailed = true
+	}
+	err = addLabels(mmLocalPath, githubToken, pr, r)
+	if err != nil {
+		fmt.Println("Error adding TPGB labels to PR: ", err)
+	}
+	err = cleanDiffProcessor(mmLocalPath, r)
+	if err != nil {
+		fmt.Println("Error cleaning up diff processor: ", err)
+		os.Exit(1)
+	}
+
+	var breakingChanges string
+	if showBreakingChangesFailed {
+		breakingChanges = `## Breaking Change Detection Failed
+The breaking change detector crashed during execution. This is usually due to the downstream provider(s) failing to compile. Please investigate or follow up with your reviewer.`
+	} else {
+		breakingChanges = combineBreakingChanges(tpgBreaking, tpgbBreaking)
+	}
+
+	// Missing test detector
 	missingTests, err := detectMissingTests(mmLocalPath, tpgbLocalPath, oldBranch, r)
 	if err != nil {
 		fmt.Println("Error setting up missing test detector: ", err)
@@ -210,54 +256,51 @@ func cloneAndDiff(repoName, path, oldBranch, newBranch, diffTitle, githubToken s
 	return "", nil
 }
 
-// Run the breaking change detector and return the results.
-// Returns an empty string unless there are breaking changes or the detector failed.
-// Error will be nil unless an error occurs manipulating files.
-func detectBreakingChanges(mmLocalPath, tpgLocalPath, tpgbLocalPath, oldBranch, newBranch string, r gcRunner) (string, error) {
-	// Breaking change setup and execution
+// Build the diff processor for tpg or tpgb
+func buildDiffProcessor(mmLocalPath, providerLocalPath, oldBranch, newBranch string, r gcRunner) error {
 	diffProcessorPath := filepath.Join(mmLocalPath, "tools", "diff-processor")
+	r.Chdir(diffProcessorPath)
 	for _, path := range []string{"old", "new"} {
-		if err := r.Copy(tpgLocalPath, filepath.Join(diffProcessorPath, path)); err != nil {
-			return "", err
+		if err := r.Copy(providerLocalPath, filepath.Join(diffProcessorPath, path)); err != nil {
+			return err
 		}
 	}
-	var tpgBreaking, tpgbBreaking, breakingChanges string
-	var diffProccessorErr error
-	r.Chdir(diffProcessorPath)
 	if _, err := r.Run("make", []string{"build"}, []string{"OLD_REF=" + oldBranch, "NEW_REF=" + newBranch}); err != nil {
-		fmt.Printf("Error running make build in %s: %v\n", diffProcessorPath, err)
-		diffProccessorErr = err
-	} else {
-		tpgBreaking, err = r.Run("bin/diff-processor", []string{"breaking-changes"}, nil)
-		if err != nil {
-			fmt.Println("Diff processor error: ", err)
-			diffProccessorErr = err
-		}
+		return fmt.Errorf("Error running make build in %s: %v\n", diffProcessorPath, err)
+	}
+	return nil
+}
+
+func computeBreakingChanges(mmLocalPath string, r gcRunner) (string, error) {
+	diffProcessorPath := filepath.Join(mmLocalPath, "tools", "diff-processor")
+	r.Chdir(diffProcessorPath)
+	breakingChanges, err := r.Run("bin/diff-processor", []string{"breaking-changes"}, nil)
+	if err != nil {
+		return "", err
 	}
+	return breakingChanges, nil
+}
+
+func addLabels(mmLocalPath, githubToken, pr string, r gcRunner) error {
+	diffProcessorPath := filepath.Join(mmLocalPath, "tools", "diff-processor")
+	r.Chdir(diffProcessorPath)
+	output, err := r.Run("bin/diff-processor", []string{"add-labels", pr}, []string{fmt.Sprintf("GITHUB_TOKEN=%s", githubToken)})
+	fmt.Println(output)
+	return err
+}
+
+func cleanDiffProcessor(mmLocalPath string, r gcRunner) error {
+	diffProcessorPath := filepath.Join(mmLocalPath, "tools", "diff-processor")
 	for _, path := range []string{"old", "new", "bin"} {
 		if err := r.RemoveAll(filepath.Join(diffProcessorPath, path)); err != nil {
-			return "", err
+			return err
 		}
 	}
-	for _, path := range []string{"old", "new"} {
-		if err := r.Copy(tpgbLocalPath, filepath.Join(diffProcessorPath, path)); err != nil {
-			return "", err
-		}
-	}
-
-	if diffProccessorErr != nil {
-		fmt.Println("Breaking changes failed")
-		breakingChanges = `## Breaking Change Detection Failed
-The breaking change detector crashed during execution. This is usually due to the downstream provider(s) failing to compile. Please investigate or follow up with your reviewer.`
-	} else {
-		fmt.Println("Breaking changes succeeded")
-		breakingChanges = compareBreakingChanges(tpgBreaking, tpgbBreaking)
-	}
-	return breakingChanges, nil
+	return nil
 }
 
 // Get the breaking change message including the unique tpg messages and all tpgb messages.
-func compareBreakingChanges(tpgBreaking, tpgbBreaking string) string {
+func combineBreakingChanges(tpgBreaking, tpgbBreaking string) string {
 	var allMessages []string
 	if tpgBreaking == "" {
 		if tpgbBreaking == "" {

.ci/magician/cmd/generate_comment_test.go

@@ -54,6 +54,9 @@ func TestExecGenerateComment(t *testing.T) {
 			{"/mock/dir/magic-modules/tools/diff-processor/old"},
 			{"/mock/dir/magic-modules/tools/diff-processor/new"},
 			{"/mock/dir/magic-modules/tools/diff-processor/bin"},
+			{"/mock/dir/magic-modules/tools/diff-processor/old"},
+			{"/mock/dir/magic-modules/tools/diff-processor/new"},
+			{"/mock/dir/magic-modules/tools/diff-processor/bin"},
 		},
 		"Run": {
 			{"", "git", []string{"clone", "-b", "auto-pr-pr1", "https://modular-magician:*******@github.com/modular-magician/terraform-provider-google", "/mock/dir/tpg"}, []string(nil)},
@@ -70,6 +73,10 @@ func TestExecGenerateComment(t *testing.T) {
 			{"/mock/dir/tfoics", "git", []string{"diff", "origin/auto-pr-pr1-old", "origin/auto-pr-pr1", "--shortstat"}, []string(nil)},
 			{"/mock/dir/magic-modules/tools/diff-processor", "make", []string{"build"}, []string{"OLD_REF=auto-pr-pr1-old", "NEW_REF=auto-pr-pr1"}},
 			{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"breaking-changes"}, []string(nil)},
+			{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"add-labels", "pr1"}, []string{"GITHUB_TOKEN=*******"}},
+			{"/mock/dir/magic-modules/tools/diff-processor", "make", []string{"build"}, []string{"OLD_REF=auto-pr-pr1-old", "NEW_REF=auto-pr-pr1"}},
+			{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"breaking-changes"}, []string(nil)},
+			{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"add-labels", "pr1"}, []string{"GITHUB_TOKEN=*******"}},
 			{"/mock/dir/tpgbold", "git", []string{"checkout", "origin/auto-pr-pr1-old"}, []string(nil)},
 			{"/mock/dir/tpgbold", "find", []string{".", "-type", "f", "-name", "*.go", "-exec", "sed", "-i.bak", "s~github.com/hashicorp/terraform-provider-google-beta~google/provider/old~g", "{}", "+"}, []string(nil)},
 			{"/mock/dir/tpgbold", "sed", []string{"-i.bak", "s|github.com/hashicorp/terraform-provider-google-beta|google/provider/old|g", "go.mod"}, []string(nil)},

.ci/scripts/go-plus/github-differ/generate_comment.sh

@@ -57,29 +57,50 @@ if ! git diff --exit-code origin/$OLD_BRANCH origin/$NEW_BRANCH; then
 fi
 popd
 
-## Breaking change setup and execution
+## Diff processor - TPG
 set +e
 pushd $MM_LOCAL_PATH/tools/diff-processor
 cp -r $TPG_LOCAL_PATH old/
 cp -r $TPG_LOCAL_PATH new/
 make build OLD_REF=$OLD_BRANCH NEW_REF=$NEW_BRANCH
+
+### Breaking changes
 TPG_BREAKING="$(bin/diff-processor breaking-changes)"
 retVal=$?
 if [ $retVal -ne 0 ]; then
     TPG_BREAKING=""
     BREAKING_CHANGE_BUILD_FAILURE=1
 fi
 
+### Add labels
+GITHUB_TOKEN=$GITHUB_TOKEN $MM_LOCAL_PATH/tools/diff-processor/bin/diff-processor add-labels $PR_NUMBER
 rm -rf ./old/ ./new/ ./bin/
+popd
+set -e
+
+## Diff processor - TPGB
+set +e
+pushd $MM_LOCAL_PATH/tools/diff-processor
 cp -r $TPGB_LOCAL_PATH old/
 cp -r $TPGB_LOCAL_PATH new/
 make build OLD_REF=$OLD_BRANCH NEW_REF=$NEW_BRANCH
+
+### Breaking changes
 TPGB_BREAKING="$(bin/diff-processor breaking-changes)"
 retVal=$?
 if [ $retVal -ne 0 ]; then
     TPGB_BREAKING=""
     BREAKING_CHANGE_BUILD_FAILURE=1
 fi
+
+### Add labels
+GITHUB_TOKEN=$GITHUB_TOKEN $MM_LOCAL_PATH/tools/diff-processor/bin/diff-processor add-labels $PR_NUMBER
+rm -rf ./old/ ./new/ ./bin/
+popd
+set -e
+
+## Report breaking change failures
+set +e
 if [ $BREAKING_CHANGE_BUILD_FAILURE -eq 0 ]; then
     echo "Breaking changes succeeded"
     # Export variables here so that they can be used in compare_breaking_changes
@@ -91,7 +112,6 @@ else
     echo "Breaking changes failed"
     BREAKINGCHANGES="## Breaking Change Detection Failed${NEWLINE}The breaking change detector crashed during execution. This is usually due to the downstream provider(s) failing to compile. Please investigate or follow up with your reviewer."
 fi
-popd
 set -e
 
 ## Missing test setup and execution

tools/issue-labeler/labeler/backfill.go

@@ -146,7 +146,7 @@ func UpdateIssues(repository string, issueUpdates []IssueUpdate, dryRun bool) {
 		}
 		fmt.Printf("Existing labels: %v\n", issueUpdate.OldLabels)
 		fmt.Printf("New labels: %v\n", issueUpdate.Labels)
-		fmt.Printf("%s %s (https://github.com/%s/issues/%d)\n", repository, req.Method, req.URL, issueUpdate.Number)
+		fmt.Printf("%s %s (https://github.com/%s/issues/%d)\n", req.Method, req.URL, repository, issueUpdate.Number)
 		b, err := json.MarshalIndent(updateBody, "", "  ")
 		if err != nil {
 			glog.Errorf("Error marshalling json: %v", err)
@@ -167,9 +167,11 @@ func UpdateIssues(repository string, issueUpdates []IssueUpdate, dryRun bool) {
 			var errResp ErrorResponse
 			json.Unmarshal(body, &errResp)
 			if errResp.Message != "" {
-				glog.Errorf("API error: %s", errResp.Message)
+				fmt.Printf("API error: %s", errResp.Message)
+				continue
 			}
 
 		}
+		fmt.Printf("GitHub Issue %s %d updated successfully", repository, issueUpdate.Number)
 	}
 }