Merge pull request #277 from utilitywarehouse/as-pr-cleanup

merged output upload will be re-implemented

go.mod

@@ -23,10 +23,12 @@ require (
 	github.com/utilitywarehouse/go-operational v0.0.0-20220413104526-79ce40a50281
 	golang.org/x/exp v0.0.0-20240823005443-9b4947da3948
 	golang.org/x/oauth2 v0.23.0
+	golang.org/x/time v0.6.0
 	gopkg.in/yaml.v2 v2.4.0
 	k8s.io/api v0.31.1
 	k8s.io/apimachinery v0.31.1
 	k8s.io/client-go v0.31.1
+	k8s.io/klog/v2 v2.130.1
 	sigs.k8s.io/controller-runtime v0.19.0
 	sigs.k8s.io/controller-runtime/tools/setup-envtest v0.0.0-20240820183333-e6c3d139d2b6
 	sigs.k8s.io/controller-tools v0.16.3
@@ -108,15 +110,13 @@ require (
 	golang.org/x/sys v0.24.0 // indirect
 	golang.org/x/term v0.23.0 // indirect
 	golang.org/x/text v0.17.0 // indirect
-	golang.org/x/time v0.6.0 // indirect
 	golang.org/x/tools v0.24.0 // indirect
 	gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect
 	google.golang.org/protobuf v1.34.2 // indirect
 	gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 	k8s.io/apiextensions-apiserver v0.31.0 // indirect
-	k8s.io/klog/v2 v2.130.1 // indirect
 	k8s.io/kube-openapi v0.0.0-20240822171749-76de80e0abd9 // indirect
 	k8s.io/utils v0.0.0-20240821151609-f90d01438635 // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect

prplanner/outputs.go

@@ -2,9 +2,7 @@ package prplanner
 
 import (
 	"context"
-	"fmt"
 	"regexp"
-	"strconv"
 	"strings"
 
 	"github.com/redis/go-redis/v9"
@@ -81,20 +79,6 @@ func (p *Planner) processRedisKeySetMsg(ctx context.Context, ch <-chan *redis.Me
 			CommentID = run.Request.PR.CommentID
 		}
 
-		// if run is apply run try and get pr num from commit msg
-		if prNum == 0 && run.Applied {
-			prNum = findPRNumber(run.CommitMsg)
-			if prNum != 0 {
-				// this commit is PR merge commit before posting output to the
-				// PR make sure there was actually plan runs on the PR
-				// this is to avoid uploading apply output on filtered PR
-				runs, _ := p.RedisClient.Runs(ctx, run.Module, fmt.Sprintf("PR:%d:*", prNum))
-				if len(runs) == 0 {
-					continue
-				}
-			}
-		}
-
 		// this is required in case this run is not a PR run && not apply run
 		if prNum == 0 {
 			continue
@@ -125,21 +109,3 @@ func (p *Planner) processRedisKeySetMsg(ctx context.Context, ch <-chan *redis.Me
 		p.Log.Info("run output posted", "module", run.Module, "pr", prNum)
 	}
 }
-
-// findPRNumber will try and find PR number from the following 2 types of
-// commit msg used by github when merging a PR
-// 'Merge pull request #268 from ....'
-// 'some commit msg... (#95532)'
-func findPRNumber(msg string) int {
-	if matches := mergePRRegex.FindStringSubmatch(msg); len(matches) == 2 {
-		prNum, _ := strconv.Atoi(matches[1])
-		return prNum
-	}
-
-	if matches := prNumSuffixRegex.FindStringSubmatch(msg); len(matches) == 2 {
-		prNum, _ := strconv.Atoi(matches[1])
-		return prNum
-	}
-
-	return 0
-}

prplanner/outputs_test.go

@@ -104,23 +104,6 @@ func Test_processRedisKeySetMsg(t *testing.T) {
 		time.Sleep(2 * time.Second)
 	})
 
-	t.Run("valid apply key", func(t *testing.T) {
-		key := "foo:admins:default:lastApply"
-
-		testRedis.EXPECT().Run(gomock.Any(), key).
-			Return(&tfaplv1beta1.Run{Module: types.NamespacedName{Namespace: "foo", Name: "admins"}, Applied: true, Request: &tfaplv1beta1.Request{}, CommitHash: "hash1", CommitMsg: "some commit msg... (#4)", Output: "terraform apply output"}, nil)
-
-		testRedis.EXPECT().Runs(gomock.Any(), types.NamespacedName{Namespace: "foo", Name: "admins"}, "PR:4:*").
-			Return([]*tfaplv1beta1.Run{{Module: types.NamespacedName{Namespace: "foo", Name: "admins"}, Request: &tfaplv1beta1.Request{PR: &tfaplv1beta1.PullRequest{Number: 4, CommentID: 123}}, CommitHash: "hash1", Output: "terraform plan output"}}, nil)
-
-		// mock github API Call adding new request info
-		testGithub.EXPECT().postComment("utilitywarehouse", "terraform-applier", 0, 4, gomock.Any()).
-			Return(123, nil)
-
-		ch <- &redis.Message{Channel: "__keyevent@0__:set", Payload: key}
-		time.Sleep(2 * time.Second)
-	})
-
 	t.Run("invalid channel", func(t *testing.T) {
 		key := "foo:admins:PR:4:d91f6ff"
 		// not expecting any other calls
@@ -139,26 +122,3 @@ func Test_processRedisKeySetMsg(t *testing.T) {
 		time.Sleep(2 * time.Second)
 	})
 }
-
-func Test_findPRNumber(t *testing.T) {
-	tests := []struct {
-		name string
-		msg  string
-		want int
-	}{
-		{"1", `Merge pull request #53 from utilitywarehouse/as-api`, 53},
-		{"2", `Merge pull request #123304040 from utilitywarehouse/as-api`, 123304040},
-		{"3", `remove duplicate groups`, 0},
-		{"5", `Something for customer data (#13531)`, 13531},
-		{"6", `whatever tis this . (#13529)`, 13529},
-		{"7", `(#13529)`, 13529},
-		{"8", `some (#13529) else `, 0},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := findPRNumber(tt.msg); got != tt.want {
-				t.Errorf("findPRNumber() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}

prplanner/type.go

@@ -138,10 +138,6 @@ type GitHubWebhook struct {
 	Action string `json:"action"`
 	Number int    `json:"number"`
 
-	Issue struct {
-		Number int `json:"number"`
-	} `json:"issue"`
-
 	Repository struct {
 		Name  string `json:"name"`
 		Owner struct {
@@ -151,6 +147,10 @@ type GitHubWebhook struct {
 	} `json:"repository"`
 
 	// only for comments
+	Issue struct {
+		Number int `json:"number"`
+	} `json:"issue"`
+
 	Comment struct {
 		User struct {
 			Login string `json:"login"`

prplanner/webhook.go

@@ -84,7 +84,7 @@ func (p *Planner) handleWebhook(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	w.WriteHeader(http.StatusBadRequest)
+	w.WriteHeader(http.StatusOK)
 }
 
 func (p *Planner) processPRWebHookEvent(event GitHubWebhook, prNumber int) {

prplanner/webhook_test.go

@@ -86,8 +86,8 @@ func Test_webhook(t *testing.T) {
 			t.Fatalf("Failed to send request: %v", err)
 		}
 
-		if resp.StatusCode != http.StatusBadRequest {
-			t.Errorf("Expected status %v, got %v", http.StatusBadRequest, resp.StatusCode)
+		if resp.StatusCode != http.StatusOK {
+			t.Errorf("Expected status %v, got %v", http.StatusOK, resp.StatusCode)
 		}
 	})
 
@@ -109,8 +109,8 @@ func Test_webhook(t *testing.T) {
 			t.Fatalf("Failed to send request: %v", err)
 		}
 
-		if resp.StatusCode != http.StatusBadRequest {
-			t.Errorf("Expected status %v, got %v", http.StatusBadRequest, resp.StatusCode)
+		if resp.StatusCode != http.StatusOK {
+			t.Errorf("Expected status %v, got %v", http.StatusOK, resp.StatusCode)
 		}
 	})
 }