Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Atlantis runs plan when bitbucket pr review request or description changed #3344

Closed
Almenon opened this issue Apr 21, 2023 · 7 comments · Fixed by #3402
Closed

Atlantis runs plan when bitbucket pr review request or description changed #3344

Almenon opened this issue Apr 21, 2023 · 7 comments · Fixed by #3402
Labels
bug Something isn't working help wanted Good feature for contributors provider/bitbucket

Comments

@Almenon
Copy link
Contributor

Almenon commented Apr 21, 2023

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request. Searching for pre-existing feature requests helps us consolidate datapoints for identical requirements into a single place, thank you!
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment.

Overview of the Issue

Atlantis runs plan when bitbucket pr review request or description changed

Reproduction Steps

Edit description or request PR review

Logs

My guess is this is less of a bug that needs logs for debugging and more of a design flaw caused by bitbucket changing their webhooks. As seen in 69d6fb2 there used to be more webhook types. I'm hoping there's still information inside the webhook payload that can be used to dynamically trigger the atlantis comment so it only comments when necessary.

Environment details

We're using the atlantis docker image ghcr.io/runatlantis/atlantis:v0.23.
We're using bitbucket cloud and have the following webhook triggers: https://i.imgur.com/SbIH5Je.png

Additional Context

@Almenon Almenon added the bug Something isn't working label Apr 21, 2023
@Almenon
Copy link
Contributor Author

Almenon commented Apr 21, 2023

Looking at the API it looks like it's possible? The updated event payload has a pullrequest field, and the pull request entity has a source property you could use to see if the SHA changed. You would have to store the state of the previous SHA somewhere to verify that it's different.

@GenPage
Copy link
Member

GenPage commented Apr 21, 2023

@Almenon Thank you for the bug report. The PR you linked to is for bitbucker self-hosted server, not cloud.

It does look like the API did change for listing changed files from the API:
Atlantis:

nextPageURL := fmt.Sprintf("%s/2.0/repositories/%s/pullrequests/%d/diffstat", b.BaseURL, repo.FullName, pull.Num)

Bitbucket docs: https://confluence.atlassian.com/bbkb/find-list-of-changed-files-within-a-pull-request-1206791779.html

@GenPage GenPage added help wanted Good feature for contributors provider/bitbucket labels Apr 21, 2023
@Almenon
Copy link
Contributor Author

Almenon commented Apr 28, 2023

I did some more digging. Looks like both editing PR description and requesting reviewers triggers the pullrequest:updated event.

Atlantis calls GetModifiedFiles in atlantis/server/events/vcs/bitbucketcloud/client.go through a chain of other methods triggered by handlePullRequestEvent in server\controllers\events\events_controller.go, but there's no logic there to handle cases where a pull request is updated without a commit being made. It just always runs the plan:

	case models.OpenedPullEvent, models.UpdatedPullEvent:
		// If the pull request was opened or updated, we will try to autoplan.

		// Respond with success and then actually execute the command asynchronously.
		// We use a goroutine so that this function returns and the connection is
		// closed.
		if !e.TestingMode {
			go e.CommandRunner.RunAutoplanCommand(baseRepo, headRepo, pull, user)
		} else {
			// When testing we want to wait for everything to complete.
			e.CommandRunner.RunAutoplanCommand(baseRepo, headRepo, pull, user)
		}
		return HTTPResponse{
			body: "Processing...",
		}

Also Atlantis doesn't pass in any state to GetModifiedFiles, so GetModifiedFiles has no way to tell if the files modified were changed in the last pullrequest:updated event or an older pullrequest:updated event.

@lkysow fixed a similar issue in #946. They updated their handler in events_controller (HandleAzureDevopsPullRequestEvent, in this case) to ignore specific events. However, their case was simpler because the payload had a message explaining the cause of the event.

In this case we would have to be statefull. We would need to save the commit sha in the source property and check if that changed since the last event. Maybe a global variable var bitbucketLastSha = "" at the top of the file? We wouldn't need to persist data to the disk, in the unlikely event of a server crash it's not a big deal if it runs a extra plan.

The SHA is located in .pullrequest.source.commit.hash

Event Payload:
{
  "repository": {
    "type": "repository",
    "full_name": "almenon/atlantis-test-1",
    "links": {
      "self": {
        "href": "https://api.bitbucket.org/2.0/repositories/almenon/atlantis-test-1"
      },
      "html": {
        "href": "https://bitbucket.org/almenon/atlantis-test-1"
      },
      "avatar": {
        "href": "https://bytebucket.org/ravatar/%7Ba3b849c8-95f5-481b-8281-8c5dc6734e74%7D?ts=default"
      }
    },
    "name": "atlantis-test-1",
    "scm": "git",
    "website": null,
    "owner": {
      "display_name": "Almenon",
      "links": {
        "self": {
          "href": "https://api.bitbucket.org/2.0/users/%7Bbba8c402-f0e9-4950-afb5-7f7d1bdf18ad%7D"
        },
        "avatar": {
          "href": "https://secure.gravatar.com/avatar/6191269ac249a0295af04975b2990165?d=https%3A%2F%2Favatar-management--avatars.us-west-2.prod.public.atl-paas.net%2Finitials%2FCC-3.png"
        },
        "html": {
          "href": "https://bitbucket.org/%7Bbba8c402-f0e9-4950-afb5-7f7d1bdf18ad%7D/"
        }
      },
      "type": "user",
      "uuid": "{bba8c402-f0e9-4950-afb5-7f7d1bdf18ad}",
      "account_id": "557058:780c686b-ad69-4fc6-a202-fb9ccf0e933f",
      "nickname": "almenon"
    },
    "workspace": {
      "type": "workspace",
      "uuid": "{bba8c402-f0e9-4950-afb5-7f7d1bdf18ad}",
      "name": "Almenon",
      "slug": "almenon",
      "links": {
        "avatar": {
          "href": "https://bitbucket.org/workspaces/almenon/avatar/?ts=1543630138"
        },
        "html": {
          "href": "https://bitbucket.org/almenon/"
        },
        "self": {
          "href": "https://api.bitbucket.org/2.0/workspaces/almenon"
        }
      }
    },
    "is_private": false,
    "project": {
      "type": "project",
      "key": "PROJ",
      "uuid": "{6eaa9c3d-759d-4af6-9a34-ba460b0292eb}",
      "name": "Untitled project",
      "links": {
        "self": {
          "href": "https://api.bitbucket.org/2.0/workspaces/almenon/projects/PROJ"
        },
        "html": {
          "href": "https://bitbucket.org/almenon/workspace/projects/PROJ"
        },
        "avatar": {
          "href": "https://bitbucket.org/account/user/almenon/projects/PROJ/avatar/32?ts=1543630138"
        }
      }
    },
    "uuid": "{a3b849c8-95f5-481b-8281-8c5dc6734e74}"
  },
  "actor": {
    "display_name": "Almenon",
    "links": {
      "self": {
        "href": "https://api.bitbucket.org/2.0/users/%7Bbba8c402-f0e9-4950-afb5-7f7d1bdf18ad%7D"
      },
      "avatar": {
        "href": "https://secure.gravatar.com/avatar/6191269ac249a0295af04975b2990165?d=https%3A%2F%2Favatar-management--avatars.us-west-2.prod.public.atl-paas.net%2Finitials%2FCC-3.png"
      },
      "html": {
        "href": "https://bitbucket.org/%7Bbba8c402-f0e9-4950-afb5-7f7d1bdf18ad%7D/"
      }
    },
    "type": "user",
    "uuid": "{bba8c402-f0e9-4950-afb5-7f7d1bdf18ad}",
    "account_id": "557058:780c686b-ad69-4fc6-a202-fb9ccf0e933f",
    "nickname": "almenon"
  },
  "pullrequest": {
    "comment_count": 4,
    "task_count": 0,
    "type": "pullrequest",
    "id": 1,
    "title": "main.tf edited online with Bitbucket",
    "description": "main.tf edited online with Bitbucket",
    "rendered": {
      "title": {
        "type": "rendered",
        "raw": "main.tf edited online with Bitbucket",
        "markup": "markdown",
        "html": "<pre class=\"plaintext\">main.tf edited online with Bitbucket</pre>"
      },
      "description": {
        "type": "rendered",
        "raw": "main.tf edited online with Bitbucket",
        "markup": "markdown",
        "html": "<pre class=\"plaintext\">main.tf edited online with Bitbucket</pre>"
      }
    },
    "state": "OPEN",
    "merge_commit": null,
    "close_source_branch": true,
    "closed_by": null,
    "author": {
      "display_name": "Almenon",
      "links": {
        "self": {
          "href": "https://api.bitbucket.org/2.0/users/%7Bbba8c402-f0e9-4950-afb5-7f7d1bdf18ad%7D"
        },
        "avatar": {
          "href": "https://secure.gravatar.com/avatar/6191269ac249a0295af04975b2990165?d=https%3A%2F%2Favatar-management--avatars.us-west-2.prod.public.atl-paas.net%2Finitials%2FCC-3.png"
        },
        "html": {
          "href": "https://bitbucket.org/%7Bbba8c402-f0e9-4950-afb5-7f7d1bdf18ad%7D/"
        }
      },
      "type": "user",
      "uuid": "{bba8c402-f0e9-4950-afb5-7f7d1bdf18ad}",
      "account_id": "557058:780c686b-ad69-4fc6-a202-fb9ccf0e933f",
      "nickname": "almenon"
    },
    "reason": "",
    "created_on": "2023-04-28T20:30:05.606390+00:00",
    "updated_on": "2023-04-28T21:03:42.302938+00:00",
    "destination": {
      "branch": {
        "name": "main"
      },
      "commit": {
        "type": "commit",
        "hash": "ab76ead84349",
        "links": {
          "self": {
            "href": "https://api.bitbucket.org/2.0/repositories/almenon/atlantis-test-1/commit/ab76ead84349"
          },
          "html": {
            "href": "https://bitbucket.org/almenon/atlantis-test-1/commits/ab76ead84349"
          }
        }
      },
      "repository": {
        "type": "repository",
        "full_name": "almenon/atlantis-test-1",
        "links": {
          "self": {
            "href": "https://api.bitbucket.org/2.0/repositories/almenon/atlantis-test-1"
          },
          "html": {
            "href": "https://bitbucket.org/almenon/atlantis-test-1"
          },
          "avatar": {
            "href": "https://bytebucket.org/ravatar/%7Ba3b849c8-95f5-481b-8281-8c5dc6734e74%7D?ts=default"
          }
        },
        "name": "atlantis-test-1",
        "uuid": "{a3b849c8-95f5-481b-8281-8c5dc6734e74}"
      }
    },
    "source": {
      "branch": {
        "name": "Almenon/maintf-edited-online-with-bitbucket-1682713796798"
      },
      "commit": {
        "type": "commit",
        "hash": "f4dc2ecb02bf",
        "links": {
          "self": {
            "href": "https://api.bitbucket.org/2.0/repositories/almenon/atlantis-test-1/commit/f4dc2ecb02bf"
          },
          "html": {
            "href": "https://bitbucket.org/almenon/atlantis-test-1/commits/f4dc2ecb02bf"
          }
        }
      },
      "repository": {
        "type": "repository",
        "full_name": "almenon/atlantis-test-1",
        "links": {
          "self": {
            "href": "https://api.bitbucket.org/2.0/repositories/almenon/atlantis-test-1"
          },
          "html": {
            "href": "https://bitbucket.org/almenon/atlantis-test-1"
          },
          "avatar": {
            "href": "https://bytebucket.org/ravatar/%7Ba3b849c8-95f5-481b-8281-8c5dc6734e74%7D?ts=default"
          }
        },
        "name": "atlantis-test-1",
        "uuid": "{a3b849c8-95f5-481b-8281-8c5dc6734e74}"
      }
    },
    "reviewers": [],
    "participants": [
      {
        "type": "participant",
        "user": {
          "display_name": "Almenon",
          "links": {
            "self": {
              "href": "https://api.bitbucket.org/2.0/users/%7Bbba8c402-f0e9-4950-afb5-7f7d1bdf18ad%7D"
            },
            "avatar": {
              "href": "https://secure.gravatar.com/avatar/6191269ac249a0295af04975b2990165?d=https%3A%2F%2Favatar-management--avatars.us-west-2.prod.public.atl-paas.net%2Finitials%2FCC-3.png"
            },
            "html": {
              "href": "https://bitbucket.org/%7Bbba8c402-f0e9-4950-afb5-7f7d1bdf18ad%7D/"
            }
          },
          "type": "user",
          "uuid": "{bba8c402-f0e9-4950-afb5-7f7d1bdf18ad}",
          "account_id": "557058:780c686b-ad69-4fc6-a202-fb9ccf0e933f",
          "nickname": "almenon"
        },
        "role": "PARTICIPANT",
        "approved": false,
        "state": null,
        "participated_on": "2023-04-28T21:03:02.364549+00:00"
      }
    ],
    "links": {
      "self": {
        "href": "https://api.bitbucket.org/2.0/repositories/almenon/atlantis-test-1/pullrequests/1"
      },
      "html": {
        "href": "https://bitbucket.org/almenon/atlantis-test-1/pull-requests/1"
      },
      "commits": {
        "href": "https://api.bitbucket.org/2.0/repositories/almenon/atlantis-test-1/pullrequests/1/commits"
      },
      "approve": {
        "href": "https://api.bitbucket.org/2.0/repositories/almenon/atlantis-test-1/pullrequests/1/approve"
      },
      "request-changes": {
        "href": "https://api.bitbucket.org/2.0/repositories/almenon/atlantis-test-1/pullrequests/1/request-changes"
      },
      "diff": {
        "href": "https://api.bitbucket.org/2.0/repositories/almenon/atlantis-test-1/diff/almenon/atlantis-test-1:f4dc2ecb02bf%0Dab76ead84349?from_pullrequest_id=1&topic=true"
      },
      "diffstat": {
        "href": "https://api.bitbucket.org/2.0/repositories/almenon/atlantis-test-1/diffstat/almenon/atlantis-test-1:f4dc2ecb02bf%0Dab76ead84349?from_pullrequest_id=1&topic=true"
      },
      "comments": {
        "href": "https://api.bitbucket.org/2.0/repositories/almenon/atlantis-test-1/pullrequests/1/comments"
      },
      "activity": {
        "href": "https://api.bitbucket.org/2.0/repositories/almenon/atlantis-test-1/pullrequests/1/activity"
      },
      "merge": {
        "href": "https://api.bitbucket.org/2.0/repositories/almenon/atlantis-test-1/pullrequests/1/merge"
      },
      "decline": {
        "href": "https://api.bitbucket.org/2.0/repositories/almenon/atlantis-test-1/pullrequests/1/decline"
      },
      "statuses": {
        "href": "https://api.bitbucket.org/2.0/repositories/almenon/atlantis-test-1/pullrequests/1/statuses"
      }
    },
    "summary": {
      "type": "rendered",
      "raw": "main.tf edited online with Bitbucket",
      "markup": "markdown",
      "html": "<pre class=\"plaintext\">main.tf edited online with Bitbucket</pre>"
    }
  }
}

