Skip to content
Snippets Groups Projects

ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups

What does this MR do?

$CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX is currently broken for groups containing uppercase letters, as docker does not allow uppercase characters in the image path, but GitLab allows them in the group name, which is used in the dependency proxy's image prefix.

The proxy already works with lowercased group names, the only thing needed is to make sure we pass the correct prefix to the CI and to the web page user.

Screenshots (strongly suggested)

Taking my current employer as an example, our group name is Datapred with an uppercase initial.

before after
image image

Notice the change in the URL shown. The same change is also applied to the $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX CI variable.

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

I haven't done this in this MR (yet), but it would probably be good to add tests using group names containing all the characters allowed by GitLab, to make sure that those are all handled correctly by Docker.

Security

N/A

TODO

  • add tests showing this bug, and that this MR fixes it
  • maybe: mention in the doc that this variable cannot be used in groups containing uppercase letters until GitLab 13.10

Closes #296171 (closed)

Labels: ~"group::package" devopspackage Category:Dependency Proxy ~bug severity3 (as defined by @ddavison in the bug linked above)

/cc PM @trizzi

Edited by Thong Kuah

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Thong Kuah
  • Thong Kuah
  • Thong Kuah
  • Thanks @1ace for finding and contributing this fix :green_heart:

    I have a few suggestions above

  • Thong Kuah assigned to @1ace and unassigned @tkuah and @nmezzopera

    assigned to @1ace and unassigned @tkuah and @nmezzopera

  • Nicolò Maria Mezzopera approved this merge request

    approved this merge request

    • Author Contributor
      Resolved by Thong Kuah

      @tkuah pointed to me that the namespace (which includes the group name) is bound on GitLab's side by NAMESPACE_FORMAT_REGEX, which is quite different from the restrictions on Docker's side (see nameComponentRegexp in main/reference/regexp.go).

      Aside from uppercase characters only being allowed on GitLab's side, GitLab's namespace can start with _ or ., which Docker doesn't allow either, and it can contain two or more occurrences of these within the namespace, which Docker also doesn't allow.

      This seems like a much bigger issue to fix than just downcasing the namespace, so I suggest we merge this MR (once I've added the other test) and start thinking about how to address this new issue.

      I expect @trizzi will want to manage this decision, but my first thought would be to:

      1. check if we can strip these offending chars and still have unique group names on GitLab.com, and if so tighten NAMESPACE_FORMAT_REGEX (and maybe add a DB check) so that it stays that way, and add the stripping in the same two places I modified here.
      2. figure out a migration for self-hosted instances, in case they do have conflicting group names.

      This is not ideal as this might cause issues during the following upgrade for self-hosted instances, but it's all I got for now :shrug:

  • Eric Engestrom added 1 commit

    added 1 commit

    • 8aef2d02 - ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups

    Compare with previous version

  • Eric Engestrom added 1 commit

    added 1 commit

    • 8aef2d02 - ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups

    Compare with previous version

  • Eric Engestrom added 1 commit

    added 1 commit

    • 10e94002 - ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups

    Compare with previous version

  • Author Contributor

    (rebased to try and get the CI to run)

  • Eric Engestrom added 589 commits

    added 589 commits

    • 10e94002...6af8d67c - 588 commits from branch gitlab-org:master
    • 51d69536 - ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups

    Compare with previous version

  • Tim Rizzi mentioned in merge request !54627 (merged)

    mentioned in merge request !54627 (merged)

  • mentioned in issue #322521 (closed)

  • Eric Engestrom added 1 commit

    added 1 commit

    • 5b709bae - ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups

    Compare with previous version

  • Eric Engestrom added 1 commit

    added 1 commit

    • cd0d6888 - ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups

    Compare with previous version

  • Eric Engestrom added 1 commit

    added 1 commit

    • 2b49047b - ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups

    Compare with previous version

  • Eric Engestrom added 2 commits

    added 2 commits

    • 25117a3b - ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups
    • 2291f529 - fixup! ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups

    Compare with previous version

  • Eric Engestrom marked this merge request as draft from 1ace/gitlab@2291f529

    marked this merge request as draft from 1ace/gitlab@2291f529

  • Eric Engestrom added 1 commit

    added 1 commit

    • 7211a0ee - ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups

    Compare with previous version

  • Thong Kuah
  • Thong Kuah
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading