Skip to content

Commit 8fba58b

Browse files
committed
Improve secrets check on dry-run
- Support SecretsManager values - Support SSM Parameters in different regions
1 parent 4c477cb commit 8fba58b

File tree

5 files changed

+281
-11
lines changed

5 files changed

+281
-11
lines changed

hako.gemspec

+2
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,14 @@ Gem::Specification.new do |spec|
3030
spec.add_dependency 'aws-sdk-elasticloadbalancing'
3131
spec.add_dependency 'aws-sdk-elasticloadbalancingv2', '>= 1.54.0'
3232
spec.add_dependency 'aws-sdk-s3'
33+
spec.add_dependency 'aws-sdk-secretsmanager'
3334
spec.add_dependency 'aws-sdk-servicediscovery'
3435
spec.add_dependency 'aws-sdk-sns'
3536
spec.add_dependency 'aws-sdk-ssm'
3637
spec.add_dependency 'jsonnet'
3738

3839
spec.add_development_dependency 'bundler'
40+
spec.add_development_dependency 'oga' # XML library for aws-sdk
3941
spec.add_development_dependency 'rake'
4042
spec.add_development_dependency 'rspec'
4143
spec.add_development_dependency 'rubocop', '>= 0.53.0'

lib/hako/schedulers/ecs.rb

+73-11
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
require 'aws-sdk-ec2'
55
require 'aws-sdk-ecs'
66
require 'aws-sdk-s3'
7+
require 'aws-sdk-secretsmanager'
78
require 'aws-sdk-sns'
89
require 'aws-sdk-ssm'
910
require 'hako'
@@ -400,9 +401,18 @@ def ec2_client
400401
@ec2_client ||= Aws::EC2::Client.new(region: @region)
401402
end
402403

404+
# @param [String] region
403405
# @return [Aws::SSM::Client]
404-
def ssm_client
405-
@ssm_client ||= Aws::SSM::Client.new(region: @region)
406+
def ssm_client(region)
407+
@ssm_clients ||= {}
408+
@ssm_clients[region] ||= Aws::SSM::Client.new(region: region)
409+
end
410+
411+
# @param [String] region
412+
# @return [Aws::SecretsManager::Client]
413+
def secretsmanager_client(region)
414+
@secretsmanager_clients ||= {}
415+
@secretsmanager_clients[region] ||= Aws::SecretsManager::Client.new(region: region)
406416
end
407417

