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 uploading an ORAS Artifact Manifest type without a subject #361

Closed
Tracked by #369
SteveLasker opened this issue Mar 30, 2022 · 12 comments
Closed
Tracked by #369
Assignees
Labels
breaking change Public API changes good first issue Good for newcomers
Milestone

Comments

@SteveLasker
Copy link
Contributor

Repro Steps

ACR_NAME=wabbitnetworks
REGISTRY=wabbitnetworks.azurecr.io
REPO=net-monitor
TAG=v1
IMAGE=$REGISTRY/${REPO}:$TAG

Assuming you have $IMAGE, the following command works:

oras push $REGISTRY/$REPO \
    --artifact-type 'signature/example' \
    --subject $IMAGE \
    ./readme.md:application/md

However, if I want to upload a readme file, that doesn't have a subject, I'm unable to exclude the --subject field:

oras push $REGISTRY/$REPO:readme \
    --artifact-type 'signature/example' \
    ./readme.md:application/md
@shizhMSFT shizhMSFT added this to the v0.14.0 milestone May 7, 2022
@shizhMSFT
Copy link
Contributor

Linking oras-project/distribution#9

nima added a commit to nima/oras that referenced this issue Jun 30, 2022
nima added a commit to nima/oras that referenced this issue Jun 30, 2022
nima added a commit to nima/oras that referenced this issue Jun 30, 2022
Fixes oras-project#361

Does NOT fix oras-project/oras-go#206; since this client does not use the go library.
nima added a commit to nima/oras that referenced this issue Jun 30, 2022
Fixes oras-project#361

Does NOT fix oras-project/oras-go#206; since this client does not use the go library.

Signed-off-by: Nima Talebi <[email protected]>
nima added a commit to nima/oras that referenced this issue Jul 1, 2022
Fixes oras-project#361

Does NOT fix oras-project/oras-go#206; since this client does not use the go library.

Signed-off-by: Nima Talebi <[email protected]>
nima added a commit to nima/oras that referenced this issue Jul 1, 2022
Fixes oras-project#361

Does NOT fix oras-project/oras-go#206; since this client does not use the go library.

Signed-off-by: Nima Talebi <[email protected]>
@shizhMSFT shizhMSFT added the good first issue Good for newcomers label Jul 26, 2022
@yuehaoliang
Copy link
Contributor

oras push $REGISTRY/$REPO \
    --artifact-type 'signature/example' \
    --subject $IMAGE \
    ./readme.md:application/md

The command above now can be substituted by
oras attach $IMAGE ./readme.md:application/md --artifact-type signature/example
for the sake of UX. It will pack an ORAS artifact and push it to the registry.

oras push $REGISTRY/$REPO:readme \
    ./readme.md:application/md

This command without --artifact-type 'signature/example' \ will push an OCI artifact.
Since artifactType is a property of ORAS artifact, if we want to add artifactType, we want it be packed as an ORAS artifact as well, even though it does not have a subject.

I'm currently working on supporting this feature.

@SteveLasker
Copy link
Contributor Author

Thanks @yuehaoliang-microsoft,
Being able to pivot when to use the new oras aritfact manivest, vs. the oci image manifest is a good clarification.
As the oras artifact manifest isn't yet broadly supported, and the oci wg for reference types isn't yet complete, may I suggest the following:

Regardless of which manifest is used to persist, I'd suggest we use the artifact-type parameter for both push and attach

  • When the push command is used, we use the oci.manifest, as the user isn't benefiting from any unique capabilities of the oras.artifact.manifest. The --artifact-type parameter sets manifest.config.mediaType property.
  • When the attach command is used, we use the oras.artifact.manifest, as it's the only supported spec at this point. The --artifact-type parameter sets manifest.artifactType property.
  • When push is used, with --annotations, and no files are pushed, we use the oras.artifact.manifest, as it's the only supported manifest that doesn't require blobs/layers.

That pushing of artifacts that only contain annotations are being used to create root references for supply chain artifacts that represent non-deployed scenarios. For instance, we need to support shipping an SBOM for the Office 365 service. The user would push an artifact, with a reference of mcr.microsoft.com/services/office/365:2022-03-q1

@yuehaoliang
Copy link
Contributor

yuehaoliang commented Jul 28, 2022

Thank you for your response, Steve!

To clarify our previous thought (as discussed with @qweeah):

  • For the attach command, it has already been doing well: it pushes an oras.artifact.manifest, and the --artifact-type will set manifest.artifactType property.
  • For the push command, if user doesn't give an --artifact-type flag, it pushes an oci.manifest as usual. If user gives an --artifact-type flag, it packs an oras.artifact.manifest and sets manifest.artifactType property.
    Eg.
oras push $REGISTRY/$REPO:readme \
    --artifact-type 'signature/example' \
    ./readme.md:application/md

This command will push an oras.artifact.manifest.

For my understanding for your new suggestion, you'd like the the push command with an --artifact-type flag still pushes an oci.manifest and let --artifact-type set manifest.config.mediaType, instead of an oras.artifact.manifest because there's no benefit for this change and ORAS artifact manifest type isn't yet broadly supported.

