Improve secrets check on dry-run

- Support SecretsManager values
- Support SSM Parameters in different regions

Author: eagletmt

hako.gemspec

@@ -30,6 +30,7 @@ Gem::Specification.new do |spec|
   spec.add_dependency 'aws-sdk-elasticloadbalancing'
   spec.add_dependency 'aws-sdk-elasticloadbalancingv2', '>= 1.54.0'
   spec.add_dependency 'aws-sdk-s3'
+  spec.add_dependency 'aws-sdk-secretsmanager'
   spec.add_dependency 'aws-sdk-servicediscovery'
   spec.add_dependency 'aws-sdk-sns'
   spec.add_dependency 'aws-sdk-ssm'

lib/hako/schedulers/ecs.rb

@@ -4,6 +4,7 @@
 require 'aws-sdk-ec2'
 require 'aws-sdk-ecs'
 require 'aws-sdk-s3'
+require 'aws-sdk-secretsmanager'
 require 'aws-sdk-sns'
 require 'aws-sdk-ssm'
 require 'hako'
@@ -400,9 +401,18 @@ def ec2_client
         @ec2_client ||= Aws::EC2::Client.new(region: @region)
       end
 
+      # @param [String] region
       # @return [Aws::SSM::Client]
-      def ssm_client
-        @ssm_client ||= Aws::SSM::Client.new(region: @region)
+      def ssm_client(region)
+        @ssm_clients ||= {}
+        @ssm_clients[region] ||= Aws::SSM::Client.new(region: region)
+      end
+
+      # @param [String] region
+      # @return [Aws::SecretsManager::Client]
+      def secretsmanager_client(region)
+        @secretsmanager_clients ||= {}
+        @secretsmanager_clients[region] ||= Aws::SecretsManager::Client.new(region: region)
       end
 
       # @return [EcsElb, EcsElbV2]
