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 |
---|---|
![]() |
![]() |
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
Merge request reports
Activity
Thank you for your contributions to GitLab. We believe that everyone can contribute and contributions like yours are what make GitLab great!
Our Merge Request coaches will ensure your contribution is reviewed in a timely manner.
To bring your merge request to the attention of the relevant team within GitLab, you can ask our bot to label it with a group label. For example, if your merge request changes a project management feature, it can be labelled by commenting
@gitlab-bot label ~"group::project management"
. To find the most relevant group for your change, you can look up the group based on the most relevant product category in the product categories table. Once you have found the group name, type@gitlab-bot label ~group::
, then start to type the group name and select the applicable group label, then submit the comment and the bot will apply the label for you.If after a few days, there's no message from a GitLab team member, feel free to ping
@gitlab-org/coaches
or ask for help by commenting@gitlab-bot help
.These resources may help you to move your Merge Request to the next steps:
This message was generated automatically. You're welcome to improve it.
added Community contribution label
marked the checklist item Changelog entry as completed
added 1 commit
- f193b40c - ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups
added 1 commit
- d143782d - ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups
- Resolved by Thong Kuah
I've done everything I can now :)
I would appreciate help locating the CI + dependency proxy tests, as I haven't been able to. Perhaps @sabrams can help?
I would also like to know whether adding a note in
doc/user/packages/dependency_proxy/index.md
was the right thing here. Perhaps @sselhorn can help?
Hi @ngaskill, please review this documentation Merge Request.
added documentation label
added 1 commit
- 2111a34b - ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups
Hi @ngaskill, please review this documentation Merge Request.
added 1 commit
- 47463a57 - ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups
Hi @ngaskill, please review this documentation Merge Request.
added Category:Dependency Proxy typefeature labels
changed milestone to %13.10
added Technical Writing label
added docsfeature label
added twfinished label
assigned to @sabrams
added devopspackage grouppackage registry sectionops typemaintenance labels
assigned to @nmezzopera
- Resolved by Eric Engestrom
- Resolved by Thong Kuah
- Resolved by Eric Engestrom
- Resolved by Eric Engestrom
Thanks @1ace for finding and contributing this fix
I have a few suggestions above
assigned to @1ace and unassigned @tkuah and @nmezzopera
- 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 (seenameComponentRegexp
inmain/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:
- 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. - 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
- check if we can strip these offending chars and still have unique group names on GitLab.com, and if so tighten
added 1 commit
- 8aef2d02 - ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups
added 1 commit
- 8aef2d02 - ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups
added 1 commit
- 10e94002 - ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups
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
-
10e94002...6af8d67c - 588 commits from branch
mentioned in merge request !54627 (merged)
mentioned in issue #322521 (closed)
added 1 commit
- 5b709bae - ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups
added 1 commit
- cd0d6888 - ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups
added 1 commit
- 2b49047b - ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups
marked this merge request as draft from 1ace/gitlab@2291f529
added 1 commit
- 7211a0ee - ci: fix $CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX for non-lowercase groups
- Resolved by Eric Engestrom
- Resolved by Eric Engestrom