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

aws-opensearchservice: Domain creates new ResourcePolicy if logging enabled #23637

Closed
andresblancomorales opened this issue Jan 10, 2023 · 5 comments · Fixed by #28707
Closed
Labels
@aws-cdk/aws-opensearch Related to the @aws-cdk/aws-opensearchservice package bug This issue is a bug. effort/medium Medium work item – several days of effort p2

Comments

@andresblancomorales
Copy link

Describe the bug

Creating a new OpenSearch domain creates a new ResourcePolicy every time if any of: slowSearchLogEnabled, slowIndexLogEnabled, appLogEnabled, auditLogEnabled is true.

According to the CloudWatch limits:

Up to 10 CloudWatch Logs resource policies per Region per account. This quota can't be changed.

We're hitting this quota easily since every domain we create creates a new ResourcePolicy

Expected Behavior

Not sure what what would be a good expected behavior.

May be: Reuse a previously created ResourcePolicy and add the new log policy statement.

Current Behavior

A new resource policy is created with every new domain that is created with logging enabled.

Reproduction Steps

Instantiate a new Domain and set any of the log types as true:

                 const domain = new Domain(stack, 'TheDomain', {
                      version: EngineVersion.OPENSEARCH_1_2,
                      logging: {
                          appLogEnabled: true
                      }
                  })

Possible Solution

Reuse a previously created ResourcePolicy and add the new log policy statement.

Additional Information/Context

No response

CDK CLI Version

1.187.0

Framework Version

1.187.0

Node.js Version

16.19.0

OS

Linux

Language

Typescript

Language Version

4.4.3

Other information

No response

@andresblancomorales andresblancomorales added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 10, 2023
@github-actions github-actions bot added the @aws-cdk/aws-opensearch Related to the @aws-cdk/aws-opensearchservice package label Jan 10, 2023
@pahud
Copy link
Contributor

pahud commented Feb 22, 2023

related to #22307

Yes we are aware of the resource policy number limit and we definitely should have a workaround for that.

@pahud pahud added p2 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Feb 22, 2023
@keenangraham
Copy link

This is an issue for us too.

We run into a hard AWS limit of ten Opensearch domains per region when logging is enabled, preventing us from spinning up as many demos as we need for development.

https://docs.aws.amazon.com/opensearch-service/latest/developerguide/createdomain-configure-slow-logs.html#createdomain-configure-slow-logs-cli says:

Important
CloudWatch Logs supports 10 resource policies per Region. If you plan to enable slow logs for several OpenSearch Service domains, you should create and reuse a broader policy that includes multiple log groups to avoid reaching this limit.

Looking at the CDK code a new CustomResource LogGroupResourcePolicy is created automatically for every domain (when any logging is enabled) without the ability to prevent its creation or use a broader policy:

if (logGroups.length > 0) {
const logPolicyStatement = new iam.PolicyStatement({
effect: iam.Effect.ALLOW,
actions: ['logs:PutLogEvents', 'logs:CreateLogStream'],
resources: logGroups.map((lg) => lg.logGroupArn),
principals: [new iam.ServicePrincipal('es.amazonaws.com')],
});
// Use a custom resource to set the log group resource policy since it is not supported by CDK and cfn.
// https://github.com/aws/aws-cdk/issues/5343
logGroupResourcePolicy = new LogGroupResourcePolicy(this, `ESLogGroupPolicy${this.node.addr}`, {
// create a cloudwatch logs resource policy name that is unique to this domain instance
policyName: `ESLogPolicy${this.node.addr}`,
policyStatements: [logPolicyStatement],
});
}

Would it be possible to use a previously created broader resource policy per the suggestion in the docs, or somehow stop the CDK from trying to create a new one if logs are enabled?

@XiaowenMaoA
Copy link

what's the workaround here?

@sakurai-ryo
Copy link
Contributor

sakurai-ryo commented Jan 14, 2024

@XiaowenMaoA
Could you try this code as a workaround until the PR is merged?

    const domain = new opensearch.Domain(this, 'Domain', domainProps);
    const domainResource = domain.node.defaultChild as opensearch.CfnDomain;
    domainResource.addOverride('DependsOn', undefined);

    domain.node.children
      .filter(child => child instanceof AwsCustomResource)
      .forEach(value => domain.node.tryRemoveChild(value.node.id));

