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_cloudwatch_action <ec2 autorecover actions>: works only in aws partition #19765

Closed
GregChapmanAtRack opened this issue Apr 5, 2022 · 3 comments · Fixed by #20224
Closed

Comments

@GregChapmanAtRack
Copy link

GregChapmanAtRack commented Apr 5, 2022

Describe the bug

When deploying EC2 instances in any partition other than commercial, stack rolls back when EC2 recover and reboot actions are added to an alarm.

Expected Behavior

Alarm to be created with specified action, for example:

cw_alarm4.add_alarm_action(cloudwatch_actions.Ec2Action(cloudwatch_actions.Ec2InstanceAction.REBOOT))

Current Behavior

"<timestamp>| CREATE_FAILED        | AWS::CloudWatch::Alarm    | CloudwatchAlarm287DB52F9
Invalid partition aws specified. Only aws-us-gov is supported. (Service: AmazonCloudWatch; Status Code: 400; Error Code: ValidationError; Request ID: f61066f0-d59c-42f0-bf39-99ea1c0d3cc2;
Proxy: null)"

Reproduction Steps

    cw_alarm4 = cloudwatch.Alarm(
        scope=self,
        id="Cloudwatch Alarm 4",
        comparison_operator=cloudwatch.ComparisonOperator.GREATER_THAN_THRESHOLD,
        evaluation_periods=1,
        actions_enabled=True,
        alarm_description="Status checks have failed, REBOOTING/RECOVERING system - " +
        instance1,
        alarm_name="Instance Status Check Failed - "+instance1+" recovery",
        datapoints_to_alarm=None,
        metric=instance1_metric2,
        threshold=0,
        treat_missing_data=None
    )
    cw_alarm4.add_alarm_action(cloudwatch_actions.Ec2Action(
        cloudwatch_actions.Ec2InstanceAction.REBOOT))

Works in AWS partition. Does not work in aws-us-gov

Possible Solution

I believe the error resides in https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-cloudwatch-actions/lib/ec2.ts#L40-L46

Suggest distilling partition from env variable AWS_DEFAULT_REGION, .aws/config values, etc.

Additional Information/Context

No response

CDK CLI Version

2.19.0 (build e0d3e62)

Framework Version

No response

Node.js Version

v16.11.1

OS

NAME="Ubuntu" VERSION="20.04.4 LTS (Focal Fossa)"

Language

Python

Language Version

Python 3.10.0

Other information

No response

@GregChapmanAtRack GregChapmanAtRack added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 5, 2022
@GregChapmanAtRack
Copy link
Author

GregChapmanAtRack commented Apr 5, 2022

I have confirmed in the json output that this function is returning the offending arn:

    "CloudwatchAlarm287DB52F9": {
      "Type": "AWS::CloudWatch::Alarm",
      "Properties": {
        "ComparisonOperator": "GreaterThanThreshold",
        "EvaluationPeriods": 1,
        "ActionsEnabled": true,
        "AlarmActions": [
          "arn:aws:automate:us-gov-west-1:ec2:reboot",
          "arn:aws:automate:us-gov-west-1:ec2:recover"
        ],
        "AlarmDescription": {
          "Fn::Join": [
            "",
            [
              "Status checks have failed, REBOOTING/RECOVERING system - ",
              {
                "Ref": "AlarmTestlinuxB31B50A2"
              }
            ]
          ]
        },
        "AlarmName": {
          "Fn::Join": [
            "",
            [
              "System Status Check Failed - ",
              {
                "Ref": "AlarmTestlinuxB31B50A2"
              },
              " recovery"
            ]
          ]
        },
        "Dimensions": [
          {
            "Name": "InstanceId",
            "Value": {
              "Ref": "AlarmTestlinuxB31B50A2"
            }
          }
        ],
        "MetricName": "StatusCheckFailed_System",
        "Namespace": "AWS/EC2",
        "Period": 60,
        "Statistic": "Minimum",
        "Threshold": 0
      },
      "Metadata": {
        "aws:cdk:path": "Alarm-Test/Cloudwatch Alarm 2/Resource"
      }
    },

@NGL321 NGL321 added p1 and removed needs-triage This issue or PR still needs to be triaged. labels Apr 7, 2022
@NGL321
Copy link
Contributor

NGL321 commented Apr 7, 2022

From what I can tell in the Gov Cloud docs it doesn't seem like there should be any limitations on this behavior, so its safe to say this is a bug either on our end or with the way the govcloud APIs are handled internally.

@mergify mergify bot closed this as completed in #20224 May 13, 2022
mergify bot pushed a commit that referenced this issue May 13, 2022
… arn (#20224)

This removes the hardcoded partition in the ARNs of Alarm Actions for
EC2 and SSM. This ensures that these don't unnecessarily break in other
non-standard partitions. This uses the ARN of the stack, as done for the
region and account.

This updates a regular expression in `@aws-cdk/aws-cloudwatch` as well
to make sure that EC2 actions are still validated as-expected in GovCloud
and other partitions that may support AlarmActions.

Closes #19765

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

wphilipw pushed a commit to wphilipw/aws-cdk that referenced this issue May 23, 2022
… arn (aws#20224)

This removes the hardcoded partition in the ARNs of Alarm Actions for
EC2 and SSM. This ensures that these don't unnecessarily break in other
non-standard partitions. This uses the ARN of the stack, as done for the
region and account.

This updates a regular expression in `@aws-cdk/aws-cloudwatch` as well
to make sure that EC2 actions are still validated as-expected in GovCloud
and other partitions that may support AlarmActions.

Closes aws#19765

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants