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

[Multi Data Source] Render credential form registered from AuthMethod #6002

Conversation

xinruiba
Copy link
Member

@xinruiba xinruiba commented Mar 2, 2024

Description

Partial of Refactoring create and edit form to use authentication registry user story.

  • Render credential form registered from AuthMethod

Issues Resolved

Partially fixes #5692, #5838

Screenshot

CredentialForm.mov

Testing the changes

Test cases added

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

@xinruiba xinruiba marked this pull request as ready for review March 2, 2024 20:45
@bandinib-amzn bandinib-amzn changed the title [TokenExchange] Render credential form registered from AuthMethod [Multi Data Source] Render credential form registered from AuthMethod Mar 3, 2024
@BionIT
Copy link
Collaborator

BionIT commented Mar 4, 2024

In #5987, we found a bug caused by test connection API schema would fail input validation due to missing credentials for no authentication method, I refactored the schema validation to validate based on auth tyep, then wonder for custom authentication type, how does test connection work now, can you add recording for the test connection before data source creation?

Copy link

codecov bot commented Mar 4, 2024

Codecov Report

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

Project coverage is 67.10%. Comparing base (9901bea) to head (b55ddb3).

Files Patch % Lines
...components/create_form/create_data_source_form.tsx 76.92% 2 Missing and 1 partial ⚠️
...rce/components/edit_form/edit_data_source_form.tsx 76.92% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6002      +/-   ##
==========================================
- Coverage   67.11%   67.10%   -0.02%     
==========================================
  Files        3315     3315              
  Lines       63904    63922      +18     
  Branches    10220    10220              
==========================================
  Hits        42892    42892              
- Misses      18528    18544      +16     
- Partials     2484     2486       +2     
Flag Coverage Δ
Linux_1 31.64% <ø> (ø)
Linux_2 55.07% <ø> (ø)
Linux_3 44.60% <76.92%> (+0.02%) ⬆️
Linux_4 35.17% <ø> (ø)
Windows_1 31.66% <ø> (-0.03%) ⬇️
Windows_2 55.04% <ø> (ø)
Windows_3 44.61% <76.92%> (+0.01%) ⬆️
Windows_4 35.17% <ø> (ø)

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.

@xinruiba
Copy link
Member Author

xinruiba commented Mar 4, 2024

In #5987, we found a bug caused by test connection API schema would fail input validation due to missing credentials for no authentication method, I refactored the schema validation to validate based on auth tyep, then wonder for custom authentication type, how does test connection work now, can you add recording for the test connection before data source creation?

Thanks for the comment.
Test connection for token exchange is still in progress.
Current auth method won't be effected and will still act as the same.

Signed-off-by: Xinrui Bai <[email protected]>
@Flyingliuhub Flyingliuhub added multiple datasource multiple datasource project backport 2.x labels Mar 4, 2024
@Flyingliuhub Flyingliuhub merged commit 5ef10da into opensearch-project:main Mar 5, 2024
68 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Mar 5, 2024
…#6002)

* [TokenExchange] Render credential form registered from AuthMethod

Signed-off-by: Xinrui Bai <[email protected]>

* [UT] Add unittest to test registered credential form get rendered in create datasource page

Signed-off-by: Xinrui Bai <[email protected]>

* [UT] Update  test case descriptions

Signed-off-by: Xinrui Bai <[email protected]>

* [Token Exchange] improve code format in create datasource page

Signed-off-by: Xinrui Bai <[email protected]>

* [UT] Add unit test for edit datasource page

Signed-off-by: Xinrui Bai <[email protected]>

* Update changelog file

Signed-off-by: Xinrui Bai <[email protected]>

* update yml config file to original status

Signed-off-by: Xinrui Bai <[email protected]>

* Resolving comments

Signed-off-by: Xinrui Bai <[email protected]>

* [UT] Add more unit test to cover existing auth type and plugin registered Auth type scenario

Signed-off-by: Xinrui Bai <[email protected]>

* Resolving comments, update pmport path

Signed-off-by: Xinrui Bai <[email protected]>

---------

Signed-off-by: Xinrui Bai <[email protected]>
(cherry picked from commit 5ef10da)
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 5, 2024
…#6002) (#6024)

* [TokenExchange] Render credential form registered from AuthMethod

Signed-off-by: Xinrui Bai <[email protected]>

* [UT] Add unittest to test registered credential form get rendered in create datasource page

Signed-off-by: Xinrui Bai <[email protected]>

* [UT] Update  test case descriptions

Signed-off-by: Xinrui Bai <[email protected]>

* [Token Exchange] improve code format in create datasource page

Signed-off-by: Xinrui Bai <[email protected]>

* [UT] Add unit test for edit datasource page

Signed-off-by: Xinrui Bai <[email protected]>

* Update changelog file

Signed-off-by: Xinrui Bai <[email protected]>

* update yml config file to original status

Signed-off-by: Xinrui Bai <[email protected]>

* Resolving comments

Signed-off-by: Xinrui Bai <[email protected]>

* [UT] Add more unit test to cover existing auth type and plugin registered Auth type scenario

Signed-off-by: Xinrui Bai <[email protected]>

* Resolving comments, update pmport path

Signed-off-by: Xinrui Bai <[email protected]>

---------

Signed-off-by: Xinrui Bai <[email protected]>
(cherry picked from commit 5ef10da)
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>
@xinruiba xinruiba deleted the xinrui_Token_Exchange_CredentialForm branch March 18, 2024 05:57
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
4 participants