Merge branch 'main' into dc-terraform

Author: ihnarayanan

.ci/containers/build-environment/Dockerfile

@@ -23,7 +23,7 @@ RUN mkdir -p "$GOPATH/src" "$GOPATH/bin" && chmod -R 1777 "$GOPATH"
 WORKDIR $GOPATH
 
 # terraform binary used by tfv/tgc
-COPY --from=hashicorp/terraform:1.8.3 /bin/terraform /bin/terraform
+COPY --from=hashicorp/terraform:1.10.0 /bin/terraform /bin/terraform
 
 SHELL ["/bin/bash", "-c"]
 

.ci/containers/go-plus/Dockerfile

@@ -29,7 +29,7 @@ RUN apt-get update && \
     apt-get clean && \
     rm -rf /var/lib/apt/lists/*
 
-RUN wget https://releases.hashicorp.com/terraform/1.8.3/terraform_1.8.3_linux_amd64.zip \
-    && unzip terraform_1.8.3_linux_amd64.zip \
-    && rm terraform_1.8.3_linux_amd64.zip \
+RUN wget https://releases.hashicorp.com/terraform/1.10.0/terraform_1.10.0_linux_amd64.zip \
+    && unzip terraform_1.10.0_linux_amd64.zip \
+    && rm terraform_1.10.0_linux_amd64.zip \
     && mv ./terraform /bin/terraform

.ci/magician/cmd/generate_downstream.go

@@ -335,12 +335,15 @@ func createCommit(scratchRepo *source.Repo, commitMessage string, rnr ExecRunner
 	commitSha = strings.TrimSpace(commitSha)
 	fmt.Printf("Commit sha on the branch is: `%s`\n", commitSha)
 
-	variablePath := fmt.Sprintf("/workspace/commitSHA_modular-magician_%s.txt", scratchRepo.Name)
-	fmt.Println("variablePath: ", variablePath)
-
-	err = rnr.WriteFile(variablePath, commitSha)
-	if err != nil {
-		fmt.Println("Error:", err)
+	// auto-pr's use commitSHA_modular-magician_<repo>_.txt file to communicate commmit hash
+	// across cloudbuild steps. Used in test-tpg to execute unit tests for the HEAD commit
+	if strings.HasPrefix(scratchRepo.Branch, "auto-pr-") && !strings.HasSuffix(scratchRepo.Branch, "-old") {
+		variablePath := fmt.Sprintf("/workspace/commitSHA_modular-magician_%s.txt", scratchRepo.Name)
+		fmt.Println("variablePath: ", variablePath)
+		err = rnr.WriteFile(variablePath, commitSha)
+		if err != nil {
+			fmt.Println("Error:", err)
+		}
 	}
 
 	return commitSha, err

.ci/magician/cmd/scheduled_pr_reminders.go

@@ -267,29 +267,20 @@ func (s pullRequestReviewState) String() string {
 // We don't specially handle cases where the contributor has "acted" because it would be
 // significant additional effort, and this case is already handled by re-requesting review
 // automatically based on contributor actions.
-func notificationState(pr *github.PullRequest, issueEvents []*github.IssueEvent, reviews []*github.PullRequestReview) (pullRequestReviewState, time.Time, error) {
-	slices.SortFunc(issueEvents, func(a, b *github.IssueEvent) int {
-		if a.CreatedAt.Before(*b.CreatedAt.GetTime()) {
-			return 1
+func notificationState(pr *github.PullRequest, issueEventsDesc []*github.IssueEvent, reviewsDesc []*github.PullRequestReview) (pullRequestReviewState, time.Time, error) {
+	issueEventsDesc = sortedEventsDesc(issueEventsDesc)
+	reviewsDesc = sortedReviewsDesc(reviewsDesc)
+
+	var readyForReviewTime = *pr.CreatedAt.GetTime()
+	for _, event := range issueEventsDesc {
+		if "ready_for_review" == *event.Event {
+			readyForReviewTime = maxTime(*event.CreatedAt.GetTime(), readyForReviewTime)
 		}
-		if a.CreatedAt.After(*b.CreatedAt.GetTime()) {
-			return -1
-		}
-		return 0
-	})
-	slices.SortFunc(reviews, func(a, b *github.PullRequestReview) int {
-		if a.SubmittedAt.Before(*b.SubmittedAt.GetTime()) {
-			return 1
-		}
-		if a.SubmittedAt.After(*b.SubmittedAt.GetTime()) {
-			return -1
-		}
-		return 0
-	})
+	}
 
 	var latestReviewRequest *github.IssueEvent
 	removedRequests := map[string]struct{}{}
-	for _, event := range issueEvents {
+	for _, event := range issueEventsDesc {
 		if *event.Event == "review_request_removed" && event.RequestedReviewer != nil {
 			removedRequests[*event.RequestedReviewer.Login] = struct{}{}
 			continue
@@ -320,7 +311,7 @@ func notificationState(pr *github.PullRequest, issueEvents []*github.IssueEvent,
 	var earliestCommented *github.PullRequestReview
 
 	ignoreBy := map[string]struct{}{}
-	for _, review := range reviews {
+	for _, review := range reviewsDesc {
 		if review.SubmittedAt.Before(*latestReviewRequest.CreatedAt.GetTime()) {
 			break
 		}
@@ -355,15 +346,59 @@ func notificationState(pr *github.PullRequest, issueEvents []*github.IssueEvent,
 	}
 
 	if earliestChangesRequested != nil {
-		return waitingForContributor, *earliestChangesRequested.SubmittedAt.GetTime(), nil
+		timeState := maxTime(*earliestChangesRequested.SubmittedAt.GetTime(), readyForReviewTime)
+		return waitingForContributor, timeState, nil
 	}
 	if earliestApproved != nil {
-		return waitingForMerge, *earliestApproved.SubmittedAt.GetTime(), nil
+		timeState := maxTime(*earliestApproved.SubmittedAt.GetTime(), readyForReviewTime)
+		return waitingForMerge, timeState, nil
 	}
 	if earliestCommented != nil {
-		return waitingForContributor, *earliestCommented.SubmittedAt.GetTime(), nil
+		timeState := maxTime(*earliestCommented.SubmittedAt.GetTime(), readyForReviewTime)
+		return waitingForContributor, timeState, nil
+	}
+	timeState := maxTime(*latestReviewRequest.CreatedAt.GetTime(), readyForReviewTime)
+	return waitingForReview, timeState, nil
+}
+
+// compareTimeDesc returns sort ordering for descending time comparison (newest first)
+func compareTimeDesc(a, b time.Time) int {
+	if a.Before(b) {
+		return 1
+	}
+	if a.After(b) {
+		return -1
+	}
+	return 0
+}
+
+// sortedEventsDesc returns events sorted by creation time, newest first
+func sortedEventsDesc(events []*github.IssueEvent) []*github.IssueEvent {
+	sortedEvents := make([]*github.IssueEvent, len(events))
+	copy(sortedEvents, events)
+	slices.SortFunc(sortedEvents, func(a, b *github.IssueEvent) int {
+		return compareTimeDesc(*a.CreatedAt.GetTime(), *b.CreatedAt.GetTime())
+	})
+	return sortedEvents
+}
+
+// sortedReviewsDesc returns reviews sorted by submission time, newest first
+func sortedReviewsDesc(reviews []*github.PullRequestReview) []*github.PullRequestReview {
+	sortedReviews := make([]*github.PullRequestReview, len(reviews))
+	copy(sortedReviews, reviews)
+	slices.SortFunc(sortedReviews, func(a, b *github.PullRequestReview) int {
+		return compareTimeDesc(*a.SubmittedAt.GetTime(), *b.SubmittedAt.GetTime())
+	})
+	return sortedReviews
+}
+
+// maxTime - returns whichever time occured after the other
+func maxTime(time1, time2 time.Time) time.Time {
+	// Return the later time
+	if time1.After(time2) {
+		return time1
 	}
-	return waitingForReview, *latestReviewRequest.CreatedAt.GetTime(), nil
+	return time2
 }
 
 // Calculates the number of PDT days between from and to (by calendar date, not # of hours).

.ci/magician/cmd/scheduled_pr_reminders_test.go

@@ -11,8 +11,9 @@ import (
 )
 
 func TestNotificationState(t *testing.T) {
-	firstCoreReviewer := membership.AvailableReviewers()[0]
-	secondCoreReviewer := membership.AvailableReviewers()[1]
+	availableReviewers := membership.AvailableReviewers()
+	firstCoreReviewer := availableReviewers[0]
+	secondCoreReviewer := availableReviewers[1]
 	cases := map[string]struct {
 		pullRequest *github.PullRequest
 		issueEvents []*github.IssueEvent
@@ -442,6 +443,183 @@ func TestNotificationState(t *testing.T) {
 			expectState: waitingForMerge,
 			expectSince: time.Date(2024, 2, 1, 0, 0, 0, 0, time.UTC),
 		},
+		"ready_for_review event after creation": {
+			pullRequest: &github.PullRequest{
+				User:      &github.User{Login: github.String("author")},
+				CreatedAt: &github.Timestamp{time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)},
+			},
+			issueEvents: []*github.IssueEvent{
+				&github.IssueEvent{
+					Event:             github.String("review_requested"),
+					CreatedAt:         &github.Timestamp{time.Date(2024, 1, 2, 0, 0, 0, 0, time.UTC)},
+					RequestedReviewer: &github.User{Login: github.String(firstCoreReviewer)},
+				},
+				&github.IssueEvent{
+					Event:     github.String("ready_for_review"),
+					CreatedAt: &github.Timestamp{time.Date(2024, 1, 3, 0, 0, 0, 0, time.UTC)},
+				},
+			},
+			expectState: waitingForReview,
+			expectSince: time.Date(2024, 1, 3, 0, 0, 0, 0, time.UTC),
+		},
+
+		"ready_for_review event before review request": {
+			pullRequest: &github.PullRequest{
+				User:      &github.User{Login: github.String("author")},
+				CreatedAt: &github.Timestamp{time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)},
+			},
+			issueEvents: []*github.IssueEvent{
+				&github.IssueEvent{
+					Event:     github.String("ready_for_review"),
+					CreatedAt: &github.Timestamp{time.Date(2024, 1, 2, 0, 0, 0, 0, time.UTC)},
+				},
+				&github.IssueEvent{
+					Event:             github.String("review_requested"),
+					CreatedAt:         &github.Timestamp{time.Date(2024, 1, 3, 0, 0, 0, 0, time.UTC)},
+					RequestedReviewer: &github.User{Login: github.String(firstCoreReviewer)},
+				},
+			},
+			expectState: waitingForReview,
+			expectSince: time.Date(2024, 1, 3, 0, 0, 0, 0, time.UTC),
+		},
+
+		"review after ready_for_review": {
+			pullRequest: &github.PullRequest{
+				User:      &github.User{Login: github.String("author")},
+				CreatedAt: &github.Timestamp{time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)},
+			},
+			issueEvents: []*github.IssueEvent{
+				&github.IssueEvent{
+					Event:     github.String("ready_for_review"),
+					CreatedAt: &github.Timestamp{time.Date(2024, 1, 2, 0, 0, 0, 0, time.UTC)},
+				},
+				&github.IssueEvent{
+					Event:             github.String("review_requested"),
+					CreatedAt:         &github.Timestamp{time.Date(2024, 1, 3, 0, 0, 0, 0, time.UTC)},
+					RequestedReviewer: &github.User{Login: github.String(firstCoreReviewer)},
+				},
+			},
+			reviews: []*github.PullRequestReview{
+				&github.PullRequestReview{
+					User:        &github.User{Login: github.String(firstCoreReviewer)},
+					State:       github.String("APPROVED"),
+					SubmittedAt: &github.Timestamp{time.Date(2024, 1, 4, 0, 0, 0, 0, time.UTC)},
+				},
+			},
+			expectState: waitingForMerge,
+			expectSince: time.Date(2024, 1, 4, 0, 0, 0, 0, time.UTC),
+		},
+		"changes_requested after ready_for_review": {
+			pullRequest: &github.PullRequest{
+				User:      &github.User{Login: github.String("author")},
+				CreatedAt: &github.Timestamp{time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)},
+			},
+			issueEvents: []*github.IssueEvent{
+				&github.IssueEvent{
+					Event:             github.String("review_requested"),
+					CreatedAt:         &github.Timestamp{time.Date(2024, 1, 2, 0, 0, 0, 0, time.UTC)}, // Earlier request
+					RequestedReviewer: &github.User{Login: github.String(firstCoreReviewer)},
+				},
+				&github.IssueEvent{
+					Event:     github.String("ready_for_review"),
+					CreatedAt: &github.Timestamp{time.Date(2024, 1, 3, 0, 0, 0, 0, time.UTC)},
+				},
+			},
+			reviews: []*github.PullRequestReview{
+				&github.PullRequestReview{
+					User:        &github.User{Login: github.String(firstCoreReviewer)},
+					State:       github.String("CHANGES_REQUESTED"),
+					SubmittedAt: &github.Timestamp{time.Date(2024, 1, 4, 0, 0, 0, 0, time.UTC)}, // Early changes requested
+				},
+			},
+			expectState: waitingForContributor,
+			expectSince: time.Date(2024, 1, 4, 0, 0, 0, 0, time.UTC), // Should use ready_for_review time
+		},
+
+		"changes_requested before ready_for_review": {
+			pullRequest: &github.PullRequest{
+				User:      &github.User{Login: github.String("author")},
+				CreatedAt: &github.Timestamp{time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)},
+			},
+			issueEvents: []*github.IssueEvent{
+				&github.IssueEvent{
+					Event:             github.String("review_requested"),
+					CreatedAt:         &github.Timestamp{time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)},
+					RequestedReviewer: &github.User{Login: github.String(firstCoreReviewer)},
+				},
+				&github.IssueEvent{
+					Event:     github.String("ready_for_review"),
+					CreatedAt: &github.Timestamp{time.Date(2024, 1, 3, 0, 0, 0, 0, time.UTC)}, // Earlier ready
+				},
+			},
+			reviews: []*github.PullRequestReview{
+				&github.PullRequestReview{
+					User:        &github.User{Login: github.String(firstCoreReviewer)},
+					State:       github.String("CHANGES_REQUESTED"),
+					SubmittedAt: &github.Timestamp{time.Date(2024, 1, 2, 0, 0, 0, 0, time.UTC)},
+				},
+			},
+			expectState: waitingForContributor,
+			expectSince: time.Date(2024, 1, 3, 0, 0, 0, 0, time.UTC), // Should use changes requested time since it's later
+		},
+
+		"comment review after ready_for_review": {
+			pullRequest: &github.PullRequest{
+				User:      &github.User{Login: github.String("author")},
+				CreatedAt: &github.Timestamp{time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)},
+			},
+			issueEvents: []*github.IssueEvent{
+				&github.IssueEvent{
+					Event:     github.String("ready_for_review"),
+					CreatedAt: &github.Timestamp{time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)},
+				},
+				&github.IssueEvent{
+					Event:             github.String("review_requested"),
+					CreatedAt:         &github.Timestamp{time.Date(2024, 1, 2, 0, 0, 0, 0, time.UTC)},
+					RequestedReviewer: &github.User{Login: github.String(firstCoreReviewer)},
+				},
+			},
+			reviews: []*github.PullRequestReview{
+				&github.PullRequestReview{
+					User:        &github.User{Login: github.String(firstCoreReviewer)},
+					State:       github.String("COMMENTED"),
+					SubmittedAt: &github.Timestamp{time.Date(2024, 1, 3, 0, 0, 0, 0, time.UTC)},
+				},
+			},
+			expectState: waitingForContributor,
+			expectSince: time.Date(2024, 1, 3, 0, 0, 0, 0, time.UTC), // Should use ready_for_review time
+		},
+
+		"multiple ready_for_review events": {
+			pullRequest: &github.PullRequest{
+				User:      &github.User{Login: github.String("author")},
+				CreatedAt: &github.Timestamp{time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)},
+			},
+			issueEvents: []*github.IssueEvent{
+				&github.IssueEvent{
+					Event:     github.String("ready_for_review"),
+					CreatedAt: &github.Timestamp{time.Date(2024, 1, 2, 0, 0, 0, 0, time.UTC)},
+				},
+				&github.IssueEvent{
+					Event:             github.String("review_requested"),
+					CreatedAt:         &github.Timestamp{time.Date(2024, 1, 3, 0, 0, 0, 0, time.UTC)},
+					RequestedReviewer: &github.User{Login: github.String(firstCoreReviewer)},
+				},
+				&github.IssueEvent{
+					Event:     github.String("ready_for_review"),
+					CreatedAt: &github.Timestamp{time.Date(2024, 1, 5, 0, 0, 0, 0, time.UTC)}, // Later ready_for_review
+				},
+			},
+			reviews: []*github.PullRequestReview{
+				&github.PullRequestReview{
+					User:        &github.User{Login: github.String(firstCoreReviewer)},
+					State:       github.String("CHANGES_REQUESTED"),
+					SubmittedAt: &github.Timestamp{time.Date(2024, 1, 4, 0, 0, 0, 0, time.UTC)},
+				},
+			},
+			expectState: waitingForContributor,
+			expectSince: time.Date(2024, 1, 5, 0, 0, 0, 0, time.UTC), // Should use latest ready_for_review time
+		},
 	}
 
 	for tn, tc := range cases {
@@ -786,8 +964,9 @@ func TestShouldNotify(t *testing.T) {
 }
 
 func TestFormatReminderComment(t *testing.T) {
-	firstCoreReviewer := membership.AvailableReviewers()[0]
-	secondCoreReviewer := membership.AvailableReviewers()[1]
+	availableReviewers := membership.AvailableReviewers()
+	firstCoreReviewer := availableReviewers[0]
+	secondCoreReviewer := availableReviewers[1]
 	cases := map[string]struct {
 		pullRequest        *github.PullRequest
 		state              pullRequestReviewState

.ci/magician/cmd/templates/vcr/with_replay_failed_tests.tmpl

@@ -9,4 +9,4 @@
 </blockquote>
 </details>
 
-[Get to know how VCR tests work](https://googlecloudplatform.github.io/magic-modules/docs/getting-started/contributing/#general-contributing-steps)
+[Get to know how VCR tests work](https://googlecloudplatform.github.io/magic-modules/develop/test/test/)

.ci/magician/cmd/test_terraform_vcr_test.go

@@ -447,7 +447,7 @@ func TestWithReplayFailedTests(t *testing.T) {
 					"</blockquote>",
 					"</details>",
 					"",
-					"[Get to know how VCR tests work](https://googlecloudplatform.github.io/magic-modules/docs/getting-started/contributing/#general-contributing-steps)",
+					"[Get to know how VCR tests work](https://googlecloudplatform.github.io/magic-modules/develop/test/test/)",
 				},
 				"\n",
 			),