-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
rename config option .url and .ca to .hosts and .certificate_authorit #10380
rename config option .url and .ca to .hosts and .certificate_authorit #10380
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good.
Do we need to deprecate these settings on the 6.x branch before 6.7 is cut?
@@ -26,8 +26,8 @@ def additionals_settings(settings) | |||
settings.register(LogStash::Setting::ArrayCoercible.new("xpack.management.pipeline.id", String, ["main"])) | |||
settings.register(LogStash::Setting::NullableString.new("xpack.management.elasticsearch.username", "logstash_system")) | |||
settings.register(LogStash::Setting::NullableString.new("xpack.management.elasticsearch.password")) | |||
settings.register(LogStash::Setting::ArrayCoercible.new("xpack.management.elasticsearch.url", String, [ "https://localhost:9200" ] )) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will we need a backport in which we merely deprecate the current behaviour for 6.x (6.7, which is cut tomorrow)
@jsvd WDYT for the deprecation? |
@colinsurprenant thinking about how users will take their logstash.yml configuration and apply it to 7.0.0 this will present a breaking change without any heads up or message pointing them to the right setting, so having 6.7 deprecate these settings would be ideal. can you please give it a go? |
rebasing and merging. |
9e446f4
to
10b9033
Compare
unrelated failure #10371 - merging. |
In keeping with the changes made in elastic/logstash#10380.
In keeping with the changes made in elastic/logstash#10380.
In keeping with the changes made in elastic/logstash#10380.
In keeping with the changes made in elastic/logstash#10380.
Per the recent Kibana options changes in kibana#29496 we are also changing this options for 7.0.0-beta1.
*.elasticsearch.url
to*.elasticsearch.hosts
*.elasticsearch.ca
to*.elasticsearch.certificate_authority
This PR has 3 commits; one for the code changes, tests and docs.