@mergify mergify bot closed this as completed in #28707 Jan 25, 2024
mergify bot pushed a commit that referenced this issue Jan 25, 2024
…ogging is enabled (#28707)

This PR adds an option to suppress the creation of logs resource policy when logging is enabled.

### Description
Currently, a CloudWatch Logs resource policy is created by default when the Domain logging is enabled.
However, since only ten resource policies can be created per region, deploying multiple Domains may cause errors.
The `tryRemoveChild` method can be used as a workaround to delete custom resources, but a better user experience is desirable.
```ts
    const domain = new opensearch.Domain(this, 'Domain', domainProps);
    const domainResource = domain.node.defaultChild as opensearch.CfnDomain;
    domainResource.addOverride('DependsOn', undefined); // remove dependency on the custom resource

    domain.node.children
      .filter(child => child instanceof AwsCustomResource)
      .forEach(value => domain.node.tryRemoveChild(value.node.id));
```

So, I add an option to suppress the creation of resource policies.
This option allows users to reuse a broader resource policy and successfully deploy several domains.
https://docs.aws.amazon.com/opensearch-service/latest/developerguide/createdomain-configure-slow-logs.html#:~:text=Resource%22%3A%20%22cw_log_group_arn%3A*%22%7D%5D%7D%27-,Important,-CloudWatch%20Logs%20supports

Closes #23637

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Vandita2020 pushed a commit to Vandita2020/aws-cdk that referenced this issue Jan 30, 2024
…ogging is enabled (aws#28707)

This PR adds an option to suppress the creation of logs resource policy when logging is enabled.

### Description
Currently, a CloudWatch Logs resource policy is created by default when the Domain logging is enabled.
However, since only ten resource policies can be created per region, deploying multiple Domains may cause errors.
The `tryRemoveChild` method can be used as a workaround to delete custom resources, but a better user experience is desirable.
```ts
    const domain = new opensearch.Domain(this, 'Domain', domainProps);
    const domainResource = domain.node.defaultChild as opensearch.CfnDomain;
    domainResource.addOverride('DependsOn', undefined); // remove dependency on the custom resource

    domain.node.children
      .filter(child => child instanceof AwsCustomResource)
      .forEach(value => domain.node.tryRemoveChild(value.node.id));
```

So, I add an option to suppress the creation of resource policies.
This option allows users to reuse a broader resource policy and successfully deploy several domains.
https://docs.aws.amazon.com/opensearch-service/latest/developerguide/createdomain-configure-slow-logs.html#:~:text=Resource%22%3A%20%22cw_log_group_arn%3A*%22%7D%5D%7D%27-,Important,-CloudWatch%20Logs%20supports

Closes aws#23637

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
SankyRed pushed a commit that referenced this issue Feb 8, 2024
…ogging is enabled (#28707)

This PR adds an option to suppress the creation of logs resource policy when logging is enabled.

### Description
Currently, a CloudWatch Logs resource policy is created by default when the Domain logging is enabled.
However, since only ten resource policies can be created per region, deploying multiple Domains may cause errors.
The `tryRemoveChild` method can be used as a workaround to delete custom resources, but a better user experience is desirable.
```ts
    const domain = new opensearch.Domain(this, 'Domain', domainProps);
    const domainResource = domain.node.defaultChild as opensearch.CfnDomain;
    domainResource.addOverride('DependsOn', undefined); // remove dependency on the custom resource

    domain.node.children
      .filter(child => child instanceof AwsCustomResource)
      .forEach(value => domain.node.tryRemoveChild(value.node.id));
```

So, I add an option to suppress the creation of resource policies.
This option allows users to reuse a broader resource policy and successfully deploy several domains.
https://docs.aws.amazon.com/opensearch-service/latest/developerguide/createdomain-configure-slow-logs.html#:~:text=Resource%22%3A%20%22cw_log_group_arn%3A*%22%7D%5D%7D%27-,Important,-CloudWatch%20Logs%20supports

Closes #23637

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-opensearch Related to the @aws-cdk/aws-opensearchservice package bug This issue is a bug. effort/medium Medium work item – several days of effort p2
Projects
None yet
5 participants