Fix sslConfig for multiple datasource to handle when certificateAuthorities is unset #6282

cwperks · 2024-03-27T22:38:21Z

Description

Fixes the logic in TLS configuration for multiple datasources to match the behavior of opensearch_client_config.ts which sets the ca portion of the ssl config to undefined (instead of empty list) when the datasource.ssl.certificateAuthorities setting is not set in opensearch_dashboards.yml.

Issues Resolved

[BUG] Add datasource with test connect failed #6281

Check List

All tests pass
- yarn test:jest
- yarn test:jest_integration
New functionality includes testing.
New functionality has been documented.
Update CHANGELOG.md
Commits are signed per the DCO using --signoff

…rities is unset Signed-off-by: Craig Perkins <[email protected]>

codecov · 2024-03-27T22:54:34Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.46%. Comparing base (91a0530) to head (d1ed005).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6282   +/-   ##
=======================================
  Coverage   67.45%   67.46%           
=======================================
  Files        3368     3368           
  Lines       65423    65422    -1     
  Branches    10559    10557    -2     
=======================================
+ Hits        44134    44136    +2     
  Misses      18716    18716           
+ Partials     2573     2570    -3

Flag	Coverage Δ
Linux_1	`32.14% <ø> (ø)`
Linux_2	`55.58% <ø> (ø)`
Linux_3	`44.92% <100.00%> (+<0.01%)`	⬆️
Linux_4	`34.98% <ø> (ø)`
Windows_1	`32.16% <ø> (ø)`
Windows_2	`55.54% <ø> (ø)`
Windows_3	`44.92% <100.00%> (+<0.01%)`	⬆️
Windows_4	`34.98% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Signed-off-by: Craig Perkins <[email protected]>

BionIT · 2024-03-28T02:12:58Z

src/plugins/data_source/server/legacy/client_config.ts

@@ -55,7 +55,7 @@ export function parseClientOptions(
      config.ssl?.certificateAuthorities
    );

-    sslConfig.ca = certificateAuthorities || [];
+    sslConfig.ca = certificateAuthorities;


we also need to fix the interface to make ca as optional I think, at line 16

That should already be covered because DataSourceSSLConfigOptions is a Partial

BionIT · 2024-03-28T02:13:53Z

src/plugins/data_source/server/client/client_config.ts

@@ -56,7 +56,7 @@ export function parseClientOptions(
      config.ssl?.certificateAuthorities
    );

-    sslConfig.ca = certificateAuthorities || [];
+    sslConfig.ca = certificateAuthorities;


we also need to fix the interface to make ca as optional I think, at line 16

That should already be covered because DataSourceSSLConfigOptions is a Partial

Oh I see, thanks!

BionIT · 2024-03-28T18:29:55Z

@cwperks Can you fix the test which is failing https://github.com/opensearch-project/OpenSearch-Dashboards/actions/runs/8459709350/job/23211770139?pr=6282

Signed-off-by: Craig Perkins <[email protected]>

cwperks · 2024-03-28T19:46:01Z

@cwperks Can you fix the test which is failing https://github.com/opensearch-project/OpenSearch-Dashboards/actions/runs/8459709350/job/23211770139?pr=6282

@BionIT Pushed a fix for the tests.

cwperks · 2024-03-29T17:25:05Z

Creating a manual backport. The backport bot still has not run after the label was added.

…rities is unset (#6282) * Fix sslConfig for multiple datasource to handle when certificateAuthorities is unset Signed-off-by: Craig Perkins <[email protected]> * Add to CHANGELOG Signed-off-by: Craig Perkins <[email protected]> * Adjust test in tls_settings_provider.test.ts Signed-off-by: Craig Perkins <[email protected]> --------- Signed-off-by: Craig Perkins <[email protected]> (cherry picked from commit 40da92c) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md

…rities is unset (opensearch-project#6282) * Fix sslConfig for multiple datasource to handle when certificateAuthorities is unset Signed-off-by: Craig Perkins <[email protected]> * Add to CHANGELOG Signed-off-by: Craig Perkins <[email protected]> * Adjust test in tls_settings_provider.test.ts Signed-off-by: Craig Perkins <[email protected]> --------- Signed-off-by: Craig Perkins <[email protected]> (cherry picked from commit 40da92c)

…rities is unset (#6282) (#6296) * Fix sslConfig for multiple datasource to handle when certificateAuthorities is unset Signed-off-by: Craig Perkins <[email protected]> * Add to CHANGELOG Signed-off-by: Craig Perkins <[email protected]> * Adjust test in tls_settings_provider.test.ts Signed-off-by: Craig Perkins <[email protected]> --------- Signed-off-by: Craig Perkins <[email protected]> (cherry picked from commit 40da92c) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Fix sslConfig for multiple datasource to handle when certificateAutho…

534ff45

…rities is unset Signed-off-by: Craig Perkins <[email protected]>

github-actions bot added the repeat-contributor label Mar 27, 2024

cwperks mentioned this pull request Mar 27, 2024

[BUG] Add datasource with test connect failed #6281

Closed

Add to CHANGELOG

b399ea9

Signed-off-by: Craig Perkins <[email protected]>

cwperks marked this pull request as ready for review March 27, 2024 22:55

cwperks requested review from ananzh, kavilla, AMoo-Miki, ashwin-pc, joshuarrrr, abbyhu2000, zengyan-amazon, zhongnansu, manasvinibs, ZilongX, Flyingliuhub, BSFishy, curq, bandinib-amzn, SuZhou-Joe, ruanyl, BionIT and xinruiba as code owners March 27, 2024 22:55

BionIT reviewed Mar 28, 2024

View reviewed changes

BionIT previously approved these changes Mar 28, 2024

View reviewed changes

ZilongX previously approved these changes Mar 28, 2024

View reviewed changes

Adjust test in tls_settings_provider.test.ts

d1ed005

Signed-off-by: Craig Perkins <[email protected]>

cwperks dismissed ZilongX’s stale review via d1ed005 March 28, 2024 19:45

cwperks dismissed BionIT’s stale review via d1ed005 March 28, 2024 19:45

BionIT approved these changes Mar 28, 2024

View reviewed changes

ZilongX self-requested a review March 28, 2024 23:19

ZilongX approved these changes Mar 28, 2024

View reviewed changes

ZilongX merged commit 40da92c into opensearch-project:main Mar 28, 2024
70 checks passed

ZilongX added the backport 2.x label Mar 29, 2024

opensearch-trigger-bot bot mentioned this pull request Mar 29, 2024

[Backport 2.x] Fix sslConfig for multiple datasource to handle when certificateAuthorities is unset #6296

Merged

cwperks mentioned this pull request Mar 29, 2024

[Backport 2.x] Fix sslConfig for multiple datasource to handle when certificateAuthorities is unset (#6282) #6297

Closed

kavilla linked an issue Apr 2, 2024 that may be closed by this pull request

[BUG] Add datasource with test connect failed #6281

Closed

kavilla added the v2.14.0 label Apr 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix sslConfig for multiple datasource to handle when certificateAuthorities is unset #6282

Fix sslConfig for multiple datasource to handle when certificateAuthorities is unset #6282

cwperks commented Mar 27, 2024 •

edited

Loading

codecov bot commented Mar 27, 2024 •

edited

Loading

BionIT Mar 28, 2024

cwperks Mar 28, 2024

BionIT Mar 28, 2024

cwperks Mar 28, 2024

BionIT Mar 28, 2024

BionIT commented Mar 28, 2024

cwperks commented Mar 28, 2024

cwperks commented Mar 29, 2024

Fix sslConfig for multiple datasource to handle when certificateAuthorities is unset #6282

Fix sslConfig for multiple datasource to handle when certificateAuthorities is unset #6282

Conversation

cwperks commented Mar 27, 2024 • edited Loading

Description

Issues Resolved

Check List

codecov bot commented Mar 27, 2024 • edited Loading

Codecov Report

BionIT Mar 28, 2024

Choose a reason for hiding this comment

cwperks Mar 28, 2024

Choose a reason for hiding this comment

BionIT Mar 28, 2024

Choose a reason for hiding this comment

cwperks Mar 28, 2024

Choose a reason for hiding this comment

BionIT Mar 28, 2024

Choose a reason for hiding this comment

BionIT commented Mar 28, 2024

cwperks commented Mar 28, 2024

cwperks commented Mar 29, 2024

cwperks commented Mar 27, 2024 •

edited

Loading

codecov bot commented Mar 27, 2024 •

edited

Loading