Skip to content

Commit

Permalink
Merge pull request #3 from NickolasHKraus/improve-delete-logic
Browse files Browse the repository at this point in the history 
Improve delete logic
  • Loading branch information
Nickolas Kraus committed Sep 19, 2019
2 parents 43c5378 + b69dd0d commit 55e12c1
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 13 deletions.
2 changes: 1 addition & 1 deletion certificate_validator/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ check: ## check complexity and docstrings
echo 'Checking complexity...' && \
failed=0 && \
for f in $$(find $$(pwd)/certificate_validator/ -name '*.py';); do \
output=$$(python3 -m mccabe --min 7 "$$f") && \
output=$$(python3 -m mccabe --min 9 "$$f") && \
if [ $(strip "$$output") != "" ]; then \
echo "$(YELLOW)$$(basename $$f) is too complex.$(RESET)"; \
echo "$(YELLOW)$$output$(RESET)\n"; \
Expand Down
25 changes: 21 additions & 4 deletions certificate_validator/certificate_validator/resources.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,12 @@ def delete(self) -> None:
)
self.response.set_status(success=True)
except exceptions.ClientError as ex:
self.response.set_status(success=False)
self.response.set_reason(reason=str(ex))
if ex.response['Error']['Code'] == 'ResourceNotFoundException':
self.response.set_status(success=True)
self.response.set_reason(reason='Certificate not found.')
else:
self.response.set_status(success=False)
self.response.set_reason(reason=str(ex))


class CertificateValidator(CertificateMixin, Provider):
Expand Down Expand Up @@ -191,8 +195,21 @@ def change_resource_record_sets(
)
self.response.set_status(success=True)
except exceptions.ClientError as ex:
self.response.set_status(success=False)
self.response.set_reason(reason=str(ex))
if ex.response['Error']['Code'] == 'ResourceNotFoundException':
self.response.set_status(success=True)
self.response.set_reason(reason='Certificate not found.')
elif ex.response['Error']['Code'] == 'InvalidChangeBatch':
if 'not found' in ex.response['Error']['Message']:
self.response.set_status(success=True)
self.response.set_reason(
reason='Resource Record Set not found.'
)
else:
self.response.set_status(success=False)
self.response.set_reason(reason=str(ex))
else:
self.response.set_status(success=False)
self.response.set_reason(reason=str(ex))

def create(self) -> None:
"""
Expand Down
100 changes: 92 additions & 8 deletions certificate_validator/tests/test_resources.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def test_create_failed(self):
c = Certificate(self.request, self.mock_response)
self.mock_request_certificate.side_effect = exceptions.ClientError(
error_response={'Error': {
'Code': '1337',
'Code': 'Code',
'Message': 'Message'
}},
operation_name='Operation'
Expand All @@ -76,7 +76,7 @@ def test_create_failed(self):
)
self.mock_response.set_status.assert_called_with(success=False)
reason = \
'An error occurred (1337) when calling the Operation operation: ' \
'An error occurred (Code) when calling the Operation operation: ' \
'Message'
self.mock_response.set_reason.assert_called_with(reason=reason)

Expand All @@ -95,6 +95,28 @@ def test_delete_success_certificate_does_not_exist(self):
reason='Certificate does not exist.'
)

def test_delete_success_certificate_not_found(self):
self.mock_request.physical_resource_id = \
'arn:aws:acm:us-east-1:123:certificate/1337'
self.mock_delete_certificate.side_effect = exceptions.ClientError(
error_response={
'Error': {
'Code': 'ResourceNotFoundException',
'Message': 'Message'
}
},
operation_name='Operation'
)
c = Certificate(self.mock_request, self.mock_response)
c.delete()
self.mock_delete_certificate.assert_called_with(
certificate_arn='arn:aws:acm:us-east-1:123:certificate/1337'
)
self.mock_response.set_status.assert_called_with(success=True)
self.mock_response.set_reason.assert_called_with(
reason='Certificate not found.'
)

def test_delete_success(self):
self.mock_request.physical_resource_id = \
'arn:aws:acm:us-east-1:123:certificate/1337'
Expand All @@ -120,7 +142,7 @@ def test_delete_failed(self):
c = Certificate(self.mock_request, self.mock_response)
self.mock_delete_certificate.side_effect = exceptions.ClientError(
error_response={'Error': {
'Code': '1337',
'Code': 'Code',
'Message': 'Message'
}},
operation_name='Operation'
Expand All @@ -131,7 +153,7 @@ def test_delete_failed(self):
)
self.mock_response.set_status.assert_called_with(success=False)
reason = \
'An error occurred (1337) when calling the Operation operation: ' \
'An error occurred (Code) when calling the Operation operation: ' \
'Message'
self.mock_response.set_reason.assert_called_with(reason=reason)

Expand Down Expand Up @@ -198,7 +220,7 @@ def test_change_resource_record_sets_create_failed(self):
self.mock_get_domain_validation_options.side_effect = \
exceptions.ClientError(
error_response={'Error': {
'Code': '1337',
'Code': 'Code',
'Message': 'Message'
}},
operation_name='Operation'
Expand All @@ -207,10 +229,30 @@ def test_change_resource_record_sets_create_failed(self):
cv.change_resource_record_sets(self.certificate_arn, Action.CREATE)
self.mock_response.set_status.assert_called_with(success=False)
reason = \
'An error occurred (1337) when calling the Operation operation: ' \
'An error occurred (Code) when calling the Operation operation: ' \
'Message'
self.mock_response.set_reason.assert_called_with(reason=reason)

def test_change_resource_record_sets_create_failed_invalid_cb(self):
self.mock_request.resource_properties = {
'CertificateArn': self.certificate_arn
}
self.mock_get_domain_validation_options.side_effect = \
exceptions.ClientError(
error_response={'Error': {
'Code': 'InvalidChangeBatch',
'Message': 'Message'
}},
operation_name='Operation'
)
cv = CertificateValidator(self.mock_request, self.mock_response)
cv.change_resource_record_sets(self.certificate_arn, Action.DELETE)
self.mock_response.set_status.assert_called_with(success=False)
reason = \
'An error occurred (InvalidChangeBatch) when calling the ' \
'Operation operation: Message'
self.mock_response.set_reason.assert_called_with(reason=reason)

def test_change_resource_record_sets_upsert(self):
self.mock_request.resource_properties = {
'CertificateArn': self.certificate_arn
Expand Down Expand Up @@ -290,7 +332,7 @@ def test_change_resource_record_sets_delete_failed(self):
self.mock_get_domain_validation_options.side_effect = \
exceptions.ClientError(
error_response={'Error': {
'Code': '1337',
'Code': 'Code',
'Message': 'Message'
}},
operation_name='Operation'
Expand All @@ -299,10 +341,52 @@ def test_change_resource_record_sets_delete_failed(self):
cv.change_resource_record_sets(self.certificate_arn, Action.DELETE)
self.mock_response.set_status.assert_called_with(success=False)
reason = \
'An error occurred (1337) when calling the Operation operation: ' \
'An error occurred (Code) when calling the Operation operation: ' \
'Message'
self.mock_response.set_reason.assert_called_with(reason=reason)

def test_change_resource_record_sets_delete_failed_cert_not_found(self):
self.mock_request.resource_properties = {
'CertificateArn': self.certificate_arn
}
self.mock_get_domain_validation_options.side_effect = \
exceptions.ClientError(
error_response={'Error': {
'Code': 'ResourceNotFoundException',
'Message': 'Message'
}},
operation_name='Operation'
)
cv = CertificateValidator(self.mock_request, self.mock_response)
cv.change_resource_record_sets(self.certificate_arn, Action.DELETE)
self.mock_response.set_status.assert_called_with(success=True)
self.mock_response.set_reason.assert_called_with(
reason='Certificate not found.'
)

def test_change_resource_record_sets_delete_failed_rrset_not_found(self):
self.mock_request.resource_properties = {
'CertificateArn': self.certificate_arn
}
message = \
'Tried to delete resource record set ' \
'[name=\'_x1.certificate-validator.com.\', type=\'CNAME\'] but ' \
'it was not found'
self.mock_get_domain_validation_options.side_effect = \
exceptions.ClientError(
error_response={'Error': {
'Code': 'InvalidChangeBatch',
'Message': message
}},
operation_name='Operation'
)
cv = CertificateValidator(self.mock_request, self.mock_response)
cv.change_resource_record_sets(self.certificate_arn, Action.DELETE)
self.mock_response.set_status.assert_called_with(success=True)
self.mock_response.set_reason.assert_called_with(
reason='Resource Record Set not found.'
)

def test_create(self):
mock_wait = patch.object(resources.ACM, 'wait').start()
mock_change_resource_record_sets = \
Expand Down

0 comments on commit 55e12c1

Please sign in to comment.