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

Add http-headers as flag to docker-entrypoint.sh of driver-loader, legacy and falco #3070

Closed
toamto94 opened this issue Feb 10, 2024 · 19 comments · Fixed by #3075
Closed

Add http-headers as flag to docker-entrypoint.sh of driver-loader, legacy and falco #3070

toamto94 opened this issue Feb 10, 2024 · 19 comments · Fixed by #3075
Milestone

Comments

@toamto94
Copy link
Contributor

Motivation
http-headers has been added to the falcoctl driver install command. Since this tool is used for the driver compile/download in the falco, driver-loader and legacy image, it should be added to the respective docker entrypoints scripts. Otherwise there would be no possibilty to use this from a containerised deployment.

Feature
Add http-headers as an additional flag of the docker-entrypoint script of driver-loader, legacy driver-loader and falco. It will be forwarded to the falcoctl driver install command in the end of the day

Alternatives
There is no alternative implementation about how http-headers could be enabled.

Additional context

http-headers has been recently added to falcoctl so it must be made sure that version is used in the driver-loader, legacy and falco images. If an old version is used, the command will of course fail.

@toamto94
Copy link
Contributor Author

@FedeDP
Is it okay if I add this feature to the docker-entrypoints? Implementation should be pretty easy as there are multiple examples around.

@FedeDP
Copy link
Contributor

FedeDP commented Feb 10, 2024

Yes feel free to add the feat! We will release a new falcoctl (v0.7.2) soon that will be included in the upcoming Falco 0.37.1 patch release.
I will make sure that your patch on Falco will also be ported to the 0.37.1 release!

@FedeDP
Copy link
Contributor

FedeDP commented Feb 10, 2024

/milestone 0.37.1

@poiana poiana added this to the 0.37.1 milestone Feb 10, 2024
@toamto94
Copy link
Contributor Author

toamto94 commented Feb 11, 2024

The repo flag is also not enabled yet in docker right @FedeDP ?
There is a hint for an environment variable in the entrypoint script:

echo " FALCOCTL_DRIVER_REPOS specify different URL(s) where to look for prebuilt Falco drivers (comma separated)"

But it is not used anywhere in the script, so I guess that needs to be enabled as well right?

@toamto94
Copy link
Contributor Author

I assume we cannot simply add it via --repos=$FALCOCTL_DRIVER_REPOS as it would be empty in the default case. I am wondering how this could be implemented. Is there a way to dynamically create the argument string and parse it to falcoctl driver in the end of the script?

@FedeDP
Copy link
Contributor

FedeDP commented Feb 11, 2024

Nope because FALCOCTL_DRIVER_{REPOS/NAME} are env variable consumed by falcoctl, therefore there is no need to also add the flag (every falcoctl driver related flag is also bound to a FALCOCTL_DRIVER env variable!).

@toamto94
Copy link
Contributor Author

Nope because FALCOCTL_DRIVER_{REPOS/NAME} are env variable consumed by falcoctl, therefore there is no need to also add the flag (every falcoctl driver related flag is also bound to a FALCOCTL_DRIVER env variable!).

Ah alright! Let me check

@toamto94
Copy link
Contributor Author

@FedeDP I decided to close the pull request again because I was wondering if this should also be implemented as a global flag? Are there any usages of this outside falcoctl driver download? If repos is a global flag, shouldn't this then also be one?

@toamto94
Copy link
Contributor Author

However, http-headers is only used in driver install rn, so I assume it does not have other usages.

@FedeDP
Copy link
Contributor

FedeDP commented Feb 11, 2024

Yep it might be kept as env var only (and documented in the driver loader images helper message). My rule of thumb would be: was it exposed as a flag by old falco-driver-loader script? If it was, then we should expose it once again, otherwise keep it as env variable only ;)

@FedeDP
Copy link
Contributor

FedeDP commented Feb 12, 2024

Old driver-loader did expose:

	echo ""
	echo "Usage:"
	echo "  falco-driver-loader [driver] [options]"
	echo ""
	echo "Available drivers:"
	echo "  module        kernel module (default)"
	echo "  bpf           eBPF probe"
	echo ""
	echo "Options:"
	echo "  --help         show brief help"
	echo "  --clean        try to remove an already present driver installation"
	echo "  --compile      try to compile the driver locally (default true)"
	echo "  --download     try to download a prebuilt driver (default true)"
	echo "  --source-only  skip execution and allow sourcing in another script using `. falco-driver-loader`"
	echo "  --print-env    skip execution and print env variables for other tools to consume"
	echo ""
	echo "Environment variables:"
	echo "  DRIVERS_REPO             specify different URL(s) where to look for prebuilt Falco drivers (comma separated)"
	echo "  DRIVER_NAME              specify a different name for the driver"
	echo "  DRIVER_INSECURE_DOWNLOAD whether you want to allow insecure downloads or not"
	echo "  DRIVER_CURL_OPTIONS      specify additional options to be passed to curl command used to download Falco drivers"
	echo "  DRIVER_KERNEL_RELEASE    specify the kernel release for which to download/build the driver in the same format used by 'uname -r' (e.g. '6.1.0-10-cloud-amd64')"
	echo "  DRIVER_KERNEL_VERSION    specify the kernel version for which to download/build the driver in the same format used by 'uname -v' (e.g. '#1 SMP PREEMPT_DYNAMIC Debian 6.1.38-2 (2023-07-27)')"
	echo ""
	echo "Versions:"
	echo "  Falco version  ${FALCO_VERSION}"
	echo "  Driver version ${DRIVER_VERSION}"
	echo ""

