Skip to content
Snippets Groups Projects

Fix deployment approval popup to support multiple approval rules

All threads resolved!

What does this MR do and why?

This merge request ensures the deployment approval popup supports multiple approval rules in environments index/detail pages.

It builds on the fix provided by @shinya.maeda in his proof of concept merge request, while taking into consideration the following:

  • The required_approval_count method of EE::Environment model is in fact used for determining if the popup should be displayed given a unified approval setting.
  • This can be seen in how pending_approval_count which uses the former method internally is used in views like deployment.vue and environment_approval.vue.
  • So in order to ensure we have that logic in one single place, I opted for updating required_approval_count to consider multiple approval rules.
  • With that in mind, I have also decided to update needs_approval? method in the same class to depend only on required_approval_count as we already check for existence of approval rules there via has_approval_rules? method.

Additionally, in order to make sure we show the popup in environment detail page, I had to do move the logic in show_deployment_approval? method in EE::EnvironmentsHelper to be in a new method can_approve_deployment? and adjust it to also consider multiple approval rules. This new method is also used in the aforementiond views to determine if a user can approve deployments based on their access and whether there exist an approval rule for a group or access level to which this user belong.

Resolves #362237 (closed).

Screenshots or screen recordings

Unified approval setting

In the screenshots below, the staging environment is protected and has unified approval setting configured via UI (where required_approval_count = 1).

This worked fine before (and still works fine after) the fix introduced in this merge request:

Protected environment added via UI Screenshot_2022-07-01_at_2.24.11_PM
Approval popup showing in both environment index/detail pages Screenshot_2022-07-01_at_2.28.52_PM

Screenshot_2022-07-01_at_2.29.01_PM

Screenshot_2022-07-01_at_2.29.18_PM

Multiple approval rules

In the screenshows below, the staging environment is also used, and is protected. This time, however, multiple approval rules are configured via the API (explained below) and there are two approval rules for different groups, each with its own required_approvals = 1 (so the sum of required approvals is 2; please note the current approvals count inside the popup).

The popup was not showing before the fix introduced in this merge request:

Protected environment added via API Screenshot_2022-07-01_at_2.38.10_PM
Approval popup showing in both environment index/detail pages Screenshot_2022-07-01_at_2.28.52_PM

Screenshot_2022-07-01_at_2.40.00_PM

Screenshot_2022-07-01_at_2.40.40_PM

How to set up and validate locally

To validate locally, please follow the steps below:

  • Make sure your local GDK setup is configured to work on GitLab EE (since both Protected Environments and Deployment Approvals are Premium features).
  • Create a new project or use an existing one.
  • Create a new group or use an existing one.
  • In your project, create a new environment from Deployments > Environments, and let's name it staging.
  • There's currently no way to configure multiple approval rules via UI, therefore, you need to configure them for this environment via REST API:
curl --header 'Content-Type: application/json' \
     --header "PRIVATE-TOKEN: YOUR_TOKEN" \
     --request POST \
     --data '{"name": "staging", "deploy_access_levels": [{"access_level": 40}], "approval_rules": [{"group_id": GROUP_ID}]}' \
     "http://gdk.test:3000/api/v4/projects/PROJECT_ID/protected_environments"

Please remember to change GROUP_ID, PROJECT_ID, YOUR_TOKEN with the respective values and the token you wish to use.

  • From Settings > CI/CD > Protected Environments, verify that staging is now protected.
  • In your project, update .gitlab-ci.yml file to include a deployment step:
deploy_staging:
  stage: deploy
  script:
    - echo "Deploy to staging"
  environment:
    name: staging
  • Push a commit to the project. A new deployment will be created.
  • Verify that this deployment is blocked waiting for approval by going to Deployments > Environments.
  • You should see the deployment approval popup under staging environment as shown in the screenshows above.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Ahmed Hemdan

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Allen Cook approved this merge request

    approved this merge request

  • Allen Cook requested review from @shinya.maeda

    requested review from @shinya.maeda

  • Allen Cook removed review request for @acook.gitlab

    removed review request for @acook.gitlab

  • Ahmed Hemdan added 1 commit

    added 1 commit

    • 232a535e - Update formatting to remove rubocop disable comments

    Compare with previous version

  • Ahmed Hemdan resolved all threads

    resolved all threads

  • Shinya Maeda
  • Shinya Maeda resolved all threads

    resolved all threads

  • mentioned in issue #367410 (closed)

  • Shinya Maeda approved this merge request

    approved this merge request

  • Shinya Maeda enabled an automatic merge when the pipeline for 09b6e87b succeeds

    enabled an automatic merge when the pipeline for 09b6e87b succeeds

  • merged

  • Shinya Maeda mentioned in commit dc1cb0d1

    mentioned in commit dc1cb0d1

  • added workflowstaging label and removed workflowcanary label

  • Ahmed Hemdan resolved all threads

    resolved all threads

  • mentioned in issue #367700 (closed)

  • Ahmed Hemdan mentioned in merge request !100382 (closed)

    mentioned in merge request !100382 (closed)

  • Please register or sign in to reply
    Loading