Validated the region passed before instantiating the service class #16463

nico-stefani · 2023-03-20T20:07:38Z

Related issue
#16301

Description

This PR closes #16301. Adds validation for the region passed to the service classes, in order to prevent requesting inexistent endpoints.

Tests

root@ubuntu-jammy:/home/vagrant/qa/tests/integration/test_aws# pytest -k 'cloudwatchlogs or inspector' --disable-warnings --tb=line --tier 0 test_regions.py
================================================================================================ test session starts ================================================================================================
platform linux -- Python 3.10.6, pytest-7.1.2, pluggy-1.0.0
rootdir: /home/vagrant/qa/tests/integration, configfile: pytest.ini
plugins: metadata-2.0.2, html-3.1.1, testinfra-5.0.0
collected 24 items / 18 deselected / 6 selected

test_regions.py ......                                                                                                                                                                                        [100%]

============================================================================= 6 passed, 18 deselected, 2 warnings in 132.06s (0:02:12) ==============================================================================

fdalmaup

Adding to the suggested change in the code, why does the module continue with its execution and not exit when an invalid region is passed? Wouldn't be better if the module shows an error and exits to alert the user of the wrong configuration?

wodles/aws/aws_s3.py

nico-stefani · 2023-03-28T12:01:26Z

Adding to the suggested change in the code, why does the module continue with its execution and not exit when an invalid region is passed? Wouldn't be better if the module shows an error and exits to alert the user of the wrong configuration?

I chose this behavior because the module can receive multiple regions for one execution. Then if one valid and one invalid region are passed, the execution can process the valid one.

fdalmaup · 2023-03-28T20:01:47Z

wodles/aws/aws_s3.py

+                try:
+                    service_type.check_region(region)
+                except ValueError:
+                    debug(f"+++ WARNING: The region '{region}' is not a valid one.", 1)


The warning message is only displayed when the module is manually executed but not when configured in the ossec.conf file. We should check this behavior so the users acknowledge the problem as soon as possible and do not need to set the debug level and reset the module.

As we discuss, the best solution, for now, is to raise an exception and quit the execution. Until we have #16301.
I will do the changes.

fdalmaup

LGTM!

Co-authored-by: Facundo Dalmau <facundo.dalmau@wazuh.com>

…16463) * Validated the region passed before instantiating the service class * Apply suggestions from code review Co-authored-by: Facundo Dalmau <facundo.dalmau@wazuh.com> * Exit with error when receive and invalid region --------- Co-authored-by: Facundo Dalmau <facundo.dalmau@wazuh.com>

* Validated the region passed before instantiating the service class (#16463) * Validated the region passed before instantiating the service class * Apply suggestions from code review Co-authored-by: Facundo Dalmau <facundo.dalmau@wazuh.com> * Exit with error when receive and invalid region --------- Co-authored-by: Facundo Dalmau <facundo.dalmau@wazuh.com> * Show explicit messages when there aren't logs to process (#16365) * Improve filter aguments for custom buckets * Improve check for empty bucket in server access * Fix custom bucket markers (#16410) * Fix query paramters for CustomBucket.sql_find_last_key_processed * Show message when there aren't logs to process in custom buckets * Apply improvements to ServerAccessBucket.iter_files_in_bucket * Add missing counter for processed_logs * Apply suggestions from code review Co-authored-by: Facundo Dalmau <facundo.dalmau@wazuh.com> * Fix method callbacks --------- Co-authored-by: Facundo Dalmau <facundo.dalmau@wazuh.com> * Improve VPCFlow and Config items iteration (#16325) * Implement basic changes * Use methods inherited from AWSBucket * Improve items iteration AWSVPCFlowBucket * Use methods inherited from AWSBucket in AWSVPCFlowBucket * Clean up unused methods and variables * Fix too many blank lines * Handle exception when an inexistent region was provided (#16332) * Handle exception when an inexistent region was provided * Get regions from constant * Use AWSBucket.empty_bucket_message_template in AWSLBBucket class * Add AWS parser validations (#16493) * Fix bucket and service empty messages * Fix bucket and service invalid value messages * Improve regex validation for bucket name * Improve regex validation for prefix and rename function * Improved regex validation for region and avoided repeated ones * Show error and exit for empty log group * Added function to validate aws_log_groups argument * Sorted regions after validation * Use AWSBucket.empty_bucket_message_template native guarduty case --------- Co-authored-by: Facundo Dalmau <facundo.dalmau@wazuh.com>

nico-stefani added type/bug Something isn't working module/aws module/cloud monitoring Monitoring external services (AWS, Azure, GCP, O365...) team/framework labels Mar 20, 2023

nico-stefani self-assigned this Mar 20, 2023

nico-stefani linked an issue Mar 20, 2023 that may be closed by this pull request

Show an explicit message when an invalid region is passed #16301

Closed

8 tasks

fdalmaup requested changes Mar 27, 2023

View reviewed changes

wodles/aws/aws_s3.py Outdated Show resolved Hide resolved

fdalmaup requested changes Mar 28, 2023

View reviewed changes

fdalmaup approved these changes Mar 29, 2023

View reviewed changes

nico-stefani changed the base branch from 4.5 to fix/aws-module-improvements March 30, 2023 15:45

nico-stefani and others added 3 commits April 13, 2023 10:11

Validated the region passed before instantiating the service class

a401945

Apply suggestions from code review

c2eb1a2

Co-authored-by: Facundo Dalmau <facundo.dalmau@wazuh.com>

Exit with error when receive and invalid region

d401ddf

nico-stefani force-pushed the fix/16301-explicit-messages-for-services-invalid-region branch from 9306286 to d401ddf Compare April 13, 2023 13:12

davidjiglesias merged commit d02485f into fix/aws-module-improvements Apr 14, 2023
2 checks passed

davidjiglesias deleted the fix/16301-explicit-messages-for-services-invalid-region branch April 14, 2023 07:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Validated the region passed before instantiating the service class #16463

Validated the region passed before instantiating the service class #16463

nico-stefani commented Mar 20, 2023

fdalmaup left a comment

nico-stefani commented Mar 28, 2023

fdalmaup Mar 28, 2023

nico-stefani Mar 29, 2023

fdalmaup left a comment

Validated the region passed before instantiating the service class #16463

Validated the region passed before instantiating the service class #16463

Conversation

nico-stefani commented Mar 20, 2023

Description

Tests

fdalmaup left a comment

Choose a reason for hiding this comment

nico-stefani commented Mar 28, 2023

fdalmaup Mar 28, 2023

Choose a reason for hiding this comment

nico-stefani Mar 29, 2023

Choose a reason for hiding this comment

fdalmaup left a comment

Choose a reason for hiding this comment