Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions lib/stack_master/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ def execute!
c.description = "Creates or updates a stack. Shows a diff of the proposed stack's template and parameters. Tails stack events until CloudFormation has completed."
c.example 'update a stack named myapp-vpc in us-east-1', 'stack_master apply us-east-1 myapp-vpc'
c.option '--on-failure ACTION', String, "Action to take on CREATE_FAILURE. Valid Values: [ DO_NOTHING | ROLLBACK | DELETE ]. Default: ROLLBACK\nNote: You cannot use this option with Serverless Application Model (SAM) templates."
c.option '--yes-param PARAM_NAME', String, "Auto-approve stack updates when only parameter PARAM_NAME changes"
c.action do |args, options|
options.defaults config: default_config_file
execute_stacks_command(StackMaster::Commands::Apply, args, options)
Expand Down
17 changes: 15 additions & 2 deletions lib/stack_master/commands/apply.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ def initialize(config, stack_definition, options = Commander::Command::Options.n
@from_time = Time.now
@options = options
@options.on_failure ||= nil
@options.yes_param ||= nil
end

def perform
Expand Down Expand Up @@ -59,7 +60,11 @@ def use_s3?

def diff_stacks
abort_if_review_in_progress
StackDiffer.new(proposed_stack, stack).output_diff
differ.output_diff
end

def differ
@differ ||= StackDiffer.new(proposed_stack, stack)
end

def create_or_update_stack
Expand Down Expand Up @@ -125,12 +130,20 @@ def update_stack
halt!(@change_set.status_reason)
end

if differ.single_param_update?(@options.yes_param)
StackMaster.stdout.puts("Auto-approving update to single parameter #{@options.yes_param}")
else
ask_update_confirmation!
end
@change_set.display(StackMaster.stdout)
execute_change_set
end

def ask_update_confirmation!
unless ask?("Apply change set (y/n)? ")
ChangeSet.delete(@change_set.id)
halt! "Stack update aborted"
end
execute_change_set
end

def upload_files
Expand Down
9 changes: 9 additions & 0 deletions lib/stack_master/stack_differ.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require "diffy"
require "hashdiff"

module StackMaster
class StackDiffer
Expand Down Expand Up @@ -72,6 +73,14 @@ def noecho_keys
end
end

def single_param_update?(param_name)
return false if param_name.blank? || @current_stack.blank? || body_different?
differences = HashDiff.diff(@current_stack.parameters_with_defaults, @proposed_stack.parameters_with_defaults)
return false if differences.count != 1
diff = differences[0]
diff[0] == "~" && diff[1] == param_name
end

private

def display_diff(thing, diff)
Expand Down
18 changes: 17 additions & 1 deletion spec/stack_master/commands/apply_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
let(:proposed_stack) { StackMaster::Stack.new(template_body: template_body, template_format: template_format, tags: { 'environment' => 'production' } , parameters: parameters, role_arn: role_arn, notification_arns: [notification_arn], stack_policy_body: stack_policy_body ) }
let(:stack_policy_body) { '{}' }
let(:change_set) { double(display: true, failed?: false, id: '1') }
let(:differ) { instance_double(StackMaster::StackDiffer, output_diff: nil, single_param_update?: false) }

before do
allow(StackMaster::Stack).to receive(:find).with(region, stack_name).and_return(stack)
Expand All @@ -21,7 +22,7 @@
allow(Aws::CloudFormation::Client).to receive(:new).and_return(cf)
allow(Aws::S3::Client).to receive(:new).and_return(s3)
allow(cf).to receive(:create_stack)
allow(StackMaster::StackDiffer).to receive(:new).with(proposed_stack, stack).and_return double.as_null_object
allow(StackMaster::StackDiffer).to receive(:new).with(proposed_stack, stack).and_return(differ)
allow(StackMaster::StackEvents::Streamer).to receive(:stream)
allow(StackMaster).to receive(:interactive?).and_return(false)
allow(cf).to receive(:create_change_set).and_return(OpenStruct.new(id: '1'))
Expand Down Expand Up @@ -135,6 +136,21 @@ def apply
expect(StackMaster::ChangeSet).to_not have_received(:execute).with(change_set.id)
end
end

context 'yes_param option is set' do
let(:yes_param) { 'YesParam' }
let(:options) { double(yes_param: yes_param).as_null_object }

before do
allow(StackMaster).to receive(:non_interactive_answer).and_return('n')
allow(differ).to receive(:single_param_update?).with(yes_param).and_return(true)
end

it "skips asking for confirmation on single param updates" do
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

maybe also verify that it does not skip when there are multiple param updates?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That is tested in the unit test for Differ#single_param_update? below, along with various other input combinations.

Here I'm just stubbing Differ#single_param_update?, so that I don't have to retest all those combinations here.

It is stubbed to false by default in all the other tests of stack update above this one, so the scenarios where it returns false are covered there. (it can be multiple param updates, but also other combinations like no --yes-param set, removal of the yes param, etc.)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@ejoubaud 👍

expect(StackMaster::ChangeSet).to receive(:execute).with(change_set.id, stack_name)
StackMaster::Commands::Apply.perform(config, stack_definition, options)
end
end
end

context 'the stack does not exist' do
Expand Down
52 changes: 50 additions & 2 deletions spec/stack_master/stack_differ_spec.rb
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
RSpec.describe StackMaster::StackDiffer do
subject(:differ) { described_class.new(proposed_stack, stack) }
let(:current_body) { '{}' }
let(:proposed_body) { "{\"a\": 1}" }
let(:current_params) { Hash.new }
let(:proposed_params) { { 'param1' => 'hello'} }
let(:stack) { StackMaster::Stack.new(stack_name: stack_name,
region: region,
stack_id: 123,
template_body: '{}',
template_body: current_body,
template_format: :json,
parameters: current_params) }
let(:proposed_stack) { StackMaster::Stack.new(stack_name: stack_name,
region: region,
parameters: proposed_params,
template_body: "{\"a\": 1}",
template_body: proposed_body,
template_format: :json) }
let(:stack_name) { 'myapp-vpc' }
let(:region) { 'us-east-1' }
Expand Down Expand Up @@ -43,4 +45,50 @@
end
end
end

describe "#single_param_update?" do
let(:yes_param) { 'YesParam' }
let(:old_value) { 'old' }
let(:new_value) { 'new' }
let(:current_params) { { yes_param => old_value } }
let(:proposed_params) { { yes_param => new_value } }
let(:current_body) { proposed_body }

subject(:result) { differ.single_param_update?(yes_param) }

context "when only param changes" do
it { is_expected.to be_truthy }
end

context "when new stack" do
let(:stack) { nil }
it { is_expected.to be_falsey }
end

context "when no changes" do
let(:current_params) { proposed_params }
it { is_expected.to be_falsey }
end

context "when body changes" do
let(:current_body) { '{}' }
it { is_expected.to be_falsey }
end

context "on param removal" do
let(:proposed_params) { {} }
it { is_expected.to be_falsey }
end

context "on param first addition" do
let(:current_params) { {} }
it { is_expected.to be_falsey }
end

context "when another param also changes" do
let(:current_params) { { yes_param => old_value, 'other' => 'old' } }
let(:proposed_params) { { yes_param => new_value, 'other' => 'new' } }
it { is_expected.to be_falsey }
end
end
end
1 change: 1 addition & 0 deletions stack_master.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ Gem::Specification.new do |spec|
spec.add_dependency "deep_merge"
spec.add_dependency "cfndsl"
spec.add_dependency "multi_json"
spec.add_dependency "hashdiff"
spec.add_dependency "dotgpg" unless windows_build
spec.add_dependency "diff-lcs" if windows_build
end