Repository: cloud-custodian/cloud-custodian

Issue:
enabling perfromance insight on RDS failing due wrong key in property



**Describe the bug**
response of describe db instance does not have any key "EnablePerformanceInsights" but "PerformanceInsightsEnabled" :
https://github.com/cloud-custodian/cloud-custodian/blob/master/c7n/resources/rds.py#L1625
Hence, using EnablePerformanceInsights under action fail
          - type: modify-db
              update:
                - property: 'PerformanceInsightsEnabled'
                  value: true
              immediate: true

https://github.com/cloud-custodian/cloud-custodian/blob/master/c7n/resources/rds.py#L1655
    if r[update['property']] != update['value']:
KeyError: 'EnablePerformanceInsights'


**To Reproduce**
Steps to reproduce the behavior:

**Expected behavior**
A clear and concise description of what you expected to happen.


**Background (please complete the following information):**

```yaml
policies: 
       actions:
            - type: modify-db
              update:
                - property: 'EnablePerformanceInsights'
                  value: true
              immediate: true
```
 - Traceback: [if applicable, please exclude sensitive/account information]
 - `custodian version --debug` output

**Additional context**
Add any other context about the problem here.


Discussion:
kapilt: > actually do want the shape here.. just because its pretty obtuse otherwise to figure out the params

sorry I meant the other way around, we generally want to prefer jsonschema over shape, as it helps on discoverability of the options, without having to flip to api docs and back to custodian. alternatively we can look at more formerly adding in some class level support for using shape validation, so that we can surface it in our documentation.
JohnHillegass: > > actually do want the shape here.. just because its pretty obtuse otherwise to figure out the params
> 
> sorry I meant the other way around, we generally want to prefer jsonschema over shape, as it helps on discoverability of the options, without having to flip to api docs and back to custodian. alternatively we can look at more formerly adding in some class level support for using shape validation, so that we can surface it in our documentation.

Thanks for clarifying that. I reverted to jsonschema. I like the idea of class level support for using shape validation will open a ticket as a reminder to research. Something like that could really help future proof against api changes.

Patch:
diff --git a/c7n/resources/rds.py b/c7n/resources/rds.py
index b7e5dce63..56db7181e 100644
--- a/c7n/resources/rds.py
+++ b/c7n/resources/rds.py
@@ -1626,8 +1626,11 @@ class ModifyDb(BaseAction):
                         'PerformanceInsightsKMSKeyId',
                         'PerformanceInsightsRetentionPeriod',
                         'CloudwatchLogsExportConfiguration',
+                        'ProcessorFeatures',
                         'UseDefaultProcessorFeatures',
-                        'DeletionProtection']},
+                        'DeletionProtection',
+                        'MaxAllocatedStorage',
+                        'CertificateRotationRestart']},
                     'value': {}
                 },
             },
@@ -1635,6 +1638,17 @@ class ModifyDb(BaseAction):
         required=('update',))
 
     permissions = ('rds:ModifyDBInstance',)
+    conversion_map = {
+        'DBSubnetGroupName': 'DBSubnetGroup.DBSubnetGroupName',
+        'VpcSecurityGroupIds': 'VpcSecurityGroups[].VpcSecurityGroupId',
+        'DBParameterGroupName': 'DBParameterGroups[].DBParameterGroupName',
+        'OptionGroupName': 'OptionGroupMemberships[].OptionGroupName',
+        'NewDBInstanceIdentifier': 'DBInstanceIdentifier',
+        'Domain': 'DomainMemberships[].DomainName',
+        'DBPortNumber': 'Endpoint.Port',
+        'EnablePerformanceInsights': 'PerformanceInsightsEnabled',
+        'CloudwatchLogsExportConfiguration': 'EnabledCloudwatchLogsExports'
+    }
 
     def validate(self):
         if self.data.get('update'):
@@ -1644,16 +1658,25 @@ class ModifyDb(BaseAction):
                 raise PolicyValidationError(
                     "A MonitoringRoleARN value is required \
                     if you specify a MonitoringInterval value other than 0")
+            if ('CloudwatchLogsExportConfiguration' in update_dict
+                and all(
+                    k not in update_dict.get('CloudwatchLogsExportConfiguration')
+                    for k in ('EnableLogTypes', 'DisableLogTypes'))):
+                raise PolicyValidationError(
+                    "A EnableLogTypes or DisableLogTypes input list is required\
+                    for setting CloudwatchLogsExportConfiguration")
         return self
 
     def process(self, resources):
         c = local_session(self.manager.session_factory).client('rds')
-
         for r in resources:
-            param = {}
-            for update in self.data.get('update'):
-                if r[update['property']] != update['value']:
-                    param[update['property']] = update['value']
+            param = {
+                u['property']: u['value'] for u in self.data.get('update')
+                if r.get(
+                    u['property'],
+                    jmespath.search(
+                        self.conversion_map.get(u['property'], 'None'), r))
+                    != u['value']}
             if not param:
                 continue
             param['ApplyImmediately'] = self.data.get('immediate', False)


