Add RDS missing permissions #164

marknet15 · 2021-09-20T19:00:19Z

Overview

I'm in the progress of making a small RDS change here to fix a bug with enabling enhanced monitoring, and as part of that I need to enable the integration tests.
ansible-collections/community.aws#712

Currently it seems some permissions are missing for example DescribeDBInstances:

TASK [rds_instance : Ensure the resource doesn't exist] ************************
task path: /home/zuul/.ansible/collections/ansible_collections/community/aws/tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_tags.yml:4
The full traceback is:
Traceback (most recent call last):
  File "/tmp/ansible_rds_instance_payload_x6h5cji8/ansible_rds_instance_payload.zip/ansible_collections/community/aws/plugins/modules/rds_instance.py", line 811, in get_instance
  File "/home/zuul/venv/lib/python3.6/site-packages/botocore/client.py", line 337, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/home/zuul/venv/lib/python3.6/site-packages/botocore/client.py", line 656, in _make_api_call
    raise error_class(parsed_response, operation_name)
botocore.exceptions.ClientError: An error occurred (AccessDenied) when calling the DescribeDBInstances operation: User: arn:aws:sts::966509639900:assumed-role/ansible-core-ci-test-dev/dev=remote=zuul-cloud is not authorized to perform: rds:DescribeDBInstances on resource: arn:aws:rds:us-east-1:966509639900:db:ansible-test-52620875-ip-172-16-123-238-tags
fatal: [tags]: FAILED! => {
    "boto3_version": "1.15.0",
    "botocore_version": "1.18.0",
    "changed": false,
    "error": {
        "code": "AccessDenied",
        "message": "User: arn:aws:sts::966509639900:assumed-role/ansible-core-ci-test-dev/dev=remote=zuul-cloud is not authorized to perform: rds:DescribeDBInstances on resource: arn:aws:rds:us-east-1:966509639900:db:ansible-test-52620875-ip-172-16-123-238-tags",
        "type": "Sender"
    },

This also adds specific IAM permissions needed for policy attachments for the enhanced monitoring

jillr · 2021-09-20T21:04:33Z

The rds tests are slow so it'll take a while to test; at first pass the policies look reasonable. You'll also need to add a class to the terminator in terminator/data_services.py for RdsInstance or some similar name. The RdsDbParameterGroup and RdsOptionGroup classes should be good examples to reference.

jillr · 2021-09-20T22:22:12Z

A couple of things I found testing this one. I'll note the test task that triggered the AccessDenied failure and the policy that's being reported as missing. This is as far as I got with it today, thanks for your work on this!

rds:RestoreDBInstanceToPointInTime
tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_restore_instance.yml:34

rds:DescribeDBSnapshots
tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_snapshot.yml:4
For Describe policies they can just be a wildcard, like rds:DescribeDB*

rds:CreateDBInstanceReadReplica
tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_read_replica.yml:43

In policy/security-services.yaml you'll need to add an iam:PolicyArn: condition for the task at tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_enhanced_monitoring.yml:32
"Unable to attach policy arn:aws:iam::aws:policy/service-role/AmazonRDSEnhancedMonitoringRole to role ansible-test-lab-65106811-enhanced-monitoring-role: An error occurred (AccessDenied) when calling the AttachRolePolicy opera
tion: User: arn:aws:sts::966509639900:assumed-role/ansible-core-ci-test-dev/dev=remote=jillr is not authorized to perform: iam:AttachRolePolicy on resource: role ansible-test-lab-65106811-enhanced-monitoring-role",

rds:StopDBInstance
tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_states.yml:86

marknet15 · 2021-09-21T15:56:08Z

@jillr thanks again for testing this out, I think I've done the class correctly and I included all the extra permissions

jillr · 2021-09-23T23:57:31Z

aws/policy/security-services.yaml

@@ -138,6 +138,14 @@ Statement:
      - 'arn:aws:iam::{{ aws_account_id }}:role/ansible-test-*'
      # This is hard coded into DMS...
      - 'arn:aws:iam::{{ aws_account_id }}:role/dms-vpc-role'
+  - Sid: AllowEnhancedMonitoringPolicyAttach


Apologies @marknet15 I wasn't able to finish testing this today. I believe you need to add 'arn:aws:iam::aws:policy/service-role/AmazonRDSEnhancedMonitoringRole' around line 22 instead of this section. However because the rds test suite takes so long to run I wasn't able to get a successful test run. I'll keep working on this review.

So far when running it against the resources created by the test suite the terminator class seems good

Hm, maybe not - just caught this on my latest test run. Will need to debug more.
"msg": "Unable to modify DB instance: An error occurred (InvalidParameterValue) when calling the ModifyDBInstance operation: IAM role ARN value is invalid or does not include the required permissions for: ENHANCED_MONITORING"

@jillr No worries at all :) No I think you are correct ! I totally missed that section on policies ! I've removed my blob and added the policy to the allowed list. I think that's the only actual missing bit.

@jillr Actually I think I know whats happening, I spotted something similar internally but I think there's some eventual consistency delay, when creating the IAM role. After a retry it seems to pass once the IAM role has fully provisioned..

I removed the IAM deletion at the end of the tests, as it's not cost incurring and wouldn't change. And I moved the creation to before the first rds_instance creation.

@marknet15 - 'terminator' (the python code living in this module) will delete any roles you leave lying around for too long. If you're having problems with IAM roles being 'eventually consistent', I find it helps to move the iam role creation towards to the start of the tests. Given how slow the RDS tests are this should give enough time for everything to settle down.

@tremble thanks🙏 yep that’s a good suggestion, I moved the IAM creation to the start earlier, so fingers crossed that sorts it out as I think the issue was eventual consistency.

jillr

Unfortunately the tests are exceeding the 1 hour token duration limit we have on our CI role and are going to have to be either condensed or parallelized. I was only able to test up to around tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_read_replica.yml:107 before token expiration. @tremble has done some work getting the ec2_instance tests to run in parallel but I'm not sure offhand if that approach would work for RDS.

We really appreciate the time you've put into these PRs to get these tests in a better place!

aws/policy/data-services.yaml

tremble · 2021-09-28T07:48:18Z

@tremble has done some work getting the ec2_instance tests to run in parallel but I'm not sure offhand if that approach would work for RDS.

RDS instance tests are already running in parallel. RDS is just a pretty slow service to work with.

marknet15 · 2021-09-29T11:36:48Z

@tremble @jillr No worries :) I'm in the process of condensing the tests right down, as I've spotted some easy ones to do so with.

jillr

This is getting much closer. Given how slow RDS is as a service generally it might be difficult to get these under 60 minutes, I can plan to try to look at your test PR more closely next week after AnsibleFest and Contributor Summit are wrapped up.

aws/policy/data-services.yaml

aws/terminator/data_services.py

Co-authored-by: Jill R <4121322+jillr@users.noreply.github.com>

jillr

I ran into one more thing, we don't have - 'arn:aws:rds:{{ aws_region }}:{{ aws_account_id }}:snashots:*' in the resource list. But I'm going to just add a follow up PR in this repo right away so we don't have to keep your other PR waiting. I did have a token expiration still but it'll be easier to test in CI with these policies deployed. Thanks for all your work on this one!

Rds enhanced monitoring bug fix SUMMARY (a copy of #712 as I messed up my branch by accident) This is a fix for an issue when an RDS instance already exists and you wish to enable enhanced monitoring, for the full details see the linked old reported issue: ansible/ansible#51772 But in summary currently if you enable enhanced monitoring on an RDS instance that already exists where it isn't already enabled then the following is returned: An exception occurred during task execution. To see the full traceback, use -vvv. The error was: KeyError: 'MonitoringRoleArn' fatal: [localhost_eu-west-1-pdv-qa-1 -> 127.0.0.1]: FAILED! => changed=false module_stderr: |- Traceback (most recent call last): File "master:/opt/mitogen/mitogen-0.2.9/ansible_mitogen/runner.py", line 975, in _run self._run_code(code, mod) File "master:/opt/mitogen/mitogen-0.2.9/ansible_mitogen/runner.py", line 939, in _run_code exec(code, vars(mod)) File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 1245, in <module> File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 1210, in main File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 855, in get_parameters File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 885, in get_options_with_changing_values File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 983, in get_changing_options_with_consistent_keys KeyError: 'MonitoringRoleArn' module_stdout: '' msg: |- MODULE FAILURE See stdout/stderr for the exact error Originally-Depends-On: mattclay/aws-terminator#164 Other changes A load of issues have surfaced in the integration tests due to how slow RDS is to create / modify etc. I've condensed down the tests where possible reducing the number of inventory jobs to 6 and bumped serial to 6 so that hopefully all tests can run at once and finish within the 1 hr AWS session duration. ISSUE TYPE Bugfix Pull Request COMPONENT NAME rds_instance Reviewed-by: Mark Chappell <None> Reviewed-by: Mark Woolley <mw@marknet15.com> Reviewed-by: None <None>

Rds enhanced monitoring bug fix SUMMARY (a copy of ansible-collections#712 as I messed up my branch by accident) This is a fix for an issue when an RDS instance already exists and you wish to enable enhanced monitoring, for the full details see the linked old reported issue: ansible/ansible#51772 But in summary currently if you enable enhanced monitoring on an RDS instance that already exists where it isn't already enabled then the following is returned: An exception occurred during task execution. To see the full traceback, use -vvv. The error was: KeyError: 'MonitoringRoleArn' fatal: [localhost_eu-west-1-pdv-qa-1 -> 127.0.0.1]: FAILED! => changed=false module_stderr: |- Traceback (most recent call last): File "master:/opt/mitogen/mitogen-0.2.9/ansible_mitogen/runner.py", line 975, in _run self._run_code(code, mod) File "master:/opt/mitogen/mitogen-0.2.9/ansible_mitogen/runner.py", line 939, in _run_code exec(code, vars(mod)) File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 1245, in <module> File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 1210, in main File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 855, in get_parameters File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 885, in get_options_with_changing_values File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 983, in get_changing_options_with_consistent_keys KeyError: 'MonitoringRoleArn' module_stdout: '' msg: |- MODULE FAILURE See stdout/stderr for the exact error Originally-Depends-On: mattclay/aws-terminator#164 Other changes A load of issues have surfaced in the integration tests due to how slow RDS is to create / modify etc. I've condensed down the tests where possible reducing the number of inventory jobs to 6 and bumped serial to 6 so that hopefully all tests can run at once and finish within the 1 hr AWS session duration. ISSUE TYPE Bugfix Pull Request COMPONENT NAME rds_instance Reviewed-by: Mark Chappell <None> Reviewed-by: Mark Woolley <mw@marknet15.com> Reviewed-by: None <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@8fe00cb

Rds enhanced monitoring bug fix SUMMARY (a copy of ansible-collections#712 as I messed up my branch by accident) This is a fix for an issue when an RDS instance already exists and you wish to enable enhanced monitoring, for the full details see the linked old reported issue: ansible/ansible#51772 But in summary currently if you enable enhanced monitoring on an RDS instance that already exists where it isn't already enabled then the following is returned: An exception occurred during task execution. To see the full traceback, use -vvv. The error was: KeyError: 'MonitoringRoleArn' fatal: [localhost_eu-west-1-pdv-qa-1 -> 127.0.0.1]: FAILED! => changed=false module_stderr: |- Traceback (most recent call last): File "master:/opt/mitogen/mitogen-0.2.9/ansible_mitogen/runner.py", line 975, in _run self._run_code(code, mod) File "master:/opt/mitogen/mitogen-0.2.9/ansible_mitogen/runner.py", line 939, in _run_code exec(code, vars(mod)) File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 1245, in <module> File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 1210, in main File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 855, in get_parameters File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 885, in get_options_with_changing_values File "master:/tmp/build/4bef5c86/framework/library/cloud/aws/rds_instance.py", line 983, in get_changing_options_with_consistent_keys KeyError: 'MonitoringRoleArn' module_stdout: '' msg: |- MODULE FAILURE See stdout/stderr for the exact error Originally-Depends-On: mattclay/aws-terminator#164 Other changes A load of issues have surfaced in the integration tests due to how slow RDS is to create / modify etc. I've condensed down the tests where possible reducing the number of inventory jobs to 6 and bumped serial to 6 so that hopefully all tests can run at once and finish within the 1 hr AWS session duration. ISSUE TYPE Bugfix Pull Request COMPONENT NAME rds_instance Reviewed-by: Mark Chappell <None> Reviewed-by: Mark Woolley <mw@marknet15.com> Reviewed-by: None <None>

Add RDS describe permissions

506bdd0

marknet15 changed the title ~~Add RDS describe permissions~~ Add RDS DescribeDBInstances permissions Sep 20, 2021

extra missing permissions

bf7b38b

marknet15 changed the title ~~Add RDS DescribeDBInstances permissions~~ Add RDS missing permissions Sep 20, 2021

Merge branch 'master' into add-rds-describe

f620ec2

marknet15 added 5 commits September 21, 2021 09:47

PR feedback

45de876

missed restoredb

a2d70e3

lint failure

4345732

correct property

2dab909

Merge branch 'master' into add-rds-describe

4571135

alinabuzachis mentioned this pull request Sep 22, 2021

Fix RDS Enhanced Monitoring bug ansible-collections/community.aws#712

Closed

jillr reviewed Sep 23, 2021

View reviewed changes

Update IAM

8291a9f

jillr reviewed Sep 27, 2021

View reviewed changes

aws/policy/data-services.yaml Outdated Show resolved Hide resolved

aws/policy/data-services.yaml Show resolved Hide resolved

Merge branch 'master' into add-rds-describe

63f9332

PR feedback

ce80f0a

jillr reviewed Sep 29, 2021

View reviewed changes

aws/policy/data-services.yaml Show resolved Hide resolved

aws/terminator/data_services.py Show resolved Hide resolved

marknet15 and others added 3 commits October 1, 2021 15:57

Update aws/policy/data-services.yaml

ea70f19

Co-authored-by: Jill R <4121322+jillr@users.noreply.github.com>

Update aws/terminator/data_services.py

af6ea68

Co-authored-by: Jill R <4121322+jillr@users.noreply.github.com>

fix lint

840653b

jillr approved these changes Oct 7, 2021

View reviewed changes

jillr merged commit 9623f6d into mattclay:master Oct 7, 2021

marknet15 deleted the add-rds-describe branch October 8, 2021 08:22

This was referenced Oct 8, 2021

Rds enhanced monitoring bug fix ansible-collections/community.aws#747

Merged

Rds enhanced monitoring fix (retry) ansible-collections/community.aws#751

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add RDS missing permissions #164

Add RDS missing permissions #164

marknet15 commented Sep 20, 2021 •

edited

Loading

jillr commented Sep 20, 2021

jillr commented Sep 20, 2021

marknet15 commented Sep 21, 2021

jillr Sep 23, 2021

jillr Sep 24, 2021

marknet15 Sep 24, 2021 •

edited

Loading

marknet15 Sep 27, 2021 •

edited

Loading

marknet15 Sep 27, 2021 •

edited

Loading

tremble Sep 27, 2021

marknet15 Sep 27, 2021

jillr left a comment

tremble commented Sep 28, 2021

marknet15 commented Sep 29, 2021

jillr left a comment

jillr left a comment

Add RDS missing permissions #164

Add RDS missing permissions #164

Conversation

marknet15 commented Sep 20, 2021 • edited Loading

Overview

jillr commented Sep 20, 2021

jillr commented Sep 20, 2021

marknet15 commented Sep 21, 2021

jillr Sep 23, 2021

Choose a reason for hiding this comment

jillr Sep 24, 2021

Choose a reason for hiding this comment

marknet15 Sep 24, 2021 • edited Loading

Choose a reason for hiding this comment

marknet15 Sep 27, 2021 • edited Loading

Choose a reason for hiding this comment

marknet15 Sep 27, 2021 • edited Loading

Choose a reason for hiding this comment

tremble Sep 27, 2021

Choose a reason for hiding this comment

marknet15 Sep 27, 2021

Choose a reason for hiding this comment

jillr left a comment

Choose a reason for hiding this comment

tremble commented Sep 28, 2021

marknet15 commented Sep 29, 2021

jillr left a comment

Choose a reason for hiding this comment

jillr left a comment

Choose a reason for hiding this comment

marknet15 commented Sep 20, 2021 •

edited

Loading

marknet15 Sep 24, 2021 •

edited

Loading

marknet15 Sep 27, 2021 •

edited

Loading

marknet15 Sep 27, 2021 •

edited

Loading