Skip to content

Comment on stable branch MR when approved

Steve Abrams requested to merge delivery2840-package-and-test-message into master

What does this MR do and why?

Delivery is working to extend the maintenance policy for patch releases from one previous version to three to align with the security release process. Part of this change is we are going to allow developers to merge their backports directly into stable branches which are used to generate the releases.

In order to feel confident that an MR is safe to be included in a release, we are relying on the e2e:package-and-test-ee pipeline. This pipeline has some flaky tests so we currently need to allow it to fail, but want to be sure that any failures are reviewed by a Software Engineer in Test (SET) before it is merged.

This adds a new reactive processor that will post a new discussion on MRs targeting stable branches when they are initially approved. It will ping the reviewer to let them know the package-and-test job must be passing or the SET must be contacted for approval. WE are using a discussion rather than a comment, because this forces the maintainer or SET to resolve this discussion before the MR is merged.

Expected impact & dry-runs

To test this locally, I created a file spec/fixtures/reactive/merge_request_approval.json. The content of this fixture is fabricated so it matches our test case (an MR on gitlab-org/gitlab targeting a stable branch). I wasn't sure if it was something I should be committing with this MR or not, let me know if I should:

Fixture file content 
{
  "object_kind": "merge_request",
  "event_type": "merge_request",
  "user": {
    "id": 1,
    "name": "Steve Abrams",
    "username": "sabrams",
    "avatar_url": "https://secure.gravatar.com/avatar/cac831f7b515c191dfba32c54f01c375?s=800&d=identicon",
    "email": "sabrams@gitlab.com"
  },
  "project": {
    "id": 278964,
    "name":"gitlab",
    "description":"Triage operations for GitLab issues and merge requests. This is powered by https://gitlab.com/gitlab-org/ruby/gems/gitlab-triage.",
    "web_url":"https://gitlab.com/gitlab-org/gitlab/gitlab",
    "avatar_url":null,
    "git_ssh_url":"git@gitlab.com:gitlab-org/gitlab/gitlab.git",
    "git_http_url":"https://gitlab.com/gitlab-org/gitlab/gitlab.git",
    "namespace":"gitlab-org",
    "visibility_level":20,
    "path_with_namespace":"gitlab-org/gitlab",
    "default_branch":"master",
    "homepage":"https://gitlab.com/gitlab-org/gitlab/gitlab",
    "url":"https://gitlab.com/gitlab-org/gitlab/gitlab.git",
    "ssh_url":"git@gitlab.com:gitlab-org/gitlab/gitlab.git",
    "http_url":"https://gitlab.com/gitlab-org/gitlab/gitlab.git"
  },
  "repository": {
    "name": "gitlab",
    "url": "https://gitlab.com/gitlab-org/gitlab/gitlab.git",
    "description": "Triage operations for GitLab issues and merge requests.",
    "homepage": "https://gitlab.com/gitlab-org/gitlab/gitlab"
  },
  "object_attributes": {
    "id": 99,
    "iid": 112733,
    "target_branch": "15-8-stable-ee",
    "source_branch": "delivery2840-package-and-test-message",
    "source_project_id": 278964,
    "author_id": 4059254,
    "assignee_ids": [4059254],
    "assignee_id": 4059254,
    "reviewer_ids": [],
    "title": "Comment on stable branch MR when approved",
    "created_at": "2023-02-03T17:23:34Z",
    "updated_at": "2023-02-03T17:23:34Z",
    "milestone_id": null,
    "state": "opened",
    "blocking_discussions_resolved": true,
    "work_in_progress": false,
    "first_contribution": false,
    "merge_status": "unchecked",
    "target_project_id": 278964,
    "description": "foo",
    "url": "https://gitlab.com/gitlab-org/gitlab/gitlab/-/merge_requests/112733",
    "source": {
      "name":"gitlab",
      "description":"Triage operations for GitLab issues and merge requests.",
      "web_url":"https://gitlab.com/gitlab-org/gitlab/gitlab",
      "avatar_url":null,
      "git_ssh_url":"git@gitlab.com:gitlab-org/gitlab/gitlab.git",
      "git_http_url":"https://gitlab.com/gitlab-org/gitlab/gitlab.git",
      "namespace":"gitlab-org",
      "visibility_level":20,
      "path_with_namespace":"gitlab-org/gitlab",
      "default_branch":"master",
      "homepage":"https://gitlab.com/gitlab-org/gitlab/gitlab",
      "url":"https://gitlab.com/gitlab-org/gitlab/gitlab.git",
      "ssh_url":"git@gitlab.com:gitlab-org/gitlab/gitlab.git",
      "http_url":"https://gitlab.com/gitlab-org/gitlab/gitlab.git"
    },
    "target": {
      "name":"gitlab",
      "description":"Triage operations for GitLab issues and merge requests.",
      "web_url":"https://gitlab.com/gitlab-org/gitlab/gitlab",
      "avatar_url":null,
      "git_ssh_url":"git@gitlab.com:gitlab-org/gitlab/gitlab.git",
      "git_http_url":"https://gitlab.com/gitlab-org/gitlab/gitlab.git",
      "namespace":"gitlab-org",
      "visibility_level":20,
      "path_with_namespace":"gitlab-org/gitlab",
      "default_branch":"master",
      "homepage":"https://gitlab.com/gitlab-org/gitlab/gitlab",
      "url":"https://gitlab.com/gitlab-org/gitlab/gitlab.git",
      "ssh_url":"git@gitlab.com:gitlab-org/gitlab/gitlab.git",
      "http_url":"https://gitlab.com/gitlab-org/gitlab/gitlab.git"
    },
    "last_commit": {
      "id": "e3e6cd061b13648aa075a48552e162dd45ed63b9",
      "message": "Comment on stable branch MR when approved",
      "timestamp": "2023-02-04T23:36:29+02:00",
      "url": "https://gitlab.com/gitlab-org/gitlab/gitlab/commits/da1560886d4f094c3e6c9ef40349f7d38b5d27d7",
      "author": {
        "name": "GitLab dev user",
        "email": "gitlabdev@dv6700.(none)"
      }
    },
    "labels": [{
      "id": 206,
      "title": "API",
      "color": "#ffffff",
      "project_id": 14,
      "created_at": "2013-12-03T17:15:43Z",
      "updated_at": "2013-12-03T17:15:43Z",
      "template": false,
      "description": "API related issues",
      "type": "ProjectLabel",
      "group_id": 41
    }],
    "action": "approval",
    "detailed_merge_status": "mergeable"
  },
  "labels": [{
    "id": 206,
    "title": "API",
    "color": "#ffffff",
    "project_id": 14,
    "created_at": "2013-12-03T17:15:43Z",
    "updated_at": "2013-12-03T17:15:43Z",
    "template": false,
    "description": "API related issues",
    "type": "ProjectLabel",
    "group_id": 41
  }],
  "changes": {
    "updated_by_id": {
      "previous": null,
      "current": 4059254
    },
    "updated_at": {
      "previous": "2023-02-03 16:50:55 UTC",
      "current":"2023-02-05 16:52:00 UTC"
    }
  },
  "assignees": [
    {
      "id": 4059254,
      "name": "Steve Abrams",
      "username": "sabrams",
      "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=40\u0026d=identicon"
    }
  ],
  "reviewers": []
}

Then I was able to test aspects in the console:

~ DRY_RUN=1 bundle exec rack-console
[1] pry(main)> payload = JSON.parse(File.read('spec/fixtures/reactive/merge_request_approval.json'))
[2] pry(main)> event = Triage::Event.build(payload)
=> #<Triage::MergeRequestEvent:0x0000000152589f50
 @payload=
  {"object_kind"=>"merge_request",
   "event_type"=>"merge_request",
   "user"=>{"id"=>1, "name"=>"Steve Abrams", "username"=>"sabrams", "avatar_url"=>"https://secure.gravatar.com/avatar/cac831f7b515c191dfba32c54f01c375?s=800&d=identicon", "email"=>"sabrams@gitlab.com"},
   "project"=>
    {"id"=>278964,
...
[3] pry(main)> p = Triage::StableE2eCommentOnApproval.new(event)
=> #<Triage::StableE2eCommentOnApproval:0x0000000152418fe0
 @event=
  #<Triage::MergeRequestEvent:0x0000000152589f50
   @payload=
    {"object_kind"=>"merge_request",
     "event_type"=>"merge_request",
     "user"=>{"id"=>1, "name"=>"Steve Abrams", "username"=>"sabrams", "avatar_url"=>"https://secure.gravatar.com/avatar/cac831f7b515c191dfba32c54f01c375?s=800&d=identicon", "email"=>"sabrams@gitlab.com"},
     "project"=>
      {"id"=>278964,
...
[4] pry(main)> event.target_branch_is_stable_branch?
=> true
[5] pry(main)> event.from_gitlab_org_gitlab?
=> true
[6] pry(main)> p.applicable?
Gitlab::Error::Unauthorized: Server responded with code 401, message: 401 Unauthorized. Request URI: https://gitlab.com/api/v4/projects/278964/merge_requests/112733/notes
[7] pry(main)> p.process
=> "POST https://gitlab.com/api/v4/projects/278964/merge_requests/112733/discussions, body: `<!-- triage-serverless StableE2eCommentOnApproval -->\n:wave: `@sabrams`, thanks for approving this merge request.\n\nThis is the first time the merge request is approved. Please ensure the `e2e:package-and-test-ee` job\nhas succeeded. If there is a failure, a Software Engineer in Test (SET) needs to confirm the failures are unrelated to the merge request.\nIf there's no SET assigned to this team, ask for assistance on the `#quality` Slack channel.`"

Notes:

  • Command [6] - when checking applicable?, this error is expected since I do not have tokens set locally via environment variables, but we can see it is making the API call to check unique_comment.no_previous_comment?.
  • Command [7] - Since I have DRY_RUN=1, we do not make an API call to post a comment, but can see the request and comment body that would be posted.

Action items

  • [-] If adding environment variables for reactive processors, update config/triage-web.yaml and .gitlab/ci/triage-web.yml
  • [-] (If applicable) Add documentation to the handbook pages for Triage Operations =>
  • (If applicable) Identify the affected groups and how to communicate to them: These updates will all be communicated together when the new maintenance policy is rolled out.
    • [-] /cc @person_or_group =>
    • [-] Relevant Slack channels =>
    • [-] Engineering week-in-review

Related to gitlab-com/gl-infra/delivery#2840 (closed)

Edited by Steve Abrams

Merge request reports