Skip to content

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Apr 9, 2025

Release notes

  • Fixes an issue introduced in 8.16.4, 8.17.2, and 8.18.0 where null values were incorrectly accepted for non-nullable settings.

What does this PR do?

Ensures that SettingString is correctly non-nullable and SettingNullableString is nullable.

Why is it important/What is the impact to the user?

Improves error messages and reliability of behaviour when Logstash is misconfigured.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • [ ] I have made corresponding changes to the documentation
  • [ ] I have made corresponding change to the default configuration files (and/or docker env variables)
  • I have added tests that prove my fix is effective or that my feature works

How to test this PR locally

With Logstash 8.15.5, set a non-nullable string setting to null in the logstash.yml and run Logstash, observing that it refuses to start and gives guidance about the wrong setting

(cd logstash-8.15.5 && echo 'keystore.file: null' > config/logstash.yml && bin/logstash -e 'input { generator { count => 1 } }')
Your settings are invalid. Reason: Setting "keystore.file" must be a String. Received:  (NilClass)
[2025-04-08T21:51:30,611][FATAL][org.logstash.Logstash    ] Logstash stopped processing because of an error: (SystemExit) exit

Repeat with Logstash 8.17.4 and observe that runtime errors that are less clear occur later

(cd logstash-8.17.4 && echo 'keystore.file: null' > config/logstash.yml && bin/logstash -e 'input { generator { count => 1 } }')
[2025-04-08T22:46:34,161][ERROR][logstash.agent           ] Failed to execute action {:action=>LogStash::PipelineAction::Create/pipeline_id:main, :exception=>"TypeError", :message=>"nil is not a string", :backtrace=>["org/logstash/execution/AbstractPipelineExt.java:293:in `initialize'", "org/logstash/execution/AbstractPipelineExt.java:227:in `initialize'", "/Users/rye/src/elastic/scratch/logstash-8.17.4/logstash-core/lib/logstash/java_pipeline.rb:47:in `initialize'", "org/jruby/RubyClass.java:949:in `new'", "/Users/rye/src/elastic/scratch/logstash-8.17.4/logstash-core/lib/logstash/pipeline_action/create.rb:50:in `execute'", "/Users/rye/src/elastic/scratch/logstash-8.17.4/logstash-core/lib/logstash/agent.rb:420:in `block in converge_state'"]}
[2025-04-08T22:46:34,188][INFO ][logstash.agent           ] Successfully started Logstash API endpoint {:port=>9600, :ssl_enabled=>false}
[2025-04-08T22:46:34,192][INFO ][logstash.runner          ] Logstash shut down.
[2025-04-08T22:46:34,194][FATAL][org.logstash.Logstash    ] Logstash stopped processing because of an error: (SystemExit) exit

Repeat with this patch, and observe the restored behaviour:

echo 'keystore.file: null' > config/logstash.yml && bin/logstash -e 'input { generator { count => 1 } }'
Your settings are invalid. Reason: Setting "keystore.file" must be a String. Received:  (NilClass)
[2025-04-08T22:49:04,092][FATAL][org.logstash.Logstash    ] Logstash stopped processing because of an error: (SystemExit) exit

Related issues

Caused-By: #16576


This is an automatic backport of pull request #17522 done by [Mergify](https://mergify.com).

@mergify mergify bot added the backport label Apr 9, 2025
@mergify mergify bot assigned yaauie Apr 9, 2025
@elastic-sonarqube
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube

@elasticmachine
Copy link

💚 Build Succeeded

cc @yaauie

Copy link
Member

@yaauie yaauie left a comment

Choose a reason for hiding this comment

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

✅ Clean and Green 🎉

@yaauie yaauie merged commit d0d3697 into 8.x Apr 9, 2025
7 checks passed
@yaauie yaauie deleted the mergify/bp/8.x/pr-17522 branch April 9, 2025 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants