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.

Sign up for GitHub

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

Jump to bottom

fix(plugins): trim whitespace in open_params #2267

Merged
merged 1 commit into from
Oct 21, 2022
Merged

fix(plugins): trim whitespace in open_params #2267

merged 1 commit into from
Oct 21, 2022

Conversation

yardenshoham
Copy link
Contributor

@yardenshoham yardenshoham commented Oct 19, 2022 •
edited by leogr
Loading

What type of PR is this?

/kind bug

Any specific area of the project related to this PR?

/area engine

What this PR does / why we need it:

open_params is read from the falco YAML configuration file and parsed using Go's URL.

For example:
https://github.com/falcosecurity/plugins/blob/c349be6e84d2230698b5ca5d51ddadcda42c5e21/plugins/k8saudit/pkg/k8saudit/source.go#L41-L42

Go's URL parser does not handle whitespace, so if a user defines the open_params in the falco configuration file as follows

open_params: >
/file/path

the parser returns an error. To avoid this, we now trim this parameter so no whitespace will be left for Go's URL parser to error out on.

Which issue(s) this PR fixes:

It doesn't fix it but it started from falcosecurity/plugins#182.

Special notes for your reviewer:

Read falcosecurity/plugins#182.

Does this PR introduce a user-facing change?:

update(falco.yaml): `open_params` under plugins configuration is now trimmed from surrounding whitespace

/cc @jasondellaluce
/assign @yardenshoham

@yardenshoham
fix(plugins): trim whitespace in open_params
c1cf2e8 
`open_params` is read from the falco YAML configuration file and parsed using Go's URL.

For example:
https://github.com/falcosecurity/plugins/blob/c349be6e84d2230698b5ca5d51ddadcda42c5e21/plugins/k8saudit/pkg/k8saudit/source.go#L41-L42

Go's URL parser does not handle whitespace, so if a user defines the `open_params` in the falco configuration file as follows

```yaml
open_params: >
/file/path
```

the parser returns an error. To avoid this, we now trim this parameter so no whitespace will be left for Go's URL parser to error out on.

For reference see #2262.

Signed-off-by: Yarden Shoham <hrsi88@gmail.com>
@poiana poiana requested a review from jasondellaluce October 19, 2022 20:14
@poiana
Copy link
Contributor

poiana commented Oct 19, 2022

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

@poiana poiana added the size/XS label Oct 19, 2022
@jasondellaluce jasondellaluce mentioned this pull request Oct 20, 2022
Closed
jasondellaluce
jasondellaluce approved these changes Oct 20, 2022
View reviewed changes
Copy link
Contributor

@jasondellaluce jasondellaluce left a comment

Choose a reason for hiding this comment

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

/milestone 0.34.0

/ok-to-test

/approve

@poiana poiana added this to the 0.34.0 milestone Oct 20, 2022
@poiana poiana added the lgtm label Oct 20, 2022
@poiana
Copy link
Contributor

poiana commented Oct 20, 2022

LGTM label has been added.

Git tree hash: 2565a394157ce6bd135e45e4cbcd36ab32578cee

@FedeDP
Copy link
Contributor

FedeDP commented Oct 20, 2022

Could not find a usable config.yml, you may have revoked the CircleCI OAuth app.
Please sign out of CircleCI and log back in with your VCS before triggering a new pipeline.

Sorry WHAT

FedeDP
FedeDP approved these changes Oct 20, 2022
View reviewed changes
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

@leogr
Copy link
Member

leogr commented Oct 21, 2022

Closing and reopening

/close

@poiana poiana closed this Oct 21, 2022
@poiana
Copy link
Contributor

poiana commented Oct 21, 2022

@leogr: Closed this PR.

In response to this:

Closing and reopening

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@leogr
Copy link
Member

leogr commented Oct 21, 2022

/reopen

@poiana poiana reopened this Oct 21, 2022
@poiana
Copy link
Contributor

poiana commented Oct 21, 2022

@leogr: Reopened this PR.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@poiana
Copy link
Contributor

poiana commented Oct 21, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, jasondellaluce, leogr, yardenshoham

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,jasondellaluce,leogr]

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 4a4fa25 into falcosecurity:master Oct 21, 2022
@yardenshoham yardenshoham deleted the trim branch October 21, 2022 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasondellaluce jasondellaluce jasondellaluce approved these changes

@FedeDP FedeDP FedeDP approved these changes

@leogr leogr leogr approved these changes

Assignees

@leogr leogr

@FedeDP FedeDP

@jasondellaluce jasondellaluce

@yardenshoham yardenshoham

Labels
approved area/build dco-signoff: yes kind/bug lgtm release-note size/XS
Projects
Falco Roadmap
Archived in project
Milestone
0.34.0
Development

Successfully merging this pull request may close these issues.
5 participants
@yardenshoham @poiana @FedeDP @leogr @jasondellaluce