Skip to content

Conversation

drinkingjasminegreentea
Copy link

@drinkingjasminegreentea drinkingjasminegreentea commented Jun 14, 2025

  • support yaml config format
  • simplify config data
  • remove deprecated upstream config

@drinkingjasminegreentea drinkingjasminegreentea changed the title Support yaml config feat: fluent-bit - support yaml config Jun 14, 2025
@drinkingjasminegreentea drinkingjasminegreentea changed the title feat: fluent-bit - support yaml config feat-fluent-bit: support yaml config Jun 14, 2025
@drinkingjasminegreentea drinkingjasminegreentea changed the title feat-fluent-bit: support yaml config feat(fluent-bit): support yaml config Jun 14, 2025
Copy link
Collaborator

@stevehipwell stevehipwell left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @drinkingjasminegreentea but I have a couple of major issues with the approach. Firstly it would introduce a significant breaking change and for this chart, IMHO the YAML support needs to be an opt in feature not a wholesale replacement of the existing logic. Secondly one of the main benefits of the YAML format would be native values support but this PR treats the YAML as just a string; to do this correctly the YAML config needs to be a dict type and we need custom logic to merge in values from elsewhere in the chart values.

For reference I've started this work in my seperate FB charts at stevehipwell/helm-charts#1170 where I am making it a breaking change. I've not taken this further given the the questionable stability and status of the YAML config implementation.

@tropnikovvl
Copy link

Hey, any updates here?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants