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

[TECH] Utiliser le token de github pour sécuriser l'auto merge. #3446

Merged
merged 3 commits into from
Oct 15, 2021

Conversation

yschoueri
Copy link
Contributor

@yschoueri yschoueri commented Sep 3, 2021

🦄 Problème

Un utilisateur externe existe et a un accès Maintainer sur le projet pour automatiser le merge des PRs

🤖 Solution

Github a un token spécifique pour les actions de projet qui sont auto-généré au moment de l'action. Ceci permet d'éviter de donner accès à des comptes externes et de devoir garder le token en variable secrète sur le repo. Voir détail ici

Montée de version de 0.8.3 à 0.14.3
CHANGELOG

🌈 Remarques

On a fait pareil sur Pix-UI : 1024pix/pix-ui#150
Voir la doc : https://docs.github.com/en/actions/security-guides/automatic-token-authentication

@pix-service
Copy link
Contributor

Copy link
Contributor

@jbuget jbuget left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@bpetetot
Copy link
Contributor

bpetetot commented Sep 22, 2021

Il me semblait qu'il y avait un problème lorsqu'on utilisait le token github car il a les droits "admin" et peut donc outrepasser les checks pour merger. Par conséquence, il peut merger même s'il y a des checks non valides. (par ex: le nombre d'approuve minimums)

Une explication ici : https://1024pix.atlassian.net/wiki/spaces/DEV/pages/1789591617/Auto-merge+de+Pull+Request

NB: Un utilisateur Github spécial a été créé pour cela, car ce dernier ne doit pas avoir les droits Admin (dans le répo Pix, un Admin peut merger n’importe quand). Il doit avoir les droits Maintain, pour pouvoir merger. De plus, au sein de l’organisation 1024Pix, par défaut les membres sont Admin, donc à défaut de pouvoir changer ça pour l’instant, notre user pix-service-auto-merge est un Outside collaborator.

Mais, désormais, il est possible de modifier les permissions du GITHUB TOKEN par Job : https://docs.github.com/en/actions/security-guides/automatic-token-authentication#modifying-the-permissions-for-the-github_token

Copy link
Contributor

@octo-topi octo-topi left a comment

Choose a reason for hiding this comment

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

🙏 J'ai rebase pour faire passer la CI sur API (bug SIECLE)
image

J'ai rebase encore une fois pour le test flaky du 30
image

Là, ça passe en local 😅
image

@octo-topi octo-topi force-pushed the tech-auto-merge-token branch 2 times, most recently from c83a444 to ff1f384 Compare October 4, 2021 07:31
@Anne-Gaelle-S Anne-Gaelle-S force-pushed the tech-auto-merge-token branch from ff1f384 to 68d105e Compare October 4, 2021 09:32
@github-actions github-actions bot force-pushed the tech-auto-merge-token branch from 6b4f50c to b988559 Compare October 15, 2021 07:33
@octo-topi
Copy link
Contributor

Je vois que cette PR est délaissée alors que la revue est Ok.
Je mets 🚀 , le vrai test sera au prochain merge

@github-actions github-actions bot merged commit cd8f9dc into dev Oct 15, 2021
@github-actions github-actions bot deleted the tech-auto-merge-token branch October 15, 2021 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants