fix(bitbucket): avoid extra plans using a local LRU (runatlantis#3402)

Author: Almenon

.gitignore

@@ -24,3 +24,6 @@ dist/
 tmp-CHANGELOG.md
 
 .envrc
+
+# IDE files
+*.code-workspace

go.mod

@@ -20,6 +20,7 @@ require (
 	github.com/hashicorp/go-getter/v2 v2.2.1
 	github.com/hashicorp/go-multierror v1.1.1
 	github.com/hashicorp/go-version v1.6.0
+	github.com/hashicorp/golang-lru/v2 v2.0.2
 	github.com/hashicorp/terraform-config-inspect v0.0.0-20230614215431-f32df32a01cd
 	github.com/kr/pretty v0.3.1
 	github.com/mcdafydd/go-azuredevops v0.12.1

go.sum

@@ -253,6 +253,9 @@ github.com/hashicorp/go-version v1.6.0 h1:feTTfFNnjP967rlCxM/I9g701jU+RN74YKx2mO
 github.com/hashicorp/go-version v1.6.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
 github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
 github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
+github.com/hashicorp/golang-lru v0.5.4 h1:YDjusn29QI/Das2iO9M0BHnIbxPeyuCHsjMW+lJfyTc=
+github.com/hashicorp/golang-lru/v2 v2.0.2 h1:Dwmkdr5Nc/oBiXgJS3CDHNhJtIHkuZ3DZF5twqnfBdU=
+github.com/hashicorp/golang-lru/v2 v2.0.2/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM=
 github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4=
 github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ=
 github.com/hashicorp/hcl/v2 v2.17.0 h1:z1XvSUyXd1HP10U4lrLg5e0JMVz6CPaJvAgxM0KNZVY=

runatlantis.io/docs/autoplanning.md

@@ -32,6 +32,12 @@ Given the directory structure:
 * If `project1/modules/module1/main.tf` were modified, we would look one level above `project1/modules`
 into `project1/`, see that there was a `main.tf` file and so run plan in `project1/`
 
+## Bitbucket-Specific Notes
+Bitbucket does not have a webhook that triggers only upon a new PR or commit. To fix this we cache the last commit to see if it has changed. If the cache is emptied, Atlantis will think your commit is new and you may see extra plans.
+This scenario can happen if:
+* Atlantis restarts
+* You are running multiple Atlantis instances behind a load balancer
+
 ## Customizing
 If you would like to customize how Atlantis determines which directory to run in
 or disable it all together you need to create an `atlantis.yaml` file.

server/controllers/events/events_controller.go

@@ -366,7 +366,8 @@ func (e *VCSEventsController) handleBitbucketCloudPullRequestEvent(w http.Respon
 		e.respond(w, logging.Error, http.StatusBadRequest, "Error parsing pull data: %s %s=%s", err, bitbucketCloudRequestIDHeader, reqID)
 		return
 	}
-	pullEventType := e.Parser.GetBitbucketCloudPullEventType(eventType)
+	e.Logger.Debug("SHA is %q", pull.HeadCommit)
+	pullEventType := e.Parser.GetBitbucketCloudPullEventType(eventType, pull.HeadCommit, pull.URL)
 	e.Logger.Info("identified event as type %q", pullEventType.String())
 	resp := e.handlePullRequestEvent(e.Logger, baseRepo, headRepo, pull, user, pullEventType)
 

server/events/event_parser.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/go-playground/validator/v10"
 	"github.com/google/go-github/v53/github"
+	lru "github.com/hashicorp/golang-lru/v2"
 	"github.com/mcdafydd/go-azuredevops/azuredevops"
 	"github.com/pkg/errors"
 	"github.com/runatlantis/atlantis/server/events/command"
@@ -34,6 +35,8 @@ import (
 const gitlabPullOpened = "opened"
 const usagesCols = 90
 
+var lastBitbucketSha, _ = lru.New[string, string](300)
+
 // PullCommand is a command to run on a pull request.
 type PullCommand interface {
 	// CommandName is the name of the command we're running.
@@ -267,7 +270,7 @@ type EventParsing interface {
 
 	// GetBitbucketCloudPullEventType returns the type of the pull request
 	// event given the Bitbucket Cloud header.
-	GetBitbucketCloudPullEventType(eventTypeHeader string) models.PullRequestEventType
+	GetBitbucketCloudPullEventType(eventTypeHeader string, sha string, pr string) models.PullRequestEventType
 
 	// ParseBitbucketServerPullEvent parses a pull request event from Bitbucket
 	// Server.
@@ -343,11 +346,18 @@ func (e *EventParser) ParseAPIPlanRequest(vcsHostType models.VCSHostType, repoFu
 
 // GetBitbucketCloudPullEventType returns the type of the pull request
 // event given the Bitbucket Cloud header.
-func (e *EventParser) GetBitbucketCloudPullEventType(eventTypeHeader string) models.PullRequestEventType {
+func (e *EventParser) GetBitbucketCloudPullEventType(eventTypeHeader string, sha string, pr string) models.PullRequestEventType {
 	switch eventTypeHeader {
 	case bitbucketcloud.PullCreatedHeader:
+		lastBitbucketSha.Add(pr, sha)
 		return models.OpenedPullEvent
 	case bitbucketcloud.PullUpdatedHeader:
+		lastSha, _ := lastBitbucketSha.Get(pr)
+		if sha == lastSha {
+			// No change, ignore
+			return models.OtherPullEvent
+		}
+		lastBitbucketSha.Add(pr, sha)
 		return models.UpdatedPullEvent
 	case bitbucketcloud.PullFulfilledHeader, bitbucketcloud.PullRejectedHeader:
 		return models.ClosedPullEvent

server/events/event_parser_test.go

@@ -998,6 +998,42 @@ func TestParseBitbucketCloudPullEvent_States(t *testing.T) {
 	}
 }
 
+func TestBitBucketNonCodeChangesAreIgnored(t *testing.T) {
+	// lets say a user opens a PR
+	act := parser.GetBitbucketCloudPullEventType("pullrequest:created", "fakeSha", "https://github.com/fakeorg/fakerepo/pull/1")
+	Equals(t, models.OpenedPullEvent, act)
+	// Another update with same SHA should be ignored
+	act = parser.GetBitbucketCloudPullEventType("pullrequest:updated", "fakeSha", "https://github.com/fakeorg/fakerepo/pull/1")
+	Equals(t, models.OtherPullEvent, act)
+	// Only if SHA changes do we act
+	act = parser.GetBitbucketCloudPullEventType("pullrequest:updated", "fakeSha2", "https://github.com/fakeorg/fakerepo/pull/1")
+	Equals(t, models.UpdatedPullEvent, act)
+
+	// If sha changes in seperate PR,
+	act = parser.GetBitbucketCloudPullEventType("pullrequest:updated", "otherPRSha", "https://github.com/fakeorg/fakerepo/pull/2")
+	Equals(t, models.UpdatedPullEvent, act)
+	// We will still ignore same shas in first PR
+	act = parser.GetBitbucketCloudPullEventType("pullrequest:updated", "fakeSha2", "https://github.com/fakeorg/fakerepo/pull/1")
+	Equals(t, models.OtherPullEvent, act)
+}
+
+func TestBitbucketShaCacheExpires(t *testing.T) {
+	// lets say a user opens a PR
+	act := parser.GetBitbucketCloudPullEventType("pullrequest:created", "fakeSha", "https://github.com/fakeorg/fakerepo/pull/1")
+	Equals(t, models.OpenedPullEvent, act)
+	// Another update with same SHA should be ignored
+	act = parser.GetBitbucketCloudPullEventType("pullrequest:updated", "fakeSha", "https://github.com/fakeorg/fakerepo/pull/1")
+	Equals(t, models.OtherPullEvent, act)
+	// But after 300 times, the cache should expire
+	// this is so we don't have ever increasing memory usage
+	for i := 0; i < 302; i++ {
+		parser.GetBitbucketCloudPullEventType("pullrequest:updated", "fakeSha", fmt.Sprintf("https://github.com/fakeorg/fakerepo/pull/%d", i))
+	}
+	// and now SHA will seen as a change again
+	act = parser.GetBitbucketCloudPullEventType("pullrequest:updated", "fakeSha", "https://github.com/fakeorg/fakerepo/pull/1")
+	Equals(t, models.UpdatedPullEvent, act)
+}
+
 func TestGetBitbucketCloudEventType(t *testing.T) {
 	cases := []struct {
 		header string
@@ -1026,7 +1062,9 @@ func TestGetBitbucketCloudEventType(t *testing.T) {
 	}
 	for _, c := range cases {
 		t.Run(c.header, func(t *testing.T) {
-			act := parser.GetBitbucketCloudPullEventType(c.header)
+			// we pass in the header as the SHA so the SHA changes each time
+			// the code will ignore duplicate SHAS to avoid extra TF plans
+			act := parser.GetBitbucketCloudPullEventType(c.header, c.header, "https://github.com/fakeorg/fakerepo/pull/1")
 			Equals(t, c.exp, act)
 		})
 	}