Fix Error 500 when creating a merge request that contains an image that was deleted and added
_Originally opened at !4816 by @stanhu._
- - -
## What does this MR do?
This MR fixes an Error 500 when creating a merge request that contains an image that was deleted and added. Before, when displaying the before and after image, the code would always retrieve the image from the parent commit. However, in a diff, this could cause two different problems:
The "before" image may not actually be the image you want to compare against (regression of #14327)
It may appear as though a file was modified when it was really just added during the diff
## Are there points in the code the reviewer needs to double check?
There may be a more elegant to fix this bug.
## What are the relevant issue numbers?
Closes#3893, gitlab-org/gitlab-ee#678
See merge request !7457
Omniauth auto link LDAP user falls back to find by DN when user cannot be found by uid
Unfortunately, SAML IDs can be an LDAP UID, DN, or something else entirely. UID and DN are most common, though. This adds a fallback scenario so we first try to find a matching LDAP user by UID, then by DN. This will fix a problem for the customer in https://gitlab.zendesk.com/agent/tickets/43298
See merge request !7002
Steps to reproduce:
1. Start with a repo with an image
2. Add a commit to delete the image
3. Add another commit to replace the image with another image
In a diff comparison, we really just compare about what the image was before the diff, not
the direct parent of the last commit. This MR fixes that.
Closes#3893, gitlab-org/gitlab-ee#678
Signed-off-by: Rémy Coutable <remy@rymai.me>
Improve naming convention in ci configuration module
## What does this MR do?
This MR improves the naming convention in CI configuration module to reflect the domain design better.
## What are the relevant issue numbers?
Related to #15060
See merge request !7448
Centralize all LDAP config logic in `GitLab::LDAP::Config`. Previously,
some logic was in the Devise initializer and it was not honoring the
`user_filter`. If a user outside the configured `user_filter` signed
in, an account would be created but they would then be denied access.
Now that logic is centralized, the filter is honored and users outside
the filter are never created.
Respect project visibility settings in the contributions calendar
This MR fixes a number of bugs relating to access controls and date selection of events for the contributions calendar
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/23403
See merge request !2019
Signed-off-by: Rémy Coutable <remy@rymai.me>
disable markdown in comments when referencing disabled features
fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/23548
This MR prevents the following references when tool is disabled:
- issues
- snippets
- commits - when repo is disabled
- commit range - when repo is disabled
- milestones
This MR does not prevent references to repository files, since they are just markdown links and don't leak
information.
See merge request !2011
Signed-off-by: Rémy Coutable <remy@rymai.me>
It was previously possible for invalid credential errors to go unnoticed
in this task. Users would believe everything was configured correctly and
then sign in would fail with 'invalid credentials'. This adds a specific
bind check, plus catches errors connecting to the server. Also, specs :)
Allow owners to fetch source code in CI builds
Due to different way of handling owners of a project, they were not allowed to fetch CI sources for project.
This adds a separate code path for handling owners, that are not admins.
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/23437
See merge request !6943
Use optimistic locking
## What does this MR do?
Removes the usage of pessimistic locking in favor of optimistic which is way cheaper and doesn't block database operation.
Since this is very simple change it should be safe. If we receive `StaleObjectError` message we will reload object a retry operations in lock.
However, I still believe that we need this one: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7005 as this will reduce a load on Database and FS.
This changes a behavior from:
### Pesimistic locking (previous behavior)
#### For updating
1. SELECT * FOR UPDATE (other updates wait on this)
2. we update ci_pipeline
3. latest_build_status
4. enqueue: (use: transition :created -> :pending)
5. [state_machine] we are in state created, we can go to pending
6. [state_machine] ci_pipeline.status = created
7. [state_machine] ci_pipeline.save
8. [state_machine] after_transition: (if for success): PipelineSuccessWorker on Sidekiq
9. release DB lock
#### If no update is required
1. SELECT * FOR UPDATE (other updates wait on this)
2. we update ci_pipeline
3. latest_build_status
4. we are in pending, we can't transition to pending, because it's forbidden
5. release DB lock
### Optimistic locking (implemented by this MR)
#### For updating
1. latest_build_status
2. enqueue: (use `transition :created -> :pending`)
3. [state_machine] we are in state created, we can go to pending
4. [state_machine] ci_pipeline.status = created
5. [state_machine] ci_pipeline.save
6. [state_machine] [save] where(lock_version: ci_pipeline.lock_version).update_all(status: :created, updated_at: Time.now)
7. [state_machine] [save] unless we_updated_row then raise ObjectInconsistentError
#### If no update is required
1. we update ci_pipeline
2. latest_build_status
3. we are in pending, we can't transition to pending, because it's forbidden
## Why was this MR needed?
We have been seeing a number of problems when we migrated Pipeline/Build processing to Sidekiq. Especially we started seeing a lot of blocking queries.
We used a pessimistic locking which doesn't seem to be required. This effectively allows us to fix our issues with blocked queries by using more efficient method of operation.
## What are the relevant issue numbers?
Issues: https://gitlab.com/gitlab-com/infrastructure/issues/623 and https://gitlab.com/gitlab-com/infrastructure/issues/584, but also there's a bunch of Merge Requests that try to improve behavior of scheduled jobs.
cc @pcarranza @yorickpeterse @stanhu
See merge request !7040
This changes ProjectCacheWorker.perform_async so it only schedules a job
when no lease for the given project is present. This ensures we don't
end up scheduling hundreds of jobs when they won't be executed anyway.
Fixed all related specs and also changed the logic to handle edge cases. This includes exporting and exporting of group labels, which will get associated with the new group (if any) or they will become normal project labels otherwise.
Found other issues to do with not being able to import all labels at once in the beginning of the JSON - code was much simpler when we import all labels and milestones associated to a project first, then the associations will find the already created labels instead of creating them from the associations themselves.
Z.J. van de Weg
Kamil Trzcinski
Rémy Coutable
Valery Sizov
Douwe Maan
Stan Hu
Grzegorz Bizon
Drew Blessing
Patricio Cano
Sean McGivern
Robert Speicher
tiagonbotelho
Douwe Maan
Jacob Vosmaer
Felipe Artur
Ahmad Sherif
Gabriel Mazetto
Yorick Peterse
Lin Jen-Shin
James Lopez
Douglas Barbosa Alexandre