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

Update NewDot deploy permissions. #4796

Merged
merged 6 commits into from
Aug 27, 2021
Merged

Conversation

roryabraham
Copy link
Contributor

@roryabraham roryabraham commented Aug 24, 2021

Details

  1. Opens up manual CP privileges to the @Expensify/mobile-deployers group
  2. Restricts production deploy abilities to the @Expensify/mobile-deployers group
  3. Allows @Expensify/mobile-deployers to run the platformDeploy.yml workflow. This means we can now retry failed deploy workflows. But be warned! With great power comes great responsibility. You must understand that if you push a tag or create a GH release, you will trigger a staging/production deploy.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/171977
$ https://github.com/Expensify/Expensify/issues/173687

Tests

This must be live-tested. Testing steps can happen out-of-order, but the complete list of things we need to verify is:

  • Merge a PR when the deploy checklist is unlocked. Verify that a staging deploy occurs as expected.
  • Have a mobile-deployer restart an active platformDeploy.yml workflow. Verify that it restarts and works as expected.
  • Have a non-mobile deployer restart a platformDeploy.yml workflow. Verify that it does not restart.
  • With the deploy checklist locked, merge a PR with the CP Staging label. It should be CP'd to staging.
  • Have a mobile-deployer (not a member of the deprecated expensify-cash-deployers team) attempt to manually trigger a CP. It should work as expected.
  • Have a non-mobile-deployer attempt to manually trigger a CP. It should not work.
  • Have a non-mobile deployer attempt to close out a StagingDeployCash where there are unchecked boxes in the description. It should not work.
  • Have a mobile-deployer attempt to close out a StagingDeployCash where there are unchecked boxes in the description. It should not work.
  • Have a non-mobile-deployer attempt to close out a StagingDeployCash with all the boxes checked and with the :shipit: comment. It should not work.
  • Have a mobile-deployer attempt to close out a StagingDeployCash with all the boxes checked and with the :shipit: comment. It should work and trigger the prod deploy.

@roryabraham roryabraham self-assigned this Aug 24, 2021
@roryabraham roryabraham requested a review from a team as a code owner August 24, 2021 00:59
@MelvinBot MelvinBot requested review from MonilBhavsar and removed request for a team August 24, 2021 01:00
MonilBhavsar
MonilBhavsar previously approved these changes Aug 24, 2021
Copy link
Contributor

@MonilBhavsar MonilBhavsar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

Julesssss
Julesssss previously approved these changes Aug 24, 2021
Copy link
Contributor

@Julesssss Julesssss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, what's the plan for testing? Should we turn each numbered item into a checkbox maybe?

Copy link
Contributor

@Jag96 Jag96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a couple of questions, but looks good

.github/workflows/finishReleaseCycle.yml Show resolved Hide resolved
.github/workflows/finishReleaseCycle.yml Outdated Show resolved Hide resolved
@roryabraham
Copy link
Contributor Author

Should we turn each numbered item into a checkbox maybe?

Excellent idea!

Jag96
Jag96 previously approved these changes Aug 24, 2021
MonilBhavsar
MonilBhavsar previously approved these changes Aug 25, 2021
Julesssss
Julesssss previously approved these changes Aug 25, 2021
@Julesssss
Copy link
Contributor

I imagine @AndrewGable is busy, but let's give him a chance to review too?

@roryabraham
Copy link
Contributor Author

Agreed, no big rush here.

@roryabraham
Copy link
Contributor Author

Updated this slightly so that we first validate the actor, then check for deploy blockers. This allows us to print a more specific error message.

@AndrewGable AndrewGable merged commit 2257b67 into main Aug 27, 2021
@AndrewGable AndrewGable deleted the Rory-MobileDeployerPermissions branch August 27, 2021 22:17
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@roryabraham
Copy link
Contributor Author

Have a non-mobile-deployer attempt to manually trigger a CP. It should not work.

Looks like this didn't work as expected – probably needed needs.validateActor.outputs.IS_DEPLOYER == 'true', which we've seen before.

@roryabraham
Copy link
Contributor Author

Okay, there is more than one way this did not work. Isabella was just able to run a prod deploy w/o the :shipit: comment, then the deploy checklist was reopened but no comment was left. This is basically a 🔥 now

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @AndrewGable in version: 1.0.88-3 🚀

platform result
🤖 android 🤖 cancelled 🔪
🖥 desktop 🖥 cancelled 🔪
🍎 iOS 🍎 cancelled 🔪
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

OSBotify commented Sep 1, 2021

🚀 Deployed to production by @roryabraham in version: 1.0.90-2 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 failure ❌
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants