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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CI_REGISTRY_IMAGE (again) #1087

Merged
merged 2 commits into from
Feb 9, 2024
Merged

Conversation

PigeonF
Copy link
Collaborator

@PigeonF PigeonF commented Feb 6, 2024

Hi, while running the master branch locally, I realized that my change in #1085 actually broke CI_REGISTRY_IMAGE. The reason is that the full string gets lowercased (i.e. something like $ci_registry/some/lowercased/path), which means $CI_REGISTRY does not get resolved (because the variable name was lowercased) 馃槄 . Thats what I get for not testing properly 馃う .

I have added some tests now to make sure the behaviour is correct now (incidentally, I checked the gitlab source code and it seems they only lowercase the path, not the host, so this code matches the upstream behaviour more closely).

While writing the test I realized that CI_PROJECT_PATH_SLUG also needs to be lowercased to match the upstream behaviour.

Sorry for the trouble 馃槗

Copy link
Owner

@firecow firecow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@firecow firecow merged commit 49261ef into firecow:master Feb 9, 2024
9 checks passed
@PigeonF PigeonF deleted the fix-ci-registry-image branch May 22, 2024 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants