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.

Sign up for GitHub

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

Jump to bottom

descriptor schema: add missing data and artifactType definitions #1022

Merged
merged 1 commit into from
Mar 10, 2023
Merged

descriptor schema: add missing data and artifactType definitions #1022

merged 1 commit into from
Mar 10, 2023

Conversation

vbatts
Copy link
Member

@vbatts vbatts commented Feb 12, 2023

Fixes: #1019

Signed-off-by: Vincent Batts [email protected]

@vbatts
Copy link
Member Author

vbatts commented Feb 12, 2023

I'm honestly not sure I've done the base64 definition correct. It's basically only a string.
https://cswr.github.io/JsonSchema/spec/grammar/
We'd have to find a failing test of invalid data to refine it. I was lacking creativity to make that.

@vbatts vbatts force-pushed the fix_descriptor_schema branch from 59e7093 to 2b0a16b Compare February 13, 2023 15:55
@vbatts
Copy link
Member Author

vbatts commented Feb 13, 2023

updated PTAL

tianon
tianon reviewed Feb 13, 2023
View reviewed changes
schema/defs.json
"base64": {
"type": "string",
"media": {
"binaryEncoding": "base64"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be contentEncoding?
(http://json-schema.org/draft/2020-12/json-schema-validation.html#name-contentencoding)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in draft 4 which we are using. See https://json-schema.org/specification-links.html which links to https://datatracker.ietf.org/doc/html/draft-luff-json-hyper-schema-00#section-4.3.1.1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doh, sorry @vbatts 🙃 ❤️

@vbatts vbatts force-pushed the fix_descriptor_schema branch from 2b0a16b to 5f78e4c Compare February 13, 2023 20:03
@vbatts
Copy link
Member Author

vbatts commented Feb 13, 2023

updated PTAL

sajayantony
sajayantony previously approved these changes Feb 13, 2023
View reviewed changes
sudo-bmitch
sudo-bmitch requested changes Mar 2, 2023
View reviewed changes
schema/defs.json Outdated Show resolved Hide resolved
@sudo-bmitch
Copy link
Contributor

Here's a test for invalid base64 (I just removed the required == from the end of the other example):

		{
			descriptor: `
		{
			"mediaType": "text/plain",
			"size": 34,
			"data": "aHR0cHM6Ly9naXRodWIuY29tL29wZW5jb250YWluZXJzCg",
			"digest": "sha256:2690af59371e9eca9453dc29882643f46e5ca47ec2862bd517b5e17351325153"
		}
		`,
			fail: true,
		},

@vbatts vbatts force-pushed the fix_descriptor_schema branch from 5f78e4c to 4136bec Compare March 10, 2023 16:57
@vbatts
Copy link
Member Author

vbatts commented Mar 10, 2023

updated PTAL

@sajayantony sajayantony requested a review from sudo-bmitch March 10, 2023 16:58
sudo-bmitch
sudo-bmitch approved these changes Mar 10, 2023
View reviewed changes
Copy link
Contributor

@sudo-bmitch sudo-bmitch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@sudo-bmitch sudo-bmitch requested a review from sajayantony March 10, 2023 20:19
@sajayantony sajayantony merged commit cc3684d into opencontainers:main Mar 10, 2023
@vbatts vbatts deleted the fix_descriptor_schema branch March 10, 2023 20:24
KilloKoxu pushed a commit to KilloKoxu/image-spec that referenced this pull request Mar 11, 2023
@sajayantony @KilloKoxu
Merge pull request opencontainers#1022 from vbatts/fix_descriptor_schema
f55e368 
descriptor schema: add missing data and artifactType definitions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tianon tianon tianon left review comments

@sudo-bmitch sudo-bmitch sudo-bmitch approved these changes

@sajayantony sajayantony sajayantony approved these changes

@jonjohnsonjr jonjohnsonjr Awaiting requested review from jonjohnsonjr jonjohnsonjr is a code owner

@jonboulle jonboulle Awaiting requested review from jonboulle

@stevvooe stevvooe Awaiting requested review from stevvooe stevvooe is a code owner

@cyphar cyphar Awaiting requested review from cyphar cyphar is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

BUG: schema/ for descriptor is missing data and artifactType
4 participants
@vbatts @sudo-bmitch @tianon @sajayantony