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.

Sign up for GitHub

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

Jump to bottom

[MD][Cred mngment] - PoC Create credential #9

Merged
merged 3 commits into from
Jul 9, 2022
Merged

[MD][Cred mngment] - PoC Create credential #9

merged 3 commits into from
Jul 9, 2022

Conversation

noCharger
Copy link
Collaborator

Description

  1. Route to creation page from list page
  2. Create credential with password encrypted

Screen Shot 2022-07-07 at 9 44 38 AM
Screen Shot 2022-07-07 at 9 44 13 AM

seraphjiang
seraphjiang reviewed Jul 8, 2022
View reviewed changes
...redential_management/public/components/create_credential_wizard/components/header/header.tsx Outdated
<p>
<FormattedMessage
id="credentialManagement.createCredential.description"
defaultMessage="An credential can be attached to multiple sources. For example, {credential} can be attached to two data sources {first} and {second}."
Copy link

Choose a reason for hiding this comment

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

typo An credential

seraphjiang
seraphjiang reviewed Jul 8, 2022
View reviewed changes
src/plugins/credential_management/public/plugin.ts
Comment on lines +65 to +76
const legacyPatternsPath = 'management/opensearch-dashboards/credentials';

urlForwarding.forwardApp(
'management/opensearch-dashboards/credentials',
newAppPath,
(path) => '/create'
);

urlForwarding.forwardApp(legacyPatternsPath, newAppPath, (path) => {
const pathInApp = path.substr(legacyPatternsPath.length + 1);
return pathInApp && `/patterns${pathInApp}`;
});
Copy link

Choose a reason for hiding this comment

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

there is no need to support legacyPath,

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is used for Dock navigation:

Screen Shot 2022-07-08 at 7 58 45 PM

Copy link

Choose a reason for hiding this comment

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

I also doubt if legacy path support is necessary

@noCharger noCharger merged commit ff5d568 into 2.x Jul 9, 2022
@noCharger noCharger deleted the feature-create-credential branch July 9, 2022 03:26
@seraphjiang
Copy link

try save a credential, but got error?

i guess it is because missing crypto_material?

if so, how could i got a working dummy crypto_material file?

server    log   [00:56:48.119] [info][plugins-system] Stopping all plugins.
 restarting server  due to changes in "src/plugins/credential_management/server/crypto/crypto_material"
server    log   [00:56:51.422] [info][plugins-service] Plugin "visTypeXy" is disabled.
server    log   [00:56:51.514] [info][plugins-system] Setting up [38] plugins: [usageCollection,opensearchDashboardsUsageCollection,opensearchDashboardsLegacy,mapsLegacy,share,opensearchUiShared,legacyExport,embeddable,expressions,data,home,console,apmOss,management,indexPatternManagement,dataSourceManagement,advancedSettings,savedObjects,dashboard,visualizations,visTypeVega,visTypeTimeline,timeline,visTypeTable,visTypeMarkdown,tileMap,regionMap,inputControlVis,visualize,credentialManagement,charts,visTypeVislib,visTypeTimeseries,visTypeTagcloud,visTypeMetric,discover,savedObjectsManagement,bfetch]
server    log   [00:56:51.722] [info][savedobjects-service] Waiting until all OpenSearch nodes are compatible with OpenSearch Dashboards before starting saved objects migrations...
server    log   [00:56:51.743] [info][savedobjects-service] Starting saved objects migrations
server    log   [00:56:51.787] [info][plugins-system] Starting [38] plugins: [usageCollection,opensearchDashboardsUsageCollection,opensearchDashboardsLegacy,mapsLegacy,share,opensearchUiShared,legacyExport,embeddable,expressions,data,home,console,apmOss,management,indexPatternManagement,dataSourceManagement,advancedSettings,savedObjects,dashboard,visualizations,visTypeVega,visTypeTimeline,timeline,visTypeTable,visTypeMarkdown,tileMap,regionMap,inputControlVis,visualize,credentialManagement,charts,visTypeVislib,visTypeTimeseries,visTypeTagcloud,visTypeMetric,discover,savedObjectsManagement,bfetch]
server    log   [00:56:51.972] [info][listening] Server running at http://localhost:5601
server    log   [00:56:52.006] [info][server][OpenSearchDashboards][http] http server running at http://localhost:5601
server    log   [00:57:04.225] [error][http] SyntaxError: Unexpected end of JSON input
    at JSON.parse (<anonymous>)
    at new CryptoCli (/Users/huanji/src/ea-osd/src/plugins/credential_management/server/crypto/cli/crypto_cli.ts:35:34)
    at Function.getInstance (/Users/huanji/src/ea-osd/src/plugins/credential_management/server/crypto/cli/crypto_cli.ts:66:29)
    at createHandler (/Users/huanji/src/ea-osd/src/plugins/credential_management/server/credential/credential_manager.ts:21:31)
    at handler (/Users/huanji/src/ea-osd/src/plugins/credential_management/server/routes/create.ts:44:32)
    at /Users/huanji/src/ea-osd/src/core/utils/context.ts:259:14
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at Router.handle (/Users/huanji/src/ea-osd/src/core/server/http/router/router.ts:286:44)
    at handler (/Users/huanji/src/ea-osd/src/core/server/http/router/router.ts:241:11)
    at exports.Manager.execute (/Users/huanji/src/ea-osd/node_modules/@hapi/hapi/lib/toolkit.js:60:28)
    at Object.internals.handler (/Users/huanji/src/ea-osd/node_modules/@hapi/hapi/lib/handler.js:46:20)
    at exports.execute (/Users/huanji/src/ea-osd/node_modules/@hapi/hapi/lib/handler.js:31:20)
    at Request._lifecycle (/Users/huanji/src/ea-osd/node_modules/@hapi/hapi/lib/request.js:371:32)
    at Request._execute (/Users/huanji/src/ea-osd/node_modules/@hapi/hapi/lib/request.js:281:9)
server   error  [00:57:04.221]  Error: Internal Server Error
    at HapiResponseAdapter.toInternalError (/Users/huanji/src/ea-osd/src/core/server/http/router/response_adapter.ts:71:19)
    at Router.handle (/Users/huanji/src/ea-osd/src/core/server/http/router/router.ts:303:34)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at handler (/Users/huanji/src/ea-osd/src/core/server/http/router/router.ts:241:11)
    at exports.Manager.execute (/Users/huanji/src/ea-osd/node_modules/@hapi/hapi/lib/toolkit.js:60:28)
    at Object.internals.handler (/Users/huanji/src/ea-osd/node_modules/@hapi/hapi/lib/handler.js:46:20)
    at exports.execute (/Users/huanji/src/ea-osd/node_modules/@hapi/hapi/lib/handler.js:31:20)
    at Request._lifecycle (/Users/huanji/src/ea-osd/node_modules/@hapi/hapi/lib/request.js:371:32)
    at Request._execute (/Users/huanji/src/ea-osd/node_modules/@hapi/hapi/lib/request.js:281:9)

image

@seraphjiang
Copy link

don't quite understand what does credential file do? why do we need this?

image

@noCharger
Copy link
Collaborator Author

try save a credential, but got error?

i guess it is because missing crypto_material?

if so, how could i got a working dummy crypto_material file?

server    log   [00:56:48.119] [info][plugins-system] Stopping all plugins.
 restarting server  due to changes in "src/plugins/credential_management/server/crypto/crypto_material"
