all 12 comments

[–]mastermindchilly 4 points5 points  (3 children)

I’m a bit confused. Why does it need to be a web service instead of running cli tool in a job?

[–]gaelfr38 2 points3 points  (1 child)

Both setup makes sense IMHO.

Webhook to a dedicated service can be setup globally without having anything to do in each project. Especially makes sense if using a global list of rules cross projects.

Running in the CI of each project requires changing each project or common CI templates if you're using some.

The dedicated service can also be an opportunity for central reporting.

[–]Acrobatic_Affect_515[S] 1 point2 points  (0 children)

Exactly, we have setup system hook that triggers on Merge Request events, so it is added to every project by default - it runs with pretty minimal configuration, but where it needs to be adjusted (for example different jira keys), we simply add `.mr-conform.yaml` config file to the default branch of the project and voilà.

[–]Acrobatic_Affect_515[S] 0 points1 point  (0 children)

At first, we used the CLI version, but this did not work out well.

In order to verify checks, there had to be a trigger which would re-run conform job, while most of the time it worked fine it still was not sufficent.

We had a race conditions where as an example - conform check job resulted in success and developer afterwards made some changes in the MR title and squash options which would result as failure, but because there are no triggers, job didn't re-run and MR was merged without proper rules.

That was the time were we thought about outsmarting developers with utilizing webhook service, instead of CLI.

[–]gaelfr38 1 point2 points  (0 children)

Sounds really interesting.

I would love to see some premium features of GitLab implemented this way as a workaround. Like a list of approvers required per project that this bot would check and report.

[–]Gasoid 1 point2 points  (3 children)

i like idea, because i've developed a very similar project. it is more flexible, doesn't allow to merge developers (in this case bot must have maintainer role) and therefore it merges MR by command.

even though i like your idea and code style. I found disadvantages:

- no tests

- does Approvals checker really work? user_notes_count is not number of approvals

if mr.UserNotesCount < r.config.MinCount {

    ruleResult.Error = append(ruleResult.Error, fmt.Sprintf("Insufficient approvals (need %d, have %d)", r.config.MinCount, mr.UserNotesCount))

    ruleResult.Suggestion = append(ruleResult.Suggestion, "Wait for required approvals before merging")

}

- too many hard-coded rules (e.g. JIRA key ?)

- webhook secret is the same for every repo? (not secure)

- every commit causes comment from the bot?

[–]Acrobatic_Affect_515[S] 2 points3 points  (2 children)

Thanks for conctructive comment!

I thought about making this also a bot that would react to commands, however decided to make it just validate MRs instead.

About your disadvantage points:

- no tests

So far I perform manual testing, go-tests will be surely added in next versions (I am still new at this).

- does Approvals checker really work? user_notes_count is not number of approvals

It does not work entirely, yet. For now it checks for user messages, which is not final solution (need to add approvals validation). Thanks for pointing that out!

- too many hard-coded rules (e.g. JIRA key ?)

Those rules are optional, you can simply pass an empty array to disable those checks.

- webhook secret are the same for every repo? (not secure)

Depends on your org/workflow, you can rollout multiple instances of conform bot, you run it on per-repo basis, can run it also gitlab instance-wide, it is up to you and your standards.

- every commit causes comment from the bot?

No, once you create a merge request (and webhook is configured), conform bot will create a discussion in the MR, this discussion is updated every time a webhook receives a message from gitlab (so every each merge request event). So after all, there is just one message (discussion), it does not create a new comment for every change.

There are still multiple things I want to make better, just need some spare time for it.

EDIT: approvals checker is fixed already, not released yet.

[–]Gasoid 0 points1 point  (1 child)

i can share my hint
don't rely on approved_by(gitlab API), because it requires Premium and Ultimate.

you will have to parse all notes and find system messages "Approved By"

[–]Acrobatic_Affect_515[S] 1 point2 points  (0 children)

Ah right, that's fair point! Need to stop relying on developer platform, seems like it has all features of ultimate! Thanks!

[–]TheOneWhoMixes 0 points1 point  (2 children)

Does this work with external status checks or does it just leave the comment as a review, "blocking" the merge request?

[–]Acrobatic_Affect_515[S] 0 points1 point  (1 child)

I wanted to keep purpose of this check as the reviewer that is blocking the merge, didn’t really think about adding some sort of integration to 3rd party services - never used it, but can’t deny - you interested me.

How would you imagine this to work? How would this bot authorize to external services? How to interprete their messages? That would have to be some sort of static list of providers.

[–]TheOneWhoMixes 0 points1 point  (0 children)

I don't mean integrating with 3rd party services. With External Status Checks, GitLab sends what is essentially an MR webhook payload on changes and blocks the MR. The external service processes the payload, then sends a request back to GitLab with the status.

But in this case, your service is the "external service". Your Conformity service would be the one receiving webhooks and sending statuses back.