-
Notifications
You must be signed in to change notification settings - Fork 602
Updating deprecated options #9602
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
base: main
Are you sure you want to change the base?
Conversation
Thank you for submitting your PR. The PR states are In progress (or Draft) -> Tech review -> Doc review -> Editorial review -> Merged. Before you submit your PR for doc review, make sure the content is technically accurate. If you need help finding a tech reviewer, tag a maintainer. When you're ready for doc review, tag the assignee of this PR. The doc reviewer may push edits to the PR directly or leave comments and editorial suggestions for you to address (let us know in a comment if you have a preference). The doc reviewer will arrange for an editorial review. |
@landon-lengyel Could you please resolve DCO? |
@cwperks Could you please review this PR? |
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.
@landon-lengyel These settings still exist in the security plugin and are not marked as deprecated.
I see logic in the security plugin that either configuration is permissible.
Are you experiencing any errors configuring with the documented settings?
@kolchfa-aws I'm sorry, I'm not sure what to do for the DCO. I did include |
@reta I see you have the security plugin settings marked as deprecated in opensearch-project/security@e2a06f0. Is there a plan to remove them? The options I see are 1) remove the deprecated setting or 2) undeprecate the setting and support with both the |
@cwperks It looks like you were able to answer your own question, but to clarify yes I am seeing warnings in the deprecation logs. Such as:
There's a bit more info about that on security issue 4780 |
@landon-lengyel To fix DCO, rebase the branch as described here https://github.com/opensearch-project/documentation-website/pull/9602/checks?check_run_id=40407368811. |
Oh yeah, thanks @cwperks , I think those all are in core (netty4 transport module) now, we could removed then in 3.0.0, thank you |
Signed-off-by: Landon Lengyel <[email protected]> Signed-off-by: Landon Lengyel <[email protected]> Signed-off-by: Landon Lengyel <[email protected]>
@landon-lengyel Do you still plan to proceed with this PR? |
@landon-lengyel I see more settings are present in server/src/main/java/org/opensearch/common/network/NetworkModule.java of opensearch-project/OpenSearch#12435 which I think should be updated in the docs. 2 of the settings were already in the doc which you updated, but the PR itself introduces I think a good direction might be to evaluate the cc: @reta @peternied I think it might be useful if either of you can help with review as well this based on your understanding of opensearch-project/OpenSearch#12435 |
@cwperks could you help find someone to build on the great updates to our documentation by @landon-lengyel? |
Thanks @sandeshkr419 , I think TRANSPORT_SSL_DUAL_MODE_ENABLED_KEY is the only one that is missed, @landon-lengyel could you please document it as well? AFAIK it is used in clusters where mix of SSL / non-SSL transports is present (fe when there is gradual rollout of transport security withing running cluster). Thank you. |
Signed-off-by: Landon Lengyel <[email protected]>
Signed-off-by: Landon Lengyel <[email protected]>
I accidentally removed the commits and I can't seem to re-associate them with this request. Do you have any idea how? Or should I open a new PR? Sorry for the inconvenience. I keep thinking git will start making sense, but it hasn't yet.. |
Never mind, I seem to have got them back I just had to re-open it for Github to refresh. In regards to the |
@landon-lengyel Seems like you lost a few of your commits while rebasing - I am checking files associated with the PR but I don't see the intended changes anymore. |
Description
Updates options that are marked as deprecated.
Issues Resolved
Resolves issue #9466 if these two options are all that's changed.
Version
v2.19.1
Frontend features
n/a
Checklist
For more information on following Developer Certificate of Origin and signing off your commits, please check here.