server    log   [00:56:51.422] [info][plugins-service] Plugin "visTypeXy" is disabled.
server    log   [00:56:51.514] [info][plugins-system] Setting up [38] plugins: [usageCollection,opensearchDashboardsUsageCollection,opensearchDashboardsLegacy,mapsLegacy,share,opensearchUiShared,legacyExport,embeddable,expressions,data,home,console,apmOss,management,indexPatternManagement,dataSourceManagement,advancedSettings,savedObjects,dashboard,visualizations,visTypeVega,visTypeTimeline,timeline,visTypeTable,visTypeMarkdown,tileMap,regionMap,inputControlVis,visualize,credentialManagement,charts,visTypeVislib,visTypeTimeseries,visTypeTagcloud,visTypeMetric,discover,savedObjectsManagement,bfetch]
server    log   [00:56:51.722] [info][savedobjects-service] Waiting until all OpenSearch nodes are compatible with OpenSearch Dashboards before starting saved objects migrations...
server    log   [00:56:51.743] [info][savedobjects-service] Starting saved objects migrations
server    log   [00:56:51.787] [info][plugins-system] Starting [38] plugins: [usageCollection,opensearchDashboardsUsageCollection,opensearchDashboardsLegacy,mapsLegacy,share,opensearchUiShared,legacyExport,embeddable,expressions,data,home,console,apmOss,management,indexPatternManagement,dataSourceManagement,advancedSettings,savedObjects,dashboard,visualizations,visTypeVega,visTypeTimeline,timeline,visTypeTable,visTypeMarkdown,tileMap,regionMap,inputControlVis,visualize,credentialManagement,charts,visTypeVislib,visTypeTimeseries,visTypeTagcloud,visTypeMetric,discover,savedObjectsManagement,bfetch]
server    log   [00:56:51.972] [info][listening] Server running at http://localhost:5601
server    log   [00:56:52.006] [info][server][OpenSearchDashboards][http] http server running at http://localhost:5601
server    log   [00:57:04.225] [error][http] SyntaxError: Unexpected end of JSON input
    at JSON.parse (<anonymous>)
    at new CryptoCli (/Users/huanji/src/ea-osd/src/plugins/credential_management/server/crypto/cli/crypto_cli.ts:35:34)
    at Function.getInstance (/Users/huanji/src/ea-osd/src/plugins/credential_management/server/crypto/cli/crypto_cli.ts:66:29)
    at createHandler (/Users/huanji/src/ea-osd/src/plugins/credential_management/server/credential/credential_manager.ts:21:31)
    at handler (/Users/huanji/src/ea-osd/src/plugins/credential_management/server/routes/create.ts:44:32)
    at /Users/huanji/src/ea-osd/src/core/utils/context.ts:259:14
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at Router.handle (/Users/huanji/src/ea-osd/src/core/server/http/router/router.ts:286:44)
    at handler (/Users/huanji/src/ea-osd/src/core/server/http/router/router.ts:241:11)
    at exports.Manager.execute (/Users/huanji/src/ea-osd/node_modules/@hapi/hapi/lib/toolkit.js:60:28)
    at Object.internals.handler (/Users/huanji/src/ea-osd/node_modules/@hapi/hapi/lib/handler.js:46:20)
    at exports.execute (/Users/huanji/src/ea-osd/node_modules/@hapi/hapi/lib/handler.js:31:20)
    at Request._lifecycle (/Users/huanji/src/ea-osd/node_modules/@hapi/hapi/lib/request.js:371:32)
    at Request._execute (/Users/huanji/src/ea-osd/node_modules/@hapi/hapi/lib/request.js:281:9)
server   error  [00:57:04.221]  Error: Internal Server Error
    at HapiResponseAdapter.toInternalError (/Users/huanji/src/ea-osd/src/core/server/http/router/response_adapter.ts:71:19)
    at Router.handle (/Users/huanji/src/ea-osd/src/core/server/http/router/router.ts:303:34)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at handler (/Users/huanji/src/ea-osd/src/core/server/http/router/router.ts:241:11)
    at exports.Manager.execute (/Users/huanji/src/ea-osd/node_modules/@hapi/hapi/lib/toolkit.js:60:28)
    at Object.internals.handler (/Users/huanji/src/ea-osd/node_modules/@hapi/hapi/lib/handler.js:46:20)
    at exports.execute (/Users/huanji/src/ea-osd/node_modules/@hapi/hapi/lib/handler.js:31:20)
    at Request._lifecycle (/Users/huanji/src/ea-osd/node_modules/@hapi/hapi/lib/request.js:371:32)
    at Request._execute (/Users/huanji/src/ea-osd/node_modules/@hapi/hapi/lib/request.js:281:9)

