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

fix: add an FTL tag and filter ASM secrets #1888

Merged
merged 4 commits into from
Jun 27, 2024
Merged

fix: add an FTL tag and filter ASM secrets #1888

merged 4 commits into from
Jun 27, 2024

Conversation

safeer
Copy link
Contributor

@safeer safeer commented Jun 27, 2024

Fixes #1876

@ftl-robot ftl-robot mentioned this pull request Jun 27, 2024
@safeer safeer force-pushed the saf/tag-asm-secrets branch from 15ba666 to 20c9f9e Compare June 27, 2024 00:11
@safeer safeer force-pushed the saf/tag-asm-secrets branch from 23da737 to b170a92 Compare June 27, 2024 00:25
@safeer safeer marked this pull request as ready for review June 27, 2024 00:30
@safeer safeer requested a review from alecthomas as a code owner June 27, 2024 00:30
@safeer safeer requested review from a team and matt2e and removed request for a team June 27, 2024 00:30
@matt2e
Copy link
Collaborator

matt2e commented Jun 27, 2024

LGTM 👍
is _ftl preferred over ftl?

Copy link
Collaborator

@alecthomas alecthomas left a comment

Choose a reason for hiding this comment

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

Nice!

@@ -149,6 +155,9 @@ func (l *asmLeader) store(ctx context.Context, ref Ref, value []byte) (*url.URL,
_, err := l.client.CreateSecret(ctx, &secretsmanager.CreateSecretInput{
Name: aws.String(ref.String()),
SecretString: aws.String(string(value)),
Tags: []types.Tag{
{Key: aws.String("_ftl"), Value: aws.String(ref.Module.Default("_"))},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason for the _?

@@ -54,6 +54,9 @@ func (l *asmLeader) sync(ctx context.Context, secrets *xsync.MapOf[Ref, cachedSe
out, err := l.client.ListSecrets(ctx, &secretsmanager.ListSecretsInput{
MaxResults: aws.Int32(100),
NextToken: nextToken.Ptr(),
Filters: []types.Filter{
{Key: types.FilterNameStringTypeTagKey, Values: []string{"_ftl"}},
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 pull the tag value out into a constant.

@@ -149,6 +155,9 @@ func (l *asmLeader) store(ctx context.Context, ref Ref, value []byte) (*url.URL,
_, err := l.client.CreateSecret(ctx, &secretsmanager.CreateSecretInput{
Name: aws.String(ref.String()),
SecretString: aws.String(string(value)),
Tags: []types.Tag{
{Key: aws.String("ftl"), Value: aws.String(ref.Module.Default("_"))},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can the value not be empty? _ is actually a valid module name, so if it can't be empty it should at least be - which is not a valid identifier.

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 can be empty, pushing a fix

@safeer safeer force-pushed the saf/tag-asm-secrets branch from 9c4190c to 2930b2c Compare June 27, 2024 00:58
@safeer safeer enabled auto-merge (squash) June 27, 2024 00:58
@michaelneale
Copy link
Contributor

nice! and much needed. Most AWS accounts will have many secrets in many formats

@safeer safeer merged commit 29438c5 into main Jun 27, 2024
43 checks passed
@safeer safeer deleted the saf/tag-asm-secrets branch June 27, 2024 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add labels to all AWS secrets
4 participants