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

feat: add secrets adapter for aws secrets manager #1141

Merged
merged 5 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
25 changes: 25 additions & 0 deletions lib/kamal/secrets/adapters/aws_secretsmanager.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
class Kamal::Secrets::Adapters::AwsSecretsmanager < Kamal::Secrets::Adapters::Base
Copy link
Collaborator

@djmb djmb Nov 4, 2024

Choose a reason for hiding this comment

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

The official name is AWS Secrets Manager, so let's call this AwsSecretsManager (and rename the file to aws_secrets_manager.rb

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's funny, I went back and forth on the name but ultimately decided on one word because the AWS CLI calls it "secretsmanager" over "secrets-manager". I agree though, I like the two word version better and will get that changed.

private
def login(_account)
nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's no sensible default way to implement this is there? Would be nice if we could prompt for MFA from here if that was enabled, but we'd need to know all about the IAM setup for that to work I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think there's just too many different ways to authenticate with AWS. I believe the preferred method is SSO using the aws sso login command, but the user could also be authenticated with access keys, environment variables, automatically (if they're on an EC2 instance with an IAM Role), or even an external process.

One option would be to check if the user is authenticated with aws sts get-caller-identity, and if that command returns an error, attempt to run aws sso login. However, this could be confusing if they normally authenticate a different way, or even worse have aws cli v1 installed which doesn't include sso login.

What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes let's leave it for now. We could I guess add a separate SsoAwsSecretsManager or something like that later.

end

def fetch_secrets(secrets, account:, session:)
{}.tap do |results|
JSON.parse(get_from_secrets_manager(secrets, account: account))["SecretValues"].each do |secret|
secret_name = secret["Name"]
secret_string = JSON.parse(secret["SecretString"])

secret_string.each do |key, value|
results["#{secret_name}/#{key}"] = value
end
end
end
end

def get_from_secrets_manager(secrets, account:)
`aws secretsmanager batch-get-secret-value --secret-id-list #{secrets.map(&:shellescape).join(" ")} --profile #{account}`.tap do
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's shell escape the account here as well 👍

raise RuntimeError, "Could not read #{secret} from AWS Secrets Manager" unless $?.success?
end
end
end
87 changes: 87 additions & 0 deletions test/secrets/aws_secretsmanager_adapter_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
require "test_helper"

class AwsSecretsmanagerAdapterTest < SecretAdapterTestCase
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we rename this too?

test "fetch" do
stub_ticks
.with("aws secretsmanager batch-get-secret-value --secret-id-list secret/KEY1 secret/KEY2 secret2/KEY3 --profile default")
.returns(<<~JSON)
{
"SecretValues": [
{
"ARN": "arn:aws:secretsmanager:us-east-1:aaaaaaaaaaaa:secret:secret",
"Name": "secret",
"VersionId": "vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv",
"SecretString": "{\\"KEY1\\":\\"VALUE1\\", \\"KEY2\\":\\"VALUE2\\"}",
"VersionStages": [
"AWSCURRENT"
],
"CreatedDate": "2024-01-01T00:00:00.000000"
},
{
"ARN": "arn:aws:secretsmanager:us-east-1:aaaaaaaaaaaa:secret:secret2",
"Name": "secret2",
"VersionId": "vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv",
"SecretString": "{\\"KEY3\\":\\"VALUE3\\"}",
"VersionStages": [
"AWSCURRENT"
],
"CreatedDate": "2024-01-01T00:00:00.000000"
}
],
"Errors": []
}
JSON

json = JSON.parse(shellunescape(run_command("fetch", "secret/KEY1", "secret/KEY2", "secret2/KEY3")))

expected_json = {
"secret/KEY1"=>"VALUE1",
"secret/KEY2"=>"VALUE2",
"secret2/KEY3"=>"VALUE3"
}

assert_equal expected_json, json
end

test "fetch with secret names" do
stub_ticks
.with("aws secretsmanager batch-get-secret-value --secret-id-list secret/KEY1 secret/KEY2 --profile default")
.returns(<<~JSON)
{
"SecretValues": [
{
"ARN": "arn:aws:secretsmanager:us-east-1:aaaaaaaaaaaa:secret:secret",
"Name": "secret",
"VersionId": "vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv",
"SecretString": "{\\"KEY1\\":\\"VALUE1\\", \\"KEY2\\":\\"VALUE2\\"}",
"VersionStages": [
"AWSCURRENT"
],
"CreatedDate": "2024-01-01T00:00:00.000000"
}
],
"Errors": []
}
JSON

json = JSON.parse(shellunescape(run_command("fetch", "--from", "secret", "KEY1", "KEY2")))

expected_json = {
"secret/KEY1"=>"VALUE1",
"secret/KEY2"=>"VALUE2"
}

assert_equal expected_json, json
end

private
def run_command(*command)
stdouted do
Kamal::Cli::Secrets.start \
[ *command,
"-c", "test/fixtures/deploy_with_accessories.yml",
"--adapter", "aws_secretsmanager",
"--account", "default" ]
end
end
end