remove non-inclusive language for 3.0 #9455
Conversation
Signed-off-by: Justin Kim <[email protected]>
angle943
requested review from
ananzh,
kavilla,
AMoo-Miki,
ashwin-pc,
joshuarrrr,
abbyhu2000,
zengyan-amazon,
zhongnansu,
manasvinibs,
ZilongX,
Flyingliuhub,
curq,
bandinib-amzn,
SuZhou-Joe,
ruanyl,
BionIT,
xinruiba,
zhyuanqi,
mengweieric,
LDrago27,
virajsanghvi,
sejli,
joshuali925 and
huyaboo
as code owners
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9455 +/- ##
==========================================
- Coverage 61.74% 61.74% -0.01%
==========================================
Files 3816 3816
Lines 91886 91875 -11
Branches 14564 14556 -8
==========================================
- Hits 56734 56725 -9
+ Misses 31494 31493 -1
+ Partials 3658 3657 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -50,9 +50,9 @@ const dataPathDeprecation: ConfigDeprecation = (settings, fromPath, log) => { | |||
| }; | |||
|
|
|||
| const xsrfDeprecation: ConfigDeprecation = (settings, fromPath, log) => { | |||
| if ((settings.server?.xsrf?.whitelist ?? []).length > 0) { | |||
| if ((settings.server?.xsrf?.allowlist ?? []).length > 0) { | |||
There was a problem hiding this comment.
Have we actually deprecated this already if allowList isn't already in place?
There was a problem hiding this comment.
This is a mistake on my part, sorry. i will fix in a new pr
| requestHeadersWhitelistConfigured: isConfigured.stringOrArray( | ||
| opensearchConfig.requestHeadersWhitelist, | ||
| requestHeadersAllowlistConfigured: isConfigured.stringOrArray( | ||
| opensearchConfig.requestHeadersAllowlist, |
There was a problem hiding this comment.
curious, were we using allowList somehow before this change?
| @@ -51,7 +51,7 @@ export type LegacyOpenSearchClientConfig = Pick<ConfigOptions, 'keepAlive' | 'lo | |||
| | 'apiVersion' | |||
| | 'customHeaders' | |||
| | 'logQueries' | |||
| | 'requestHeadersWhitelist' | |||
| | 'requestHeadersAllowlist' | |||
There was a problem hiding this comment.
just making sure this works right?
some of these left over non-inclusive language was actually because the openseach-js client did not support the inclusive language
| expect(messages).toMatchInlineSnapshot(` | ||
| Array [ | ||
| "\\"elasticsearch.requestHeadersWhitelist\\" is deprecated and has been replaced by \\"opensearch.requestHeadersWhitelist\\"", | ||
| "\\"opensearch.requestHeadersWhitelist\\" is deprecated and has been replaced by \\"opensearch.requestHeadersAllowlist\\"", | ||
| "\\"elasticsearch.requestHeadersAllowlist\\" is deprecated and has been replaced by \\"opensearch.requestHeadersAllowlist\\"", |
There was a problem hiding this comment.
i think since it's 3.0 we can remove the elasticsearch. like we do not allow providers to set elasticsearch.requestHeadersAllowlist then can only set opensearch.requestHeadersAllowlist
There was a problem hiding this comment.
Oops this was a mistake, I will fix in next commit
|
hmm @kavilla @virajsanghvi admittedly I sort of went ham yesterday when working on this PR. I was mainly relying on tests to verify this works, but that's prob not a good idea 💩 . I will take a finer approach today. There's some part that I don't understand, so if I run into questions I'll be reaching out. |
Description
For OSD 3.0, we are removing non-inclusive language such as
whitelisting.Issues Resolved
Part of #9253
Changelog
Check List
yarn test:jestyarn test:jest_integration