From 55983c64313af9487be0d55501e2e94b9609e4d1 Mon Sep 17 00:00:00 2001 From: Nick Hammond Date: Thu, 12 Dec 2024 04:10:48 -0700 Subject: [PATCH 1/6] AWS secrets manager value can be a string --- .../secrets/adapters/aws_secrets_manager.rb | 2 + .../aws_secrets_manager_adapter_test.rb | 42 +++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/lib/kamal/secrets/adapters/aws_secrets_manager.rb b/lib/kamal/secrets/adapters/aws_secrets_manager.rb index e23ea1f1e..5db8fd7c9 100644 --- a/lib/kamal/secrets/adapters/aws_secrets_manager.rb +++ b/lib/kamal/secrets/adapters/aws_secrets_manager.rb @@ -13,6 +13,8 @@ def fetch_secrets(secrets, account:, session:) secret_string.each do |key, value| results["#{secret_name}/#{key}"] = value end + rescue JSON::ParserError + results["#{secret_name}"] = secret["SecretString"] end end end diff --git a/test/secrets/aws_secrets_manager_adapter_test.rb b/test/secrets/aws_secrets_manager_adapter_test.rb index 42a0f48ae..7ca947a66 100644 --- a/test/secrets/aws_secrets_manager_adapter_test.rb +++ b/test/secrets/aws_secrets_manager_adapter_test.rb @@ -44,6 +44,48 @@ class AwsSecretsManagerAdapterTest < SecretAdapterTestCase assert_equal expected_json, json end + test "fetch with string value" do + stub_ticks.with("aws --version 2> /dev/null") + stub_ticks + .with("aws secretsmanager batch-get-secret-value --secret-id-list secret secret2/KEY1 --profile default") + .returns(<<~JSON) + { + "SecretValues": [ + { + "ARN": "arn:aws:secretsmanager:us-east-1:aaaaaaaaaaaa:secret:secret", + "Name": "secret", + "VersionId": "vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv", + "SecretString": "a-string-secret", + "VersionStages": [ + "AWSCURRENT" + ], + "CreatedDate": "2024-01-01T00:00:00.000000" + }, + { + "ARN": "arn:aws:secretsmanager:us-east-1:aaaaaaaaaaaa:secret:secret2", + "Name": "secret2", + "VersionId": "vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv", + "SecretString": "{\\"KEY2\\":\\"VALUE2\\"}", + "VersionStages": [ + "AWSCURRENT" + ], + "CreatedDate": "2024-01-01T00:00:00.000000" + } + ], + "Errors": [] + } + JSON + + json = JSON.parse(shellunescape(run_command("fetch", "secret", "secret2/KEY1"))) + + expected_json = { + "secret"=>"a-string-secret", + "secret/KEY2"=>"VALUE2" + } + + assert_equal expected_json, json + end + test "fetch with secret names" do stub_ticks.with("aws --version 2> /dev/null") stub_ticks From 68e6f82b30b1b7b57f1e7c4ed6b1a5d6b8831439 Mon Sep 17 00:00:00 2001 From: Nick Hammond Date: Thu, 12 Dec 2024 04:17:03 -0700 Subject: [PATCH 2/6] Grab from secret2 for assertion --- test/secrets/aws_secrets_manager_adapter_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/secrets/aws_secrets_manager_adapter_test.rb b/test/secrets/aws_secrets_manager_adapter_test.rb index 7ca947a66..3fbfe3e9c 100644 --- a/test/secrets/aws_secrets_manager_adapter_test.rb +++ b/test/secrets/aws_secrets_manager_adapter_test.rb @@ -80,7 +80,7 @@ class AwsSecretsManagerAdapterTest < SecretAdapterTestCase expected_json = { "secret"=>"a-string-secret", - "secret/KEY2"=>"VALUE2" + "secret2/KEY2"=>"VALUE2" } assert_equal expected_json, json From e4641773499192f7ba0b0b8c27706540a37fa78e Mon Sep 17 00:00:00 2001 From: Nick Hammond Date: Thu, 12 Dec 2024 04:58:53 -0700 Subject: [PATCH 3/6] Check for errors from AWS secrets manager --- .../secrets/adapters/aws_secrets_manager.rb | 14 ++++++++--- .../aws_secrets_manager_adapter_test.rb | 24 +++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/lib/kamal/secrets/adapters/aws_secrets_manager.rb b/lib/kamal/secrets/adapters/aws_secrets_manager.rb index 5db8fd7c9..e3f54687e 100644 --- a/lib/kamal/secrets/adapters/aws_secrets_manager.rb +++ b/lib/kamal/secrets/adapters/aws_secrets_manager.rb @@ -6,7 +6,15 @@ def login(_account) def fetch_secrets(secrets, account:, session:) {}.tap do |results| - JSON.parse(get_from_secrets_manager(secrets, account: account))["SecretValues"].each do |secret| + secrets = JSON.parse(get_from_secrets_manager(secrets, account: account)) + + if secrets["Errors"].present? + first_error = secrets["Errors"].first + + raise RuntimeError, "#{first_error['SecretId']}: #{first_error['Message']}" + end + + secrets["SecretValues"].each do |secret| secret_name = secret["Name"] secret_string = JSON.parse(secret["SecretString"]) @@ -20,8 +28,8 @@ def fetch_secrets(secrets, account:, session:) end def get_from_secrets_manager(secrets, account:) - `aws secretsmanager batch-get-secret-value --secret-id-list #{secrets.map(&:shellescape).join(" ")} --profile #{account.shellescape}`.tap do - raise RuntimeError, "Could not read #{secret} from AWS Secrets Manager" unless $?.success? + `aws secretsmanager batch-get-secret-value --secret-id-list #{secrets.map(&:shellescape).join(" ")} --profile #{account.shellescape}`.tap do |secrets| + raise RuntimeError, "Could not read #{secrets} from AWS Secrets Manager" unless $?.success? end end diff --git a/test/secrets/aws_secrets_manager_adapter_test.rb b/test/secrets/aws_secrets_manager_adapter_test.rb index 3fbfe3e9c..5873731e5 100644 --- a/test/secrets/aws_secrets_manager_adapter_test.rb +++ b/test/secrets/aws_secrets_manager_adapter_test.rb @@ -1,6 +1,30 @@ require "test_helper" class AwsSecretsManagerAdapterTest < SecretAdapterTestCase + test "fails when errors are present" do + stub_ticks.with("aws --version 2> /dev/null") + stub_ticks + .with("aws secretsmanager batch-get-secret-value --secret-id-list unknown-secret-id --profile default") + .returns(<<~JSON) + { + "SecretValues": [], + "Errors": [ + { + "SecretId": "unknown-secret-id", + "ErrorCode": "ResourceNotFoundException", + "Message": "Secrets Manager can't find the specified secret." + } + ] + } + JSON + + error = assert_raises RuntimeError do + JSON.parse(shellunescape(run_command("fetch", "unknown-secret-id"))) + end + + assert_equal "unknown-secret-id: Secrets Manager can't find the specified secret.", error.message + end + test "fetch" do stub_ticks.with("aws --version 2> /dev/null") stub_ticks From ba567e0474e77450e54267ac6237c7ae907bc19d Mon Sep 17 00:00:00 2001 From: Nick Hammond Date: Thu, 12 Dec 2024 05:09:12 -0700 Subject: [PATCH 4/6] Just map the secrets returned from AWS --- .../secrets/adapters/aws_secrets_manager.rb | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/kamal/secrets/adapters/aws_secrets_manager.rb b/lib/kamal/secrets/adapters/aws_secrets_manager.rb index e3f54687e..c9314ca52 100644 --- a/lib/kamal/secrets/adapters/aws_secrets_manager.rb +++ b/lib/kamal/secrets/adapters/aws_secrets_manager.rb @@ -6,15 +6,7 @@ def login(_account) def fetch_secrets(secrets, account:, session:) {}.tap do |results| - secrets = JSON.parse(get_from_secrets_manager(secrets, account: account)) - - if secrets["Errors"].present? - first_error = secrets["Errors"].first - - raise RuntimeError, "#{first_error['SecretId']}: #{first_error['Message']}" - end - - secrets["SecretValues"].each do |secret| + get_from_secrets_manager(secrets, account: account).each do |secret| secret_name = secret["Name"] secret_string = JSON.parse(secret["SecretString"]) @@ -30,6 +22,12 @@ def fetch_secrets(secrets, account:, session:) def get_from_secrets_manager(secrets, account:) `aws secretsmanager batch-get-secret-value --secret-id-list #{secrets.map(&:shellescape).join(" ")} --profile #{account.shellescape}`.tap do |secrets| raise RuntimeError, "Could not read #{secrets} from AWS Secrets Manager" unless $?.success? + + secrets = JSON.parse(secrets) + + return secrets["SecretValues"] unless secrets["Errors"].present? + + raise RuntimeError, secrets["Errors"].map { |error| "#{error['SecretId']}: #{error['Message']}" }.join(", ") end end From 84a874e63b39d42d4c82dc9411b5af9e4853f766 Mon Sep 17 00:00:00 2001 From: Nick Hammond Date: Thu, 12 Dec 2024 05:15:52 -0700 Subject: [PATCH 5/6] Update secrets manager spec to render multiple errors --- lib/kamal/secrets/adapters/aws_secrets_manager.rb | 2 +- test/secrets/aws_secrets_manager_adapter_test.rb | 13 +++++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/kamal/secrets/adapters/aws_secrets_manager.rb b/lib/kamal/secrets/adapters/aws_secrets_manager.rb index c9314ca52..4bcac21d5 100644 --- a/lib/kamal/secrets/adapters/aws_secrets_manager.rb +++ b/lib/kamal/secrets/adapters/aws_secrets_manager.rb @@ -27,7 +27,7 @@ def get_from_secrets_manager(secrets, account:) return secrets["SecretValues"] unless secrets["Errors"].present? - raise RuntimeError, secrets["Errors"].map { |error| "#{error['SecretId']}: #{error['Message']}" }.join(", ") + raise RuntimeError, secrets["Errors"].map { |error| "#{error['SecretId']}: #{error['Message']}" }.join(" ") end end diff --git a/test/secrets/aws_secrets_manager_adapter_test.rb b/test/secrets/aws_secrets_manager_adapter_test.rb index 5873731e5..60074778b 100644 --- a/test/secrets/aws_secrets_manager_adapter_test.rb +++ b/test/secrets/aws_secrets_manager_adapter_test.rb @@ -4,13 +4,18 @@ class AwsSecretsManagerAdapterTest < SecretAdapterTestCase test "fails when errors are present" do stub_ticks.with("aws --version 2> /dev/null") stub_ticks - .with("aws secretsmanager batch-get-secret-value --secret-id-list unknown-secret-id --profile default") + .with("aws secretsmanager batch-get-secret-value --secret-id-list unknown1 unknown2 --profile default") .returns(<<~JSON) { "SecretValues": [], "Errors": [ { - "SecretId": "unknown-secret-id", + "SecretId": "unknown1", + "ErrorCode": "ResourceNotFoundException", + "Message": "Secrets Manager can't find the specified secret." + }, + { + "SecretId": "unknown2", "ErrorCode": "ResourceNotFoundException", "Message": "Secrets Manager can't find the specified secret." } @@ -19,10 +24,10 @@ class AwsSecretsManagerAdapterTest < SecretAdapterTestCase JSON error = assert_raises RuntimeError do - JSON.parse(shellunescape(run_command("fetch", "unknown-secret-id"))) + JSON.parse(shellunescape(run_command("fetch", "unknown1", "unknown2"))) end - assert_equal "unknown-secret-id: Secrets Manager can't find the specified secret.", error.message + assert_equal ["unknown1: Secrets Manager can't find the specified secret.", "unknown2: Secrets Manager can't find the specified secret."].join(" "), error.message end test "fetch" do From 725da6aa68ae632b1e651b7ae7fbdfaf5414a40d Mon Sep 17 00:00:00 2001 From: Nick Hammond Date: Thu, 12 Dec 2024 05:29:15 -0700 Subject: [PATCH 6/6] Rubocop, Rubocop --- test/secrets/aws_secrets_manager_adapter_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/secrets/aws_secrets_manager_adapter_test.rb b/test/secrets/aws_secrets_manager_adapter_test.rb index 60074778b..7616342db 100644 --- a/test/secrets/aws_secrets_manager_adapter_test.rb +++ b/test/secrets/aws_secrets_manager_adapter_test.rb @@ -27,7 +27,7 @@ class AwsSecretsManagerAdapterTest < SecretAdapterTestCase JSON.parse(shellunescape(run_command("fetch", "unknown1", "unknown2"))) end - assert_equal ["unknown1: Secrets Manager can't find the specified secret.", "unknown2: Secrets Manager can't find the specified secret."].join(" "), error.message + assert_equal [ "unknown1: Secrets Manager can't find the specified secret.", "unknown2: Secrets Manager can't find the specified secret." ].join(" "), error.message end test "fetch" do