fix(core): prevent the error when the condition is split into groups of 10 and 1 in Fn.conditionOr()

[wafuwafu13]

Closes #25696

The problem I'm running into is for a list of 11 elements. CDK generates two Fn::Or expressions: One with 10 elements and one with 1 element. When deployment this stack, CloudFormation complains that an Fn::Or must contain at least 2 elements.

reproduce code: #25696 (comment)
approach: #25696 (comment)

---

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

[gitpod-io]

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

[wafuwafu13]

Exemption Request

I add test case in packages/aws-cdk-lib/core/test/condition.test.ts

[tmokmss]

I think this can break when conditions.length % 90 == 1 :)

[wafuwafu13]

Yes, but would it be necessary to consider conditions for numbers larger than 91 or 181...?

[tmokmss]

I guess yes, unless we are totally sure that there is no user to use such configuration.

[wafuwafu13]

I agree that if we can't be sure, I should respond.
But in this case, I don't have a simple solution to solve this 91 or 181 or more problem...
And I'd prefer not to add more complex patch and test(should check the case of 91 and doesn't use snapshot for condition now) for specific funky code of 91 or 181 or more conditions.

[tmokmss]

I see. I feel the solution @MalikAtalla-AWS proposed is also simple without any edge cases, but let's wait for comments from the maintainers.

[corymhall]

Lets go with @MalikAtalla-AWS's solution. Something like this might work

public static conditionOr(...conditions: ICfnConditionExpression[]): ICfnRuleConditionExpression {
    if (conditions.length === 0) {
      throw new Error('Fn.conditionOr() needs at least one argument');
    }
    if (conditions.length === 1) {
      return conditions[0] as ICfnRuleConditionExpression;
    }
    // prevent the error "Fn::Or object requires a list of at least 2" when the condition is split into groups of 10 and 1
    if (conditions.length > 10) {
      return Fn.conditionOr(..._inGroupsOf(conditions, 10).map(group => Fn.conditionOr(...group));
    }
    return Fn.conditionOr(..._inGroupsOf(conditions, 10).map(group => new FnOr(...group)));
  }

[MalikAtalla-AWS]

This is totally a nice-to-have, but in terms of the simplicity of the resulting CFN snippet, I think it would be preferable to simply drop the last wrapping Fn::Or, like this

    'Fn::Or': [
      {
        'Fn::Or': [
          { 'Fn::Equals': ['a', '1'] },
          { 'Fn::Equals': ['b', '2'] },
          { 'Fn::Equals': ['c', '3'] },
          { 'Fn::Equals': ['d', '4'] },
          { 'Fn::Equals': ['e', '5'] },
          { 'Fn::Equals': ['f', '6'] },
          { 'Fn::Equals': ['g', '7'] },
          { 'Fn::Equals': ['h', '8'] },
          { 'Fn::Equals': ['i', '9'] },
          { 'Fn::Equals': ['j', '10'] }
        ],
      },
      {
          'Fn::Equals': ['k', '11']
      },
    ]

[corymhall]

Lets go with @MalikAtalla-AWS's solution. Something like this might work

public static conditionOr(...conditions: ICfnConditionExpression[]): ICfnRuleConditionExpression {
    if (conditions.length === 0) {
      throw new Error('Fn.conditionOr() needs at least one argument');
    }
    if (conditions.length === 1) {
      return conditions[0] as ICfnRuleConditionExpression;
    }
    // prevent the error "Fn::Or object requires a list of at least 2" when the condition is split into groups of 10 and 1
    if (conditions.length > 10) {
      return Fn.conditionOr(..._inGroupsOf(conditions, 10).map(group => Fn.conditionOr(...group));
    }
    return Fn.conditionOr(..._inGroupsOf(conditions, 10).map(group => new FnOr(...group)));
  }

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

Pull request has been modified.

[otaviomacedo]

Just a matter of preference, but this seems more readable:

public static conditionOr(...conditions: ICfnConditionExpression[]): ICfnRuleConditionExpression {
  if (conditions.length === 0) {
    throw new Error('Fn.conditionOr() needs at least one argument');
  }
  if (conditions.length === 1) {
    return conditions[0] as ICfnRuleConditionExpression;
  }
  if (conditions.length <= 10) {
    return new FnOr(...conditions);
  }
  return Fn.conditionOr(..._inGroupsOf(conditions, 10).map(group => Fn.conditionOr(...group)));
}

The three base cases are stated explicitly, falling back to the recursive case at the end.

Pull request has been modified.

[mergify]

Thank you for contributing! Your pull request will be updated from main 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: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 2901214
• 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 main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).