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.

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

Add ability to restrict which accounts can work with stacks #283

Merged
merged 14 commits into from
Jul 2, 2019
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
42 changes: 42 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,14 @@ stacks:
staging:
myapp-vpc:
template: myapp_vpc.rb
allowed_accounts: '123456789'
tags:
purpose: front-end
myapp-db:
template: myapp_db.rb
allowed_accounts:
- '1234567890'
- '9876543210'
tags:
purpose: back-end
myapp-web:
Expand Down Expand Up @@ -545,6 +549,44 @@ end

Note though that if a dynamic with the same name exists in your `templates/dynamics/` directory it will get loaded since it has higher precedence.

## Allowed accounts

The AWS account the command is executing in can be restricted to a specific list of allowed accounts. This is useful in reducing the possibility of applying non-production changes in a production account. Each stack definition can specify the `allowed_accounts` property with an array of AWS account IDs the stack is allowed to work with.

This is an opt-in feature which is enabled by specifying at least one account to allow.

Unlike other stack defaults, the `allowed_accounts` property values specified in the stack definition override values specified in the stack defaults (i.e., other stack property values are merged together with those specified in the stack defaults). This allows specifying allowed accounts in the stack defaults (inherited by all stacks) and override them for specific stacks. See below example config for an example.

```yaml
stack_defaults:
allowed_accounts: '555555555'
stacks:
us-east-1:
myapp-vpc: # only allow account 555555555 (inherited from the stack defaults)
template: myapp_vpc.rb
tags:
purpose: front-end
myapp-db:
template: myapp_db.rb
allowed_accounts: # only allow these accounts (overrides the stack defaults)
- '1234567890'
- '9876543210'
tags:
purpose: back-end
myapp-web:
template: myapp_web.rb
allowed_accounts: [] # allow all accounts (overrides the stack defaults)
tags:
purpose: front-end
myapp-redis:
template: myapp_redis.rb
allowed_accounts: '888888888' # only allow this account (overrides the stack defaults)
tags:
purpose: back-end
```

In the cases where you want to bypass the account check, there is StackMaster flag `--skip-account-check` that can be used.

## Commands

```bash
Expand Down
55 changes: 55 additions & 0 deletions features/apply_with_allowed_accounts.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
Feature: Apply command with allowed accounts

Background:
Given a file named "stack_master.yml" with:
"""
stack_defaults:
allowed_accounts:
- '11111111'
stacks:
us_east_1:
myapp_vpc:
template: myapp.rb
myapp_db:
template: myapp.rb
allowed_accounts: '22222222'
myapp_web:
template: myapp.rb
allowed_accounts: []
"""
And a directory named "templates"
And a file named "templates/myapp.rb" with:
"""
SparkleFormation.new(:myapp) do
description "Test template"
set!('AWSTemplateFormatVersion', '2010-09-09')
end
"""

Scenario: Run apply with stack inheriting allowed accounts from stack defaults
Given I stub the following stack events:
| stack_id | event_id | stack_name | logical_resource_id | resource_status | resource_type | timestamp |
| 1 | 1 | myapp-vpc | myapp-vpc | CREATE_COMPLETE | AWS::CloudFormation::Stack | 2020-10-29 00:00:00 |
When I use the account "11111111"
And I run `stack_master apply us-east-1 myapp-vpc`
And the output should match /2020-10-29 00:00:00 (\+|\-)[0-9]{4} myapp-vpc AWS::CloudFormation::Stack CREATE_COMPLETE/
Then the exit status should be 0

Scenario: Run apply with stack overriding allowed accounts with its own list
Given I stub the following stack events:
| stack_id | event_id | stack_name | logical_resource_id | resource_status | resource_type | timestamp |
| 1 | 1 | myapp-db | myapp-db | CREATE_COMPLETE | AWS::CloudFormation::Stack | 2020-10-29 00:00:00 |
When I use the account "11111111"
And I run `stack_master apply us-east-1 myapp-db`
And the output should contain all of these lines:
| Account '11111111' is not an allowed account. Allowed accounts are ["22222222"].|
Then the exit status should be 0

Scenario: Run apply with stack overriding allowed accounts to allow all accounts
Given I stub the following stack events:
| stack_id | event_id | stack_name | logical_resource_id | resource_status | resource_type | timestamp |
| 1 | 1 | myapp-web | myapp-web | CREATE_COMPLETE | AWS::CloudFormation::Stack | 2020-10-29 00:00:00 |
When I use the account "33333333"
And I run `stack_master apply us-east-1 myapp-web`
And the output should match /2020-10-29 00:00:00 (\+|\-)[0-9]{4} myapp-web AWS::CloudFormation::Stack CREATE_COMPLETE/
Then the exit status should be 0
11 changes: 11 additions & 0 deletions features/step_definitions/identity_steps.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
Given(/^I use the account "([^"]*)"$/) do |account_id|
Aws.config[:sts] = {
stub_responses: {
get_caller_identity: {
account: account_id,
arn: 'an-arn',
user_id: 'a-user-id'
}
}
}
end
12 changes: 12 additions & 0 deletions lib/stack_master.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ module StackMaster
autoload :PagedResponseAccumulator, 'stack_master/paged_response_accumulator'
autoload :StackDefinition, 'stack_master/stack_definition'
autoload :TemplateCompiler, 'stack_master/template_compiler'
autoload :Identity, 'stack_master/identity'

autoload :StackDiffer, 'stack_master/stack_differ'
autoload :Validator, 'stack_master/validator'
Expand Down Expand Up @@ -97,6 +98,7 @@ module StackEvents
NON_INTERACTIVE_DEFAULT = false
DEBUG_DEFAULT = false
QUIET_DEFAULT = false
SKIP_ACCOUNT_CHECK_DEFAULT = false

def interactive?
!non_interactive?
Expand Down Expand Up @@ -136,6 +138,16 @@ def quiet?

def reset_flags
@quiet = QUIET_DEFAULT
@skip_account_check = SKIP_ACCOUNT_CHECK_DEFAULT
end

def skip_account_check!
@skip_account_check = true
end
@skip_account_check = SKIP_ACCOUNT_CHECK_DEFAULT

def skip_account_check?
@skip_account_check
end

attr_accessor :non_interactive_answer
Expand Down
39 changes: 33 additions & 6 deletions lib/stack_master/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ def execute!
global_option '-q', '--quiet', 'Do not output the resulting Stack Events, just return immediately' do
StackMaster.quiet!
end
global_option '--skip-account-check', 'Do not check if command is allowed to execute in account' do
StackMaster.skip_account_check!
end

command :apply do |c|
c.syntax = 'stack_master apply [region_or_alias] [stack_name]'
Expand Down Expand Up @@ -178,18 +181,22 @@ def execute!
return
end

stack_name = Utils.underscore_to_hyphen(args[1])
allowed_accounts = []

# Because delete can work without a stack_master.yml
if options.config and File.file?(options.config)
config = load_config(options.config)
region = Utils.underscore_to_hyphen(config.unalias_region(args[0]))
allowed_accounts = config.find_stack(region, stack_name)&.allowed_accounts
else
region = args[0]
end

stack_name = Utils.underscore_to_hyphen(args[1])

StackMaster.cloud_formation_driver.set_region(region)
StackMaster::Commands::Delete.perform(region, stack_name)
execute_if_allowed_account(allowed_accounts) do
StackMaster.cloud_formation_driver.set_region(region)
StackMaster::Commands::Delete.perform(region, stack_name)
end
end
end

Expand Down Expand Up @@ -223,15 +230,35 @@ def execute_stacks_command(command, args, options)
success = false
end
stack_definitions = stack_definitions.select do |stack_definition|
StackStatus.new(config, stack_definition).changed?
running_in_allowed_account?(stack_definition.allowed_accounts) && StackStatus.new(config, stack_definition).changed?
end if options.changed
stack_definitions.each do |stack_definition|
StackMaster.cloud_formation_driver.set_region(stack_definition.region)
StackMaster.stdout.puts "Executing #{command.command_name} on #{stack_definition.stack_name} in #{stack_definition.region}"
success = false unless command.perform(config, stack_definition, options).success?
success = execute_if_allowed_account(stack_definition.allowed_accounts) do
command.perform(config, stack_definition, options).success?
end
end
end
success
end

def execute_if_allowed_account(allowed_accounts, &block)
raise ArgumentError, "Block required to execute this method" unless block_given?
if running_in_allowed_account?(allowed_accounts)
block.call
else
StackMaster.stdout.puts "Account '#{identity.account}' is not an allowed account. Allowed accounts are #{allowed_accounts}."
false
end
end

def running_in_allowed_account?(allowed_accounts)
StackMaster.skip_account_check? || identity.running_in_allowed_account?(allowed_accounts)
end

def identity
@identity ||= StackMaster::Identity.new
end
end
end
13 changes: 11 additions & 2 deletions lib/stack_master/commands/status.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@ def perform
progress if @show_progress
status = @config.stacks.map do |stack_definition|
stack_status = StackStatus.new(@config, stack_definition)
allowed_accounts = stack_definition.allowed_accounts
progress.increment if @show_progress
{
region: stack_definition.region,
stack_name: stack_definition.stack_name,
stack_status: stack_status.status,
different: stack_status.changed_message,
stack_status: running_in_allowed_account?(allowed_accounts) ? stack_status.status : "Disallowed account",
different: running_in_allowed_account?(allowed_accounts) ? stack_status.changed_message : "N/A",
}
end
tp.set :max_width, self.window_size
Expand All @@ -41,6 +42,14 @@ def progress
def sort_params(hash)
hash.sort.to_h
end

def running_in_allowed_account?(allowed_accounts)
StackMaster.skip_account_check? || identity.running_in_allowed_account?(allowed_accounts)
end

def identity
@identity ||= StackMaster::Identity.new
end
end
end
end
1 change: 1 addition & 0 deletions lib/stack_master/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ def load_stacks(stacks)
'base_dir' => @base_dir,
'template_dir' => @template_dir,
'additional_parameter_lookup_dirs' => @region_to_aliases[region])
stack_attributes['allowed_accounts'] = attributes['allowed_accounts'] if attributes['allowed_accounts']
@stacks << StackDefinition.new(stack_attributes)
end
end
Expand Down
23 changes: 23 additions & 0 deletions lib/stack_master/identity.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
module StackMaster
class Identity
def running_in_allowed_account?(allowed_accounts)

Choose a reason for hiding this comment

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

Seems odd to pass a nil or empty variable to a function.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe if nil was the default value for allowed_accounts (in StackDefinition, indicating unspecified) the expected values would either be nil or an array of account IDs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. Done in b30dede.

allowed_accounts.nil? || allowed_accounts.empty? || allowed_accounts.include?(account)
end

def account
@account ||= sts.get_caller_identity.account
end

private

attr_reader :sts

def region
@region ||= ENV['AWS_REGION'] || Aws.config[:region] || Aws.shared_config.region || 'us-east-1'
end
Copy link
Contributor

Choose a reason for hiding this comment

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

This method isn't necessary. If we don't specify a region the SDK does a lookup for us automatically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right it does, however I added this method to ensure that a default region is used if it couldn't find a region from the environment. It's also the same as the CloudFormation driver class except for the default value.

This was me being more defensive, but happy to remove it if you think it'll be fine without it?


def sts
@sts ||= Aws::STS::Client.new(region: region)
end
end
end
4 changes: 4 additions & 0 deletions lib/stack_master/stack_definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ class StackDefinition
:template,
:tags,
:role_arn,
:allowed_accounts,
:notification_arns,
:base_dir,
:template_dir,
Expand All @@ -23,8 +24,10 @@ def initialize(attributes = {})
@notification_arns = []
@s3 = {}
@files = []
@allowed_accounts = nil
super
@template_dir ||= File.join(@base_dir, 'templates')
@allowed_accounts = Array(@allowed_accounts)
end

def ==(other)
Expand All @@ -34,6 +37,7 @@ def ==(other)
@template == other.template &&
@tags == other.tags &&
@role_arn == other.role_arn &&
@allowed_accounts == other.allowed_accounts &&
@notification_arns == other.notification_arns &&
@base_dir == other.base_dir &&
@secret_file == other.secret_file &&
Expand Down
6 changes: 6 additions & 0 deletions spec/fixtures/stack_master.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ region_aliases:
production: us-east-1
staging: ap-southeast-2
stack_defaults:
allowed_accounts:
- '555555555'
tags:
application: my-awesome-blog
s3:
Expand Down Expand Up @@ -35,6 +37,7 @@ stacks:
role_arn: test_service_role_arn2
myapp_web:
template: myapp_web.rb
allowed_accounts: '1234567890'
myapp_vpc_with_secrets:
template: myapp_vpc.json
ap-southeast-2:
Expand All @@ -45,5 +48,8 @@ stacks:
role_arn: test_service_role_arn4
myapp_web:
template: myapp_web
allowed_accounts:
- '1234567890'
- '9876543210'
tags:
test_override: 2
Loading