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

Sync images with tags matching a semantic version constraint #1918

Closed
wants to merge 8 commits into from

Conversation

kubasobon
Copy link

We are running skopeo to ensure our own registries contain specific images/tags. We do not want to sync every tag, but rather limit them to versions we utilize. This gets increasingly difficult for images using semantic versioning in their tags. See @SamStenton's reasoning in #1756 for examples.

This PR aims at adding an option to synchronize images with tags matching a particular semantic version constraint. Below is an example of YAML config that can be used with code added by this PR:

docker.io:
  images-by-semver:
    alpine:
      - '3.12 - 3.13' # Sync all images in the range
      - '>= 3.17' # Sync all patch, minor, and major releases after 3.17.0
    ubuntu:
      - '~22' # Sync all releases >= 22.0 and < 23.0: 22.04, 22.10

This PR uses github.com/Masterminds/semver v3 package to parse and match semantic versions. This package respects the upstream (https://semver.org/) definition of semantic versioning.

Closes #1756

@kubasobon kubasobon force-pushed the semver branch 2 times, most recently from 3eb4bea to e674cd5 Compare February 17, 2023 16:40
@mtrmac mtrmac added the kind/feature A request for, or a PR adding, new functionality label Feb 20, 2023
Copy link
Contributor

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

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

Thanks! The use of constraint expressions like this seems very versatile and results in elegant configuration.

Compare #1918 — this is likely to be a more general solution.


One thing that we need to get “right” now is the constraint syntax — apparently https://semver.org doesn’t define anything like that as a part of the standard.

So, in addition to https://github.com/Masterminds/semver, there is also https://github.com/blang/semver (used in Podman, although not in a user-visible way), and https://github.com/hashicorp/go-version (used in previous #995 ); each seems to have a different syntax and/or feature set.

I don’t really know anything about the trade-offs, and in general the the PR author’s choice is the preferred one by default — still, I’d very much like others to weigh in here, if there are any issues to consider.

cmd/skopeo/sync.go Outdated Show resolved Hide resolved
cmd/skopeo/sync.go Outdated Show resolved Hide resolved
cmd/skopeo/sync.go Outdated Show resolved Hide resolved
cmd/skopeo/sync.go Outdated Show resolved Hide resolved
cmd/skopeo/sync.go Outdated Show resolved Hide resolved
docs/skopeo-sync.1.md Show resolved Hide resolved
@kubasobon
Copy link
Author

Thanks! The use of constraint expressions like this seems very versatile and results in elegant configuration.

Compare #1918 — this is likely to be a more general solution.

Thank you for taking the time to review my PR!

One thing that we need to get “right” now is the constraint syntax — apparently https://semver.org doesn’t define anything like that as a part of the standard.

So, in addition to https://github.com/Masterminds/semver, there is also https://github.com/blang/semver (used in Podman, although not in a user-visible way), and https://github.com/hashicorp/go-version (used in previous #995 ); each seems to have a different syntax and/or feature set.

I don’t really know anything about the trade-offs, and in general the the PR author’s choice is the preferred one by default — still, I’d very much like others to weigh in here, if there are any issues to consider.

I have done a bit of research on the side and created a test suite to compare the three libraries you mentioned. You can find it here. My main goal was highlighting differences between the libraries and how well they handle edge cases, like incomplete versions, logical operators, etc. From my point of view https://github.com/Masterminds/semver offers the most features and covers every use case one might want/need from the other two. This hopefully satisfies the broadest spectrum of skopeo users.

@kubasobon kubasobon requested a review from mtrmac March 3, 2023 15:45
Copy link
Contributor

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

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

Thanks!

@vrothberg (others), WDYT about the semver filter syntax?

cmd/skopeo/sync.go Show resolved Hide resolved
cmd/skopeo/sync.go Show resolved Hide resolved
Comment on lines +381 to +382
// include repository descriptors for cfg.ImagesByTagRegex
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Non-blocking: I don’t think the { } blocks are that necessary — and the comments don’t say anything new either.

(Absolutely non-blocking: Possibly s/filterCollection/filters/g, s/additionRepoDescList/descs/; the variables have short enough scope that short names are sufficient. But this works just fine.)

Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

@vrothberg (others), WDYT about the semver filter syntax?

LGTM
I like the idea quite a lot and the dependency looks good.

Thanks a lot for contributing!

@rhatdan PTAL

@SamStenton
Copy link

Awesome stuff @kubasobon! Would love to start making use of this in some of our internal tooling - when can we expect this to be reviewed and merged?

@github-actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

@MShekow
Copy link

MShekow commented May 8, 2023

Merging this PR would be amazing

@rhatdan
Copy link
Member

rhatdan commented May 8, 2023

Well this PR needs to be rebased to even consider it.

@github-actions github-actions bot removed the stale-pr label May 9, 2023
@github-actions
Copy link

github-actions bot commented Jun 8, 2023

A friendly reminder that this PR had no activity for 30 days.

@derbauer97
Copy link

Is there any chance this will be merged? We would be happy to have such a feature it is exactly what we need :)

@github-actions github-actions bot removed the stale-pr label Jun 13, 2023
@github-actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

@husseinferas
Copy link
Contributor

Hey @kubasobon
Thank you for the changes. I am looking forward to using this feature; is there any plan to move this forward and merge the PR in the near future?

@dmitri-lerko
Copy link

Came here to propose a similar PR only to find it implemented and approved. How can I help to move this forward?

@MShekow
Copy link

MShekow commented Jan 3, 2024

We also have been using the code of this PR for a couple of months, and would really love to get this merged.

What this PR is still lacking (but would be very sensible to add) is the ability to somehow additionally pre-filter tags that the semantic version parser library (here: semver) cannot possibly parse. The core problem is that version tags (in the Docker world) are a "wild west". For instance, look what the LinkerD-team has done: make up weird tag structures like stable-<version>. And similarly, cosign signatures may end up with tags such as sha256-744544149f18ef2f80a2f8b1324d0ec6a05252d2b6613e60310881ecf476bcfa.sig.

For this reason, I propose that there could be an additional optional configuration option to configure this, e.g. using a named capture group inside a regular expression.

Example:

images-by-semver:
  ghcr.io/linkerd/proxy:
    version-regex-filters:
    - "stable-(?P<semver>\\d+\\.\\d+\\.\\d)"
    version-ranges:
      - "3.12 - 3.13"
      - ">= 3.17"

The idea would be that if any version-regex-filters are defined, only the content of the semver capture group is forwarded to the semver parser.

@mtrmac
Copy link
Contributor

mtrmac commented Jan 3, 2024

Came here to propose a similar PR only to find it implemented and approved. How can I help to move this forward?

  • Rebase on top of main
  • Implement the outstanding cleanups

@husseinferas
Copy link
Contributor

Hello, everyone
I've created a new PR with the same changes. I rebased it with the main branch
#2189

@mtrmac
Copy link
Contributor

mtrmac commented Jan 19, 2024

#2189 was merged. @kubasobon , @husseinferas thanks for all your effort!

@mtrmac mtrmac closed this Jan 19, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/feature A request for, or a PR adding, new functionality locked - please file new issue/PR stale-pr
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sync all images from a starting version
9 participants