(https://github.com/falcosecurity/falco/blob/1b62b5ccd1c64cd972ef0252262075cbf42a130c/scripts/falco-driver-loader).

We are exposing the same except for:

  • source-only that does not make sense in the new golang context
  • DRIVER_CURL_OPTIONS env variable that does not make sense now because we are not using curl anymore. There was no http headers option (well, they were passed using the DRIVER_CURL_OPTIONS env variable!), thus i think you are right, we can keep this env only. Feel free to just bump images helper message with the new env var!

@FedeDP
Copy link
Contributor

FedeDP commented Feb 12, 2024

(Some env variables were renamed but their functionality is still present)

@toamto94
Copy link
Contributor Author

Yes I had been using DRIVER_CURL_OPTIONS like "-H..." before the update. I will do some additional tests locally and then reopen the pull request. Also, if there is anyhting else I can do please let me know, it has been a lot of fun working on this :)

@FedeDP
Copy link
Contributor

FedeDP commented Feb 12, 2024

I think that for the patch release (0.37.1) we are good to go with your upcoming changes! You did a great job, thank you very much for your contribution!
You can check if falcoctl driver might need some more options for your use cases; i think it is pretty complete at the moment but perhaps we are missing some edge use cases ;)

@FedeDP
Copy link
Contributor

FedeDP commented Feb 12, 2024

Feel free to close this one once you tested it :)

@toamto94
Copy link
Contributor Author

Nope because FALCOCTL_DRIVER_{REPOS/NAME} are env variable consumed by falcoctl, therefore there is no need to also add the flag (every falcoctl driver related flag is also bound to a FALCOCTL_DRIVER env variable!).

Maybe I am missing something here but my tests were not successfull so far as the FALCOCTL_DRIVER_REPOS env variable in the container is not consumed by falcoctl. Is it possible that the binding (probably by viper.BindEnv) is missing for FALCOCTL_DRIVER_REPOS? @FedeDP

@FedeDP
Copy link
Contributor

FedeDP commented Feb 12, 2024

Mmmh, i just tried:

sudo FALCOCTL_DRIVER_REPOS="http://foo.com" ./falcoctl driver printenv
DRIVER="ebpf"
DRIVERS_REPO="http://foo.com"
DRIVER_VERSION="6.0.1+driver"
DRIVER_NAME="falco"
HOST_ROOT="/"
TARGET_ID="arch"
ARCH="x86_64"
KERNEL_RELEASE="6.7.4-arch1-1"
KERNEL_VERSION="#1 SMP PREEMPT_DYNAMIC Mon, 05 Feb 2024 22:07:49 +0000"
FIXED_KERNEL_RELEASE="6.7.4-arch1-1"
FIXED_KERNEL_VERSION="1"

It should work fine. We should not miss anything since the env variable gets automatically parsed by viper. How did you set the env variable in the container?

@FedeDP
Copy link
Contributor

FedeDP commented Feb 12, 2024

Also:

docker run -ti --rm -e FALCOCTL_DRIVER_REPOS="http://foo.com" falcosecurity/falco-driver-loader:0.37.0 --print-env
* Setting up /usr/src links from host
DRIVER="kmod"
DRIVERS_REPO="http://foo.com"
DRIVER_VERSION="7.0.0+driver"
DRIVER_NAME="falco"
HOST_ROOT="/host"
TARGET_ID="undetermined"
ARCH="x86_64"
KERNEL_RELEASE="6.7.4-arch1-1"
KERNEL_VERSION="#1 SMP PREEMPT_DYNAMIC Mon, 05 Feb 2024 22:07:49 +0000"
FIXED_KERNEL_RELEASE="6.7.4-arch1-1"
FIXED_KERNEL_VERSION="1"

@toamto94
Copy link
Contributor Author

Ahh yes it works. I am sorry, used FALCOCTL_DRIVERS_REPO instead of FALCOCTL_DRIVER_REPOS in my container :D Thx for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants