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(datasource/docker): Artifactory next link is broken for tags api #20745

Merged
merged 7 commits into from Mar 5, 2023
Merged

fix(datasource/docker): Artifactory next link is broken for tags api #20745

merged 7 commits into from Mar 5, 2023

Conversation

chezsmithy
Copy link
Contributor

@chezsmithy chezsmithy commented Mar 4, 2023 •
edited

Changes

  • Updated getDockerApiTags to include branch logic for JFrog Artifactory when constructing the next page call to get tags when the number of docker tags is over 10,000.
  • Updated the tests for Frog Artifactory to trigger this specific branch to meet code coverage requirements
  • Validated the approach will not break if/when JFrog resolves the issue.

Context

  • Docker Images such as openjdk have now exceeded 10,000 tags.
  • Currently when pulling the tags through JFrog tags API when using path based virtual docker repositories such as https://jrog.io/docker/openjdk the path "docker" is lost on the link portion of the response to the API calls. As such subsequent calls for the tags beyond the first 10,000 fails with a 404 causing the entire image pull to fail.
  • This is documented in JFrog deferred issue RTFACT-18971 and noted in code comments. JFrog has not committed a date to resolve this issue.
  • Given openjdk is a common image targeted for image replacements, when attempting to replace the image against a JForg virtual docker repository containing docker hub the lookup fails causing all replacements to fail.

This has been tested against repo: https://github.com/chezsmithy/openjdk-replacement

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 select 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

@chezsmithy chezsmithy changed the title fix(docker-api-tags)-artifactory-next-link-broken fix(docker-api-tags): Artifactory next link is broken for tags api Mar 4, 2023
rarkins
rarkins requested changes Mar 4, 2023
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.

How exactly will this "fall back" to normal behavior once it's fixed?

lib/modules/datasource/docker/common.ts Outdated Show resolved Hide resolved
@chezsmithy
Copy link
Contributor Author

@rarkins I've validated that the manually constructed next link matches what should be returned in the API call so it won't break if/when Jfrog fixes it. I could add further validation to check if the next link is actually broken by checking for the missing virtual path. Thoughts?

@chezsmithy @rarkins
Update lib/modules/datasource/docker/common.ts
3b8948d 
Co-authored-by: Rhys Arkins <rhys@arkins.net>
@rarkins
Copy link
Collaborator

rarkins commented Mar 4, 2023

It would be nice if this could fall back to using a fixed (correct) path once one is served, but I don't consider it mandatory to accept this PR

viceice
viceice reviewed Mar 4, 2023
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.

failed tests

lib/modules/datasource/docker/common.ts Outdated Show resolved Hide resolved
@viceice viceice changed the title fix(docker-api-tags): Artifactory next link is broken for tags api fix(datasource/docker): Artifactory next link is broken for tags api Mar 4, 2023
@chezsmithy @viceice
Update lib/modules/datasource/docker/common.ts
9879f95 
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
@rarkins rarkins requested review from viceice and rarkins March 4, 2023 16:09
@rarkins rarkins enabled auto-merge March 5, 2023 18:28
@rarkins rarkins added this pull request to the merge queue Mar 5, 2023
Merged via the queue into renovatebot:main with commit eb5db5b Mar 5, 2023
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 34.157.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@chezsmithy
Copy link
Contributor Author

@setchy FYI on this artifactory fix for another bug.

@chezsmithy chezsmithy deleted the fix(docker-api-tags)-artifactory-tags-api-link-fix branch March 6, 2023 19:04
RahulGautamSingh pushed a commit to RahulGautamSingh/renovate that referenced this pull request Mar 19, 2023
@chezsmithy @rarkins
@viceice @RahulGautamSingh
fix(datasource/docker): Artifactory next link is broken for tags api (r…
db32995 
…enovatebot#20745)

Co-authored-by: Rhys Arkins <rhys@arkins.net>
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@viceice viceice viceice approved these changes

@rarkins rarkins rarkins approved these changes

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@chezsmithy @rarkins @renovate-release @viceice