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

Add flyctl commands for managing secrets that are kms keys #3901

Merged
merged 19 commits into from
Sep 14, 2024

Conversation

timflyio
Copy link
Contributor

@timflyio timflyio commented Aug 27, 2024

Change Summary

What and Why: Adds flyctl secrets keys commands for adding, removing, and listing KMS keys. This will allow users to manage secrets that they can use with the kmsfs via /.fly/kms in their machine.

How: Makes calls via the flaps client to new endpoints.

Related to: nomad-firecracker PR #2599 which adds flaps endpoints for managing KMS secrets.


Documentation

  • Fresh Produce
  • In superfly/docs, or asked for help from docs team
  • n/a

@timflyio
Copy link
Contributor Author

TODO: This needs go.mod changes for new fly-go/flaps version once that lands.

@timflyio
Copy link
Contributor Author

manually tested listing, setting, and deleting kms secrets (with pr flaps client, and flaps server on dev gw), and verified that keys are visible in machine's kmsfs (with flyd pr that allows qmx machines to start kmsfs).

@timflyio
Copy link
Contributor Author

TODO: I should probably hide this command until we're ready to announce it. I think we'll want to iterate on a few of the rough edges in kmsfs/petsem/flaps/flyctl before we freshproduce this.

@timflyio timflyio requested a review from btoews August 28, 2024 02:19
internal/command/secrets/key_delete.go Outdated Show resolved Hide resolved
internal/command/secrets/key_set.go Outdated Show resolved Hide resolved
internal/command/secrets/ver.go Outdated Show resolved Hide resolved
@timflyio timflyio marked this pull request as ready for review August 29, 2024 02:59
Copy link
Member

@btoews btoews left a comment

Choose a reason for hiding this comment

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

I gave this a closer read through now that we've sorted out the philosophical questions. This is looking good to me!

internal/command/secrets/key_delete.go Outdated Show resolved Hide resolved
Key deletion will iterate all existing keys, and delete any keys that match the command line label. If the command line label is unversioned, it will match all key versions. It will prompt before each deletion and print out a message after each deletion. The force flag suppresses prompting. Note: this means if there are no matching keys, no delete is attempted, and no message is printed, and no error is returned.
@timflyio
Copy link
Contributor Author

I updated key deletion to include some versioning smarts, prompting (suppressable with force flag) and output indicating which keys were deleted:

Key deletion will iterate all existing keys, and delete any keys that match the command line label. If the command line label is unversioned, it will match all key versions. It will prompt before each deletion and print out a message after each deletion. The force flag suppresses prompting. Note: this means if there are no matching keys, no delete is attempted, and no message is printed, and no error is returned.

Copy link
Member

@btoews btoews left a comment

Choose a reason for hiding this comment

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

LGTM

internal/command/secrets/key_delete.go Outdated Show resolved Hide resolved
@timflyio timflyio merged commit 76d027c into master Sep 14, 2024
28 of 30 checks passed
@timflyio timflyio deleted the tim-secretskeys branch September 14, 2024 20:26
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.

2 participants