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

Implement configuration of webserver listening address #2890

Conversation

sgaist
Copy link
Contributor

@sgaist sgaist commented Oct 26, 2023

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 release

Any specific area of the project related to this PR?

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

/area build

/area engine

/area tests

/area proposals

/area CI

What this PR does / why we need it:

Currently the webserver is listening on the hard coded 0.0.0.0 address.
This patch keeps this default but allows the administrator to change it through the falco.yaml file.

Which issue(s) this PR fixes:

Fixes #2774

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

feat(userspace/falco): falco administrators can now configure the address on which the webserver listen using the new listen_address field in the webserver block of the `falco.yaml` file.

@poiana
Copy link
Contributor

poiana commented Oct 26, 2023

Welcome @sgaist! It looks like this is your first PR to falcosecurity/falco 🎉

@poiana poiana requested review from incertum and Kaizhe October 26, 2023 18:52
@poiana poiana added the size/S label Oct 26, 2023
falco.yaml Show resolved Hide resolved
Copy link
Contributor

@incertum incertum left a comment

Choose a reason for hiding this comment

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

Thank you @sgaist!

Totally makes sense to me and implementation LGTM.
Since I am not using this functionality myself, let's have another maintainer take a closer look.

CC @LucaGuerra @FedeDP thanks!

Copy link
Contributor

@FedeDP FedeDP left a comment

Choose a reason for hiding this comment

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

LGTM, i really like it, thank you!
Left only a comment!

@FedeDP
Copy link
Contributor

FedeDP commented Oct 27, 2023

/milestone 0.37.0

@poiana poiana added this to the 0.37.0 milestone Oct 27, 2023
@sgaist sgaist force-pushed the 2774_implement_web_server_address_selection branch from 5e5d761 to 724ae63 Compare October 27, 2023 19:30
@sgaist sgaist force-pushed the 2774_implement_web_server_address_selection branch from 724ae63 to 1072525 Compare October 27, 2023 19:31
@sgaist sgaist force-pushed the 2774_implement_web_server_address_selection branch from 1072525 to 4277128 Compare October 28, 2023 20:45
unit_tests/falco/test_configuration.cpp Outdated Show resolved Hide resolved
falco.yaml Show resolved Hide resolved
Copy link
Contributor

@incertum incertum left a comment

Choose a reason for hiding this comment

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

LGTM, will approve after you re-push the last commit with signed off, ty!

static re2::RE2 ipv4_address_re("^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$");

// Reference: https://digitalfortress.tech/tips/top-15-commonly-used-regex/
static re2::RE2 ip_address_re("((^\\s*((([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]))\\s*$)|(^\\s*((([0-9A-Fa-f]{1,4}:){7}([0-9A-Fa-f]{1,4}|:))|(([0-9A-Fa-f]{1,4}:){6}(:[0-9A-Fa-f]{1,4}|((25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]?\\d)(\\.(25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]?\\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){5}(((:[0-9A-Fa-f]{1,4}){1,2})|:((25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]?\\d)(\\.(25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]?\\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){4}(((:[0-9A-Fa-f]{1,4}){1,3})|((:[0-9A-Fa-f]{1,4})?:((25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]?\\d)(\\.(25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]?\\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){3}(((:[0-9A-Fa-f]{1,4}){1,4})|((:[0-9A-Fa-f]{1,4}){0,2}:((25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]?\\d)(\\.(25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]?\\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){2}(((:[0-9A-Fa-f]{1,4}){1,5})|((:[0-9A-Fa-f]{1,4}){0,3}:((25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]?\\d)(\\.(25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]?\\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){1}(((:[0-9A-Fa-f]{1,4}){1,6})|((:[0-9A-Fa-f]{1,4}){0,4}:((25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]?\\d)(\\.(25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]?\\d)){3}))|:))|(:(((:[0-9A-Fa-f]{1,4}){1,7})|((:[0-9A-Fa-f]{1,4}){0,5}:((25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]?\\d)(\\.(25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]?\\d)){3}))|:)))(%.+)?\\s*$))");
Copy link
Contributor

Choose a reason for hiding this comment

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

😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"savage" might be a good description 😁

Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you share my password manager passphrase here?? 🤣

address

Currently the webserver is listening on the hard coded 0.0.0.0. This
patch keeps this default but allows the administrator to change it.

Signed-off-by: Samuel Gaist <[email protected]>
…isten address

The IPV6 capabilities is provided through cpp-httplib.

Signed-off-by: Samuel Gaist <[email protected]>
@sgaist sgaist force-pushed the 2774_implement_web_server_address_selection branch from 9e0e7b1 to 0c881a1 Compare November 2, 2023 19:19
@sgaist
Copy link
Contributor Author

sgaist commented Nov 2, 2023

LGTM, will approve after you re-push the last commit with signed off, ty!

sign-off added and re-based as well :-)

Copy link
Contributor

@incertum incertum left a comment

Choose a reason for hiding this comment

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

/approve

@sgaist 🎉 thanks so much for this contributions, hopefully we see more in the future 🙃 !

@poiana
Copy link
Contributor

poiana commented Nov 2, 2023

LGTM label has been added.

Git tree hash: 5afa56f2ba5006f250cd66d1233175859e261fba

@poiana poiana added the approved label Nov 2, 2023
Copy link
Contributor

@FedeDP FedeDP left a comment

Choose a reason for hiding this comment

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

/approve

Great job, thank you very much!

@poiana
Copy link
Contributor

poiana commented Nov 3, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, incertum, sgaist

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:

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

@poiana poiana merged commit d074728 into falcosecurity:master Nov 3, 2023
14 of 15 checks passed
@sgaist sgaist deleted the 2774_implement_web_server_address_selection branch November 3, 2023 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Adding the opportunity to select Falco webserver port in configuration file
5 participants