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

oci/build: Specify ArtifactType in the correct place #2572

Merged
merged 1 commit into from
Mar 5, 2024

Conversation

blanquicet
Copy link
Member

Specify ArtifactType in the correct place when building the image

As discussed in #2177, the ArtifactType must be specified when config.mediaType is set MediaTypeEmptyJSON. In our case, when the metadata file is not provided. However, such PR added the ArtifactType inside the config and not to the general manifest as described in [1].

Fixes: 1b04865

[1] https://github.com/opencontainers/image-spec/blob/f5f87016de46439ccf91b5381cf76faaae2bc28f/manifest.md?plain=1#L210

Before this PR

$ IG_EXPERIMENTAL=true go run --exec "sudo -E" ../../cmd/ig/... image build -t ghcr.io/blanquicet/gadget/trace_exec:latest .
$ IG_EXPERIMENTAL=true go run --exec "sudo -E" ../../cmd/ig/... image push ghcr.io/blanquicet/gadget/trace_exec:latest
$ crane manifest ghcr.io/blanquicet/gadget/trace_exec:latest | jq .
... Get SHA of one of the manifests ...
$ crane manifest ghcr.io/blanquicet/gadget/trace_exec:latest@sha256:ec03ea97b5b0368a823a472f54fcbbf07312501b8b298a54ca9f1565f73b50c1 | jq .
{
  "schemaVersion": 2,
  "config": {
    "mediaType": "application/vnd.oci.empty.v1+json",
    "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
    "size": 2,
    "data": "e30=",
    "artifactType": "application/vnd.gadget.ebpf.program.v1+binary"
  },
  "layers": [
    {
      "mediaType": "application/vnd.gadget.ebpf.program.v1+binary",
      "digest": "sha256:2f16368166aee02a9a44768abe85dec3b0340392927e4a1f55269df09ea0c87b",
      "size": 86904
    }
  ]
}

After this PR

The new result now looks like the example provided by image-spec:

$ crane manifest ghcr.io/blanquicet/gadget/trace_exec:latest@sha256:ec03ea97b5b0368a823a472f54fcbbf07312501b8b298a54ca9f1565f73b50c1 | jq .
{
  "schemaVersion": 2,
  "artifactType": "application/vnd.gadget.ebpf.program.v1+binary",
  "config": {
    "mediaType": "application/vnd.oci.empty.v1+json",
    "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
    "size": 2,
    "data": "e30="
  },
  "layers": [
    {
      "mediaType": "application/vnd.gadget.ebpf.program.v1+binary",
      "digest": "sha256:e0725896ea02f1ff09e58b85d55de0453dcfcef94b7c51e907c54faa35df60cf",
      "size": 149960
    }
  ]
}

As discussed in
#2177, the
ArtifactType must be specified when config.mediaType is set
MediaTypeEmptyJSON. In our case, when the metadata file is not
provided. However, such PR added the ArtifactType inside the config and
not to the general manifest as described in [1].

Fixes: 1b04865

[1] https://github.com/opencontainers/image-spec/blob/f5f87016de46439ccf91b5381cf76faaae2bc28f/manifest.md?plain=1#L210

Signed-off-by: Jose Blanquicet <josebl@microsoft.com>
Copy link
Member

@mqasimsarfraz mqasimsarfraz left a comment

Choose a reason for hiding this comment

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

Great Catch! Out of curiosity how did you catch this? Perhaps we can consider to create tests for our manifest validation in future.

Tested with steps mentioned in #2177 and no problems in pushing the image as well.

@mauriciovasquezbernal
Copy link
Member

Does it solve #2289?

@blanquicet
Copy link
Member Author

Great Catch! Out of curiosity how did you catch this?

While reviewing #2512 (comment), I noticed it was looking at the ArtifactType and I noticed there was a "general" ArtifactType for the whole manifest and another for the config.

Perhaps we can consider to create tests for our manifest validation in future.

Agree! #2577

Tested with steps mentioned in #2177 and no problems in pushing the image as well.

