Skip to content
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

Handles auth methods from auth registry in DataSourceSavedObjectsClientWrapper #6062

Merged

Conversation

bandinib-amzn
Copy link
Member

@bandinib-amzn bandinib-amzn commented Mar 7, 2024

Description

This PR handles auth methods from auth registry in DataSourceSavedObjectsClientWrapper.

Issues Resolved

Partially fixes #5692, #5838

Screenshot

Testing the changes

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

Copy link

codecov bot commented Mar 7, 2024

Codecov Report

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

Project coverage is 67.12%. Comparing base (89fcf63) to head (80deb83).

❗ Current head 80deb83 differs from pull request most recent head b3382a5. Consider uploading reports for the commit b3382a5 to get more accurate results

Files Patch % Lines
...bjects/data_source_saved_objects_client_wrapper.ts 87.50% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main    #6062    +/-   ##
========================================
  Coverage   67.12%   67.12%            
========================================
  Files        3321     3319     -2     
  Lines       64085    64192   +107     
  Branches    10280    10308    +28     
========================================
+ Hits        43015    43090    +75     
- Misses      18566    18592    +26     
- Partials     2504     2510     +6     
Flag Coverage Δ
Linux_1 31.63% <ø> (ø)
Linux_2 55.20% <ø> (ø)
Linux_3 44.65% <87.50%> (+0.04%) ⬆️
Linux_4 35.14% <ø> (+0.04%) ⬆️
Windows_1 31.68% <ø> (+0.02%) ⬆️
Windows_2 55.17% <ø> (ø)
Windows_3 44.67% <87.50%> (+0.05%) ⬆️
Windows_4 35.14% <ø> (+0.04%) ⬆️

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.

Flyingliuhub
Flyingliuhub previously approved these changes Mar 8, 2024
@xinruiba
Copy link
Member

xinruiba commented Mar 8, 2024

Thanks for the change.
Do we also need to handle registered auth method here?

throw Error(`${type} is not a supported auth type for data source`);

@bandinib-amzn
Copy link
Member Author

Thanks for the change. Do we also need to handle registered auth method here?

throw Error(`${type} is not a supported auth type for data source`);

No. This should be fine. On server side how we store in auth registry is little bit different than browser side. On server side, type basically tells with which method request needs to sign. Authentication method can be anything. But there are only two ways we can sign the request - either Basic Auth or AWS SigV4 auth. or don't sign request at all which is No Auth. When we on server side, when authentication method will be registered from plugin or any other component, it will be registered as follows:

{
 name: 'customAuth',
authType: AuthType.SigV4 or AuthType.UsernamePassword,
credentialProvider: () => {retruns credentials required to sign the request}
}

@bandinib-amzn bandinib-amzn force-pushed the fix-create-data-source branch from 2c59b21 to 229da5b Compare March 8, 2024 02:47
Flyingliuhub
Flyingliuhub previously approved these changes Mar 8, 2024
Signed-off-by: Bandini Bhopi <[email protected]>
@bandinib-amzn bandinib-amzn merged commit 9f3a689 into opensearch-project:main Mar 8, 2024
55 checks passed
@bandinib-amzn bandinib-amzn deleted the fix-create-data-source branch March 8, 2024 02:59
opensearch-trigger-bot bot pushed a commit that referenced this pull request Mar 8, 2024
…ntWrapper (#6062)

* Handles auth methods from auth registry while saving data source

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

* Add more UT and remove repetitive code

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

* Adds changelog

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

* Refactor code

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

---------

Signed-off-by: Bandini Bhopi <[email protected]>
(cherry picked from commit 9f3a689)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
bandinib-amzn pushed a commit that referenced this pull request Mar 9, 2024
…ntWrapper (#6062)

* Handles auth methods from auth registry while saving data source

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

* Add more UT and remove repetitive code

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

* Adds changelog

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

* Refactor code

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

---------

Signed-off-by: Bandini Bhopi <[email protected]>
(cherry picked from commit 9f3a689)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
ZilongX pushed a commit that referenced this pull request Mar 9, 2024
…ntWrapper (#6062) (#6096)

* Handles auth methods from auth registry while saving data source

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

* Add more UT and remove repetitive code

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

* Adds changelog

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

* Refactor code

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

---------

Signed-off-by: Bandini Bhopi <[email protected]>
(cherry picked from commit 9f3a689)
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Proposal] Refactoring data source plugin to support add-on authentication method with plug-in module
5 participants