-
Notifications
You must be signed in to change notification settings - Fork 885
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
[MD]Use placeholder for data source credentials fields when export saved object #6928
[MD]Use placeholder for data source credentials fields when export saved object #6928
Conversation
❌ Empty Changelog SectionThe Changelog section in your PR description is empty. Please add a valid changelog entry or entries. If you did add a changelog entry, check to make sure that it was not accidentally included inside the comment block in the Changelog section. |
❌ Empty Changelog SectionThe Changelog section in your PR description is empty. Please add a valid changelog entry or entries. If you did add a changelog entry, check to make sure that it was not accidentally included inside the comment block in the Changelog section. |
5ed194d
to
438daa7
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6928 +/- ##
=======================================
Coverage 67.42% 67.42%
=======================================
Files 3443 3443
Lines 67770 67782 +12
Branches 11023 11027 +4
=======================================
+ Hits 45693 45703 +10
- Misses 19412 19413 +1
- Partials 2665 2666 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@zhongnansu is this PR ready for review? |
… saved object Signed-off-by: Zhongnan Su <szhongna@amazon.com>
2a844de
to
fe8a800
Compare
const hasCredentials = auth && auth.credentials; | ||
const updatedCredentials = hasCredentials | ||
? Object.keys(auth.credentials).reduce((acc, key) => { | ||
acc[key] = DATA_SOURCE_CREDENTIALS_PLACEHOLDER; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this change~
Just one question, SigV4 and TokenExchange also have regions under credential attribute, will this logic have any side effect?
Maybe we can also test SigV4 scenario while waiting CI to complete? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
every field under "credentials" attributes will be replaced to use placeholder value, including region. I have tested sigV4 case, you can find it from the video. The exported sigV4 data source will look like this
{
"attributes": {
"auth": {
"credentials": {
"accessKey": "pleaseUpdateCredentials",
"region": "pleaseUpdateCredentials",
"secretKey": "pleaseUpdateCredentials",
"service": "pleaseUpdateCredentials"
},
"type": "sigv4"
},
"dataSourceVersion": "",
"description": "",
"endpoint": "https://mjdty4727oifwax8ycdk.us-west-2.aoss.amazonaws.com",
"installedPlugins": [],
"title": "testSigV4"
},
"id": "7f8e13c0-2378-11ef-a345-bd55de69299f",
"migrationVersion": {
"data-source": "2.4.0"
},
"references": [],
"type": "data-source",
"updated_at": "2024-06-05T20:16:31.484Z",
"version": "WzQsMV0="
}
…ved object (#6928) * [MD]Use placeholder for data source credentials fields when exporting saved object Signed-off-by: Zhongnan Su <szhongna@amazon.com> * Changeset file for PR #6928 created/updated --------- Signed-off-by: Zhongnan Su <szhongna@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 0188efe) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…ved object (#6928) (#6931) * [MD]Use placeholder for data source credentials fields when exporting saved object * Changeset file for PR #6928 created/updated --------- (cherry picked from commit 0188efe) Signed-off-by: Zhongnan Su <szhongna@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
Description
We want to avoid exposing data source credentials during export, even tho they are encrypted values.
The data source objects in the exported
nd.json
file will look like below, with credentials fields replaced with placeholder values. Import will still succeed.Issues Resolved
#6892
Screenshot
Testing the changes
Changelog
Check List
yarn test:jest
yarn test:jest_integration