Test patch:
diff --git a/tests/data/placebo/test_rds_modify_db_enable_cloudwatch/rds.DescribeDBInstances_1.json b/tests/data/placebo/test_rds_modify_db_enable_cloudwatch/rds.DescribeDBInstances_1.json
new file mode 100644
index 000000000..12f8fcb9f
--- /dev/null
+++ b/tests/data/placebo/test_rds_modify_db_enable_cloudwatch/rds.DescribeDBInstances_1.json
@@ -0,0 +1,17 @@
+{
+    "status_code": 200,
+    "data": {
+        "DBInstances": [
+            {
+                "DBInstanceIdentifier": "database-2",
+                "DBInstanceArn": "arn:aws:rds:us-east-1:644160558196:db:database-2",                
+                "Engine": "mysql",
+                "EnabledCloudwatchLogsExports": [
+                    "general",
+                    "slowquery"
+                ]
+            }
+        ],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_rds_modify_db_enable_cloudwatch/rds.DescribeDBInstances_2.json b/tests/data/placebo/test_rds_modify_db_enable_cloudwatch/rds.DescribeDBInstances_2.json
new file mode 100644
index 000000000..e22b24bab
--- /dev/null
+++ b/tests/data/placebo/test_rds_modify_db_enable_cloudwatch/rds.DescribeDBInstances_2.json
@@ -0,0 +1,18 @@
+{
+    "status_code": 200,
+    "data": {
+        "DBInstances": [
+            {
+                "DBInstanceIdentifier": "database-2",
+                "DBInstanceArn": "arn:aws:rds:us-east-1:644160558196:db:database-2",
+                "Engine": "mysql",
+                "EnabledCloudwatchLogsExports": [
+                    "error",                    
+                    "general",
+                    "slowquery"
+                ]
+            }
+        ],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_rds_modify_db_enable_cloudwatch/rds.ModifyDBInstance_1.json b/tests/data/placebo/test_rds_modify_db_enable_cloudwatch/rds.ModifyDBInstance_1.json
new file mode 100644
index 000000000..00e23edba
--- /dev/null
+++ b/tests/data/placebo/test_rds_modify_db_enable_cloudwatch/rds.ModifyDBInstance_1.json
@@ -0,0 +1,11 @@
+{
+    "status_code": 200,
+    "data": {
+        "DBInstance": {
+            "DBInstanceIdentifier": "database-2",
+            "Engine": "mysql",
+            "DBInstanceArn": "arn:aws:rds:us-east-1:644160558196:db:database-2"
+        },
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_rds_modify_db_enable_cloudwatch/tagging.GetResources_1.json b/tests/data/placebo/test_rds_modify_db_enable_cloudwatch/tagging.GetResources_1.json
new file mode 100644
index 000000000..8b704d185
--- /dev/null
+++ b/tests/data/placebo/test_rds_modify_db_enable_cloudwatch/tagging.GetResources_1.json
@@ -0,0 +1,8 @@
+{
+    "status_code": 200,
+    "data": {
+        "PaginationToken": "",
+        "ResourceTagMappingList": [],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_rds_modify_db_enable_perfinsights/rds.DescribeDBInstances_1.json b/tests/data/placebo/test_rds_modify_db_enable_perfinsights/rds.DescribeDBInstances_1.json
new file mode 100644
index 000000000..5a6773ae1
--- /dev/null
+++ b/tests/data/placebo/test_rds_modify_db_enable_perfinsights/rds.DescribeDBInstances_1.json
@@ -0,0 +1,14 @@
+{
+    "status_code": 200,
+    "data": {
+        "DBInstances": [
+            {
+                "DBInstanceIdentifier": "database-4",
+                "Engine": "postgres",
+                "DBInstanceArn": "arn:aws:rds:us-east-1:644160558196:db:database-4",
+                "PerformanceInsightsEnabled": false
+            }
+        ],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_rds_modify_db_enable_perfinsights/rds.DescribeDBInstances_2.json b/tests/data/placebo/test_rds_modify_db_enable_perfinsights/rds.DescribeDBInstances_2.json
new file mode 100644
index 000000000..6636842fd
--- /dev/null
+++ b/tests/data/placebo/test_rds_modify_db_enable_perfinsights/rds.DescribeDBInstances_2.json
@@ -0,0 +1,14 @@
+{
+    "status_code": 200,
+    "data": {
+        "DBInstances": [
+            {
+                "DBInstanceIdentifier": "database-4",
+                "Engine": "postgres",
+                "DBInstanceArn": "arn:aws:rds:us-east-1:644160558196:db:database-4",
+                "PerformanceInsightsEnabled": true
+            }
+        ],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_rds_modify_db_enable_perfinsights/rds.ModifyDBInstance_1.json b/tests/data/placebo/test_rds_modify_db_enable_perfinsights/rds.ModifyDBInstance_1.json
new file mode 100644
index 000000000..b3a4d9d91
--- /dev/null
+++ b/tests/data/placebo/test_rds_modify_db_enable_perfinsights/rds.ModifyDBInstance_1.json
@@ -0,0 +1,11 @@
+{
+    "status_code": 200,
+    "data": {
+        "DBInstance": {
+            "DBInstanceIdentifier": "database-4",
+            "Engine": "postgres",
+            "DBInstanceArn": "arn:aws:rds:us-east-1:644160558196:db:database-4"
+        },
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_rds_modify_db_enable_perfinsights/tagging.GetResources_1.json b/tests/data/placebo/test_rds_modify_db_enable_perfinsights/tagging.GetResources_1.json
new file mode 100644
index 000000000..8b704d185
--- /dev/null
+++ b/tests/data/placebo/test_rds_modify_db_enable_perfinsights/tagging.GetResources_1.json
@@ -0,0 +1,8 @@
+{
+    "status_code": 200,
+    "data": {
+        "PaginationToken": "",
+        "ResourceTagMappingList": [],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/test_rds.py b/tests/test_rds.py
index ba3c50aba..2a7bdbf4c 100644
--- a/tests/test_rds.py
+++ b/tests/test_rds.py
@@ -744,11 +744,139 @@ class RDSTest(BaseTest):
         )
         resources = p.run()
         self.assertEqual(len(resources), 1)
-
         client = session_factory().client("rds")
         db_info = client.describe_db_instances(DBInstanceIdentifier="testtest")
         self.assertFalse(db_info["DBInstances"][0]["DeletionProtection"])
 
+    def test_rds_modify_db_enable_cloudwatch(self):
+        session_factory = self.replay_flight_data("test_rds_modify_db_enable_cloudwatch")
+        p = self.load_policy(
+            {
+                "name": "rds-modify-enable-cloudwatch",
+                "resource": "rds",
+                "filters": [
+                    {
+                        "type": "value",
+                        "key": "DBInstanceIdentifier",
+                        "value": "database-2"
+                    },
+                    {
+                        "type": "value",
+                        "key": "EnabledCloudwatchLogsExports[]",
+                        "value": [
+                            "error"
+                        ],
+                        "op": "ni"
+                    }
+                ],
+                "actions": [
+                    {
+                        "type": "modify-db",
+                        "update": [
+                            {
+                                "property": 'CloudwatchLogsExportConfiguration',
+                                "value": {
+                                    'EnableLogTypes': [
+                                        "error"
+                                    ]
+                                }
+                            }
+                        ],
+                        "immediate": True
+                    }
+                ],
+            },
+            session_factory=session_factory,
+        )
+        resources = p.run()
+        self.assertEqual(len(resources), 1)
+        client = session_factory().client("rds")
+        db_info = client.describe_db_instances(DBInstanceIdentifier="database-2")
+        self.assertIn('error', db_info["DBInstances"][0]["EnabledCloudwatchLogsExports"])
+
+    def test_rds_modify_db_validation_monitoring_error(self):
+        with self.assertRaises(PolicyValidationError) as err:
+            self.load_policy({
+                'name': 'enable-monitoring',
+                'resource': 'rds',
+                "actions": [
+                    {
+                        "type": "modify-db",
+                        "update": [
+                            {
+                                "property": 'MonitoringInterval',
+                                "value": 60
+                            }
+                        ],
+                        "immediate": True
+                    }
+                ]})
+        self.assertIn((
+            'A MonitoringRoleARN value is required'),
+            str(err.exception))
+
+    def test_rds_modify_db_validation_cloudwatch_error(self):
+        with self.assertRaises(PolicyValidationError) as err:
+            self.load_policy({
+                'name': 'enable-cloudwatch',
+                'resource': 'rds',
+                "actions": [
+                    {
+                        "type": "modify-db",
+                        "update": [
+                            {
+                                "property": 'CloudwatchLogsExportConfiguration',
+                                "value": [
+                                    "error"
+                                ]
+                            }
+                        ],
+                        "immediate": True
+                    }
+                ]})
+        self.assertIn((
+            'EnableLogTypes or DisableLogTypes input list is required'),
+            str(err.exception))
+
+    def test_rds_modify_db_enable_perfinsights(self):
+        session_factory = self.replay_flight_data("test_rds_modify_db_enable_perfinsights")
+        p = self.load_policy(
+            {
+                "name": "rds-modify-enable-perfinsights",
+                "resource": "rds",
+                "filters": [
+                    {
+                        "type": "value",
+                        "key": "DBInstanceIdentifier",
+                        "value": "database-4"
+                    },
+                    {
+                        "type": "value",
+                        "key": "PerformanceInsightsEnabled",
+                        "value": False
+                    }
+                ],
+                "actions": [
+                    {
+                        "type": "modify-db",
+                        "update": [
+                            {
+                                "property": "EnablePerformanceInsights",
+                                "value": True
+                            }
+                        ],
+                        "immediate": True
+                    }
+                ],
+            },
+            session_factory=session_factory,
+        )
+        resources = p.run()
+        self.assertEqual(len(resources), 1)
+        client = session_factory().client("rds")
+        db_info = client.describe_db_instances(DBInstanceIdentifier="database-4")
+        self.assertTrue(db_info["DBInstances"][0]["PerformanceInsightsEnabled"])
+
 
 class RDSSnapshotTest(BaseTest):