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

feature: add --scoped-append-registry flag #1531

Conversation

art-shutter
Copy link

Introduces a new flag to simplify work with YAML source as referenced in #1072

Signed-off-by: Artem Kamenev artem@kamenev.pw

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, but as discussed in #854 (comment) , I don’t think it makes sense to continue adding individual options for every mirror organization options a user might choose to design.

@mtrmac
Copy link
Contributor

mtrmac commented Jan 3, 2022

For YAML, contrary to the existing images and images-by-tag-regex, we need a way to add more per-repo options.

Maybe something vaguely like (based on the man page example):

registry.example.com:
    repos:
        busybox:
            destination: mirror.local/busybox
            assume-mutable-tags: latest
        redis:
            destination: mirror.local/redis
            mirror-tags: ["1.0", "2.0"]
            mirror-digests: ["sha256:0000000000000000000000000000000011111111111111111111111111111111"]
        nginx:
            destination: mirror.local/completely/different/policy/nginx
            mirror-by-tag-regex: ^1\.13\.[12]-alpine-perl$
    credentials:
        username: john
        password: this is a secret
    tls-verify: true
    cert-dir: /home/John/certs

with a per-repo destination directly in the config file, not on the command line.

This would also allow adding per-repo assume-mutable-tags options discussed in #1498 , and other options over time.

@art-shutter
Copy link
Author

art-shutter commented Jan 10, 2022

I would agree that supplying complete destination in the YAML itself looks better.
I suggest the following:

  • add global settings section for repos
    • per-repo settings would supersede global settings
  • add two destination-related keys
    • custom-destination with value of any custom path
    • destination-path-style with these possible values: "full", "full-no-registry", "flat"
  • custom-destination would take precedence over destination-path-style
  • default behavior would be as it is now, destination-path-style: "flat"

I would also raise the question of deprecating/refactoring the --scoped flag. As of now, I would leave it and when parsing YAML it would, if neither destination-related keys are specified, switch default behavior from destination-path-style: "flat" to destination-path-style: "full"

So, the resulting YAML might look something like

---

global-settings:
  destination-path-style: "full-no-registry"
  mirror-tags: ["latest"]
  assume-mutable-tags: ["latest"]
  tls-verify: true
docker.io:
  repo-settings:
    destination-path-style: "full"
    mirror-tags: ["newest"]
    credentials:
      username: "john"
      password: "this is a secret"
    cert-dir: "/home/John/certs"
  images:
    busybox:
      custom-destination: "mirror.local/busybox"
      assume-mutable-tags: ["best-tag"]
      destination-path-style: "flat"
      mirror-tags: ["1.0", "2.0"]
      mirror-digests: ["sha256:0000000000000000000000000000000011111111111111111111111111111111"]
    nginx:
      mirror-by-tag-regex: ^1\.13\.[12]-alpine-perl$
quay.io:
  repo-settings:
    destination-path-style: "flat"
  images:
    prometheus/prometheus:
      mirror-tags: ["3.1", "whatever"]

However, I would leave assume-mutable-tags functionality outside the scope of this PR.

@mtrmac What do you think?

upd:

  • a three-tier system would also look good and be highly configurable, with destinaton repo -> source repo -> image settings management (reflected this in the example above)

upd2:

  • updated yaml format

@anisimovdk
Copy link

I agree with @art-shutter.
In our case we need to sync a lot of images to a bunch of registries, that why defining destination per image looks bad.
Global settings much more preferable or defining destination registry by argument from cli.

@github-actions
Copy link

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

@ToroNZ
Copy link

ToroNZ commented Jun 27, 2022

any chance we can have something while the ideal solution is worked on? this kinda ruins the tidyness of using the YAML source option.

@github-actions github-actions bot removed the stale-pr label Jun 28, 2022
@github-actions
Copy link

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

@davidspek
Copy link

I'm also needing this feature and it'd be great if this PR can be merged.

@github-actions github-actions bot removed the stale-pr label Sep 20, 2022
Signed-off-by: Artem Kamenev <artem@kamenev.pw>
@art-shutter
Copy link
Author

since there hasn't been much happening in terms of updating the yaml to the new format, I have updated changes here with the default branch and have also proposed another per-repo solution in #1769

@mhowell24
Copy link

I would be interested in working on the updated yaml format mentioned above if nobody else already is. @mtrmac would you accept the PR if I were to put one together?

@mtrmac
Copy link
Contributor

mtrmac commented Oct 6, 2022

I do think it is valuable to finally solve the sync repo mapping deficiencies, but I’m afraid it’s also not something I can spend sufficient time on in the next few weeks or likely months.

@vrothberg do you have the time to help make this happen?

@art-shutter
Copy link
Author

@mhowell24 I would definitely like to help, be it reviewing/testing or anything else.
@mtrmac are we ok with introducing breaking changes to manifests?

@mtrmac
Copy link
Contributor

mtrmac commented Oct 7, 2022

If you are talking about the YAML format read by skopeo copy, I think breaking existing scripts is very undesirable, and easily avoidable — if nothing else, sync --source yaml2 allows us to define a new format without affecting the old one at all.

@mhowell24
Copy link

Brilliant, thanks @art-shutter and @mtrmac I will try and put something together this week

@mhowell24
Copy link

Hi @art-shutter and @mtrmac I have opened #1792 as a draft to implement the yaml config from above, let me know what you think

@mtrmac mtrmac added the kind/feature A request for, or a PR adding, new functionality label Dec 7, 2022
Copy link

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

@art-shutter art-shutter closed this Sep 4, 2024
@stale-locking-app stale-locking-app bot locked as resolved and limited conversation to collaborators Dec 4, 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.

6 participants