feat(rds): add ReadReplicaSourceDBInstanceIdentifier to db_instance

[uridealo]

Context

added ReadReplicaSourceDBInstanceIdentifier to aws rds_service

Description

Including ReadReplicaSourceDBInstanceIdentifier helps skipping replica instances in rds_instance_backup_enabled

License

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

[uridealo]

@jfagoagas yes, i wrote a customized check on the basis of rds_instance_backup_enabled to skip replica instances. What means the failing linter? Can i do something here? I can reproduce but don't know how to fix.

[jfagoagas]

@jfagoagas yes, i wrote a customized check on the basis of rds_instance_backup_enabled to skip replica instances.

Are you planning to make a PR with that? Could be interesting to include a flag in the configuration to check that instances whether the value is set or not.

What means the failing linter? Can i do something here? I can reproduce but don't know how to fix.

I think you have to configure your local development environment following this guide in our documentation. You have to enable the pre-commit to run the formatters and linters.

[uridealo]

@jfagoagas i committed the customized check but not sure if its worth. Its very close to rds_instance_backup_enabled and would be better to have it as an option for this. But i don't know how to do so. Let me know what think about the check.

I followed the guide but i still don't understand the error message. Maybe i start from scratch.

[jfagoagas]

@jfagoagas i committed the customized check but not sure if its worth. Its very close to rds_instance_backup_enabled and would be better to have it as an option for this. But i don't know how to do so. Let me know what think about the check.

I think it would be better to adjust that check, let me push some changes and I'll get back to you.

I followed the guide but i still don't understand the error message. Maybe i start from scratch.

[jfagoagas]

Hi @uridealo, I've pushed a new version using the Prowler configuration file with a new check_rds_replicas value set to True by default. Please execute it agains your environment and let me know if that works, thanks!

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 86.40%. Comparing base (722554a) to head (9ee1b94). Report is 34 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3912      +/-   ##
==========================================
+ Coverage   86.32%   86.40%   +0.08%     
==========================================
  Files         748      749       +1     
  Lines       23295    23389      +94     
==========================================
+ Hits        20110    20210     +100     
+ Misses       3185     3179       -6     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[sergargar]

@jfagoagas @uridealo why we have only done this for this check? It would be interesting to add this logic for the rest of RDS Instance checks.

[jfagoagas]

@jfagoagas @uridealo why we have only done this for this check? It would be interesting to add this logic for the rest of RDS Instance checks.

I think for know we should apply this to the instance backups and then analyze if this can be included in other checks.

[uridealo]

@jfagoagas i tested your changes. ImO the logic is weird. I only skip replicas when check_rds_replicas is NOT set in config. When leaving out "True" in rds_instance_backup_enabled.py:24 i can control the skipping via True/False in config. Do i see something wrong?

[jfagoagas]

@jfagoagas i tested your changes. ImO the logic is weird. I only skip replicas when check_rds_replicas is NOT set in config. When leaving out "True" in rds_instance_backup_enabled.py:24 i can control the skipping via True/False in config. Do i see something wrong?

I don't get that. Currently the logic does the following:

if db_instance.replica_source and not rds_client.audit_config.get(
    "check_rds_replicas", True
):
    continue

So as you can see in the tests there are two possible scenarios:

• check_rds_replicas == True -> If the DB instance is a replica the finding is added to the list of findings.
• check_rds_replicas == False -> If the DB instance is a replica the finding is discarded.

Also by default we set that value to True since it is the current behaviour.

[uridealo]

@jfagoagas yes i see your tests. i tested in real, but maybe its my bad. When you are sure you can merge. I am fine with this

[jfagoagas]

Hi @uridealo we've made the decision to set the check_rds_instance_replicas by default to False so you don't need to do anything and your RDS instance replicas won't be included in the report.

[uridealo]

@jfagoagas That sounds great. Thanks a lot for your help

[uridealo]

@jfagoagas In which version will this be released? Also in v3?

[jfagoagas]

@jfagoagas In which version will this be released? Also in v3?

Hi @uridealo this will be included in the following v4.2.0 and v3.16.5. Thanks!