Prevent "Accept Merge Request" if a pipeline hasn't been run yet.
Summary
If a pipeline is set to run manually (there is no automatic steps), it is possible to accept a merge request when the "Only allow merge requests to be merged if the pipeline succeeds" setting is enabled.
Steps to reproduce
- Create a one-step CI pipeline where it has to be triggered manually.
- Make sure "Only allow merge requests to be merged if the pipeline succeeds" is enabled.
- Create a merge request.
- Note that you can accept the merge request without ever having run the pipeline.
What is the current bug behavior?
Merge requests can be accepted with no pipeline ever being run when you have "Only allow merge requests to be merged if the pipeline succeeds" enabled.
What is the expected correct behavior?
If I have enabled "Only allow merge requests to be merged if the pipeline succeeds", then I would expect the Accept Merge Request button to be disabled until such time that a pipeline has been successfully run, whether its automatically run or manually run.
Results of GitLab environment info
Expand for output related to GitLab environment info
System information System: Ubuntu 16.04 Current User: git Using RVM: no Ruby Version: 2.3.3p222 Gem Version: 2.6.6 Bundler Version:1.13.7 Rake Version: 10.5.0 Redis Version: 3.2.5 Git Version: 2.11.1 Sidekiq Version:5.0.0GitLab information Version: 9.2.2 Revision: cbde95c Directory: /opt/gitlab/embedded/service/gitlab-rails DB Adapter: postgresql URL: http://192.168.0.200 HTTP Clone URL: http://192.168.0.200/some-group/some-project.git SSH Clone URL: git@192.168.0.200:some-group/some-project.git Using LDAP: no Using Omniauth: no
GitLab Shell Version: 5.0.4 Repository storage paths:
- default: /var/opt/gitlab/git-data/repositories Hooks: /opt/gitlab/embedded/service/gitlab-shell/hooks Git: /opt/gitlab/embedded/bin/git
Results of GitLab application Check
Expand for output related to the GitLab application check
Checking GitLab Shell ...GitLab Shell version >= 5.0.4 ? ... OK (5.0.4) Repo base directory exists? default... yes Repo storage directories are symlinks? default... no Repo paths owned by git:root, or git:git? default... yes Repo paths access is drwxrws---? default... yes hooks directories in repos are links: ... 2/1 ... ok 1/2 ... ok 3/3 ... ok Running /opt/gitlab/embedded/service/gitlab-shell/bin/check Check GitLab API access: OK Access to /var/opt/gitlab/.ssh/authorized_keys: OK Send ping to redis server: OK gitlab-shell self-check successful
Checking GitLab Shell ... Finished
Checking Sidekiq ...
Running? ... yes Number of Sidekiq processes ... 1
Checking Sidekiq ... Finished
Checking Reply by email ...
Reply by email is disabled in config/gitlab.yml
Checking Reply by email ... Finished
Checking LDAP ...
LDAP is disabled in config/gitlab.yml
Checking LDAP ... Finished
Checking GitLab ...
Git configured with autocrlf=input? ... yes Database config exists? ... yes All migrations up? ... yes Database contains orphaned GroupMembers? ... no GitLab config exists? ... yes GitLab config outdated? ... no Log directory writable? ... yes Tmp directory writable? ... yes Uploads directory setup correctly? ... yes Init script exists? ... skipped (omnibus-gitlab has no init script) Init script up-to-date? ... skipped (omnibus-gitlab has no init script) projects have namespace: ... 2/1 ... yes 1/2 ... yes 3/3 ... yes Redis version >= 2.8.0? ... yes Ruby version >= 2.1.0 ? ... yes (2.3.3) Your git bin path is "/opt/gitlab/embedded/bin/git" Git version >= 2.7.3 ? ... yes (2.11.1) Active users: 3
Checking GitLab ... Finished
Activity
/cc @grzesiek
Just wanted to add that this is in regards to a customer ticket... https://gitlab.zendesk.com/agent/tickets/78717
added customer label
-
/cc @smcgivern
@grzesiek what's the expected status here? I tried to reproduce with:
test: script: echo passed when: manualAnd the pipeline shows as skipped; is that expected? Is that the same as the example in the description?
We do seem to deliberately ignore skipped pipelines for this setting; probably because of the
[ci skip]option in commit messages.
@smcgivern Adding
allow_failure: falsewill make this a blocking manual action, I believe that this is the case.-
@smcgivern Interesting! Thanks for trying it! Maybe this behavior has changed after redesigning MR widget. It appears to be correct now. Should we close this issue in that case?
We are experiencing this behavior on v9.2.5 as well. We have the yml to only run CI when triggered or a merge to master has been done.
Attached the yml, couldn't get it to format correctly in pageplayground_yml.txt
Edited by username-removed-658126@smcgivern Robert is the original reporting customer. Sounds like its still there in the latest release.
Let me know if there's anything I can do to help.
We're getting a different message on the merge request.
If you click the 'Merge' button, the change will still be allowed to merge into master
Edited by username-removed-658126-
I get the same merge request widget state that @lasirona does.
The reason it looks like this is that we do not create a pipeline when it has no jobs for given preconditions.
When you use
only/exceptand effecitvely no job needs to be done, we don't create pipeline. And because there is no pipeline for the merge request, we allow to merge it.@lasirona @smcgivern does it make sense? Is there something we could improve here?
Edited by Grzegorz Bizon IMHO, if the feature is selected that 'Only allow merge requests to be merged if the pipeline succeeds', that the behavior should default to blocking the merge. Otherwise, IMHO the text for this checkbox is deceiving. Regardless, we would like it blocked so either approach is acceptable. Default to blocking when this checkbox is selected. Or, add as a feature with another checkbox as a sub-feature.
@lasirona that makes sense, but I think how this feature works without a pipeline in general is not very well-defined. For instance, if you broke your CI settings so that you weren't creating pipelines, and wanted to create an MR to fix that, you wouldn't want merging that MR to be blocked ... but I don't know how common that is.
@grzesiek is there a way to work around this? For instance, if you set
when: alwayson another job, does that implicitly create the prior stage, or does it just have the job withwhen: always?-
@smcgivern There is only one workaround for that; always have a least one job in the pipeline for every branch /tag.
whenis not going to help with that if the job that defineswhenis excluded for given branch / tag. @grzesiek right, but in that case, if the defined job passes, the MR is still mergeable, and if it doesn't pass, then the MR will never be mergeable
-
@smcgivern Yes, the problem is that pipeline has no jobs. @lasirona do we correctly understood your problem? If yes, then there is no good solution, until you add jobs to the pipeline.
@lasirona that makes sense, but I think how this feature works without a pipeline in general is not very well-defined. For instance, if you broke your CI settings so that you weren't creating pipelines, and wanted to create an MR to fix that, you wouldn't want merging that MR to be blocked ... but I don't know how common that is.
We've broken it many times because the controls have to be on every stage. There's always someone who starts a change and forgets one stage (me included). The CI runs based on the current .gitlab-ci.yml, so however that is set. The CI runs according to those settings, so if we have it off and decide to turn it on, it'll run as expected, not based on the previous CI settings.
Besides, if something is completely broken. There's always being able to force push, it just requires the extra steps of unprotecting the destination branch.
Based on the direction of the discussion, I feel like I'm missing something?
-
@lasirona I think that you suggest that if Only allow merge requests to be merged if the pipeline succeeds feature is enabled, we should require not only pipeline to pass but also that a pipeline should exist. Is that correct?
@grzesiek I think the problem lies more with manual pipelines. If a manual pipeline exists, but hasn't been run, then the merge can be accepted. This is counter-intuitive behaviour to the "Only allow merge requests to be merged if the pipeline succeeds". The pipeline hasn't succeeded because it hasn't been run yet. With that setting enabled, there should be two conditions...
- Has a pipeline been run?
- Has the pipeline succeeded?
If both those conditions are yes, then the merge can be accepted. If not, then no merge is allowed.
@lasirona I think that you suggest that if Only allow merge requests to be merged if the pipeline succeeds feature is enabled, we should require not only pipeline to pass but also that a pipeline should exist. Is that correct?
Yes, my assumption is such when applying a setting for merge requests to only be allowed to be merged if there is a successful pipeline implies a pipeline result is there to validate against.
@jwoods06 I think maybe a build job should have more than just a pass/fail state. Maybe build jobs should also have a manual state, where this job isn't scored.
-
@jwoods06 As you can see on a screenshot provided by Sean, merge button is disabled when a pipeline has blocking manual actions. We can also have optional manual actions, but this is not obvious for me that we should block pipeline when the job is optional, and is the only one in the pipeline. In that case pipeline is considered skipped. Is the problem you refer to related to skipped pipelines like when there is only one optional manual action present?
@grzesiek Yes, that's exactly it... if there is a single manual step required, I think that considering it "skipped" is dangerous. It hasn't been skipped, it just hasn't been run yet. Its a big difference (at least, I think so). Maybe its a manual step because there needs to be a discussion first or that kind of thing. There are probably lots of reasons why the step is manual and you need it to be run before you can accept the MR and you want to enforce that. Right now, the feature is called "only allow merge requests to be merged if the pipeline succeeds". Even if we accept the "skipped" terminology, that is not success. Until that pipeline is run and is successful, the MR should not be able to merge, IMO.
@jwoods06 I missed the disabled button, in my browser, the button looks very similar. I'm just assuming that in Sean's screenshot the button is dimmed. As in the yml example I provided, no manual steps in pipeline.
@grzesiek I would have to agree with you. Lumping skipped builds and unrun builds into a single category of accepting them would break other projects for us. We do have some projects that have manual jobs, but no pipelines are run until triggered. In this case, we have some projects that receive dependency libraries from other groups. The manual job allows us to test against those libraries before merging them into mainstream CI. Some are from test docker containers, while others move SHA's in git submodules.
mentioned in issue #34136 (closed)
-
@jwoods06 @lasirona Is there a reason why this important manual step can't be a blocking manual action? Do we agree that an optional manual actions should not block the pipeline or merge? Docs https://docs.gitlab.com/ee/ci/yaml/#manual-actions.
Edited by Grzegorz Bizon @grzesiek as I mentioned before, currently we don't have a manual job in the yaml. I know that in the future we will add it and yes I understand it can't block. What I don't understand is why a merge request with no pipelines run against it is allowed to merge, especially when the CI is configured to only allow successful pipelines to merge. I'm just trying to understand why 'no pipeline' == 'successful pipeline'? Because, that's the behavior exhibited and that doesn't agree with the setting.
-
@lasirona Thanks for the explanation. I agree that we have some room for improvement here. Making it possible to configure your project the way that CI/CD pipeline is required make some sense. We would need to think about edge cases, but this seems feasible. What do you think @smcgivern?
@grzesiek would we need a configuration setting, or could we just make this one block merges without a pipeline? In https://gitlab.com/gitlab-org/gitlab-ce/issues/33663#note_32981309 I proposed one problem for this, but I don't know how common that is. I think introducing a new setting would be too much overhead.
-
@smcgivern https://gitlab.com/gitlab-org/gitlab-ce/issues/33663#note_32981309 shouldn't be a problem, pipelines are being created for code on a merge request branch, so it should be possible to push a code there that fixes CI/CD pipeline, thus making it possible to merge it. Am I missing something? We can probably use the same setting to make is possible.
Anyway, let's pass this issue to our product team. /cc @bikebilly
mentioned in issue #26927 (moved)
This is triggering a problem with creating and setting automatic merge for merge requests via the API. I have a trigger that creates a merge request and then immediately attempts to set
merge_when_pipeline_succeedsvia the v4 API. This immediately merges the MR, presumably because the CI job hasn't been created yet. See my comment on #26927 for more information.-
@bikebilly Do you have any idea how could we improve it?
@grzesiek @bikebilly I don't know if this will break horribly for some people, but I'm willing to try just making MRs without a pipeline unable to merge if this setting is enabled. If you don't have a pipeline on every MR, but want this setting set ... I'm not sure I understand that use-case
-
@smcgivern I think that would solve my use case, presumably it should still be possible to set
merge_when_pipeline_succeedseven if a pipeline hasn't technically been created yet? @jbrandhorst I don't think so, sadly. The problem is that we don't know if a pipeline will ever be created for the merge request until it is. @grzesiek any ideas?
-
From a technical point of view, I think that we should know if a merge request is expected to have a pipeline or not, just at the moment that push happens. This would solve a lot of problems. And it should be possible to easily implement that.
Edited by Grzegorz Bizon @bikebilly how would the waiting time work exactly? I'm not sure I'm clear on that.
@bikebilly ah, I see. I don't think that's guaranteed to be correct, either: if someone has disabled Sidekiq for some reason, or just has a large backlog of jobs to be processed, we may make an MR mergeable that isn't.
I think if we're not going for a comprehensive solution here (https://gitlab.com/gitlab-org/gitlab-ce/issues/33663#note_33750822), then we should pick the absolute simplest option.
Seems like a duplicate of #28682 (moved) ?
mentioned in issue #34547 (closed)
-
I think that in this case we should seek a good solution. The only missing piece of the puzzle is - how could we know if someone will post a commit status in the merge request. Maybe we should have a checkbox like 'Require a pipeline to be present in merge request' (no matter if pipeline is internal or external).
Edited by Grzegorz Bizon Ah, I see
added SP2 label
@bikebilly so the plan here is to just do https://gitlab.com/gitlab-org/gitlab-ce/issues/33663#note_33580391? Could you update the description, please?
@bikebilly I'm sorry, I really don't like the idea of a fixed delay for something like this. I think that is more confusing, even if it will break fewer workflows. The scenario is: you visit an MR without a pipeline, and you can't merge it. You visit it N minutes later, and you can merge it, but nothing has changed. It's also a lot of code to write for a temporary fix
Maybe I'm overestimating the confusion here. @grzesiek wdyt?
-
@smcgivern @bikebilly I think that a fixed delay does not resolve the problem, it makes it even more tricky. You can post an external status to a merge request an hour after you submitted the MR. I think that we should have a setting -> every merge requests needs a pipeline. This might resolve more problems than just this one. I'm not sure if we should use existing setting or introduce a new one. What do you think @bikebilly?
Edited by Grzegorz Bizon -
@bikebilly Manual merging is always possible if a user can push merge commits. I agree this is a very tricky and that we should solve it the right way. We can handle internal pipelines pretty well, but the problem is mostly related to external "pipelines". Maybe we should have two configuration options: require internal pipeline and require external commit status?
-
@bikebilly I think that you are correct
@bikebilly OK.
@bikebilly this isn't marked as Deliverable, so my team will get to it once they've handled their Deliverables.
changed milestone to %Next 2-3 months
@smcgivern : Changed the milestone
moved to gitlab#10420
Hey!
GitLab is moving all development for both GitLab Community Edition and Enterprise Edition into a single codebase. The current
gitlab-cerepository will become a read-only mirror, without any proprietary code. All development is moved to the currentgitlab-eerepository, which we will rename to justgitlabin the coming weeks. As part of this migration, issues will be moved to the currentgitlab-eeproject.If you have any questions about all of this, please ask them in our dedicated FAQ issue.
https://gitlab.com/gitlab-org/gitlab-ee/issues/13855You can also find more information here:
https://gitlab.com/gitlab-org/gitlab-ee/issues/13304- https://about.gitlab.com/2019/08/23/a-single-codebase-for-gitlab-community-and-enterprise-edition/
Frequently Asked Questions
For an up to date list of questions and answers, please take a look at
https://gitlab.com/gitlab-org/gitlab-ee/issues/13855What will we do with the repository names?
https://gitlab.com/gitlab-org/gitlab-ee/ will become https://gitlab.com/gitlab-org/gitlab, and https://gitlab.com/gitlab-org/gitlab-ce will become https://gitlab.com/gitlab-org/gitlab-foss.
Why rename gitlab-ce to gitlab-foss?
Using "gitlab" and "gitlab-ce" would be confusing, so we decided to rename gitlab-ce to gitlab-foss to make the purpose of this FOSS repository more clear
I created a merge requests for CE, and this got closed. What do I need to do?
You will need to create a merge request at https://gitlab.com/gitlab-org/gitlab-ee/. This link will eventually redirect to https://gitlab.com/gitlab-org/gitlab.
How does the licensing work in this new setup?
Everything in the
ee/directory is proprietary. Everything else is free and open source software. If your merge request does not change anything in theee/directory, the process of contributing changes is the same as when using the gitlab-ce repository.Will you accept merge requests on the gitlab-ce/gitlab-foss project after it has been renamed?
No. Merge requests submitted to this project will be closed automatically.
Will I still be able to view old issues and merge requests in gitlab-ce/gitlab-foss?
Yes.
How will this affect users of GitLab CE using Omnibus?
No changes will be necessary, as the packages built remain the same.
How will this affect users of GitLab CE that build from source?
Once the project has been renamed, you will need to change your Git remotes to use this new URL. GitLab will take care of redirecting Git operations so there is no hard deadline, but we recommend doing this as soon as the projects have been renamed.
Where can I see a timeline of the remaining steps?
https://gitlab.com/gitlab-org/gitlab-ee/issues/13304Will community contributions submitted to the new "gitlab" repository be available in the new gitlab-foss repository?
Yes, these changes will be synced to gitlab-foss automatically, several times per day.
