Add tag/untag commands for container repositories #429
Conversation
pulpcore/cli/container/context.py
Outdated
| VERSION_CONTEXT = PulpContainerRepositoryVersionContext | ||
| CAPABILITIES = { | ||
| "sync": [PluginRequirement("container")], |
There was a problem hiding this comment.
If adding/removing tags between the two repository types is supported from different versions of pulp_container then I can add capabilities here. Might be worth to do it anyway since I don't know which version tagging was added in.
There was a problem hiding this comment.
Support for tagging manifests within push repositories was added in version 2.3.0 (2021-02-08). According to the pulp-cli docs, we support 5 versions of pulpcore from 3.11-3.16 and 5 of Pulp's plugins: pulp_ansible, pulp_container. Version 2.3.0 falls into the pulpcore requirement when looking at requirements.txt.
pulpcore/cli/container/repository.py
Outdated
| if len(tag) == 0: | ||
| raise click.ClickException("Please pass a non empty tag name.") | ||
|
|
||
| digest = digest.strip() | ||
| if not digest.startswith("sha256:"): | ||
| digest = f"sha256:{digest}" | ||
| if len(digest) != 71: # len("sha256:" + 64 | ||
| raise click.ClickException("Improper SHA256, please provide a valid 64 digit digest.") |
There was a problem hiding this comment.
Tried doing some bare minimum checks and leaving the rest to the endpoint's serializer. If there is anything else needed please tell me.
There was a problem hiding this comment.
You may also add a validation regex for tags.
For validation purposes, it would be better to have a separate callback (https://click.palletsprojects.com/en/8.0.x/options/#callbacks-for-validation). Do you agree?
gerrod3
force-pushed
the
tag-container
branch
from
5b51ae1 to
2e75eb4
Compare
pulpcore/cli/container/context.py
Outdated
| TAG_ID: ClassVar[str] = "repositories_container_container_tag" | ||
| UNTAG_ID: ClassVar[str] = "repositories_container_container_untag" |
There was a problem hiding this comment.
This looks like a rebase is needed.
There was a problem hiding this comment.
Are you talking about your new PR refactoring the IDs? #448 I can wait till after it's merged to rebase.
There was a problem hiding this comment.
Oh, yes, i get confused what's merged or not.
gerrod3
force-pushed
the
tag-container
branch
4 times, most recently
from
531367a to
ba3e164
Compare
gerrod3
force-pushed
the
tag-container
branch
from
ba3e164 to
4f868a2
Compare
| if len(value) == 0: | ||
| raise click.ClickException("Please pass a non empty tag name.") |
There was a problem hiding this comment.
FWIW, an empty string would not pass the regex. But i see this check provide a better error condition.
| if not repository_ctx.capable("tag"): | ||
| raise click.ClickException(_("pulp_container 2.3.0 is required to tag images")) |
There was a problem hiding this comment.
(Note for later:) Ha, we should have a needs_capability that raises itself.
Rationale: All mentions of Pulp versions should be in context.py.
fixes: #423
See issue for discussion around command structure. I would like to have #425 merged first so I can use those commands for creating tests for these commands. I'll update the
suppported_workflows.mdafter I'm done with all the new container commands to avoid merge conflicts.