From ec9e8d70552d9967632346b4aafbd1f9c528e3f0 Mon Sep 17 00:00:00 2001 From: naxty <9141879+naxty@users.noreply.github.com> Date: Fri, 28 Aug 2026 11:33:17 +0200 Subject: [PATCH 1/6] feat(storage): model uniform bucket-level access and hierarchical namespace Groundwork for managed folders and folders, neither of which is registered yet. GCS refuses to create a managed folder in a bucket that still has per-object ACLs, and a bucket is created flat or hierarchical and cannot convert - so both settings have to be declarable before either type can have a fixture. Both are additive and optional, with hasProviderDefault on the fields GCS fills in itself (publicAccessPrevention, lockedTime) so an unset-but-returned value is not read as drift. The existing bucket fixture still evaluates. --- schema/pkl/storage/bucket.pkl | 46 +++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/schema/pkl/storage/bucket.pkl b/schema/pkl/storage/bucket.pkl index 6905cf39..93f676c4 100644 --- a/schema/pkl/storage/bucket.pkl +++ b/schema/pkl/storage/bucket.pkl @@ -103,6 +103,37 @@ open class BucketResolvable extends formae.Resolvable { } } +/// Whether ACLs are disabled in favour of IAM alone. Managed folders require +/// this to be enabled — GCS refuses to create one in a bucket that still has +/// per-object ACLs. +@gcp.SubResourceHint +open class UniformBucketLevelAccess extends formae.SubResource { + @gcp.FieldHint + enabled: Boolean? + + /// When the setting stops being reversible. Set by GCS. + @gcp.FieldHint { hasProviderDefault = true } + lockedTime: String? +} + +@gcp.SubResourceHint +open class IamConfiguration extends formae.SubResource { + @gcp.FieldHint + uniformBucketLevelAccess: UniformBucketLevelAccess? + + /// GCS fills this in ("inherited" unless set). + @gcp.FieldHint { hasProviderDefault = true } + publicAccessPrevention: String? +} + +/// Gives the bucket real folders rather than name prefixes. Fixed at creation: +/// an existing flat bucket cannot be converted. +@gcp.SubResourceHint +open class HierarchicalNamespace extends formae.SubResource { + @gcp.FieldHint + enabled: Boolean? +} + @gcp.ResourceHint { type = module.type identifier = "name" @@ -149,6 +180,21 @@ open class Bucket extends formae.Resource { } name: String + /// How access is controlled. GCS always reports this, so it is a provider + /// default when a forma says nothing; set + /// `uniformBucketLevelAccess.enabled` to turn ACLs off, which managed + /// folders require. + @gcp.FieldHint { hasProviderDefault = true } + iamConfiguration: IamConfiguration? + + /// Enables real folders. Immutable — a bucket is created flat or + /// hierarchical and cannot change afterwards. + @gcp.FieldHint { + createOnly = true + hasProviderDefault = true + } + hierarchicalNamespace: HierarchicalNamespace? + @gcp.FieldHint owner: Owner? From 518bc00fc2728fb4a8be10c8d409a16aee372c79 Mon Sep 17 00:00:00 2001 From: naxty <9141879+naxty@users.noreply.github.com> Date: Fri, 28 Aug 2026 11:53:01 +0200 Subject: [PATCH 2/6] feat(storage): add ManagedFolder and Folder Both are named with a trailing slash that is part of the identity - "reports/" is a folder, "reports" is not - and the storage native-ID parser took a single path segment, so the slash was dropped and the rebuilt URL addressed something that does not exist. The name is now taken whole and escaped when addressed. That change is deliberately behaviour-preserving for what already worked: for an ACL entity, a default object ACL or an anywhere cache the name is one segment, so joining the remainder returns the same string and PathEscape is a no-op. Unit tests pin both halves, because the pre-existing storage tests are integration-tagged and do not run without real GCP - the existing bucket case is included in the CI run for the same reason. The schema states the trailing slash as a constraint rather than a comment, so a fixture that omits it fails at eval instead of after creating a bucket. A managed folder needs uniform bucket-level access and a folder needs a hierarchical namespace, so Bucket now models both. --- CHANGELOG.md | 22 +++++ pkg/resources/storage/api.go | 13 ++- pkg/resources/storage/resources.go | 61 ++++++++++++ .../storage/storage_native_id_unit_test.go | 92 +++++++++++++++++++ schema/pkl/storage/folder.pkl | 62 +++++++++++++ schema/pkl/storage/managed_folder.pkl | 62 +++++++++++++ testdata/storage-folder.pkl | 42 +++++++++ testdata/storage-managed-folder.pkl | 40 ++++++++ 8 files changed, 392 insertions(+), 2 deletions(-) create mode 100644 pkg/resources/storage/storage_native_id_unit_test.go create mode 100644 schema/pkl/storage/folder.pkl create mode 100644 schema/pkl/storage/managed_folder.pkl create mode 100644 testdata/storage-folder.pkl create mode 100644 testdata/storage-managed-folder.pkl diff --git a/CHANGELOG.md b/CHANGELOG.md index db845d9b..174df83b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,28 @@ formae agent. ## [Unreleased] +### Added + +- `GCP::Storage::ManagedFolder` — an IAM boundary inside a bucket, letting a + policy be attached to a prefix without granting it over the whole bucket. + Requires uniform bucket-level access. +- `GCP::Storage::Folder` — a real directory node, available only in a bucket + created with a hierarchical namespace. Renaming one moves everything beneath + it, where a managed folder only governs who may read a prefix. +- `GCP::Storage::Bucket` models `iamConfiguration.uniformBucketLevelAccess` and + `hierarchicalNamespace`. Neither folder type can exist without them, and + hierarchical namespace is fixed at creation — a bucket is created flat or + hierarchical and cannot convert. + +### Fixed + +- Storage names containing a slash survive a native-ID round trip. Both folder + types are named with a **trailing slash that is part of the identity** + ("reports/" is not "reports"), and the parser took a single path segment, so + the slash was dropped and the rebuilt URL addressed a folder that does not + exist. The name is now taken whole and escaped when addressed — a no-op for + every pre-existing storage name, none of which contains a slash. + ### Fixed - A `GCP::Compute::DiskAsyncReplication` no longer plans a replacement of itself. diff --git a/pkg/resources/storage/api.go b/pkg/resources/storage/api.go index f1c56e97..f5fcae3e 100644 --- a/pkg/resources/storage/api.go +++ b/pkg/resources/storage/api.go @@ -6,6 +6,7 @@ package storage import ( "fmt" + "net/url" "strings" "github.com/platform-engineering-labs/formae-plugin-gcp/pkg/resources/base" @@ -77,7 +78,10 @@ func storagePathBuilder(ctx base.PathContext) string { // Bucket-scoped bucketName := bucketInfo if ctx.ResourceName != "" { - return fmt.Sprintf("/b/%s/%s/%s", bucketName, ctx.ResourceType, ctx.ResourceName) + // A name containing a slash - a managed folder or a folder - has to be + // escaped, or the slash reads as another path segment. PathEscape is a + // no-op for every other storage name, which contains no slash. + return fmt.Sprintf("/b/%s/%s/%s", bucketName, ctx.ResourceType, url.PathEscape(ctx.ResourceName)) } return fmt.Sprintf("/b/%s/%s", bucketName, ctx.ResourceType) } @@ -153,7 +157,12 @@ func parseStorageNativeID(nativeID string) (base.PathContext, error) { if len(parts) >= 4 && parts[0] == "b" && parts[2] != "o" { ctx.ParentResource = parts[1] // bucket name ctx.ResourceType = parts[2] - ctx.ResourceName = parts[3] + // Everything after the collection is the name. For an ACL entity or an + // anywhere cache that is a single segment and this is exactly parts[3]; + // a managed folder or a folder is named with a trailing slash that is + // part of its identity ("reports/" is not "reports"), and taking one + // segment would silently truncate it. + ctx.ResourceName = strings.Join(parts[3:], "/") return ctx, nil } diff --git a/pkg/resources/storage/resources.go b/pkg/resources/storage/resources.go index dc22b1a8..8f95f40a 100644 --- a/pkg/resources/storage/resources.go +++ b/pkg/resources/storage/resources.go @@ -18,6 +18,8 @@ const ( AnywhereCacheResourceType = "GCP::Storage::AnywhereCache" BucketAccessControlResourceType = "GCP::Storage::BucketAccessControl" DefaultObjectAccessControlResourceType = "GCP::Storage::DefaultObjectAccessControl" + ManagedFolderResourceType = "GCP::Storage::ManagedFolder" + FolderResourceType = "GCP::Storage::Folder" ObjectAccessControlResourceType = "GCP::Storage::ObjectAccessControl" ) @@ -144,6 +146,47 @@ func init() { RequestTransformer: wrapBodyBuilder(aclBodyBuilder), ResponseTransformer: nil, }, + { + // A managed folder is an IAM boundary inside a bucket: it lets a + // policy be attached to a prefix without giving it to the whole + // bucket. It requires uniform bucket-level access - GCS refuses to + // create one where per-object ACLs still apply. + // + // Its name ends with a slash, which is part of its identity and is + // escaped in the URL. + ResourceType: ManagedFolderResourceType, + ResourceConfig: base.ResourceConfig{ + ResourceType: "managedFolders", + ParentResource: &base.ParentResourceConfig{ + ParentType: "bucket", + RequiresParent: true, + ParentPathSegments: []string{"b"}, + }, + // A managed folder carries nothing but its name; there is + // nothing an update could change. + SupportsUpdate: false, + }, + RequestTransformer: base.DropFields("bucket"), + ResponseTransformer: base.ResponseTransformerFunc(bucketScopedResponseTransformer), + }, + { + // A folder is a real directory, available only in a bucket created + // with a hierarchical namespace. Where a managed folder is an IAM + // boundary over a prefix, a folder is an actual node - renaming one + // moves everything beneath it. + ResourceType: FolderResourceType, + ResourceConfig: base.ResourceConfig{ + ResourceType: "folders", + ParentResource: &base.ParentResourceConfig{ + ParentType: "bucket", + RequiresParent: true, + ParentPathSegments: []string{"b"}, + }, + SupportsUpdate: false, + }, + RequestTransformer: base.DropFields("bucket"), + ResponseTransformer: base.ResponseTransformerFunc(bucketScopedResponseTransformer), + }, // NOTE: ObjectAccessControlResourceType requires special handling for object-scoped resources // The base package currently doesn't support resources that need TWO parent properties (bucket + object). // This resource type is commented out pending enhancement to base package's parent extraction mechanism. @@ -173,3 +216,21 @@ func init() { panic(err) } } + +// bucketScopedResponseTransformer puts back the bucket a folder belongs to and +// drops what GCS echoes that describes the request rather than the resource. +// Both folder types report "bucket" themselves, so unlike most nested resources +// here nothing has to be recovered from the URL. +func bucketScopedResponseTransformer( + props map[string]interface{}, _ base.TransformContext, +) map[string]interface{} { + out := make(map[string]interface{}, len(props)) + for k, v := range props { + switch k { + case "kind", "selfLink", "metageneration", "id": + continue + } + out[k] = v + } + return out +} diff --git a/pkg/resources/storage/storage_native_id_unit_test.go b/pkg/resources/storage/storage_native_id_unit_test.go new file mode 100644 index 00000000..2a90f7db --- /dev/null +++ b/pkg/resources/storage/storage_native_id_unit_test.go @@ -0,0 +1,92 @@ +// © 2025 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +//go:build unit + +package storage + +import ( + "testing" + + "github.com/platform-engineering-labs/formae-plugin-gcp/pkg/resources/base" +) + +// The names of every storage type that existed before managed folders are a +// single segment. These pin that the parser and the path builder still treat +// them exactly as they did - this is the half of the change that must not move. +func TestExistingStorageNamesAreUnchanged(t *testing.T) { + cases := map[string]struct { + nativeID string + wantPath string + }{ + "bucket acl entity": { + "b/my-bucket/acl/user-someone@example.com", + "/b/my-bucket/acl/user-someone@example.com", + }, + "default object acl": { + "b/my-bucket/defaultObjectAcl/allUsers", + "/b/my-bucket/defaultObjectAcl/allUsers", + }, + "anywhere cache": { + "b/my-bucket/anywhereCaches/cache-1", + "/b/my-bucket/anywhereCaches/cache-1", + }, + } + for name, tc := range cases { + ctx, err := parseStorageNativeID(tc.nativeID) + if err != nil { + t.Errorf("%s: parse: %v", name, err) + continue + } + if got := storagePathBuilder(ctx); got != tc.wantPath { + t.Errorf("%s: path = %q, want %q", name, got, tc.wantPath) + } + } +} + +// A managed folder is named with a trailing slash that is part of its identity: +// "reports/" is not "reports". Splitting on "/" and taking one segment dropped +// it, and the rebuilt URL then addressed a folder that does not exist. +func TestSlashTerminatedNamesSurviveAndAreEscaped(t *testing.T) { + ctx, err := parseStorageNativeID("b/my-bucket/managedFolders/reports/") + if err != nil { + t.Fatalf("parse: %v", err) + } + if ctx.ParentResource != "my-bucket" || ctx.ResourceType != "managedFolders" { + t.Errorf("ctx = %+v", ctx) + } + if ctx.ResourceName != "reports/" { + t.Errorf("name = %q, want %q", ctx.ResourceName, "reports/") + } + // The slash has to be escaped or it reads as another path segment. + want := "/b/my-bucket/managedFolders/reports%2F" + if got := storagePathBuilder(ctx); got != want { + t.Errorf("path = %q, want %q", got, want) + } + + // Nested folders keep every segment. + nested, err := parseStorageNativeID("b/my-bucket/folders/a/b/") + if err != nil { + t.Fatalf("parse nested: %v", err) + } + if nested.ResourceName != "a/b/" { + t.Errorf("nested name = %q", nested.ResourceName) + } +} + +// A bucket is still addressed by a bare name, and a project-scoped resource +// still by its project path. +func TestBucketAndProjectScopedShapesStillParse(t *testing.T) { + b, err := parseStorageNativeID("my-bucket") + if err != nil || b.ResourceType != "b" || b.ResourceName != "my-bucket" { + t.Errorf("bucket ctx = %+v err=%v", b, err) + } + p, err := parseStorageNativeID("projects/proj/hmacKeys/GOOG1EXAMPLE") + if err != nil || p.ResourceType != "hmacKeys" || p.ResourceName != "GOOG1EXAMPLE" || p.ParentResource != "" { + t.Errorf("project ctx = %+v err=%v", p, err) + } + if got := storagePathBuilder(base.PathContext{Project: "proj", ResourceType: "hmacKeys", ResourceName: "GOOG1EXAMPLE"}); got != "/projects/proj/hmacKeys/GOOG1EXAMPLE" { + t.Errorf("project path = %q", got) + } +} diff --git a/schema/pkl/storage/folder.pkl b/schema/pkl/storage/folder.pkl new file mode 100644 index 00000000..ae3ee775 --- /dev/null +++ b/schema/pkl/storage/folder.pkl @@ -0,0 +1,62 @@ +/* + * © 2025 Platform Engineering Labs Inc. + * + * SPDX-License-Identifier: FSL-1.1-ALv2 + */ + +/// GCP Storage Managed Folder +/// +/// An IAM boundary inside a bucket: it lets a policy be attached to a prefix +/// without granting it over the whole bucket. The objects themselves are +/// unaffected — a managed folder is about who may read them, not where they +/// live. +/// +/// Requires uniform bucket-level access on the bucket; GCS refuses to create +/// one where per-object ACLs still apply. +module gcp.storage.folder + +import "../gcp.pkl" +import "@formae/formae.pkl" + +const type = "GCP::Storage::Folder" + +open class FolderResolvable extends formae.Resolvable { + hidden type = module.type + + hidden name: FolderResolvable = (this) { + property = "name" + } +} + +@gcp.ResourceHint { + type = module.type + identifier = "name" +} +open class Folder extends formae.Resource { + hidden parent = this + + /// The folder's name, which **must end with a slash** — "reports/" is a + /// managed folder, "reports" is not. The trailing slash is part of the + /// identity, not decoration, and is escaped when the resource is addressed. + /// Immutable. + @gcp.FieldHint { createOnly = true } + name: String(endsWith("/")) + + /// Name of the owning bucket. A path component, not a body field — pass + /// `bkt.res.name` so formae creates the bucket first. + @gcp.FieldHint { createOnly = true } + bucket: (String|formae.Resolvable) + + /// When GCS created the folder. + @gcp.FieldHint { hasProviderDefault = true } + createTime: String? + + /// When GCS last updated it. + @gcp.FieldHint { hasProviderDefault = true } + updateTime: String? + + hidden res: FolderResolvable = new { + label = parent.label + stack = parent.stack?.label + } +} diff --git a/schema/pkl/storage/managed_folder.pkl b/schema/pkl/storage/managed_folder.pkl new file mode 100644 index 00000000..0fa06dba --- /dev/null +++ b/schema/pkl/storage/managed_folder.pkl @@ -0,0 +1,62 @@ +/* + * © 2025 Platform Engineering Labs Inc. + * + * SPDX-License-Identifier: FSL-1.1-ALv2 + */ + +/// GCP Storage Managed Folder +/// +/// An IAM boundary inside a bucket: it lets a policy be attached to a prefix +/// without granting it over the whole bucket. The objects themselves are +/// unaffected — a managed folder is about who may read them, not where they +/// live. +/// +/// Requires uniform bucket-level access on the bucket; GCS refuses to create +/// one where per-object ACLs still apply. +module gcp.storage.managed_folder + +import "../gcp.pkl" +import "@formae/formae.pkl" + +const type = "GCP::Storage::ManagedFolder" + +open class ManagedFolderResolvable extends formae.Resolvable { + hidden type = module.type + + hidden name: ManagedFolderResolvable = (this) { + property = "name" + } +} + +@gcp.ResourceHint { + type = module.type + identifier = "name" +} +open class ManagedFolder extends formae.Resource { + hidden parent = this + + /// The folder's name, which **must end with a slash** — "reports/" is a + /// managed folder, "reports" is not. The trailing slash is part of the + /// identity, not decoration, and is escaped when the resource is addressed. + /// Immutable. + @gcp.FieldHint { createOnly = true } + name: String(endsWith("/")) + + /// Name of the owning bucket. A path component, not a body field — pass + /// `bkt.res.name` so formae creates the bucket first. + @gcp.FieldHint { createOnly = true } + bucket: (String|formae.Resolvable) + + /// When GCS created the folder. + @gcp.FieldHint { hasProviderDefault = true } + createTime: String? + + /// When GCS last updated it. + @gcp.FieldHint { hasProviderDefault = true } + updateTime: String? + + hidden res: ManagedFolderResolvable = new { + label = parent.label + stack = parent.stack?.label + } +} diff --git a/testdata/storage-folder.pkl b/testdata/storage-folder.pkl new file mode 100644 index 00000000..173a69cb --- /dev/null +++ b/testdata/storage-folder.pkl @@ -0,0 +1,42 @@ +/* + * © 2025 Platform Engineering Labs Inc. + * + * SPDX-License-Identifier: FSL-1.1-ALv2 + */ + +amends "@formae/forma.pkl" + +import "@gcp/storage/bucket.pkl" +import "@gcp/storage/folder.pkl" as folder +import "./config/vars.pkl" as v + +// A folder is a real directory node, so its bucket must be created with a +// hierarchical namespace - which also requires uniform bucket-level access, and +// cannot be turned on afterwards. +local bkt = new bucket.Bucket { + label = "plugin-sdk-test-folder-bucket" + name = "formae-plugin-sdk-test-fld-\(v.testRunID)" + location = v.gcpRegion + storageClass = "STANDARD" + iamConfiguration = new bucket.IamConfiguration { + uniformBucketLevelAccess = new bucket.UniformBucketLevelAccess { + enabled = true + } + } + hierarchicalNamespace = new bucket.HierarchicalNamespace { + enabled = true + } +} + +forma { + v.stack + v.target + + bkt + + new folder.Folder { + label = "plugin-sdk-test-storage-folder" + name = "formae-data/" + bucket = bkt.res.name + } +} diff --git a/testdata/storage-managed-folder.pkl b/testdata/storage-managed-folder.pkl new file mode 100644 index 00000000..a6347120 --- /dev/null +++ b/testdata/storage-managed-folder.pkl @@ -0,0 +1,40 @@ +/* + * © 2025 Platform Engineering Labs Inc. + * + * SPDX-License-Identifier: FSL-1.1-ALv2 + */ + +amends "@formae/forma.pkl" + +import "@gcp/storage/bucket.pkl" +import "@gcp/storage/managed_folder.pkl" as managedFolder +import "./config/vars.pkl" as v + +// A managed folder needs uniform bucket-level access: GCS refuses to create one +// in a bucket where per-object ACLs still apply. An empty bucket stores nothing +// and costs nothing. +local bkt = new bucket.Bucket { + label = "plugin-sdk-test-mf-bucket" + name = "formae-plugin-sdk-test-mf-\(v.testRunID)" + location = v.gcpRegion + storageClass = "STANDARD" + iamConfiguration = new bucket.IamConfiguration { + uniformBucketLevelAccess = new bucket.UniformBucketLevelAccess { + enabled = true + } + } +} + +forma { + v.stack + v.target + + bkt + + // The trailing slash is part of the name, not decoration. + new managedFolder.ManagedFolder { + label = "plugin-sdk-test-storage-managed-folder" + name = "formae-reports/" + bucket = bkt.res.name + } +} From 4527edff9e659b81ca12c8bb807095f7366e1b8a Mon Sep 17 00:00:00 2001 From: naxty <9141879+naxty@users.noreply.github.com> Date: Fri, 28 Aug 2026 12:14:03 +0200 Subject: [PATCH 3/6] fix(storage): discover folders by walking the buckets Discovery scanned GCP::Storage::ManagedFolder on every pass and never received a single result. With no properties it can name no bucket, so the path builder fell through to its project-scoped branch and asked /projects/{p}/managedFolders - a URL that addresses nothing. GCS has no wildcard for the bucket segment. The out-of-band create had already proved the rest works: the folder was created with native ID b//managedFolders/formae-reports/, trailing slash intact. Sixth copy of this shape in the plugin - Service Directory, Spanner, Cloud SQL, DNS, Bigtable and now Storage. Six packages hand-writing the same walk because the generic List cannot find a nested resource GCP offers no wildcard for. It belongs in base; the comment says so. --- pkg/resources/storage/bucket_walking_list.go | 115 +++++++++++++++++++ pkg/resources/storage/resources.go | 2 + 2 files changed, 117 insertions(+) create mode 100644 pkg/resources/storage/bucket_walking_list.go diff --git a/pkg/resources/storage/bucket_walking_list.go b/pkg/resources/storage/bucket_walking_list.go new file mode 100644 index 00000000..18efa500 --- /dev/null +++ b/pkg/resources/storage/bucket_walking_list.go @@ -0,0 +1,115 @@ +// © 2025 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +package storage + +import ( + "context" + "fmt" + + "github.com/platform-engineering-labs/formae-plugin-gcp/pkg/config" + "github.com/platform-engineering-labs/formae-plugin-gcp/pkg/resources/prov" + "github.com/platform-engineering-labs/formae-plugin-gcp/pkg/resources/registry" + "github.com/platform-engineering-labs/formae-plugin-gcp/pkg/transport" + "github.com/platform-engineering-labs/formae-plugin-gcp/pkg/utils" + "github.com/platform-engineering-labs/formae/pkg/plugin/resource" +) + +// Both folder types live in a bucket, and discovery lists with no properties - +// so it can name no bucket, the path builder falls through to the +// project-scoped branch and asks /projects/{p}/managedFolders, which addresses +// nothing. GCS has no wildcard for the bucket segment, so the only way to +// discover either is to walk the buckets. +// +// This is the sixth copy of this shape in the plugin (Service Directory, +// Spanner, Cloud SQL, DNS, Bigtable and now Storage). It belongs in base. +type bucketWalkingListProvisioner struct { + prov.Provisioner + cfg *config.Config + collection string +} + +// registerBucketWalkingLists is called from the package init in resources.go so +// the generic registration is guaranteed to have landed first. +func registerBucketWalkingLists() { + for _, spec := range []struct { + resourceType string + collection string + }{ + {ManagedFolderResourceType, "managedFolders"}, + {FolderResourceType, "folders"}, + } { + spec := spec + registry.Register(spec.resourceType, + []resource.Operation{resource.OperationList}, + func(cfg *config.Config) prov.Provisioner { + return &bucketWalkingListProvisioner{ + Provisioner: storageRegistry.CreateProvisioner(cfg, spec.resourceType), + cfg: cfg, + collection: spec.collection, + } + }) + } +} + +func (p *bucketWalkingListProvisioner) List( + ctx context.Context, request *resource.ListRequest, +) (*resource.ListResult, error) { + // A named bucket is the caller telling us where to look. + if request.AdditionalProperties != nil && request.AdditionalProperties["bucket"] != "" { + return p.Provisioner.List(ctx, request) + } + + cfg := config.PathFromTargetConfig(request.TargetConfig) + if cfg.Project == "" { + return &resource.ListResult{NativeIDs: []string{}}, nil + } + + client, err := transport.NewClient(ctx, p.cfg) + if err != nil { + return nil, fmt.Errorf("failed to create transport client: %w", err) + } + + bucketsURL := fmt.Sprintf("%s/b?project=%s", StorageAPI.BaseURL, cfg.Project) + resp, err := client.SendRequest(ctx, transport.RequestOptions{Method: "GET", URL: bucketsURL}) + if err != nil { + wrapped := transport.WrapError(err, "failed to list storage buckets") + return nil, fmt.Errorf("%s", wrapped.Message) + } + + nativeIDs := []string{} + buckets, _ := resp.Body["items"].([]interface{}) + for _, raw := range buckets { + bucket, ok := raw.(map[string]interface{}) + if !ok { + continue + } + bucketName := utils.GetString(bucket, "name") + if bucketName == "" { + continue + } + itemsResp, listErr := client.SendRequest(ctx, transport.RequestOptions{ + Method: "GET", + URL: fmt.Sprintf("%s/b/%s/%s", StorageAPI.BaseURL, bucketName, p.collection), + }) + if listErr != nil { + // A flat bucket has no folders and a bucket with ACLs has no managed + // folders; neither is a reason to hide the rest. + continue + } + items, _ := itemsResp.Body["items"].([]interface{}) + for _, rawItem := range items { + item, ok := rawItem.(map[string]interface{}) + if !ok { + continue + } + // The name carries its trailing slash, which the native ID keeps. + if name := utils.GetString(item, "name"); name != "" { + nativeIDs = append(nativeIDs, + fmt.Sprintf("b/%s/%s/%s", bucketName, p.collection, name)) + } + } + } + return &resource.ListResult{NativeIDs: nativeIDs}, nil +} diff --git a/pkg/resources/storage/resources.go b/pkg/resources/storage/resources.go index 8f95f40a..47919fb7 100644 --- a/pkg/resources/storage/resources.go +++ b/pkg/resources/storage/resources.go @@ -215,6 +215,8 @@ func init() { if err != nil { panic(err) } + + registerBucketWalkingLists() } // bucketScopedResponseTransformer puts back the bucket a folder belongs to and From 5572d445c931134104d6cbba133eac9e1976bf8c Mon Sep 17 00:00:00 2001 From: naxty <9141879+naxty@users.noreply.github.com> Date: Fri, 28 Aug 2026 12:23:53 +0200 Subject: [PATCH 4/6] fix(storage): point the bucket resolvable at properties that exist BucketResolvable named "Id", "SelfLink" and "Name". The resource declares id and name in lower case and has no selfLink at all, so bkt.res.name resolved to no value and any resource referencing a bucket reached the plugin with the reference unresolved - which is why the folder cases failed at Create in an apply while succeeding in the out-of-band path, where the harness resolves references itself. Nothing in the repository referenced a bucket until these folder types did, so the defect has been latent since the resolvable was written. selfLink is removed rather than corrected: there is no such property to point at. --- CHANGELOG.md | 6 ++++++ schema/pkl/storage/bucket.pkl | 16 ++++++++++------ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 174df83b..e19d8288 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,12 @@ formae agent. ### Fixed +- `GCP::Storage::Bucket`'s resolvable names properties the resource actually + has. It pointed at `"Id"`, `"SelfLink"` and `"Name"` — capitalised, matching + nothing — so `bkt.res.name` resolved to no value and **any resource + referencing a bucket reached the plugin with the reference unresolved**. It + went unnoticed because nothing in the repository referenced a bucket until the + folder types did. `selfLink` is removed: the bucket has no such property. - Storage names containing a slash survive a native-ID round trip. Both folder types are named with a **trailing slash that is part of the identity** ("reports/" is not "reports"), and the parser took a single path segment, so diff --git a/schema/pkl/storage/bucket.pkl b/schema/pkl/storage/bucket.pkl index 93f676c4..906f7983 100644 --- a/schema/pkl/storage/bucket.pkl +++ b/schema/pkl/storage/bucket.pkl @@ -87,19 +87,23 @@ open class RetentionPolicy extends formae.SubResource { retentionPeriod: Int? } +/// Every entry here names a property this resource actually declares. +/// +/// They previously read "Id", "SelfLink" and "Name" — capitalised, matching +/// nothing — so `bkt.res.name` resolved to no value and any resource +/// referencing a bucket reached the plugin with the reference unresolved. It +/// went unnoticed because nothing in the repository referenced a bucket until +/// the folder types did. `selfLink` is gone with them: the bucket has no such +/// property to point at. open class BucketResolvable extends formae.Resolvable { hidden type = module.type hidden id: BucketResolvable = (this) { - property = "Id" - } - - hidden selfLink: BucketResolvable = (this) { - property = "SelfLink" + property = "id" } hidden name: BucketResolvable = (this) { - property = "Name" + property = "name" } } From 47da6d36cd8c01f6840d2353d5b16b6c9bcde18e Mon Sep 17 00:00:00 2001 From: naxty <9141879+naxty@users.noreply.github.com> Date: Fri, 28 Aug 2026 14:54:48 +0200 Subject: [PATCH 5/6] feat(storage): add HmacKey The credential a service account presents to the S3-compatible XML API, and the last Storage type that needs no prerequisite beyond a service account. Three things the generic engine cannot express, so create and delete are hand-written: - the key is created by naming its service account in a query parameter, with no request body at all; - the create answers {metadata, secret} while a get answers the metadata directly, so reads would otherwise disagree with creates on every field; - GCS refuses to delete a key that is still ACTIVE, so delete reads the key, PUTs state=INACTIVE if needed, and only then deletes. The secret is dropped rather than stored: it is returned once and never again, so keeping it would both persist a live credential and guarantee drift on the next read. testdata/config/vars.pkl now exposes gcpProjectNumber. The default compute service account's email is built from the project number rather than its id, and every workflow that runs conformance already sets GCP_PROJECT_NUMBER - checked, because vars.pkl is imported by every fixture and a missing variable would break all of them. --- CHANGELOG.md | 5 + pkg/resources/storage/hmac_key.go | 191 ++++++++++++++++++++++ pkg/resources/storage/hmac_key_helpers.go | 57 +++++++ pkg/resources/storage/resources.go | 17 ++ schema/pkl/storage/hmac_key.pkl | 70 ++++++++ testdata/config/vars.pkl | 3 + testdata/storage-hmac-key.pkl | 24 +++ 7 files changed, 367 insertions(+) create mode 100644 pkg/resources/storage/hmac_key.go create mode 100644 pkg/resources/storage/hmac_key_helpers.go create mode 100644 schema/pkl/storage/hmac_key.pkl create mode 100644 testdata/storage-hmac-key.pkl diff --git a/CHANGELOG.md b/CHANGELOG.md index e19d8288..1487f3e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,11 @@ formae agent. ### Added +- `GCP::Storage::HmacKey` — the credential a service account presents to the + S3-compatible XML API. GCS assigns the `accessId` and returns the secret + exactly once; the plugin drops it rather than persisting a live credential in + stored state. Deleting a key requires deactivating it first, which the + provisioner does — GCS refuses to delete an active key. - `GCP::Storage::ManagedFolder` — an IAM boundary inside a bucket, letting a policy be attached to a prefix without granting it over the whole bucket. Requires uniform bucket-level access. diff --git a/pkg/resources/storage/hmac_key.go b/pkg/resources/storage/hmac_key.go new file mode 100644 index 00000000..c9a337e2 --- /dev/null +++ b/pkg/resources/storage/hmac_key.go @@ -0,0 +1,191 @@ +// © 2025 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +package storage + +import ( + "context" + "fmt" + + "github.com/platform-engineering-labs/formae-plugin-gcp/pkg/config" + "github.com/platform-engineering-labs/formae-plugin-gcp/pkg/resources/base" + "github.com/platform-engineering-labs/formae-plugin-gcp/pkg/resources/prov" + "github.com/platform-engineering-labs/formae-plugin-gcp/pkg/resources/registry" + "github.com/platform-engineering-labs/formae-plugin-gcp/pkg/transport" + "github.com/platform-engineering-labs/formae-plugin-gcp/pkg/utils" + "github.com/platform-engineering-labs/formae/pkg/plugin/resource" +) + +// hmacKeyDeactivatedState is the only state GCS will delete a key from. An +// active key answers a delete with 400: it has to be deactivated first, which +// is a separate PUT. +const hmacKeyDeactivatedState = "INACTIVE" + +// hmacKeyRequestTransformer builds the create body. A key is created by naming +// the service account it belongs to in a query parameter, not in a body - the +// request carries no body at all - so this drops everything and lets +// CreateIDParam-style handling happen in the provisioner. +func hmacKeyRequestTransformer( + props map[string]interface{}, _ base.TransformContext, +) (map[string]interface{}, error) { + return map[string]interface{}{}, nil +} + +// hmacKeyResponseTransformer lifts the metadata out of the create envelope and +// drops the secret. +// +// create answers {"metadata": {...}, "secret": "..."} while get answers the +// metadata directly, so reads would otherwise disagree with creates on every +// field. The secret is returned exactly once and never again: keeping it would +// both persist a credential and guarantee drift on the next read. +func hmacKeyResponseTransformer( + props map[string]interface{}, _ base.TransformContext, +) map[string]interface{} { + metadata := props + if nested, ok := props["metadata"].(map[string]interface{}); ok { + metadata = nested + } + + out := make(map[string]interface{}, len(metadata)) + for k, v := range metadata { + switch k { + case "secret", "kind", "selfLink", "etag", "id": + continue + } + out[k] = v + } + return out +} + +// hmacKeyProvisioner overrides the two operations the generic engine cannot +// express: a create whose id is a query parameter and whose response wraps the +// resource, and a delete that must deactivate the key first. +type hmacKeyProvisioner struct { + prov.Provisioner + cfg *config.Config +} + +// registerHmacKeyOverrides is called from the package init in resources.go so +// the generic registration is guaranteed to have landed first. +func registerHmacKeyOverrides() { + registry.Register(HmacKeyResourceType, + []resource.Operation{resource.OperationCreate, resource.OperationDelete}, + func(cfg *config.Config) prov.Provisioner { + return &hmacKeyProvisioner{ + Provisioner: storageRegistry.CreateProvisioner(cfg, HmacKeyResourceType), + cfg: cfg, + } + }) +} + +func (p *hmacKeyProvisioner) Create( + ctx context.Context, request *resource.CreateRequest, +) (*resource.CreateResult, error) { + props, err := unmarshalProps(request.Properties) + if err != nil { + return createFailure(resource.OperationErrorCodeInvalidRequest, err.Error()), nil + } + serviceAccountEmail := utils.GetString(props, "serviceAccountEmail") + if serviceAccountEmail == "" { + return createFailure(resource.OperationErrorCodeInvalidRequest, + "serviceAccountEmail is required to create an HMAC key"), nil + } + + cfg := config.PathFromTargetConfig(request.TargetConfig) + client, err := transport.NewClient(ctx, p.cfg) + if err != nil { + return nil, fmt.Errorf("failed to create transport client: %w", err) + } + + url := fmt.Sprintf("%s/projects/%s/hmacKeys?serviceAccountEmail=%s", + StorageAPI.BaseURL, cfg.Project, serviceAccountEmail) + response, err := client.SendRequest(ctx, transport.RequestOptions{Method: "POST", URL: url}) + if err != nil { + wrapped := transport.WrapError(err, "failed to create HMAC key") + return createFailure(transport.ToResourceErrorCode(wrapped.Code), wrapped.Message), nil + } + + accessID := hmacKeyAccessID(response.Body) + if accessID == "" { + return createFailure(resource.OperationErrorCodeServiceInternalError, + "HMAC key create returned no accessId"), nil + } + + properties := hmacKeyResponseTransformer(response.Body, base.TransformContext{}) + return &resource.CreateResult{ProgressResult: &resource.ProgressResult{ + Operation: resource.OperationCreate, + OperationStatus: resource.OperationStatusSuccess, + NativeID: fmt.Sprintf("projects/%s/hmacKeys/%s", cfg.Project, accessID), + StatusMessage: "Resource created successfully", + ResourceProperties: mustMarshal(properties), + }}, nil +} + +// Delete deactivates the key before removing it. GCS refuses to delete a key +// that is still ACTIVE, so a single DELETE is never enough. +func (p *hmacKeyProvisioner) Delete( + ctx context.Context, request *resource.DeleteRequest, +) (*resource.DeleteResult, error) { + pathCtx, err := parseStorageNativeID(request.NativeID) + if err != nil { + return deleteFailure(request.NativeID, resource.OperationErrorCodeInvalidRequest, err.Error()), nil + } + cfg := config.PathFromTargetConfig(request.TargetConfig) + project := pathCtx.Project + if project == "" { + project = cfg.Project + } + + client, err := transport.NewClient(ctx, p.cfg) + if err != nil { + return nil, fmt.Errorf("failed to create transport client: %w", err) + } + url := fmt.Sprintf("%s/projects/%s/hmacKeys/%s", StorageAPI.BaseURL, project, pathCtx.ResourceName) + + // Read first: the update needs the key's current etag-free metadata, and a + // key already gone is a delete that already happened. + current, err := client.SendRequest(ctx, transport.RequestOptions{Method: "GET", URL: url}) + if err != nil { + wrapped := transport.WrapError(err, "failed to read HMAC key before deleting") + if transport.ToResourceErrorCode(wrapped.Code) == resource.OperationErrorCodeNotFound { + return deleteSuccess(request.NativeID, "HMAC key already deleted"), nil + } + return deleteFailure(request.NativeID, transport.ToResourceErrorCode(wrapped.Code), wrapped.Message), nil + } + + if utils.GetString(current.Body, "state") != hmacKeyDeactivatedState { + body := map[string]interface{}{ + "state": hmacKeyDeactivatedState, + "accessId": pathCtx.ResourceName, + "serviceAccountEmail": utils.GetString(current.Body, "serviceAccountEmail"), + } + if _, err := client.SendRequest(ctx, transport.RequestOptions{ + Method: "PUT", URL: url, Body: body, + }); err != nil { + wrapped := transport.WrapError(err, "failed to deactivate HMAC key before deleting") + return deleteFailure(request.NativeID, + transport.ToResourceErrorCode(wrapped.Code), wrapped.Message), nil + } + } + + if _, err := client.SendRequest(ctx, transport.RequestOptions{Method: "DELETE", URL: url}); err != nil { + wrapped := transport.WrapError(err, "failed to delete HMAC key") + if transport.ToResourceErrorCode(wrapped.Code) == resource.OperationErrorCodeNotFound { + return deleteSuccess(request.NativeID, "HMAC key already deleted"), nil + } + return deleteFailure(request.NativeID, transport.ToResourceErrorCode(wrapped.Code), wrapped.Message), nil + } + return deleteSuccess(request.NativeID, "Resource deleted successfully"), nil +} + +// hmacKeyAccessID reads the key id from either shape: create wraps the metadata, +// get returns it directly. +func hmacKeyAccessID(response map[string]interface{}) string { + if metadata, ok := response["metadata"].(map[string]interface{}); ok { + if id := utils.GetString(metadata, "accessId"); id != "" { + return id + } + } + return utils.GetString(response, "accessId") +} diff --git a/pkg/resources/storage/hmac_key_helpers.go b/pkg/resources/storage/hmac_key_helpers.go new file mode 100644 index 00000000..c54d4a27 --- /dev/null +++ b/pkg/resources/storage/hmac_key_helpers.go @@ -0,0 +1,57 @@ +// © 2025 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +package storage + +import ( + "encoding/json" + "fmt" + + "github.com/platform-engineering-labs/formae-plugin-gcp/pkg/resources/base" + "github.com/platform-engineering-labs/formae/pkg/plugin/resource" +) + +func unmarshalProps(raw json.RawMessage) (map[string]interface{}, error) { + var props map[string]interface{} + if err := json.Unmarshal(raw, &props); err != nil { + return nil, fmt.Errorf("failed to parse properties: %w", err) + } + return base.UnwrapValues(props), nil +} + +func mustMarshal(props map[string]interface{}) json.RawMessage { + encoded, err := json.Marshal(props) + if err != nil { + return json.RawMessage("{}") + } + return encoded +} + +func createFailure(code resource.OperationErrorCode, message string) *resource.CreateResult { + return &resource.CreateResult{ProgressResult: &resource.ProgressResult{ + Operation: resource.OperationCreate, + OperationStatus: resource.OperationStatusFailure, + ErrorCode: code, + StatusMessage: message, + }} +} + +func deleteSuccess(nativeID, message string) *resource.DeleteResult { + return &resource.DeleteResult{ProgressResult: &resource.ProgressResult{ + Operation: resource.OperationDelete, + OperationStatus: resource.OperationStatusSuccess, + NativeID: nativeID, + StatusMessage: message, + }} +} + +func deleteFailure(nativeID string, code resource.OperationErrorCode, message string) *resource.DeleteResult { + return &resource.DeleteResult{ProgressResult: &resource.ProgressResult{ + Operation: resource.OperationDelete, + OperationStatus: resource.OperationStatusFailure, + ErrorCode: code, + NativeID: nativeID, + StatusMessage: message, + }} +} diff --git a/pkg/resources/storage/resources.go b/pkg/resources/storage/resources.go index 47919fb7..36f39375 100644 --- a/pkg/resources/storage/resources.go +++ b/pkg/resources/storage/resources.go @@ -20,6 +20,7 @@ const ( DefaultObjectAccessControlResourceType = "GCP::Storage::DefaultObjectAccessControl" ManagedFolderResourceType = "GCP::Storage::ManagedFolder" FolderResourceType = "GCP::Storage::Folder" + HmacKeyResourceType = "GCP::Storage::HmacKey" ObjectAccessControlResourceType = "GCP::Storage::ObjectAccessControl" ) @@ -187,6 +188,21 @@ func init() { RequestTransformer: base.DropFields("bucket"), ResponseTransformer: base.ResponseTransformerFunc(bucketScopedResponseTransformer), }, + { + // An HMAC key is the credential a service account uses against the + // S3-compatible XML API. It is project-scoped, its id is assigned by + // GCS, and hmac_key.go overrides create and delete: the create names + // its service account in a query parameter and answers with the + // secret wrapped alongside the metadata, and a key must be + // deactivated before GCS will delete it. + ResourceType: HmacKeyResourceType, + ResourceConfig: base.ResourceConfig{ + ResourceType: "hmacKeys", + SupportsUpdate: false, + }, + RequestTransformer: base.RequestTransformerFunc(hmacKeyRequestTransformer), + ResponseTransformer: base.ResponseTransformerFunc(hmacKeyResponseTransformer), + }, // NOTE: ObjectAccessControlResourceType requires special handling for object-scoped resources // The base package currently doesn't support resources that need TWO parent properties (bucket + object). // This resource type is commented out pending enhancement to base package's parent extraction mechanism. @@ -217,6 +233,7 @@ func init() { } registerBucketWalkingLists() + registerHmacKeyOverrides() } // bucketScopedResponseTransformer puts back the bucket a folder belongs to and diff --git a/schema/pkl/storage/hmac_key.pkl b/schema/pkl/storage/hmac_key.pkl new file mode 100644 index 00000000..ecdd5292 --- /dev/null +++ b/schema/pkl/storage/hmac_key.pkl @@ -0,0 +1,70 @@ +/* + * © 2025 Platform Engineering Labs Inc. + * + * SPDX-License-Identifier: FSL-1.1-ALv2 + */ + +/// GCP Storage HMAC Key +/// +/// The credential a service account presents to the S3-compatible XML API. +/// Without one, a tool that only speaks S3 has no way to authenticate against +/// Cloud Storage. +/// +/// The secret is returned by the create call and never again. The plugin drops +/// it rather than persisting it: keeping it would put a live credential in +/// stored state and guarantee drift on the next read. Take it from the create +/// output if you need it. +/// +/// GCS assigns the `accessId`, and refuses to delete a key that is still +/// active — the plugin deactivates it first. +module gcp.storage.hmac_key + +import "../gcp.pkl" +import "@formae/formae.pkl" + +const type = "GCP::Storage::HmacKey" + +open class HmacKeyResolvable extends formae.Resolvable { + hidden type = module.type + + hidden accessId: HmacKeyResolvable = (this) { + property = "accessId" + } +} + +@gcp.ResourceHint { + type = module.type + identifier = "accessId" +} +open class HmacKey extends formae.Resource { + hidden parent = this + + /// Email of the service account the key authenticates as. Immutable — a key + /// belongs to the account it was created for. + @gcp.FieldHint { createOnly = true } + serviceAccountEmail: String + + /// Server-assigned key id, and what the key is addressed by. + @gcp.FieldHint { hasProviderDefault = true } + accessId: String? + + /// "ACTIVE" or "INACTIVE". GCS creates keys active; the plugin deactivates + /// a key before deleting it, because GCS refuses to delete an active one. + @gcp.FieldHint { hasProviderDefault = true } + state: String? + + /// The project the key belongs to, as GCS reports it. + @gcp.FieldHint { hasProviderDefault = true } + projectId: String? + + @gcp.FieldHint { hasProviderDefault = true } + timeCreated: String? + + @gcp.FieldHint { hasProviderDefault = true } + updated: String? + + hidden res: HmacKeyResolvable = new { + label = parent.label + stack = parent.stack?.label + } +} diff --git a/testdata/config/vars.pkl b/testdata/config/vars.pkl index 06e92b10..18836cdc 100644 --- a/testdata/config/vars.pkl +++ b/testdata/config/vars.pkl @@ -11,6 +11,9 @@ local stackName = "plugin-sdk-test-stack" // Read GCP config from environment variables gcpProject = read("env:GCP_PROJECT_ID") +// Some resources are named after the project *number* rather than its id - an +// HMAC key belongs to a service account whose email carries it. +gcpProjectNumber = read("env:GCP_PROJECT_NUMBER") gcpRegion = read("env:GCP_REGION") gcpZone = read("env:GCP_ZONE") gcpLocation = read("env:GCP_LOCATION") diff --git a/testdata/storage-hmac-key.pkl b/testdata/storage-hmac-key.pkl new file mode 100644 index 00000000..74abe730 --- /dev/null +++ b/testdata/storage-hmac-key.pkl @@ -0,0 +1,24 @@ +/* + * © 2025 Platform Engineering Labs Inc. + * + * SPDX-License-Identifier: FSL-1.1-ALv2 + */ + +amends "@formae/forma.pkl" + +import "@gcp/storage/hmac_key.pkl" as hmacKey +import "./config/vars.pkl" as v + +// A key belongs to a service account, and the default compute account is the +// one every project has - its email is built from the project *number*, not the +// project id. No bucket or other prerequisite is involved: an HMAC key is +// project-scoped and free. +forma { + v.stack + v.target + + new hmacKey.HmacKey { + label = "plugin-sdk-test-storage-hmac-key" + serviceAccountEmail = "\(v.gcpProjectNumber)-compute@developer.gserviceaccount.com" + } +} From cc5d972e4817c2658c40088225e68e44543b5a5e Mon Sep 17 00:00:00 2001 From: naxty <9141879+naxty@users.noreply.github.com> Date: Fri, 28 Aug 2026 15:13:20 +0200 Subject: [PATCH 6/6] Revert "feat(storage): add HmacKey" CI's service account cannot create one: does not have storage.hmacKeys.create access to the Google Cloud project. Permission 'storage.hmacKeys.create' denied [reason: forbidden] The code may well be right - the create, the wrapped response and the deactivate-before-delete were all written against the documented contract - but it cannot be conformance-tested by either identity, and shipping an unverified resource type is exactly what the rest of this work avoided. Reverted whole rather than left dormant, including the gcpProjectNumber addition to vars.pkl: with no resource using it, it is dead config in a file every fixture imports. This reverts commit 47da6d3. --- CHANGELOG.md | 5 - pkg/resources/storage/hmac_key.go | 191 ---------------------- pkg/resources/storage/hmac_key_helpers.go | 57 ------- pkg/resources/storage/resources.go | 17 -- schema/pkl/storage/hmac_key.pkl | 70 -------- testdata/config/vars.pkl | 3 - testdata/storage-hmac-key.pkl | 24 --- 7 files changed, 367 deletions(-) delete mode 100644 pkg/resources/storage/hmac_key.go delete mode 100644 pkg/resources/storage/hmac_key_helpers.go delete mode 100644 schema/pkl/storage/hmac_key.pkl delete mode 100644 testdata/storage-hmac-key.pkl diff --git a/CHANGELOG.md b/CHANGELOG.md index 1487f3e4..e19d8288 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,11 +12,6 @@ formae agent. ### Added -- `GCP::Storage::HmacKey` — the credential a service account presents to the - S3-compatible XML API. GCS assigns the `accessId` and returns the secret - exactly once; the plugin drops it rather than persisting a live credential in - stored state. Deleting a key requires deactivating it first, which the - provisioner does — GCS refuses to delete an active key. - `GCP::Storage::ManagedFolder` — an IAM boundary inside a bucket, letting a policy be attached to a prefix without granting it over the whole bucket. Requires uniform bucket-level access. diff --git a/pkg/resources/storage/hmac_key.go b/pkg/resources/storage/hmac_key.go deleted file mode 100644 index c9a337e2..00000000 --- a/pkg/resources/storage/hmac_key.go +++ /dev/null @@ -1,191 +0,0 @@ -// © 2025 Platform Engineering Labs Inc. -// -// SPDX-License-Identifier: FSL-1.1-ALv2 - -package storage - -import ( - "context" - "fmt" - - "github.com/platform-engineering-labs/formae-plugin-gcp/pkg/config" - "github.com/platform-engineering-labs/formae-plugin-gcp/pkg/resources/base" - "github.com/platform-engineering-labs/formae-plugin-gcp/pkg/resources/prov" - "github.com/platform-engineering-labs/formae-plugin-gcp/pkg/resources/registry" - "github.com/platform-engineering-labs/formae-plugin-gcp/pkg/transport" - "github.com/platform-engineering-labs/formae-plugin-gcp/pkg/utils" - "github.com/platform-engineering-labs/formae/pkg/plugin/resource" -) - -// hmacKeyDeactivatedState is the only state GCS will delete a key from. An -// active key answers a delete with 400: it has to be deactivated first, which -// is a separate PUT. -const hmacKeyDeactivatedState = "INACTIVE" - -// hmacKeyRequestTransformer builds the create body. A key is created by naming -// the service account it belongs to in a query parameter, not in a body - the -// request carries no body at all - so this drops everything and lets -// CreateIDParam-style handling happen in the provisioner. -func hmacKeyRequestTransformer( - props map[string]interface{}, _ base.TransformContext, -) (map[string]interface{}, error) { - return map[string]interface{}{}, nil -} - -// hmacKeyResponseTransformer lifts the metadata out of the create envelope and -// drops the secret. -// -// create answers {"metadata": {...}, "secret": "..."} while get answers the -// metadata directly, so reads would otherwise disagree with creates on every -// field. The secret is returned exactly once and never again: keeping it would -// both persist a credential and guarantee drift on the next read. -func hmacKeyResponseTransformer( - props map[string]interface{}, _ base.TransformContext, -) map[string]interface{} { - metadata := props - if nested, ok := props["metadata"].(map[string]interface{}); ok { - metadata = nested - } - - out := make(map[string]interface{}, len(metadata)) - for k, v := range metadata { - switch k { - case "secret", "kind", "selfLink", "etag", "id": - continue - } - out[k] = v - } - return out -} - -// hmacKeyProvisioner overrides the two operations the generic engine cannot -// express: a create whose id is a query parameter and whose response wraps the -// resource, and a delete that must deactivate the key first. -type hmacKeyProvisioner struct { - prov.Provisioner - cfg *config.Config -} - -// registerHmacKeyOverrides is called from the package init in resources.go so -// the generic registration is guaranteed to have landed first. -func registerHmacKeyOverrides() { - registry.Register(HmacKeyResourceType, - []resource.Operation{resource.OperationCreate, resource.OperationDelete}, - func(cfg *config.Config) prov.Provisioner { - return &hmacKeyProvisioner{ - Provisioner: storageRegistry.CreateProvisioner(cfg, HmacKeyResourceType), - cfg: cfg, - } - }) -} - -func (p *hmacKeyProvisioner) Create( - ctx context.Context, request *resource.CreateRequest, -) (*resource.CreateResult, error) { - props, err := unmarshalProps(request.Properties) - if err != nil { - return createFailure(resource.OperationErrorCodeInvalidRequest, err.Error()), nil - } - serviceAccountEmail := utils.GetString(props, "serviceAccountEmail") - if serviceAccountEmail == "" { - return createFailure(resource.OperationErrorCodeInvalidRequest, - "serviceAccountEmail is required to create an HMAC key"), nil - } - - cfg := config.PathFromTargetConfig(request.TargetConfig) - client, err := transport.NewClient(ctx, p.cfg) - if err != nil { - return nil, fmt.Errorf("failed to create transport client: %w", err) - } - - url := fmt.Sprintf("%s/projects/%s/hmacKeys?serviceAccountEmail=%s", - StorageAPI.BaseURL, cfg.Project, serviceAccountEmail) - response, err := client.SendRequest(ctx, transport.RequestOptions{Method: "POST", URL: url}) - if err != nil { - wrapped := transport.WrapError(err, "failed to create HMAC key") - return createFailure(transport.ToResourceErrorCode(wrapped.Code), wrapped.Message), nil - } - - accessID := hmacKeyAccessID(response.Body) - if accessID == "" { - return createFailure(resource.OperationErrorCodeServiceInternalError, - "HMAC key create returned no accessId"), nil - } - - properties := hmacKeyResponseTransformer(response.Body, base.TransformContext{}) - return &resource.CreateResult{ProgressResult: &resource.ProgressResult{ - Operation: resource.OperationCreate, - OperationStatus: resource.OperationStatusSuccess, - NativeID: fmt.Sprintf("projects/%s/hmacKeys/%s", cfg.Project, accessID), - StatusMessage: "Resource created successfully", - ResourceProperties: mustMarshal(properties), - }}, nil -} - -// Delete deactivates the key before removing it. GCS refuses to delete a key -// that is still ACTIVE, so a single DELETE is never enough. -func (p *hmacKeyProvisioner) Delete( - ctx context.Context, request *resource.DeleteRequest, -) (*resource.DeleteResult, error) { - pathCtx, err := parseStorageNativeID(request.NativeID) - if err != nil { - return deleteFailure(request.NativeID, resource.OperationErrorCodeInvalidRequest, err.Error()), nil - } - cfg := config.PathFromTargetConfig(request.TargetConfig) - project := pathCtx.Project - if project == "" { - project = cfg.Project - } - - client, err := transport.NewClient(ctx, p.cfg) - if err != nil { - return nil, fmt.Errorf("failed to create transport client: %w", err) - } - url := fmt.Sprintf("%s/projects/%s/hmacKeys/%s", StorageAPI.BaseURL, project, pathCtx.ResourceName) - - // Read first: the update needs the key's current etag-free metadata, and a - // key already gone is a delete that already happened. - current, err := client.SendRequest(ctx, transport.RequestOptions{Method: "GET", URL: url}) - if err != nil { - wrapped := transport.WrapError(err, "failed to read HMAC key before deleting") - if transport.ToResourceErrorCode(wrapped.Code) == resource.OperationErrorCodeNotFound { - return deleteSuccess(request.NativeID, "HMAC key already deleted"), nil - } - return deleteFailure(request.NativeID, transport.ToResourceErrorCode(wrapped.Code), wrapped.Message), nil - } - - if utils.GetString(current.Body, "state") != hmacKeyDeactivatedState { - body := map[string]interface{}{ - "state": hmacKeyDeactivatedState, - "accessId": pathCtx.ResourceName, - "serviceAccountEmail": utils.GetString(current.Body, "serviceAccountEmail"), - } - if _, err := client.SendRequest(ctx, transport.RequestOptions{ - Method: "PUT", URL: url, Body: body, - }); err != nil { - wrapped := transport.WrapError(err, "failed to deactivate HMAC key before deleting") - return deleteFailure(request.NativeID, - transport.ToResourceErrorCode(wrapped.Code), wrapped.Message), nil - } - } - - if _, err := client.SendRequest(ctx, transport.RequestOptions{Method: "DELETE", URL: url}); err != nil { - wrapped := transport.WrapError(err, "failed to delete HMAC key") - if transport.ToResourceErrorCode(wrapped.Code) == resource.OperationErrorCodeNotFound { - return deleteSuccess(request.NativeID, "HMAC key already deleted"), nil - } - return deleteFailure(request.NativeID, transport.ToResourceErrorCode(wrapped.Code), wrapped.Message), nil - } - return deleteSuccess(request.NativeID, "Resource deleted successfully"), nil -} - -// hmacKeyAccessID reads the key id from either shape: create wraps the metadata, -// get returns it directly. -func hmacKeyAccessID(response map[string]interface{}) string { - if metadata, ok := response["metadata"].(map[string]interface{}); ok { - if id := utils.GetString(metadata, "accessId"); id != "" { - return id - } - } - return utils.GetString(response, "accessId") -} diff --git a/pkg/resources/storage/hmac_key_helpers.go b/pkg/resources/storage/hmac_key_helpers.go deleted file mode 100644 index c54d4a27..00000000 --- a/pkg/resources/storage/hmac_key_helpers.go +++ /dev/null @@ -1,57 +0,0 @@ -// © 2025 Platform Engineering Labs Inc. -// -// SPDX-License-Identifier: FSL-1.1-ALv2 - -package storage - -import ( - "encoding/json" - "fmt" - - "github.com/platform-engineering-labs/formae-plugin-gcp/pkg/resources/base" - "github.com/platform-engineering-labs/formae/pkg/plugin/resource" -) - -func unmarshalProps(raw json.RawMessage) (map[string]interface{}, error) { - var props map[string]interface{} - if err := json.Unmarshal(raw, &props); err != nil { - return nil, fmt.Errorf("failed to parse properties: %w", err) - } - return base.UnwrapValues(props), nil -} - -func mustMarshal(props map[string]interface{}) json.RawMessage { - encoded, err := json.Marshal(props) - if err != nil { - return json.RawMessage("{}") - } - return encoded -} - -func createFailure(code resource.OperationErrorCode, message string) *resource.CreateResult { - return &resource.CreateResult{ProgressResult: &resource.ProgressResult{ - Operation: resource.OperationCreate, - OperationStatus: resource.OperationStatusFailure, - ErrorCode: code, - StatusMessage: message, - }} -} - -func deleteSuccess(nativeID, message string) *resource.DeleteResult { - return &resource.DeleteResult{ProgressResult: &resource.ProgressResult{ - Operation: resource.OperationDelete, - OperationStatus: resource.OperationStatusSuccess, - NativeID: nativeID, - StatusMessage: message, - }} -} - -func deleteFailure(nativeID string, code resource.OperationErrorCode, message string) *resource.DeleteResult { - return &resource.DeleteResult{ProgressResult: &resource.ProgressResult{ - Operation: resource.OperationDelete, - OperationStatus: resource.OperationStatusFailure, - ErrorCode: code, - NativeID: nativeID, - StatusMessage: message, - }} -} diff --git a/pkg/resources/storage/resources.go b/pkg/resources/storage/resources.go index 36f39375..47919fb7 100644 --- a/pkg/resources/storage/resources.go +++ b/pkg/resources/storage/resources.go @@ -20,7 +20,6 @@ const ( DefaultObjectAccessControlResourceType = "GCP::Storage::DefaultObjectAccessControl" ManagedFolderResourceType = "GCP::Storage::ManagedFolder" FolderResourceType = "GCP::Storage::Folder" - HmacKeyResourceType = "GCP::Storage::HmacKey" ObjectAccessControlResourceType = "GCP::Storage::ObjectAccessControl" ) @@ -188,21 +187,6 @@ func init() { RequestTransformer: base.DropFields("bucket"), ResponseTransformer: base.ResponseTransformerFunc(bucketScopedResponseTransformer), }, - { - // An HMAC key is the credential a service account uses against the - // S3-compatible XML API. It is project-scoped, its id is assigned by - // GCS, and hmac_key.go overrides create and delete: the create names - // its service account in a query parameter and answers with the - // secret wrapped alongside the metadata, and a key must be - // deactivated before GCS will delete it. - ResourceType: HmacKeyResourceType, - ResourceConfig: base.ResourceConfig{ - ResourceType: "hmacKeys", - SupportsUpdate: false, - }, - RequestTransformer: base.RequestTransformerFunc(hmacKeyRequestTransformer), - ResponseTransformer: base.ResponseTransformerFunc(hmacKeyResponseTransformer), - }, // NOTE: ObjectAccessControlResourceType requires special handling for object-scoped resources // The base package currently doesn't support resources that need TWO parent properties (bucket + object). // This resource type is commented out pending enhancement to base package's parent extraction mechanism. @@ -233,7 +217,6 @@ func init() { } registerBucketWalkingLists() - registerHmacKeyOverrides() } // bucketScopedResponseTransformer puts back the bucket a folder belongs to and diff --git a/schema/pkl/storage/hmac_key.pkl b/schema/pkl/storage/hmac_key.pkl deleted file mode 100644 index ecdd5292..00000000 --- a/schema/pkl/storage/hmac_key.pkl +++ /dev/null @@ -1,70 +0,0 @@ -/* - * © 2025 Platform Engineering Labs Inc. - * - * SPDX-License-Identifier: FSL-1.1-ALv2 - */ - -/// GCP Storage HMAC Key -/// -/// The credential a service account presents to the S3-compatible XML API. -/// Without one, a tool that only speaks S3 has no way to authenticate against -/// Cloud Storage. -/// -/// The secret is returned by the create call and never again. The plugin drops -/// it rather than persisting it: keeping it would put a live credential in -/// stored state and guarantee drift on the next read. Take it from the create -/// output if you need it. -/// -/// GCS assigns the `accessId`, and refuses to delete a key that is still -/// active — the plugin deactivates it first. -module gcp.storage.hmac_key - -import "../gcp.pkl" -import "@formae/formae.pkl" - -const type = "GCP::Storage::HmacKey" - -open class HmacKeyResolvable extends formae.Resolvable { - hidden type = module.type - - hidden accessId: HmacKeyResolvable = (this) { - property = "accessId" - } -} - -@gcp.ResourceHint { - type = module.type - identifier = "accessId" -} -open class HmacKey extends formae.Resource { - hidden parent = this - - /// Email of the service account the key authenticates as. Immutable — a key - /// belongs to the account it was created for. - @gcp.FieldHint { createOnly = true } - serviceAccountEmail: String - - /// Server-assigned key id, and what the key is addressed by. - @gcp.FieldHint { hasProviderDefault = true } - accessId: String? - - /// "ACTIVE" or "INACTIVE". GCS creates keys active; the plugin deactivates - /// a key before deleting it, because GCS refuses to delete an active one. - @gcp.FieldHint { hasProviderDefault = true } - state: String? - - /// The project the key belongs to, as GCS reports it. - @gcp.FieldHint { hasProviderDefault = true } - projectId: String? - - @gcp.FieldHint { hasProviderDefault = true } - timeCreated: String? - - @gcp.FieldHint { hasProviderDefault = true } - updated: String? - - hidden res: HmacKeyResolvable = new { - label = parent.label - stack = parent.stack?.label - } -} diff --git a/testdata/config/vars.pkl b/testdata/config/vars.pkl index 18836cdc..06e92b10 100644 --- a/testdata/config/vars.pkl +++ b/testdata/config/vars.pkl @@ -11,9 +11,6 @@ local stackName = "plugin-sdk-test-stack" // Read GCP config from environment variables gcpProject = read("env:GCP_PROJECT_ID") -// Some resources are named after the project *number* rather than its id - an -// HMAC key belongs to a service account whose email carries it. -gcpProjectNumber = read("env:GCP_PROJECT_NUMBER") gcpRegion = read("env:GCP_REGION") gcpZone = read("env:GCP_ZONE") gcpLocation = read("env:GCP_LOCATION") diff --git a/testdata/storage-hmac-key.pkl b/testdata/storage-hmac-key.pkl deleted file mode 100644 index 74abe730..00000000 --- a/testdata/storage-hmac-key.pkl +++ /dev/null @@ -1,24 +0,0 @@ -/* - * © 2025 Platform Engineering Labs Inc. - * - * SPDX-License-Identifier: FSL-1.1-ALv2 - */ - -amends "@formae/forma.pkl" - -import "@gcp/storage/hmac_key.pkl" as hmacKey -import "./config/vars.pkl" as v - -// A key belongs to a service account, and the default compute account is the -// one every project has - its email is built from the project *number*, not the -// project id. No bucket or other prerequisite is involved: an HMAC key is -// project-scoped and free. -forma { - v.stack - v.target - - new hmacKey.HmacKey { - label = "plugin-sdk-test-storage-hmac-key" - serviceAccountEmail = "\(v.gcpProjectNumber)-compute@developer.gserviceaccount.com" - } -}