(aws-cloudwatch): AlarmRule static methods can synth into an invalid template

[jacklin213]

Describe the bug

AlarmRule.anyOf(...alarms) or AlarmRule.allOf(...alarms) where alarms is empty will result in alarmRule: "()" in the synth'ed CFN template which is not a valid alarm rule.

Expected Behavior

Users should be able to cdk synth if Alarm Rule is invalid

Current Behavior

cdk synth passes and users will only find out the alarm rule is invalid during deployment time in CloudFormation with a not so straight forward error message

Resource handler returned message: "Error in AlarmRule [Unsupported token ')' at char 1] (Service: AmazonCloudWatch ...)

Reproduction Steps

const alarms: Alarm[] = [];
new CompositeAlarm(this, 'CompositeAlarm', {
  compositeAlarmName: "CompositeAlarm",
  alarmRule: AlarmRule.anyOf(...alarms),
});

Possible Solution

Both AlarmRule.anyOf(...alarms) and AlarmRule.allOf(...alarms) calls the concat function below

aws-cdk/packages/aws-cdk-lib/aws-cloudwatch/lib/alarm-rule.ts

Lines 111 to 121 in dc30faa

	private static concat(operator: Operator, ...operands: IAlarmRule[]): IAlarmRule {
	return new class implements IAlarmRule {
	public renderAlarmRule(): string {
	const expression = operands
	.map(operand => `${operand.renderAlarmRule()}`)
	.join(` ${operator} `);
	return `(${expression})`;
	}
	};
	}
	}

We could consider having CDK throw an error if expression is undefined so users don't need to wait till deployment time to find out the alarm rule is invalid. Maybe something like

private static concat(operator: Operator, ...operands: IAlarmRule[]): IAlarmRule {
    return new class implements IAlarmRule {
      public renderAlarmRule(): string {
        const expression = operands
          .map(operand => `${operand.renderAlarmRule()}`)
          .join(` ${operator} `);
        if (!expression) {
            // Throw exception here
        }
        return `(${expression})`;
      }
    };
}

Additional Information/Context

Originally reported in #14387 but was closed for staleness. It is still an issue with the latest cdk version

AWS CDK Library version (aws-cdk-lib)

2.181.1

AWS CDK CLI version

2.1007.0

Node.js Version

20.18.0

OS

Amazon Linux 2

Language

TypeScript

Language Version

5.8.2

Other information

No response

[ykethan]

Hey @jacklin213, thank you for reaching out. When AlarmRule.anyOf() or AlarmRule.allOf() are called with an empty array of alarms, the CDK synth generates AlarmRule: () in the CloudFormation template, which is not a valid CloudWatch alarm rule expression.

const alarms: cloudwatch.Alarm[] = [];

const compositeAlarm = new cloudwatch.CompositeAlarm(this, 'CompositeAlarm', {
  compositeAlarmName: 'CompositeAlarm',
  alarmRule: cloudwatch.AlarmRule.anyOf(...alarms),
});

Result: The synthesized CloudFormation template contains:

CompositeAlarm:
  Type: AWS::CloudWatch::CompositeAlarm
  Properties:
    AlarmName: CompositeAlarm
    AlarmRule: ()  # Invalid expression

As you have correctly pointed from the codebase
File: packages/aws-cdk-lib/aws-cloudwatch/lib/alarm-rule.ts
Method: private static concat(operator: Operator, ...operands: IAlarmRule[]): IAlarmRule

private static concat(operator: Operator, ...operands: IAlarmRule[]): IAlarmRule {
  return new class implements IAlarmRule {
    public renderAlarmRule(): string {
      const expression = operands
        .map(operand => `${operand.renderAlarmRule()}`)
        .join(` ${operator} `);
      return `(${expression})`;  // When operands is empty, this returns "()"
    }
  };
}


Adding a validation in the `concat` method to throw an error at synthesis time when no operands are provided:

```typescript
private static concat(operator: Operator, ...operands: IAlarmRule[]): IAlarmRule {
  return new class implements IAlarmRule {
    public renderAlarmRule(): string {
      if (operands.length === 0) {
        throw new Error(`AlarmRule.${operator === Operator.AND ? 'allOf' : 'anyOf'}() requires at least one alarm operand`);
      }
      const expression = operands
        .map(operand => `${operand.renderAlarmRule()}`)
        .join(` ${operator} `);
      return `(${expression})`;
    }
  };
}

Current workaround similar to #14387, I've tested the following workarounds:

For anyOf with empty alarms (use FALSE):

const alarms: cloudwatch.Alarm[] = [];

const compositeAlarm = new cloudwatch.CompositeAlarm(this, 'CompositeAlarm', {
  alarmRule: alarms.length > 0 
    ? cloudwatch.AlarmRule.anyOf(...alarms)
    : cloudwatch.AlarmRule.fromBoolean(false), // anyOf with no alarms = FALSE
});

For allOf with empty alarms (use TRUE):

const alarms: cloudwatch.Alarm[] = [];

const compositeAlarm = new cloudwatch.CompositeAlarm(this, 'CompositeAlarm', {
  alarmRule: alarms.length > 0 
    ? cloudwatch.AlarmRule.allOf(...alarms)
    : cloudwatch.AlarmRule.fromBoolean(true), // allOf with no alarms = TRUE
});

Verified Results: These workarounds generate valid CloudFormation templates:

• anyOf: AlarmRule: "FALSE"
• allOf: AlarmRule: "TRUE"

Marking as P2 bug.
We greatly appreciate community contributions! If you are interested in implementing this on a Pull Request do review the CDK contribution guidelines. Meanwhile, welcome to add 👍 to this issue to help us prioritize this.

[jacklin213]

Thanks for the explanations and testing the workarounds. Out of curiosity, why would allOf use TRUE instead of FALSE ? TRUE would result in a compositeAlarm permanently in an IN_ALARM state.

Given that there is a safe fallback option, should we opt for those as defaults and log a warning instead of throwing an error? Thinking it might be a better user experience

eg)

private static concat(operator: Operator, ...operands: IAlarmRule[]): IAlarmRule {
  return new class implements IAlarmRule {
    public renderAlarmRule(): string {
      if (operands.length === 0) {
        console.warn(`Did not detect any operands for AlarmRule.${operator === Operator.AND ? 'allOf' : 'anyOf'}(), defaulting to FALSE`);
        return "FALSE";
      }
      const expression = operands
        .map(operand => `${operand.renderAlarmRule()}`)
        .join(` ${operator} `);
      return `(${expression})`;
    }
  };
}

[github-actions]

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.