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.

Sign up for GitHub

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

Jump to bottom

fix(manager/gitlabci): strip Gitlab Dependecy Proxy variable from depName #14734

Merged
merged 29 commits into from
Apr 25, 2022
Merged

fix(manager/gitlabci): strip Gitlab Dependecy Proxy variable from depName #14734

merged 29 commits into from
Apr 25, 2022

Conversation

dsch
Copy link
Contributor

@dsch dsch commented Mar 20, 2022

Changes

Remove image prefix variable from images pulled through the Dependency Proxy.

Context

The predefined variables CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX and CI_DEPENDENCY_PROXY_DIRECT_GROUP_IMAGE_PREFIX can be used to pull images from Docker Hub through Gitlab Dependency Proxy. Since the part after the prefix is a resolvable image name, only this part use used as depName. Also the replaceString does not contain the variable, since it's not relevant for replacement.

Closes #9958

Note: This change does not solve the two additionally mentioned use-cases in #9958: CI_REGISTRY and Dependency Proxy in Dockerfiles.

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please tick one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

@dsch
strip Gitlab Dependecy Proxy variable from depName
e23f362 
Remove image prefix variable from images pulled through the Dependency Proxy.
@JamieMagee
Copy link
Contributor

@dsch Overall your change looks good. I had a go at resolving the conflicts with main, but it wasn't straightforward 😅

@rarkins rarkins marked this pull request as draft March 25, 2022 10:20
rarkins
rarkins requested changes Mar 25, 2022
View reviewed changes
Copy link
Collaborator

@rarkins rarkins left a comment

Choose a reason for hiding this comment

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

Needs deconflicting

@dsch
Copy link
Contributor Author

dsch commented Mar 25, 2022

@rarkins thank you for marking the PR as draft.
I'm kind of blocked on this: The conflict was introduced in 6558d05 which causes issue #14765 and makes the newly created test cases fail.
I'm considering two option:

  • Weakening my testcase so they don't fail
  • rewriting the gitlab-manager by using js-yaml

@dsch dsch marked this pull request as ready for review March 28, 2022 09:21
@dsch dsch changed the title fix(gitlab): strip Gitlab Dependecy Proxy variable from depName fix(manager/gitlabci): strip Gitlab Dependecy Proxy variable from depName Mar 28, 2022
JamieMagee
JamieMagee reviewed Mar 29, 2022
View reviewed changes
tsconfig.strict.json Outdated Show resolved Hide resolved
JamieMagee
JamieMagee previously approved these changes Mar 29, 2022
View reviewed changes
rarkins
rarkins previously approved these changes Mar 29, 2022
View reviewed changes
@rarkins rarkins enabled auto-merge (squash) March 29, 2022 03:51
auto-merge was automatically disabled March 29, 2022 04:35

Head branch was pushed to by a user without write access

@dsch dsch dismissed stale reviews from rarkins and JamieMagee via 0c4ac20 March 29, 2022 04:35
@viceice
Copy link
Member

viceice commented Apr 7, 2022

Your latest commit is unverified 😕

0c4ac20

@dsch dsch requested a review from viceice April 7, 2022 11:13
@rarkins rarkins requested a review from viceice April 23, 2022 04:38
@viceice
Copy link
Member

viceice commented Apr 23, 2022

https://github.com/renovatebot/renovate/blob/main/.github/contributing.md#re-requesting-a-review

viceice
viceice reviewed Apr 23, 2022
View reviewed changes
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

otherwise LGTM

@dsch dsch marked this pull request as draft April 24, 2022 07:50
@dsch
Copy link
Contributor Author

dsch commented Apr 24, 2022 •
edited

By using the full name for replacement, the prefix gets dropped in the PR.
I marked the PR as draft until this is resolved.

Tested with https://gitlab.com/dsch1/renovate-gitlab-dependency-proxy
image

@rarkins
Copy link
Collaborator

rarkins commented Apr 24, 2022

@viceice This may be hard to fix due to the way autoReplace works, unless we manually specify the replace template during extract. Shall we revert to how it was?

@viceice
Copy link
Member

viceice commented Apr 24, 2022

yes, let's revert. sorry

@dsch
Copy link
Contributor Author

dsch commented Apr 24, 2022

@viceice I implemented a prefixed version of autoReplaceTemplateString. I'm not sure if this goes in the right direction, because it still does not work as expected.
Any hint where I can learn about autoReplace?

@dsch
Copy link
Contributor Author

dsch commented Apr 24, 2022 •
edited

After deleting the branch from my previous test it works.
https://gitlab.com/dsch1/renovate-gitlab-dependency-proxy/-/merge_requests/3

@dsch dsch marked this pull request as ready for review April 24, 2022 19:15
@dsch dsch requested a review from viceice April 24, 2022 19:15
viceice
viceice reviewed Apr 24, 2022
View reviewed changes
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

i don't see failing tests yet 🙃

lib/modules/manager/gitlabci/utils.ts Outdated Show resolved Hide resolved
lib/modules/manager/gitlabci/utils.ts Outdated Show resolved Hide resolved
lib/modules/manager/gitlabci/utils.spec.ts Outdated Show resolved Hide resolved
lib/modules/manager/gitlabci/utils.spec.ts Outdated Show resolved Hide resolved
@dsch dsch requested a review from viceice April 25, 2022 04:18
@viceice viceice merged commit 3eec5b7 into renovatebot:main Apr 25, 2022
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 32.31.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@dsch
Copy link
Contributor Author

dsch commented Apr 25, 2022

@viceice thank you for supporting me on my endeavor to create my first TypeScript PR.

@dsch dsch deleted the gitlab-dependency-proxy branch April 25, 2022 07:34
@rarkins
Copy link
Collaborator

rarkins commented Apr 25, 2022

@dsch most of all, thanks to you!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rarkins rarkins rarkins left review comments

@viceice viceice viceice approved these changes

@JamieMagee JamieMagee Awaiting requested review from JamieMagee

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support Gitlab Dependency Proxy Images
5 participants
@dsch @JamieMagee @viceice @rarkins @renovate-release