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

add hook for deployment-time validation #1784

Closed
jesterhazy opened this issue Feb 18, 2019 · 2 comments · Fixed by #1807
Closed

add hook for deployment-time validation #1784

jesterhazy opened this issue Feb 18, 2019 · 2 comments · Fixed by #1807

Comments

@jesterhazy
Copy link
Contributor

jesterhazy commented Feb 18, 2019

cdk should provide a hook for deployment-time validation checks.

for example, consider an app that includes two stacks... a Dev stack with implicit environment, and a Prod stack with explicit prod account/region set.

When I run my stack with prod credentials, there is nothing to prevent me from deploying my Dev stack into my prod account (and probably causing many problems that way).

I'd like to add a check somewhere in the Dev stack code to say if (currentAccount === prodAccount) throw Error(...), but this means I the app doesn't work at all when run with Prod credentials (even cdk list throws the account check error).

Ideally, if I added this check in this new hook, I'd only get the error if i try diff/deploy/destroy the Dev stack.

@rix0rrr rix0rrr added the feature-request A feature should be added or improved. label Feb 19, 2019
@rix0rrr
Copy link
Contributor

rix0rrr commented Feb 19, 2019

I think your solution would be to not throw an Error but instead use

this.node.addError()

to attach an error to the current construct node. This will not abort your program but WILL prevent the stack from deploying.

@rix0rrr rix0rrr added documentation and removed feature-request A feature should be added or improved. labels Feb 19, 2019
@jesterhazy
Copy link
Contributor Author

jesterhazy commented Feb 20, 2019

@rix0rrr I tried the solution you suggested... every op, even cdk list throws the dev-related error if I run under prod credentials, so this won't work. since dev and prod stacks are in same app (but different stacks), this approach makes the whole app unusable if run with prod creds.

rix0rrr pushed a commit that referenced this issue Feb 20, 2019
If we report synthesis errors, they should only prevent the
affected stack from deploying; right now, since they are checked
during the synthesis step, they would abort the entire toolkit
run regardless of whether they would be selected or not.

Makes it possible to do region-dependent verification.

Fixes #1784.

ALSO IN THIS COMMIT

If the 'cdk synth' output goes to the screen, don't automatically
select upstream stacks for synthesis (as that would lead to an
immediate error).

Fixes #1783.
rix0rrr added a commit that referenced this issue Feb 22, 2019
If we report synthesis errors, they should only prevent the
affected stack from deploying; right now, since they are checked
during the synthesis step, they would abort the entire toolkit
run regardless of whether they would be selected or not.

Makes it possible to do region-dependent verification.

Fixes #1784.

ALSO IN THIS COMMIT

If the 'cdk synth' output goes to the screen, don't automatically
select upstream stacks for synthesis (as that would lead to an
immediate error).

Fixes #1783.
eladb pushed a commit that referenced this issue Feb 26, 2019
If we report synthesis errors, they should only prevent the
affected stack from deploying; right now, since they are checked
during the synthesis step, they would abort the entire toolkit
run regardless of whether they would be selected or not.

Makes it possible to do region-dependent verification.

Fixes #1784.

ALSO IN THIS COMMIT

If the 'cdk synth' output goes to the screen, don't automatically
select upstream stacks for synthesis (as that would lead to an
immediate error).

Fixes #1783.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants