refactor: move EBS encryption posture to domains/ec2/ebs/encryption and delete utils

The master-key loader was used only by EC2 image, instance and volume to
derive the AWS-visible Encrypted flag. It becomes ebsencryption.Enabled,
EC2's temporary reading of the provider's key-file setting; the cache,
empty-path meaning, error text and each caller's error handling are
unchanged (image still reports unencrypted when the key will not load).
Its successor is a providers/ebs capability reporting volume encryption
posture.

utils/ is now empty and deleted. The layering test drops utils and
services from the legacy roots, removes their allowlist entries and fails
if either retired root regains files. The S7 subject scan follows the
vpc.* literals from utils to contracts/network so its gap list stays
complete.
This commit is contained in:
joshsiv-mulga
2026-10-09 19:21:34 +10:00
parent 46f4489631
commit fd88e8ceb0
16 changed files with 118 additions and 107 deletions
-4
View File
@@ -63,16 +63,12 @@ var allowlist = []allowance{
{From: "domains/ec2/guestmetadata", To: "handlers/sts", Debt: "Guest metadata: the cross-domain and runtime imports above", Reason: "guest-metadata relocation residual; needs capabilities, projections or contracts"},
{From: "domains/ec2/guestmetadata", To: "runtime/compute/vm", Debt: "Guest metadata: the cross-domain and runtime imports above", Reason: "guest-metadata relocation residual; needs capabilities, projections or contracts"},
{From: "domains/ec2/image", To: "utils", Debt: "utils", Reason: "consumes a recorded utils residual file awaiting its owner"},
{From: "domains/ec2/instance", To: "domains/dns", Debt: "domains/ec2/instance", Reason: "cross-domain implementation import of DNS"},
{From: "domains/ec2/instance", To: "domains/network/topology", Debt: "EC2 → network implementation", Reason: "EC2 reads network implementation; clears with the EC2-owned projection"},
{From: "domains/ec2/instance", To: "runtime/compute/gpu", Debt: "domains/ec2/instance", Reason: "VM/GPU runtime state read directly; needs a domain-owned projection"},
{From: "domains/ec2/instance", To: "runtime/compute/vm", Debt: "domains/ec2/instance", Reason: "VM/GPU runtime state read directly; needs a domain-owned projection"},
{From: "domains/ec2/instance", To: "utils", Debt: "utils", Reason: "consumes a recorded utils residual file awaiting its owner"},
{From: "domains/ec2/volume", To: "runtime/compute/vm", Debt: "Domain → runtime", Reason: "VM runtime state read directly; needs a domain-owned projection"},
{From: "domains/ec2/volume", To: "utils", Debt: "utils", Reason: "consumes a recorded utils residual file awaiting its owner"},
{From: "domains/ec2/vpc", To: "domains/network/external", Debt: "EC2 → network implementation", Reason: "EC2 reads network implementation; clears with the EC2-owned projection"},
{From: "domains/ec2/vpc", To: "domains/network/external/dhcp", Debt: "EC2 → network implementation", Reason: "EC2 reads network implementation; clears with the EC2-owned projection"},
+2 -2
View File
@@ -59,7 +59,7 @@ func (c class) isLegacy(roots ...string) bool {
// legacyRoots are pre-ADR-0001 application roots still standing.
var legacyRoots = map[string]bool{
"handlers": true, "gateway": true, "services": true, "utils": true, "daemon": true,
"handlers": true, "gateway": true, "daemon": true,
"vpcd": true, "admin": true, "accountteardown": true, "lbagent": true,
}
@@ -73,7 +73,7 @@ var gatewayDomains = map[string]string{
// runtimeLegacy and operatorLegacy are the legacy roots whose ADR-0001 target
// is runtime/ and operator/ respectively.
var (
runtimeLegacy = []string{"daemon", "vpcd", "services"}
runtimeLegacy = []string{"daemon", "vpcd"}
operatorLegacy = []string{"admin", "accountteardown"}
)
+10 -36
View File
@@ -3,8 +3,6 @@ package layering
import (
"os"
"path/filepath"
"sort"
"strings"
"testing"
)
@@ -34,16 +32,15 @@ var frozenLegacyPackages = []string{
"handlers/rds",
"handlers/sts",
"lbagent",
"utils",
"vpcd",
}
// frozenUtilsFiles are the only production files utils may hold; each awaits
// the owner named in the migration record's close-out table.
var frozenUtilsFiles = []string{"encryption.go"}
// retiredLegacyRoots were legacy roots whose every package has moved to its
// ADR-0001 target; recreating one would reopen a closed migration row.
var retiredLegacyRoots = []string{"services", "utils"}
// TestADR0001_LegacyRootsFrozen: the current-to-target map gives every legacy
// root a destination, so none may gain a package and utils no new file.
// root a destination, so none may gain a package and no retired root may return.
func TestADR0001_LegacyRootsFrozen(t *testing.T) {
root, err := moduleRoot()
if err != nil {
@@ -76,36 +73,13 @@ func TestADR0001_LegacyRootsFrozen(t *testing.T) {
}
}
entries, err := os.ReadDir(filepath.Join(root, "spinifex", "utils"))
if err != nil {
t.Fatal(err)
}
var files []string
for _, e := range entries {
name := e.Name()
if e.IsDir() {
t.Errorf("ADR-0001 utils row \"no new utils replacement\": utils/%s is a new directory; give the code a named owner instead", name)
continue
for _, r := range retiredLegacyRoots {
entries, err := os.ReadDir(filepath.Join(root, "spinifex", r))
if err != nil && !os.IsNotExist(err) {
t.Fatal(err)
}
if strings.HasSuffix(name, ".go") && !strings.HasSuffix(name, "_test.go") {
files = append(files, name)
}
}
sort.Strings(files)
allowedFiles := map[string]bool{}
for _, f := range frozenUtilsFiles {
allowedFiles[f] = true
}
have := map[string]bool{}
for _, f := range files {
have[f] = true
if !allowedFiles[f] {
t.Errorf("ADR-0001 utils row \"dissolve by responsibility; no new utils replacement\": utils/%s is new; move it to its owner", f)
}
}
for _, f := range frozenUtilsFiles {
if !have[f] {
t.Errorf("utils/%s has moved out; delete it from frozenUtilsFiles", f)
if len(entries) > 0 {
t.Errorf("ADR-0001 current-to-target map: spinifex/%s is a retired legacy root; give the code its target home instead", r)
}
}
}
+1 -1
View File
@@ -121,7 +121,7 @@ var rules = []rule{
return false
}
switch {
case t.layer == layerAgents || t.isLegacy("lbagent", "utils"):
case t.layer == layerAgents || t.isLegacy("lbagent"):
return true
case t.layer == layerIngress:
return f.kind != domainAWSAPI && f.kind != legacyAdapter
@@ -0,0 +1,50 @@
// Package ebsencryption owns EC2's AWS-visible EBS encryption posture: whether
// a volume the control plane reports is Encrypted. It is EC2's temporary
// reading of provider configuration (the shared Viperblock master-key file),
// not an encryption implementation; the provider encrypts on its own. Its
// intended successor is a providers/ebs capability that reports each volume's
// encryption posture, after which EC2 stops reading key files.
package ebsencryption
import (
"fmt"
"sync"
"github.com/mulgadc/bluebottle/pkg/masterkey"
)
// viperblockKeyCache memoises masterkey.LoadShared by path. The *Key holds an AEAD safe for concurrent use.
var (
viperblockKeyCacheMu sync.Mutex
viperblockKeyCache = map[string]*masterkey.Key{}
)
// Enabled reports whether volumes are encrypted at rest under the configured
// key file. An empty path means encryption is disabled. A configured key that
// cannot be loaded is an error; each caller decides what that means for it.
func Enabled(keyFile string) (bool, error) {
k, err := loadViperblockMasterKey(keyFile)
if err != nil {
return false, err
}
return k != nil, nil
}
// loadViperblockMasterKey returns the cached *masterkey.Key for path, loading on first use.
// An empty path returns (nil, nil), meaning encryption is disabled.
func loadViperblockMasterKey(path string) (*masterkey.Key, error) {
if path == "" {
return nil, nil
}
viperblockKeyCacheMu.Lock()
defer viperblockKeyCacheMu.Unlock()
if k, ok := viperblockKeyCache[path]; ok {
return k, nil
}
k, err := masterkey.LoadShared(path)
if err != nil {
return nil, fmt.Errorf("load viperblock encryption key %s: %w", path, err)
}
viperblockKeyCache[path] = k
return k, nil
}
@@ -1,4 +1,4 @@
package utils
package ebsencryption
import (
"crypto/rand"
@@ -33,14 +33,14 @@ func resetKeyCache() {
viperblockKeyCache = map[string]*masterkey.Key{}
}
func TestLoadViperblockMasterKey_EmptyPath(t *testing.T) {
func TestLoadMasterKey_EmptyPath(t *testing.T) {
resetKeyCache()
k, err := LoadViperblockMasterKey("")
k, err := loadViperblockMasterKey("")
require.NoError(t, err)
assert.Nil(t, k, "empty path must return (nil, nil) — encryption disabled")
}
func TestLoadViperblockMasterKey_LoadsAndCaches(t *testing.T) {
func TestLoadMasterKey_LoadsAndCaches(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("POSIX permission semantics not enforced on Windows")
}
@@ -48,30 +48,30 @@ func TestLoadViperblockMasterKey_LoadsAndCaches(t *testing.T) {
path := writeMasterKeyFile(t, "key", 0o640)
k1, err := LoadViperblockMasterKey(path)
k1, err := loadViperblockMasterKey(path)
require.NoError(t, err)
require.NotNil(t, k1)
assert.NotEmpty(t, k1.Fingerprint)
// Second call must return the cached *Key (pointer-equal), proving
// the path is memoised rather than re-stat'd + re-parsed.
k2, err := LoadViperblockMasterKey(path)
k2, err := loadViperblockMasterKey(path)
require.NoError(t, err)
assert.Same(t, k1, k2, "second LoadViperblockMasterKey must return cached pointer")
assert.Same(t, k1, k2, "second loadViperblockMasterKey must return cached pointer")
}
func TestLoadViperblockMasterKey_LoadError(t *testing.T) {
func TestLoadMasterKey_LoadError(t *testing.T) {
resetKeyCache()
missing := filepath.Join(t.TempDir(), "nonexistent.key")
k, err := LoadViperblockMasterKey(missing)
k, err := loadViperblockMasterKey(missing)
require.Error(t, err)
assert.Nil(t, k)
assert.Contains(t, err.Error(), "load viperblock encryption key")
assert.Contains(t, err.Error(), missing)
}
func TestLoadViperblockMasterKey_FailedLoadNotCached(t *testing.T) {
func TestLoadMasterKey_FailedLoadNotCached(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("POSIX permission semantics not enforced on Windows")
}
@@ -79,13 +79,39 @@ func TestLoadViperblockMasterKey_FailedLoadNotCached(t *testing.T) {
// 0o644 has the world-read bit set, which masterkey.LoadShared rejects.
path := writeMasterKeyFile(t, "loose.key", 0o644)
_, err := LoadViperblockMasterKey(path)
_, err := loadViperblockMasterKey(path)
require.Error(t, err)
// Fix the permissions; a subsequent call must succeed (proving the
// previous failure was not memoised).
require.NoError(t, os.Chmod(path, 0o640))
k, err := LoadViperblockMasterKey(path)
k, err := loadViperblockMasterKey(path)
require.NoError(t, err)
require.NotNil(t, k)
}
func TestEnabled_EmptyPathIsUnencrypted(t *testing.T) {
resetKeyCache()
enabled, err := Enabled("")
require.NoError(t, err)
assert.False(t, enabled)
}
func TestEnabled_LoadableKeyIsEncrypted(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("POSIX permission semantics not enforced on Windows")
}
resetKeyCache()
enabled, err := Enabled(writeMasterKeyFile(t, "key", 0o640))
require.NoError(t, err)
assert.True(t, enabled)
}
func TestEnabled_UnloadableKeyIsAnError(t *testing.T) {
resetKeyCache()
missing := filepath.Join(t.TempDir(), "nonexistent.key")
enabled, err := Enabled(missing)
require.Error(t, err)
assert.False(t, enabled)
assert.Contains(t, err.Error(), "load viperblock encryption key")
}
+3 -3
View File
@@ -16,6 +16,7 @@ import (
"github.com/aws/aws-sdk-go/service/s3"
"github.com/mulgadc/bluebottle/pkg/safecast"
"github.com/mulgadc/spinifex/spinifex/bootstrap/config"
ebsencryption "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/encryption"
"github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/metadata"
ec2platform "github.com/mulgadc/spinifex/spinifex/domains/ec2/platform"
ec2snapshot "github.com/mulgadc/spinifex/spinifex/domains/ec2/snapshot"
@@ -25,7 +26,6 @@ import (
awstags "github.com/mulgadc/spinifex/spinifex/foundation/aws/tags"
"github.com/mulgadc/spinifex/spinifex/providers/ebs"
"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
"github.com/mulgadc/spinifex/spinifex/utils"
"github.com/nats-io/nats.go"
)
@@ -993,12 +993,12 @@ func (s *ImageServiceImpl) clusterEncryptionEnabled() bool {
if s.config == nil {
return false
}
mkey, err := utils.LoadViperblockMasterKey(s.config.Viperblock.EncryptionKeyFile)
encrypted, err := ebsencryption.Enabled(s.config.Viperblock.EncryptionKeyFile)
if err != nil {
slog.Warn("clusterEncryptionEnabled: failed to load master key, reporting false", "err", err)
return false
}
return mkey != nil
return encrypted
}
// synthesizeRootBlockDeviceMapping returns /dev/sda1 with size+snapshot from AMIMetadata,
@@ -23,6 +23,7 @@ import (
viperblocklegacyv1 "github.com/mulgadc/spinifex/contracts/viperblockd/legacy/v1"
"github.com/mulgadc/spinifex/spinifex/bootstrap/config"
"github.com/mulgadc/spinifex/spinifex/domains/dns"
ebsencryption "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/encryption"
"github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/metadata"
ebspolicy "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/policy"
"github.com/mulgadc/spinifex/spinifex/domains/ec2/instancetypes"
@@ -37,7 +38,6 @@ import (
"github.com/mulgadc/spinifex/spinifex/providers/ebs"
"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
"github.com/mulgadc/spinifex/spinifex/runtime/compute/vm"
"github.com/mulgadc/spinifex/spinifex/utils"
"github.com/nats-io/nats.go"
)
@@ -1798,7 +1798,7 @@ func (s *InstanceServiceImpl) createRootVolumeViaProvider(ctx context.Context, s
// The control plane cannot see how a provider encrypts its volumes, but this
// shared config knob is the same one the legacy path derives Encrypted from.
mkey, err := utils.LoadViperblockMasterKey(s.config.Viperblock.EncryptionKeyFile)
encrypted, err := ebsencryption.Enabled(s.config.Viperblock.EncryptionKeyFile)
if err != nil {
slog.ErrorContext(ctx, "Could not load encryption key for root volume", "volumeId", spec.volumeID, "err", err)
return errors.New(awserrors.ErrorServerInternal)
@@ -1834,7 +1834,7 @@ func (s *InstanceServiceImpl) createRootVolumeViaProvider(ctx context.Context, s
CreatedAt: time.Now(), AvailabilityZone: s.config.AZ,
VolumeType: ebspolicy.VolumeTypeGP3, IOPS: rootVolumeIOPS(spec.iops),
Throughput: ebspolicy.DefaultGP3Throughput, SnapshotID: amiConfig.SnapshotID,
DeleteOnTermination: spec.deleteOnTermination, Encrypted: mkey != nil,
DeleteOnTermination: spec.deleteOnTermination, Encrypted: encrypted,
Tags: spec.tags,
ProviderHandle: created.Handle,
}); err != nil {
@@ -4188,7 +4188,7 @@ func TestPrepareRunInstances_PublicIPAllocFailureAbortsLaunch(t *testing.T) {
}
}
// natWirePayload mirrors utils.natEvent for test-side decoding.
// natWirePayload mirrors networkv1.NATEvent for test-side decoding.
type natWirePayload struct {
VpcId string `json:"vpc_id"`
ExternalIP string `json:"external_ip"`
+4 -4
View File
@@ -18,6 +18,7 @@ import (
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/mulgadc/bluebottle/pkg/safecast"
"github.com/mulgadc/spinifex/spinifex/bootstrap/config"
ebsencryption "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/encryption"
"github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/metadata"
ebspolicy "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/policy"
ec2instance "github.com/mulgadc/spinifex/spinifex/domains/ec2/instance"
@@ -28,7 +29,6 @@ import (
"github.com/mulgadc/spinifex/spinifex/providers/ebs"
"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
"github.com/mulgadc/spinifex/spinifex/runtime/compute/vm"
"github.com/mulgadc/spinifex/spinifex/utils"
"github.com/nats-io/nats.go"
"github.com/nats-io/nats.go/jetstream"
)
@@ -234,7 +234,7 @@ func (s *VolumeServiceImpl) CreateVolume(ctx context.Context, input *ec2.CreateV
// The control plane cannot see how a provider encrypts its volumes, but this
// shared config knob is the one the provider resolves the same way.
mkey, err := utils.LoadViperblockMasterKey(s.config.Viperblock.EncryptionKeyFile)
encrypted, err := ebsencryption.Enabled(s.config.Viperblock.EncryptionKeyFile)
if err != nil {
slog.ErrorContext(ctx, "CreateVolume: failed to load encryption key", "volumeId", volumeID, "err", err)
_ = s.provider.DeleteVolume(ctx, ebsprovider.DeleteVolumeRequest{Versioned: ebsprovider.NewVersioned(), VolumeID: volumeID, Handle: created.Handle})
@@ -244,7 +244,7 @@ func (s *VolumeServiceImpl) CreateVolume(ctx context.Context, input *ec2.CreateV
VolumeID: volumeID, TenantID: accountID, CapacityGiB: uint64(size), State: string(ebsprovider.VolumeStateAvailable),
CreatedAt: now, AvailabilityZone: *input.AvailabilityZone, VolumeType: volumeType,
IOPS: iops, Throughput: throughput, SnapshotID: snapshotID, Tags: tags, ProviderHandle: created.Handle,
Encrypted: mkey != nil,
Encrypted: encrypted,
}); err != nil {
slog.ErrorContext(ctx, "CreateVolume: failed to persist provider metadata", "volumeId", volumeID, "err", err)
_ = s.provider.DeleteVolume(ctx, ebsprovider.DeleteVolumeRequest{Versioned: ebsprovider.NewVersioned(), VolumeID: volumeID, Handle: created.Handle})
@@ -255,7 +255,7 @@ func (s *VolumeServiceImpl) CreateVolume(ctx context.Context, input *ec2.CreateV
return &ec2.Volume{VolumeId: aws.String(volumeID), Size: aws.Int64(size), VolumeType: aws.String(volumeType),
State: aws.String("available"), AvailabilityZone: input.AvailabilityZone, CreateTime: aws.Time(now),
Iops: aws.Int64(int64(iops)), Throughput: aws.Int64(int64(throughput)), Encrypted: aws.Bool(mkey != nil),
Iops: aws.Int64(int64(iops)), Throughput: aws.Int64(int64(throughput)), Encrypted: aws.Bool(encrypted),
SnapshotId: snapshotIDOrNil(snapshotID), Tags: awstags.MapToEC2(tags)}, nil
}
@@ -52,7 +52,7 @@ func newTestVolumeServiceWithEncryptionKey(az, keyFile string) *VolumeServiceImp
}
func TestCreateVolume_EncryptionKeyLoadError(t *testing.T) {
// Point EncryptionKeyFile at a non-existent path so LoadViperblockMasterKey
// Point EncryptionKeyFile at a non-existent path so ebsencryption.Enabled
// fails — CreateVolume must abort with ServerInternal and roll the
// provider's allocation back rather than leave an orphaned volume.
missing := filepath.Join(t.TempDir(), "absent.key")
@@ -37,7 +37,7 @@ func TestS7_NATSSubjectsCarryAZPrefix(t *testing.T) {
filepath.Join(repoRoot(t), "spinifex", "daemon"),
filepath.Join(repoRoot(t), "spinifex", "handlers"),
filepath.Join(repoRoot(t), "internal", "testkit"),
filepath.Join(repoRoot(t), "spinifex", "utils"),
filepath.Join(repoRoot(t), "contracts", "network"),
}
type hit struct {
+1 -1
View File
@@ -607,7 +607,7 @@ func TestBuildWeightsImage_CreateFileErrorSkipsRunner(t *testing.T) {
// fakeClusterConfig returns a minimal single-node ClusterConfig sufficient
// to drive the ochre weights Run wrappers: all nested config structs are
// value types defaulting to the zero value, which every collaborator on the
// refusal paths (LoadViperblockMasterKey, NewS3ObjectStoreFromConfig) treats
// refusal paths (masterkey.ReadShared, NewS3ObjectStoreFromConfig) treats
// as "unset", not invalid.
func fakeClusterConfig() *config.ClusterConfig {
return &config.ClusterConfig{
-35
View File
@@ -1,35 +0,0 @@
// Package utils holds the helpers still shared across Spinifex services: the
// Viperblock master key loader and vpcd events.
package utils
import (
"fmt"
"sync"
"github.com/mulgadc/bluebottle/pkg/masterkey"
)
// viperblockKeyCache memoises masterkey.LoadShared by path. The *Key holds an AEAD safe for concurrent use.
var (
viperblockKeyCacheMu sync.Mutex
viperblockKeyCache = map[string]*masterkey.Key{}
)
// LoadViperblockMasterKey returns the cached *masterkey.Key for path, loading on first use.
// An empty path returns (nil, nil), meaning encryption is disabled.
func LoadViperblockMasterKey(path string) (*masterkey.Key, error) {
if path == "" {
return nil, nil
}
viperblockKeyCacheMu.Lock()
defer viperblockKeyCacheMu.Unlock()
if k, ok := viperblockKeyCache[path]; ok {
return k, nil
}
k, err := masterkey.LoadShared(path)
if err != nil {
return nil, fmt.Errorf("load viperblock encryption key %s: %w", path, err)
}
viperblockKeyCache[path] = k
return k, nil
}
+2 -2
View File
@@ -147,11 +147,11 @@ func StartDaemonLite(t *testing.T, gw *Gateway, opts ...DaemonLiteOption) *Daemo
// vpcd (the OVN topology-translation daemon) to provision/tear down each
// VPC's default security group (domains/ec2/vpc/security_group.go
// createDefaultSecurityGroupInternal/deleteSecurityGroupInternal ->
// requestSGEvent -> utils.RequestEvent). vpcd itself is out of scope for
// requestSGEvent -> projection.Client). vpcd itself is out of scope for
// this tier (it's an external OVN process, not a key/tags/routetable/vpc
// service impl), so it is stubbed here exactly like any other
// out-of-scope daemon-side responder: a fixed {"success":true} ack on
// "vpc.create-sg"/"vpc.delete-sg", satisfying utils.RequestEvent's
// "vpc.create-sg"/"vpc.delete-sg", satisfying the projection client's
// {success,error} reply contract. The SG record itself is written to the
// KV store by the real service impl before this event is even sent, so
// stubbing the vpcd ack never substitutes for in-scope logic under test —
+1 -1
View File
@@ -14,7 +14,7 @@ import (
// TestVPCD_SecurityGroupReachesOVN asserts the synchronous half of the
// handler-to-vpcd contract against a real responder. The SG topics use
// utils.RequestEvent, so unlike the topology events these need no polling: by
// the network projection client, so unlike the topology events these need no polling: by
// the time the SDK call returns, vpcd has already committed to OVN.
//
// This is the case the tier previously faked outright with a canned