new(cmd/driver): support user provided headers option for driver download #428

toamto94 · 2024-02-08T21:11:40Z

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind feature

/kind flaky-test

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area library

/area cli

/area tests

/area examples

What this PR does / why we need it:
Added option to add headers in the http GET request for the driver download

Which issue(s) this PR fixes:
Option had been possible via environment variables in the script based download but is not possible anymore since the falcoctl migration

Fixes #426

Special notes for your reviewer:
example flag (for testing):
--http-headers="x-emc-namespace: default,Proxy-Authenticate: Basic"

poiana · 2024-02-08T21:11:51Z

Welcome @toamto94! It looks like this is your first PR to falcosecurity/falcoctl 🎉

FedeDP · 2024-02-08T21:13:44Z

Thanks for this PR! I'll give it a look tomorrow!
/assign

cmd/driver/install/install.go

FedeDP · 2024-02-09T07:04:05Z

/milestone v0.7.2

FedeDP · 2024-02-09T08:05:19Z

To fix linting: make sure to split the line:

driverdistro.Download(ctx, d, o.Printer.WithWriter(&buf), kr, o.Driver.Name, o.Driver.Type, o.Driver.Version, o.Driver.Repos, o.HTTPHeaders)

Into multiple lines ;)

pkg/driver/distro/distro.go

FedeDP

/approve

poiana · 2024-02-09T09:27:05Z

LGTM label has been added.

Git tree hash: 1b9a401266077b97a5b366b061fa90ac478b163c

FedeDP · 2024-02-09T09:28:49Z

Can you run make fmt to fix linting? Thanks!

FedeDP · 2024-02-09T09:42:35Z

cmd/driver/install/install.go

 }

 type driverInstallOptions struct {
 	*options.Common
 	*options.Driver
-	Download            bool
-	Compile             bool
+	Download: true,


toamto94 · 2024-02-09T09:47:43Z

Ah I am sorry! I have a meeting now and will fix it afterwords. Gesendet mit der mobilen Mail AppAm 09.02.24 um 10:42 schrieb Federico Di Pierro Von: "Federico Di Pierro" ***@***.***>Datum: 9. Februar 2024An: "falcosecurity/falcoctl" ***@***.***>Cc: "Mention" ***@***.***>,"Tom Müller" ***@***.***>Betreff: Re: [falcosecurity/falcoctl] Fb header options (PR #428) @FedeDP commented on this pull request. In cmd/driver/install/install.go:

}

type driverInstallOptions struct { *options.Common *options.Driver - Download bool - Compile bool + Download: true, bool ? —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

FedeDP · 2024-02-09T10:52:12Z

I'd say we are quite ready; you just miss to run make fmt and perhaps i'd squash everything in a single commit :)

toamto94

My IDE is messing around, I am sorry for the confusion

toamto94 · 2024-02-09T10:54:44Z

I'd say we are quite ready; you just miss to run make fmt and perhaps i'd squash everything in a single commit :)

Alright, let me get things right now. Sorry for the confusion :D

toamto94 · 2024-02-09T11:42:41Z

Oh you need to also bump helper message here: https://github.com/falcosecurity/falcoctl/blob/main/cmd/driver/install/install_test.go#L29 Sorry, forgot about it!

Ok no problem!

toamto94 · 2024-02-09T12:08:00Z

ah need to sign of. sec

toamto94 · 2024-02-09T12:20:42Z

Aah wait let me fix this

