Pass registry of headers from ActionPlugin.getRestHeaders to ActionPlugin.getRestHandlerWrapper

[cwperks]

Description

Effectively, the security plugin is the only implementer of ActionPlugin.getRestHandlerWrapper and needs this registry to now which headers are explicitly allowlisted to be carried internally within the transport layer in order to trace a requests end-to-end through the entire lifecycle of a request.

Currently, security has logic to sanitize the threadcontext headers on internal transport requests so there's an existing bug in the ActionPlugin.getRestHeaders extension point when running a cluster with security. Security has created a PR to fix the bug, but it uses TheadContext.getHeaders() which creates an new HashMap on every call.

We would like to avoid that and explicitly call ThreadContext.getHeader(String headerName), but that requires knowing the header name to allowlist. With the changes in this PR, security will be able to iterate through the registry accordingly.

Related Issues

Resolves opensearch-project/security#4799

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

✅ Gradle check result for 316f2ab: SUCCESS

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.10%. Comparing base (d822f62) to head (316f2ab).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #19875      +/-   ##
============================================
+ Coverage     73.09%   73.10%   +0.01%     
- Complexity    71060    71109      +49     
============================================
  Files          5754     5754              
  Lines        325230   325260      +30     
  Branches      47035    47036       +1     
============================================
+ Hits         237735   237792      +57     
- Misses        68359    68369      +10     
+ Partials      19136    19099      -37     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[msfroh]

So, fun fact -- this change will break any plugins that implemented getRestHandlerWrapper with the old signature. Assuming they put the @Override annotation (which is a good idea, but still optional), it would break at compile time. If they built their plugin against OpenSearch 3.0-3.3 using semantic versioning for OpenSearch compatibility, they could load it against OpenSearch 3.4 and the getRestHandlerWrapper method would be silently ignored, which seems bad.

I'm thinking it would be a good idea to add an overloaded signature for backwards compatibility. That is, something like:

default UnaryOperator<RestHandler> getRestHandlerWrapper(ThreadContext threadContext, Set<RestHeaderDefinition> headersToCopy) {
    // Backward compatibility -- delegate to old method signature that didn't take header definitions
    return getRestHandlerWrapper(threadContext);
}

@Deprecated
default UnaryOperator<RestHandler> getRestHandlerWrapper(ThreadContext threadContext) {
    return null;
}

I'm a little shocked that the BWC check will complain about a change to public methods on the RemoteSegmentMetadata class, but it doesn't complain about a signature change on a plugin interface.

[cwperks]

@msfroh I plan to raise a PR shortly after talking with @aparajita31pandey. We did something similar for a recent change in ActionFilter here: https://github.com/opensearch-project/OpenSearch/blob/main/server/src/main/java/org/opensearch/action/support/ActionFilter.java#L70-L106