Skip to content

Commit

Permalink
fix(service-accounts): allow for rotating forever keys
Browse files Browse the repository at this point in the history
  • Loading branch information
parkedwards committed Dec 14, 2024
1 parent bd2c37f commit 91c0834
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 6 deletions.
6 changes: 5 additions & 1 deletion create-dev-testfile.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ terraform {
}
}
provider "prefect" {}
provider "prefect" {
$([ -n "$PREFECT_API_URL" ] && echo ' endpoint = "'$PREFECT_API_URL'"')
$([ -n "$PREFECT_API_KEY" ] && echo ' api_key = "'$PREFECT_API_KEY'"')
$([ -n "$PREFECT_CLOUD_ACCOUNT_ID" ] && echo ' account_id = "'$PREFECT_CLOUD_ACCOUNT_ID'"')
}
resource "${resource}" "${name}" {}
EOF
Expand Down
35 changes: 34 additions & 1 deletion examples/resources/prefect_service_account/resource.tf
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,40 @@ resource "time_rotating" "ninety_days" {
}
# Pass the time_rotating resource to the `api_key_expiration` attribute
# in order to automate the rotation of the Service Account key
resource "prefect_service_account" "example" {
resource "prefect_service_account" "example_rotate_time_key" {
name = "my-service-account"
api_key_expiration = time_rotating.ninety_days.rotation_rfc3339
}

# Optionally, rotate non-expiring Service Account keys
# using the `api_key_keepers` attribute, which is an
# arbitrary map of values that, if changed, will
# trigger a key rotation (but not a re-creation of the Service Account)
resource "prefect_service_account" "example_rotate_forever_key" {
name = "my-service-account"
api_key_expiration = null # never expires
api_key_keepers = {
foo = "value-1"
bar = "value-2"
}
}

# Use the optional `old_key_expires_in_seconds`, which applies
# a TTL to the old key when rotation takes place.
# This is useful to ensure that your consumers don't experience
# downtime when the new key is being rolled out.
resource "prefect_service_account" "example_old_key_expires_later" {
name = "my-service-account"
old_key_expires_in_seconds = 300

# Remember that `old_key_expires_in_seconds` is only applied
# when a key rotation takes place, such as changing the
# `api_key_expiration` attribute
api_key_expiration = time_rotating.ninety_days.rotation_rfc3339

# or the `api_key_keepers` attribute
api_key_keepers = {
foo = "value-1"
bar = "value-2"
}
}
15 changes: 14 additions & 1 deletion internal/provider/resources/service_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package resources
import (
"context"
"fmt"
"maps"
"strings"
"time"

Expand Down Expand Up @@ -49,6 +50,7 @@ type ServiceAccountResourceModel struct {
APIKeyName types.String `tfsdk:"api_key_name"`
APIKeyCreated customtypes.TimestampValue `tfsdk:"api_key_created"`
APIKeyExpiration customtypes.TimestampValue `tfsdk:"api_key_expiration"`
APIKeyKeepers types.Map `tfsdk:"api_key_keepers"`
OldKeyExpiresInSeconds types.Int32 `tfsdk:"old_key_expires_in_seconds"`
APIKey types.String `tfsdk:"api_key"`
}
Expand Down Expand Up @@ -158,6 +160,11 @@ func (r *ServiceAccountResource) Schema(_ context.Context, _ resource.SchemaRequ
CustomType: customtypes.TimestampType{},
Description: "Timestamp of the API Key expiration (RFC3339). If left as null, the API Key will not expire. Modify this attribute to force a key rotation.",
},
"api_key_keepers": schema.MapAttribute{
Optional: true,
Description: "A map of values that, if changed, will trigger a key rotation (but not a re-creation of the Service Account)",
ElementType: types.StringType,
},
"old_key_expires_in_seconds": schema.Int32Attribute{
Optional: true,
Computed: true,
Expand Down Expand Up @@ -431,7 +438,13 @@ func (r *ServiceAccountResource) Update(ctx context.Context, req resource.Update
// ServiceAccount object with the new API Key value included in the response.
providedExpiration := plan.APIKeyExpiration.ValueTimePointer()
currentExpiration := serviceAccount.APIKey.Expiration
if !ArePointerTimesEqual(providedExpiration, currentExpiration) {

// Optionally, practitioners can rotate the key by modifying the `api_key_keepers` map.
// This is useful for rotating keys that are not expiring.
currentKeepers := state.APIKeyKeepers.Elements()
providedKeepers := plan.APIKeyKeepers.Elements()

if !ArePointerTimesEqual(providedExpiration, currentExpiration) || !maps.Equal(currentKeepers, providedKeepers) {
serviceAccount, err = client.RotateKey(ctx, plan.ID.ValueString(), api.ServiceAccountRotateKeyRequest{
APIKeyExpiration: providedExpiration,
OldKeyExpiresInSeconds: plan.OldKeyExpiresInSeconds.ValueInt32(),
Expand Down
52 changes: 49 additions & 3 deletions internal/provider/resources/service_account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,16 @@ resource "prefect_service_account" "bot" {
}`, name, expiration.Format(time.RFC3339))
}

func fixtureAccServiceAccountResourceKeyKeepers(name string, keeperValue string) string {
return fmt.Sprintf(`
resource "prefect_service_account" "bot" {
name = "%s"
api_key_keepers = {
foo = "%s"
}
}`, name, keeperValue)
}

func fixtureAccServiceAccountResourceUpdateAccountRoleName(name string, roleName string) string {
return fmt.Sprintf(`
resource "prefect_service_account" "bot" {
Expand Down Expand Up @@ -92,12 +102,12 @@ func TestAccResource_service_account(t *testing.T) {
},
{
// Ensure non-expiration time change DOESN'T trigger a key rotation
Config: fixtureAccServiceAccountResource(botRandomName),
Config: fixtureAccServiceAccountResource(botRandomName2),
Check: resource.ComposeTestCheckFunc(
testAccCheckServiceAccountResourceExists(botResourceName, &bot),
testAccCheckServiceAccountValues(&bot, &api.ServiceAccount{Name: botRandomName, AccountRoleName: "Member"}),
testAccCheckServiceAccountValues(&bot, &api.ServiceAccount{Name: botRandomName2, AccountRoleName: "Member"}),
testAccCheckServiceAccountAPIKeyUnchanged(botResourceName, &apiKey),
resource.TestCheckResourceAttr(botResourceName, "name", botRandomName),
resource.TestCheckResourceAttr(botResourceName, "name", botRandomName2),
),
},
{
Expand All @@ -110,6 +120,36 @@ func TestAccResource_service_account(t *testing.T) {
resource.TestCheckResourceAttr(botResourceName, "name", botRandomName),
),
},
{
// Ensure that switching to key keepers DOES trigger a key rotation
Config: fixtureAccServiceAccountResourceKeyKeepers(botRandomName, "keeper-value-1"),
Check: resource.ComposeTestCheckFunc(
testAccCheckServiceAccountResourceExists(botResourceName, &bot),
testAccCheckServiceAccountValues(&bot, &api.ServiceAccount{Name: botRandomName, AccountRoleName: "Member"}),
testAccCheckServiceAccountAPIKeyRotated(botResourceName, &apiKey),
resource.TestCheckResourceAttr(botResourceName, "name", botRandomName),
),
},
{
// Ensure that key keepers change DOES trigger a key rotation
Config: fixtureAccServiceAccountResourceKeyKeepers(botRandomName, "keeper-value-2"),
Check: resource.ComposeTestCheckFunc(
testAccCheckServiceAccountResourceExists(botResourceName, &bot),
testAccCheckServiceAccountValues(&bot, &api.ServiceAccount{Name: botRandomName, AccountRoleName: "Member"}),
testAccCheckServiceAccountAPIKeyRotated(botResourceName, &apiKey),
resource.TestCheckResourceAttr(botResourceName, "name", botRandomName),
),
},
{
// Ensure that a non-key keeper change DOES NOT trigger a key rotation
Config: fixtureAccServiceAccountResourceKeyKeepers(botRandomName2, "keeper-value-2"),
Check: resource.ComposeTestCheckFunc(
testAccCheckServiceAccountResourceExists(botResourceName, &bot),
testAccCheckServiceAccountValues(&bot, &api.ServiceAccount{Name: botRandomName2, AccountRoleName: "Member"}),
testAccCheckServiceAccountAPIKeyUnchanged(botResourceName, &apiKey),
resource.TestCheckResourceAttr(botResourceName, "name", botRandomName2),
),
},
{
// Ensure updates of the account role
Config: fixtureAccServiceAccountResourceUpdateAccountRoleName(botRandomName, "Admin"),
Expand Down Expand Up @@ -202,6 +242,8 @@ func textAccCheckServiceAccountAPIKeyStored(resourceName string, passedKey *stri
}
}

// testAccCheckServiceAccountAPIKeyUnchanged is a helper function that checks if the API key was unchanged.
// Upon success, it will ensure that the passeKey is updated to the state key (which is a no-op).
func testAccCheckServiceAccountAPIKeyUnchanged(n string, passedKey *string) resource.TestCheckFunc {
return func(s *terraform.State) error {
// find the corresponding state object
Expand All @@ -215,11 +257,14 @@ func testAccCheckServiceAccountAPIKeyUnchanged(n string, passedKey *string) reso
if *passedKey != key {
return fmt.Errorf("key was incorrectly rotated, since the old key=%s is different from new key=%s", *passedKey, key)
}
*passedKey = key

return nil
}
}

// testAccCheckServiceAccountAPIKeyRotated is a helper function that checks if the API key was rotated correctly.
// Upon success, it will ensure that the passeKey is updated to the state key (which is new).
func testAccCheckServiceAccountAPIKeyRotated(n string, passedKey *string) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]
Expand All @@ -232,6 +277,7 @@ func testAccCheckServiceAccountAPIKeyRotated(n string, passedKey *string) resour
if *passedKey == key {
return fmt.Errorf("key rotation did not occur correctly, as the old key=%s is the same as the new key=%s", *passedKey, key)
}
*passedKey = key

return nil
}
Expand Down

0 comments on commit 91c0834

Please sign in to comment.