Thanks!

@mqasimsarfraz
Copy link
Member

Does it solve #2289?

Things looking better now:

→ go run -exec "sudo -E" ./cmd/ig image push qasimregistry.azurecr.io/gadget/trace_open:latest
INFO[0000] Experimental features enabled                
Pushing qasimregistry.azurecr.io/gadget/trace_open:latest...
Successfully pushed qasimregistry.azurecr.io/gadget/trace_open:latest@sha256:b78f41657ea1e0492122fba1898728f19a2610cb7cff93b67009610ba41407ff
→ crane manifest qasimregistry.azurecr.io/gadget/trace_open:latest@sha256:aad3988d194c1fd90b7fe85d46cadbb8aa722ca6a251d53d90f9fe32f4c6a3d3 | jq .
{
  "schemaVersion": 2,
  "artifactType": "application/vnd.gadget.ebpf.program.v1+binary",
  "config": {
    "mediaType": "application/vnd.oci.empty.v1+json",
    "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
    "size": 2,
    "data": "e30="
  },
  "layers": [
    {
      "mediaType": "application/vnd.gadget.ebpf.program.v1+binary",
      "digest": "sha256:d6b7f6f3b13b12b50b2fd91f9d81c1df4c295fcf4701c130be3e90a324fa83d3",
      "size": 88408
    }
  ]
}

@blanquicet
Copy link
Member Author

blanquicet commented Mar 5, 2024

Does it solve #2289?

Yep. It does! I test it with trace_open in my ACR registry:

jose trace_open git:(main *)$ ls
program.bpf.c

Before this PR

jose trace_open git:(main *)$ IG_EXPERIMENTAL=true go run --exec "sudo -E" ../../cmd/ig/... image build -t joseregistry.azurecr.io/trace_open:latest .
INFO[0000] Experimental features enabled
Successfully built joseregistry.azurecr.io/trace_open:latest@sha256:4dd9d2a330a85d15eb69b75a261dda17045d5c3fb68287e16030ba429af779b6

jose trace_open git:(main *)$ IG_EXPERIMENTAL=true go run --exec "sudo -E" ../../cmd/ig/... image push joseregistry.azurecr.io/trace_open:latest
INFO[0000] Experimental features enabled
Pushing joseregistry.azurecr.io/trace_open:latest...
Error: pushing gadget: copying to remote repository: PUT "https://joseregistry.azurecr.io/v2/trace_open/manifests/sha256:5b0c1ddffd0b687226fc850a1a3b3d514310f4ebbc7e0d148e9ce35c4beeb831": response status code 400: manifest invalid: manifest invalid: map[Reason:using empty config descriptor requires artifactType to be defined]
exit status 1

After this PR

jose trace_open git:(jose/empty-descriptor *)$ IG_EXPERIMENTAL=true go run --exec "sudo -E" ../../cmd/ig/... image build -t joseregistry.azurecr.io/trace_open:latest .
INFO[0000] Experimental features enabled
Successfully built 
joseregistry.azurecr.io/trace_open:latest@sha256:b78f41657ea1e0492122fba1898728f19a2610cb7cff93b67009610ba41407ff

jose trace_open git:(jose/empty-descriptor *)$ IG_EXPERIMENTAL=true go run --exec "sudo -E" ../../cmd/ig/... image push joseregistry.azurecr.io/trace_open:latest
INFO[0000] Experimental features enabled
Pushing joseregistry.azurecr.io/trace_open:latest...
Successfully pushed joseregistry.azurecr.io/trace_open:latest@sha256:b78f41657ea1e0492122fba1898728f19a2610cb7cff93b67009610ba41407ff

@blanquicet blanquicet merged commit 3c78efb into main Mar 5, 2024
59 checks passed
@blanquicet blanquicet deleted the jose/empty-descriptor branch March 5, 2024 15:19
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.

Cannot push gadgets without metadata: "ig image push" fails with "manifest invalid"
3 participants