408418
# @return [EcsElb, EcsElbV2]
@@ -1415,22 +1425,74 @@ def print_definition_in_cli_format(definition, additional_env: {}, overrides: ni
14151425
nil
14161426
end
14171427

1428+
SecretValueFrom = Struct.new(:arn, :secret_name, :json_key, :version_stage, :version_id)
1429+
14181430
# @param [Hash] container_definition
14191431
# @return [nil]
14201432
def check_secrets(container_definition)
1421-
parameter_names = (container_definition[:secrets] || []).map { |secret| secret.fetch(:value_from) }
1422-
invalid_parameter_names = parameter_names.each_slice(10).flat_map do |names|
1423-
names = names.map do |name|
1424-
if name.start_with?('arn:')
1425-
name.slice(%r{:parameter(/.+)\z}, 1)
1433+
parameter_arns = []
1434+
secret_value_arns = []
1435+
invalid_arns = []
1436+
1437+
(container_definition[:secrets] || []).each do |secret|
1438+
arn = Aws::ARNParser.parse(secret.fetch(:value_from))
1439+
case arn.service
1440+
when 'ssm'
1441+
parameter_arns << arn
1442+
when 'secretsmanager'
1443+
secret_value_arns << arn
1444+
else
1445+
invalid_arns << arn.to_s
1446+
end
1447+
end
1448+
1449+
parameter_arns.group_by(&:region).each do |region, region_arns|
1450+
region_arns.each_slice(10) do |arns|
1451+
invalid_arns += ssm_client(region).get_parameters(names: arns.map(&:to_s)).invalid_parameters
1452+
end
1453+
end
1454+
1455+
secret_value_arns.group_by(&:region).each do |region, region_arns|
1456+
secret_value_froms = region_arns.filter_map do |arn|
1457+
m = arn.resource.match(/\Asecret:(?<secret_name>[^:]+):(?<json_key>[^:]*):(?<version_stage>[^:]*):(?<version_id>[^:]*)\z/)
1458+
if m
1459+
f = SecretValueFrom.new(arn, m[:secret_name])
1460+
unless m[:json_key].empty?
1461+
f.json_key = m[:json_key]
1462+
end
1463+
unless m[:version_stage].empty?
1464+
f.version_stage = m[:version_stage]
1465+
end
1466+
unless m[:version_id].empty?
1467+
f.version_id = m[:version_id]
1468+
end
1469+
f
14261470
else
1427-
name
1471+
invalid_arns << arn.to_s
1472+
nil
1473+
end
1474+
end
1475+
secret_value_froms.group_by { |f| [f.secret_name, f.version_stage, f.version_id] }.each do |(secret_name, version_stage, version_id), fs|
1476+
secret_arn = Aws::ARN.new(**fs[0].arn.to_h.merge(resource: "secret:#{secret_name}")).to_s
1477+
begin
1478+
secret_string = secretsmanager_client(region).get_secret_value(secret_id: secret_arn, version_stage: version_stage, version_id: version_id).secret_string
1479+
secret_json = nil
1480+
fs.each do |f|
1481+
if f.json_key
1482+
secret_json ||= JSON.parse(secret_string)
1483+
unless secret_json.key?(f.json_key)
1484+
invalid_arns << f.arn.to_s
1485+
end
1486+
end
1487+
end
1488+
rescue Aws::SecretsManager::Errors::ResourceNotFoundException
1489+
invalid_arns += fs.map { |f| f.arn.to_s }
14281490
end
14291491
end
1430-
ssm_client.get_parameters(names: names).invalid_parameters
14311492
end
1432-
unless invalid_parameter_names.empty?
1433-
raise Error.new("Invalid parameters for secrets: #{invalid_parameter_names}")
1493+
1494+
unless invalid_arns.empty?
1495+
raise Error.new("Invalid ARNs for secrets: #{invalid_arns}")
14341496
end
14351497

14361498
nil
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
{
2+
scheduler: {
3+
type: 'ecs',
4+
region: 'ap-northeast-1',
5+
cluster: 'eagletmt',
6+
desired_count: 1,
7+
role: 'ECSServiceRole',
8+
},
9+
app: {
10+
image: 'busybox',
11+
cpu: 32,
12+
memory: 64,
13+
secrets: [
14+
{
15+
name: 'SECRET_MESSAGE1',
16+
value_from: 'arn:aws:ssm:ap-northeast-1:012345678901:parameter/hoge/fuga/SECRET_MESSAGE1',
17+
},
18+
{
19+
name: 'SECRET_MESSAGE2',
20+
value_from: 'arn:aws:ssm:ap-northeast-1:012345678901:parameter/hoge/fuga/SECRET_MESSAGE2',
21+
},
22+
{
23+
name: 'SECRET_MESSAGE3',
24+
value_from: 'arn:aws:ssm:us-east-1:012345678901:parameter/hoge/fuga/SECRET_MESSAGE3',
25+
},
26+
],
27+
},
28+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
{
2+
scheduler: {
3+
type: 'ecs',
4+
region: 'ap-northeast-1',
5+
cluster: 'eagletmt',
6+
desired_count: 1,
7+
role: 'ECSServiceRole',
8+
},
9+
app: {
10+
image: 'busybox',
11+
cpu: 32,
12+
memory: 64,
13+
secrets: [
14+
{
15+
name: 'SECRET_VALUE',
16+
value_from: 'arn:aws:secretsmanager:ap-northeast-1:012345678901:secret:hoge/fuga1-abcdef:::',
17+
},
18+
{
19+
name: 'SECRET_MESSAGE1',
20+
value_from: 'arn:aws:secretsmanager:ap-northeast-1:012345678901:secret:hoge/fuga2-abcdef:SECRET_MESSAGE1::',
21+
},
22+
{
23+
name: 'SECRET_MESSAGE2',
24+
value_from: 'arn:aws:secretsmanager:ap-northeast-1:012345678901:secret:hoge/fuga2-abcdef:SECRET_MESSAGE2::',
25+
},
26+
{
27+
name: 'SECRET_MESSAGE3',
28+
value_from: 'arn:aws:secretsmanager:us-east-1:012345678901:secret:hoge/fuga3-abcdef:SECRET_MESSAGE3::',
29+
},
30+
],
31+
},
32+
}

spec/hako/schedulers/ecs_spec.rb

+146
Original file line numberDiff line numberDiff line change
@@ -642,4 +642,150 @@
642642
end
643643
end
644644
end
645+
646+
describe '#check_secrets' do
647+
let(:dry_run) { true }
648+
649+
context 'with Parameter Store values' do
650+
let(:app) { Hako::Application.new(fixture_root.join('jsonnet', 'parameter_store.jsonnet')) }
651+
let(:ssm_client) { Aws::SSM::Client.new(stub_responses: true) }
652+
653+
before do
654+
allow(scheduler).to receive(:ssm_client).and_return(ssm_client)
655+
allow(scheduler).to receive(:puts) # Suppress dry-run output
656+
end
657+
658+
it 'checks Parameter Store values' do
659+
scheduler.deploy(containers)
660+
expect(ssm_client.api_requests.size).to eq(2)
661+
expect(ssm_client.api_requests).to match_array(
662+
[
663+
hash_including(
664+
operation_name: :get_parameters,
665+
params: {
666+
names: match_array(
667+
[
668+
'arn:aws:ssm:ap-northeast-1:012345678901:parameter/hoge/fuga/SECRET_MESSAGE1',
669+
'arn:aws:ssm:ap-northeast-1:012345678901:parameter/hoge/fuga/SECRET_MESSAGE2',
670+
],
671+
),
672+
},
673+
),
674+
hash_including(
675+
operation_name: :get_parameters,
676+
params: {
677+
names: ['arn:aws:ssm:us-east-1:012345678901:parameter/hoge/fuga/SECRET_MESSAGE3'],
678+
},
679+
),
680+
],
681+
)
682+
end
683+
684+
context 'with invalid parameters' do
685+
before do
686+
ssm_client.stub_responses(:get_parameters, lambda { |context|
687+
if context.params[:names].include?('arn:aws:ssm:ap-northeast-1:012345678901:parameter/hoge/fuga/SECRET_MESSAGE2')
688+
Aws::SSM::Types::GetParametersResult.new(
689+
invalid_parameters: ['arn:aws:ssm:ap-northeast-1:012345678901:parameter/hoge/fuga/SECRET_MESSAGE2'],
690+
)
691+
else
692+
Aws::SSM::Types::GetParametersResult.new(invalid_parameters: [])
693+
end
694+
})
695+
end
696+
697+
it 'raises an error' do
698+
expect { scheduler.deploy(containers) }.to raise_error(Hako::Error) { |e|
699+
expect(e.message).to_not include('SECRET_MESSAGE1')
700+
expect(e.message).to include('SECRET_MESSAGE2')
701+
expect(e.message).to_not include('SECRET_MESSAGE3')
702+
}
703+
end
704+
end
705+
end
706+
707+
context 'with SecretsManager values' do
708+
let(:app) { Hako::Application.new(fixture_root.join('jsonnet', 'secretsmanager.jsonnet')) }
709+
let(:secretsmanager_client) { Aws::SecretsManager::Client.new(stub_responses: true) }
710+
let(:secrets) do
711+
{
712+
'arn:aws:secretsmanager:ap-northeast-1:012345678901:secret:hoge/fuga1-abcdef' => 'SECRET_VALUE0',
713+
'arn:aws:secretsmanager:ap-northeast-1:012345678901:secret:hoge/fuga2-abcdef' => '{"SECRET_MESSAGE1":"SECRET_VALUE1","SECRET_MESSAGE2":"SECRET_VALUE2"}',
714+
'arn:aws:secretsmanager:us-east-1:012345678901:secret:hoge/fuga3-abcdef' => '{"SECRET_MESSAGE3":"SECRET_VALUE3"}',
715+
}
716+
end
717+
718+
before do
719+
allow(scheduler).to receive(:secretsmanager_client).and_return(secretsmanager_client)
720+
allow(scheduler).to receive(:puts) # Suppress dry-run output
721+
secretsmanager_client.stub_responses(:get_secret_value, lambda { |context|
722+
secret_string = secrets[context.params[:secret_id]]
723+
if secret_string
724+
Aws::SecretsManager::Types::GetSecretValueResponse.new(secret_string: secret_string)
725+
else
726+
'ResourceNotFoundException'
727+
end
728+
})
729+
end
730+
731+
it 'checks SecretsManager values' do
732+
scheduler.deploy(containers)
733+
expect(secretsmanager_client.api_requests.size).to eq(3)
734+
expect(secretsmanager_client.api_requests).to match_array(
735+
[
736+
hash_including(
737+
operation_name: :get_secret_value,
738+
params: { secret_id: 'arn:aws:secretsmanager:ap-northeast-1:012345678901:secret:hoge/fuga1-abcdef', version_stage: nil, version_id: nil, },
739+
),
740+
hash_including(
741+
operation_name: :get_secret_value,
742+
params: {
743+
secret_id: 'arn:aws:secretsmanager:ap-northeast-1:012345678901:secret:hoge/fuga2-abcdef',
744+
version_stage: nil,
745+
version_id: nil,
746+
},
747+
),
748+
hash_including(
749+
operation_name: :get_secret_value,
750+
params: {
751+
secret_id: 'arn:aws:secretsmanager:us-east-1:012345678901:secret:hoge/fuga3-abcdef',
752+
version_stage: nil,
753+
version_id: nil,
754+
},
755+
),
756+
],
757+
)
758+
end
759+
760+
context 'when json-key is missing' do
761+
before do
762+
secrets['arn:aws:secretsmanager:ap-northeast-1:012345678901:secret:hoge/fuga2-abcdef'] = '{"SECRET_MESSAGE1":"SECRET_VALUE1"}'
763+
end
764+
765+
it 'raises an error' do
766+
expect { scheduler.deploy(containers) }.to raise_error(Hako::Error) { |e|
767+
expect(e.message).to_not include('SECRET_VALUE')
768+
expect(e.message).to_not include('SECRET_MESSAGE1')
769+
expect(e.message).to include('SECRET_MESSAGE2')
770+
expect(e.message).to_not include('SECRET_MESSAGE3')
771+
}
772+
end
773+
end
774+
775+
context 'when secret value is missing' do
776+
before do
777+
secrets.delete('arn:aws:secretsmanager:ap-northeast-1:012345678901:secret:hoge/fuga2-abcdef')
778+
end
779+
780+
it 'raises an error' do
781+
expect { scheduler.deploy(containers) }.to raise_error(Hako::Error) { |e|
782+
expect(e.message).to_not include('SECRET_VALUE')
783+
expect(e.message).to include('SECRET_MESSAGE1')
784+
expect(e.message).to include('SECRET_MESSAGE2')
785+
expect(e.message).to_not include('SECRET_MESSAGE3')
786+
}
787+
end
788+
end
789+
end
790+
end
645791
end

0 commit comments

Comments
 (0)