However, the subject of this issue mentioned that "uploading an ORAS artifact manifest type without a subject", so I'd like to confirm which type of manifest is preferred now. Also, a little confusion of my own, using --artifact-type flag to set manifest.config.mediaType seems mismatched. Maybe we can come up with a better name for the command flag?

Pushing of artifacts with annotations is out of scope for this issue, @junczhuMSFT is working on it. Please refer to #362.

UPDATE:

OCI Artifact and ORAS Artifact here it says manifest.config.mediaType is used to determine the artifact type, so now I understand they're basically the same thing. For push command, we already have a --manifest-config flag to let user provide a config.json and set manifest.config.mediaType. The use case is:

Example - Push file "hi.txt" with the custom manifest config "config.json" of the custom "application/vnd.me.config" media type:
 oras push --manifest-config config.json:application/vnd.me.config localhost:5000/hello:latest hi.txt

Therefore, using --artifact-type flag to set manifest.config.mediaType is kind of a subset of the above functionality, duplicated but more straightforward to use. I see a lot of changes were made after this issue was created in March. As there's a --manifest-config flag, do we still want to add an --artifact-type flag to let user simply set manifest.config.mediaType in oci.manifest? Or, do we want to pack it as an oras.artifact.manifest as described in this issue's subject?

@yuehaoliang
Copy link
Contributor

@SteveLasker Forgot to pin you in the above comments. Need some confirmation to continue the implementation. Thank you!!

@shizhMSFT
Copy link
Contributor

@SteveLasker I have few comments and elaborations on your suggestions.

  • When the push command is used, we use the oci.manifest, as the user isn't benefiting from any unique capabilities of the oras.artifact.manifest. The --artifact-type parameter sets manifest.config.mediaType property.

The artifact type is different from the media type. However, given the fact that we use manifest.config.mediaType as the artifact type for OCI artifact, we can set manifest.config.mediaType to the value by --artifact-type if --manifest-config is not present so that users will get a meaningful media type other than the default unknown.

  • When push is used, with --annotations, and no files are pushed, we use the oras.artifact.manifest, as it's the only supported manifest that doesn't require blobs/layers.

Did you mean --manifest-annotation (as in #362, /cc @junczhuMSFT) for --annotations?
If OCI manifest does not support empty blobs/layers is the reason, why don't we only use the oras.artifact.manifest for artifacts with no blobs even if --manifest-annotation is not set?

@shizhMSFT shizhMSFT added the breaking change Public API changes label Jul 29, 2022
@SteveLasker
Copy link
Contributor Author

Thanks @shizhMSFT
Clarifying a few things:

The artifact type is different from the media type.

Can you explain this? What I'm suggesting is we always present -- artifact-type as the property users set from an experience perspective. When using oci.manifest, we pass this value to manifest.config.mediaType. When using oras.artifact.manifest we set the manifest.artifactType value.

You bring up a good point that we need to account for scnearios when the user passes in --manifest-config. What if we either no longer support :mediaType on this property, or document it overrides --artifact-type? I'm willing to bet that 99.9% of scenarios are passing in /dev/null as they're only setting this to get --artifact-type

If OCI manifest does not support empty blobs/layers is the reason, why don't we only use the oras.artifact.manifest for artifacts with no blobs even if --manifest-annotation is not set?

Your wording is more accurate. I couldn't think of another reason to call oras push if someone wasn't setting annotations, but you're correct. We should make the logic, no files = oras.artifact.manifest

@shizhMSFT
Copy link
Contributor

Can you explain this? What I'm suggesting is we always present -- artifact-type as the property users set from an experience perspective. When using oci.manifest, we pass this value to manifest.config.mediaType. When using oras.artifact.manifest we set the manifest.artifactType value.

It is a separate issue. If we always present --artifact-type, the user experience will be changed and may be unexpected. /cc @FeynmanZhou @yizha1

@SteveLasker
Copy link
Contributor Author

Would like to hear more.
config.mediaTyoe was considered the artifactTyoe. It just requires a user to pass in a null config, with cryptic and different syntax between windows and linux. :(
Was there something else?

@shizhMSFT
Copy link
Contributor

@SteveLasker Wait... There is an UX conflict. When the user do

oras push --artifact-type some/type some/repo blablabla.txt

we should always push an oras artifact manifest as long as --artifact-type is specified.

@shizhMSFT
Copy link
Contributor

shizhMSFT commented Aug 1, 2022

To be clear, the changes are summarized as follows

  • oras push --artifact-type will push an oras artifact manifest with no subject.
  • oras push no files without --artifact-type will push an oras artifact manifest.

2 PRs are required to address above changes.

@shizhMSFT
Copy link
Contributor

Closing as discussed with @SteveLasker that UX for uploading an ORAS artifact manifest without a subject is not supported in CLI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Public API changes good first issue Good for newcomers
Projects
No open projects
Status: Done
Development

No branches or pull requests

5 participants