Skip to content

Conversation

bogsi17
Copy link
Contributor

@bogsi17 bogsi17 commented Apr 15, 2025

We want to restrict the IAM role used by the Github workflows such that it can only be assumed by workflow runs from the main and release branches. This causes the problem that deployment workflows run from tags could not assume the role anymore.

To mitigate this issue and still allow deploying specific tags, this PR allows to specify the git tag that shall be deployed in a dedicated field. While the workflow can be run from main or release, this still allows to deploy a specific git tag. If no value is set, it defaults to the regular behaviour.

@bogsi17 bogsi17 marked this pull request as draft April 15, 2025 10:45
@bogsi17 bogsi17 force-pushed the MAV-997_permission_restriction branch from e7f61f0 to 72bccfe Compare April 16, 2025 09:55
@bogsi17 bogsi17 changed the base branch from main to v2.2.0-wip April 16, 2025 09:57
@bogsi17 bogsi17 marked this pull request as ready for review April 16, 2025 09:58
@bogsi17 bogsi17 force-pushed the MAV-997_permission_restriction branch from 72bccfe to a534a2f Compare April 16, 2025 10:48
@misaka
Copy link
Contributor

misaka commented Apr 16, 2025

If we are running deploy-infrastructure with an image_tag, do we not also need to do the same thing with deploy-application?

Looking at the deploy-application workflow it appears to checkout the code and uses settings from TF. At a guess, even though it may be unlikely to diverge we probably still want to make sure that those values match whatever is on the branch we're deploying.

@bogsi17
Copy link
Contributor Author

bogsi17 commented Apr 17, 2025

If we are running deploy-infrastructure with an image_tag, do we not also need to do the same thing with deploy-application?

Looking at the deploy-application workflow it appears to checkout the code and uses settings from TF. At a guess, even though it may be unlikely to diverge we probably still want to make sure that those values match whatever is on the branch we're deploying.

@misaka You're right, I will add it

Base automatically changed from v2.2.0-wip to main April 17, 2025 11:27
@bogsi17 bogsi17 force-pushed the MAV-997_permission_restriction branch from a9c4fc0 to a710595 Compare April 17, 2025 13:56
@bogsi17 bogsi17 force-pushed the MAV-997_permission_restriction branch from f4cd1e6 to 09b2903 Compare April 17, 2025 14:29
@bogsi17 bogsi17 force-pushed the MAV-997_permission_restriction branch from 09b2903 to 496129f Compare April 17, 2025 14:35
@bogsi17 bogsi17 force-pushed the MAV-997_permission_restriction branch from 496129f to 4151e0d Compare April 17, 2025 14:39
@thomasleese thomasleese added the infrastructure Related to infrastructure changes label Apr 20, 2025
@bogsi17 bogsi17 force-pushed the MAV-997_permission_restriction branch from 71eff9c to 0aaf1a1 Compare April 22, 2025 11:19
@bogsi17 bogsi17 force-pushed the MAV-997_permission_restriction branch from 0aaf1a1 to dd9d56e Compare April 22, 2025 11:20
@bogsi17 bogsi17 force-pushed the MAV-997_permission_restriction branch from dd9d56e to 2482517 Compare April 22, 2025 11:22
@bogsi17 bogsi17 requested a review from misaka April 22, 2025 15:17
bogsi17 added 3 commits April 23, 2025 10:21
* We want to restrict the IAM role used by the Github workflows such that it can only be assumed from the main branch.
* This causes the problem that deployment workflows run from tags could not assume the role anymore
* To mitigate this issue, this PR allows to specify the git tag that shall be deployed in a dedicated field. While the workflow can be run from main, this still allows to deploy a specific git tag. If no value is set, it defaults to the regular behaviour.
* It is ensured that the terraform files and the docker image being deployed always originate from the same commit
* 'main' is the only protected branch, 'release' is not protected
@bogsi17 bogsi17 force-pushed the MAV-997_permission_restriction branch from 9130fcf to 987db19 Compare April 23, 2025 09:23
@tvararu tvararu temporarily deployed to mavis-pr-3407 April 23, 2025 09:24 Inactive
Copy link

Copy link
Contributor

@misaka misaka 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! We agreed we'd leave the new CodeQL warning alone for now, and address in a later PR. 👍

@bogsi17 bogsi17 merged commit cd31e21 into main Apr 23, 2025
34 of 35 checks passed
@bogsi17 bogsi17 deleted the MAV-997_permission_restriction branch April 23, 2025 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Related to infrastructure changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants