Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes issue with non-interactive changeset updates & stack policies #657

Merged
merged 1 commit into from
Sep 7, 2018
Merged

Fixes issue with non-interactive changeset updates & stack policies #657

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 47 additions & 10 deletions stacker/providers/aws/default.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,8 @@ def generate_cloudformation_args(stack_name, parameters, tags, template,
with create_change_set.
service_role (str, optional): An optional service role to use when
interacting with Cloudformation.
stack_policy (:class:`stacker.providers.base.Template`): A template
object representing a stack policy.
change_set_name (str, optional): An optional change set name to use
with create_change_set.

Expand Down Expand Up @@ -434,6 +436,16 @@ def generate_cloudformation_args(stack_name, parameters, tags, template,


def generate_stack_policy_args(stack_policy=None):
""" Converts a stack policy object into keyword args.

Args:
stack_policy (:class:`stacker.providers.base.Template`): A template
object representing a stack policy.

Returns:
dict: A dictionary of keyword arguments to be used elsewhere.
"""

args = {}
if stack_policy:
logger.debug("Stack has a stack policy")
Expand Down Expand Up @@ -666,6 +678,8 @@ def create_stack(self, fqn, template, parameters, tags,
tags (list): A list of dictionaries that defines the tags
that should be applied to the Cloudformation stack.
force_change_set (bool): Whether or not to force change set use.
stack_policy (:class:`stacker.providers.base.Template`): A template
object representing a stack policy.
"""

logger.debug("Attempting to create stack %s:.", fqn)
Expand Down Expand Up @@ -813,6 +827,8 @@ def update_stack(self, fqn, template, old_parameters, parameters, tags,
not. False will follow the behavior of the provider.
force_change_set (bool): A flag that indicates whether the update
must be executed with a change set.
stack_policy (:class:`stacker.providers.base.Template`): A template
object representing a stack policy.
"""
logger.debug("Attempting to update stack %s:", fqn)
logger.debug(" parameters: %s", parameters)
Expand All @@ -824,11 +840,28 @@ def update_stack(self, fqn, template, old_parameters, parameters, tags,
update_method = self.select_update_method(force_interactive,
force_change_set)

return update_method(fqn, template, old_parameters, parameters, tags,
stack_policy=stack_policy, **kwargs)
return update_method(fqn, template, old_parameters, parameters,
stack_policy=stack_policy, tags=tags, **kwargs)

def deal_with_changeset_stack_policy(self, fqn, stack_policy):
""" Set a stack policy when using changesets.

ChangeSets don't allow you to set stack policies in the same call to
update them. This sets it before executing the changeset if the
stack policy is passed in.

Args:
stack_policy (:class:`stacker.providers.base.Template`): A template
object representing a stack policy.
"""
if stack_policy:
kwargs = generate_stack_policy_args(stack_policy)
kwargs["StackName"] = fqn
logger.debug("Setting stack policy on %s.", fqn)
self.cloudformation.set_stack_policy(**kwargs)

def interactive_update_stack(self, fqn, template, old_parameters,
parameters, tags, stack_policy=None,
parameters, stack_policy, tags,
**kwargs):
"""Update a Cloudformation stack in interactive mode.

Expand All @@ -840,6 +873,8 @@ def interactive_update_stack(self, fqn, template, old_parameters,
parameter list on the existing Cloudformation stack.
parameters (list): A list of dictionaries that defines the
parameter list to be applied to the Cloudformation stack.
stack_policy (:class:`stacker.providers.base.Template`): A template
object representing a stack policy.
tags (list): A list of dictionaries that defines the tags
that should be applied to the Cloudformation stack.
"""
Expand Down Expand Up @@ -878,19 +913,15 @@ def interactive_update_stack(self, fqn, template, old_parameters,
finally:
ui.unlock()

# ChangeSets don't support specifying a stack policy inline, like
# CreateStack/UpdateStack, so we just SetStackPolicy if there is one.
if stack_policy:
kwargs = generate_stack_policy_args(stack_policy)
kwargs["StackName"] = fqn
self.cloudformation.set_stack_policy(**kwargs)
self.deal_with_changeset_stack_policy(fqn, stack_policy)

self.cloudformation.execute_change_set(
ChangeSetName=change_set_id,
)

def noninteractive_changeset_update(self, fqn, template, old_parameters,
parameters, tags, **kwargs):
parameters, stack_policy, tags,
**kwargs):
"""Update a Cloudformation stack using a change set.

This is required for stacks with a defined Transform (i.e. SAM), as the
Expand All @@ -904,6 +935,8 @@ def noninteractive_changeset_update(self, fqn, template, old_parameters,
parameter list on the existing Cloudformation stack.
parameters (list): A list of dictionaries that defines the
parameter list to be applied to the Cloudformation stack.
stack_policy (:class:`stacker.providers.base.Template`): A template
object representing a stack policy.
tags (list): A list of dictionaries that defines the tags
that should be applied to the Cloudformation stack.
"""
Expand All @@ -914,6 +947,8 @@ def noninteractive_changeset_update(self, fqn, template, old_parameters,
'UPDATE', service_role=self.service_role, **kwargs
)

self.deal_with_changeset_stack_policy(fqn, stack_policy)
Copy link
Member

Choose a reason for hiding this comment

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

Ok cool, so this is the bug, you refactored the logic so you could call it here.


self.cloudformation.execute_change_set(
ChangeSetName=change_set_id,
)
Expand All @@ -932,6 +967,8 @@ def default_update_stack(self, fqn, template, old_parameters, parameters,
parameter list to be applied to the Cloudformation stack.
tags (list): A list of dictionaries that defines the tags
that should be applied to the Cloudformation stack.
stack_policy (:class:`stacker.providers.base.Template`): A template
object representing a stack policy.
"""

logger.debug("Using default provider mode for %s.", fqn)
Expand Down
100 changes: 99 additions & 1 deletion stacker/tests/providers/aws/test_default.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,64 @@ def test_prepare_stack_for_update_recreate(self):
self.assertFalse(
self.provider.prepare_stack_for_update(stack, []))

def test_noninteractive_changeset_update_no_stack_policy(self):
stack_name = "MockStack"

self.stubber.add_response(
"create_change_set",
{'Id': 'CHANGESETID', 'StackId': 'STACKID'}
)
changes = []
changes.append(generate_change())

self.stubber.add_response(
"describe_change_set",
generate_change_set_response(
status="CREATE_COMPLETE", execution_status="AVAILABLE",
changes=changes,
)
)

self.stubber.add_response("execute_change_set", {})

with self.stubber:
self.provider.noninteractive_changeset_update(
fqn=stack_name,
template=Template(url="http://fake.template.url.com/"),
old_parameters=[],
parameters=[], stack_policy=None, tags=[],
)

def test_noninteractive_changeset_update_with_stack_policy(self):
stack_name = "MockStack"

self.stubber.add_response(
"create_change_set",
{'Id': 'CHANGESETID', 'StackId': 'STACKID'}
)
changes = []
changes.append(generate_change())

self.stubber.add_response(
"describe_change_set",
generate_change_set_response(
status="CREATE_COMPLETE", execution_status="AVAILABLE",
changes=changes,
)
)

self.stubber.add_response("set_stack_policy", {})

self.stubber.add_response("execute_change_set", {})

with self.stubber:
self.provider.noninteractive_changeset_update(
fqn=stack_name,
template=Template(url="http://fake.template.url.com/"),
old_parameters=[],
parameters=[], stack_policy=Template(body="{}"), tags=[],
)


class TestProviderInteractiveMode(unittest.TestCase):
def setUp(self):
Expand All @@ -516,7 +574,8 @@ def test_successful_init(self):
self.assertEqual(p.replacements_only, replacements)

@patch("stacker.providers.aws.default.ask_for_approval")
def test_update_stack_execute_success(self, patched_approval):
def test_update_stack_execute_success_no_stack_policy(self,
patched_approval):
stack_name = "my-fake-stack"

self.stubber.add_response(
Expand Down Expand Up @@ -550,6 +609,45 @@ def test_update_stack_execute_success(self, patched_approval):

self.assertEqual(patched_approval.call_count, 1)

@patch("stacker.providers.aws.default.ask_for_approval")
def test_update_stack_execute_success_with_stack_policy(self,
patched_approval):
stack_name = "my-fake-stack"

self.stubber.add_response(
"create_change_set",
{'Id': 'CHANGESETID', 'StackId': 'STACKID'}
)
changes = []
changes.append(generate_change())

self.stubber.add_response(
"describe_change_set",
generate_change_set_response(
status="CREATE_COMPLETE", execution_status="AVAILABLE",
changes=changes,
)
)

self.stubber.add_response("set_stack_policy", {})

self.stubber.add_response("execute_change_set", {})

with self.stubber:
self.provider.update_stack(
fqn=stack_name,
template=Template(url="http://fake.template.url.com/"),
old_parameters=[],
parameters=[], tags=[],
stack_policy=Template(body="{}"),
)

patched_approval.assert_called_with(full_changeset=changes,
params_diff=[],
include_verbose=True)

self.assertEqual(patched_approval.call_count, 1)

def test_select_update_method(self):
for i in [[{'force_interactive': False,
'force_change_set': False},
Expand Down