Signed-off-by: Tom Müller <[email protected]> fixed linting for http-header option fixed syntax error in driver download options Signed-off-by: Tom Müller <[email protected]> fixed syntax error in driver download options Signed-off-by: Tom Müller <[email protected]> fixed syntax error in driver download options Signed-off-by: Tom Müller <[email protected]> fixed linting for driver download options Signed-off-by: Tom Müller <[email protected]> moved header injection below error check No need to inject a header into an empty request Signed-off-by: Tom Müller <[email protected]> fixed linting for Download function call Line was too long so needed to be split in multiple lines Signed-off-by: Tom Müller <[email protected]> rearranged http-headers flag http-headers flag moved to driverDownloadOptions Signed-off-by: Tom Müller <[email protected]> added leading space trimming for header keys Added leading space trimming for header keys in order to make the function more robust Signed-off-by: Tom Müller <[email protected]> forwarding of the headers variable into the http GET request Added functionality which parses the comma separated string of headers which were provided with the --http-headers flag to the Download function. The headers are unpacked and injected into the http GET request. Signed-off-by: Tom Müller <[email protected]> added header options for driver download Added header options for the driver download via http GET. Headers should be provided via the --http-headers flag as a comma separated string (e.g. --http-headers="x-emc-namespace:default") Signed-off-by: Tom Müller <[email protected]>

toamto94 · 2024-02-09T12:34:33Z

Fighting with git is always fun :D

toamto94 · 2024-02-09T12:40:31Z

Failed. What exactly needs to be done there @FedeDP

toamto94 · 2024-02-09T13:27:12Z

Don't seem to work hm. Any idea what I am doing wrong?

FedeDP · 2024-02-09T13:32:42Z

You can quickly locally test using go test ./cmd/driver/... ; am not sure about what's failing though :(

toamto94 · 2024-02-09T13:33:33Z

You can quickly locally test using go test ./cmd/driver/... ; am not sure about what's failing though :(

Ah alright! Thx for the tip

FedeDP · 2024-02-09T13:40:20Z

  --http-headers string     Optional comma-separated list of headers for the http GET request (e.g. --http-headers="x-emc-namespace: default,Proxy-Authenticate: Basic"). Not necessary if default repo is used
      --http-insecure           Whether you want to allow insecure downloads or not

--http-headers should be above --http-insecure :D

FedeDP · 2024-02-09T13:40:44Z

cmd/driver/install/install_test.go

@@ -37,6 +37,7 @@ Flags:
      --download                Whether to enable download of prebuilt drivers (default true)
  -h, --help                    help for install
      --http-insecure           Whether you want to allow insecure downloads or not
+      --http-headers string     Optional comma-separated list of headers for the http GET request (e.g. --http-headers='x-emc-namespace: default,Proxy-Authenticate: Basic'). Not necessary if default repo is used


Here, --http-headers should be before --http-insecure.

Awesome!! Thank you very much! :)

Signed-off-by: Tom Müller <[email protected]>

FedeDP

/approve

poiana · 2024-02-09T13:54:05Z

LGTM label has been added.

Git tree hash: d40677f83658c1e4257ccbcc645000c3189d4d3d

poiana · 2024-02-09T13:54:06Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, toamto94

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [FedeDP]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

poiana added kind/feature dco-signoff: yes area/cli labels Feb 8, 2024

poiana requested a review from alacuku February 8, 2024 21:11

poiana requested a review from loresuso February 8, 2024 21:11

poiana added the size/S label Feb 8, 2024

poiana assigned FedeDP Feb 8, 2024

FedeDP reviewed Feb 9, 2024

View reviewed changes

cmd/driver/install/install.go Outdated Show resolved Hide resolved

FedeDP reviewed Feb 9, 2024

View reviewed changes

cmd/driver/install/install.go Outdated Show resolved Hide resolved

poiana added this to the v0.7.2 milestone Feb 9, 2024

FedeDP mentioned this pull request Feb 9, 2024

sync: release v0.7.x #429

Merged

FedeDP reviewed Feb 9, 2024

View reviewed changes

pkg/driver/distro/distro.go Show resolved Hide resolved

FedeDP previously approved these changes Feb 9, 2024

View reviewed changes

poiana added the lgtm label Feb 9, 2024

poiana added the approved label Feb 9, 2024

toamto94 dismissed FedeDP’s stale review via 99c7bd7 February 9, 2024 09:37

poiana removed the lgtm label Feb 9, 2024

poiana requested a review from FedeDP February 9, 2024 09:37

FedeDP reviewed Feb 9, 2024

View reviewed changes

FedeDP changed the title ~~Fb header options~~ new(cmd/driver): support user provided headers option for driver download Feb 9, 2024

toamto94 commented Feb 9, 2024

View reviewed changes

poiana added dco-signoff: no and removed dco-signoff: yes labels Feb 9, 2024

toamto94 force-pushed the fb_header_options branch from 45a0d08 to 65ebe6d Compare February 9, 2024 12:29

toamto94 force-pushed the fb_header_options branch from 65ebe6d to fa715ce Compare February 9, 2024 12:32

poiana added dco-signoff: yes and removed dco-signoff: no labels Feb 9, 2024

toamto94 force-pushed the fb_header_options branch 4 times, most recently from 8ab30b1 to 1676aed Compare February 9, 2024 13:16

toamto94 force-pushed the fb_header_options branch from 1676aed to 84726a4 Compare February 9, 2024 13:30

FedeDP reviewed Feb 9, 2024

View reviewed changes

bumped helper message

24760db

Signed-off-by: Tom Müller <[email protected]>

toamto94 force-pushed the fb_header_options branch from 84726a4 to 24760db Compare February 9, 2024 13:47

FedeDP approved these changes Feb 9, 2024

View reviewed changes

poiana added the lgtm label Feb 9, 2024

poiana merged commit e03c73c into falcosecurity:main Feb 9, 2024
14 checks passed

BrewTestBot mentioned this pull request May 28, 2024

falcoctl 0.8.0 Homebrew/homebrew-core#173001

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new(cmd/driver): support user provided headers option for driver download #428

new(cmd/driver): support user provided headers option for driver download #428

toamto94 commented Feb 8, 2024

poiana commented Feb 8, 2024

FedeDP commented Feb 8, 2024

FedeDP commented Feb 9, 2024

FedeDP commented Feb 9, 2024

FedeDP left a comment

poiana commented Feb 9, 2024

FedeDP commented Feb 9, 2024

FedeDP Feb 9, 2024

toamto94 commented Feb 9, 2024 via email

FedeDP commented Feb 9, 2024

toamto94 left a comment

toamto94 commented Feb 9, 2024

toamto94 commented Feb 9, 2024

toamto94 commented Feb 9, 2024

toamto94 commented Feb 9, 2024

toamto94 commented Feb 9, 2024

toamto94 commented Feb 9, 2024

toamto94 commented Feb 9, 2024

FedeDP commented Feb 9, 2024

toamto94 commented Feb 9, 2024

FedeDP commented Feb 9, 2024

FedeDP Feb 9, 2024

toamto94 Feb 9, 2024

FedeDP left a comment

poiana commented Feb 9, 2024

poiana commented Feb 9, 2024

new(cmd/driver): support user provided headers option for driver download #428

new(cmd/driver): support user provided headers option for driver download #428

Conversation

toamto94 commented Feb 8, 2024

poiana commented Feb 8, 2024

FedeDP commented Feb 8, 2024

FedeDP commented Feb 9, 2024

FedeDP commented Feb 9, 2024

FedeDP left a comment

Choose a reason for hiding this comment

poiana commented Feb 9, 2024

FedeDP commented Feb 9, 2024

FedeDP Feb 9, 2024

Choose a reason for hiding this comment

toamto94 commented Feb 9, 2024 via email

FedeDP commented Feb 9, 2024

toamto94 left a comment

Choose a reason for hiding this comment

toamto94 commented Feb 9, 2024

toamto94 commented Feb 9, 2024

toamto94 commented Feb 9, 2024

toamto94 commented Feb 9, 2024

toamto94 commented Feb 9, 2024

toamto94 commented Feb 9, 2024

toamto94 commented Feb 9, 2024

FedeDP commented Feb 9, 2024

toamto94 commented Feb 9, 2024

FedeDP commented Feb 9, 2024

FedeDP Feb 9, 2024

Choose a reason for hiding this comment

toamto94 Feb 9, 2024

Choose a reason for hiding this comment

FedeDP left a comment

Choose a reason for hiding this comment

poiana commented Feb 9, 2024

poiana commented Feb 9, 2024