rds_instance - fix check_mode and idempotence bugs and support adding/removing iam roles #1002
Conversation
ansibullbot
added
community_review
feature
jatorcasso
force-pushed
the
rds_instance_iam_roles
branch
from
a987ca1
to
f857278
Compare
|
Build failed.
|
|
Not all RDS engines supports IAM roles. E.g. mariadb. |
That's a good point. I suppose that can be added as a pre-check in rds_instance or as a caught error in amazon.aws.module_utils.rds.handle_errors. I would guess add it to the utils handle_errors since that already exists, but im never really sure which collection I should be modifying.. |
I guess a pre-check inside rds_instance will be enough. Please also document the engines supporting iam roles. |
I actually think it makes more sense in this case to add an extra conditional in the Only issue is the accepted engines here are not well documented |
Where did you find that iam roles are not supported by all the engines https://boto3.amazonaws.com/v1/documentation/api/1.9.185/reference/services/rds.html#RDS.Client.add_role_to_db_instance? This only happens for rds cluster (https://boto3.amazonaws.com/v1/documentation/api/1.9.185/reference/services/rds.html#RDS.Client.add_role_to_db_cluster). |
|
its convoluted, but some engines dont support any features. for example from https://docs.aws.amazon.com/AmazonRDS/latest/APIReference/API_DBEngineVersion.html:
I had to learn this in order to properly test.. many of the db engines simply return an empty list of supported features. |
Ok. yeah! So I guess you need to handle that as you suggested directly inside module_utils in amazon.aws. Cover it by a unit test. |
... just experiences :) |
I'm not sure .... if |
|
looks like the valid db engines need to be updated as well. I can make that a separate issue/corresponding PR? |
I'll add in |
If we want to land this in 3.2.0, it might be easier to include the updated engine list also in this PR. But it's upt to you. |
jatorcasso
force-pushed
the
rds_instance_iam_roles
branch
from
45231b9
to
3ec9854
Compare
tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_upgrade.yml
Outdated
Show resolved
Hide resolved
Backport to stable-3: 💚 backport PR created✅ Backport PR branch: Backported as #1055 🤖 @patchback |
…/removing iam roles (#1002) rds_instance - fix check_mode and idempotence bugs and support adding/removing iam roles SUMMARY Support the addition and deletion of iam roles to db instances Fixes #464 Fixes #1013 Integration tests to test both this and the amazon.aws module_util rds changes Depends-On ansible-collections/amazon.aws#714 ISSUE TYPE Feature Pull Request COMPONENT NAME rds_instance ADDITIONAL INFORMATION Wasn't sure the best way to go about deleting IAM roles - ended up using a purge_iam_roles param that defaults to False, which seems consistent with other modules I've looked at. Reviewed-by: Mark Woolley <mw@marknet15.com> Reviewed-by: Joseph Torcasso <None> Reviewed-by: Gonéri Le Bouder <goneri@lebouder.net> Reviewed-by: Alina Buzachis <None> Reviewed-by: Markus Bergholz <git@osuv.de> (cherry picked from commit c403552)
…ing/removing IAM roles (ansible-collections#714) rds module_util - fix check_mode and idempotence bugs and support adding/removing IAM roles SUMMARY Add waiter for promoting read replica to fix idempotence bug in community.aws integration testing Support modifying IAM roles to a db instance for ansible-collections/community.aws#1002 Add necessary waiters for both adding and removing an IAM role compare and ensure iam_roles methods for idempotency unit & integration tests for coverage (integration tests in community.aws PR) ISSUE TYPE Feature Pull Request COMPONENT NAME module_util/rds community.aws.rds_instance ADDITIONAL INFORMATION Refs: (adding role) https://awscli.amazonaws.com/v2/documentation/api/latest/reference/rds/add-role-to-db-instance.html (removing role) https://awscli.amazonaws.com/v2/documentation/api/latest/reference/rds/remove-role-from-db-instance.html (waiters) https://awscli.amazonaws.com/v2/documentation/api/latest/reference/rds/describe-db-instances.html Reviewed-by: Alina Buzachis <None> Reviewed-by: Joseph Torcasso <None> Reviewed-by: Mark Woolley <mw@marknet15.com> Reviewed-by: Mandar Kulkarni <mandar242@gmail.com> Reviewed-by: Jill R <None> Reviewed-by: Mike Graves <mgraves@redhat.com> (cherry picked from commit 00c752e)
…/removing iam roles (#1002) (#1055) [PR #1002/c403552f backport][stable-3] rds_instance - fix check_mode and idempotence bugs and support adding/removing iam roles This is a backport of PR #1002 as merged into main (c403552). SUMMARY Support the addition and deletion of iam roles to db instances Fixes #464 Fixes #1013 Integration tests to test both this and the amazon.aws module_util rds changes Depends-On ansible-collections/amazon.aws#714 ISSUE TYPE Feature Pull Request COMPONENT NAME rds_instance ADDITIONAL INFORMATION Wasn't sure the best way to go about deleting IAM roles - ended up using a purge_iam_roles param that defaults to False, which seems consistent with other modules I've looked at. Reviewed-by: Mark Chappell <None> Reviewed-by: Markus Bergholz <git@osuv.de>
…/removing iam roles (ansible-collections#1002) rds_instance - fix check_mode and idempotence bugs and support adding/removing iam roles SUMMARY Support the addition and deletion of iam roles to db instances Fixes ansible-collections#464 Fixes ansible-collections#1013 Integration tests to test both this and the amazon.aws module_util rds changes Depends-On ansible-collections/amazon.aws#714 ISSUE TYPE Feature Pull Request COMPONENT NAME rds_instance ADDITIONAL INFORMATION Wasn't sure the best way to go about deleting IAM roles - ended up using a purge_iam_roles param that defaults to False, which seems consistent with other modules I've looked at. Reviewed-by: Mark Woolley <mw@marknet15.com> Reviewed-by: Joseph Torcasso <None> Reviewed-by: Gonéri Le Bouder <goneri@lebouder.net> Reviewed-by: Alina Buzachis <None> Reviewed-by: Markus Bergholz <git@osuv.de> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections@c403552
SUMMARY
Depends-On ansible-collections/amazon.aws#714
ISSUE TYPE
COMPONENT NAME
rds_instance
ADDITIONAL INFORMATION
Wasn't sure the best way to go about deleting IAM roles - ended up using a
purge_iam_rolesparam that defaults toFalse, which seems consistent with other modules I've looked at.