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

config: extract config structs to its own pkg #3228

Merged
merged 3 commits into from
Jun 26, 2023

Conversation

geyslan
Copy link
Member

@geyslan geyslan commented Jun 12, 2023

1. Explain what the PR does

Fix: #3031

1f91b80 config: move pcaps Config struct to pkg/config (2023/jun/13) Geyslan Gregório <geyslan@gmail.com>

It names the struct `PcapsConfig` and moves it to pkg/config.

ec36fe9 flags: rename OutputConfig to PrepareOutputResult (2023/jun/13) Geyslan Gregório <geyslan@gmail.com>

0cb1395 config: extract config structs to its own pkg (2023/jun/13) Geyslan Gregório <geyslan@gmail.com>

This extracts the config file from the ebpf package to its own.
It also moves into the config package PrinterConfig and
flags/OutputConfig renaming the latter to PrinterOutputConfig.

2. Explain how to test it

3. Other comments

pkg/config/config.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@josedonizetti josedonizetti left a comment

Choose a reason for hiding this comment

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

@geyslan why didn't we also bring the engine.Config, since all configs will be under pkg/config why not the engine as well?

@geyslan
Copy link
Member Author

geyslan commented Jun 13, 2023

@geyslan why didn't we also bring the engine.Config, since all configs will be under pkg/config why not the engine as well?

I think it makes sense. Besides engine we have also pcaps.

@geyslan
Copy link
Member Author

geyslan commented Jun 14, 2023

Bringing the engine configuration into pkg/config made the tracee-rules now dependent on pkg/config. So this build change is required:

https://github.com/aquasecurity/tracee/pull/3228/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R498-R503


For signatures as well. So, @rafaeldtinoco and @NDStrahilevitz, is there some impact of depending (in the build process) on the libbpf in signatures and tracee-rules - due to that type relocation (Config).

@geyslan
Copy link
Member Author

geyslan commented Jun 14, 2023

Bringing the engine configuration into pkg/config made the tracee-rules now dependent on pkg/config. So this build change is required:

https://github.com/aquasecurity/tracee/pull/3228/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R498-R503

For signatures as well. So, @rafaeldtinoco and @NDStrahilevitz, is there some impact of depending (in the build process) on the libbpf in signatures and tracee-rules - due to that type relocation (Config).

@josedonizetti I'm going to drop that engine config relocation commit. Currently, it would create dependencies not really needed. We'll come back to that when we get rid of the rules bin.

This extracts the config file from the ebpf package to its own.
It also moves into the config package PrinterConfig and
flags/OutputConfig renaming the latter to PrinterOutputConfig.
It names the struct `PcapsConfig` and moves it to pkg/config.
Copy link
Contributor

@rafaeldtinoco rafaeldtinoco left a comment

Choose a reason for hiding this comment

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

I agree on not making tracee-rules depending on libbpf just now (add a TODO for when we have the single binary if some other change is needed). LGTM

@rafaeldtinoco rafaeldtinoco merged commit 43f97c8 into aquasecurity:main Jun 26, 2023
25 checks passed
@geyslan geyslan deleted the 3031-extract-config branch July 31, 2023 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extract config package from cmd and tracee packages
3 participants