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

fix dynamic config API calls to pass correct input #6474

Merged
merged 4 commits into from
Apr 16, 2024

Conversation

tianleh
Copy link
Member

@tianleh tianleh commented Apr 16, 2024

Description

During https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6364/files, we simplify the input to getConfigurationClient from a client to a request. We were focusing on the use case of other plugins to onboard onto it. However, we also have API use case which we missed during the related PR. So, this PR is to fix the issue by passing request into the function instead of a client.

Currently it is not failing because the asScoped function doesn't fail. Check the code reference here

It just treats it as using default headers and then doesn't pass user information to get a real scoped client.

Issues Resolved

Screenshot

Testing the changes

Enable the following in YML.

# Set the value of this setting to true to enable plugin application config. By default it is disabled.
application_config.enabled: true

# Set the value of this setting to true to enable plugin CSP handler. By default it is disabled.
# It requires the application config plugin as its dependency.
csp_handler.enabled: true


call Get API

curl -X GET 'http://localhost:5601/api/appconfig/csp.rules.frame-ancestors'
{"statusCode":404,"error":"Not Found","message":"Response Error"}%

call update API

curl 'http://localhost:5601/api/appconfig/csp.rules.frame-ancestors' -X POST -H 'Accept: application/json' -H 'Content-Type: application/json' -H 'osd-xsrf: osd-fetch' -H 'Sec-Fetch-Dest: empty' --data-raw '{"newValue":"file://* filesystem:"}'
{"newValue":"file://* filesystem:"}%

call get API again

curl -X GET 'http://localhost:5601/api/appconfig/csp.rules.frame-ancestors'
{"value":"file://* filesystem:"}%

call delete API

curl 'http://localhost:5601/api/appconfig/csp.rules.frame-ancestors' -X DELETE -H 'osd-xsrf: osd-fetch' -H 'Sec-Fetch-Dest: empty'
{"deletedEntity":"csp.rules.frame-ancestors"}%

call get API again.

curl -X GET 'http://localhost:5601/api/appconfig/csp.rules.frame-ancestors'
{"statusCode":404,"error":"Not Found","message":"Response Error"}%

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

@tianleh tianleh changed the title update dynamic API calls to pass correct input update dynamic config API calls to pass correct input Apr 16, 2024
Copy link

codecov bot commented Apr 16, 2024

Codecov Report

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

Project coverage is 62.68%. Comparing base (e785fd3) to head (17f78b2).
Report is 6 commits behind head on main.

❗ Current head 17f78b2 differs from pull request most recent head ddbcc36. Consider uploading reports for the commit ddbcc36 to get more accurate results

Files Patch % Lines
.../plugins/application_config/server/routes/index.ts 0.00% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #6474       +/-   ##
===========================================
+ Coverage   49.55%   62.68%   +13.12%     
===========================================
  Files        2670     2081      -589     
  Lines       54290    40763    -13527     
  Branches     8878     7455     -1423     
===========================================
- Hits        26906    25554     -1352     
+ Misses      25720    13565    -12155     
+ Partials     1664     1644       -20     
Flag Coverage Δ
Windows_1 ?
Windows_2 55.58% <ø> (?)
Windows_3 45.15% <0.00%> (+0.05%) ⬆️

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.

@tianleh tianleh changed the title update dynamic config API calls to pass correct input fix dynamic config API calls to pass correct input Apr 16, 2024
tianleh added 4 commits April 16, 2024 08:22
Signed-off-by: Tianle Huang <[email protected]>
Signed-off-by: Tianle Huang <[email protected]>
Signed-off-by: Tianle Huang <[email protected]>
@tianleh
Copy link
Member Author

tianleh commented Apr 16, 2024

@SuZhou-Joe Since you have some prior context about https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6364/files could you please help review this one? Thanks.

Copy link
Member

@SuZhou-Joe SuZhou-Joe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ZilongX
Copy link
Collaborator

ZilongX commented Apr 16, 2024

All checks passed and ready for merging

@ZilongX ZilongX merged commit 9a97b43 into opensearch-project:main Apr 16, 2024
93 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 16, 2024
* update dynamic API calls to pass correct input

Signed-off-by: Tianle Huang <[email protected]>

* add unit tests

Signed-off-by: Tianle Huang <[email protected]>

* add changelog

Signed-off-by: Tianle Huang <[email protected]>

* revert yml

Signed-off-by: Tianle Huang <[email protected]>

---------

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

# Conflicts:
#	CHANGELOG.md
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 16, 2024
* update dynamic API calls to pass correct input

Signed-off-by: Tianle Huang <[email protected]>

* add unit tests

Signed-off-by: Tianle Huang <[email protected]>

* add changelog

Signed-off-by: Tianle Huang <[email protected]>

* revert yml

Signed-off-by: Tianle Huang <[email protected]>

---------

Signed-off-by: Tianle Huang <[email protected]>
(cherry picked from commit 9a97b43)
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 Apr 16, 2024
* update dynamic API calls to pass correct input

Signed-off-by: Tianle Huang <[email protected]>

* add unit tests

Signed-off-by: Tianle Huang <[email protected]>

* add changelog

Signed-off-by: Tianle Huang <[email protected]>

* revert yml

Signed-off-by: Tianle Huang <[email protected]>

---------

Signed-off-by: Tianle Huang <[email protected]>
(cherry picked from commit 9a97b43)
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>
zhongnansu pushed a commit that referenced this pull request Apr 16, 2024
* update dynamic API calls to pass correct input

Signed-off-by: Tianle Huang <[email protected]>

* add unit tests

Signed-off-by: Tianle Huang <[email protected]>

* add changelog

Signed-off-by: Tianle Huang <[email protected]>

* revert yml

Signed-off-by: Tianle Huang <[email protected]>

---------

Signed-off-by: Tianle Huang <[email protected]>
(cherry picked from commit 9a97b43)
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.

3 participants