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

Enforce exact matching for GitLab groups #4473

Merged
merged 1 commit into from
Nov 28, 2024
Merged

Conversation

pat-s
Copy link
Contributor

@pat-s pat-s commented Nov 28, 2024

fix #2922

Until now, GL groups were matched partially. This caused an issue in the following case:

  1. In a new instance with only subgroup repos enabled
  2. Enable a non-subgroup repo
  3. Partial match for the first owner/subgroup of the existing orgs
  4. Attempt to (re)create owner/subgroup although it already exists -> error

By enforcing an exact match, the query returns nil for a non-existing top-level group and will attempt to (correctly) create a new WP org for it, letting the overall repo add operation succeed.

Fix has already been tested to work.

I don't think there should be any potential downsides when switching to exact matching.

Besides, I also updated the error message wording I encountered.

Should be backported.

@pat-s pat-s added bug Something isn't working forge/gitlab gitlab forge related labels Nov 28, 2024
@woodpecker-bot
Copy link
Collaborator

woodpecker-bot commented Nov 28, 2024

Deployment of preview was torn down

@6543
Copy link
Member

6543 commented Nov 28, 2024

that should be backported, wana do it or should I cherry pick?

@6543 6543 enabled auto-merge (squash) November 28, 2024 14:30
@6543 6543 merged commit 6327dcd into main Nov 28, 2024
7 checks passed
@6543 6543 deleted the fix/gitlab-partial-group-match branch November 28, 2024 14:32
Copy link

codecov bot commented Nov 28, 2024

Codecov Report

Attention: Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.

Project coverage is 28.22%. Comparing base (896e550) to head (50717a3).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
server/forge/gitlab/gitlab.go 0.00% 9 Missing ⚠️
server/api/repo.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4473      +/-   ##
==========================================
+ Coverage   28.20%   28.22%   +0.01%     
==========================================
  Files         385      385              
  Lines       27847    27852       +5     
==========================================
+ Hits         7855     7860       +5     
  Misses      19301    19301              
  Partials      691      691              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@woodpecker-bot woodpecker-bot mentioned this pull request Nov 28, 2024
1 task
6543 pushed a commit to 6543-forks/woodpecker that referenced this pull request Nov 28, 2024
@6543
Copy link
Member

6543 commented Nov 28, 2024

Backport: #4474

@6543 6543 added the backport-done indicates that this pull has been backported label Nov 28, 2024
6543 added a commit that referenced this pull request Nov 28, 2024
Co-authored-by: Patrick Schratz <patrick.schratz@gmail.com>
@woodpecker-bot woodpecker-bot mentioned this pull request Nov 28, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-done indicates that this pull has been backported bug Something isn't working forge/gitlab gitlab forge related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enabling repository results in: pq: duplicate key value violates unique constraint "UQE_orgs_name"
3 participants