feat(elasticache): Ensure Redis replication groups have automatic failover enabled #4853

HugoPBrito · 2024-08-23T10:35:19Z

Context

A check was done to ensure ElastiCache (Redis OSS) replication groups have automatic failover enabled for high availability.

Description

I added a new check elasticache_redis_cluster_automatic_failover_enabled that ensures Elasticache Redis OSS cache clusters use automatic failover to maximize high availability and minimize downtime during failures.

Checklist

Are there new checks included in this PR? Yes / No
- If so, do we need to update permissions for the provider? Please review this carefully.
Review if the code is being covered by tests.
Review if code is being documented following this specification https://github.com/google/styleguide/blob/gh-pages/pyguide.md#38-comments-and-docstrings

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…r enabled

…o PRWLR-4508-elasti-cache-redis-oss-replication-groups-should-have-automatic-failover-enabled

codecov · 2024-08-23T11:04:37Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.93%. Comparing base (b7e2034) to head (7b62c9c).
Report is 1293 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4853      +/-   ##
==========================================
- Coverage   89.20%   88.93%   -0.28%     
==========================================
  Files         933      954      +21     
  Lines       28567    29256     +689     
==========================================
+ Hits        25483    26018     +535     
- Misses       3084     3238     +154

Components	Coverage Δ
prowler	`88.93% <100.00%> (-0.28%)`	⬇️
api	`∅ <ø> (∅)`

MrCloudSec · 2024-08-23T16:27:54Z

prowler/providers/aws/services/elasticache/elasticache_service.py

    auto_minor_version_upgrade: bool = False
+    automatic_failover: Optional[str] = None


You are already setting a default value when creating the model inside the function.

Suggested change

auto_minor_version_upgrade: bool = False

automatic_failover: Optional[str] = None

auto_minor_version_upgrade: bool

automatic_failover: str

If I don't set the default value here, several tests for this and other checks will fail as a result. This occurs because when we mock them, the value is never assigned.

Should I leave it as it was, or should I update every mock instance across all checks?

MrCloudSec · 2024-08-23T16:29:10Z

prowler/providers/aws/services/elasticache/elasticache_service.py

@@ -106,6 +106,9 @@ def _describe_replication_groups(self, regional_client):
                            auto_minor_version_upgrade=repl_group.get(
                                "AutoMinorVersionUpgrade", False
                            ),
+                            automatic_failover=repl_group.get(
+                                "AutomaticFailover", None


We always set the non-compliant value as the default.

Suggested change

"AutomaticFailover", None

"AutomaticFailover", "disabled"

MrCloudSec · 2024-08-23T16:29:31Z

tests/providers/aws/services/elasticache/elasticache_service_test.py

@@ -192,4 +192,5 @@ def test_describe_replication_groups(self):
            multi_az=REPLICATION_GROUP_MULTI_AZ,
            tags=REPLICATION_GROUP_TAGS,
            auto_minor_version_upgrade=AUTO_MINOR_VERSION_UPGRADE,
+            automatic_failover=None,


Suggested change

automatic_failover=None,

automatic_failover="disabled",

...utomatic_failover_enabled/elasticache_redis_cluster_automatic_failover_enabled.metadata.json

puchy22 · 2024-08-28T19:24:59Z

...utomatic_failover_enabled/elasticache_redis_cluster_automatic_failover_enabled.metadata.json

@@ -0,0 +1,32 @@
+{
+  "Provider": "aws",
+  "CheckID": "elasticache_redis_cluster_automatic_failover_enabled",


I think we should now something more like: elasticache_replication_group_redis_has_automatic_failover

puchy22 · 2024-08-28T19:25:33Z

...utomatic_failover_enabled/elasticache_redis_cluster_automatic_failover_enabled.metadata.json

+  "SubServiceName": "",
+  "ResourceIdTemplate": "arn:partition:service:region:account-id:resource-id",
+  "Severity": "medium",
+  "ResourceType": "AWSElastiCacheClusters",


I think this ResourceType is not supported by AWS, please refer to the docs

puchy22 · 2024-08-28T20:12:31Z

...s_cluster_automatic_failover_enabled/elasticache_redis_cluster_automatic_failover_enabled.py

+            report.resource_arn = repl_group.arn
+            report.resource_tags = repl_group.tags
+            report.status = "FAIL"
+            report.status_extended = f"Elasticache Redis cache cluster {repl_group.id} does not have automatic failover enabled."


Suggested change

report.status_extended = f"Elasticache Redis cache cluster {repl_group.id} does not have automatic failover enabled."

report.status_extended = f"ElastiCache Redis replication group {repl_group.id} does not have automatic failover enabled."

puchy22 · 2024-08-28T20:17:44Z

...ster_automatic_failover_enabled/elasticache_redis_cluster_automatic_failover_enabled_test.py

+            assert result[0].status == "PASS"
+            assert (
+                result[0].status_extended
+                == f"Elasticache Redis cache cluster {REPLICATION_GROUP_ID} does have automatic failover enabled."


Change this according to new changes in the checks

..._automatic_failover/elasticache_redis_replication_group_has_automatic_failover.metadata.json

puchy22 · 2024-09-11T08:09:06Z

..._automatic_failover/elasticache_redis_replication_group_has_automatic_failover.metadata.json

+    },
+    "Recommendation": {
+      "Text": "Enable automatic failover for ElastiCache (Redis OSS) clusters to ensure high availability and minimize downtime during failures.",
+      "Url": "https://docs.aws.amazon.com/AmazonElastiCache/latest/red-ug/Clusters.Modify.html#Clusters.Modify.CON"


I think this link is not working as expected, the tags at the end are not doing anything. And I cannot see the Automatic Failover recommendation. Please, review it.

puchy22

LGTM!

HugoPBrito added 3 commits August 22, 2024 15:47

feat(elasticache): Ensure Redis cache clusters have automatic failove…

d431ea1

…r enabled

Merge branch 'master' of https://github.com/prowler-cloud/prowler int…

5bd4682

…o PRWLR-4508-elasti-cache-redis-oss-replication-groups-should-have-automatic-failover-enabled

feat(elasticache): changed automatic_failover type from bool to str

f7593c2

HugoPBrito requested review from a team as code owners August 23, 2024 10:35

github-actions bot added the provider/aws Issues/PRs related with the AWS provider label Aug 23, 2024

fix(elasticache): Try to fix codecov

b6826f8

MrCloudSec reviewed Aug 23, 2024

View reviewed changes

...utomatic_failover_enabled/elasticache_redis_cluster_automatic_failover_enabled.metadata.json Outdated Show resolved Hide resolved

feat(elasticache): resolved comments and updated mocking

c86d9b1

puchy22 requested changes Aug 28, 2024

View reviewed changes

HugoPBrito added 2 commits August 29, 2024 10:26

feat(elaticache): resolved comments

2aced89

chore(elasticache): put caps in ElastiCache check output

61a15c6

puchy22 requested changes Sep 11, 2024

View reviewed changes

feat(elasticache): improved links in metadata

4c0faee

puchy22 previously approved these changes Sep 12, 2024

View reviewed changes

chore: enhance check

7b62c9c

MrCloudSec dismissed puchy22’s stale review via 7b62c9c September 12, 2024 15:44

MrCloudSec self-requested a review September 12, 2024 15:45

MrCloudSec approved these changes Sep 12, 2024

View reviewed changes

MrCloudSec merged commit cc8bc78 into master Sep 12, 2024
10 of 11 checks passed

MrCloudSec deleted the PRWLR-4508-elasti-cache-redis-oss-replication-groups-should-have-automatic-failover-enabled branch September 12, 2024 16:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(elasticache): Ensure Redis replication groups have automatic failover enabled #4853

feat(elasticache): Ensure Redis replication groups have automatic failover enabled #4853

HugoPBrito commented Aug 23, 2024

codecov bot commented Aug 23, 2024 •

edited

Loading

MrCloudSec Aug 23, 2024

HugoPBrito Aug 26, 2024

MrCloudSec Aug 23, 2024

MrCloudSec Aug 23, 2024

puchy22 Aug 28, 2024

puchy22 Aug 28, 2024

puchy22 Aug 28, 2024

puchy22 Aug 28, 2024

puchy22 Sep 11, 2024

puchy22 left a comment

		auto_minor_version_upgrade: bool = False
		automatic_failover: Optional[str] = None

	report.status_extended = f"Elasticache Redis cache cluster {repl_group.id} does not have automatic failover enabled."
	report.status_extended = f"ElastiCache Redis replication group {repl_group.id} does not have automatic failover enabled."

feat(elasticache): Ensure Redis replication groups have automatic failover enabled #4853

feat(elasticache): Ensure Redis replication groups have automatic failover enabled #4853

Conversation

HugoPBrito commented Aug 23, 2024

Context

Description

Checklist

License

codecov bot commented Aug 23, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

puchy22 left a comment

Choose a reason for hiding this comment

codecov bot commented Aug 23, 2024 •

edited

Loading