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.

Sign up for GitHub

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

Jump to bottom

rds_param_group: WARN on updating DBParameterGroupFamily (engine) #1169

Merged
merged 5 commits into from
Oct 20, 2022
Merged

rds_param_group: WARN on updating DBParameterGroupFamily (engine) #1169

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9e44560
fail on trying to modify parameter group engine/family
mandar242 Oct 13, 2022
c953fac
Added changelogs fragment
mandar242 Oct 13, 2022
6c41bc3
Fix integration test task name
mandar242 Oct 14, 2022
fbab796
show warning instead of failure on modifying engine/family
mandar242 Oct 18, 2022
32c5bb7
fix task name for integration test task
mandar242 Oct 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/1169-rds_param_group-fail-on-updating-engine.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
bugfixes:
- rds_param_group - added a check to fail the task while modifying/updating rds_param_group if trying to change DB parameter group family. (https://github.com/ansible-collections/amazon.aws/pull/1169).
7 changes: 7 additions & 0 deletions plugins/modules/rds_param_group.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
- The type of database for this group.
- Please use following command to get list of all supported db engines and their respective versions.
- '# aws rds describe-db-engine-versions --query "DBEngineVersions[].DBParameterGroupFamily"'
- The DB parameter group family can't be changed when updating a DB parameter group.
mandar242 marked this conversation as resolved.
Show resolved Hide resolved
See U(https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-rds-dbparametergroup.html)
- Required for I(state=present).
type: str
immediate:
Expand Down Expand Up @@ -266,6 +268,11 @@ def ensure_present(module, connection):
module.fail_json_aws(e, msg="Couldn't create parameter group")
else:
group = response['DBParameterGroups'][0]
db_parameter_group_family = group['DBParameterGroupFamily']

if module.params.get('engine') != db_parameter_group_family:
module.fail_json(msg="The DB parameter group family (engine) can't be changed when updating a DB parameter group.")
Copy link
Contributor

@alinabuzachis alinabuzachis Oct 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It the API doesn't originally fail, wouldn't it make more sense to only warn instead of fail? It there isn't any other update to do, it would show up the warning and changed=False. This is only a suggestion.

What do you think @mandar242 @tremble @jillr ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alinabuzachis that seems reasonable, I was going back and forth on this but later decided to fail on it
as there could be case where user wants to update more parameter (modifiable) along with DBParameterGroupFamily, if we give a warn instead of fail, and modify parameters that are possible, it could be easily misunderstood as original reported has mentioned that DBParameterGroupFamily is also changed.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's be really clear that this is an immutable option in the docs (I added a small suggestion) and then give the user a warning, that way we don't introduce a breaking change and we're consistent with the API.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alinabuzachis, as @jillr suggested, changed the failure to warning, to avoid introducing breaking changes.
Now the task shows a warning as below

TASK [rds_param_group : test modifying rds parameter group engine/family] ******
[WARNING]: The DB parameter group family (engine) can't be changed when
updating a DB parameter group.
ok: [testhost] => {"changed": false .....


if tags:
changed = update_tags(module, connection, group, tags)

Expand Down
1 change: 1 addition & 0 deletions tests/integration/targets/rds_param_group/defaults/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ rds_param_group:
name: '{{ resource_prefix}}rds-param-group'
description: Test group for rds_param_group Ansible module
engine: postgres9.6
engine_to_modify_to: postgres10

rds_long_param_list:
application_name: Test
Expand Down
20 changes: 20 additions & 0 deletions tests/integration/targets/rds_param_group/tasks/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,26 @@
- result.tags["Test"] == '123'

# ============================================================

- name: test modifying rds parameter group engine/family
rds_param_group:
name: '{{ rds_param_group.name }}'
engine: '{{ rds_param_group.engine_to_modify_to }}'
description: '{{ rds_param_group.description }}'
state: present
tags:
Environment: test
Test: 123
register: result
ignore_errors: true

- name: verify that modifying rds param group engine/family failed
mandar242 marked this conversation as resolved.
Show resolved Hide resolved
assert:
that:
- not result.changed
- result.failed

# ============================================================
- name: test tagging existing group - CHECK_MODE
rds_param_group:
name: '{{ rds_param_group.name }}'
Expand Down