New 'node-agent' that listens to pod events and loads eBPF programs #224

orishavit · 2024-07-11T14:05:11Z

Please see the contributing guidelines for how to create and submit a high-quality PR for this repo. This template is based on Auth0's excellent template.

Description

Describe the purpose of this PR along with any background information and the impacts of the proposed change. For the benefit of the community, please do not assume prior context.

Provide details that support your chosen implementation, including: breaking changes, alternatives considered, changes to the API, etc.

References

Include any links supporting this change such as a:

GitHub Issue/PR number addressed or fixed
StackOverflow post
Related pull requests/issues from other repos

If there are no references, simply delete this section.

Testing

Describe how this can be tested by reviewers. Be specific about anything not tested and reasons why. If this library has unit and/or integration testing, tests should be added for new functionality and existing tests should complete without errors.

Please include any manual steps for testing end-to-end or functionality not covered by unit/integration tests.

Also include details of the environment this PR was developed in (language/platform/browser version).

This change adds test coverage for new/changed/fixed functionality

Checklist

I have added documentation for new/changed functionality in this PR and in github.com/otterize/docs

src/shared/kubeutils/labels.go

src/node-agent/pkg/reconcilers/ebpfreconciler.go

build/agent.Dockerfile

…ts-operator

Co-authored-by: Ori Shavit <[email protected]>

- only handle pods on the same node - fix a crash where accessing a static var before it is initialized

amitlicht · 2024-09-23T05:10:03Z

build/bpfman.Dockerfile

@@ -0,0 +1,20 @@
+FROM --platform=$TARGETPLATFORM golang:1.22.1 AS ebpf-buildenv


do we still need this? it's not referenced anywhere and AFAIU bpfman is no longer in use

amitlicht · 2024-09-23T05:17:20Z

src/bintools/elf.go

+	var sectionsToSearchForSymbol []*elf.Section
+
+	for i := range f.Sections {
+		if f.Sections[i].Flags == elf.SHF_ALLOC+elf.SHF_EXECINSTR {
+			sectionsToSearchForSymbol = append(sectionsToSearchForSymbol, f.Sections[i])
+		}
+	}
+
+	if len(sectionsToSearchForSymbol) == 0 {
+		return 0, fmt.Errorf("symbol %q not found in file - no sections to search", symbol)
+	}


consider splitting this to a dedicated function getSectionsToSearchForSymbol(f) ([]*elf.Section) for readability and to shorten SymbolToOffset.

amitlicht · 2024-09-23T05:18:51Z

src/bintools/elf.go

+	var executableSection *elf.Section
+
+	// Find what section the symbol is in by checking the executable section's addr space.
+	for m := range sectionsToSearchForSymbol {
+		sectionStart := sectionsToSearchForSymbol[m].Addr
+		sectionEnd := sectionStart + sectionsToSearchForSymbol[m].Size
+		if symbol.Value >= sectionStart && symbol.Value < sectionEnd {
+			executableSection = sectionsToSearchForSymbol[m]
+			break
+		}
+	}
+
+	if executableSection == nil {
+		return 0, errors.New("could not find symbol in executable sections of binary")
+	}


same here - consider splitting to a dedicated function findExecutableSection(sectionsToSearchForSymbol, symbol) *elf.Section.
This will also help with replacing the break with a return (which will help readability).

amitlicht · 2024-09-23T05:20:16Z

src/bintools/gobin/types.go

+	"github.com/otterize/network-mapper/src/bintools/bininfo"
+)
+
+// FunctionMetadata  used to attach a uprobe to a function.


Suggested change

// FunctionMetadata used to attach a uprobe to a function.

// FunctionMetadata used to attach a uprobe to a function.

amitlicht · 2024-09-23T05:21:13Z

src/bintools/process.go

+		if cErr == nil {
+			err = cErr
+		}


would you really want to fail the entire flow if Close() failed? perhaps just log and continue

amitlicht · 2024-09-23T05:50:02Z

src/node-agent/pkg/ebpf/eventreader.go

+	}, nil
+}
+
+func (e *EventReader) Start() {


Consider using context.Context to be able to trigger graceful shutdown here.

amitlicht · 2024-09-23T06:09:19Z

src/node-agent/pkg/reconcilers/ebpfreconciler.go

+	if containerInfo.ExecutableInfo.Language == bininfo.SourceLanguageNodeJs {
+		err := r.tracer.AttachToOpenSSL(containerInfo)
+		if err != nil {
+			return errors.Wrap(err)
+		}
+	} else if containerInfo.ExecutableInfo.Language == bininfo.SourceLanguageGoLang {
+		err := r.tracer.AttachToGoTls(containerInfo)
+		if err != nil {
+			return errors.Wrap(err)
+		}
+	}


consider using switch containerInfo.ExecutableInfo.Language, which is more idiomatic here.

amitlicht · 2024-09-23T06:13:17Z

src/node-agent/pkg/service/env.go

+	printHttpRequests = false
+)
+
+func init() {


we typically use viper rather than raw env vars, which provides better control over argument types etc. Consider switching to it, both for consistency with other portions of our application, and for better code. you can take a look at the sniffer code for a relatively simple example.

amitlicht · 2024-09-23T06:15:17Z

src/shared/config/config.go


 	EnvPodKey       = "pod"
 	EnvNamespaceKey = "namespace"

+	// experimental features
+
+	EnableEBPFKey     = "experimental-ebpf"


should be called enable-ebpf (the experimental portion can go into description), or enable-ebpf-experimental if you really insist on including the experimental part in the flag name.

amitlicht · 2024-09-23T06:17:42Z

src/sniffer/cmd/main.go

@@ -37,6 +37,7 @@ func main() {
 	})
 	errgrp, errGroupCtx := errgroup.WithContext(signals.SetupSignalHandler())
 	clusterUID := clusterutils.GetOrCreateClusterUID(errGroupCtx)
