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

Fix duplicate "id" values in JSON schema #750

Merged
merged 2 commits into from
Sep 18, 2018

Conversation

mtrmac
Copy link
Contributor

@mtrmac mtrmac commented Sep 17, 2018

The "id" values in JSON schema files must be unique, per RFC draft 8.3.1:

A schema MAY (and likely will) have multiple URIs, but there is no way for a URI to identify more than one schema.

and recent gojsonschema fails when handling such inputs (fairly nontransparently, it silently fails to resolve $ref references to absolute URIs and reports something like

Reference defs.json#/definitions/mapStringString must be canonical

. This is visible in make test.)

In particular, the https://opencontainers.org/schema/image/descriptor/annotations id value had three definitions. To resolve this:

  • Leave the definition in image-index-schema.json; although using the /descriptor subnamespace for the "manifests" array is a bit surprising, the /image/ part clearly belongs to image-index-schema.json.
  • Rename the id definition in content-descriptor.json, to use the generic "blob descriptor" namespace.
  • Remove the definition in defs-descriptor.json; that seems to be an "utility" schema file describing common structures, but it's better for users to reference schema fragments by purpose than by common structure (so that we can let the structure diverge in the future if necessary).

Finally, changing the content-descriptor.json "id" value changes the resolved absolute value of the reference to defs-descriptor.json, so add another namespace to be handled by fsLoaderFactory.

(As in #739, all I want to do is make the validation work, or, well, not fail. I am expressing no opinion on JSON schema or its use in this project.)

The "id" values in JSON schema files must be unique, per RFC draft 8.3.1:
> A schema MAY (and likely will) have multiple URIs, but there is no
> way for a URI to identify more than one schema.
and recent gojsonschema fails when handling such inputs (fairly
nontransparently, it silently fails to resolve $ref references to
absolute URIs and reports something like
> Reference defs.json#/definitions/mapStringString must be canonical
.)

In particular, the https://opencontainers.org/schema/image/descriptor/annotations
id value had three definitions.  To resolve this:
- Leave the definition in image-index-schema.json; although using the /descriptor
  subnamespace for the "manifests" array is a bit surprising, the /image/ part
  clearly belongs to image-index-schema.json
- Rename the id definition in content-descriptor.json, to use the generic
  "blob descriptor" namespace.
- Remove the definition in defs-descriptor.json; that seems to be an "utility"
  schema file describing common structures, but it's better for users to
  reference schema fragments by purpose than by common structure (so that
  we can let the structure diverge in the future if necessary).

Finally, changing the content-descriptor.json "id" value changes the
resolved absolute value of the reference to defs-descriptor.json,
so add another namespace to be handled by fsLoaderFactory.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
mtrmac added a commit to mtrmac/skopeo that referenced this pull request Sep 17, 2018
Temporarily vendor opencontainers/image-spec from a fork
to fix "id" value duplication, which is detected and
refused by gojsonschema now
( opencontainers/image-spec#750 ).

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@runcom
Copy link
Member

runcom commented Sep 17, 2018

👍

@cyphar
Copy link
Member

cyphar commented Sep 17, 2018

LGTM.

Approved with PullApprove

@cyphar cyphar merged commit cf56476 into opencontainers:master Sep 18, 2018
cyphar added a commit that referenced this pull request Sep 18, 2018
  Run (make schema/fs.go) to make the previous commit effective
  Fix duplicate "id" values in JSON schema

LGTMs: @jonboulle @cyphar
Closes #750
mtrmac added a commit to mtrmac/skopeo that referenced this pull request Sep 29, 2018
... after opencontainers/image-spec#750 was merged.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
mtrmac added a commit to mtrmac/skopeo that referenced this pull request Oct 1, 2018
... after opencontainers/image-spec#750 was merged.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
mtrmac added a commit to mtrmac/skopeo that referenced this pull request Oct 20, 2018
... after opencontainers/image-spec#750 was merged.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
mtrmac added a commit to mtrmac/skopeo that referenced this pull request Nov 3, 2018
... after opencontainers/image-spec#750 was merged.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
mtrmac added a commit to mtrmac/skopeo that referenced this pull request Nov 3, 2018
... after opencontainers/image-spec#750 was merged.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
mtrmac added a commit to mtrmac/skopeo that referenced this pull request Nov 8, 2018
... after opencontainers/image-spec#750 was merged.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
mtrmac added a commit to mtrmac/skopeo that referenced this pull request Nov 29, 2018
... after opencontainers/image-spec#750 was merged.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
lsm5 pushed a commit to lsm5/skopeo that referenced this pull request Apr 11, 2019
... after opencontainers/image-spec#750 was merged.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@mtrmac mtrmac deleted the unique-ids branch September 26, 2019 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants