search

smarkets / marge-bot

A merge-bot for GitLab
BSD 3-Clause "New" or "Revised" License
701 stars 136 forks source link

flood when CI failed #202

Closed micheelengronne closed 5 years ago

micheelengronne commented 5 years ago

Marge-bot sometimes floods comments of a MR with "I couldn't merge this branch: CI failed!" when the CI failed on a MR with marge-bot assigned.

It generally happens when trying to merge a MR created by renovate and using the same bot account.

Here is the log: 2019-06-11 08:54:58,436 INFO Processing !43 - 'feat: Update **** Docker tag to v1.35.0' 2019-06-11 08:54:58,921 INFO Ensuring MR !43 is mergeable 2019-06-11 08:54:58,948 INFO Running git -C /tmpxxo1dnyv/tmpupfoojcg fetch --prune origin 2019-06-11 08:54:59,276 INFO Running git -C /tmpxxo1dnyv/tmpupfoojcg checkout -B renovate/**** origin/renovate/**** -- 2019-06-11 08:54:59,399 INFO Running git -C /tmpxxo1dnyv/tmpupfoojcg rebase origin/master 2019-06-11 08:54:59,474 INFO Running git -C /tmpxxo1dnyv/tmpupfoojcg rev-parse HEAD 2019-06-11 08:54:59,479 INFO Running git -C /tmpxxo1dnyv/tmpupfoojcg rev-parse origin/master 2019-06-11 08:54:59,484 INFO Adding trailers for MR !43 2019-06-11 08:54:59,484 INFO Running git -C /tmpxxo1dnyv/tmpupfoojcg filter-branch --force --msg-filter 'TRAILERS='"'"'Tested-by: BOT <https://****/merge_requests/43>'"'"' python3 /nix/store/rsc74jcy093n1qdiqda61sj3valz7839-python3.6-marge-0.8.1/lib/python3.6/site-packages/marge/trailerfilter.py' 'renovate/****^..renovate/****' 2019-06-11 08:54:59,632 INFO Running git -C /tmpxxo1dnyv/tmpupfoojcg rev-parse HEAD 2019-06-11 08:54:59,637 INFO Running git -C /tmpxxo1dnyv/tmpupfoojcg filter-branch --force --msg-filter 'TRAILERS='"'"'Part-of: <https://****/merge_requests/43>'"'"' python3 /nix/store/rsc74jcy093n1qdiqda61sj3valz7839-python3.6-marge-0.8.1/lib/python3.6/site-packages/marge/trailerfilter.py' origin/master..renovate/**** 2019-06-11 08:54:59,776 INFO Running git -C /tmpxxo1dnyv/tmpupfoojcg rev-parse HEAD 2019-06-11 08:54:59,783 INFO Running git -C /tmpxxo1dnyv/tmpupfoojcg checkout renovate/**** -- 2019-06-11 08:55:00,033 INFO Running git -C /tmpxxo1dnyv/tmpupfoojcg diff-index --quiet HEAD 2019-06-11 08:55:00,039 INFO Running git -C /tmpxxo1dnyv/tmpupfoojcg ls-files --others 2019-06-11 08:55:00,045 INFO Running git -C /tmpxxo1dnyv/tmpupfoojcg push --force origin renovate/****:renovate/**** 2019-06-11 08:55:00,391 INFO Running git -C /tmpxxo1dnyv/tmpupfoojcg checkout -B master -- 2019-06-11 08:55:00,399 INFO Running git -C /tmpxxo1dnyv/tmpupfoojcg branch -D renovate/**** 2019-06-11 08:55:00,406 INFO Commit id to merge 'e4955d281d48b423a5d50bbdf4b17e672d3990f4' (into: '31a0835d5f5e8a4174b949df4afb22fea2d82c52') 2019-06-11 08:55:05,464 INFO Waiting for CI to pass for MR !43 2019-06-11 08:55:05,523 WARNING I couldn't merge this branch: CI failed! 2019-06-11 08:55:05,523 INFO Unassigning from MR !43

The bot is not unassigned as renovate created the MR with this same bot.

As a workaround, is it possible to have a regexp to exclude source branches as renovate branches always start with renovate ?

I try not to multiply bots as adding more accounts makes things more complex.

micheelengronne commented 5 years ago

Or is it possible to check already existing comments and not add one if the same CI pipeline failed ?

aschmolck commented 5 years ago

I think sharing the marge-bot account with other bots is probably not a good idea and I'm not keen on adding some workarounds to lessen the problems this causes.

Are you trying to conserve user licenses? If so, it might be worth talking to Gitlab and see if they are willing to not count the marge-bot account towards your allowance.

aschmolck commented 5 years ago

BTW, whilst I'm not keen on the ignore renovate branches solution, I'm happy to accept a merge request that ensures marge-bot does not re-assign to itself and instead unassigns (I might write something myself when I get around to it, but there's higher priority stuff for me ATM). That would likely also solve the flood problem.

aschmolck commented 5 years ago

I consider this the same as #189, so will close in favor of that.

micheelengronne commented 5 years ago

It has nothing to do with gitlab.com limitations. Limiting bots is a need for security reasons as it reduces the attack surface on bot authentication.

I was looking on a more general solution, but as you closed this issue I reformulated it here https://github.com/smarkets/marge-bot/issues/205

aschmolck commented 5 years ago

I understand the desire to not multiply bot accounts without need, because it means more credentials/accounts to audit for security purposes. On the other hand, in the limit, having a single bot account with maximal privileges on everything is hardly ideal either. Since marge-bot, due to the way gitlab works requires fairly extensive privileges if you want to use all features (e.g. automated re-approval requires the ability to impersonate users) from a security POV you probably don't want to spread these privileges to bots that require far less access, no?

In any case, the marge-bot workflow inherently assumes that merge requester and marge-bot are different users, because it's based on hand-off of responsibility ("should be good to merge -> assign to marge-bot -> marge-bot takes action -> oops, CI failure -> re-assign to merge-request submitting user -> user takes action to fix" etc).

In the special case that the merge requester never needs to take action on merge failure (I assume renovate doesn't try to do anything clever if some MR it created turns out to fail CI), you might be able to make it kinda work nonteheless without seperate user accounts for merge-requester and marge-bot. Wouldn't fixing #189 (by marge-bot never re-assigning to self) mostly do what you want? It should certainly get rid of the flood problem.

In any case, I'm also open to a PR for #205, since I could see this being useful for other scenarios. Will comment there.

micheelengronne commented 5 years ago

Normally https://github.com/smarkets/marge-bot/issues/189 should do the trick. I submitted a PR there.

As I do not want a bot to impersonate users (a security risk in my POV by lack of accounting), permissions needed for marge-bot and renovate-bot are the same, so It will be better to handle them with only one bot account.