Adding support for semver filter in sync command

[husseinferas]

Adding Kubasobon's changes in the PR and rebase it with main to move it forward

more changes added:

• using single semver constraint

docker.io:
  images-by-semver:
    node: ">= 20.0.0"

[mtrmac]

Thanks for taking this on!

Outstanding review comments: #1918 (review)

(Also, eventually please squash the “formate sync.go file” commit, so that the code passes tests on every commit.)

[husseinferas]

error: RPC failed; HTTP 500 curl 22 The requested URL returned error: 500

@mtrmac
Regarding the CI failure, I can't retry the step on cirrus-ci, I think it's only a network glitch

[mtrmac]

Thanks!

(Eventually a rebase&squash would be preferred over intermediate commits and merges, but that’s non-blocking)

RPC failed; HTTP 500 curl 22 The requested URL returned error: 500

Let’s see if a future version passes; the macOS failures at that stage are certainly unrelated.

[husseinferas]

I now have problem: when I run the sync command, it prints "Start filtering using the regular expression:" log message multiple times @mtrmac

I think we can delete this log message because the code will need extra unnecessary changes if we want to keep it

[MShekow]

Looking forward to getting this PR merged.

Do @husseinferas or @mtrmac have any opinions on what I posted in #1918 (comment) ?

From long-term experience I can conclude that:

• using only regular expressions is not a good solution, because the regexp either matches too many tags, or you need to build a very complicated regex that is hard to understand and maintain
• using only semver comparisons is not possible in practice, because many container images do not use "proper" semver tags
• The only solution I can think of is to mix both approaches, as I discussed in Sync images with tags matching a semantic version constraint #1918 (comment)

My gut feeling is that now would be the best time to solve that problem, as otherwise breaking changes might be inevitable.

[husseinferas]

Thank you @MShekow
I really like your proposal but I would say this is an improvement that we can make later after introducing images-by-semver first.
I'd prefer to implement this in different PR

[MShekow]

Thank you @MShekow I really like your proposal but I would say this is an improvement that we can make later after introducing images-by-semver first. I'd prefer to implement this in different PR

I fully agree with you to keep PRs (which implement things) small. What I'm suggesting is merely that we should already brainstorm whether any potential syntax changes (in the input yaml file) would be good to introduce now.

[mtrmac]

@MShekow Looking at your proposal, it does seem to be compatible with the proposed structure, right now. Is there anything that you think needs changing?

[MShekow]

@MShekow Looking at your proposal, it does seem to be compatible with the proposed structure, right now. Is there anything that you think needs changing?

To reiterate, the currently proposed structure (that is also implemented in this PR) is something like this:

images-by-semver:
  alpine:
    - "3.12 - 3.13"

I can't think of a "good" solution for adding the prefilter-regex (that contains the named capture group) that I suggested in #1918 (comment). You would have to use funky syntax, mangling the prefilter-regex into the version string somehow, e.g. by replacing the version string (here: "3.12 - 3.13") with something like "3.12 - 3.13|||stable-(?P<semver>\\d+\\.\\d+\\.\\d)" (using an arbitrary delimiter like |||).

That was the entire reason why I proposed to add an additional YAML "layer" in #1918 (comment) that separates the list of semver-strings from the list of prefilter-regexes.

[husseinferas]

good, so our current structure will work just fine in future with regex layer

images-by-semver:
  alpine: "3.12 - 3.13"

with regex, in next implementation

images-by-semver:
  alpine: "3.12 - 3.13 ||| stable-(?P<semver>\\d+\\.\\d+\\.\\d)"

[TomSweeneyRedHat]

Code LGTM

[husseinferas]

Thank you @TomSweeneyRedHat can you approve the PR?

[TomSweeneyRedHat]

@husseinferas, the branch needs updating. I've just kicked that off. Once completed, I'd like @mtrmac to do the final approval to ensure his comments were addressed.

[mtrmac]

That was the entire reason why I proposed to add an additional YAML "layer" in #1918 (comment) that separates the list of semver-strings from the list of prefilter-regexes.

Oh… now I get what you mean.

I definitely agree that a separate field is better than trying to stuff things inside the server filter. OTOH it’s a bit tedious to force every user to add one more level of nesting for a hopefully fairly rare case.

Instead of

registry.example.com:
    images-by-semver:
        alpine:
            regex-filters: "…"
            versions: ">= 3.12.0"

we can do

registry.example.com:
    images-by-semver:
        alpine: ">= 3.12.0"
    semver-regex-filters:
        alpine: "…"

and only the repos which need that filter would pay the price.

(Really long-term we should probably end up with

sync:
   -  source: registry.source.com/source-repo
       destination: registry.source.com/dest-repo
       tags: …
       regex: …
       semver: …
       semver-regex-filter: …

but that’s a long-term future direction.)

[mtrmac]

Thanks!

LGTM; please squash&rebase into a single commit.

[mtrmac]

(For transparency, I have rebased the work, to avoid WIP commits; per https://github.com/containers/skopeo/compare/db8f93e33ea3b99b157ef0c74226c922d7ecd658..177d4adb205dffa0e0e50d0709ee27b953ee0c00 the code is identical.)

[mtrmac]

Thanks again!

[MShekow]

I'm super happy that this PR has come to a successful conclusion :).

@husseinferas are you interested in implementing the regex filter I mentioned (using the proposal made in #2189 (comment)), to increase the usefulness of this PR's semver filter? If not, I would have a go at it, it might take a while, however.

[husseinferas]

Thanks @MShekow @mtrmac
@mtrmac, should we wait for the next version release to start using Semver?

I think we need to update the sync YAML file structure for the next iteration because the current sync file is not scalable and has another problem that we need to fix, which is the destination repository mentioned here.

sync:
   -  source: registry.source.com/source-repo
       destination: registry.source.com/dest-repo
       tags: …
       regex: …
       semver: …
       semver-regex-filter: … 

I am willing to work on this feature, @mtrmac How does the team communicate? Is there a Slack channel or chat group somewhere? I had many questions regarding this update.

[MShekow]

Congrats on the release in v1.15.0 🎉

Thanks @MShekow @mtrmac @mtrmac, should we wait for the next version release to start using Semver?

I think we need to update the sync YAML file structure for the next iteration because the current sync file is not scalable and has another problem that we need to fix, which is the destination repository mentioned here.

sync:
   -  source: registry.source.com/source-repo
       destination: registry.source.com/dest-repo
       tags: …
       regex: …
       semver: …
       semver-regex-filter: … 

I am willing to work on this feature, @mtrmac How does the team communicate? Is there a Slack channel or chat group somewhere? I had many questions regarding this update.

Really looking forward to the next iteration. I suppose that we're waiting for input from @mtrmac ?

[mtrmac]

In general there’s a long-term need to redesign the new YAML format; the GitHub repo contains quite a few open issues directly or indirectly touching on the needs and design requirements. I’m afraid it’s also not a feature I’ve not been able to invest significant time in.

These GitHub issues are the primary collaboration mechanism of the project.