fix(gcp): default KMS key rotation to 90 days and reject sub-30-day periods

pkg/clouds/gcloud/auth.go

@@ -6,6 +6,10 @@ package gcloud
 import (
 	"encoding/json"
 	"fmt"
+	"strconv"
+	"strings"
+
+	"github.com/pkg/errors"
 
 	"github.com/simple-container-com/api/pkg/api"
 )
@@ -18,6 +22,27 @@ const (
 	SecretsProviderTypeGcpKms = "gcp-kms"
 )
 
+const (
+	// DefaultKeyRotationPeriod is applied when keyRotationPeriod is unset.
+	//
+	// Cloud KMS bills every ACTIVE key version (ENABLED, DISABLED and
+	// DESTROY_SCHEDULED all count; only DESTROYED is free) and rotation never
+	// re-encrypts existing ciphertext, so every version a key mints stays
+	// load-bearing and billed for the lifetime of the key. That makes the
+	// rotation period a direct, compounding cost multiplier: one provisioned
+	// key per stack rotating daily adds a billed version per stack per day,
+	// forever.
+	DefaultKeyRotationPeriod = "7776000s" // 90 days
+
+	// MinKeyRotationPeriodSeconds is the lower bound accepted for an explicit
+	// keyRotationPeriod. GCP's own floor is 86400s (1 day), which is far too
+	// low to be a sane default for a per-stack provisioned key: a period of a
+	// few hours or days passes GCP validation and silently accrues versions.
+	// Rejecting anything under 30 days turns that class of typo into a
+	// config-parse error instead of an unattributed bill months later.
+	MinKeyRotationPeriodSeconds = 2592000 // 30 days
+)
+
 type ServiceAccountConfig struct {
 	ProjectId string `json:"projectId" yaml:"projectId"`
 }
@@ -59,6 +84,12 @@ type SecretsProviderConfig struct {
 	KeyLocation string `json:"keyLocation" yaml:"keyLocation"`
 	// only applicable when provision=true
 	KeyRotationPeriod string `json:"keyRotationPeriod" yaml:"keyRotationPeriod"`
+	// AllowShortKeyRotation opts out of the MinKeyRotationPeriodSeconds floor.
+	// Rotating faster than 30 days is a legitimate compliance choice; it is
+	// gated only because every rotation mints a permanently billed key version,
+	// so the common case of a mistyped period should fail loudly. Setting this
+	// makes the short period a deliberate, reviewable decision.
+	AllowShortKeyRotation bool `json:"allowShortKeyRotation" yaml:"allowShortKeyRotation"`
 
 	// whether to provision key
 	Provision bool `json:"provision" yaml:"provision"`
@@ -80,6 +111,37 @@ func (r *SecretsProviderConfig) KeyUrl() string {
 	return r.KeyName
 }
 
+// EffectiveKeyRotationPeriod returns the configured rotation period, or
+// DefaultKeyRotationPeriod when unset.
+func (r *SecretsProviderConfig) EffectiveKeyRotationPeriod() string {
+	if r.KeyRotationPeriod == "" {
+		return DefaultKeyRotationPeriod
+	}
+	return r.KeyRotationPeriod
+}
+
+// ValidateKeyRotationPeriod checks an explicitly configured rotation period.
+// Only applicable when provision=true; an empty value is valid and means the
+// default applies.
+func (r *SecretsProviderConfig) ValidateKeyRotationPeriod() error {
+	if r.KeyRotationPeriod == "" {
+		return nil
+	}
+	raw := r.KeyRotationPeriod
+	if !strings.HasSuffix(raw, "s") {
+		return errors.Errorf("keyRotationPeriod %q must be a duration in seconds with an 's' suffix, e.g. %q", raw, DefaultKeyRotationPeriod)
+	}
+	secs, err := strconv.Atoi(strings.TrimSuffix(raw, "s"))
+	if err != nil {
+		return errors.Errorf("keyRotationPeriod %q must be a whole number of seconds with an 's' suffix, e.g. %q", raw, DefaultKeyRotationPeriod)
+	}
+	if secs < MinKeyRotationPeriodSeconds && !r.AllowShortKeyRotation {
+		return errors.Errorf("keyRotationPeriod %q is %d seconds, below the minimum of %d (30 days): every rotation mints a key version that Cloud KMS bills for the lifetime of the key, so short periods accrue cost indefinitely. Set allowShortKeyRotation: true if the faster rotation is deliberate",
+			raw, secs, MinKeyRotationPeriodSeconds)
+	}
+	return nil
+}
+
 func (r *Credentials) ProviderType() string {
 	return ProviderType
 }

pkg/clouds/gcloud/kms_rotation_test.go

@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: MIT
+// Copyright (c) Simple Container
+
+package gcloud
+
+import (
+	"strconv"
+	"strings"
+	"testing"
+
+	. "github.com/onsi/gomega"
+)
+
+// The rotation period is a cost multiplier, not a cosmetic setting: Cloud KMS
+// bills every active key version, rotation never re-encrypts existing
+// ciphertext (so old versions stay load-bearing and billed), and provisioning
+// creates one key per stack. A too-short period therefore accrues billed
+// versions for the lifetime of the key. These tests pin the default and the
+// floor so a regression shows up here rather than on an invoice.
+func TestSecretsProviderConfig_EffectiveKeyRotationPeriod(t *testing.T) {
+	RegisterTestingT(t)
+
+	Expect((&SecretsProviderConfig{}).EffectiveKeyRotationPeriod()).To(Equal(DefaultKeyRotationPeriod),
+		"unset rotation period must fall back to the default")
+	Expect((&SecretsProviderConfig{KeyRotationPeriod: "31536000s"}).EffectiveKeyRotationPeriod()).
+		To(Equal("31536000s"), "an explicit value must win over the default")
+}
+
+func TestDefaultKeyRotationPeriodIsSane(t *testing.T) {
+	RegisterTestingT(t)
+
+	secs, err := strconv.Atoi(strings.TrimSuffix(DefaultKeyRotationPeriod, "s"))
+	Expect(err).To(BeNil(), "default must be a parseable seconds value")
+	Expect(secs).To(BeNumerically(">=", MinKeyRotationPeriodSeconds),
+		"the default must itself satisfy the validation floor")
+	// Guards against reintroducing a value like 100000s (27.8 hours), which is
+	// above GCP's own 86400s floor and so passes provider validation while
+	// minting a billed key version roughly every day.
+	Expect(secs).To(BeNumerically(">", 86400),
+		"default must be well clear of GCP's 1-day minimum")
+}
+
+func TestSecretsProviderConfig_ValidateKeyRotationPeriod(t *testing.T) {
+	tests := []struct {
+		name      string
+		period    string
+		errSubstr string
+	}{
+		{name: "unset is valid and means default", period: ""},
+		{name: "90 days", period: "7776000s"},
+		{name: "exactly the 30-day floor", period: "2592000s"},
+		{name: "one year", period: "31536000s"},
+		{name: "below floor: 27.8 hours", period: "100000s", errSubstr: "below the minimum"},
+		{name: "below floor: GCP minimum of one day", period: "86400s", errSubstr: "below the minimum"},
+		{name: "below floor: one second under", period: "2591999s", errSubstr: "below the minimum"},
+		{name: "missing seconds suffix", period: "7776000", errSubstr: "'s' suffix"},
+		// "ninetydays" ends in 's', so it reaches the numeric branch, not the
+		// suffix branch. Asserting the numeric message keeps the two branches
+		// distinguishable — otherwise a regression that collapsed them would
+		// still pass.
+		{name: "not a number but ends in s", period: "ninetydays", errSubstr: "whole number of seconds"},
+		{name: "suffix only", period: "s", errSubstr: "whole number of seconds"},
+		{name: "negative", period: "-100s", errSubstr: "below the minimum"},
+		{name: "fractional seconds", period: "2592000.5s", errSubstr: "whole number of seconds"},
+		{name: "duration shorthand is not accepted", period: "90d", errSubstr: "'s' suffix"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			RegisterTestingT(t)
+			err := (&SecretsProviderConfig{Provision: true, KeyRotationPeriod: tt.period}).
+				ValidateKeyRotationPeriod()
+			if tt.errSubstr == "" {
+				Expect(err).To(BeNil())
+				return
+			}
+			Expect(err).NotTo(BeNil(), "expected %q to be rejected", tt.period)
+			Expect(err.Error()).To(ContainSubstring(tt.errSubstr))
+		})
+	}
+}
+
+// A deliberate sub-30-day rotation must remain expressible: this is a shared
+// library, and a compliance requirement for faster rotation is legitimate. The
+// opt-out is what keeps the floor a typo-catcher rather than a policy imposed on
+// every consumer.
+func TestSecretsProviderConfig_AllowShortKeyRotationOptsOutOfTheFloor(t *testing.T) {
+	RegisterTestingT(t)
+
+	short := &SecretsProviderConfig{Provision: true, KeyRotationPeriod: "604800s"} // 7 days
+	Expect(short.ValidateKeyRotationPeriod()).NotTo(BeNil(),
+		"a short period must fail by default so typos surface")
+	Expect(short.ValidateKeyRotationPeriod().Error()).To(ContainSubstring("allowShortKeyRotation"),
+		"the error must name the escape hatch")
+
+	deliberate := &SecretsProviderConfig{Provision: true, KeyRotationPeriod: "604800s", AllowShortKeyRotation: true}
+	Expect(deliberate.ValidateKeyRotationPeriod()).To(BeNil(),
+		"an explicit opt-out must be honoured")
+
+	// The opt-out must not disable the malformed-value checks: it is about the
+	// floor, not about accepting garbage.
+	malformed := &SecretsProviderConfig{Provision: true, KeyRotationPeriod: "90d", AllowShortKeyRotation: true}
+	Expect(malformed.ValidateKeyRotationPeriod()).NotTo(BeNil())
+}

pkg/clouds/pulumi/gcp/kms_key.go

@@ -12,7 +12,6 @@ import (
 	"google.golang.org/api/serviceusage/v1"
 
 	"github.com/pkg/errors"
-	"github.com/samber/lo"
 
 	"github.com/pulumi/pulumi-gcp/sdk/v8/go/gcp/kms"
 	sdk "github.com/pulumi/pulumi/sdk/v3/go/pulumi"
@@ -29,6 +28,14 @@ func KmsKeySecretsProvider(ctx *sdk.Context, stack api.Stack, input api.Resource
 		return nil, errors.Errorf("failed to convert KmsKeyInput for %q", input.Descriptor.Type)
 	}
 
+	// Validate before any side effect. Enabling service APIs mutates the
+	// project, and a KeyRing can never be deleted in GCP (destroying the Pulumi
+	// resource only drops it from state), so failing later would leave a
+	// permanent, un-recreatable-by-name KeyRing behind for a mere typo.
+	if err := kmsInput.ValidateKeyRotationPeriod(); err != nil {
+		return nil, err
+	}
+
 	if err := enableServicesAPI(ctx.Context(), input.Descriptor.Config.Config,
 		fmt.Sprintf("projects/%s/services/serviceusage.googleapis.com", kmsInput.ProjectId)); err != nil {
 		_, _ = os.Stderr.WriteString(color.RedFmt("service usage API seems to be disabled on project %q, "+
@@ -54,7 +61,7 @@ func KmsKeySecretsProvider(ctx *sdk.Context, stack api.Stack, input api.Resource
 	}
 
 	// Create a new CryptoKey associated with the KeyRing.
-	rotationPeriod := lo.If(kmsInput.KeyRotationPeriod == "", "100000s").Else(kmsInput.KeyRotationPeriod)
+	rotationPeriod := kmsInput.EffectiveKeyRotationPeriod()
 
 	key, err := kms.NewCryptoKey(ctx, input.ToResName(input.Descriptor.Name), &kms.CryptoKeyArgs{
 		Name:           sdk.String(input.Descriptor.Name),