Refactor getClient and getLegacyClient to use authentication method registry #5881

bandinib-amzn · 2024-02-15T22:31:15Z

Description

Refactoring client and legacy client to use authentication registry.

Issues Resolved

Partially fixes #5692, #5838

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

codecov · 2024-02-15T22:44:45Z

Codecov Report

Attention: Patch coverage is 88.57143% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 67.00%. Comparing base (4ab0ca8) to head (d0b1d2b).

Files	Patch %	Lines
...gins/data_source/server/client/configure_client.ts	85.71%	0 Missing and 2 partials ⚠️
...ta_source/server/legacy/configure_legacy_client.ts	91.66%	0 Missing and 1 partial ⚠️
...ins/data_source/server/util/credential_provider.ts	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5881      +/-   ##
==========================================
- Coverage   67.01%   67.00%   -0.01%     
==========================================
  Files        3305     3307       +2     
  Lines       63585    63614      +29     
  Branches    10155    10163       +8     
==========================================
+ Hits        42611    42627      +16     
- Misses      18505    18518      +13     
  Partials     2469     2469

Flag	Coverage Δ
Linux_1	`35.21% <ø> (ø)`
Linux_2	`55.09% <ø> (ø)`
Linux_3	`43.63% <88.57%> (+0.04%)`	⬆️
Linux_4	`35.20% <ø> (ø)`
Windows_1	`35.24% <ø> (-0.04%)`	⬇️
Windows_2	`55.06% <ø> (ø)`
Windows_3	`43.63% <88.57%> (+0.04%)`	⬆️
Windows_4	`35.20% <ø> (ø)`

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.

BionIT · 2024-02-20T21:16:03Z

src/plugins/data_source/common/data_sources/types.ts

@@ -14,6 +14,7 @@ export interface DataSourceAttributes extends SavedObjectAttributes {
    credentials: UsernamePasswordTypedContent | SigV4Content | undefined | AuthTypeContent;
  };
  lastUpdatedTime?: string;
+  name: AuthType | string;


what's the difference of name vs auth.type?

Name can be any unique string to identify the authentication method.

auth.type is to specify using which method request needs to be signed. Right now we only have three ways: No auth, Basic authentication and AWS SigV4 authentication to sign the request.

So for example, I want to add new authentication method named abc for role based authentication. So my plugin will do all the work such as assume role etc to get temporary credentials (Access key, secret key and session token) and provide those data source plugin and will ask to sign request using AWS SigV4 with provided credentials.

I see, just curious not a blocker, do we show name on the UI or is it solely used as identifier internally?

BionIT · 2024-02-20T21:19:42Z

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

  } = dataSourceAttr;
+  const { endpoint: nodeUrl } = dataSourceAttr;


can we extract endpoint as before, eg. line 103

nodeUrl is never reassigned. Therefore I used const instead

bandinib-amzn · 2024-02-21T01:42:13Z

Failed functional test is not related to the code change in this PR.

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

src/plugins/data_source/server/client/configure_client.test.ts

…egistry Signed-off-by: Bandini Bhopi <[email protected]>

Signed-off-by: Bandini Bhopi <[email protected]>

opensearch-trigger-bot · 2024-02-23T03:07:22Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-5881-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 d56b04d0aac6aed21825f44eb70bb4a9b512dde0
# Push it to GitHub
git push --set-upstream origin backport/backport-5881-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-5881-to-2.x.

opensearch-trigger-bot · 2024-02-23T19:03:59Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-5881-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 d56b04d0aac6aed21825f44eb70bb4a9b512dde0
# Push it to GitHub
git push --set-upstream origin backport/backport-5881-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-5881-to-2.x.

…egistry (opensearch-project#5881) * Refactor getClient and getLegacyClient to use authentication method registry Signed-off-by: Bandini Bhopi <[email protected]> * Adds changelog and UT Signed-off-by: Bandini Bhopi <[email protected]> --------- Signed-off-by: Bandini Bhopi <[email protected]> (cherry picked from commit d56b04d)

…uthentication method registry (#5881) (#5940) * Refactor getClient and getLegacyClient to use authentication method registry (#5881) * Refactor getClient and getLegacyClient to use authentication method registry Signed-off-by: Bandini Bhopi <[email protected]> * Adds changelog and UT Signed-off-by: Bandini Bhopi <[email protected]> --------- Signed-off-by: Bandini Bhopi <[email protected]> (cherry picked from commit d56b04d) * Reverted accidental changes Signed-off-by: Bandini Bhopi <[email protected]> --------- Signed-off-by: Bandini Bhopi <[email protected]>

…egistry (opensearch-project#5881) * Refactor getClient and getLegacyClient to use authentication method registry Signed-off-by: Bandini Bhopi <[email protected]> * Adds changelog and UT Signed-off-by: Bandini Bhopi <[email protected]> --------- Signed-off-by: Bandini Bhopi <[email protected]>

manasvinibs · 2024-03-13T20:14:36Z

Manual backport #5940

github-actions bot added the seasoned-contributor label Feb 15, 2024

bandinib-amzn mentioned this pull request Feb 15, 2024

[Meta] Refactoring data source plugin to support add-on authentication method with plug-in module #5838

Closed

5 tasks

bandinib-amzn force-pushed the refactor-datasource-client branch from 3468a37 to c2c4bb9 Compare February 16, 2024 00:16

bandinib-amzn marked this pull request as ready for review February 16, 2024 00:58

bandinib-amzn requested review from ananzh, kavilla, AMoo-Miki, ashwin-pc, joshuarrrr, abbyhu2000, zengyan-amazon, kristenTian, zhongnansu, manasvinibs, ZilongX, Flyingliuhub, BSFishy, curq and SuZhou-Joe as code owners February 16, 2024 00:58

bandinib-amzn force-pushed the refactor-datasource-client branch from c2c4bb9 to 95f3a54 Compare February 20, 2024 20:47

BionIT reviewed Feb 20, 2024

View reviewed changes

BionIT approved these changes Feb 21, 2024

View reviewed changes

ZilongX previously approved these changes Feb 21, 2024

View reviewed changes

ZilongX added the multiple datasource multiple datasource project label Feb 21, 2024

bandinib-amzn force-pushed the refactor-datasource-client branch from 95f3a54 to f9ee790 Compare February 22, 2024 18:39

Flyingliuhub reviewed Feb 22, 2024

View reviewed changes

src/plugins/data_source/server/client/configure_client.ts Show resolved Hide resolved

Flyingliuhub previously approved these changes Feb 22, 2024

View reviewed changes

Flyingliuhub reviewed Feb 22, 2024

View reviewed changes

src/plugins/data_source/server/client/configure_client.test.ts Show resolved Hide resolved

bandinib-amzn force-pushed the refactor-datasource-client branch from f9ee790 to deac092 Compare February 22, 2024 23:20

bandinib-amzn added 2 commits February 23, 2024 00:27

Refactor getClient and getLegacyClient to use authentication method r…

f332650

…egistry Signed-off-by: Bandini Bhopi <[email protected]>

Adds changelog and UT

d0b1d2b

Signed-off-by: Bandini Bhopi <[email protected]>

bandinib-amzn dismissed stale reviews from ZilongX and Flyingliuhub via d0b1d2b February 23, 2024 00:28

bandinib-amzn force-pushed the refactor-datasource-client branch from deac092 to d0b1d2b Compare February 23, 2024 00:28

ZilongX self-requested a review February 23, 2024 00:31

ZilongX approved these changes Feb 23, 2024

View reviewed changes

Flyingliuhub approved these changes Feb 23, 2024

View reviewed changes

bandinib-amzn mentioned this pull request Feb 23, 2024

[BUG] Data source plugin authentication registry should not allow name of type AuthType. #5932

Closed

bandinib-amzn merged commit d56b04d into opensearch-project:main Feb 23, 2024
68 checks passed

bandinib-amzn deleted the refactor-datasource-client branch February 23, 2024 03:05

bandinib-amzn added backport 2.x v2.13.0 labels Feb 23, 2024

opensearch-trigger-bot bot added the failed backport label Feb 23, 2024

bandinib-amzn mentioned this pull request Feb 23, 2024

[Manual Backport 2.x] Refactor getClient and getLegacyClient to use authentication method registry (#5881) #5940

Merged

7 tasks

manasvinibs removed the failed backport label Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor getClient and getLegacyClient to use authentication method registry #5881

Refactor getClient and getLegacyClient to use authentication method registry #5881

bandinib-amzn commented Feb 15, 2024 •

edited

Loading

codecov bot commented Feb 15, 2024 •

edited

Loading

BionIT Feb 20, 2024

bandinib-amzn Feb 20, 2024

BionIT Feb 21, 2024

BionIT Feb 20, 2024

bandinib-amzn Feb 20, 2024

BionIT Feb 21, 2024

bandinib-amzn commented Feb 21, 2024

opensearch-trigger-bot bot commented Feb 23, 2024

opensearch-trigger-bot bot commented Feb 23, 2024

manasvinibs commented Mar 13, 2024

		} = dataSourceAttr;
		const { endpoint: nodeUrl } = dataSourceAttr;

Refactor getClient and getLegacyClient to use authentication method registry #5881

Refactor getClient and getLegacyClient to use authentication method registry #5881

Conversation

bandinib-amzn commented Feb 15, 2024 • edited Loading

Description

Issues Resolved

Check List

codecov bot commented Feb 15, 2024 • edited Loading

Codecov Report

BionIT Feb 20, 2024

Choose a reason for hiding this comment

bandinib-amzn Feb 20, 2024

Choose a reason for hiding this comment

BionIT Feb 21, 2024

Choose a reason for hiding this comment

BionIT Feb 20, 2024

Choose a reason for hiding this comment

bandinib-amzn Feb 20, 2024

Choose a reason for hiding this comment

BionIT Feb 21, 2024

Choose a reason for hiding this comment

bandinib-amzn commented Feb 21, 2024

opensearch-trigger-bot bot commented Feb 23, 2024

opensearch-trigger-bot bot commented Feb 23, 2024

manasvinibs commented Mar 13, 2024

bandinib-amzn commented Feb 15, 2024 •

edited

Loading

codecov bot commented Feb 15, 2024 •

edited

Loading