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 #182

Merged
merged 3 commits into from
Dec 16, 2024
Merged

Conversation

robbavey
Copy link
Contributor

@robbavey robbavey commented Nov 13, 2024

This commit marks SSL settings obsolete that were previously marked as deprecated as part

of the SSL Settings Standardization process implemented in 3.7.0 of this plugin.

Marking these settings as obsolete is a breaking change that will stop the plugin from starting, and forces users to move to the new standard settings.

This commit makes the following configuration settings obsolete:

This commit cleans up some code to handle duplicate settings, and removes tests that were put in place to support the co-existence of deprecated and non-deprecated settings, replacing them with tests that verify that obsolete settings are identified early, and information about the deprecation is related to the user.

Thanks for contributing to Logstash! If you haven't already signed our CLA, here's a handy link:

Relates: #181

Copy link
Contributor

@donoghuc donoghuc left a comment

Choose a reason for hiding this comment

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

Removal looks clean. The only open question/suggestion is consistency on whether we validate an obsolete parameter: #182 (comment)

Copy link
Contributor

@donoghuc donoghuc left a comment

Choose a reason for hiding this comment

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

LGTM, CI appears to be backed up. Assuming that gets through green this should be g2g :)

@donoghuc
Copy link
Contributor

donoghuc commented Dec 3, 2024

I think test failures are due to expired cert fixtures. Regenerated in #184

Will probably be worth rebasing on that once it lands as this has some overlap.

UPDATE: #184 has been merged. This should be ready for rebase to confirm tests are all passing :)

This commit marks SSL settings `obsolete` that were previously marked as `deprecated` as part

of the SSL Settings Standardization process implemented in `3.7.0` of this plugin.

Marking these settings as `obsolete` is a *breaking change* that will stop the plugin from starting, and forces users to move to the new standard settings.

This commit makes the following configuration settings obsolete:

This commit cleans up some code to handle duplicate settings, and removes tests that were put in place to support the co-existence of deprecated and non-deprecated settings, replacing them with tests that verify that obsolete settings are identified early, and information about the deprecation is related to the user.
Copy link
Contributor

@donoghuc donoghuc left a comment

Choose a reason for hiding this comment

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

All green after rebase! 🍏

@robbavey robbavey requested a review from karenzone December 10, 2024 21:22
@robbavey
Copy link
Contributor Author

Ready for docs review @karenzone

Copy link
Contributor

@karenzone karenzone left a comment

Choose a reason for hiding this comment

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

LGTM

@robbavey robbavey merged commit 587efec into logstash-plugins:main Dec 16, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove deprecated SSL settings from HTTP input
4 participants