Skip to content

Commit

Permalink
[Resolve #683] Fixing diff command to handle (and not create) invalid…
Browse files Browse the repository at this point in the history
… templates (#1159)

Some testing on the recent diff command made a couple things evident:
* The way we were generating the placeholder values for sceptre_user_data was not fully safe for rendering templates. A safer approach is to only use alphanumeric values.
* When retrieving a local template summary, if the template is invalid (such as invalid yaml or having invalid characters), we were currently interpreting that as the stack not existing... which wasn't true. This would happen specifically when getting a template summary for the LOCALLY generated template, so that didn't make sense.
  • Loading branch information
jfalkenstein committed Nov 10, 2021
1 parent 9822736 commit 7194886
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 5 deletions.
4 changes: 3 additions & 1 deletion sceptre/diffing/stack_differ.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,9 @@ def resolve_or_replace(attr, key, value: Resolver):
try:
attr[key] = value.resolve()
except Exception:
attr[key] = self._represent_unresolvable_resolver(value)
explicit_resolver_repr = self._represent_unresolvable_resolver(value)
only_alphanumeric = ''.join(c for c in explicit_resolver_repr if c.isalnum())
attr[key] = only_alphanumeric

# Because the name is mangled, we cannot access the user data normally, so we need to access
# it directly out of the __dict__.
Expand Down
7 changes: 5 additions & 2 deletions sceptre/plan/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -979,8 +979,11 @@ def _get_template_summary(self, **kwargs) -> Optional[dict]:
)
return template_summary
except botocore.exceptions.ClientError as e:
# AWS returns a ValidationError if the stack doesn't exist
if e.response['Error']['Code'] == 'ValidationError':
error_response = e.response['Error']
if (
error_response['Code'] == 'ValidationError'
and 'does not exist' in error_response['Message']
):
return None
raise

Expand Down
16 changes: 15 additions & 1 deletion tests/test_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1217,7 +1217,21 @@ def get_template_summary(service, command, kwargs):
result = self.actions.fetch_local_template_summary()
assert result == {'template': 'summary'}

def test_fetch_remote_template_summary__cloudformation_returns_validation_error__returns_none(self):
def test_fetch_local_template_summary__cloudformation_returns_validation_error_invalid_stack__raises_it(self):
self.actions.connection_manager.call.side_effect = ClientError(
{
"Error": {
"Code": "ValidationError",
"Message": "Template format error: Resource name {Invalid::Resource} is "
"non alphanumeric.'"
}
},
sentinel.operation
)
with pytest.raises(ClientError):
self.actions.fetch_local_template_summary()

def test_fetch_remote_template_summary__cloudformation_returns_validation_error_for_no_stack__returns_none(self):
self.actions.connection_manager.call.side_effect = ClientError(
{
"Error": {
Expand Down
2 changes: 1 addition & 1 deletion tests/test_diffing/test_stack_differ.py
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ def generate():

self.differ.diff(self.actions)

assert self.sceptre_user_data['unresolvable'] == '{ !Mock(test) }'
assert self.sceptre_user_data['unresolvable'] == 'Mocktest'

def test_diff__local_generation_raises_an_error__resolves_resolvable_sceptre_user_data(self):
has_raised = False
Expand Down

0 comments on commit 7194886

Please sign in to comment.