Skip to content

fix(helm): honor logstashFormat: false in the Fluentd ES and OpenSearch outputs - #2114

Open
lbarrosby wants to merge 2 commits into
fluent:masterfrom
lbarrosby:fix/helm-fluentd-logstash-format
Open

lbarrosby wants to merge 2 commits into
fluent:masterfrom
lbarrosby:fix/helm-fluentd-logstash-format

Conversation

@lbarrosby

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

Follow-up to #2113, as suggested in this review. The Fluentd elasticsearch and opensearch ClusterOutput templates hardcode logstashFormat: true, so setting fluentd.output.es.logstashFormat or fluentd.output.opensearch.logstashFormat to false has no effect.

Both templates now use dig "logstashFormat" true ., same as #2113: the default stays true when the key is not set, and an explicit false is kept.

Checked with helm template (Fluentd with both outputs enabled):

  • key not set: logstashFormat: true, full render identical to master
  • logstashFormat=true: true
  • logstashFormat=false: true on master, false with this change

helm lint charts/fluent-operator passes, and the rendered ClusterOutputs validate with kubeconform against the chart's fluentd.fluent.io_clusteroutputs CRD schema.

Which issue(s) this PR fixes:

None, follow-up from the #2113 review.

Does this PR introduced a user-facing change?

Helm chart: fluentd.output.es.logstashFormat and fluentd.output.opensearch.logstashFormat can now be set to false.

Additional documentation, usage docs, etc.:


…ch outputs

Signed-off-by: Lucas Barros <lucasbarros.kam@gmail.com>
joshuabaird
joshuabaird previously approved these changes Nov 5, 2026

@joshuabaird joshuabaird left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks, this looks good. I rendered both outputs with the key unset and with logstashFormat=false and got true and false as expected.

One small suggestion: add a commented # logstashFormat: true under fluentd.output.es in values.yaml, like the Fluent Bit ES section has, so the option is easier to find.

Two existing issues I noticed while testing, not caused by this PR:

  • fluentd-output-opensearch.yaml uses .buffer.type and .buffer.path inside {{- with .buffer }}. Setting any OpenSearch buffer value fails to render with nil pointer evaluating interface {}.type. They should be .type and .path.
  • Both Fluentd templates ignore buffer.enable, so the buffer block renders even with enable: false, which is the ES default in values.yaml.

Happy to see those handled in a follow-up.

Signed-off-by: Lucas Barros <lucasbarros.kam@gmail.com>
@lbarrosby

Copy link
Copy Markdown
Contributor Author

Added it under fluentd.output.es, thanks.

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.

2 participants