fix(service-accounts): allow for rotating forever keys (#327)

* fix(service-accounts): allow for rotating forever keys

* Generate Terraform Docs

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

create-dev-testfile.sh

@@ -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

docs/resources/service_account.md

@@ -32,9 +32,42 @@ 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"
+  }
 }
 ```
 
@@ -50,6 +83,7 @@ resource "prefect_service_account" "example" {
 - `account_id` (String) Account ID (UUID), defaults to the account set in the provider
 - `account_role_name` (String) Account Role name of the service account (one of: Admin, Member, Owner)
 - `api_key_expiration` (String) 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` (Map of String) A map of values that, if changed, will trigger a key rotation (but not a re-creation of the Service Account)
 - `old_key_expires_in_seconds` (Number) Provide this field to set an expiration for the currently active api key. If not provided or provided Null, the current key will be deleted. If provided, it cannot be more than 48 hours (172800 seconds) in the future.
 
 ### Read-Only

examples/resources/prefect_service_account/resource.tf

@@ -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"
+  }
+}

internal/provider/resources/service_account.go

@@ -3,6 +3,7 @@ package resources
 import (
 	"context"
 	"fmt"
+	"maps"
 	"strings"
 	"time"
 
@@ -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"`
 }
@@ -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,
@@ -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(),

internal/provider/resources/service_account_test.go

@@ -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" {
@@ -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),
 				),
 			},
 			{
@@ -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"),
@@ -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
@@ -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]
@@ -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
 	}