fix(opensearch): correctly validate ebs configuration against instance types

[nom3ad]

Currently, the following valid cluster configurations are not possible to be created with CDK.

new es.Domain(stack, 'Domain1', {
    version: es.EngineVersion.OPENSEARCH_1_0,
    ebs: { enabled: false },
    capacity: {
        masterNodes: 3,
        masterNodeInstanceType: 'c5.2xlarge.search',
        dataNodeInstanceType: 'i3.2xlarge.search'
    }
});


new es.Domain(stack, 'Domain2', {
    version: es.EngineVersion.OPENSEARCH_1_0,
    ebs: { enabled: false, },
    capacity: {
        masterNodes: 3,
        masterNodeInstanceType: 'c6g.large.search',
        dataNodeInstanceType: 'r6gd.large.search'
    }
});

Throws error:
EBS volumes are required when using instance types other than r3, i3 or r6gd.

Here, EBS validation is being checked on master nodes instead of just the data nodes - which causes the above failure.

This PR applies fix on both elasticsearch and opensearchservice modules

Fixes #11898

---

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

[gitpod-io]

[nom3ad]

@kaizen3031593 Would you mind taking a look when you get time?

[kaizencc]

Hi @nom3ad, sorry for the delay! Thanks for the PR and double thanks for providing the fix for both elasticsearch and opensearch.

I read through the linked issue, and it seems like folks are/were reporting a Cloudformation validation even when using the CDK escape hatch. Given that's the case, I'm not convinced that doing anything on the CDK side will help alleviate those issues -- and I'm curious to hear how you've gotten past that!

My gut tells me that even if we go ahead and merge this PR, the Cloudformation validation will still be there. Please let me know if that's not the case!

[nom3ad]

@kaizen3031593 Thanks for looking in to it.

My gut tells me that even if we go ahead and merge this PR, the Cloudformation validation will still be there. Please let me know if that's not the case!

Hmm, I don't think CloudFormation is throwing a false positive validation error in this scenario. I was able to successfully work around CDK validation by #11898 (comment) and CloudFormation happily deployed it.
Likewise, I can confirm the examples provided in the PR description #16911 (comment) is indeed deployable as I actually tried them while working on the fix. :-)

I read through the linked issue, and it seems like folks are/were reporting a Cloudformation validation even when using the CDK escape hatch. Given that's the case, I'm not convinced that doing anything on the CDK side will help alleviate those issues -- and I'm curious to hear how you've gotten past that!

I have looked into the reported code snippet. It should generate a template as follows (removed some unrelated properties):

Resources:
  ElasticsearchCluster167EBEDC:
    Type: AWS::Elasticsearch::Domain
    Properties:
      EBSOptions:
        EBSEnabled: true   # data node instance type r5.large.elasticsearch works with EBS enabled but i3.large.elasticsearch does not.
        VolumeSize: 10           
        VolumeType: gp2
      ElasticsearchClusterConfig:
        DedicatedMasterCount: 3
        DedicatedMasterEnabled: true
        DedicatedMasterType: m5.xlarge.elasticsearch  # master instance type doesn't care about EBS configuration
        InstanceCount: 4
        InstanceType: r5.large.elasticsearch    # this is the default data node instance type when users don't provide one.
        ZoneAwarenessEnabled: false
      ElasticsearchVersion: "7.7"

And the above template is deployable too. But the reported error was:

Instance type i3.large.elasticsearch does not support EBS storage (Service: AWSElasticsearch; Status Code: 400; Error Code: ValidationException; Request ID: 0dad9072-8baa-4390-a519-0c3233656a56; Proxy: null)

As we can see i3.large.elasticsearch in the logs, it's clear that, provided snippet was not deployed intact. I guess the user patched InstanceType: r5.large.elasticsearch to i3.large.elasticsearch - which actually results in an invalid template - because i3.large does not support EBS.
So nothing to blame on CloudFormation in this particular case.

Hope this clears the concern.

(edit: typos)

[nom3ad]

@kaizen3031593 would you mind taking a look again?

[kaizencc]

Sorry for the delay (again). Thanks for the detailed reply, this does look like an excess CDK validation. Just going to add a comment in the tests to explain why we are happy to just assert that the domains exist in the template.

Pull request has been modified.

[mergify]

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
• Commit ID: 2cee82a
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).