API: Support running gadgets from files and memory #2853

mauriciovasquezbernal · 2024-05-15T21:18:48Z

This PR changes some interfaces and functions to allow running a gadget from a file or from memory and implements two examples showing it.

TODO

How to avoid the github.com/quay/claircore dependency being propagated to our go.mod

alban · 2024-05-16T09:09:15Z

Thanks for working on this!

How to avoid the github.com/quay/claircore dependency being propagated to our go.mod

Can you have another set of go.mod/go.sum in the examples/gadgets/simple/from_file/ and examples/gadgets/simple/from_memory/ sub-directories?

flyth · 2024-05-16T09:22:10Z

Should we also give the option to push the binary using gRPC?
I think that would be helpful especially for debugging purposes without having to go through external registries. It should be disabled by default server-side, though.

alban · 2024-05-16T10:43:01Z

pkg/operators/oci-handler/oci.go

+		}
+
+		// Make sure the image is available, either through pulling or by just accessing a local copy
+		// TODO: add security constraints (e.g. don't allow pulling - add GlobalParams for that)


Could you do it in the same PR?

This comment was already there. Code was moved into a new conditional.

Added some prerequisites for that to #2901 and #2902 - will follow up with a PR moving the settings to global once that is merged. Then we can tighten it even more.

alban · 2024-05-16T10:46:05Z

Should we also give the option to push the binary using gRPC? I think that would be helpful especially for debugging purposes without having to go through external registries.

Why not, but in a separate PR.

It should be disabled by default server-side, though.

Why? I think options to enable/disable it is good. But I don't think it is necessarily less secure than downloading from a OCI registry. That depends on the context and if users verify the image signature.

flyth · 2024-05-16T11:01:07Z

Should we also give the option to push the binary using gRPC? I think that would be helpful especially for debugging purposes without having to go through external registries.

Why not, but in a separate PR.

It should be disabled by default server-side, though.

Why? I think options to enable/disable it is good. But I don't think it is necessarily less secure than downloading from a OCI registry. That depends on the context and if users verify the image signature.

Agree with both, just wanted to start the discussion to keep it in mind when developing this further.

I also don't think it's less secure per se if it runs through all checks as well; still there are certain things we need to consider, which is why I'd suggest to keep it disabled by default:

If you'd for example bundle just a couple of core gadgets with your IG installation, you'd keep signature checking on (by default) and deploy it. You'd probably not want for IG to be able to run any other non-bundled (but signed) core gadget using the gRPC way, because it could leak private information or whatnot (you actively chose to not bundle it in the first place).

The more use cases we add, the more complex the security situation becomes (edge cases, unwanted behavior) - and thus IMHO we should always be more restrictive than permissive by default so that the admin actively has to enable features like this after careful consideration.

mauriciovasquezbernal · 2024-05-22T21:53:47Z

Can you have another set of go.mod/go.sum in the examples/gadgets/simple/from_file/ and examples/gadgets/simple/from_memory/ sub-directories?

Added a single pair on examples/. I also needed to add a go.work file.

mauriciovasquezbernal · 2024-05-22T21:55:59Z

Marking as ready. I don't have anything against allowing something similar on the gRPC interface, but IMO it doesn't belong to this PR and the current implementation doesn't collides with that. Any thoughts?

eiffel-fl

Hi!

I just have comments on the added tar archive.
I will take another thorough look at the API change itself, because I guess I do not catch stuff as I lack knowledge on the API design.

Best regards.

eiffel-fl · 2024-05-27T08:51:12Z

examples/gadgets/simple/from_file/main.go

+	// To create this tarball run:
+	// $ sudo ig image pull trace_open:latest
+	// $ sudo ig image export trace_open:latest /tmp/trace_open.tar
+	ociStore, err := orasoci.NewFromTar(ctx, "trace_open.tar")


Anyway to rather create the tarball here or in the CI?
I would like to avoid adding as much as possible binaries.

My idea is to create an example that can be compiled without taking any additional steps. I don't think there is another possibility than storing the gadget image in the repo (the same we're doing for built-in gadgets).

eiffel-fl · 2024-05-27T08:52:21Z

examples/gadgets/simple/from_memory/README.md

+This example shows how a gadget image can be embedded in the application binary
+and how it can be run from there.
+
+The `trace_open.tar` file was created with:


Same comment.

eiffel-fl · 2024-05-27T08:56:18Z

The more use cases we add, the more complex the security situation becomes (edge cases, unwanted behavior) - and thus IMHO we should always be more restrictive than permissive by default so that the admin actively has to enable features like this after careful consideration.

The more use cases we add, the more complex the situation becomes and the easier it becomes to do mistake particularly with regard to security.
I am not against this use case, but as you pointed it should come with big consideration.
Also, I would not implement for now as long as we do not have a request to do so.

flyth

Looks really good! Nice to have this possibility as well!

pkg/oci/oci.go

flyth · 2024-05-27T17:10:22Z

pkg/operators/oci-handler/oci.go

+		}
+
+		// Make sure the image is available, either through pulling or by just accessing a local copy
+		// TODO: add security constraints (e.g. don't allow pulling - add GlobalParams for that)


Added some prerequisites for that to #2901 and #2902 - will follow up with a PR moving the settings to global once that is merged. Then we can tighten it even more.

This commit updates few interfaces and functions signatures to allow to run gadgets from a file or from memory when using the Golang API. Specifically: - Update the gadget context to allow passing an oras target. - Update GetManifestForHost() and GetContentFromDescriptor() to take the target instead of always using the local one store. Signed-off-by: Mauricio Vásquez <mauriciov@microsoft.com>

This avoids dependencies of the examples being propagated to the main go.mod file. This commit also adds a go.work file that forces the examples to use the current tree module of inspektor gadget instead of the version defined in their go.mod file. Signed-off-by: Mauricio Vásquez <mauriciov@microsoft.com>

Signed-off-by: Mauricio Vásquez <mauriciov@microsoft.com>

burak-ok

LGTM,

I had thought that this PR would additionally add a flag like --from-file to ig run to directly run from a tarball.
For ig there is no problem, but as written previously by others, for kubectl-gadget we would need to push the archive/eBPF program through gRPC

mauriciovasquezbernal · 2024-05-28T21:24:24Z

I had thought that this PR would additionally add a flag like --from-file to ig run to directly run from a tarball.

I initially implemented it that way, but then I realized it doesn't make sense now that we have the image export and import commands. We can discuss later on how to handle this for the kubectl-gadget case.

Thanks for the review.

mauriciovasquezbernal mentioned this pull request May 15, 2024

Introduce initial wasm support #2818

Open

11 tasks

alban reviewed May 16, 2024

View reviewed changes

mauriciovasquezbernal force-pushed the mauricio/gadgets-examples branch 3 times, most recently from 18aee98 to 43bb599 Compare May 22, 2024 20:35

Base automatically changed from mauricio/gadgets-examples to main May 22, 2024 20:55

mauriciovasquezbernal force-pushed the mauricio/run-from-file-2 branch from 68f5696 to 3c70086 Compare May 22, 2024 21:53

mauriciovasquezbernal marked this pull request as ready for review May 22, 2024 21:55

mauriciovasquezbernal requested a review from flyth as a code owner May 22, 2024 21:55

mauriciovasquezbernal force-pushed the mauricio/run-from-file-2 branch from 3c70086 to 86e6381 Compare May 23, 2024 15:03

burak-ok self-requested a review May 24, 2024 14:15

eiffel-fl reviewed May 27, 2024

View reviewed changes

flyth approved these changes May 27, 2024

View reviewed changes

mauriciovasquezbernal added 4 commits May 27, 2024 13:26

examples: Run a gadget from a tarball

9955ff2

Signed-off-by: Mauricio Vásquez <mauriciov@microsoft.com>

examples: Embed a gadget within the application

bff4dcc

Signed-off-by: Mauricio Vásquez <mauriciov@microsoft.com>

mauriciovasquezbernal force-pushed the mauricio/run-from-file-2 branch from 86e6381 to bff4dcc Compare May 27, 2024 18:32

mauriciovasquezbernal mentioned this pull request May 27, 2024

cmd/image/export: Allow generating deterministic tarballs #2904

Open

burak-ok approved these changes May 28, 2024

View reviewed changes

mauriciovasquezbernal merged commit a4a6391 into main May 28, 2024
59 of 60 checks passed

mauriciovasquezbernal deleted the mauricio/run-from-file-2 branch May 28, 2024 21:23

eiffel-fl mentioned this pull request May 29, 2024

tools/ekscleanup: Use go.work. #2910

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API: Support running gadgets from files and memory #2853

API: Support running gadgets from files and memory #2853

mauriciovasquezbernal commented May 15, 2024 •

edited

alban commented May 16, 2024

flyth commented May 16, 2024

alban May 16, 2024

mauriciovasquezbernal May 22, 2024

flyth May 27, 2024 •

edited

alban commented May 16, 2024

flyth commented May 16, 2024

mauriciovasquezbernal commented May 22, 2024

mauriciovasquezbernal commented May 22, 2024

eiffel-fl left a comment

eiffel-fl May 27, 2024

mauriciovasquezbernal May 27, 2024

eiffel-fl May 27, 2024

eiffel-fl commented May 27, 2024

flyth left a comment

flyth May 27, 2024 •

edited

burak-ok left a comment

mauriciovasquezbernal commented May 28, 2024

API: Support running gadgets from files and memory #2853

API: Support running gadgets from files and memory #2853

Conversation

mauriciovasquezbernal commented May 15, 2024 • edited

TODO

alban commented May 16, 2024

flyth commented May 16, 2024

alban May 16, 2024

Choose a reason for hiding this comment

mauriciovasquezbernal May 22, 2024

Choose a reason for hiding this comment

flyth May 27, 2024 • edited

Choose a reason for hiding this comment

alban commented May 16, 2024

flyth commented May 16, 2024

mauriciovasquezbernal commented May 22, 2024

mauriciovasquezbernal commented May 22, 2024

eiffel-fl left a comment

Choose a reason for hiding this comment

eiffel-fl May 27, 2024

Choose a reason for hiding this comment

mauriciovasquezbernal May 27, 2024

Choose a reason for hiding this comment

eiffel-fl May 27, 2024

Choose a reason for hiding this comment

eiffel-fl commented May 27, 2024

flyth left a comment

Choose a reason for hiding this comment

flyth May 27, 2024 • edited

Choose a reason for hiding this comment

burak-ok left a comment

Choose a reason for hiding this comment

mauriciovasquezbernal commented May 28, 2024

mauriciovasquezbernal commented May 15, 2024 •

edited

flyth May 27, 2024 •

edited

flyth May 27, 2024 •

edited