From 9469c926e565e2eef2018bdc5f2cc55eae5e27b7 Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Thu, 5 Nov 2026 18:12:30 -0600 Subject: [PATCH 1/3] feat(storage): managed folders managedFolders insert, get, list and delete, with allowNonEmpty and metageneration preconditions, and uniform bucket-level access on buckets, which managed folders need. A bucket with managed folders left is not empty. --- README.md | 12 +- gcp/storage/buckets.go | 114 ++++++++++++- gcp/storage/folders.go | 254 ++++++++++++++++++++++++++++ gcp/storage/folders_test.go | 230 +++++++++++++++++++++++++ gcp/storage/objects.go | 2 +- gcp/storage/permissions.go | 6 + gcp/storage/server.go | 67 ++++++-- sdktest/gcp/storage/folders_test.go | 142 ++++++++++++++++ 8 files changed, 807 insertions(+), 20 deletions(-) create mode 100644 gcp/storage/folders.go create mode 100644 gcp/storage/folders_test.go create mode 100644 sdktest/gcp/storage/folders_test.go diff --git a/README.md b/README.md index 9abcfef..01ebe20 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,7 @@ failure replays from its seed. | [`gcp/resourcemanager`](https://pkg.go.dev/github.com/parallelworks/squall/gcp/resourcemanager) | Cloud Resource Manager: projects (get by ID or number, getIamPolicy, setIamPolicy with etag checks, testIamPermissions answered from the server's access policy), seeded by the test. | | [`gcp/servicenetworking`](https://pkg.go.dev/github.com/parallelworks/squall/gcp/servicenetworking) | Service Networking: private service connections (create, list by network, patch with force, deleteConnection), checked against a `gcp/compute` server's networks and allocated ranges and peering the network there, and their long-running operations (get). | | [`gcp/serviceusage`](https://pkg.go.dev/github.com/parallelworks/squall/gcp/serviceusage) | Service Usage: services (enable, disable, get, list by state) in projects seeded by the test and named by number or ID, and their long-running operations (get). | -| [`gcp/storage`](https://pkg.go.dev/github.com/parallelworks/squall/gcp/storage) | Cloud Storage: buckets (insert, get, list, patch labels, versioning, CORS, retention policies, soft delete policies and Requester Pays, lock a retention policy, delete with soft delete, list and restore soft-deleted ones) and objects (multipart and media uploads, get, patch and update metadata, temporary, event-based and default holds, object retention, list with versions or soft-deleted generations, delete by generation, restore). | +| [`gcp/storage`](https://pkg.go.dev/github.com/parallelworks/squall/gcp/storage) | Cloud Storage: buckets (insert, get, list, patch labels, versioning, CORS, retention policies, soft delete policies, Requester Pays and uniform bucket-level access, lock a retention policy, delete with soft delete, list and restore soft-deleted ones), managed folders (insert, get, list with prefixes and pages, delete, empty unless allowNonEmpty) and objects (multipart and media uploads, get, patch and update metadata, temporary, event-based and default holds, object retention, list with versions or soft-deleted generations, delete by generation, restore). | Coverage grows with use: each stand-in implements the methods someone needed, answers anything else with 404, and documents how faithful it is. Contributions @@ -427,8 +427,16 @@ For Cloud Storage: tokens are positions, so a listing survives deletes between pages. Resumable uploads, media reads, IAM policies and HMAC keys are not supported. +- Managed folders need uniform bucket-level access on their bucket, and their + names end with `/`. Deleting one that holds objects or other managed + folders fails unless `allowNonEmpty=true`, which needs + `storage.managedFolders.setIamPolicy` under `WithPolicy` and leaves what it + held in place. A bucket with a managed folder left cannot be deleted, nor + can its uniform bucket-level access be turned off. `PutManagedFolder` makes + one directly. Managed folder IAM policies are not supported. - A fault rule's `Resource` is the path below `/storage/v1/`, such as - `b/my-bucket/o/my-object`, and for an insert the bucket or object it makes. + `b/my-bucket/o/my-object` or `b/my-bucket/managedFolders/logs/`, and for an + insert the bucket, object or managed folder it makes. For Azure Resource Manager: diff --git a/gcp/storage/buckets.go b/gcp/storage/buckets.go index 5036b79..a9c7e9f 100644 --- a/gcp/storage/buckets.go +++ b/gcp/storage/buckets.go @@ -54,6 +54,7 @@ type Bucket struct { ObjectRetention *BucketObjectRetention `json:"objectRetention,omitzero"` SoftDeletePolicy *SoftDeletePolicy `json:"softDeletePolicy,omitzero"` Billing *Billing `json:"billing,omitzero"` + IamConfiguration *IamConfiguration `json:"iamConfiguration,omitzero"` SoftDeleteTime string `json:"softDeleteTime,omitzero"` HardDeleteTime string `json:"hardDeleteTime,omitzero"` @@ -94,6 +95,18 @@ type BucketObjectRetention struct { Mode string `json:"mode"` } +// IamConfiguration is a bucket's uniform bucket-level access setting, reported under both of its names. +type IamConfiguration struct { + BucketPolicyOnly UniformBucketLevelAccess `json:"bucketPolicyOnly"` + UniformBucketLevelAccess UniformBucketLevelAccess `json:"uniformBucketLevelAccess"` +} + +// UniformBucketLevelAccess says whether only IAM governs access to a bucket, and until when that can be turned off. +type UniformBucketLevelAccess struct { + Enabled bool `json:"enabled"` + LockedTime string `json:"lockedTime,omitzero"` +} + // Billing is a bucket's billing setting. type Billing struct { RequesterPays bool `json:"requesterPays"` @@ -106,6 +119,7 @@ type bucket struct { // trash holds soft-deleted generations, in the order they were deleted. trash []*Object softDeleteHistory []policyStep + folders map[string]*ManagedFolder // nextPurge is when the first of trash goes for good. nextPurge time.Time // hardDelete is when a soft-deleted bucket goes for good. @@ -137,6 +151,9 @@ func (b *bucket) clone() Bucket { if b.Encryption != nil { c.Encryption = new(*b.Encryption) } + if b.IamConfiguration != nil { + c.IamConfiguration = new(*b.IamConfiguration) + } return c } @@ -152,7 +169,12 @@ func (b *bucket) softDeleteDuration() time.Duration { func (b *bucket) requesterPays() bool { return b.Billing != nil && b.Billing.RequesterPays } -func (b *bucket) empty() bool { return len(b.objects) == 0 } +// empty reports whether b holds no objects and no managed folders. +func (b *bucket) empty() bool { return len(b.objects) == 0 && len(b.folders) == 0 } + +func (b *bucket) uniformAccess() bool { + return b.IamConfiguration != nil && b.IamConfiguration.UniformBucketLevelAccess.Enabled +} // touch records a change to b's metadata. The caller holds s.mu. func (s *Server) touch(b *bucket) { touchAt(b, s.now()) } @@ -312,6 +334,49 @@ type retentionPolicyInput struct { IsLocked json.RawMessage `json:"isLocked"` } +type uniformAccessInput struct { + Enabled *bool `json:"enabled"` + LockedTime json.RawMessage `json:"lockedTime"` +} + +type iamConfigurationInput struct { + BucketPolicyOnly *uniformAccessInput `json:"bucketPolicyOnly"` + UniformBucketLevelAccess *uniformAccessInput `json:"uniformBucketLevelAccess"` + PublicAccessPrevention string `json:"publicAccessPrevention"` +} + +// uniformAccess returns whether in turns uniform bucket-level access on or off, nil if it does neither. +func (in *iamConfigurationInput) uniformAccess() (*bool, *apiError) { + if in == nil { + return nil, nil + } + if in.PublicAccessPrevention != "" && in.PublicAccessPrevention != "inherited" { + return nil, invalid("squall: publicAccessPrevention=%s is not supported", in.PublicAccessPrevention) + } + var ubla, bpo *bool + if in.UniformBucketLevelAccess != nil { + ubla = in.UniformBucketLevelAccess.Enabled + } + if in.BucketPolicyOnly != nil { + bpo = in.BucketPolicyOnly.Enabled + } + if ubla != nil && bpo != nil && *ubla != *bpo { + return nil, invalid("Invalid argument: uniformBucketLevelAccess and bucketPolicyOnly must match.") + } + return cmp.Or(ubla, bpo), nil +} + +// uniformAccessLock is how long after uniform bucket-level access is turned on it can still be turned off. +const uniformAccessLock = 90 * 24 * time.Hour + +func iamConfiguration(enabled bool, t time.Time) *IamConfiguration { + u := UniformBucketLevelAccess{Enabled: enabled} + if enabled { + u.LockedTime = t.Add(uniformAccessLock).UTC().Format(timeFormat) + } + return &IamConfiguration{BucketPolicyOnly: u, UniformBucketLevelAccess: u} +} + type softDeletePolicyInput struct { RetentionDurationSeconds *int64Value `json:"retentionDurationSeconds"` EffectiveTime json.RawMessage `json:"effectiveTime"` @@ -352,6 +417,7 @@ type bucketInput struct { SoftDeletePolicy *softDeletePolicyInput `json:"softDeletePolicy"` Billing *Billing `json:"billing"` ObjectRetention *BucketObjectRetention `json:"objectRetention"` + IamConfiguration *iamConfigurationInput `json:"iamConfiguration"` bucketOutputOnly // Lifecycle is accepted without rules, since the client sends it empty. Lifecycle *struct { @@ -413,6 +479,13 @@ func (s *Server) insertBucket(r *http.Request) (any, *apiError) { if in.DefaultEventBasedHold != nil { b.DefaultEventBasedHold = *in.DefaultEventBasedHold } + uniform, err := in.IamConfiguration.uniformAccess() + if err != nil { + return nil, err + } + if uniform != nil { + b.IamConfiguration = &IamConfiguration{UniformBucketLevelAccess: UniformBucketLevelAccess{Enabled: *uniform}} + } if objectRetention { b.ObjectRetention = &BucketObjectRetention{Mode: "Enabled"} } @@ -500,6 +573,9 @@ func (s *Server) store(b Bucket) (*bucket, *apiError) { if b.SoftDeletePolicy.RetentionDurationSeconds > 0 { b.SoftDeletePolicy.EffectiveTime = now } + if c := b.IamConfiguration; c != nil { + b.IamConfiguration = iamConfiguration(c.UniformBucketLevelAccess.Enabled || c.BucketPolicyOnly.Enabled, t) + } b.Location = strings.ToUpper(cmp.Or(b.Location, "US")) b.LocationType = locationType(b.Location, b.CustomPlacementConfig) if len(b.Labels) == 0 { @@ -566,6 +642,13 @@ func (s *Server) patchBucket(r *http.Request) (any, *apiError) { if cleared("softDeletePolicy") { return nil, invalid("squall: softDeletePolicy cannot be null; a retentionDurationSeconds of 0 turns soft delete off") } + if cleared("iamConfiguration") { + return nil, invalid("squall: iamConfiguration cannot be null") + } + uniform, err := in.IamConfiguration.uniformAccess() + if err != nil { + return nil, err + } s.mu.Lock() defer s.mu.Unlock() b, err := s.lookup(r.PathValue("bucket"), "storage.buckets.update") @@ -612,10 +695,14 @@ func (s *Server) patchBucket(r *http.Request) (any, *apiError) { if err != nil { return nil, err } + iam, err := nextIamConfiguration(b, uniform, changed) + if err != nil { + return nil, err + } if len(labels) == 0 { labels = nil } - b.Labels, b.StorageClass, b.RetentionPolicy, b.SoftDeletePolicy = labels, class, retention, softDelete + b.Labels, b.StorageClass, b.RetentionPolicy, b.SoftDeletePolicy, b.IamConfiguration = labels, class, retention, softDelete, iam recordSoftDelete(b, changed) if in.DefaultEventBasedHold != nil { b.DefaultEventBasedHold = *in.DefaultEventBasedHold @@ -647,6 +734,25 @@ func (s *Server) patchBucket(r *http.Request) (any, *apiError) { return b.clone(), nil } +// nextIamConfiguration is b's iamConfiguration once uniform, if not nil, turns uniform bucket-level access on or off at t. +func nextIamConfiguration(b *bucket, uniform *bool, t time.Time) (*IamConfiguration, *apiError) { + if uniform == nil || *uniform == b.uniformAccess() { + return b.IamConfiguration, nil + } + if !*uniform { + if len(b.folders) > 0 { + return nil, &apiError{ + Status: http.StatusConflict, Reason: "conflict", + Message: "Uniform bucket-level access cannot be disabled while the bucket contains managed folders.", + } + } + if locked, err := time.Parse(time.RFC3339, b.IamConfiguration.UniformBucketLevelAccess.LockedTime); err == nil && !t.Before(locked) { + return nil, invalid("Uniform bucket-level access cannot be disabled after it has been enabled for 90 days.") + } + } + return iamConfiguration(*uniform, t), nil +} + func (s *Server) deleteBucket(r *http.Request) (any, *apiError) { s.mu.Lock() defer s.mu.Unlock() @@ -704,7 +810,7 @@ func (s *Server) listBuckets(r *http.Request) (any, *apiError) { } } } - items, next, err := page(r, all, key) + items, next, err := page(r, "maxResults", all, key) if err != nil { return nil, err } @@ -745,7 +851,7 @@ func (s *Server) PutBucket(b Bucket) (Bucket, error) { Name: b.Name, Location: b.Location, StorageClass: b.StorageClass, Labels: b.Labels, Versioning: b.Versioning, CORS: b.CORS, CustomPlacementConfig: b.CustomPlacementConfig, Encryption: b.Encryption, RPO: b.RPO, Project: b.Project, RetentionPolicy: b.RetentionPolicy, DefaultEventBasedHold: b.DefaultEventBasedHold, ObjectRetention: b.ObjectRetention, - SoftDeletePolicy: b.SoftDeletePolicy, Billing: b.Billing, + SoftDeletePolicy: b.SoftDeletePolicy, Billing: b.Billing, IamConfiguration: b.IamConfiguration, } s.mu.Lock() defer s.mu.Unlock() diff --git a/gcp/storage/folders.go b/gcp/storage/folders.go new file mode 100644 index 0000000..23513d7 --- /dev/null +++ b/gcp/storage/folders.go @@ -0,0 +1,254 @@ +package storage + +import ( + "encoding/json" + "fmt" + "maps" + "net/http" + "net/url" + "slices" + "strings" + "unicode/utf8" +) + +const maxFolderDepth = 15 + +// ManagedFolder is a managed folder, in the API's JSON shape. Its Name ends with "/". +type ManagedFolder struct { + Kind string `json:"kind"` + ID string `json:"id"` + SelfLink string `json:"selfLink"` + Name string `json:"name"` + Bucket string `json:"bucket"` + Metageneration int64 `json:"metageneration,string"` + CreateTime string `json:"createTime"` + UpdateTime string `json:"updateTime"` +} + +// folderInput is what managedFolders.insert takes; the fields the server sets are ignored. +type folderInput struct { + Name string `json:"name"` + Bucket json.RawMessage `json:"bucket"` + Kind json.RawMessage `json:"kind"` + ID json.RawMessage `json:"id"` + SelfLink json.RawMessage `json:"selfLink"` + Metageneration json.RawMessage `json:"metageneration"` + CreateTime json.RawMessage `json:"createTime"` + UpdateTime json.RawMessage `json:"updateTime"` +} + +// folderName is name with the trailing "/" every managed folder name has. +func folderName(name string) string { + if name == "" || strings.HasSuffix(name, "/") { + return name + } + return name + "/" +} + +// validFolderName reports whether name, ending with "/", follows the managed folder naming rules. +func validFolderName(name string) bool { + base := strings.TrimSuffix(name, "/") + return base != "" && len(name) <= 1024 && utf8.ValidString(name) && !strings.ContainsAny(name, "\r\n") && + base != "." && base != ".." && !strings.HasPrefix(name, ".well-known/acme-challenge/") && + strings.Count(name, "/") <= maxFolderDepth +} + +func folderTarget(r *http.Request) string { + return "b/" + r.PathValue("bucket") + "/managedFolders/" + folderName(r.PathValue("folder")) +} + +// folderInsertTarget names the managed folder a managedFolders.insert creates, from its body. +func folderInsertTarget(r *http.Request) string { + prefix := "b/" + r.PathValue("bucket") + "/managedFolders" + var named struct { + Name string `json:"name"` + } + if json.Unmarshal(peekBody(r, maxBody), &named) != nil || named.Name == "" { + return prefix + } + return prefix + "/" + folderName(named.Name) +} + +func noSuchFolder(bucket, name string) *apiError { + return &apiError{Status: http.StatusNotFound, Reason: "notFound", Message: fmt.Sprintf("No such managed folder: %s/%s", bucket, name)} +} + +// folderBucket returns the named bucket for a managed folder call needing permission. The caller holds s.mu. +func (s *Server) folderBucket(name, permission string) (*bucket, *apiError) { + b, err := s.lookup(name, permission) + if err != nil { + return nil, err + } + if !b.uniformAccess() { + return nil, &apiError{ + Status: http.StatusConflict, Reason: "conflict", + Message: "Managed folders can only be used in buckets that have uniform bucket-level access enabled.", + } + } + return b, nil +} + +// putFolder creates the managed folder name in the named bucket. The caller holds s.mu. +func (s *Server) putFolder(bucketName, name string) (*ManagedFolder, *apiError) { + if name == "" { + return nil, required("resource.name") + } + name = folderName(name) + if !validFolderName(name) { + return nil, invalid("Invalid managed folder name: '%s'", name) + } + b, err := s.folderBucket(bucketName, "storage.managedFolders.create") + if err != nil { + return nil, err + } + if _, ok := b.folders[name]; ok { + return nil, &apiError{Status: http.StatusConflict, Reason: "conflict", Message: fmt.Sprintf("The managed folder %s/%s already exists.", b.Name, name)} + } + _, now := s.timestamp() + f := &ManagedFolder{ + Kind: "storage#managedFolder", ID: b.Name + "/" + name, + SelfLink: apiBase + "b/" + b.Name + "/managedFolders/" + url.PathEscape(name), + Name: name, Bucket: b.Name, Metageneration: 1, CreateTime: now, UpdateTime: now, + } + if b.folders == nil { + b.folders = map[string]*ManagedFolder{} + } + b.folders[name] = f + return f, nil +} + +func (s *Server) insertFolder(r *http.Request) (any, *apiError) { + body, err := readBody(r, maxBody) + if err != nil { + return nil, err + } + var in folderInput + if err := unmarshalStrict(body, &in); err != nil { + return nil, err + } + s.mu.Lock() + defer s.mu.Unlock() + f, err := s.putFolder(r.PathValue("bucket"), in.Name) + if err != nil { + return nil, err + } + return *f, nil +} + +// folder returns the managed folder a call names, once its preconditions hold. The caller holds s.mu. +func (s *Server) folder(r *http.Request, permission string) (*bucket, *ManagedFolder, *apiError) { + b, err := s.folderBucket(r.PathValue("bucket"), permission) + if err != nil { + return nil, nil, err + } + name := folderName(r.PathValue("folder")) + f, ok := b.folders[name] + if !ok { + return nil, nil, noSuchFolder(b.Name, name) + } + if err := checkMatch(r, "ifMetagenerationMatch", "ifMetagenerationNotMatch", f.Metageneration); err != nil { + return nil, nil, err + } + return b, f, nil +} + +func (s *Server) getFolder(r *http.Request) (any, *apiError) { + s.mu.Lock() + defer s.mu.Unlock() + _, f, err := s.folder(r, "storage.managedFolders.get") + if err != nil { + return nil, err + } + return *f, nil +} + +func (s *Server) deleteFolder(r *http.Request) (any, *apiError) { + allowNonEmpty, err := boolParam(r, "allowNonEmpty") + if err != nil { + return nil, err + } + s.mu.Lock() + defer s.mu.Unlock() + b, f, err := s.folder(r, "storage.managedFolders.delete") + if err != nil { + return nil, err + } + if !allowNonEmpty && b.folderHolds(f.Name) { + return nil, &apiError{ + Status: http.StatusConflict, Reason: "conflict", + Message: fmt.Sprintf("The managed folder %s/%s is not empty.", b.Name, f.Name), + } + } + delete(b.folders, f.Name) + return nil, nil +} + +// folderHolds reports whether any object generation or other managed folder in b is under name. +func (b *bucket) folderHolds(name string) bool { + for o := range b.objects { + if strings.HasPrefix(o, name) { + return true + } + } + for other := range b.folders { + if other != name && strings.HasPrefix(other, name) { + return true + } + } + return false +} + +type folderList struct { + Kind string `json:"kind"` + Items []ManagedFolder `json:"items,omitzero"` + NextPageToken string `json:"nextPageToken,omitzero"` +} + +func (s *Server) listFolders(r *http.Request) (any, *apiError) { + prefix := r.URL.Query().Get("prefix") + s.mu.Lock() + defer s.mu.Unlock() + b, err := s.folderBucket(r.PathValue("bucket"), "storage.managedFolders.list") + if err != nil { + return nil, err + } + all := b.managedFolders(prefix) + items, next, err := page(r, "pageSize", all, func(f ManagedFolder) string { return f.Name }) + if err != nil { + return nil, err + } + return folderList{Kind: "storage#managedFolders", Items: items, NextPageToken: next}, nil +} + +// managedFolders returns b's managed folders whose names start with prefix, ordered by name. +func (b *bucket) managedFolders(prefix string) []ManagedFolder { + var out []ManagedFolder + for _, name := range slices.Sorted(maps.Keys(b.folders)) { + if strings.HasPrefix(name, prefix) { + out = append(out, *b.folders[name]) + } + } + return out +} + +// ManagedFolders returns the named bucket's managed folders, ordered by name. +func (s *Server) ManagedFolders(bucketName string) []ManagedFolder { + s.mu.Lock() + defer s.mu.Unlock() + b, ok := s.buckets[bucketName] + if !ok { + return nil + } + return b.managedFolders("") +} + +// PutManagedFolder creates a managed folder in the named bucket, as managedFolders.insert does, adding the trailing "/" if name lacks it. +func (s *Server) PutManagedFolder(bucketName, name string) (ManagedFolder, error) { + s.mu.Lock() + defer s.mu.Unlock() + f, err := s.putFolder(bucketName, name) + if err != nil { + return ManagedFolder{}, err + } + return *f, nil +} diff --git a/gcp/storage/folders_test.go b/gcp/storage/folders_test.go new file mode 100644 index 0000000..f483092 --- /dev/null +++ b/gcp/storage/folders_test.go @@ -0,0 +1,230 @@ +package storage_test + +import ( + "encoding/json" + "net/http" + "strings" + "sync" + "testing" + "time" + + "github.com/parallelworks/squall/access" + "github.com/parallelworks/squall/gcp/storage" +) + +const uniformBucket = `{"name":"%s","iamConfiguration":{"uniformBucketLevelAccess":{"enabled":true}}}` + +func uniform(t *testing.T, s *storage.Server, name string) { + t.Helper() + if code, body := do(t, s, http.MethodPost, "/storage/v1/b?project=p", "application/json", strings.Replace(uniformBucket, "%s", name, 1)); code != http.StatusOK { + t.Fatalf("insert bucket = %d %s", code, body) + } +} + +func TestManagedFolderShape(t *testing.T) { + s := storage.New(storage.WithClock(func() time.Time { return time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) })) + uniform(t, s, "mfb") + code, body := do(t, s, http.MethodPost, "/storage/v1/b/mfb/managedFolders", "application/json", `{"name":"logs/app"}`) + want := `{"kind":"storage#managedFolder","id":"mfb/logs/app/","selfLink":"https://www.googleapis.com/storage/v1/b/mfb/managedFolders/logs%2Fapp%2F",` + + `"name":"logs/app/","bucket":"mfb","metageneration":"1","createTime":"2026-01-02T03:04:05.000Z","updateTime":"2026-01-02T03:04:05.000Z"}` + if code != http.StatusOK || body != want { + t.Fatalf("insert = %d %s, want %s", code, body, want) + } + for _, path := range []string{"logs%2Fapp%2F", "logs%2Fapp"} { + if code, got := do(t, s, http.MethodGet, "/storage/v1/b/mfb/managedFolders/"+path, "", ""); code != http.StatusOK || got != want { + t.Errorf("get %s = %d %s", path, code, got) + } + } + code, body = do(t, s, http.MethodGet, "/storage/v1/b/mfb", "", "") + if code != http.StatusOK || !strings.Contains(body, `"iamConfiguration":{"bucketPolicyOnly":{"enabled":true,"lockedTime":"2026-04-02T03:04:05.000Z"},"uniformBucketLevelAccess":{"enabled":true,"lockedTime":"2026-04-02T03:04:05.000Z"}}`) { + t.Errorf("get bucket = %d %s", code, body) + } +} + +func TestManagedFolderErrors(t *testing.T) { + s := storage.New() + uniform(t, s, "mfb") + if code, body := do(t, s, http.MethodPost, "/storage/v1/b?project=p", "application/json", `{"name":"fine-grained"}`); code != http.StatusOK { + t.Fatalf("insert bucket = %d %s", code, body) + } + if _, err := s.PutManagedFolder("mfb", "a/"); err != nil { + t.Fatal(err) + } + for _, tc := range []struct { + name, method, target, body string + status int + reason string + }{ + {"an existing folder", http.MethodPost, "/storage/v1/b/mfb/managedFolders", `{"name":"a/"}`, http.StatusConflict, "conflict"}, + {"no name", http.MethodPost, "/storage/v1/b/mfb/managedFolders", `{}`, http.StatusBadRequest, "required"}, + {"a dot", http.MethodPost, "/storage/v1/b/mfb/managedFolders", `{"name":"./"}`, http.StatusBadRequest, "invalid"}, + {"a line feed", http.MethodPost, "/storage/v1/b/mfb/managedFolders", `{"name":"a\nb/"}`, http.StatusBadRequest, "invalid"}, + {"16 levels", http.MethodPost, "/storage/v1/b/mfb/managedFolders", `{"name":"` + strings.Repeat("d/", 16) + `"}`, http.StatusBadRequest, "invalid"}, + {"an ACME challenge", http.MethodPost, "/storage/v1/b/mfb/managedFolders", `{"name":".well-known/acme-challenge/x/"}`, http.StatusBadRequest, "invalid"}, + {"rapid cache", http.MethodPost, "/storage/v1/b/mfb/managedFolders", `{"name":"c/","rapidCacheConfig":{}}`, http.StatusBadRequest, "invalid"}, + {"a bucket without uniform access", http.MethodPost, "/storage/v1/b/fine-grained/managedFolders", `{"name":"c/"}`, http.StatusConflict, "conflict"}, + {"a list without uniform access", http.MethodGet, "/storage/v1/b/fine-grained/managedFolders", "", http.StatusConflict, "conflict"}, + {"a missing bucket", http.MethodGet, "/storage/v1/b/nowhere/managedFolders", "", http.StatusNotFound, "notFound"}, + {"a missing folder", http.MethodGet, "/storage/v1/b/mfb/managedFolders/b%2F", "", http.StatusNotFound, "notFound"}, + {"a delete of a missing folder", http.MethodDelete, "/storage/v1/b/mfb/managedFolders/b%2F", "", http.StatusNotFound, "notFound"}, + {"a metageneration mismatch", http.MethodGet, "/storage/v1/b/mfb/managedFolders/a%2F?ifMetagenerationMatch=2", "", http.StatusPreconditionFailed, "conditionNotMet"}, + {"a delete on a metageneration match", http.MethodDelete, "/storage/v1/b/mfb/managedFolders/a%2F?ifMetagenerationNotMatch=1", "", http.StatusPreconditionFailed, "conditionNotMet"}, + {"maxResults", http.MethodGet, "/storage/v1/b/mfb/managedFolders?maxResults=1", "", http.StatusBadRequest, "invalid"}, + {"a negative page size", http.MethodGet, "/storage/v1/b/mfb/managedFolders?pageSize=-1", "", http.StatusBadRequest, "invalid"}, + {"a bad allowNonEmpty", http.MethodDelete, "/storage/v1/b/mfb/managedFolders/a%2F?allowNonEmpty=yes", "", http.StatusBadRequest, "invalid"}, + {"an IAM policy", http.MethodGet, "/storage/v1/b/mfb/managedFolders/a%2F/iam", "", http.StatusNotFound, "notFound"}, + {"a patch", http.MethodPatch, "/storage/v1/b/mfb/managedFolders/a%2F", `{}`, http.StatusNotFound, "notFound"}, + } { + t.Run(tc.name, func(t *testing.T) { + code, body := do(t, s, tc.method, tc.target, "application/json", tc.body) + if code != tc.status || !strings.Contains(body, `"reason":"`+tc.reason+`"`) { + t.Errorf("%s %s = %d %s, want %d %s", tc.method, tc.target, code, body, tc.status, tc.reason) + } + }) + } +} + +func TestManagedFolderList(t *testing.T) { + s := storage.New() + uniform(t, s, "mfb") + for _, name := range []string{"b/", "a/", "a/x/", "c/"} { + if _, err := s.PutManagedFolder("mfb", name); err != nil { + t.Fatal(err) + } + } + code, body := do(t, s, http.MethodGet, "/storage/v1/b/mfb/managedFolders?prefix=a", "", "") + if code != http.StatusOK || strings.Count(body, `"kind":"storage#managedFolder"`) != 2 || !strings.Contains(body, `"kind":"storage#managedFolders"`) { + t.Fatalf("list a = %d %s", code, body) + } + var got []string + token := "" + for { + code, body := do(t, s, http.MethodGet, "/storage/v1/b/mfb/managedFolders?pageSize=3&pageToken="+token, "", "") + if code != http.StatusOK { + t.Fatalf("list = %d %s", code, body) + } + var page struct { + Items []storage.ManagedFolder + NextPageToken string + } + if err := json.Unmarshal([]byte(body), &page); err != nil { + t.Fatal(err) + } + for _, f := range page.Items { + got = append(got, f.Name) + } + if page.NextPageToken == "" { + break + } + token = page.NextPageToken + } + if strings.Join(got, ",") != "a/,a/x/,b/,c/" { + t.Errorf("listed %v", got) + } +} + +func TestManagedFolderDeleteNeedsItEmpty(t *testing.T) { + s := storage.New() + uniform(t, s, "mfb") + for _, name := range []string{"a/", "a/b/", "c/"} { + if _, err := s.PutManagedFolder("mfb", name); err != nil { + t.Fatal(err) + } + } + if _, err := s.PutObject("mfb", "c/file", []byte("x")); err != nil { + t.Fatal(err) + } + for _, folder := range []string{"a%2F", "c%2F"} { + code, body := do(t, s, http.MethodDelete, "/storage/v1/b/mfb/managedFolders/"+folder, "", "") + if code != http.StatusConflict || !strings.Contains(body, "is not empty") { + t.Errorf("delete %s = %d %s", folder, code, body) + } + } + if code, body := do(t, s, http.MethodDelete, "/storage/v1/b/mfb/managedFolders/a%2Fb%2F", "", ""); code != http.StatusNoContent { + t.Errorf("delete a/b/ = %d %s", code, body) + } + if code, body := do(t, s, http.MethodDelete, "/storage/v1/b/mfb/managedFolders/a%2F", "", ""); code != http.StatusNoContent { + t.Errorf("delete emptied a/ = %d %s", code, body) + } + if code, body := do(t, s, http.MethodDelete, "/storage/v1/b/mfb/managedFolders/c%2F?allowNonEmpty=true", "", ""); code != http.StatusNoContent { + t.Errorf("delete c/ allowing non-empty = %d %s", code, body) + } + if objs := s.Objects("mfb"); len(objs) != 1 || objs[0].Name != "c/file" { + t.Errorf("objects after the folder went = %+v, want c/file kept", objs) + } + if got := s.ManagedFolders("mfb"); len(got) != 0 { + t.Errorf("ManagedFolders() = %+v, want none", got) + } +} + +func TestBucketWithManagedFoldersIsNotEmpty(t *testing.T) { + s := storage.New() + uniform(t, s, "mfb") + if _, err := s.PutManagedFolder("mfb", "a"); err != nil { + t.Fatal(err) + } + code, body := do(t, s, http.MethodDelete, "/storage/v1/b/mfb", "", "") + if code != http.StatusConflict || !strings.Contains(body, `"message":"The bucket you tried to delete is not empty."`) || !strings.Contains(body, `"reason":"conflict"`) { + t.Fatalf("delete bucket = %d %s", code, body) + } + code, body = do(t, s, http.MethodPatch, "/storage/v1/b/mfb", "application/json", `{"iamConfiguration":{"uniformBucketLevelAccess":{"enabled":false}}}`) + if code != http.StatusConflict { + t.Errorf("turning uniform access off = %d %s, want 409", code, body) + } + if code, body := do(t, s, http.MethodDelete, "/storage/v1/b/mfb/managedFolders/a%2F", "", ""); code != http.StatusNoContent { + t.Fatalf("delete folder = %d %s", code, body) + } + if code, body := do(t, s, http.MethodDelete, "/storage/v1/b/mfb", "", ""); code != http.StatusNoContent { + t.Errorf("delete emptied bucket = %d %s", code, body) + } +} + +func TestUniformAccessLocksAfter90Days(t *testing.T) { + var mu sync.Mutex + now := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + s := storage.New(storage.WithClock(func() time.Time { mu.Lock(); defer mu.Unlock(); return now })) + uniform(t, s, "locks") + off := `{"iamConfiguration":{"bucketPolicyOnly":{"enabled":false},"uniformBucketLevelAccess":{"enabled":false}}}` + if code, body := do(t, s, http.MethodPatch, "/storage/v1/b/locks", "application/json", off); code != http.StatusOK || !strings.Contains(body, `"uniformBucketLevelAccess":{"enabled":false}`) { + t.Fatalf("turning it off within 90 days = %d %s", code, body) + } + if code, body := do(t, s, http.MethodPatch, "/storage/v1/b/locks", "application/json", `{"iamConfiguration":{"bucketPolicyOnly":{"enabled":true}}}`); code != http.StatusOK { + t.Fatalf("turning it on = %d %s", code, body) + } + mu.Lock() + now = now.Add(90 * 24 * time.Hour) + mu.Unlock() + if code, body := do(t, s, http.MethodPatch, "/storage/v1/b/locks", "application/json", off); code != http.StatusBadRequest { + t.Errorf("turning it off after 90 days = %d %s, want 400", code, body) + } + for _, in := range []string{ + `{"iamConfiguration":{"bucketPolicyOnly":{"enabled":true},"uniformBucketLevelAccess":{"enabled":false}}}`, + `{"iamConfiguration":{"publicAccessPrevention":"enforced"}}`, + `{"iamConfiguration":null}`, + } { + if code, body := do(t, s, http.MethodPatch, "/storage/v1/b/locks", "application/json", in); code != http.StatusBadRequest { + t.Errorf("patch %s = %d %s, want 400", in, code, body) + } + } +} + +func TestPolicyNeedsSetIamPolicyToDeleteANonEmptyManagedFolder(t *testing.T) { + policy := access.Allow("storage.managedFolders.delete") + s := storage.New(storage.WithPolicy(policy)) + if _, err := s.PutBucket(storage.Bucket{Name: "mfb", Project: "p", IamConfiguration: &storage.IamConfiguration{UniformBucketLevelAccess: storage.UniformBucketLevelAccess{Enabled: true}}}); err != nil { + t.Fatal(err) + } + if _, err := s.PutManagedFolder("mfb", "a/"); err != nil { + t.Fatal(err) + } + code, body := do(t, s, http.MethodDelete, "/storage/v1/b/mfb/managedFolders/a%2F?allowNonEmpty=true", "", "") + if code != http.StatusForbidden || !strings.Contains(body, "Permission 'storage.managedFolders.setIamPolicy' denied on resource") { + t.Fatalf("delete allowing non-empty = %d %s", code, body) + } + if code, body := do(t, s, http.MethodDelete, "/storage/v1/b/mfb/managedFolders/a%2F", "", ""); code != http.StatusNoContent { + t.Errorf("delete = %d %s", code, body) + } + if code, body := do(t, s, http.MethodGet, "/storage/v1/b/mfb/managedFolders", "", ""); code != http.StatusForbidden || !strings.Contains(body, "storage.managedFolders.list") { + t.Errorf("list = %d %s", code, body) + } +} diff --git a/gcp/storage/objects.go b/gcp/storage/objects.go index aff2e07..4fdc8d9 100644 --- a/gcp/storage/objects.go +++ b/gcp/storage/objects.go @@ -614,7 +614,7 @@ func (s *Server) listObjects(r *http.Request) (any, *apiError) { } } } - items, next, err := page(r, all, versionKey) + items, next, err := page(r, "maxResults", all, versionKey) if err != nil { return nil, err } diff --git a/gcp/storage/permissions.go b/gcp/storage/permissions.go index 4ff9706..06fc2d3 100644 --- a/gcp/storage/permissions.go +++ b/gcp/storage/permissions.go @@ -35,6 +35,10 @@ var basePermissions = map[string][]string{ "storage.objects.delete": {"storage.objects.delete"}, "storage.objects.insert": {"storage.objects.create"}, "storage.objects.restore": {"storage.objects.restore", "storage.objects.create"}, + "storage.managedFolders.insert": {"storage.managedFolders.create"}, + "storage.managedFolders.list": {"storage.managedFolders.list"}, + "storage.managedFolders.get": {"storage.managedFolders.get"}, + "storage.managedFolders.delete": {"storage.managedFolders.delete"}, } // Requester Pays calls need serviceUsage with a userProject and createBillingAssignment without one. @@ -68,6 +72,8 @@ func (s *Server) needs(method, resource string, r *http.Request) []string { add(q.Get("overrideUnlockedRetention") == "true", "storage.objects.overrideUnlockedRetention") case "storage.objects.restore": add(s.exists(resource), "storage.objects.delete") + case "storage.managedFolders.delete": + add(q.Get("allowNonEmpty") == "true", "storage.managedFolders.setIamPolicy") } if method != "storage.buckets.insert" && method != "storage.buckets.list" && s.requesterPays(resource) { add(q.Get("userProject") != "", serviceUsage) diff --git a/gcp/storage/server.go b/gcp/storage/server.go index eaa46fc..daef2db 100644 --- a/gcp/storage/server.go +++ b/gcp/storage/server.go @@ -13,7 +13,8 @@ // // - buckets: insert, get, list, patch, delete, lockRetentionPolicy, restore; // - objects: insert by multipart or media upload, get (metadata only), -// patch, update, list, delete, restore. +// patch, update, list, delete, restore; +// - managedFolders: insert, get, list, delete. // // Other methods answer 404. Where it differs from Cloud Storage: // @@ -33,11 +34,11 @@ // and the rest regions. Project numbers are derived from project IDs. // - Writable bucket fields other than name, location, storageClass, // labels, versioning, cors, customPlacementConfig, encryption, rpo, -// retentionPolicy, defaultEventBasedHold, softDeletePolicy, billing and -// objectRetention are rejected with 400, as are other unknown fields and -// ACLs other than private and projectPrivate. cors, encryption and rpo -// are recorded, not enforced. There are no IAM policies on buckets or -// objects, and no lifecycle, autoclass, hardDeletePause or hierarchical +// retentionPolicy, defaultEventBasedHold, softDeletePolicy, billing, +// objectRetention and iamConfiguration are rejected with 400, as are +// other unknown fields and ACLs other than private and projectPrivate. +// cors, encryption and rpo are recorded, not enforced. There are no IAM +// policies on buckets, objects or managed folders, and no lifecycle, autoclass, hardDeletePause or hierarchical // namespace. buckets.patch has patch semantics: it merges labels, // deleting one set to null, and clears labels, versioning, cors, // encryption, rpo, retentionPolicy, defaultEventBasedHold or billing set @@ -59,6 +60,36 @@ // generation noncurrent, and objects.list with versions=true lists those // too, ordered by name and then generation. Deleting a bucket fails with // 409 conflict while any generation, live or noncurrent, remains. +// - iamConfiguration takes uniformBucketLevelAccess and its former name +// bucketPolicyOnly, which must agree (400 invalid otherwise, inferred), +// and is reported under both once set; publicAccessPrevention other +// than inherited is rejected with 400. Turning uniform bucket-level +// access on sets a lockedTime 90 days later, after which turning it off +// fails with 400 invalid, and while managed folders remain with 409 +// conflict (statuses and messages inferred). ACLs are not checked +// against it. +// - Managed folders can be used only in a bucket with uniform bucket-level +// access: every managedFolders call on another bucket fails with 409 +// conflict. Cloud Storage answers that as FAILED_PRECONDITION over gRPC; +// the JSON API's 409 for it, as for its other failed preconditions on a +// resource's state, and the message are inferred. A name ends with "/", and one sent without it gets one +// (inferred); it is 1 to 1,024 bytes of UTF-8 with at most 15 "/"s, no +// carriage return or line feed, not "." or ".." and not under +// .well-known/acme-challenge/, with 400 invalid otherwise (message +// inferred). Inserting one that exists fails with 409 conflict, and a +// missing one answers 404 notFound (both messages inferred). id is the +// bucket and name joined by "/", and selfLink the folder's path with the +// name escaped (both inferred); metageneration stays 1, as nothing +// changes a managed folder. Deleting one that holds any object +// generation, live or noncurrent, or another managed folder under its +// name fails with 409 conflict (status, reason and message inferred), +// unless allowNonEmpty=true, which leaves +// the objects and nested managed folders in place (the nested ones +// inferred). A bucket with a managed folder left is not empty, so +// deleting it fails with the 409 conflict for objects, whose message is +// inferred to be the same. managedFolders.list takes prefix, pageToken +// and pageSize (at most 1,000), and lists in name order. update, the IAM +// policy methods and rapidCacheConfig are not supported. // - A retention policy's retentionPeriod is greater than 0 and at most // 3,155,760,000 seconds (100 years), and its effectiveTime is when it // was set or its period last changed (inferred). Each object reports @@ -163,8 +194,10 @@ // // Fault method names are the API's method IDs, such as // "storage.buckets.insert". A call's resource is its path below -// /storage/v1/, such as "b/my-bucket" or "b/my-bucket/o/my-object"; an -// insert names the bucket or object it creates, and buckets.list names +// /storage/v1/, such as "b/my-bucket", "b/my-bucket/o/my-object" or +// "b/my-bucket/managedFolders/logs/", unescaped and with the managed +// folder's trailing "/"; an insert names the bucket, object or managed +// folder it creates, and buckets.list names // "projects/my-project/b". The JSON API has no long-running operations, so // [fault.ModeFailOperation] fails a call as [fault.ModeReject] does. // @@ -176,7 +209,9 @@ // storage.objects.delete, as does a restore that replaces one; a body that // includes retention, even null, needs storage.objects.setRetention, as does // an objects.update that removes it by leaving it out (inferred), and -// overrideUnlockedRetention=true storage.objects.overrideUnlockedRetention. +// overrideUnlockedRetention=true storage.objects.overrideUnlockedRetention; +// a managedFolders.delete with allowNonEmpty=true needs +// storage.managedFolders.setIamPolicy. // projection=full needs nothing more: Cloud Storage leaves out the ACLs a // caller may not read rather than refusing the call, and the official // clients ask for it on every get and list. Permissions are not checked @@ -304,6 +339,12 @@ func (s *Server) registerRoutes() { s.route("POST "+bucket+"/lockRetentionPolicy", "storage.buckets.lockRetentionPolicy", s.lockRetentionPolicy, bucketTarget, "ifMetagenerationMatch") s.route("POST "+bucket+"/restore", "storage.buckets.restore", s.restoreBucket, bucketTarget, "generation", "projection") + s.route("POST "+bucket+"/managedFolders", "storage.managedFolders.insert", s.insertFolder, folderInsertTarget) + s.route("GET "+bucket+"/managedFolders", "storage.managedFolders.list", s.listFolders, nil, "prefix", "pageToken", "pageSize") + s.route("GET "+bucket+"/managedFolders/{folder}", "storage.managedFolders.get", s.getFolder, folderTarget, + "ifMetagenerationMatch", "ifMetagenerationNotMatch") + s.route("DELETE "+bucket+"/managedFolders/{folder}", "storage.managedFolders.delete", s.deleteFolder, folderTarget, + "allowNonEmpty", "ifMetagenerationMatch", "ifMetagenerationNotMatch") s.route("GET "+bucket+"/o", "storage.objects.list", s.listObjects, nil, "projection", "prefix", "delimiter", "pageToken", "maxResults", "versions", "startOffset", "endOffset", "includeTrailingDelimiter", "matchGlob", "includeFoldersAsPrefixes", "softDeleted", "filter") @@ -579,14 +620,14 @@ func checkMatch(r *http.Request, match, notMatch string, value int64) *apiError return nil } -// page returns the key-ordered items after the page token, the last key of the page before, up to maxResults. -func page[T any](r *http.Request, items []T, key func(T) string) ([]T, string, *apiError) { +// page returns the key-ordered items after the page token, the last key of the page before, up to the sizeParam parameter. +func page[T any](r *http.Request, sizeParam string, items []T, key func(T) string) ([]T, string, *apiError) { limit := maxList - if n, ok, err := int64Param(r, "maxResults"); err != nil { + if n, ok, err := int64Param(r, sizeParam); err != nil { return nil, "", err } else if ok { if n < 0 { - return nil, "", invalid("Invalid value for maxResults: %d", n) + return nil, "", invalid("Invalid value for %s: %d", sizeParam, n) } if n > 0 && n < maxList { limit = int(n) diff --git a/sdktest/gcp/storage/folders_test.go b/sdktest/gcp/storage/folders_test.go new file mode 100644 index 0000000..8237353 --- /dev/null +++ b/sdktest/gcp/storage/folders_test.go @@ -0,0 +1,142 @@ +package storage_test + +import ( + "errors" + "net/http" + "slices" + "testing" + "time" + + gcs "cloud.google.com/go/storage" + "google.golang.org/api/googleapi" + rawstorage "google.golang.org/api/storage/v1" + + "github.com/parallelworks/squall/access" + "github.com/parallelworks/squall/gcp/storage" +) + +func uniformBucket(t *testing.T, c *gcs.Client, name string) { + t.Helper() + attrs := &gcs.BucketAttrs{UniformBucketLevelAccess: gcs.UniformBucketLevelAccess{Enabled: true}} + if err := c.Bucket(name).Create(t.Context(), project, attrs); err != nil { + t.Fatalf("create %s: %v", name, err) + } +} + +func wantCode(t *testing.T, what string, err error, code int) { + t.Helper() + gerr, ok := errors.AsType[*googleapi.Error](err) + if !ok || gerr.Code != code { + t.Errorf("%s = %v, want %d", what, err, code) + } +} + +func TestManagedFolders(t *testing.T) { + url := serve(t, storage.New()) + c := clientAt(t, url) + svc := rawService(t, url) + uniformBucket(t, c, "folders") + + attrs, err := c.Bucket("folders").Attrs(t.Context()) + if err != nil { + t.Fatal(err) + } + if !attrs.UniformBucketLevelAccess.Enabled || !attrs.BucketPolicyOnly.Enabled || attrs.UniformBucketLevelAccess.LockedTime.Sub(attrs.Created) != 90*24*time.Hour { + t.Errorf("uniform access = %+v, created %v", attrs.UniformBucketLevelAccess, attrs.Created) + } + + created, err := svc.ManagedFolders.Insert("folders", &rawstorage.ManagedFolder{Name: "team/reports/"}).Context(t.Context()).Do() + if err != nil { + t.Fatal(err) + } + if created.Kind != "storage#managedFolder" || created.Bucket != "folders" || created.Metageneration != 1 || created.CreateTime == "" || created.UpdateTime != created.CreateTime { + t.Errorf("created = %+v", created) + } + _, err = svc.ManagedFolders.Insert("folders", &rawstorage.ManagedFolder{Name: "team/reports/"}).Context(t.Context()).Do() + wantCode(t, "insert again", err, http.StatusConflict) + if _, err := svc.ManagedFolders.Insert("folders", &rawstorage.ManagedFolder{Name: "team/"}).Context(t.Context()).Do(); err != nil { + t.Fatal(err) + } + + got, err := svc.ManagedFolders.Get("folders", "team/reports/").IfMetagenerationMatch(1).Context(t.Context()).Do() + if err != nil || got.Name != "team/reports/" || got.Id != created.Id { + t.Errorf("get = %+v, %v", got, err) + } + _, err = svc.ManagedFolders.Get("folders", "team/other/").Context(t.Context()).Do() + wantCode(t, "get a missing folder", err, http.StatusNotFound) + + var listed []string + err = svc.ManagedFolders.List("folders").Prefix("team/").PageSize(1).Pages(t.Context(), func(page *rawstorage.ManagedFolders) error { + for _, f := range page.Items { + listed = append(listed, f.Name) + } + return nil + }) + if err != nil || !slices.Equal(listed, []string{"team/", "team/reports/"}) { + t.Errorf("list = %v, %v", listed, err) + } + + write(t, c, "folders", "team/reports/q1.csv", "1,2") + wantCode(t, "delete a folder holding an object", svc.ManagedFolders.Delete("folders", "team/reports/").Context(t.Context()).Do(), http.StatusConflict) + wantCode(t, "delete a folder holding a folder", svc.ManagedFolders.Delete("folders", "team/").Context(t.Context()).Do(), http.StatusConflict) + wantCode(t, "delete on a stale metageneration", svc.ManagedFolders.Delete("folders", "team/").IfMetagenerationMatch(2).AllowNonEmpty(true).Context(t.Context()).Do(), http.StatusPreconditionFailed) + wantCode(t, "delete the bucket", c.Bucket("folders").Delete(t.Context()), http.StatusConflict) + _, err = c.Bucket("folders").Update(t.Context(), gcs.BucketAttrsToUpdate{UniformBucketLevelAccess: &gcs.UniformBucketLevelAccess{Enabled: false}}) + wantCode(t, "turn off uniform access", err, http.StatusConflict) + + if err := svc.ManagedFolders.Delete("folders", "team/").AllowNonEmpty(true).Context(t.Context()).Do(); err != nil { + t.Fatalf("delete allowing non-empty: %v", err) + } + if err := svc.ManagedFolders.Delete("folders", "team/reports/").AllowNonEmpty(true).IfMetagenerationNotMatch(2).Context(t.Context()).Do(); err != nil { + t.Fatalf("delete allowing non-empty: %v", err) + } + if _, err := c.Bucket("folders").Object("team/reports/q1.csv").Attrs(t.Context()); err != nil { + t.Errorf("the object after its folder went: %v", err) + } + if err := c.Bucket("folders").Object("team/reports/q1.csv").Delete(t.Context()); err != nil { + t.Fatal(err) + } + if err := c.Bucket("folders").Delete(t.Context()); err != nil { + t.Errorf("delete the emptied bucket: %v", err) + } +} + +func TestManagedFoldersNeedUniformAccess(t *testing.T) { + url := serve(t, storage.New()) + c := clientAt(t, url) + svc := rawService(t, url) + if err := c.Bucket("acls").Create(t.Context(), project, nil); err != nil { + t.Fatal(err) + } + _, err := svc.ManagedFolders.Insert("acls", &rawstorage.ManagedFolder{Name: "a/"}).Context(t.Context()).Do() + wantCode(t, "insert", err, http.StatusConflict) + + if _, err := c.Bucket("acls").Update(t.Context(), gcs.BucketAttrsToUpdate{UniformBucketLevelAccess: &gcs.UniformBucketLevelAccess{Enabled: true}}); err != nil { + t.Fatal(err) + } + if _, err := svc.ManagedFolders.Insert("acls", &rawstorage.ManagedFolder{Name: "a/"}).Context(t.Context()).Do(); err != nil { + t.Errorf("insert once uniform access is on: %v", err) + } +} + +func TestPolicyChecksManagedFolderPermissions(t *testing.T) { + policy := access.Allow("storage.buckets.create", "storage.managedFolders.create", "storage.managedFolders.get", "storage.managedFolders.delete") + url := serve(t, storage.New(storage.WithPolicy(policy))) + c := clientAt(t, url) + svc := rawService(t, url) + uniformBucket(t, c, "guarded") + if _, err := svc.ManagedFolders.Insert("guarded", &rawstorage.ManagedFolder{Name: "a/"}).Context(t.Context()).Do(); err != nil { + t.Fatal(err) + } + if _, err := svc.ManagedFolders.Get("guarded", "a/").Context(t.Context()).Do(); err != nil { + t.Error(err) + } + wantCode(t, "list", svc.ManagedFolders.List("guarded").Pages(t.Context(), func(*rawstorage.ManagedFolders) error { return nil }), http.StatusForbidden) + wantCode(t, "delete allowing non-empty", svc.ManagedFolders.Delete("guarded", "a/").AllowNonEmpty(true).Context(t.Context()).Do(), http.StatusForbidden) + if err := svc.ManagedFolders.Delete("guarded", "a/").Context(t.Context()).Do(); err != nil { + t.Error(err) + } + if got := policy.Unused(); len(got) != 0 { + t.Errorf("Unused() = %v, want none", got) + } +} From 0890b95307613e7492327e538a2f785ab73a6c30 Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Thu, 5 Nov 2026 18:25:22 -0600 Subject: [PATCH 2/3] fix(storage): managed folder review fixes Accept maxResults beside pageSize, refuse predefined ACLs under uniform bucket-level access, record and report publicAccessPrevention, and cite sources for the managed folder rules. --- README.md | 6 +- gcp/storage/buckets.go | 57 +++++++++--- gcp/storage/folders.go | 2 +- gcp/storage/folders_test.go | 53 ++++++++++- gcp/storage/objects.go | 8 +- gcp/storage/server.go | 132 +++++++++++++++++----------- sdktest/gcp/storage/folders_test.go | 41 +++++++++ 7 files changed, 225 insertions(+), 74 deletions(-) diff --git a/README.md b/README.md index 01ebe20..8a51f5c 100644 --- a/README.md +++ b/README.md @@ -432,8 +432,10 @@ For Cloud Storage: folders fails unless `allowNonEmpty=true`, which needs `storage.managedFolders.setIamPolicy` under `WithPolicy` and leaves what it held in place. A bucket with a managed folder left cannot be deleted, nor - can its uniform bucket-level access be turned off. `PutManagedFolder` makes - one directly. Managed folder IAM policies are not supported. + can its uniform bucket-level access be turned off. While uniform access is + on, predefined ACLs fail with 400. Public access prevention is recorded and + reported, not enforced. `PutManagedFolder` makes one directly. Managed + folder IAM policies are not supported. - A fault rule's `Resource` is the path below `/storage/v1/`, such as `b/my-bucket/o/my-object` or `b/my-bucket/managedFolders/logs/`, and for an insert the bucket, object or managed folder it makes. diff --git a/gcp/storage/buckets.go b/gcp/storage/buckets.go index a9c7e9f..2ce34ae 100644 --- a/gcp/storage/buckets.go +++ b/gcp/storage/buckets.go @@ -95,10 +95,11 @@ type BucketObjectRetention struct { Mode string `json:"mode"` } -// IamConfiguration is a bucket's uniform bucket-level access setting, reported under both of its names. +// IamConfiguration is a bucket's uniform bucket-level access setting, reported under both of its names, and public access prevention. type IamConfiguration struct { BucketPolicyOnly UniformBucketLevelAccess `json:"bucketPolicyOnly"` UniformBucketLevelAccess UniformBucketLevelAccess `json:"uniformBucketLevelAccess"` + PublicAccessPrevention string `json:"publicAccessPrevention"` } // UniformBucketLevelAccess says whether only IAM governs access to a bucket, and until when that can be turned off. @@ -350,8 +351,8 @@ func (in *iamConfigurationInput) uniformAccess() (*bool, *apiError) { if in == nil { return nil, nil } - if in.PublicAccessPrevention != "" && in.PublicAccessPrevention != "inherited" { - return nil, invalid("squall: publicAccessPrevention=%s is not supported", in.PublicAccessPrevention) + if p := in.PublicAccessPrevention; p != "" && p != "inherited" && p != "enforced" { + return nil, invalid("Invalid argument: publicAccessPrevention '%s' is not valid.", p) } var ubla, bpo *bool if in.UniformBucketLevelAccess != nil { @@ -369,12 +370,32 @@ func (in *iamConfigurationInput) uniformAccess() (*bool, *apiError) { // uniformAccessLock is how long after uniform bucket-level access is turned on it can still be turned off. const uniformAccessLock = 90 * 24 * time.Hour -func iamConfiguration(enabled bool, t time.Time) *IamConfiguration { +// iamConfiguration is the setting with uniform bucket-level access turned on or off at t, and public access prevention, inherited if empty. +func iamConfiguration(enabled bool, publicAccess string, t time.Time) *IamConfiguration { u := UniformBucketLevelAccess{Enabled: enabled} if enabled { u.LockedTime = t.Add(uniformAccessLock).UTC().Format(timeFormat) } - return &IamConfiguration{BucketPolicyOnly: u, UniformBucketLevelAccess: u} + return &IamConfiguration{BucketPolicyOnly: u, UniformBucketLevelAccess: u, PublicAccessPrevention: cmp.Or(publicAccess, "inherited")} +} + +func publicAccessPrevention(in *iamConfigurationInput) string { + if in == nil { + return "" + } + return in.PublicAccessPrevention +} + +// aclRequested reports whether r sets a predefined ACL. +func aclRequested(r *http.Request) bool { + return r.URL.Query().Get("predefinedAcl") != "" || r.URL.Query().Get("predefinedDefaultObjectAcl") != "" +} + +func uniformAccessACL() *apiError { + return &apiError{ + Status: http.StatusBadRequest, Reason: "badRequest", + Message: "Cannot use ACLs on a bucket or its objects while uniform bucket-level access is enabled.", + } } type softDeletePolicyInput struct { @@ -483,8 +504,9 @@ func (s *Server) insertBucket(r *http.Request) (any, *apiError) { if err != nil { return nil, err } - if uniform != nil { - b.IamConfiguration = &IamConfiguration{UniformBucketLevelAccess: UniformBucketLevelAccess{Enabled: *uniform}} + b.IamConfiguration = &IamConfiguration{UniformBucketLevelAccess: UniformBucketLevelAccess{Enabled: isSet(uniform)}, PublicAccessPrevention: publicAccessPrevention(in.IamConfiguration)} + if isSet(uniform) && aclRequested(r) { + return nil, uniformAccessACL() } if objectRetention { b.ObjectRetention = &BucketObjectRetention{Mode: "Enabled"} @@ -574,7 +596,9 @@ func (s *Server) store(b Bucket) (*bucket, *apiError) { b.SoftDeletePolicy.EffectiveTime = now } if c := b.IamConfiguration; c != nil { - b.IamConfiguration = iamConfiguration(c.UniformBucketLevelAccess.Enabled || c.BucketPolicyOnly.Enabled, t) + b.IamConfiguration = iamConfiguration(c.UniformBucketLevelAccess.Enabled || c.BucketPolicyOnly.Enabled, c.PublicAccessPrevention, t) + } else { + b.IamConfiguration = iamConfiguration(false, "", t) } b.Location = strings.ToUpper(cmp.Or(b.Location, "US")) b.LocationType = locationType(b.Location, b.CustomPlacementConfig) @@ -695,10 +719,13 @@ func (s *Server) patchBucket(r *http.Request) (any, *apiError) { if err != nil { return nil, err } - iam, err := nextIamConfiguration(b, uniform, changed) + iam, err := nextIamConfiguration(b, uniform, publicAccessPrevention(in.IamConfiguration), changed) if err != nil { return nil, err } + if iam.UniformBucketLevelAccess.Enabled && aclRequested(r) { + return nil, uniformAccessACL() + } if len(labels) == 0 { labels = nil } @@ -734,10 +761,12 @@ func (s *Server) patchBucket(r *http.Request) (any, *apiError) { return b.clone(), nil } -// nextIamConfiguration is b's iamConfiguration once uniform, if not nil, turns uniform bucket-level access on or off at t. -func nextIamConfiguration(b *bucket, uniform *bool, t time.Time) (*IamConfiguration, *apiError) { +// nextIamConfiguration is b's iamConfiguration once uniform, if not nil, turns uniform bucket-level access on or off at t, and publicAccess, if set, replaces its public access prevention. +func nextIamConfiguration(b *bucket, uniform *bool, publicAccess string, t time.Time) (*IamConfiguration, *apiError) { + next := new(*b.IamConfiguration) + next.PublicAccessPrevention = cmp.Or(publicAccess, next.PublicAccessPrevention) if uniform == nil || *uniform == b.uniformAccess() { - return b.IamConfiguration, nil + return next, nil } if !*uniform { if len(b.folders) > 0 { @@ -750,7 +779,7 @@ func nextIamConfiguration(b *bucket, uniform *bool, t time.Time) (*IamConfigurat return nil, invalid("Uniform bucket-level access cannot be disabled after it has been enabled for 90 days.") } } - return iamConfiguration(*uniform, t), nil + return iamConfiguration(*uniform, next.PublicAccessPrevention, t), nil } func (s *Server) deleteBucket(r *http.Request) (any, *apiError) { @@ -810,7 +839,7 @@ func (s *Server) listBuckets(r *http.Request) (any, *apiError) { } } } - items, next, err := page(r, "maxResults", all, key) + items, next, err := page(r, all, key, "maxResults") if err != nil { return nil, err } diff --git a/gcp/storage/folders.go b/gcp/storage/folders.go index 23513d7..c0cceb3 100644 --- a/gcp/storage/folders.go +++ b/gcp/storage/folders.go @@ -213,7 +213,7 @@ func (s *Server) listFolders(r *http.Request) (any, *apiError) { return nil, err } all := b.managedFolders(prefix) - items, next, err := page(r, "pageSize", all, func(f ManagedFolder) string { return f.Name }) + items, next, err := page(r, all, func(f ManagedFolder) string { return f.Name }, "pageSize", "maxResults") if err != nil { return nil, err } diff --git a/gcp/storage/folders_test.go b/gcp/storage/folders_test.go index f483092..94ef227 100644 --- a/gcp/storage/folders_test.go +++ b/gcp/storage/folders_test.go @@ -36,7 +36,7 @@ func TestManagedFolderShape(t *testing.T) { } } code, body = do(t, s, http.MethodGet, "/storage/v1/b/mfb", "", "") - if code != http.StatusOK || !strings.Contains(body, `"iamConfiguration":{"bucketPolicyOnly":{"enabled":true,"lockedTime":"2026-04-02T03:04:05.000Z"},"uniformBucketLevelAccess":{"enabled":true,"lockedTime":"2026-04-02T03:04:05.000Z"}}`) { + if code != http.StatusOK || !strings.Contains(body, `"iamConfiguration":{"bucketPolicyOnly":{"enabled":true,"lockedTime":"2026-04-02T03:04:05.000Z"},"uniformBucketLevelAccess":{"enabled":true,"lockedTime":"2026-04-02T03:04:05.000Z"},"publicAccessPrevention":"inherited"}`) { t.Errorf("get bucket = %d %s", code, body) } } @@ -69,7 +69,7 @@ func TestManagedFolderErrors(t *testing.T) { {"a delete of a missing folder", http.MethodDelete, "/storage/v1/b/mfb/managedFolders/b%2F", "", http.StatusNotFound, "notFound"}, {"a metageneration mismatch", http.MethodGet, "/storage/v1/b/mfb/managedFolders/a%2F?ifMetagenerationMatch=2", "", http.StatusPreconditionFailed, "conditionNotMet"}, {"a delete on a metageneration match", http.MethodDelete, "/storage/v1/b/mfb/managedFolders/a%2F?ifMetagenerationNotMatch=1", "", http.StatusPreconditionFailed, "conditionNotMet"}, - {"maxResults", http.MethodGet, "/storage/v1/b/mfb/managedFolders?maxResults=1", "", http.StatusBadRequest, "invalid"}, + {"page sizes that disagree", http.MethodGet, "/storage/v1/b/mfb/managedFolders?pageSize=1&maxResults=2", "", http.StatusBadRequest, "invalid"}, {"a negative page size", http.MethodGet, "/storage/v1/b/mfb/managedFolders?pageSize=-1", "", http.StatusBadRequest, "invalid"}, {"a bad allowNonEmpty", http.MethodDelete, "/storage/v1/b/mfb/managedFolders/a%2F?allowNonEmpty=yes", "", http.StatusBadRequest, "invalid"}, {"an IAM policy", http.MethodGet, "/storage/v1/b/mfb/managedFolders/a%2F/iam", "", http.StatusNotFound, "notFound"}, @@ -99,7 +99,7 @@ func TestManagedFolderList(t *testing.T) { var got []string token := "" for { - code, body := do(t, s, http.MethodGet, "/storage/v1/b/mfb/managedFolders?pageSize=3&pageToken="+token, "", "") + code, body := do(t, s, http.MethodGet, "/storage/v1/b/mfb/managedFolders?maxResults=3&pageToken="+token, "", "") if code != http.StatusOK { t.Fatalf("list = %d %s", code, body) } @@ -199,7 +199,7 @@ func TestUniformAccessLocksAfter90Days(t *testing.T) { } for _, in := range []string{ `{"iamConfiguration":{"bucketPolicyOnly":{"enabled":true},"uniformBucketLevelAccess":{"enabled":false}}}`, - `{"iamConfiguration":{"publicAccessPrevention":"enforced"}}`, + `{"iamConfiguration":{"publicAccessPrevention":"open"}}`, `{"iamConfiguration":null}`, } { if code, body := do(t, s, http.MethodPatch, "/storage/v1/b/locks", "application/json", in); code != http.StatusBadRequest { @@ -228,3 +228,48 @@ func TestPolicyNeedsSetIamPolicyToDeleteANonEmptyManagedFolder(t *testing.T) { t.Errorf("list = %d %s", code, body) } } + +func TestBucketsReportIamConfiguration(t *testing.T) { + s := storage.New() + code, body := do(t, s, http.MethodPost, "/storage/v1/b?project=p", "application/json", `{"name":"plain"}`) + if code != http.StatusOK || !strings.Contains(body, `"iamConfiguration":{"bucketPolicyOnly":{"enabled":false},"uniformBucketLevelAccess":{"enabled":false},"publicAccessPrevention":"inherited"}`) { + t.Fatalf("insert = %d %s", code, body) + } + code, body = do(t, s, http.MethodPatch, "/storage/v1/b/plain", "application/json", `{"iamConfiguration":{"publicAccessPrevention":"enforced"}}`) + if code != http.StatusOK || !strings.Contains(body, `"uniformBucketLevelAccess":{"enabled":false},"publicAccessPrevention":"enforced"`) { + t.Fatalf("patch = %d %s", code, body) + } + code, body = do(t, s, http.MethodPatch, "/storage/v1/b/plain", "application/json", `{"iamConfiguration":{"uniformBucketLevelAccess":{"enabled":true}}}`) + if code != http.StatusOK || !strings.Contains(body, `"publicAccessPrevention":"enforced"`) { + t.Errorf("patch keeping public access prevention = %d %s", code, body) + } +} + +func TestUniformAccessRefusesACLs(t *testing.T) { + s := storage.New() + uniform(t, s, "uniform") + if code, body := do(t, s, http.MethodPost, "/storage/v1/b?project=p", "application/json", `{"name":"fine-grained"}`); code != http.StatusOK { + t.Fatalf("insert bucket = %d %s", code, body) + } + if _, err := s.PutObject("uniform", "k", []byte("x")); err != nil { + t.Fatal(err) + } + for _, tc := range []struct{ name, method, target, body string }{ + {"a bucket insert", http.MethodPost, "/storage/v1/b?project=p&predefinedAcl=private", strings.Replace(uniformBucket, "%s", "acl-bucket", 1)}, + {"a bucket patch", http.MethodPatch, "/storage/v1/b/uniform?predefinedDefaultObjectAcl=projectPrivate", `{}`}, + {"a patch turning it on", http.MethodPatch, "/storage/v1/b/fine-grained?predefinedAcl=private", `{"iamConfiguration":{"uniformBucketLevelAccess":{"enabled":true}}}`}, + {"an upload", http.MethodPost, "/upload/storage/v1/b/uniform/o?uploadType=media&name=n&predefinedAcl=private", "x"}, + {"an object patch", http.MethodPatch, "/storage/v1/b/uniform/o/k?predefinedAcl=projectPrivate", `{}`}, + {"an object update", http.MethodPut, "/storage/v1/b/uniform/o/k?predefinedAcl=private", `{}`}, + } { + t.Run(tc.name, func(t *testing.T) { + code, body := do(t, s, tc.method, tc.target, "application/json", tc.body) + if code != http.StatusBadRequest || !strings.Contains(body, `"reason":"badRequest"`) { + t.Errorf("%s %s = %d %s, want 400 badRequest", tc.method, tc.target, code, body) + } + }) + } + if code, body := do(t, s, http.MethodPatch, "/storage/v1/b/fine-grained?predefinedAcl=private", "application/json", `{}`); code != http.StatusOK { + t.Errorf("an ACL without uniform access = %d %s", code, body) + } +} diff --git a/gcp/storage/objects.go b/gcp/storage/objects.go index 4fdc8d9..fa79ac0 100644 --- a/gcp/storage/objects.go +++ b/gcp/storage/objects.go @@ -241,6 +241,9 @@ func (s *Server) insertObject(r *http.Request) (any, *apiError) { } s.mu.Lock() defer s.mu.Unlock() + if b := s.buckets[bucketName]; b != nil && b.uniformAccess() && aclRequested(r) { + return nil, uniformAccessACL() + } o, err := s.writeObject(bucketName, in, data, func(live int64) *apiError { if live == 0 && r.URL.Query().Has("ifGenerationNotMatch") { return conditionNotMet() @@ -493,6 +496,9 @@ func (s *Server) changeObject(r *http.Request, replace bool) (any, *apiError) { if err != nil { return nil, err } + if b.uniformAccess() && aclRequested(r) { + return nil, uniformAccessACL() + } if in.Bucket != "" && in.Bucket != b.Name || in.Name != "" && in.Name != o.Name { return nil, invalid("squall: an object's bucket and name cannot be changed") } @@ -614,7 +620,7 @@ func (s *Server) listObjects(r *http.Request) (any, *apiError) { } } } - items, next, err := page(r, "maxResults", all, versionKey) + items, next, err := page(r, all, versionKey, "maxResults") if err != nil { return nil, err } diff --git a/gcp/storage/server.go b/gcp/storage/server.go index daef2db..596aa4d 100644 --- a/gcp/storage/server.go +++ b/gcp/storage/server.go @@ -37,21 +37,22 @@ // retentionPolicy, defaultEventBasedHold, softDeletePolicy, billing, // objectRetention and iamConfiguration are rejected with 400, as are // other unknown fields and ACLs other than private and projectPrivate. -// cors, encryption and rpo are recorded, not enforced. There are no IAM -// policies on buckets, objects or managed folders, and no lifecycle, autoclass, hardDeletePause or hierarchical -// namespace. buckets.patch has patch semantics: it merges labels, -// deleting one set to null, and clears labels, versioning, cors, -// encryption, rpo, retentionPolicy, defaultEventBasedHold or billing set -// to null; a null softDeletePolicy is rejected with 400. Insert and -// patch ignore the fields the server sets, effectiveTime and isLocked in -// the policies included, and a patch accepts the insert-only name, -// location and customPlacementConfig only unchanged, so the body a get -// returns can be sent back. objectRetention is set only by -// enableObjectRetention on insert, since the docs give only the console -// for turning it on for an existing bucket, so a body's objectRetention -// must match it: on insert what the parameter gives, and on patch what -// the bucket has, with 400 otherwise. Every bucket reports a generation -// (inferred). +// cors, encryption, rpo and iamConfiguration.publicAccessPrevention are +// recorded, not enforced. There are no IAM policies on buckets, objects +// or managed folders, and no lifecycle, autoclass, hardDeletePause or +// hierarchical namespace. buckets.patch has patch semantics: it merges +// labels, deleting one set to null, and clears labels, versioning, +// cors, encryption, rpo, retentionPolicy, defaultEventBasedHold or +// billing set to null; a null softDeletePolicy is rejected with 400. +// Insert and patch ignore the fields the server sets, effectiveTime and +// isLocked in the policies included, and a patch accepts the +// insert-only name, location and customPlacementConfig only unchanged, +// so the body a get returns can be sent back. objectRetention is set +// only by enableObjectRetention on insert, since the docs give only the +// console for turning it on for an existing bucket, so a body's +// objectRetention must match it: on insert what the parameter gives, +// and on patch what the bucket has, with 400 otherwise. Every bucket +// reports a generation (inferred). // - Objects record their size, MD5 and CRC32C, not their data; a crc32c or // md5Hash in the upload's metadata must match the data. Resumable // uploads, media reads (objects.get with alt=media answers 404), copies, @@ -62,34 +63,47 @@ // 409 conflict while any generation, live or noncurrent, remains. // - iamConfiguration takes uniformBucketLevelAccess and its former name // bucketPolicyOnly, which must agree (400 invalid otherwise, inferred), -// and is reported under both once set; publicAccessPrevention other -// than inherited is rejected with 400. Turning uniform bucket-level -// access on sets a lockedTime 90 days later, after which turning it off -// fails with 400 invalid, and while managed folders remain with 409 -// conflict (statuses and messages inferred). ACLs are not checked -// against it. -// - Managed folders can be used only in a bucket with uniform bucket-level -// access: every managedFolders call on another bucket fails with 409 -// conflict. Cloud Storage answers that as FAILED_PRECONDITION over gRPC; -// the JSON API's 409 for it, as for its other failed preconditions on a -// resource's state, and the message are inferred. A name ends with "/", and one sent without it gets one -// (inferred); it is 1 to 1,024 bytes of UTF-8 with at most 15 "/"s, no -// carriage return or line feed, not "." or ".." and not under -// .well-known/acme-challenge/, with 400 invalid otherwise (message -// inferred). Inserting one that exists fails with 409 conflict, and a -// missing one answers 404 notFound (both messages inferred). id is the -// bucket and name joined by "/", and selfLink the folder's path with the -// name escaped (both inferred); metageneration stays 1, as nothing -// changes a managed folder. Deleting one that holds any object -// generation, live or noncurrent, or another managed folder under its -// name fails with 409 conflict (status, reason and message inferred), -// unless allowNonEmpty=true, which leaves -// the objects and nested managed folders in place (the nested ones -// inferred). A bucket with a managed folder left is not empty, so -// deleting it fails with the 409 conflict for objects, whose message is -// inferred to be the same. managedFolders.list takes prefix, pageToken -// and pageSize (at most 1,000), and lists in name order. update, the IAM -// policy methods and rapidCacheConfig are not supported. +// and publicAccessPrevention, inherited by default or enforced. Every +// bucket reports all three, as Cloud Storage does. Turning uniform +// bucket-level access on sets a lockedTime 90 days later, after which +// turning it off fails with 400 invalid, and while managed folders +// remain with 409 conflict (statuses and messages inferred), as the +// docs say they must be deleted first. While it is on, a predefinedAcl +// or predefinedDefaultObjectAcl on buckets.insert, buckets.patch, an +// upload, objects.patch or objects.update fails with 400 badRequest, as +// the uniform bucket-level access docs and the status codes reference +// give it; the message is inferred. acl and defaultObjectAcl bodies are +// rejected with 400 on every bucket, and copies, rewrites and composes +// are not supported. +// - Managed folders can be used only in a bucket with uniform +// bucket-level access, as the docs require. Every managedFolders call +// on another bucket fails with 409 conflict: the Go client library's +// tests expect FAILED_PRECONDITION for this over gRPC, and the JSON +// API's 409, as it answers a non-empty bucket's delete, and the message +// are inferred. A name ends with "/", and one sent without it gets one, +// as the reference's examples name folders without it (inferred). It is +// 1 to 1,024 bytes of UTF-8 with at most 15 "/"s, no carriage return or +// line feed, not "." or ".." and not under .well-known/acme-challenge/, +// with 400 invalid otherwise (message inferred). Inserting one that +// exists fails with 409 conflict, and a missing one answers 404 +// notFound (both messages inferred). id is the bucket and name joined +// by "/", and selfLink the folder's path with the name escaped (both +// inferred); metageneration stays 1, as nothing changes a managed +// folder. Deleting one that holds an object or another managed folder +// under its name fails with 409 conflict (status, reason and message +// inferred); noncurrent generations count (inferred), and soft-deleted +// ones do not. allowNonEmpty=true deletes it anyway and leaves the +// objects in place, as the simulated folder the reference says is +// retained implies, and nested managed folders too (inferred). A bucket +// with a managed folder left is not empty, as the bucket deletion docs +// imply, so deleting it fails with the 409 conflict for objects, whose +// message is inferred to be the same; a soft-deleted bucket so holds no +// managed folders to restore, and the docs say managed folders cannot +// be recovered. managedFolders.list takes prefix, pageToken and a page +// size of at most 1,000, as pageSize, which the discovery document and +// the Go client use, or maxResults, which the reference names; both, if +// set, must agree. It lists in name order. update, the IAM policy +// methods and rapidCacheConfig are not supported. // - A retention policy's retentionPeriod is greater than 0 and at most // 3,155,760,000 seconds (100 years), and its effectiveTime is when it // was set or its period last changed (inferred). Each object reports @@ -340,7 +354,8 @@ func (s *Server) registerRoutes() { "ifMetagenerationMatch") s.route("POST "+bucket+"/restore", "storage.buckets.restore", s.restoreBucket, bucketTarget, "generation", "projection") s.route("POST "+bucket+"/managedFolders", "storage.managedFolders.insert", s.insertFolder, folderInsertTarget) - s.route("GET "+bucket+"/managedFolders", "storage.managedFolders.list", s.listFolders, nil, "prefix", "pageToken", "pageSize") + s.route("GET "+bucket+"/managedFolders", "storage.managedFolders.list", s.listFolders, nil, + "prefix", "pageToken", "pageSize", "maxResults") s.route("GET "+bucket+"/managedFolders/{folder}", "storage.managedFolders.get", s.getFolder, folderTarget, "ifMetagenerationMatch", "ifMetagenerationNotMatch") s.route("DELETE "+bucket+"/managedFolders/{folder}", "storage.managedFolders.delete", s.deleteFolder, folderTarget, @@ -620,18 +635,31 @@ func checkMatch(r *http.Request, match, notMatch string, value int64) *apiError return nil } -// page returns the key-ordered items after the page token, the last key of the page before, up to the sizeParam parameter. -func page[T any](r *http.Request, sizeParam string, items []T, key func(T) string) ([]T, string, *apiError) { +// page returns the key-ordered items after the page token, and the last key +// of the page before, up to the page size that the first set of sizeParams +// gives; sizeParams that are both set must agree. +func page[T any](r *http.Request, items []T, key func(T) string, sizeParams ...string) ([]T, string, *apiError) { limit := maxList - if n, ok, err := int64Param(r, sizeParam); err != nil { - return nil, "", err - } else if ok { + var size int64 + sized := "" + for _, p := range sizeParams { + n, ok, err := int64Param(r, p) + if err != nil { + return nil, "", err + } + if !ok { + continue + } if n < 0 { - return nil, "", invalid("Invalid value for %s: %d", sizeParam, n) + return nil, "", invalid("Invalid value for %s: %d", p, n) } - if n > 0 && n < maxList { - limit = int(n) + if sized != "" && n != size { + return nil, "", invalid("squall: %s and %s disagree", sized, p) } + size, sized = n, p + } + if size > 0 && size < maxList { + limit = int(size) } if token := r.URL.Query().Get("pageToken"); token != "" { after, err := base64.RawURLEncoding.DecodeString(token) diff --git a/sdktest/gcp/storage/folders_test.go b/sdktest/gcp/storage/folders_test.go index 8237353..3f01615 100644 --- a/sdktest/gcp/storage/folders_test.go +++ b/sdktest/gcp/storage/folders_test.go @@ -140,3 +140,44 @@ func TestPolicyChecksManagedFolderPermissions(t *testing.T) { t.Errorf("Unused() = %v, want none", got) } } + +func TestPublicAccessPreventionIsRecorded(t *testing.T) { + c := newClient(t, storage.New()) + if err := c.Bucket("plain").Create(t.Context(), project, nil); err != nil { + t.Fatal(err) + } + attrs, err := c.Bucket("plain").Attrs(t.Context()) + if err != nil { + t.Fatal(err) + } + if attrs.PublicAccessPrevention != gcs.PublicAccessPreventionInherited || attrs.UniformBucketLevelAccess.Enabled { + t.Errorf("default = %v, %+v", attrs.PublicAccessPrevention, attrs.UniformBucketLevelAccess) + } + + secure := &gcs.BucketAttrs{UniformBucketLevelAccess: gcs.UniformBucketLevelAccess{Enabled: true}, PublicAccessPrevention: gcs.PublicAccessPreventionEnforced} + if err := c.Bucket("secure").Create(t.Context(), project, secure); err != nil { + t.Fatal(err) + } + attrs, err = c.Bucket("secure").Attrs(t.Context()) + if err != nil || attrs.PublicAccessPrevention != gcs.PublicAccessPreventionEnforced || !attrs.UniformBucketLevelAccess.Enabled { + t.Fatalf("secure = %+v, %v", attrs, err) + } + attrs, err = c.Bucket("secure").Update(t.Context(), gcs.BucketAttrsToUpdate{PublicAccessPrevention: gcs.PublicAccessPreventionInherited}) + if err != nil || attrs.PublicAccessPrevention != gcs.PublicAccessPreventionInherited || !attrs.UniformBucketLevelAccess.Enabled { + t.Errorf("update = %+v, %v", attrs, err) + } +} + +func TestUniformAccessRefusesPredefinedACLs(t *testing.T) { + c := newClient(t, storage.New()) + uniformBucket(t, c, "uniform") + w := c.Bucket("uniform").Object("a.txt").NewWriter(t.Context()) + w.PredefinedACL = "private" + if _, err := w.Write([]byte("x")); err != nil { + t.Fatal(err) + } + wantCode(t, "upload with a predefined ACL", w.Close(), http.StatusBadRequest) + write(t, c, "uniform", "a.txt", "x") + _, err := c.Bucket("uniform").Update(t.Context(), gcs.BucketAttrsToUpdate{PredefinedDefaultObjectACL: "projectPrivate"}) + wantCode(t, "bucket update with a predefined default object ACL", err, http.StatusBadRequest) +} From 3bfdb6cd1d9bf1e14bce9c8d8c85633dd57f1cee Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Thu, 5 Nov 2026 18:36:07 -0600 Subject: [PATCH 3/3] fix(storage): uniform access naming, object ACL errors and restore ACLs uniformBucketLevelAccess wins over bucketPolicyOnly, object ACLs under uniform access answer Cloud Storage's 400 invalid, objects.restore refuses copySourceAcl under uniform access and needs storage.objects.setIamPolicy without it, PutBucket checks publicAccessPrevention, and a page size of 0 counts as unset. --- gcp/storage/buckets.go | 31 ++++++++----- gcp/storage/folders_test.go | 67 ++++++++++++++++++++++++----- gcp/storage/objects.go | 4 +- gcp/storage/permissions.go | 10 +++++ gcp/storage/server.go | 36 ++++++++++------ gcp/storage/softdelete.go | 6 ++- sdktest/gcp/storage/folders_test.go | 27 +++++++++++- 7 files changed, 143 insertions(+), 38 deletions(-) diff --git a/gcp/storage/buckets.go b/gcp/storage/buckets.go index 2ce34ae..eaca895 100644 --- a/gcp/storage/buckets.go +++ b/gcp/storage/buckets.go @@ -346,13 +346,13 @@ type iamConfigurationInput struct { PublicAccessPrevention string `json:"publicAccessPrevention"` } -// uniformAccess returns whether in turns uniform bucket-level access on or off, nil if it does neither. +// uniformAccess returns whether in turns uniform bucket-level access on or off, nil if it does neither; uniformBucketLevelAccess wins over bucketPolicyOnly. func (in *iamConfigurationInput) uniformAccess() (*bool, *apiError) { if in == nil { return nil, nil } - if p := in.PublicAccessPrevention; p != "" && p != "inherited" && p != "enforced" { - return nil, invalid("Invalid argument: publicAccessPrevention '%s' is not valid.", p) + if err := checkPublicAccessPrevention(in.PublicAccessPrevention); err != nil { + return nil, err } var ubla, bpo *bool if in.UniformBucketLevelAccess != nil { @@ -361,9 +361,6 @@ func (in *iamConfigurationInput) uniformAccess() (*bool, *apiError) { if in.BucketPolicyOnly != nil { bpo = in.BucketPolicyOnly.Enabled } - if ubla != nil && bpo != nil && *ubla != *bpo { - return nil, invalid("Invalid argument: uniformBucketLevelAccess and bucketPolicyOnly must match.") - } return cmp.Or(ubla, bpo), nil } @@ -391,13 +388,24 @@ func aclRequested(r *http.Request) bool { return r.URL.Query().Get("predefinedAcl") != "" || r.URL.Query().Get("predefinedDefaultObjectAcl") != "" } -func uniformAccessACL() *apiError { +func checkPublicAccessPrevention(p string) *apiError { + if p != "" && p != "inherited" && p != "enforced" { + return invalid("Invalid argument: publicAccessPrevention '%s' is not valid.", p) + } + return nil +} + +func uniformAccessBucketACL() *apiError { return &apiError{ Status: http.StatusBadRequest, Reason: "badRequest", - Message: "Cannot use ACLs on a bucket or its objects while uniform bucket-level access is enabled.", + Message: "Cannot use ACLs on a bucket while uniform bucket-level access is enabled.", } } +func uniformAccessObjectACL() *apiError { + return invalid("Cannot insert legacy ACL for an object when uniform bucket-level access is enabled. Read more at https://cloud.google.com/storage/docs/uniform-bucket-level-access") +} + type softDeletePolicyInput struct { RetentionDurationSeconds *int64Value `json:"retentionDurationSeconds"` EffectiveTime json.RawMessage `json:"effectiveTime"` @@ -506,7 +514,7 @@ func (s *Server) insertBucket(r *http.Request) (any, *apiError) { } b.IamConfiguration = &IamConfiguration{UniformBucketLevelAccess: UniformBucketLevelAccess{Enabled: isSet(uniform)}, PublicAccessPrevention: publicAccessPrevention(in.IamConfiguration)} if isSet(uniform) && aclRequested(r) { - return nil, uniformAccessACL() + return nil, uniformAccessBucketACL() } if objectRetention { b.ObjectRetention = &BucketObjectRetention{Mode: "Enabled"} @@ -596,6 +604,9 @@ func (s *Server) store(b Bucket) (*bucket, *apiError) { b.SoftDeletePolicy.EffectiveTime = now } if c := b.IamConfiguration; c != nil { + if err := checkPublicAccessPrevention(c.PublicAccessPrevention); err != nil { + return nil, err + } b.IamConfiguration = iamConfiguration(c.UniformBucketLevelAccess.Enabled || c.BucketPolicyOnly.Enabled, c.PublicAccessPrevention, t) } else { b.IamConfiguration = iamConfiguration(false, "", t) @@ -724,7 +735,7 @@ func (s *Server) patchBucket(r *http.Request) (any, *apiError) { return nil, err } if iam.UniformBucketLevelAccess.Enabled && aclRequested(r) { - return nil, uniformAccessACL() + return nil, uniformAccessBucketACL() } if len(labels) == 0 { labels = nil diff --git a/gcp/storage/folders_test.go b/gcp/storage/folders_test.go index 94ef227..5b2d5da 100644 --- a/gcp/storage/folders_test.go +++ b/gcp/storage/folders_test.go @@ -2,6 +2,7 @@ package storage_test import ( "encoding/json" + "fmt" "net/http" "strings" "sync" @@ -70,6 +71,7 @@ func TestManagedFolderErrors(t *testing.T) { {"a metageneration mismatch", http.MethodGet, "/storage/v1/b/mfb/managedFolders/a%2F?ifMetagenerationMatch=2", "", http.StatusPreconditionFailed, "conditionNotMet"}, {"a delete on a metageneration match", http.MethodDelete, "/storage/v1/b/mfb/managedFolders/a%2F?ifMetagenerationNotMatch=1", "", http.StatusPreconditionFailed, "conditionNotMet"}, {"page sizes that disagree", http.MethodGet, "/storage/v1/b/mfb/managedFolders?pageSize=1&maxResults=2", "", http.StatusBadRequest, "invalid"}, + {"a page size of 0 beside another", http.MethodGet, "/storage/v1/b/mfb/managedFolders?pageSize=0&maxResults=2", "", http.StatusOK, ""}, {"a negative page size", http.MethodGet, "/storage/v1/b/mfb/managedFolders?pageSize=-1", "", http.StatusBadRequest, "invalid"}, {"a bad allowNonEmpty", http.MethodDelete, "/storage/v1/b/mfb/managedFolders/a%2F?allowNonEmpty=yes", "", http.StatusBadRequest, "invalid"}, {"an IAM policy", http.MethodGet, "/storage/v1/b/mfb/managedFolders/a%2F/iam", "", http.StatusNotFound, "notFound"}, @@ -77,7 +79,7 @@ func TestManagedFolderErrors(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { code, body := do(t, s, tc.method, tc.target, "application/json", tc.body) - if code != tc.status || !strings.Contains(body, `"reason":"`+tc.reason+`"`) { + if code != tc.status || tc.reason != "" && !strings.Contains(body, `"reason":"`+tc.reason+`"`) { t.Errorf("%s %s = %d %s, want %d %s", tc.method, tc.target, code, body, tc.status, tc.reason) } }) @@ -198,7 +200,6 @@ func TestUniformAccessLocksAfter90Days(t *testing.T) { t.Errorf("turning it off after 90 days = %d %s, want 400", code, body) } for _, in := range []string{ - `{"iamConfiguration":{"bucketPolicyOnly":{"enabled":true},"uniformBucketLevelAccess":{"enabled":false}}}`, `{"iamConfiguration":{"publicAccessPrevention":"open"}}`, `{"iamConfiguration":null}`, } { @@ -254,22 +255,66 @@ func TestUniformAccessRefusesACLs(t *testing.T) { if _, err := s.PutObject("uniform", "k", []byte("x")); err != nil { t.Fatal(err) } - for _, tc := range []struct{ name, method, target, body string }{ - {"a bucket insert", http.MethodPost, "/storage/v1/b?project=p&predefinedAcl=private", strings.Replace(uniformBucket, "%s", "acl-bucket", 1)}, - {"a bucket patch", http.MethodPatch, "/storage/v1/b/uniform?predefinedDefaultObjectAcl=projectPrivate", `{}`}, - {"a patch turning it on", http.MethodPatch, "/storage/v1/b/fine-grained?predefinedAcl=private", `{"iamConfiguration":{"uniformBucketLevelAccess":{"enabled":true}}}`}, - {"an upload", http.MethodPost, "/upload/storage/v1/b/uniform/o?uploadType=media&name=n&predefinedAcl=private", "x"}, - {"an object patch", http.MethodPatch, "/storage/v1/b/uniform/o/k?predefinedAcl=projectPrivate", `{}`}, - {"an object update", http.MethodPut, "/storage/v1/b/uniform/o/k?predefinedAcl=private", `{}`}, + if _, err := s.PutSoftDeletedObject("uniform", "gone", []byte("x")); err != nil { + t.Fatal(err) + } + gone := s.SoftDeletedObjects("uniform")[0].Generation + const legacy = "Cannot insert legacy ACL for an object when uniform bucket-level access is enabled." + for _, tc := range []struct{ name, method, target, body, reason string }{ + {"a bucket insert", http.MethodPost, "/storage/v1/b?project=p&predefinedAcl=private", strings.Replace(uniformBucket, "%s", "acl-bucket", 1), "badRequest"}, + {"a bucket patch", http.MethodPatch, "/storage/v1/b/uniform?predefinedDefaultObjectAcl=projectPrivate", `{}`, "badRequest"}, + {"a patch turning it on", http.MethodPatch, "/storage/v1/b/fine-grained?predefinedAcl=private", `{"iamConfiguration":{"uniformBucketLevelAccess":{"enabled":true}}}`, "badRequest"}, + {"an upload", http.MethodPost, "/upload/storage/v1/b/uniform/o?uploadType=media&name=n&predefinedAcl=private", "x", "invalid"}, + {"an object patch", http.MethodPatch, "/storage/v1/b/uniform/o/k?predefinedAcl=projectPrivate", `{}`, "invalid"}, + {"an object update", http.MethodPut, "/storage/v1/b/uniform/o/k?predefinedAcl=private", `{}`, "invalid"}, + {"a restore copying the ACL", http.MethodPost, fmt.Sprintf("/storage/v1/b/uniform/o/gone/restore?generation=%d©SourceAcl=true", gone), "", "invalid"}, } { t.Run(tc.name, func(t *testing.T) { code, body := do(t, s, tc.method, tc.target, "application/json", tc.body) - if code != http.StatusBadRequest || !strings.Contains(body, `"reason":"badRequest"`) { - t.Errorf("%s %s = %d %s, want 400 badRequest", tc.method, tc.target, code, body) + if code != http.StatusBadRequest || !strings.Contains(body, `"reason":"`+tc.reason+`"`) || tc.reason == "invalid" && !strings.Contains(body, legacy) { + t.Errorf("%s %s = %d %s, want 400 %s", tc.method, tc.target, code, body, tc.reason) } }) } + if code, body := do(t, s, http.MethodPost, fmt.Sprintf("/storage/v1/b/uniform/o/gone/restore?generation=%d", gone), "", ""); code != http.StatusOK { + t.Errorf("restore without copySourceAcl = %d %s", code, body) + } if code, body := do(t, s, http.MethodPatch, "/storage/v1/b/fine-grained?predefinedAcl=private", "application/json", `{}`); code != http.StatusOK { t.Errorf("an ACL without uniform access = %d %s", code, body) } } + +func TestUniformAccessNameWinsOverItsFormerName(t *testing.T) { + s := storage.New() + uniform(t, s, "names") + code, body := do(t, s, http.MethodPatch, "/storage/v1/b/names", "application/json", `{"iamConfiguration":{"bucketPolicyOnly":{"enabled":true},"uniformBucketLevelAccess":{"enabled":false}}}`) + if code != http.StatusOK || !strings.Contains(body, `"bucketPolicyOnly":{"enabled":false}`) { + t.Errorf("patch = %d %s", code, body) + } +} + +func TestPutBucketChecksPublicAccessPrevention(t *testing.T) { + s := storage.New() + if _, err := s.PutBucket(storage.Bucket{Name: "pap", Project: "p", IamConfiguration: &storage.IamConfiguration{PublicAccessPrevention: "bogus"}}); err == nil { + t.Error("PutBucket with publicAccessPrevention bogus succeeded") + } + b, err := s.PutBucket(storage.Bucket{Name: "pap", Project: "p", IamConfiguration: &storage.IamConfiguration{PublicAccessPrevention: "enforced"}}) + if err != nil || b.IamConfiguration.PublicAccessPrevention != "enforced" { + t.Errorf("PutBucket = %+v, %v", b.IamConfiguration, err) + } +} + +func TestPolicyNeedsSetIamPolicyToCopyARestoredACL(t *testing.T) { + policy := access.Allow("storage.objects.restore", "storage.objects.create") + s := storage.New(storage.WithPolicy(policy)) + if _, err := s.PutBucket(storage.Bucket{Name: "acls", Project: "p"}); err != nil { + t.Fatal(err) + } + if _, err := s.PutSoftDeletedObject("acls", "gone", []byte("x")); err != nil { + t.Fatal(err) + } + target := fmt.Sprintf("/storage/v1/b/acls/o/gone/restore?generation=%d©SourceAcl=true", s.SoftDeletedObjects("acls")[0].Generation) + if code, body := do(t, s, http.MethodPost, target, "", ""); code != http.StatusForbidden || !strings.Contains(body, "storage.objects.setIamPolicy") { + t.Errorf("restore = %d %s, want 403 naming storage.objects.setIamPolicy", code, body) + } +} diff --git a/gcp/storage/objects.go b/gcp/storage/objects.go index fa79ac0..4e4c9eb 100644 --- a/gcp/storage/objects.go +++ b/gcp/storage/objects.go @@ -242,7 +242,7 @@ func (s *Server) insertObject(r *http.Request) (any, *apiError) { s.mu.Lock() defer s.mu.Unlock() if b := s.buckets[bucketName]; b != nil && b.uniformAccess() && aclRequested(r) { - return nil, uniformAccessACL() + return nil, uniformAccessObjectACL() } o, err := s.writeObject(bucketName, in, data, func(live int64) *apiError { if live == 0 && r.URL.Query().Has("ifGenerationNotMatch") { @@ -497,7 +497,7 @@ func (s *Server) changeObject(r *http.Request, replace bool) (any, *apiError) { return nil, err } if b.uniformAccess() && aclRequested(r) { - return nil, uniformAccessACL() + return nil, uniformAccessObjectACL() } if in.Bucket != "" && in.Bucket != b.Name || in.Name != "" && in.Name != o.Name { return nil, invalid("squall: an object's bucket and name cannot be changed") diff --git a/gcp/storage/permissions.go b/gcp/storage/permissions.go index 06fc2d3..db12863 100644 --- a/gcp/storage/permissions.go +++ b/gcp/storage/permissions.go @@ -72,6 +72,7 @@ func (s *Server) needs(method, resource string, r *http.Request) []string { add(q.Get("overrideUnlockedRetention") == "true", "storage.objects.overrideUnlockedRetention") case "storage.objects.restore": add(s.exists(resource), "storage.objects.delete") + add(q.Get("copySourceAcl") == "true" && !s.uniform(resource), "storage.objects.setIamPolicy") case "storage.managedFolders.delete": add(q.Get("allowNonEmpty") == "true", "storage.managedFolders.setIamPolicy") } @@ -132,6 +133,15 @@ func (s *Server) requesterPays(resource string) bool { return b != nil && b.requesterPays() } +// uniform reports whether resource, as "b/" or a path below it, is in a bucket with uniform bucket-level access. +func (s *Server) uniform(resource string) bool { + name, _, _ := strings.Cut(strings.TrimPrefix(resource, "b/"), "/") + s.mu.Lock() + defer s.mu.Unlock() + b := s.buckets[name] + return b != nil && b.uniformAccess() +} + // exists reports whether resource, as "b//o/", names a live object. func (s *Server) exists(resource string) bool { bucketName, name, ok := strings.Cut(strings.TrimPrefix(resource, "b/"), "/o/") diff --git a/gcp/storage/server.go b/gcp/storage/server.go index 596aa4d..d7d2515 100644 --- a/gcp/storage/server.go +++ b/gcp/storage/server.go @@ -62,19 +62,28 @@ // too, ordered by name and then generation. Deleting a bucket fails with // 409 conflict while any generation, live or noncurrent, remains. // - iamConfiguration takes uniformBucketLevelAccess and its former name -// bucketPolicyOnly, which must agree (400 invalid otherwise, inferred), -// and publicAccessPrevention, inherited by default or enforced. Every +// bucketPolicyOnly; when a body has both, uniformBucketLevelAccess +// wins, as the discovery document and the Go client say, so the body a +// get returns can be sent back with only it changed. It also takes +// publicAccessPrevention, inherited by default or enforced. Every // bucket reports all three, as Cloud Storage does. Turning uniform // bucket-level access on sets a lockedTime 90 days later, after which // turning it off fails with 400 invalid, and while managed folders // remain with 409 conflict (statuses and messages inferred), as the // docs say they must be deleted first. While it is on, a predefinedAcl -// or predefinedDefaultObjectAcl on buckets.insert, buckets.patch, an -// upload, objects.patch or objects.update fails with 400 badRequest, as -// the uniform bucket-level access docs and the status codes reference -// give it; the message is inferred. acl and defaultObjectAcl bodies are -// rejected with 400 on every bucket, and copies, rewrites and composes -// are not supported. +// on an upload, objects.patch or objects.update fails with 400 invalid +// and the message "Cannot insert legacy ACL for an object when uniform +// bucket-level access is enabled. Read more at +// https://cloud.google.com/storage/docs/uniform-bucket-level-access", +// which is what Cloud Storage answers, though the status codes +// reference lists the case under badRequest. objects.restore with +// copySourceAcl=true, which its reference applies only to buckets +// without uniform access, fails the same way (inferred). A +// predefinedAcl or predefinedDefaultObjectAcl on buckets.insert or +// buckets.patch fails with 400 badRequest, as the status codes +// reference gives it (message inferred). acl and defaultObjectAcl +// bodies are rejected with 400 on every bucket, and copies, rewrites +// and composes are not supported. // - Managed folders can be used only in a bucket with uniform // bucket-level access, as the docs require. Every managedFolders call // on another bucket fails with 409 conflict: the Go client library's @@ -225,7 +234,9 @@ // an objects.update that removes it by leaving it out (inferred), and // overrideUnlockedRetention=true storage.objects.overrideUnlockedRetention; // a managedFolders.delete with allowNonEmpty=true needs -// storage.managedFolders.setIamPolicy. +// storage.managedFolders.setIamPolicy, and an objects.restore with +// copySourceAcl=true in a bucket without uniform bucket-level access +// storage.objects.setIamPolicy. // projection=full needs nothing more: Cloud Storage leaves out the ACLs a // caller may not read rather than refusing the call, and the official // clients ask for it on every get and list. Permissions are not checked @@ -635,9 +646,7 @@ func checkMatch(r *http.Request, match, notMatch string, value int64) *apiError return nil } -// page returns the key-ordered items after the page token, and the last key -// of the page before, up to the page size that the first set of sizeParams -// gives; sizeParams that are both set must agree. +// page returns the key-ordered items after the page token, and the last key of the page before, up to the page size sizeParams give, which must agree. func page[T any](r *http.Request, items []T, key func(T) string, sizeParams ...string) ([]T, string, *apiError) { limit := maxList var size int64 @@ -653,6 +662,9 @@ func page[T any](r *http.Request, items []T, key func(T) string, sizeParams ...s if n < 0 { return nil, "", invalid("Invalid value for %s: %d", p, n) } + if n == 0 { + continue + } if sized != "" && n != size { return nil, "", invalid("squall: %s and %s disagree", sized, p) } diff --git a/gcp/storage/softdelete.go b/gcp/storage/softdelete.go index d0a4a71..94104a3 100644 --- a/gcp/storage/softdelete.go +++ b/gcp/storage/softdelete.go @@ -147,7 +147,8 @@ func (s *Server) restoreObject(r *http.Request) (any, *apiError) { if !ok { return nil, required("generation") } - if _, err := boolParam(r, "copySourceAcl"); err != nil { + copyACL, err := boolParam(r, "copySourceAcl") + if err != nil { return nil, err } bucketName, name := r.PathValue("bucket"), r.PathValue("object") @@ -157,6 +158,9 @@ func (s *Server) restoreObject(r *http.Request) (any, *apiError) { if err != nil { return nil, err } + if copyACL && b.uniformAccess() { + return nil, uniformAccessObjectACL() + } if b.softDeleteDuration() == 0 && len(b.trash) == 0 { return nil, &apiError{Status: http.StatusBadRequest, Reason: "SoftDeletePolicyRequired", Message: "Soft delete policy is required to restore soft-deleted objects."} } diff --git a/sdktest/gcp/storage/folders_test.go b/sdktest/gcp/storage/folders_test.go index 3f01615..3285bda 100644 --- a/sdktest/gcp/storage/folders_test.go +++ b/sdktest/gcp/storage/folders_test.go @@ -176,8 +176,31 @@ func TestUniformAccessRefusesPredefinedACLs(t *testing.T) { if _, err := w.Write([]byte("x")); err != nil { t.Fatal(err) } - wantCode(t, "upload with a predefined ACL", w.Close(), http.StatusBadRequest) + err := w.Close() + wantCode(t, "upload with a predefined ACL", err, http.StatusBadRequest) + if gerr, ok := errors.AsType[*googleapi.Error](err); !ok || len(gerr.Errors) != 1 || gerr.Errors[0].Reason != "invalid" { + t.Errorf("upload error = %v, want reason invalid", err) + } write(t, c, "uniform", "a.txt", "x") - _, err := c.Bucket("uniform").Update(t.Context(), gcs.BucketAttrsToUpdate{PredefinedDefaultObjectACL: "projectPrivate"}) + _, err = c.Bucket("uniform").Update(t.Context(), gcs.BucketAttrsToUpdate{PredefinedDefaultObjectACL: "projectPrivate"}) wantCode(t, "bucket update with a predefined default object ACL", err, http.StatusBadRequest) } + +func TestReadModifyWriteTurnsUniformAccessOff(t *testing.T) { + url := serve(t, storage.New()) + svc := rawService(t, url) + uniformBucket(t, clientAt(t, url), "rmw") + b, err := svc.Buckets.Get("rmw").Context(t.Context()).Do() + if err != nil { + t.Fatal(err) + } + b.IamConfiguration.UniformBucketLevelAccess.Enabled = false + b.IamConfiguration.UniformBucketLevelAccess.ForceSendFields = []string{"Enabled"} + got, err := svc.Buckets.Patch("rmw", b).Context(t.Context()).Do() + if err != nil { + t.Fatal(err) + } + if got.IamConfiguration.UniformBucketLevelAccess.Enabled || got.IamConfiguration.BucketPolicyOnly.Enabled { + t.Errorf("iamConfiguration = %+v", got.IamConfiguration) + } +}