@@ -1415,22 +1425,74 @@ def print_definition_in_cli_format(definition, additional_env: {}, overrides: ni
         nil
       end
 
+      SecretValueFrom = Struct.new(:arn, :secret_name, :json_key, :version_stage, :version_id)
+
       # @param [Hash] container_definition
       # @return [nil]
       def check_secrets(container_definition)
-        parameter_names = (container_definition[:secrets] || []).map { |secret| secret.fetch(:value_from) }
-        invalid_parameter_names = parameter_names.each_slice(10).flat_map do |names|
-          names = names.map do |name|
-            if name.start_with?('arn:')
-              name.slice(%r{:parameter(/.+)\z}, 1)
+        parameter_arns = []
+        secret_value_arns = []
+        invalid_arns = []
+
+        (container_definition[:secrets] || []).each do |secret|
+          arn = Aws::ARNParser.parse(secret.fetch(:value_from))
+          case arn.service
+          when 'ssm'
+            parameter_arns << arn
+          when 'secretsmanager'
+            secret_value_arns << arn
+          else
+            invalid_arns << arn.to_s
+          end
+        end
+
+        parameter_arns.group_by(&:region).each do |region, region_arns|
+          region_arns.each_slice(10) do |arns|
+            invalid_arns += ssm_client(region).get_parameters(names: arns.map(&:to_s)).invalid_parameters
+          end
+        end
+
+        secret_value_arns.group_by(&:region).each do |region, region_arns|
+          secret_value_froms = region_arns.filter_map do |arn|
+            m = arn.resource.match(/\Asecret:(?<secret_name>[^:]+):(?<json_key>[^:]*):(?<version_stage>[^:]*):(?<version_id>[^:]*)\z/)
+            if m
+              f = SecretValueFrom.new(arn, m[:secret_name])
+              unless m[:json_key].empty?
+                f.json_key = m[:json_key]
+              end
+              unless m[:version_stage].empty?
+                f.version_stage = m[:version_stage]
+              end
+              unless m[:version_id].empty?
+                f.version_id = m[:version_id]
+              end
+              f
             else
-              name
+              invalid_arns << arn.to_s
+              nil
+            end
+          end
+          secret_value_froms.group_by { |f| [f.secret_name, f.version_stage, f.version_id] }.each do |(secret_name, version_stage, version_id), fs|
+            secret_arn = Aws::ARN.new(**fs[0].arn.to_h.merge(resource: "secret:#{secret_name}")).to_s
+            begin
+              secret_string = secretsmanager_client(region).get_secret_value(secret_id: secret_arn, version_stage: version_stage, version_id: version_id).secret_string
+              secret_json = nil
+              fs.each do |f|
+                if f.json_key
+                  secret_json ||= JSON.parse(secret_string)
+                  unless secret_json.key?(f.json_key)
+                    invalid_arns << f.arn.to_s
+                  end
+                end
+              end
+            rescue Aws::SecretsManager::Errors::ResourceNotFoundException
+              invalid_arns += fs.map { |f| f.arn.to_s }
             end
           end
-          ssm_client.get_parameters(names: names).invalid_parameters
         end
-        unless invalid_parameter_names.empty?
-          raise Error.new("Invalid parameters for secrets: #{invalid_parameter_names}")
+
+        unless invalid_arns.empty?
+          raise Error.new("Invalid ARNs for secrets: #{invalid_arns}")
         end
 
         nil

spec/fixtures/jsonnet/parameter_store.jsonnet

@@ -0,0 +1,28 @@
+{
+  scheduler: {
+    type: 'ecs',
+    region: 'ap-northeast-1',
+    cluster: 'eagletmt',
+    desired_count: 1,
+    role: 'ECSServiceRole',
+  },
+  app: {
+    image: 'busybox',
+    cpu: 32,
+    memory: 64,
+    secrets: [
+      {
+        name: 'SECRET_MESSAGE1',
+        value_from: 'arn:aws:ssm:ap-northeast-1:012345678901:parameter/hoge/fuga/SECRET_MESSAGE1',
+      },
+      {
+        name: 'SECRET_MESSAGE2',
+        value_from: 'arn:aws:ssm:ap-northeast-1:012345678901:parameter/hoge/fuga/SECRET_MESSAGE2',
+      },
+      {
+        name: 'SECRET_MESSAGE3',
+        value_from: 'arn:aws:ssm:us-east-1:012345678901:parameter/hoge/fuga/SECRET_MESSAGE3',
+      },
+    ],
+  },
+}

spec/fixtures/jsonnet/secretsmanager.jsonnet

@@ -0,0 +1,32 @@
+{
+  scheduler: {
+    type: 'ecs',
+    region: 'ap-northeast-1',
+    cluster: 'eagletmt',
+    desired_count: 1,
+    role: 'ECSServiceRole',
+  },
+  app: {
+    image: 'busybox',
+    cpu: 32,
+    memory: 64,
+    secrets: [
+      {
+        name: 'SECRET_VALUE',
+        value_from: 'arn:aws:secretsmanager:ap-northeast-1:012345678901:secret:hoge/fuga1-abcdef:::',
+      },
+      {
+        name: 'SECRET_MESSAGE1',
+        value_from: 'arn:aws:secretsmanager:ap-northeast-1:012345678901:secret:hoge/fuga2-abcdef:SECRET_MESSAGE1::',
+      },
+      {
+        name: 'SECRET_MESSAGE2',
+        value_from: 'arn:aws:secretsmanager:ap-northeast-1:012345678901:secret:hoge/fuga2-abcdef:SECRET_MESSAGE2::',
+      },
+      {
+        name: 'SECRET_MESSAGE3',
+        value_from: 'arn:aws:secretsmanager:us-east-1:012345678901:secret:hoge/fuga3-abcdef:SECRET_MESSAGE3::',
+      },
+    ],
+  },
+}

spec/hako/schedulers/ecs_spec.rb

@@ -642,4 +642,150 @@
       end
     end
   end
+
+  describe '#check_secrets' do
+    let(:dry_run) { true }
+
+    context 'with Parameter Store values' do
+      let(:app) { Hako::Application.new(fixture_root.join('jsonnet', 'parameter_store.jsonnet')) }
+      let(:ssm_client) { Aws::SSM::Client.new(stub_responses: true) }
+
+      before do
+        allow(scheduler).to receive(:ssm_client).and_return(ssm_client)
+        allow(scheduler).to receive(:puts) # Suppress dry-run output
+      end
+
+      it 'checks Parameter Store values' do
+        scheduler.deploy(containers)
+        expect(ssm_client.api_requests.size).to eq(2)
+        expect(ssm_client.api_requests).to match_array(
+          [
+            hash_including(
+              operation_name: :get_parameters,
+              params: {
+                names: match_array(
+                  [
+                    'arn:aws:ssm:ap-northeast-1:012345678901:parameter/hoge/fuga/SECRET_MESSAGE1',
+                    'arn:aws:ssm:ap-northeast-1:012345678901:parameter/hoge/fuga/SECRET_MESSAGE2',
+                  ],
+                ),
+              },
+            ),
+            hash_including(
+              operation_name: :get_parameters,
+              params: {
+                names: ['arn:aws:ssm:us-east-1:012345678901:parameter/hoge/fuga/SECRET_MESSAGE3'],
+              },
+            ),
+          ],
+        )
+      end
+
+      context 'with invalid parameters' do
+        before do
+          ssm_client.stub_responses(:get_parameters, lambda { |context|
+            if context.params[:names].include?('arn:aws:ssm:ap-northeast-1:012345678901:parameter/hoge/fuga/SECRET_MESSAGE2')
+              Aws::SSM::Types::GetParametersResult.new(
+                invalid_parameters: ['arn:aws:ssm:ap-northeast-1:012345678901:parameter/hoge/fuga/SECRET_MESSAGE2'],
+              )
+            else
+              Aws::SSM::Types::GetParametersResult.new(invalid_parameters: [])
+            end
+          })
+        end
+
+        it 'raises an error' do
+          expect { scheduler.deploy(containers) }.to raise_error(Hako::Error) { |e|
+            expect(e.message).to_not include('SECRET_MESSAGE1')
+            expect(e.message).to include('SECRET_MESSAGE2')
+            expect(e.message).to_not include('SECRET_MESSAGE3')
+          }
+        end
+      end
+    end
+
+    context 'with SecretsManager values' do
+      let(:app) { Hako::Application.new(fixture_root.join('jsonnet', 'secretsmanager.jsonnet')) }
+      let(:secretsmanager_client) { Aws::SecretsManager::Client.new(stub_responses: true) }
+      let(:secrets) do
+        {
+          'arn:aws:secretsmanager:ap-northeast-1:012345678901:secret:hoge/fuga1-abcdef' => 'SECRET_VALUE0',
+          'arn:aws:secretsmanager:ap-northeast-1:012345678901:secret:hoge/fuga2-abcdef' => '{"SECRET_MESSAGE1":"SECRET_VALUE1","SECRET_MESSAGE2":"SECRET_VALUE2"}',
+          'arn:aws:secretsmanager:us-east-1:012345678901:secret:hoge/fuga3-abcdef' => '{"SECRET_MESSAGE3":"SECRET_VALUE3"}',
+        }
+      end
+
+      before do
+        allow(scheduler).to receive(:secretsmanager_client).and_return(secretsmanager_client)
+        allow(scheduler).to receive(:puts) # Suppress dry-run output
+        secretsmanager_client.stub_responses(:get_secret_value, lambda { |context|
+          secret_string = secrets[context.params[:secret_id]]
+          if secret_string
+            Aws::SecretsManager::Types::GetSecretValueResponse.new(secret_string: secret_string)
+          else
+            'ResourceNotFoundException'
+          end
+        })
+      end
+
+      it 'checks SecretsManager values' do
+        scheduler.deploy(containers)
+        expect(secretsmanager_client.api_requests.size).to eq(3)
+        expect(secretsmanager_client.api_requests).to match_array(
+          [
+            hash_including(
+              operation_name: :get_secret_value,
+              params: { secret_id: 'arn:aws:secretsmanager:ap-northeast-1:012345678901:secret:hoge/fuga1-abcdef', version_stage: nil, version_id: nil, },
+            ),
+            hash_including(
+              operation_name: :get_secret_value,
+              params: {
+                secret_id: 'arn:aws:secretsmanager:ap-northeast-1:012345678901:secret:hoge/fuga2-abcdef',
+                version_stage: nil,
+                version_id: nil,
+              },
+            ),
+            hash_including(
+              operation_name: :get_secret_value,
+              params: {
+                secret_id: 'arn:aws:secretsmanager:us-east-1:012345678901:secret:hoge/fuga3-abcdef',
+                version_stage: nil,
+                version_id: nil,
+              },
+            ),
+          ],
+        )
+      end
+
+      context 'when json-key is missing' do
+        before do
+          secrets['arn:aws:secretsmanager:ap-northeast-1:012345678901:secret:hoge/fuga2-abcdef'] = '{"SECRET_MESSAGE1":"SECRET_VALUE1"}'
+        end
+
+        it 'raises an error' do
+          expect { scheduler.deploy(containers) }.to raise_error(Hako::Error) { |e|
+            expect(e.message).to_not include('SECRET_VALUE')
+            expect(e.message).to_not include('SECRET_MESSAGE1')
+            expect(e.message).to include('SECRET_MESSAGE2')
+            expect(e.message).to_not include('SECRET_MESSAGE3')
+          }
+        end
+      end
+
+      context 'when secret value is missing' do
+        before do
+          secrets.delete('arn:aws:secretsmanager:ap-northeast-1:012345678901:secret:hoge/fuga2-abcdef')
+        end
+
+        it 'raises an error' do
+          expect { scheduler.deploy(containers) }.to raise_error(Hako::Error) { |e|
+            expect(e.message).to_not include('SECRET_VALUE')
+            expect(e.message).to include('SECRET_MESSAGE1')
+            expect(e.message).to include('SECRET_MESSAGE2')
+            expect(e.message).to_not include('SECRET_MESSAGE3')
+          }
+        end
+      end
+    end
+  end
 end