@jamengual
Copy link
Contributor

PRs are welcome, we do not have Bitbucket maintainers

@Almenon
Copy link
Contributor Author

Almenon commented May 11, 2023

I'm working on a PR at the moment. So the problem with a single string variable is that there's multiple PR's, and one PR could overwrite another PR's last SHA. So currently I'm thinking about importing "github.com/hashicorp/golang-lru/v2" and using a lru.New[string, string](300), key being the PR url and value being the SHA.

Almenon@18a05d4

@GenPage
Copy link
Member

GenPage commented May 11, 2023

I'm not crazy about setting up a LRU cache if we don't have to. Bitbucket added new events to their Bitbucket Server offering and they would be perfect for Cloud in this use case as well. 'Source branch updated' is exactly what we are trying to figure out here

https://jira.atlassian.com/browse/BSERV-10279.

Worst case, if their support doesn't advise the best path forward, I would say implement the LRU workaround for now. There's always the option of updating the PR model to make this more consistent across VCS backends in Atlantis as well, but that's a heavier refactor.

@Almenon
Copy link
Contributor Author

Almenon commented May 11, 2023

I'd love a 'Source branch updated' webhook! It would be perfect for Cloud too, if they offered it for cloud. Unfortunately they don't and considering the slow pace of bitbucket development (they still haven't added https://jira.atlassian.com/browse/BCLOUD-12503 and it's been over 5 years) it will probably be a while.

I'll implement the LRU workaround for now. I actually already implemented it and tested it by the time I saw your comment 😅

Also I'll add a comment and a +1 to https://jira.atlassian.com/browse/BCLOUD-21184, which tracks the 'Source branch updated' feature for cloud. I suggest adding yourself as a watcher.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Good feature for contributors provider/bitbucket
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants