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

Project Audit Events are full of "Added protected branch" #178

Closed
gdubicki opened this issue Feb 17, 2021 · 13 comments · Fixed by #185 or #294
Closed

Project Audit Events are full of "Added protected branch" #178

gdubicki opened this issue Feb 17, 2021 · 13 comments · Fixed by #185 or #294

Comments

@gdubicki
Copy link
Member

Probably because we unprotect and then protect branches again to perform some operations.

@nickshulba
Copy link

Hello Greg
Could you provide more details why Gitlabforms needs to do this protect/unprotect action against branch?
It caused my pipeline to fail turning off branch protection exactly when deploy started.
If this option is configurable, please give a hint. It would help me a lot if I could control this activity (or disable completely)

@sitnik
Copy link

sitnik commented Mar 7, 2021

This issue may actually break pipelines if they depend on protected variables and run at the same moment when protection was removed and not yet put back.
Why Renovate removes protection if nothing was changed in its and in repository configuration?

@gdubicki
Copy link
Member Author

gdubicki commented Mar 8, 2021

We do it to change the file values if you use the files configuration, for example. See https://github.com/egnyte/gitlabform/blob/main/gitlabform/gitlabform/processors/project/files_processor.py#L54-L59.

@YuraBeznos
Copy link
Contributor

We do it to change the file values if you use the files configuration, for example

Another place is in the branch processor: https://github.com/egnyte/gitlabform/blob/main/gitlabform/gitlabform/processors/project/branches_processor.py#L13

@YuraBeznos
Copy link
Contributor

Interesting place: https://github.com/egnyte/gitlabform/blob/9908247e66b5c6cac47d1c147dbb9235e8aebd7a/gitlabform/gitlabform/processors/util/branch_protector.py#L37

Is it really required to unprotect first to change those values?

@gdubicki
Copy link
Member Author

Is it really required to unprotect first to change those values?

It was when I wrote that code.. Or I am not aware of some GitLab API feature? Something sudo-like? But was always using an admin token, so it should not be necessary..

@YuraBeznos
Copy link
Contributor

Is it really required to unprotect first to change those values?

It was when I wrote that code.. Or I am not aware of some GitLab API feature? Something sudo-like? But was always using an admin token, so it should not be necessary..

Thanks! It looks like still required, but in most cases those settings are already set so this part in the code means lets rewrite the same thing which already set.

How about to check if settings are different first? I've tried to implement it via above MR but tests still require to be fixed.

@amimas
Copy link
Collaborator

amimas commented Apr 26, 2021

How about to check if settings are different first?

It'd be great if we could do that. Otherwise I find the audit events are becoming too noisy; not to mention the potential of CI breaking due to protected variables not being available when this is happening.

gdubicki pushed a commit that referenced this issue May 13, 2021
Skip branch unprotect/protect if the same values are already set

This will fix #178
gdubicki pushed a commit that referenced this issue May 13, 2021
Part of #178

Patch author: Robin Moss
gdubicki pushed a commit that referenced this issue May 13, 2021
gdubicki pushed a commit that referenced this issue May 13, 2021
@YuraBeznos
Copy link
Contributor

As I can see this one is fixed in latest releases of 2.x .

@gdubicki
Copy link
Member Author

gdubicki commented Jun 6, 2021

Yes, it should be completely fixed in v2.0.2+. Please reopen if anyone is still seeing this issue after GitLabForm upgrade.

@gdubicki
Copy link
Member Author

I am seeing this. :P

@robin-moss
Copy link

So am I, but not sure when this started happening again 🤔

@gdubicki
Copy link
Member Author

This should finally be fixed in the next release, with the above PR, to be released in a day or two.

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