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

fix(elasticsearch): Vpc.fromLookup is unusable in Domain construct #12242

Closed
wants to merge 4 commits into from

Conversation

iliapolo
Copy link
Contributor

The Domain construct accepts a list of subnets and performs a validation on them. When using subnets that come from a looked up VPC, the VPC properties that are used during the first synthesis cycle is that of the dummy VPC.

This means that we cannot perform such a validation blindly without considering the VPC might be a dummy, only to be retrieved during the second synthesis cycle.

Im open for other suggestions on how to implement this because i'm definitely not in love with the current solution.

Fixes #12078


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

@gitpod-io
Copy link

gitpod-io bot commented Dec 27, 2020

@iliapolo
Copy link
Contributor Author

This also begs another question, should we maybe move the context lookup away from the CLI and into the framework, invoking it just before the validate step? This would make it possible to put these validations in the validate phase of a construct.

@github-actions github-actions bot added the @aws-cdk/aws-elasticsearch Related to Amazon Elasticsearch Service label Dec 27, 2020
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Dec 27, 2020
@iliapolo iliapolo requested a review from a team December 27, 2020 12:28
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 4750bc6
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@iliapolo iliapolo marked this pull request as draft December 28, 2020 13:42
@iliapolo iliapolo closed this Dec 28, 2020
@iliapolo iliapolo deleted the epolon/elasticsearch-domain-vpc-from-lookup branch December 28, 2020 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-elasticsearch Related to Amazon Elasticsearch Service contribution/core This is a PR that came from AWS.
Projects
None yet
2 participants