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

chore(src): improve containerd support. #10

Merged
merged 7 commits into from
Jan 17, 2025

Conversation

FedeDP
Copy link
Owner

@FedeDP FedeDP commented Jan 8, 2025

Backport changes from falcosecurity/libs#2195.

TODO:

Updated TODO with more info.

@FedeDP FedeDP force-pushed the chore/containerd_improve_support branch from a471916 to 8c99b3f Compare January 9, 2025 11:59
@@ -163,7 +163,7 @@ load_plugins: [container]
By default, all engines are enabled on **default sockets**:
* Docker: `/var/run/docker.sock`
* Podman: `/run/podman/podman.sock` for root, + `/run/user/$uid/podman/podman.sock` for each user in the system
* Containerd: [`/run/containerd/containerd.sock`, `/run/k3s/containerd/containerd.sock`]
* Containerd: [`/run/containerd/containerd.sock`, `/run/k3s/containerd/containerd.sock`, `/run/host-containerd/containerd.sock`]
Copy link
Owner Author

Choose a reason for hiding this comment

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

New bottlerocket related socket.

@@ -80,59 +82,61 @@ func (c *containerdEngine) ctrToInfo(namespacedContext context.Context, containe
}
}

// Mounts related - TODO double check
// Mounts related
Copy link
Owner Author

Choose a reason for hiding this comment

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

Fixed.

RW: !readOnly,
Propagation: spec.Linux.RootfsPropagation,
})
}

// Namespace related - FIXME
// Namespace related - see oci.WithHostNamespace() impl: it just removes the namespace from the list
Copy link
Owner Author

Choose a reason for hiding this comment

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

Fixed.

Comment on lines +137 to +138
imageDigest = image.Target().Digest.String()
if config.GetWithSize() {
size, _ = image.Size(context.TODO())
imageSize = image.Target().Size
Copy link
Owner Author

Choose a reason for hiding this comment

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

Fixed.

@@ -166,18 +170,39 @@ func (c *containerdEngine) ctrToInfo(namespacedContext context.Context, containe
}
}

// Check for privileged:
// see https://github.com/containerd/containerd/blob/main/pkg/oci/spec_opts.go#L1295
privileged := true
Copy link
Owner Author

Choose a reason for hiding this comment

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

Fixed; a bit ugly but couldn't find a better solution.

return event.Info{
Container: event.Container{
Type: typeContainerd.ToCTValue(),
ID: container.ID()[:shortIDLength],
Name: container.ID()[:shortIDLength],
ID: shortContainerID(container.ID()),
Copy link
Owner Author

Choose a reason for hiding this comment

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

Since containerd supports container_id of any length, properly cut short ID to 12 only if it is longer than 12, to avoid crashes

ImageRepo: imageRepo,
ImageTag: imageTag,
User: spec.Process.User.Username,
CniJson: "", // TODO
User: strconv.FormatUint(uint64(spec.Process.User.UID), 10),
Copy link
Owner Author

Choose a reason for hiding this comment

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

Fixed; we use uid.

// sha256:digest
// See https://github.com/therealbobo/libs/blob/8267fbb909167541c7f7ed655c93a7dc0c1d615b/userspace/libsinsp/cri.hpp#L320
// for the original c++ implementation.
imageName := ctr.GetImage().GetImage()
Copy link
Owner Author

Choose a reason for hiding this comment

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

Copied current libsinsp impl.

ImageRepo: imageRepo,
ImageTag: imageTag,
User: user.String(),
User: strconv.FormatInt(ctr.GetUser().GetLinux().GetUid(), 10),
Copy link
Owner Author

Choose a reason for hiding this comment

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

We use uid here.

@@ -14,6 +15,191 @@ import (
"time"
)

func TestCRIInfoMap(t *testing.T) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

Test that we are able to unmarshal the json to our struct.

@FedeDP FedeDP force-pushed the chore/containerd_improve_support branch from 8c99b3f to b62bf68 Compare January 9, 2025 12:05
FedeDP added 3 commits January 9, 2025 13:59
…raction.

Moreover, added tests around containerd matcher.

Signed-off-by: Federico Di Pierro <[email protected]>
Signed-off-by: Federico Di Pierro <[email protected]>
…support for docker and podman.

Signed-off-by: Federico Di Pierro <[email protected]>
@FedeDP FedeDP force-pushed the chore/containerd_improve_support branch from 3a55b0f to 603e03e Compare January 16, 2025 13:17
@FedeDP FedeDP merged commit 6d48ad7 into main Jan 17, 2025
1 check passed
@FedeDP FedeDP deleted the chore/containerd_improve_support branch January 17, 2025 10:25
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.

1 participant