Skip to content

Test apigateway rest api client certificate enabled#305

Open
prajwal-choudhari-comprinno wants to merge 36 commits intocomprinnotech:testcases_devfrom
prajwal-choudhari-comprinno:test_apigateway_rest_api_client_certificate_enabled
Open

Test apigateway rest api client certificate enabled#305
prajwal-choudhari-comprinno wants to merge 36 commits intocomprinnotech:testcases_devfrom
prajwal-choudhari-comprinno:test_apigateway_rest_api_client_certificate_enabled

Conversation

@prajwal-choudhari-comprinno
Copy link

@prajwal-choudhari-comprinno prajwal-choudhari-comprinno commented Jun 27, 2025

Context

Adding unit tests for the apigateway_rest_api_client_certificate_enabled check to validate its correctness across different API Gateway scenarios. Ensures that the check behaves as expected and improves confidence during future modifications or refactoring.

Description

This PR adds a new test suite test_apigateway_rest_api_client_certificate_enabled.py to verify the behavior of the check in various conditions:

  • When no REST APIs exist (check passes with no resources)
  • When all REST APIs have client certificate enforcement enabled (check passes)
  • When any REST API lacks client certificate enforcement (check fails)
  • When AWS raises a ClientError (check status is unknown)

Mocks are used to simulate the AWS API responses from the apigateway client.
No external dependencies added.
Covers both positive and negative test scenarios.

Checklist

License

I confirm that my contribution is made under the terms of the Apache 2.0 license.

abhay-pai-comprinno and others added 19 commits February 3, 2025 12:08
feat: checks refactored and updated
Dev to Main: (feat: masked access key IDs in report summaries)
self.mock_apigw = MagicMock()
self.mock_session.client.return_value = self.mock_apigw

@patch("boto3.Session.client")
Copy link
Contributor

Choose a reason for hiding this comment

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

@patch("boto3.Session.client") is unnecessary since you're already mocking session.client directly in setup_method.

Choose a reason for hiding this comment

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

removed

assert report.resource_ids_status == []

@patch("boto3.Session.client")
def test_waf_acl_attached(self, mock_client):
Copy link
Contributor

Choose a reason for hiding this comment

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

You are mocking get_rest_api, but the real check uses get_stages().

Choose a reason for hiding this comment

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

corrected

@patch("boto3.Session.client")
def test_waf_acl_attached(self, mock_client):
"""Test when all REST APIs have WAF ACL attached."""
self.mock_apigw.get_rest_apis.return_value = {
Copy link
Contributor

Choose a reason for hiding this comment

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

This test does not validate WAF attachment logic and always returns FAILED.

Choose a reason for hiding this comment

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

Validates that the check correctly detects WAF ACL attachment at stage level and returns PASSED when the required tag is present.

self.mock_apigw.get_rest_apis.side_effect = ClientError({"Error": {"Code": "AccessDenied"}}, "GetRestApis")
report = self.check.execute(self.mock_session)
assert report.status == CheckStatus.UNKNOWN
assert report.resource_ids_status[0].summary
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct mocking target (get_stages)

Add realistic mock returns for WAF presence/absence

Improve assertions to include summary text

Add missing cases (exception on get_stages, no stages)

Choose a reason for hiding this comment

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

corrected

assert report.status == CheckStatus.UNKNOWN
assert len(report.resource_ids_status) == 1
assert report.resource_ids_status[0].status == CheckStatus.UNKNOWN
assert "API Gateway listing error." in report.resource_ids_status[0].summary
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Pagination Test – Simulate two pages of REST APIs using position.
  2. Missing stageName – Stage object without stageName key.
  3. Assert exception message – In both get_stages and get_rest_apis error scenarios.

Choose a reason for hiding this comment

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

Addressed pagination handling via position, covered missing stageName case based on actual check logic (where clientCertificateId drives pass/fail), and asserted that exception messages are captured in resource_ids_status for both get_stages and get_rest_apis.

prajwal-choudhari-comprinno and others added 3 commits July 16, 2025 14:27
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.

4 participants