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

Cannot use Condition (or condition.ref) in FnOr #1457

Closed
mipearson opened this issue Dec 31, 2018 · 4 comments · Fixed by #1494
Closed

Cannot use Condition (or condition.ref) in FnOr #1457

mipearson opened this issue Dec 31, 2018 · 4 comments · Fixed by #1494
Labels
@aws-cdk/core Related to core CDK functionality bug This issue is a bug.

Comments

@mipearson
Copy link

eg:

    HasManagedPolicies:
      !Or [ { Condition: UseManagedPolicyARN }, { Condition: UseECR } ]

or even:

    CreateMetricsStack:
      Condition: UseAutoscaling

It feels like I should be able to do this:

    const useManagedPolicyARN = new cdk.Condition(...);
    const useEcr = new cdk.Condition(...);
    const hasManagedPolicies = new cdk.Condition(this, "HasManagedPolicies", {
      expression: new cdk.FnOr(
        useManagedPolicyARN,
        useEcr
      )
    });

But the cdk.Condition class does not match the type definition of the predicate functions (which require FnCondition.

After some mucking around, I found the following workaround, but it relies on Fn simply re-emitting exactly what it's passed:

    const hasManagedPolicies = new cdk.Condition(this, "HasManagedPolicies", {
      expression: new cdk.FnOr(
        new cdk.Fn("Condition", "UseManagedPolicyARN"),
        new cdk.Fn("Condition", "UseECR")
      )
    });

This applies to referring to Conditions within FnOr and so forth within resources as well.

What is the correct way to express this in CDK?

@rix0rrr
Copy link
Contributor

rix0rrr commented Dec 31, 2018

We've generally avoided Conditions so far, since we prefer expressing conditions in the CDK app.

I'm assuming you're using CDK to generate reusable templates that will be deployed by someone else?

Will look into the code.

@rix0rrr
Copy link
Contributor

rix0rrr commented Dec 31, 2018

I think the idea was to use

 const hasManagedPolicies = new cdk.Condition(this, "HasManagedPolicies", {
      expression: new cdk.FnOr(
        useManagedPolicyARN.ref,
        useEcr.ref
      )
    });

Does that not work?

@mipearson
Copy link
Author

Yep, this is me evaluating CDK as "a tool to synthesize templates" rather than "a tool to manage AWS".

We're not big users of conditions anyway, but nice to have coverage.

That example unfortunately doesn't work - ref is a string, and FnOr expects an FnCondition.

@rix0rrr
Copy link
Contributor

rix0rrr commented Jan 2, 2019

Got it. Will file this as a bug then.

@rix0rrr rix0rrr changed the title CFN Condition usage within conditions and functions Cannot use Condition (or condition.ref) in FnOr Jan 2, 2019
@rix0rrr rix0rrr added bug This issue is a bug. @aws-cdk/core Related to core CDK functionality labels Jan 2, 2019
eladb pushed a commit that referenced this issue Jan 8, 2019
Change `conditionXx` methods to accept an interface `IConditionExpression`
instead of concrete class and implement this interface by the `Condition`
construct.

This enables chaining conditions like so:

  const cond1, cond2, cond, cond4 = new cdk.Condition(...)
  Fn.conditionOr(cond1, cond2, Fn.conditionAnd(cond3, cond4))

Fixes #1457
eladb pushed a commit that referenced this issue Jan 8, 2019
Change `conditionXx` methods to accept an interface `IConditionExpression`
instead of concrete class and implement this interface by the `Condition`
construct.

This enables chaining conditions like so:

    const cond1, cond2, cond, cond4 = new cdk.Condition(...)
    Fn.conditionOr(cond1, cond2, Fn.conditionAnd(cond3, cond4))

Fixes #1457
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/core Related to core CDK functionality bug This issue is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants