Skip to content

config: error configure_plugins_type on invalid properties - #10296

Merged
edsiper merged 2 commits into
fluent:masterfrom
stoksc:stoksc/invalid-input-property
May 29, 2025
Merged

config: error configure_plugins_type on invalid properties#10296
edsiper merged 2 commits into
fluent:masterfrom
stoksc:stoksc/invalid-input-property

Conversation

@stoksc

@stoksc stoksc commented May 6, 2025

Copy link
Copy Markdown
Contributor

After this change, when a plugin configuration contains an invalid property, configure_plugins_type returns an error which ultimately crashes fluentbit, instead of only printing an error log and continuing on. The change also fixes a small issue where name was printed by flb_error after it was freed. To properly test this change and get the test to pass with -DSANITIZE_ADDRESS=On on all platforms, I had to update configure_plugins_type to free objects that it fails to instantiate.


Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

  • [N/A] Example configuration file for the change
  • [N/A] Debug log output from testing the change

Added tests, so the configuration files and outputs can be found there.

  • [N/A] Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • [N/A] Run local packaging test showing all targets (including any new ones) build.
  • [N/A] Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • [N/A] Documentation required for this feature

Backporting

  • Backport to latest stable release.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

@stoksc
stoksc force-pushed the stoksc/invalid-input-property branch from 82e5741 to ea1ea67 Compare May 6, 2025 19:46
@stoksc
stoksc force-pushed the stoksc/invalid-input-property branch from 511c426 to e85d7f7 Compare May 6, 2025 21:20
@stoksc
stoksc force-pushed the stoksc/invalid-input-property branch 2 times, most recently from e85d7f7 to ffffa03 Compare May 9, 2025 21:10
@stoksc
stoksc force-pushed the stoksc/invalid-input-property branch 2 times, most recently from 677fc73 to df46f10 Compare May 12, 2025 16:03
@stoksc

stoksc commented May 14, 2025

Copy link
Copy Markdown
Contributor Author

Thanks for the review, @leonardo-albertovich. I've updated the pr with the changes you requested.

Comment thread src/flb_config.c Outdated
@stoksc

stoksc commented May 14, 2025

Copy link
Copy Markdown
Contributor Author

thanks @leonardo-albertovich, again. I've pushed a fix.

@stoksc

stoksc commented May 14, 2025

Copy link
Copy Markdown
Contributor Author

do i need to squash all my commits together manually or can i squash and merge through github?

i'll try to follow https://github.com/fluent/fluent-bit-docs/blob/master/CONTRIBUTING.md, i'll post back when i've updated the commits.

@stoksc
stoksc force-pushed the stoksc/invalid-input-property branch from 77ef771 to 7130d74 Compare May 14, 2025 15:48
@stoksc

stoksc commented May 14, 2025

Copy link
Copy Markdown
Contributor Author

ok, @leonardo-albertovich I think my commits should conform to the contributing guide. what happens next? do you merge it if it passes?

@leonardo-albertovich

Copy link
Copy Markdown
Contributor

Nope, it passes, I approve it, tag it if possible and ping @edsiper so he merges it when he considers it appropriate.

Comment thread src/flb_config.c Outdated
@stoksc
stoksc force-pushed the stoksc/invalid-input-property branch from f1a0272 to 48f4886 Compare May 19, 2025 20:32
@edsiper

edsiper commented May 22, 2025

Copy link
Copy Markdown
Member

thanks for this contribution. Code looks good to me.

To get this merged, please split the commits per interface, now there is 1 commit but 2 interfaces are being touched:

image

something like:

  • config: ..
  • tests: internal: config_format_yaml: ....

stoksc added 2 commits May 22, 2025 10:59
After this change, when a plugin configuration contains an invalid property,
`configure_plugins_type` returns an error which ultimately crashes fluentbit,
instead of only printing an error log and continuing on. The change also fixes
a small issue where `name` was printed by `flb_error` after it was freed. To
properly test this change and get the test to pass with `-DSANITIZE_ADDRESS=On`
on all platforms, I had to update `configure_plugins_type` to free objects that
it fails to instantiate.

Signed-off-by: Bradley Laney <bradley.laney@chronosphere.io>
…invalid properties

Signed-off-by: Bradley Laney <bradley.laney@chronosphere.io>
@edsiper

edsiper commented May 29, 2025

Copy link
Copy Markdown
Member

thank you

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants