-
Notifications
You must be signed in to change notification settings - Fork 20
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
feat(consumption-opensearch): opensearch api as separate construct #694
base: main
Are you sure you want to change the base?
Conversation
bec0a8a
to
2bd8d2e
Compare
7f0bcbe
to
e211526
Compare
07ee0ba
to
99fa4b4
Compare
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.
Need a fix for license headers
7e95dbe
to
ef909fa
Compare
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.
few things to change in the documentation.
@@ -8,23 +8,22 @@ class ExampleOpenSearchApiStack extends cdk.Stack { | |||
constructor(scope: Construct, id: string , props:cdk.StackProps) { | |||
|
|||
super(scope, id, props); | |||
this.node.setContext('@data-solutions-framework-on-aws/removeDataOnDestroy', true); | |||
/// !show | |||
const osCluster = new dsf.consumption.OpenSearchCluster(this, 'MyOpenSearchCluster',{ |
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.
Can you change this to how the construct should be initialized, and not testing it through the openseach cluster construct. We have an example on kafka-api unit tests kafka-api.test.ts
|
||
The construct leverages the [CDK Provider Framework](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.custom_resources-readme.html#provider-framework) to deploy a custom resource to manage, and provide `addRoleMapping` and `callOpenSearchApi` methods. Both methods return the custom resource so that allows to enforce sequental execution of the API calls. By default all API calls will be executed simultaneously and are independent of each other. | ||
|
||
[example OpenSearch API](examples/opensearch-api.lit.ts) |
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.
Can you provide a dummy example without initializing the actual OS cluster with construct, but provide how to initialize the OS-api construct.
[OpenSearch Roles API](https://opensearch.org/docs/2.13/security/access-control/api#create-role-mapping) does not allow to update individual roles, requiring to pass array of roles that needs to be applied. | ||
To avoid overwriting prevously added roles `addRoleMapping` method provides `persist` parameter to store previously added roles inside the construct. To avoid racing conditions you also need to execute multiple `addRoleMapping` calls sequentionally as shown below. | ||
|
||
```typescript |
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.
Can you provide this through an example lit file.
8386018
to
539c6cb
Compare
539c6cb
to
2c05511
Compare
2c05511
to
684d202
Compare
Issue #, if available:
Description of changes:
Checklist
fix:
,feat:
,docs:
, ...)Breaking change checklist
RFC issue #:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.