Skip to content

Bump docker_registry2 to fix #3989#5436

Merged
Nishnha merged 1 commit intodependabot:mainfrom
noorul:noorul-docker-registry2-update
Aug 19, 2022
Merged

Bump docker_registry2 to fix #3989#5436
Nishnha merged 1 commit intodependabot:mainfrom
noorul:noorul-docker-registry2-update

Conversation

@noorul
Copy link
Copy Markdown
Contributor

@noorul noorul commented Jul 27, 2022

The latest version of docker_registry2 has the fix for #3989.

@noorul
Bump docker_registry2 to fix dependabot#3989
0299755 
The latest version of docker_registry2 has the fix for dependabot#3989.
@noorul noorul requested a review from a team as a code owner July 27, 2022 17:18
@Nishnha
Copy link
Copy Markdown
Member

Nishnha commented Jul 27, 2022

Thanks!
Do you think you could add a quick test for this to confirm the bug is no longer present?

Feel free to reach out if you need help!

@noorul
Copy link
Copy Markdown
Contributor Author

noorul commented Jul 27, 2022 •
edited
Loading

@Nishnha I am new to the code base. I tested the code manually on my machine. Can you point me to the test file to which I can add this scenario?

@Nishnha
Copy link
Copy Markdown
Member

Nishnha commented Jul 27, 2022 •
edited
Loading

@noorul, sure!

It looks like we specifically want to test that quay URLs follow the next page for paginated links

Paginated URLs are accessed by our docker client (docker_registry2) in

dependabot-core/docker/lib/dependabot/docker/update_checker.rb

Line 231 in 279956f

client.tags(docker_repo_name, auto_paginate: true).fetch("tags")

and we eventually use that to determine the latest version of a dependency:
#tags_from_registry -> #comparable_tags_from_registry -> #fetch_latest_version -> #latest_version

So a test you may be able to set up is: with a paginated quay URL, the update check is able to follow the paginated URL to find the latest version

dependabot-core/docker/spec/dependabot/docker/update_checker_spec.rb

Line 160 in 279956f

describe "#latest_version" do

Nishnha
Nishnha approved these changes Aug 19, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@Nishnha Nishnha left a comment

Choose a reason for hiding this comment

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

I'm going ahead with approving this since its a gem version update and I don't want to block other fixes this might offer. We will follow up with some test cases for the pagination issue @noorul mentioned after the rollout.

@Nishnha Nishnha merged commit cafef06 into dependabot:main Aug 19, 2022
@noorul
Copy link
Copy Markdown
Contributor Author

noorul commented Aug 20, 2022

@Nishnha Great! Thanks! I couldn't find time to get to this.

@pavera pavera mentioned this pull request Aug 23, 2022
Merged
@jeffwidman jeffwidman mentioned this pull request Mar 21, 2023
Merged
jeffwidman added a commit that referenced this pull request Mar 22, 2023
@jeffwidman
Bump docker_registry2 to 1.13
6f01911 
Bump `docker_registry2` to `1.13` to pickup the fix in deitch/docker_registry2#68.

This pin isn't strictly necessary since the upstream fix is mostly cosmetic making a stack trace more legible, so I originally wasn't going to force it.

However, I removed the `>=` pin as it's redundant given the `~>` pin. It crept in originally as part of forcing a newer patch release in e991abb, but should have been removed by 0299755 / #5436.

And since I was doing that, I figured I might as well bump it too.
jeffwidman added a commit that referenced this pull request Mar 22, 2023
@jeffwidman
Bump docker_registry2 to 1.13 (#6885)
0c8abcb 
* Bump `docker_registry2` to `1.13`

Bump `docker_registry2` to `1.13` to pickup the fix in deitch/docker_registry2#68.

This pin isn't strictly necessary since the upstream fix is mostly cosmetic making a stack trace more legible, so I originally wasn't going to force it.

However, I removed the `>=` pin as it's redundant given the `~>` pin. It crept in originally as part of forcing a newer patch release in e991abb, but should have been removed by 0299755 / #5436.

And since I was doing that, I figured I might as well bump it too.

* `cd updater && bundle install`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Nishnha Nishnha Nishnha 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.

2 participants

@noorul @Nishnha