Add artifact schema and tests #942
Conversation
sajayantony
requested review from
jonjohnsonjr,
jonboulle,
stevvooe,
sudo-bmitch,
vbatts and
cyphar
as code owners
|
@sajayantony #944 is merged. This should be ready to rebase and clean up the ioutil references. Thanks! |
|
I need to resubmit this with the field name changes now :) |
sajayantony
force-pushed
the
artifact-schema
branch
from
2dd253f
to
2bf82b9
Compare
There was a problem hiding this comment.
LGTM with a couple minor changes. I think it would be helpful to include some valid minimal artifacts here too. The three examples I'm thinking of are
- mediaType only (kinda useless but valid)
- mediaType + artifactType + blobs
- mediaType + artifactType + annotations
I'm happy to do that as a separate PR if that's easier.
schema/artifact-manifest-schema.json
Outdated
| @@ -0,0 +1,29 @@ | |||
| { | |||
| "description": "OpenContainer Artifact Manifest Specification", | |||
| "$schema": "https://json-schema.org/draft-04/schema#", | |||
There was a problem hiding this comment.
From the spec, this needs to be the exact string and we weren't supposed to have changed it to https: https://json-schema.org/understanding-json-schema/reference/schema.html
| "$schema": "https://json-schema.org/draft-04/schema#", | |
| "$schema": "http://json-schema.org/draft-04/schema#", |
There was a problem hiding this comment.
I've opened - #961 to fix up the remaining draft 4 identifier strings.
There was a problem hiding this comment.
Thanks for pointing me to #931 @sudo-bmitch. I’ve closed #961
schema/artifact_test.go
Outdated
| "subject": { | ||
| "mediaType": "application/vnd.oci.image.manifest.v1+json", | ||
| "digest": "sha256:c86f7763873b6c0aae22d963bab59b4f5debbed6685761b5951584f6efb0633b" | ||
| }, | ||
| "annotations": { | ||
| "key1": "value1", | ||
| "key2": "value2" | ||
| } |
There was a problem hiding this comment.
Nit, indentation needs an extra space
There was a problem hiding this comment.
I went ahead and reformatted all the examples. It seems that certain properties had spaces after the property names and some didn't and I think those that have spaces were incorrectly formatted.
There was a problem hiding this comment.
Please feel to add more examples. I'm hoping to get the baseline framework and schema in first.
There was a problem hiding this comment.
@sudo-bmitch hope I've addressed the issues.
|
Need to validate that that the tests catch example issues like |
sajayantony
force-pushed
the
artifact-schema
branch
2 times, most recently
from
bbb501a
to
2b2e760
Compare
Signed-off-by: Sajay Antony <sajaya@microsoft.com>
sajayantony
force-pushed
the
artifact-schema
branch
from
2b2e760
to
1760050
Compare
|
@shizhMSFT thanks for fixing #960 - artifact examples were not validated in this PR and your PR helped :) |
|
Bumping this issue to get feedback from other maintainers @opencontainers/image-spec-maintainers |
|
Closing this one because of #999. |
Signed-off-by: Sajay Antony sajaya@microsoft.com