Skip to content

Fix dependabot incorrectly downgrading docker versions#5886

Merged
deivid-rodriguez merged 2 commits intomainfrom
deivid-rodriguez/docker-downgrades
Oct 17, 2022
Merged

Fix dependabot incorrectly downgrading docker versions#5886
deivid-rodriguez merged 2 commits intomainfrom
deivid-rodriguez/docker-downgrades

Conversation

@deivid-rodriguez
Copy link
Copy Markdown
Contributor

In the edge case of Java versions, which include an extra "update_release" separated by _, and that can also include a variable number of segments, Dependabot would interpret something like 11.0.16_8-jdk as higher than 11.0.16.1_1-jdk.

This is because up until now Dependabot considers all components equally for sorting, and the 4th component in the first version (8) is higher than the 4th component in the second (1), while previous components are equal.

The correct way to do this is to compare the "release segment" first, in this case, 11.0.16 vs 11.0.16.1, and only if those are equal compare the "update segment".

With that logic in place, things sort as they should.

@deivid-rodriguez deivid-rodriguez requested a review from a team as a code owner October 13, 2022 22:20
@mattt mattt self-assigned this Oct 13, 2022
Copy link
Copy Markdown
Contributor

@mattt mattt left a comment

Choose a reason for hiding this comment

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

Thanks so much for looking into this, @deivid-rodriguez! I really appreciate your quick turnaround.

I left some thoughts about ways to make this more robust. Please double check me on the Ruby comparable stuff in particular, because I have no idea whether it's valid to say that two values aren't equal but are ordered the same.

Copy link
Copy Markdown
Contributor

@mattt mattt left a comment

Choose a reason for hiding this comment

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

Great work on this, @deivid-rodriguez! As we discussed in https://github.com/dependabot/dependabot-core/pull/5886/files#r995981733, it'd be great to take another look at how UpdateChecker and Version divide up responsibilities for version handling, but I don't think that should be a blocker for this fix. I'm excited to get this fix in front of customers soon 😃

@deivid-rodriguez
Copy link
Copy Markdown
Contributor Author

Thank you @mattt! I will merge and release this on Monday.

@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/docker-downgrades branch 2 times, most recently from 32dcd51 to 91d26eb Compare October 17, 2022 19:38
When one of the version would would more release segments than other,
but the other would have a higher update level than the first, then the
latter would sort first.

For example, 11.1.0_8 would be consider higher than 11.1.0.1_1.

To fix the issue, consider release segments and update level as separate
versions, and only use the latter for sorting to resolve ties in the
release level.
@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/docker-downgrades branch from 91d26eb to 79646dc Compare October 17, 2022 19:58
@deivid-rodriguez deivid-rodriguez merged commit 97b6a78 into main Oct 17, 2022
@deivid-rodriguez deivid-rodriguez deleted the deivid-rodriguez/docker-downgrades branch October 17, 2022 20:58
@pavera pavera mentioned this pull request Oct 31, 2022
@jeffwidman jeffwidman added the L: docker Docker containers label May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L: docker Docker containers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants