Skip to content
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

Remove deprecated SSL settings from Logstash core #16652

Closed
Tracked by #16352
robbavey opened this issue Nov 7, 2024 · 4 comments
Closed
Tracked by #16352

Remove deprecated SSL settings from Logstash core #16652

robbavey opened this issue Nov 7, 2024 · 4 comments

Comments

@robbavey
Copy link
Member

robbavey commented Nov 7, 2024

  • [ ] Enumerate deprecated SSL settings in core
  • [ ] Ensure that all unit and integration tests still pass
  • [ ] Add details of change to the 9.0 breaking changes documentation in core.

See #16652 (comment) for write up, but I was unable to identify any actions required for the steps listed in this ticket.

@donoghuc
Copy link
Member

donoghuc commented Dec 14, 2024

In order to get the shape and scope of this I compiled a list of all the deprecates ssl options i've seen so far in the plugin work and searched a copy of a logstash checkout. There are obviously quite a few false positives (ssl is going to be a very popular one 😅 ). I've attached the hist and where they appear. I will be going through them and first removing obvious false positives. I'll then move on to classifying/organising the rest of the occurances. Let me know if i'm way over-thinking this though!

ag -i --hidden --unrestricted "\b(ssl\b|ca_file\b|ssl_certificate_verification\b|keystore\b|keystore_password\b|keystore_type\b|ssl_verify_mode\b|tls_min_version\b|tls_max_version\b|ssl_cert\b|ssl_cacert\b|ssl_enable\b|ssl_verify\b|cacert\b|client_cert\b|client_key\b|truststore\b|truststore_password\b|truststore_type\b)" > matches.txt

matches.txt

@robbavey
Copy link
Member Author

robbavey commented Dec 16, 2024

That definitely seems like a fair few false positives... We can skip anything modules related for a start with #16794 in flight

What we want to do here is ensure that we:

  • Remove any references to obsolete SSL settings in tests, docs, or code where we map settings to plugins
    • Any Logstash configuration settings from logstash.yml or docker config, for functionality like the configuration of internal collection based monitoring and central pipeline management are correctly mapped to non-deprecated settings.
  • Remove any settings that have previously been previously deprecated in Logstash core that could now be marked as obsolete.
    • This may be a no-op - the work to replace deprecated settings did not, AFAICT, mark any SSL-related settings in Logstash core as deprecated - but I would like to get verification.
    • We should not mark any SSL settings as :obsolete that were not previously marked as deprecated.
  • Any changes that we do make that are external facing should be put in the breaking changes doc

@donoghuc
Copy link
Member

donoghuc commented Dec 17, 2024

  • Remove any references to obsolete SSL settings in tests, docs, or code where we map settings to plugins
    • Any Logstash configuration settings from logstash.yml or docker config, for functionality like the configuration of internal collection based monitoring and central pipeline management are correctly mapped to non-deprecated settings.

The part I originally missed is that the keys in the mappings (IE the value we set when configuring these on the LS side) are not changing, its the translation of those into the settings used by the underlying plugins. I went through and confirmed that everywhere we need to do a translation is now doing so with modern/standardized ssl settings for plugins.

  • Remove any settings that have previously been previously deprecated in Logstash core that could now be marked as obsolete.

I was unable to find any plugin ssl settings that are carrying a deprecation warnings in LS.

  • This may be a no-op - the work to replace deprecated settings did not, AFAICT, mark any SSL-related settings in Logstash core as deprecated - but I would like to get verification.

Verified the linkede PR was complete and no deprecations need to be promoted to obsolete/removed. (Thank you BTW for linking this PR as this was the key to me understanding the scope of this work).

  • We should not mark any SSL settings as :obsolete that were not previously marked as deprecated.

This is a noop

  • Any changes that we do make that are external facing should be put in the breaking changes doc

I did not find any external facing deprecations/removals to document.

My overall findings for this ticket is that it is resulting in a noop. It is always hardest to prove a negative, but i'm just not finding anything that needs to be removed as part of 9.0.0 relating to ssl setting standardization.

@robbavey
Copy link
Member Author

Perfect - thank you. You can close this issue, unless you have any further questions

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

No branches or pull requests

2 participants