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

fix(userspace/falco): use resolved plugin name when opening a plugin #2926

Closed
wants to merge 1 commit into from

Conversation

FedeDP
Copy link
Contributor

@FedeDP FedeDP commented Nov 27, 2023

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:

Previously, to enable a plugin users needed to know the plugin name, ie: the name exposed by the plugin_get_name plugin API.
Now, we automatically use the plugin name internally, while externally we satisfy the name given by the user in the plugins section.
Basically, user can now write:

load_plugins: [dummyFOO]

plugins:
  - name: dummyFOO
    library_path: libdummy.so
    init_config:
    jitter: 10
    open_params: "100"

And getting its dummyFOO plugin loaded correctly, while previously we would've failed, unless the load_plugins contained the real plugin name (in this case dummy).

From a UX PoV, i think the user shall know nothing about real plugin name; she/he should just have the .so of the plugin to be loaded and use the desired name in the config (like, in the example above, dummyFOO).

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

@poiana
Copy link
Contributor

poiana commented Nov 27, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP

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

@FedeDP
Copy link
Contributor Author

FedeDP commented Nov 27, 2023

/cc @jasondellaluce

@leogr
Copy link
Member

leogr commented Dec 12, 2023

@FedeDP
Copy link
Contributor Author

FedeDP commented Dec 13, 2023

/hold

@FedeDP
Copy link
Contributor Author

FedeDP commented Jan 11, 2024

Since there was no agreement among maintainers, this seems overkill and an unneeded behavioral change.
/close

@poiana poiana closed this Jan 11, 2024
@poiana
Copy link
Contributor

poiana commented Jan 11, 2024

@FedeDP: Closed this PR.

In response to this:

Since there was no agreement among maintainers, this seems overkill and an unneeded behavioral change.
/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.

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.

3 participants