image

Check readme - https://github.com/elastic-analytics/OpenSearch-Dashboards/tree/2.x/src/plugins/credential_management

@noCharger
Copy link
Collaborator Author

noCharger commented Jul 10, 2022 •
edited
Loading

don't quite understand what does credential file do? why do we need this?

image

It's placeholder allowing user to upload credential file instead of entering it.

@seraphjiang
Copy link

good idea, but i'm not sure if there is industry standard for such kind of file type.

@seraphjiang
Copy link

Thanks, readme helped.

there are a few ux issues which we could work with ux to find better experience.

  1. we should call Save credential (not creation. we don't create any, it exist other place, we just enter and save )
  2. after saved, it is better to redirect to list page

@noCharger
Copy link
Collaborator Author

good idea, but i'm not sure if there is industry standard for such kind of file type.

Because it is not a top priority, I would remove this UI component for the time being and follow up later: opensearch-project#1872

@noCharger
Copy link
Collaborator Author

Thanks, readme helped.

there are a few ux issues which we could work with ux to find better experience.

  1. we should call Save credential (not creation. we don't create any, it exist other place, we just enter and save )
  2. after saved, it is better to redirect to list page

Thanks for the feedback, should included in the next PR.

zengyan-amazon
zengyan-amazon reviewed Jul 11, 2022
View reviewed changes
src/plugins/credential_management/common/types.ts Show resolved Hide resolved
src/plugins/credential_management/public/components/breadsrumbs.ts Show resolved Hide resolved
src/plugins/credential_management/public/components/breadsrumbs.ts Show resolved Hide resolved
...redential_management/public/components/create_credential_wizard/create_credential_wizard.tsx Show resolved Hide resolved
...redential_management/public/components/create_credential_wizard/create_credential_wizard.tsx
Comment on lines +204 to +209
.then((res) => {
console.log(res);
});
} catch (e) {
console.log(e);
}
Copy link

Choose a reason for hiding this comment

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

show a toast if error

src/plugins/credential_management/public/plugin.ts
Comment on lines +65 to +76
const legacyPatternsPath = 'management/opensearch-dashboards/credentials';

urlForwarding.forwardApp(
'management/opensearch-dashboards/credentials',
newAppPath,
(path) => '/create'
);

urlForwarding.forwardApp(legacyPatternsPath, newAppPath, (path) => {
const pathInApp = path.substr(legacyPatternsPath.length + 1);
return pathInApp && `/patterns${pathInApp}`;
});
Copy link

Choose a reason for hiding this comment

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

I also doubt if legacy path support is necessary

src/plugins/credential_management/server/routes/create.ts Show resolved Hide resolved
@noCharger noCharger restored the feature-create-credential branch July 11, 2022 19:57
@noCharger noCharger mentioned this pull request Jul 11, 2022
Merged
noCharger added a commit that referenced this pull request Jul 11, 2022
@noCharger
[MD][Cred Mgr]PoC delete credentials (#10)
2fa92e0 
1. PoC delete credentials
2. Resolve some UI feedbacks in #9
@noCharger
Copy link
Collaborator Author

I also doubt if legacy path support is necessary

Resolved in #12

@noCharger noCharger deleted the feature-create-credential branch July 18, 2022 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zengyan-amazon zengyan-amazon zengyan-amazon left review comments

@seraphjiang seraphjiang seraphjiang approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@noCharger @seraphjiang @zengyan-amazon