Skip to content

fix: handle security group updates on ElastiCache modify operations - #2438

Open
kkonaparthi1 wants to merge 2 commits into
ansible-collections:mainfrom
kkonaparthi1:fix-elasticache-security-groups
Open

fix: handle security group updates on ElastiCache modify operations#2438
kkonaparthi1 wants to merge 2 commits into
ansible-collections:mainfrom
kkonaparthi1:fix-elasticache-security-groups

Conversation

@kkonaparthi1

Copy link
Copy Markdown

Fixes #661

Summary

Fix security group handling during ElastiCache cluster modifications that caused a KeyError: 'SecurityGroups' and prevented security group updates from being applied.

Changes

  • _requires_modification(): Use .get() with default empty list for both CacheSecurityGroups and SecurityGroups keys to avoid KeyError on VPC-based clusters
  • _requires_modification(): Only check cache security groups when cache_security_groups is specified (matching the existing pattern for VPC security groups)
  • modify(): Only include CacheSecurityGroupNames and SecurityGroupIds in the API call when they are actually specified, avoiding errors from passing empty lists

Backwards Compatible

No behavior change for existing playbooks. Clusters that already had matching security groups will continue to show no changes needed.

Fix two issues with security group handling during cluster modifications:

1. CacheSecurityGroups KeyError: The _requires_modification() method
   accessed self.data['CacheSecurityGroups'] directly, which raises a
   KeyError for VPC-based clusters that don't have this key. Changed to
   use self.data.get('CacheSecurityGroups', []) and only check when
   cache_security_groups is specified.

2. Empty security group parameters: The modify() method always passed
   CacheSecurityGroupNames and SecurityGroupIds even when empty, which
   could cause API errors. Changed to only include these parameters in
   the modify_cache_cluster call when they are actually specified.

Fixes ansible-collections#661

Signed-off-by: Karthik Konaparthi <konapk@amazon.com>
@centosinfra-prod-github-app

Copy link
Copy Markdown
Contributor

Build failed.
https://gateway-cloud-softwarefactory.apps.ocp.cloud.ci.centos.org/zuul/t/ansible/buildset/49f568be918140c29210844df589338a

⚠️ ansible-galaxy-importer SKIPPED Skipped due to failed job build-ansible-collection (non-voting)
build-ansible-collection RETRY_LIMIT Host unreachable in 36s
ansible-test-splitter RETRY_LIMIT Host unreachable in 33s
⚠️ integration-community.aws-1 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-2 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-3 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-4 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-5 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-6 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-7 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-8 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-9 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-10 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-11 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-12 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-13 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-14 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-15 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-16 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-17 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-18 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-19 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-20 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-21 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-22 SKIPPED Skipped due to failed job ansible-test-splitter

Signed-off-by: Karthik Konaparthi <konapk@amazon.com>
@centosinfra-prod-github-app

Copy link
Copy Markdown
Contributor

Build failed.
https://gateway-cloud-softwarefactory.apps.ocp.cloud.ci.centos.org/zuul/t/ansible/buildset/a13f7f1976fa4cb6a4cc4650e73fb422

⚠️ ansible-galaxy-importer SKIPPED Skipped due to failed job build-ansible-collection (non-voting)
build-ansible-collection RETRY_LIMIT Host unreachable in 37s
ansible-test-splitter RETRY_LIMIT Host unreachable in 33s
⚠️ integration-community.aws-1 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-2 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-3 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-4 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-5 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-6 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-7 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-8 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-9 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-10 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-11 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-12 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-13 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-14 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-15 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-16 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-17 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-18 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-19 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-20 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-21 SKIPPED Skipped due to failed job ansible-test-splitter
⚠️ integration-community.aws-22 SKIPPED Skipped due to failed job ansible-test-splitter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

elasticache : Security groups not being removed on update

1 participant