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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support version-update:ignore-{patch,minor,major} in docker ecosystem #6115

Merged

Conversation

deivid-rodriguez
Copy link
Contributor

@deivid-rodriguez deivid-rodriguez commented Nov 13, 2022

I think CI will fail since this required a few changes, but I think the idea is solid. Will explain in more detail later.

Fixes #5758.
Fixes #1971.

@deivid-rodriguez deivid-rodriguez changed the title Supportversion-update:ignore-{patch,minor,major} in docker ecosystem Support version-update:ignore-{patch,minor,major} in docker ecosystem Nov 13, 2022
@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/docker-version-update-ignores branch from e26dcb7 to 962fe80 Compare November 13, 2022 23:15
@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/docker-version-update-ignores branch from 962fe80 to 1994cc1 Compare November 23, 2022 17:33
@deivid-rodriguez
Copy link
Contributor Author

Ok, so this is the explanation.

Currently ignore conditions require semver-compatible and rubygems-compatible versions. And some Docker versions are not.

My solution may be a bit heavy-handed. It involves extracting a base Dependabot::Version class all other Version classes inherit from, implement a Dependabot::Version#semver method on it that plays nice with ignore conditions and let specific ecosystems override it as needed.

Not sure what you'll think about it, but it's the approach I liked best from what I tried.

Let me know!

Copy link
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 think this is an okay approach, but I can see how it could be considered heavy-handed.

I see this more as preventative maintenance since ecosystems could always extend their semver-compatible version ranges and introduce a bug similar to this Docker ignored version ranges one.

I do have one question around how Docker might handle a version string that does not contain an underscore, but outside of that this LGTM.

triarius added a commit to buildkite/agent that referenced this pull request Jan 17, 2023
We could pin the major and minor versions, but may not work dependabot/dependabot-core#6115.
Instead, because there is no patch version, dependabot should actually
never make a PR for these Dockerfile. Thus we can ignore them entirely.
@dependabot dependabot deleted a comment from syegappan Jan 17, 2023
@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/docker-version-update-ignores branch 4 times, most recently from 286cfdb to dfca4ac Compare February 8, 2023 11:40
@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/docker-version-update-ignores branch 2 times, most recently from e6fd8ad to 2a66ca0 Compare February 8, 2023 12:28
To keep it package manager agnostic.
The ecosystem will need to implement `Version.correct?` and
`Version#to_semver` that will ensure a semver shape that plays nice with
ignore conditions.
Currently only the numeric part of tags is instantiated as a version.
Make that more clear by feeding version specs "real life" versions.
@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/docker-version-update-ignores branch from 2a66ca0 to abd11c4 Compare February 8, 2023 13:21
@deivid-rodriguez deivid-rodriguez merged commit 7d0ff2b into main Feb 8, 2023
@deivid-rodriguez deivid-rodriguez deleted the deivid-rodriguez/docker-version-update-ignores branch February 8, 2023 16:12
@castus
Copy link

castus commented Feb 21, 2023

Hi @deivid-rodriguez , could you tell me, when this PR will be released for all users?

@deivid-rodriguez
Copy link
Contributor Author

We're planning to do public gem releases soon, but I can't tell you a specific date.

@ronan-mch
Copy link

We're planning to do public gem releases soon, but I can't tell you a specific date.

Does this apply to users of the Dependabot app in GitHub? I'm seeing on another thread that some users are getting the PRs that they expect, but I'm not seeing that on my test app. Is that because the change hasn't shipped or because there's something wrong with my setup?

@deivid-rodriguez
Copy link
Contributor Author

Nope, that only applies to people using dependabot as a library. This PR is already deployed to the Github service. If you're getting something unexpected, feel free to report a new issue!

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

Dependabot not ignoring major semver changes [Docker] stay on specific tag and update digest only
5 participants