+


revert this

orishavit force-pushed the shavit/node-agent-ebpf branch 15 times, most recently from b751a18 to df51970 Compare July 17, 2024 13:41

otterobert requested changes Aug 5, 2024

View reviewed changes

src/shared/kubeutils/labels.go Show resolved Hide resolved

src/node-agent/pkg/reconcilers/ebpfreconciler.go Outdated Show resolved Hide resolved

src/node-agent/pkg/reconcilers/ebpfreconciler.go Show resolved Hide resolved

orishavit commented Aug 7, 2024

View reviewed changes

build/agent.Dockerfile Show resolved Hide resolved

orishavit and others added 13 commits August 19, 2024 21:48

New 'node-agent' that listens to pod events and loads eBPF programs

74da47b

fix go generate

e08e1d7

copy RegisterPanicHandler to handle incompatbility with libs in inten…

be7dd6c

…ts-operator

revert go tools changes

6f6338c

vendor cri-client to fix dependency incompat with intents-op

0705d75

containerInfo

d61a0e4

logging

55cc600

ignore linter in vendored line

8810404

cross platform builds

6f32e9d

SSL_write_exit

5280182

fixes

b09e0a4

Add makefile to automate local ebpf development (#233)

8b522e5

Co-authored-by: Ori Shavit <[email protected]>

fix makefile

8b0e770

otterobert and others added 9 commits September 1, 2024 11:36

Added nodejs support

384a912

Integrate gotls into agent (#236)

2fb2a52

Autogenerate vmlinux.h file on build time

1b5bc43

use otterize/iamlive#main

6ae8aeb

Merge remote-tracking branch 'origin/main' into shavit/node-agent-ebpf

698ab5a

generate vmlinux.h in generate.go

8fa87b0

fix linter errors

bbb9400

TLS event filtering (#237)

2832c74

Add sensitive traffic detection (#239)

e2cea47

orishavit force-pushed the shavit/node-agent-ebpf branch 2 times, most recently from 438e380 to e2cea47 Compare September 10, 2024 00:57

orishavit added 2 commits September 11, 2024 17:15

add multi-arch machine to Makefile

7b144f3

delete ebpf side filters

25a59de

orishavit force-pushed the shavit/node-agent-ebpf branch 2 times, most recently from df7bcf4 to 4b891d7 Compare September 11, 2024 16:15

add aws action to Makefile

74d471c

orishavit force-pushed the shavit/node-agent-ebpf branch from 4b891d7 to 74d471c Compare September 11, 2024 17:42

only build node agent on amd64

554bbf6

orishavit force-pushed the shavit/node-agent-ebpf branch from f929652 to 554bbf6 Compare September 11, 2024 19:03

clean unwanted diff from main

efff59c

orishavit force-pushed the shavit/node-agent-ebpf branch from 564e82f to efff59c Compare September 12, 2024 11:47

do not print full http requests

cdaafb9

amitlicht self-requested a review September 16, 2024 11:20

orishavit added 3 commits September 17, 2024 17:12

feature flag

f968856

handle multiple possible cri socket locations

3b19f6c

bug fixes:

3461594

- only handle pods on the same node - fix a crash where accessing a static var before it is initialized

orishavit marked this pull request as ready for review September 19, 2024 16:04

amitlicht requested changes Sep 23, 2024

View reviewed changes

amitlicht closed this Oct 6, 2024

github-actions bot locked and limited conversation to collaborators Oct 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New 'node-agent' that listens to pod events and loads eBPF programs #224

New 'node-agent' that listens to pod events and loads eBPF programs #224

orishavit commented Jul 11, 2024

amitlicht Sep 23, 2024

amitlicht Sep 23, 2024

amitlicht Sep 23, 2024

amitlicht Sep 23, 2024

amitlicht Sep 23, 2024

amitlicht Sep 23, 2024

amitlicht Sep 23, 2024

amitlicht Sep 23, 2024

amitlicht Sep 23, 2024

amitlicht Sep 23, 2024

		@@ -0,0 +1,20 @@
		FROM --platform=$TARGETPLATFORM golang:1.22.1 AS ebpf-buildenv

	// FunctionMetadata used to attach a uprobe to a function.
	// FunctionMetadata used to attach a uprobe to a function.

New 'node-agent' that listens to pod events and loads eBPF programs #224

New 'node-agent' that listens to pod events and loads eBPF programs #224

Conversation

orishavit commented Jul 11, 2024

Description

References

Testing

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment