diff --git a/go.mod b/go.mod index 09d22d57..02946c50 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,9 @@ module github.com/stackshy/cloudemu/v2 go 1.25.0 require ( + cloud.google.com/go/artifactregistry v1.20.0 cloud.google.com/go/compute v1.60.0 + cloud.google.com/go/eventarc v1.18.0 cloud.google.com/go/firestore v1.22.0 cloud.google.com/go/storage v1.62.1 github.com/Azure/azure-sdk-for-go/sdk/ai/azopenai v0.7.2 diff --git a/go.sum b/go.sum index 5744f34f..4ffd081a 100644 --- a/go.sum +++ b/go.sum @@ -2,6 +2,8 @@ cel.dev/expr v0.25.1 h1:1KrZg61W6TWSxuNZ37Xy49ps13NUovb66QLprthtwi4= cel.dev/expr v0.25.1/go.mod h1:hrXvqGP6G6gyx8UAHSHJ5RGk//1Oj5nXQ2NI02Nrsg4= cloud.google.com/go v0.123.0 h1:2NAUJwPR47q+E35uaJeYoNhuNEM9kM8SjgRgdeOJUSE= cloud.google.com/go v0.123.0/go.mod h1:xBoMV08QcqUGuPW65Qfm1o9Y4zKZBpGS+7bImXLTAZU= +cloud.google.com/go/artifactregistry v1.20.0 h1:j/XQiQfaeTyQeNj3HNk4iDFREVnY/fxkHIjsxpaDs8A= +cloud.google.com/go/artifactregistry v1.20.0/go.mod h1:0G9wdbGyDFkvrYH+2AlQs9MuTJdbY8Vg45M8VjlI8rc= cloud.google.com/go/auth v0.22.0 h1:Xp9wAKkLoeaYb5pYZZoQGz4E9sdPxIbzS3gywZE3ciQ= cloud.google.com/go/auth v0.22.0/go.mod h1:M9o2Oz+YI2jAfxewJgb1vyI3vceHF+eohmxyzmrl+9s= cloud.google.com/go/auth/oauth2adapt v0.2.8 h1:keo8NaayQZ6wimpNSmW5OPc283g65QNIiLpZnkHRbnc= @@ -10,6 +12,8 @@ cloud.google.com/go/compute v1.60.0 h1:CqGt23ysz990ZZe1vq/9aDPKKnmwM6kcC7Y1Q05H2 cloud.google.com/go/compute v1.60.0/go.mod h1:Xm6PbsLgBpAg4va77ljbBdpMjzuU+uPp5Ze2dnZq7lw= cloud.google.com/go/compute/metadata v0.9.0 h1:pDUj4QMoPejqq20dK0Pg2N4yG9zIkYGdBtwLoEkH9Zs= cloud.google.com/go/compute/metadata v0.9.0/go.mod h1:E0bWwX5wTnLPedCKqk3pJmVgCBSM6qQI1yTBdEb3C10= +cloud.google.com/go/eventarc v1.18.0 h1:8WWG1/ogInYur1NQjML6EMHQ0ZBzAdMDGlUVpLD56cI= +cloud.google.com/go/eventarc v1.18.0/go.mod h1:/6SDoqh5+9QNUqCX4/oQcJVK16fG/snHBSXu7lrJtO8= cloud.google.com/go/firestore v1.22.0 h1:avooeboIq37vKXobrbPUFhFBxS/c3FqmWoX0xs8dO6E= cloud.google.com/go/firestore v1.22.0/go.mod h1:PaM4i7i7ruALSKmlpHXXZaPObcZw0W7ie5UOPr72iTU= cloud.google.com/go/iam v1.7.0 h1:JD3zh0C6LHl16aCn5Akff0+GELdp1+4hmh6ndoFLl8U= diff --git a/providers/gcp/gce/gce.go b/providers/gcp/gce/gce.go index 905b53c9..7b726604 100644 --- a/providers/gcp/gce/gce.go +++ b/providers/gcp/gce/gce.go @@ -291,6 +291,20 @@ func (m *Mock) TerminateInstances(ctx context.Context, instanceIDs []string) err return m.transitionInstances(ctx, instanceIDs, terminateTransition) } +// RemoveInstance hard-deletes an instance, mirroring GCP's instances.delete +// (which removes the resource, unlike EC2 terminate which leaves a TERMINATED +// tombstone). GCP-specific; reached via a type assertion from the GCE handler. +func (m *Mock) RemoveInstance(_ context.Context, instanceID string) error { + if !m.instances.Has(instanceID) { + return cerrors.Newf(cerrors.NotFound, "instance %q not found", instanceID) + } + + m.instances.Delete(instanceID) + m.sm.Remove(instanceID) + + return nil +} + func (m *Mock) DescribeInstances( _ context.Context, instanceIDs []string, filters []driver.DescribeFilter, _ ...driver.DescribeInstancesOptions, ) ([]driver.Instance, error) { @@ -514,8 +528,13 @@ func (m *Mock) DescribeSnapshots(_ context.Context, ids []string) ([]driver.Snap } func (m *Mock) CreateImage(_ context.Context, cfg driver.ImageConfig) (*driver.ImageInfo, error) { - if _, ok := m.instances.Get(cfg.InstanceID); !ok { - return nil, cerrors.Newf(cerrors.NotFound, "instance %q not found", cfg.InstanceID) + // GCP images are created from a disk, snapshot, or import — not from a + // source instance. An empty InstanceID is one of those source-based paths, + // so only validate when a specific instance was named (the EC2-style path). + if cfg.InstanceID != "" { + if _, ok := m.instances.Get(cfg.InstanceID); !ok { + return nil, cerrors.Newf(cerrors.NotFound, "instance %q not found", cfg.InstanceID) + } } id := fmt.Sprintf("projects/%s/global/images/img-%d", diff --git a/providers/gcp/gke/gke.go b/providers/gcp/gke/gke.go index a65a2129..920e138a 100644 --- a/providers/gcp/gke/gke.go +++ b/providers/gcp/gke/gke.go @@ -60,6 +60,8 @@ type Cluster struct { IPRotationActive bool NodePoolNames []string Status string + MasterVersion string + NodeVersion string CreatedAt time.Time } @@ -391,6 +393,14 @@ func (m *Mock) UpdateCluster( if input.ResourceLabels != nil { c.ResourceLabels = copyLabels(input.ResourceLabels) } + + if input.MasterVersion != "" { + c.MasterVersion = input.MasterVersion + } + + if input.NodeVersion != "" { + c.NodeVersion = input.NodeVersion + } }) } diff --git a/providers/gcp/secretmanager/secretmanager.go b/providers/gcp/secretmanager/secretmanager.go index 995f37e6..ce317a9b 100644 --- a/providers/gcp/secretmanager/secretmanager.go +++ b/providers/gcp/secretmanager/secretmanager.go @@ -65,20 +65,22 @@ func (m *Mock) CreateSecret(_ context.Context, cfg driver.SecretConfig, value [] Tags: tags, } - data := make([]byte, len(value)) - copy(data, value) - - versionID := idgen.GenerateID("ver-") - version := driver.SecretVersion{ - VersionID: versionID, - Value: data, - CreatedAt: now, - Current: true, - } - - sd := &secretData{ - info: info, - versions: []driver.SecretVersion{version}, + sd := &secretData{info: info} + + // GCP's secrets.create makes an empty container — the first version is added + // separately via addVersion. Only seed a version when a value is actually + // supplied (the AWS-style create-with-value path); otherwise the secret has + // zero versions and access(latest) fails until one is added, matching GCP. + if len(value) > 0 { + data := make([]byte, len(value)) + copy(data, value) + + sd.versions = []driver.SecretVersion{{ + VersionID: idgen.GenerateID("ver-"), + Value: data, + CreatedAt: now, + Current: true, + }} } m.secrets.Set(cfg.Name, sd) diff --git a/server/gcp/alloydb/helpers.go b/server/gcp/alloydb/helpers.go index 6bcf022e..977bb879 100644 --- a/server/gcp/alloydb/helpers.go +++ b/server/gcp/alloydb/helpers.go @@ -100,7 +100,7 @@ func (*Handler) toWireCluster(c *rdsdriver.Cluster, info *rdsdriver.AlloyDBClust DatabaseVersion: info.DatabaseVersion, Network: info.Network, ClusterType: info.ClusterType, - State: "READY", + State: alloyDBState(c.State), Uid: c.ID, ContinuousBackupConfig: &alloydb.ContinuousBackupConfig{ Enabled: info.ContinuousBackup, @@ -125,12 +125,34 @@ func (*Handler) toWireInstance(inst *rdsdriver.Instance, info *rdsdriver.AlloyDB AvailabilityType: info.AvailabilityType, IpAddress: info.IPAddress, GceZone: info.GceZone, - State: "READY", + State: alloyDBState(inst.State), Uid: inst.ID, MachineConfig: &alloydb.MachineConfig{CpuCount: int64(info.CPUCount)}, } } +// alloyDBState maps the relationaldb driver's lifecycle state to AlloyDB's +// wire state enum, so a just-created or stopped resource reports its real +// state instead of always "READY". +const stateReady = "READY" + +func alloyDBState(driverState string) string { + switch driverState { + case rdsdriver.StateAvailable, "": + return stateReady + case rdsdriver.StateCreating, rdsdriver.StateStarting: + return "CREATING" + case rdsdriver.StateDeleting: + return "DELETING" + case rdsdriver.StateStopped, rdsdriver.StateStopping: + return "STOPPED" + case rdsdriver.StateModifying, rdsdriver.StateRebooting, rdsdriver.StateBackingUp: + return "MAINTENANCE" + default: + return stateReady + } +} + func toWireBackup(s *rdsdriver.ClusterSnapshot, backupType string) *alloydb.Backup { return &alloydb.Backup{ Name: s.ARN, diff --git a/server/gcp/artifactregistry/gapic_lro_test.go b/server/gcp/artifactregistry/gapic_lro_test.go new file mode 100644 index 00000000..da3b2708 --- /dev/null +++ b/server/gcp/artifactregistry/gapic_lro_test.go @@ -0,0 +1,57 @@ +package artifactregistry_test + +import ( + "context" + "net/http/httptest" + "testing" + + artifactregistry "cloud.google.com/go/artifactregistry/apiv1" + "cloud.google.com/go/artifactregistry/apiv1/artifactregistrypb" + "google.golang.org/api/option" + + "github.com/stackshy/cloudemu/v2" + gcpserver "github.com/stackshy/cloudemu/v2/server/gcp" +) + +// TestGAPICCreateRepositoryWait is the review's #3 check: the finding targeted +// the GAPIC apiv1 client's LRO .Wait(), which the raw google.golang.org/api +// REST client never exercised. This drives the real apiv1 REST client end to +// end — CreateRepository(...).Wait() must resolve (not 404, and not a decode +// error from a missing response @type) and return the created repository. +func TestGAPICCreateRepositoryWait(t *testing.T) { + cloud := cloudemu.NewGCP() + srv := gcpserver.New(gcpserver.DriversFrom(cloud)) // full server: exercises real dispatch + ts := httptest.NewServer(srv) + t.Cleanup(ts.Close) + + ctx := context.Background() + + client, err := artifactregistry.NewRESTClient(ctx, + option.WithEndpoint(ts.URL), + option.WithoutAuthentication(), + option.WithHTTPClient(ts.Client()), + ) + if err != nil { + t.Fatalf("NewRESTClient: %v", err) + } + + t.Cleanup(func() { _ = client.Close() }) + + op, err := client.CreateRepository(ctx, &artifactregistrypb.CreateRepositoryRequest{ + Parent: "projects/demo/locations/us", + RepositoryId: "gapic-repo", + Repository: &artifactregistrypb.Repository{Description: "gapic"}, + }) + if err != nil { + t.Fatalf("CreateRepository: %v", err) + } + + repo, err := op.Wait(ctx) + if err != nil { + t.Fatalf("op.Wait (the #3 GAPIC LRO fix): %v", err) + } + + if repo == nil || repo.GetName() == "" { + t.Fatalf("Wait returned no repository: %+v", repo) + } +} diff --git a/server/gcp/artifactregistry/handler.go b/server/gcp/artifactregistry/handler.go index dd51eb6e..2dfba676 100644 --- a/server/gcp/artifactregistry/handler.go +++ b/server/gcp/artifactregistry/handler.go @@ -27,6 +27,7 @@ const ( pathPrefix = "/v1/projects/" locationsSeg = "locations" repositoriesSeg = "repositories" + operationsSeg = "operations" dockerImagesSeg = "dockerImages" ) @@ -49,6 +50,7 @@ type route struct { location string repository string // repo id; empty for the collection sub string // "dockerImages" or "" + operation string // operation id when this is an /operations/{op} path } // parseRoute extracts the components of an Artifact Registry v1 path. @@ -58,14 +60,28 @@ func parseRoute(urlPath string) (route, bool) { } parts := strings.Split(strings.TrimPrefix(urlPath, "/v1/"), "/") - // parts: [projects, {p}, locations, {l}, repositories, {id}?, {sub}?] + // parts: [projects, {p}, locations, {l}, {repositories|operations}, {id}?, {sub}?] if len(parts) < minRepoCollectionParts || - parts[0] != "projects" || parts[2] != locationsSeg || parts[4] != repositoriesSeg { + parts[0] != "projects" || parts[2] != locationsSeg { return route{}, false } rt := route{project: parts[1], location: parts[3]} + // LRO polling: GAPIC clients (.Wait()) GET the operation returned by a + // create/delete. Without this route those polls 404. + if parts[4] == operationsSeg { + if len(parts) > minRepoCollectionParts { + rt.operation = parts[5] + } + + return rt, true + } + + if parts[4] != repositoriesSeg { + return route{}, false + } + if len(parts) > minRepoCollectionParts { rt.repository = parts[5] } @@ -93,6 +109,18 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } + if rt.operation != "" { + // The mock completes operations synchronously, so any poll resolves to + // a done operation. This unblocks GAPIC .Wait() callers. Echo the exact + // operation name that was polled. + gcprest.WriteJSON(w, http.StatusOK, operationJSON{ + Name: "projects/" + rt.project + "/locations/" + rt.location + "/operations/" + rt.operation, + Done: true, + }) + + return + } + if rt.repository == "" { h.serveCollection(w, r, &rt) return diff --git a/server/gcp/artifactregistry/operations.go b/server/gcp/artifactregistry/operations.go index 8cefd501..1bc23383 100644 --- a/server/gcp/artifactregistry/operations.go +++ b/server/gcp/artifactregistry/operations.go @@ -1,6 +1,7 @@ package artifactregistry import ( + "encoding/json" "net/http" "github.com/stackshy/cloudemu/v2/server/wire/gcprest" @@ -15,16 +16,55 @@ func (h *Handler) createRepository(w http.ResponseWriter, r *http.Request, rt *r return } + // The driver models a Docker registry and has no format/description field, + // so preserve the request's values in reserved tags and echo them on read. + tags := make(map[string]string, len(body.Labels)) + for k, v := range body.Labels { + tags[k] = v + } + + if body.Format != "" { + tags[formatTag] = body.Format + } + + if body.Description != "" { + tags[descriptionTag] = body.Description + } + repo, err := h.registry.CreateRepository(r.Context(), crdriver.RepositoryConfig{ Name: repoID, - Tags: body.Labels, + Tags: tags, }) if err != nil { gcprest.WriteCErr(w, err) return } - gcprest.WriteJSON(w, http.StatusOK, doneOperation(rt, repoID, toRepositoryJSON(rt.project, rt.location, repo))) + gcprest.WriteJSON(w, http.StatusOK, doneOperation(rt, repoID, + typedResponse(repositoryTypeURL, toRepositoryJSON(rt.project, rt.location, repo)))) +} + +// repositoryTypeURL is the protobuf Any type URL a GAPIC client expects in a +// done LRO's response so CreateRepositoryOperation.Wait() can decode it. +const repositoryTypeURL = "type.googleapis.com/google.devtools.artifactregistry.v1.Repository" + +// typedResponse renders v as the JSON object a google.protobuf.Any expects: the +// resource's fields plus an "@type" URL. Without @type a GAPIC .Wait() cannot +// unmarshal the operation response. +func typedResponse(typeURL string, v any) map[string]any { + b, err := json.Marshal(v) + if err != nil { + return nil + } + + m := map[string]any{} + if err := json.Unmarshal(b, &m); err != nil { + return nil + } + + m["@type"] = typeURL + + return m } func (h *Handler) getRepository(w http.ResponseWriter, r *http.Request, rt *route) { diff --git a/server/gcp/artifactregistry/sdk_roundtrip_test.go b/server/gcp/artifactregistry/sdk_roundtrip_test.go index 08134ad0..a34c6432 100644 --- a/server/gcp/artifactregistry/sdk_roundtrip_test.go +++ b/server/gcp/artifactregistry/sdk_roundtrip_test.go @@ -82,6 +82,46 @@ func TestSDKArtifactRegistryRepositoryLifecycle(t *testing.T) { assertGoogleAPICode(t, err, 404) } +// TestSDKArtifactRegistryOperationAndFormat guards two #321 fixes: the LRO +// operation endpoint is reachable (Operations.Get resolves the create op), and +// a non-DOCKER format + description round-trip instead of being dropped. +func TestSDKArtifactRegistryOperationAndFormat(t *testing.T) { + svc, _ := newARService(t) + ctx := context.Background() + + op, err := svc.Projects.Locations.Repositories.Create(testParent, &ar.Repository{ + Format: "MAVEN", + Description: "team maven repo", + }).RepositoryId("mvn").Context(ctx).Do() + if err != nil { + t.Fatalf("Create: %v", err) + } + + // Poll the operation the create returned — this hits the /operations/{op} + // route that previously 404'd. + polled, err := svc.Projects.Locations.Operations.Get(op.Name).Context(ctx).Do() + if err != nil { + t.Fatalf("Operations.Get (the #321 LRO route): %v", err) + } + + if !polled.Done { + t.Errorf("polled operation not done: %+v", polled) + } + + repo, err := svc.Projects.Locations.Repositories.Get(testParent + "/repositories/mvn").Context(ctx).Do() + if err != nil { + t.Fatalf("Get: %v", err) + } + + if repo.Format != "MAVEN" { + t.Errorf("format=%q want MAVEN (dropped on create)", repo.Format) + } + + if repo.Description != "team maven repo" { + t.Errorf("description=%q want 'team maven repo'", repo.Description) + } +} + func TestSDKArtifactRegistryDockerImages(t *testing.T) { svc, reg := newARService(t) ctx := context.Background() diff --git a/server/gcp/artifactregistry/types.go b/server/gcp/artifactregistry/types.go index 4061ca8e..4fdf4db8 100644 --- a/server/gcp/artifactregistry/types.go +++ b/server/gcp/artifactregistry/types.go @@ -47,7 +47,11 @@ type operationJSON struct { Response any `json:"response,omitempty"` } -const dockerFormat = "DOCKER" +const ( + dockerFormat = "DOCKER" + formatTag = "cloudemu:gcpArFormat" + descriptionTag = "cloudemu:gcpArDescription" +) func repositoryResourceName(project, location, id string) string { return "projects/" + project + "/locations/" + location + "/repositories/" + id @@ -69,15 +73,44 @@ func repoName(name string) string { } func toRepositoryJSON(project, location string, r *crdriver.Repository) repositoryJSON { + format := dockerFormat + if f := r.Tags[formatTag]; f != "" { + format = f + } + return repositoryJSON{ - Name: repositoryResourceName(project, location, repoName(r.Name)), - Format: dockerFormat, - Labels: r.Tags, - CreateTime: r.CreatedAt, - UpdateTime: r.CreatedAt, + Name: repositoryResourceName(project, location, repoName(r.Name)), + Format: format, + Description: r.Tags[descriptionTag], + Labels: stripReservedTags(r.Tags), + CreateTime: r.CreatedAt, + UpdateTime: r.CreatedAt, } } +// stripReservedTags returns user labels without cloudemu-internal keys. +func stripReservedTags(in map[string]string) map[string]string { + if len(in) == 0 { + return nil + } + + out := make(map[string]string, len(in)) + + for k, v := range in { + if strings.HasPrefix(k, "cloudemu:") { + continue + } + + out[k] = v + } + + if len(out) == 0 { + return nil + } + + return out +} + func toDockerImageJSON(project, location, repo string, d *crdriver.ImageDetail) dockerImageJSON { base := repositoryResourceName(project, location, repo) + "/dockerImages/" + d.Digest diff --git a/server/gcp/cloudasset/handler.go b/server/gcp/cloudasset/handler.go index a40439f1..4a601606 100644 --- a/server/gcp/cloudasset/handler.go +++ b/server/gcp/cloudasset/handler.go @@ -2,6 +2,7 @@ package cloudasset import ( "context" + "encoding/base64" "encoding/json" "errors" "io" @@ -220,6 +221,7 @@ func (h *Handler) searchAllResources(w http.ResponseWriter, r *http.Request, _ s } pageSize := intParam(r, body, "pageSize") + assetTypes := searchAssetTypes(r, body) parsed := parseFilter(filter) if parsed.ForceEmpty { @@ -233,16 +235,74 @@ func (h *Handler) searchAllResources(w http.ResponseWriter, r *http.Request, _ s return } - if pageSize > 0 && pageSize < len(results) { - results = results[:pageSize] - } - out := make([]map[string]any, 0, len(results)) for i := range results { - out = append(out, resourceToSearchResult(&results[i], h.projectID)) + res := resourceToSearchResult(&results[i], h.projectID) + if !matchesAssetTypes(res["assetType"], assetTypes) { + continue + } + + out = append(out, res) + } + + // Offset-based pagination: pageToken carries the next start index. + start := decodePageToken(strParam(r, body, "pageToken")) + if start > len(out) { + start = len(out) + } + + page := out[start:] + + resp := map[string]any{} + + if pageSize > 0 && pageSize < len(page) { + page = page[:pageSize] + resp["nextPageToken"] = encodePageToken(start + pageSize) + } + + resp["results"] = page + + writeJSON(w, http.StatusOK, resp) +} + +// searchAssetTypes collects the assetTypes filter from repeated query params or +// a body array. +func searchAssetTypes(r *http.Request, body map[string]any) []string { + if v := r.URL.Query()["assetTypes"]; len(v) > 0 { + return v + } + + raw, ok := body["assetTypes"].([]any) + if !ok { + return nil + } + + out := make([]string, 0, len(raw)) + + for _, x := range raw { + if s, ok := x.(string); ok { + out = append(out, s) + } + } + + return out +} + +// matchesAssetTypes reports whether the result's assetType is in the filter +// (empty filter matches everything). +func matchesAssetTypes(assetType any, filter []string) bool { + if len(filter) == 0 { + return true } - writeJSON(w, http.StatusOK, map[string]any{"results": out}) + at, _ := assetType.(string) + for _, f := range filter { + if f == at { + return true + } + } + + return false } // ----- searchAllIamPolicies ----- @@ -394,19 +454,30 @@ func (h *Handler) listAssets(w http.ResponseWriter, r *http.Request, _ string) { return } - if pageSize > 0 && pageSize < len(allResults) { - allResults = allResults[:pageSize] + // Offset pagination: emit a nextPageToken when truncating so paged callers + // don't silently miss the remainder. + start := decodePageToken(r.URL.Query().Get("pageToken")) + if start > len(allResults) { + start = len(allResults) } - out := make([]map[string]any, 0, len(allResults)) - for i := range allResults { - out = append(out, resourceToAsset(&allResults[i])) + page := allResults[start:] + + resp := map[string]any{"readTime": nowRFC()} + + if pageSize > 0 && pageSize < len(page) { + page = page[:pageSize] + resp["nextPageToken"] = encodePageToken(start + pageSize) } - writeJSON(w, http.StatusOK, map[string]any{ - "assets": out, - "readTime": nowRFC(), - }) + out := make([]map[string]any, 0, len(page)) + for i := range page { + out = append(out, resourceToAsset(&page[i])) + } + + resp["assets"] = out + + writeJSON(w, http.StatusOK, resp) } // collectAssetsForTypes runs one engine query per assetType filter and @@ -627,6 +698,37 @@ func intParam(r *http.Request, body map[string]any, key string) int { return 0 } +func strParam(r *http.Request, body map[string]any, key string) string { + if v, ok := body[key].(string); ok && v != "" { + return v + } + + return r.URL.Query().Get(key) +} + +// encodePageToken/decodePageToken carry an offset as an opaque base64 token. +func encodePageToken(offset int) string { + return base64.StdEncoding.EncodeToString([]byte(strconv.Itoa(offset))) +} + +func decodePageToken(tok string) int { + if tok == "" { + return 0 + } + + b, err := base64.StdEncoding.DecodeString(tok) + if err != nil { + return 0 + } + + n, err := strconv.Atoi(string(b)) + if err != nil || n < 0 { + return 0 + } + + return n +} + func nowRFC() string { return time.Now().UTC().Format(time.RFC3339Nano) } diff --git a/server/gcp/clouddns/handler.go b/server/gcp/clouddns/handler.go index 8bbf115b..2e08b6b4 100644 --- a/server/gcp/clouddns/handler.go +++ b/server/gcp/clouddns/handler.go @@ -21,6 +21,7 @@ package clouddns import ( "net/http" "strings" + "sync/atomic" "github.com/stackshy/cloudemu/v2/server/wire/gcprest" dnsdriver "github.com/stackshy/cloudemu/v2/services/dns/driver" @@ -42,7 +43,8 @@ const ( // Handler serves dns.googleapis.com v1 requests against a dns driver. type Handler struct { - dns dnsdriver.DNS + dns dnsdriver.DNS + changeSeq atomic.Uint64 } // New returns a Cloud DNS handler backed by d. diff --git a/server/gcp/clouddns/operations.go b/server/gcp/clouddns/operations.go index 57a0016d..00ec45f3 100644 --- a/server/gcp/clouddns/operations.go +++ b/server/gcp/clouddns/operations.go @@ -2,6 +2,7 @@ package clouddns import ( "net/http" + "strconv" cerrors "github.com/stackshy/cloudemu/v2/errors" "github.com/stackshy/cloudemu/v2/server/wire/gcprest" @@ -15,10 +16,20 @@ func (h *Handler) createZone(w http.ResponseWriter, r *http.Request, rt route) { return } + tags := req.Labels + if req.DNSName != "" { + tags = make(map[string]string, len(req.Labels)+1) + for k, v := range req.Labels { + tags[k] = v + } + + tags[dnsNameTag] = req.DNSName + } + info, err := h.dns.CreateZone(r.Context(), dnsdriver.ZoneConfig{ Name: req.Name, Private: privateFor(req.Visibility), - Tags: req.Labels, + Tags: tags, Scope: scope.Scope{Project: rt.project}, }) if err != nil { @@ -108,8 +119,21 @@ func (h *Handler) createChange(w http.ResponseWriter, r *http.Request, rt route) } } + // The canonical "update a record set" change deletes the old rrset and adds + // a new one with the SAME name+type in one batch. Such an addition is not a + // real conflict — it replaces a record this same change removes — so exempt + // additions whose (name,type) also appears in the deletions. + deleting := make(map[string]bool, len(req.Deletions)) + for i := range req.Deletions { + deleting[rrsetKey(req.Deletions[i].Name, req.Deletions[i].Type)] = true + } + for i := range req.Additions { a := &req.Additions[i] + if deleting[rrsetKey(a.Name, a.Type)] { + continue + } + if _, gerr := h.dns.GetRecord(r.Context(), id, a.Name, a.Type); gerr == nil { gcprest.WriteCErr(w, cerrors.Newf(cerrors.AlreadyExists, "record set %q %s already exists", a.Name, a.Type)) @@ -143,13 +167,18 @@ func (h *Handler) createChange(w http.ResponseWriter, r *http.Request, rt route) gcprest.WriteJSON(w, http.StatusOK, changeJSON{ Kind: kindChange, - ID: "1", + ID: strconv.FormatUint(h.changeSeq.Add(1), 10), Additions: req.Additions, Deletions: req.Deletions, Status: changeStatusDone, }) } +// rrsetKey identifies a record set by name+type within a zone. +func rrsetKey(name, rtype string) string { + return name + "|" + rtype +} + func (h *Handler) listRRSets(w http.ResponseWriter, r *http.Request, rt route) { id, err := h.resolveZoneID(r.Context(), rt.project, rt.zone) if err != nil { diff --git a/server/gcp/clouddns/sdk_roundtrip_test.go b/server/gcp/clouddns/sdk_roundtrip_test.go index 39e0c844..755ebbe0 100644 --- a/server/gcp/clouddns/sdk_roundtrip_test.go +++ b/server/gcp/clouddns/sdk_roundtrip_test.go @@ -167,6 +167,58 @@ func TestSDKCloudDNSRecordChanges(t *testing.T) { } } +// TestSDKCloudDNSUpdateRecord guards the #321 fix: the canonical record update +// (delete old rrset + add new one with the SAME name+type in one change) must +// succeed, not fail with AlreadyExists. It also asserts dnsName round-trips. +func TestSDKCloudDNSUpdateRecord(t *testing.T) { + svc := newDNSService(t) + ctx := context.Background() + + zone, err := svc.ManagedZones.Create(testProject, &dns.ManagedZone{ + Name: "upd-zone", + DnsName: "upd.example.com.", + }).Context(ctx).Do() + if err != nil { + t.Fatalf("ManagedZones.Create: %v", err) + } + + if zone.DnsName != "upd.example.com." { + t.Errorf("dnsName=%q want upd.example.com. (should not be the zone name)", zone.DnsName) + } + + old := &dns.ResourceRecordSet{Name: "www.upd.example.com.", Type: "A", Ttl: 300, Rrdatas: []string{"192.0.2.1"}} + + if _, err := svc.Changes.Create(testProject, "upd-zone", &dns.Change{ + Additions: []*dns.ResourceRecordSet{old}, + }).Context(ctx).Do(); err != nil { + t.Fatalf("Changes.Create(add): %v", err) + } + + // Update = delete old + add new, same name+type, one atomic change. + updated := &dns.ResourceRecordSet{Name: "www.upd.example.com.", Type: "A", Ttl: 600, Rrdatas: []string{"192.0.2.2"}} + + change, err := svc.Changes.Create(testProject, "upd-zone", &dns.Change{ + Deletions: []*dns.ResourceRecordSet{old}, + Additions: []*dns.ResourceRecordSet{updated}, + }).Context(ctx).Do() + if err != nil { + t.Fatalf("Changes.Create(delete+add same rrset) failed (the #321 bug): %v", err) + } + + if change.Id == "" || change.Id == "1" { + t.Errorf("change id=%q want a unique non-placeholder id", change.Id) + } + + rrsets, err := svc.ResourceRecordSets.List(testProject, "upd-zone").Context(ctx).Do() + if err != nil { + t.Fatalf("List: %v", err) + } + + if len(rrsets.Rrsets) != 1 || rrsets.Rrsets[0].Ttl != 600 || rrsets.Rrsets[0].Rrdatas[0] != "192.0.2.2" { + t.Fatalf("after update rrsets=%+v want single 600/192.0.2.2", rrsets.Rrsets) + } +} + func TestSDKCloudDNSErrors(t *testing.T) { svc := newDNSService(t) ctx := context.Background() diff --git a/server/gcp/clouddns/types.go b/server/gcp/clouddns/types.go index dba4dc4b..d3b12077 100644 --- a/server/gcp/clouddns/types.go +++ b/server/gcp/clouddns/types.go @@ -4,12 +4,17 @@ import ( "context" "hash/fnv" "strconv" + "strings" cerrors "github.com/stackshy/cloudemu/v2/errors" dnsdriver "github.com/stackshy/cloudemu/v2/services/dns/driver" "github.com/stackshy/cloudemu/v2/services/scope" ) +// dnsNameTag stores a zone's DNS suffix (dnsName), which the dns driver does +// not model, so it round-trips through the zone's tags. +const dnsNameTag = "cloudemu:gcpDnsName" + // Kind values Cloud DNS stamps on its resources; the SDK tolerates them being // absent but real responses carry them, so we mirror the wire faithfully. const ( @@ -92,14 +97,45 @@ func numericID(id string) string { } func toManagedZoneJSON(info *dnsdriver.ZoneInfo) managedZoneJSON { + // dnsName is the DNS suffix (e.g. "example.com."), which the driver doesn't + // model, so it's stashed in a reserved tag at create. Fall back to the zone + // name only when absent. + dnsName := info.Name + if v, ok := info.Tags[dnsNameTag]; ok && v != "" { + dnsName = v + } + return managedZoneJSON{ Kind: kindManagedZone, Name: info.Name, ID: numericID(info.ID), Visibility: visibilityFor(info.Private), - Labels: info.Tags, - DNSName: info.Name, + Labels: stripReservedTags(info.Tags), + DNSName: dnsName, + } +} + +// stripReservedTags returns the user labels with cloudemu-internal keys removed. +func stripReservedTags(in map[string]string) map[string]string { + if len(in) == 0 { + return nil + } + + out := make(map[string]string, len(in)) + + for k, v := range in { + if strings.HasPrefix(k, "cloudemu:") { + continue + } + + out[k] = v } + + if len(out) == 0 { + return nil + } + + return out } func toRecordSetJSON(rec *dnsdriver.RecordInfo) resourceRecordSetJSON { diff --git a/server/gcp/cloudfunctions/handler.go b/server/gcp/cloudfunctions/handler.go index 8d56472e..181b50e8 100644 --- a/server/gcp/cloudfunctions/handler.go +++ b/server/gcp/cloudfunctions/handler.go @@ -104,6 +104,11 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } + if parts.action == "generateUploadUrl" { + h.generateUploadURL(w, r, parts) + return + } + if parts.name != "" { h.serveResource(w, r, parts) return @@ -136,6 +141,22 @@ func (h *Handler) serveCollection(w http.ResponseWriter, r *http.Request, p func } } +// generateUploadURL answers functions:generateUploadUrl — the first step of a +// source-upload deploy. Real Cloud Functions returns a signed GCS URL the +// client PUTs the source zip to; the emulator returns a usable stub URL so the +// deploy flow proceeds. +func (*Handler) generateUploadURL(w http.ResponseWriter, r *http.Request, p functionPath) { + if r.Method != http.MethodPost { + writeError(w, http.StatusMethodNotAllowed, "METHOD_NOT_ALLOWED", "method not allowed") + return + } + + url := "https://storage.googleapis.com/cloudemu-gcf-uploads/" + p.project + "/" + p.location + + "/source-" + strconv.FormatInt(time.Now().UnixNano(), 10) + ".zip" + + writeJSON(w, http.StatusOK, map[string]string{"uploadUrl": url}) +} + // serveOperation answers GET /v1/operations/{name}. We always return done=true // because mutations are synchronous in the mock; a poll is just an echo. func (*Handler) serveOperation(w http.ResponseWriter, r *http.Request) { @@ -401,6 +422,11 @@ func toCloudFunction(info *sdrv.FunctionInfo, p functionPath) cloudFunction { EnvVariables: info.Environment, UpdateTime: info.LastModified, VersionID: "1", + // Real Cloud Functions always advertises the HTTPS trigger URL; clients + // read it to invoke the function. + HTTPSTrigger: &httpsTrigger{ + URL: "https://" + scope.location + "-" + scope.project + ".cloudfunctions.net/" + scope.name, + }, } if info.Timeout > 0 { diff --git a/server/gcp/cloudfunctions/sdk_roundtrip_test.go b/server/gcp/cloudfunctions/sdk_roundtrip_test.go index 24d507ac..c399a79b 100644 --- a/server/gcp/cloudfunctions/sdk_roundtrip_test.go +++ b/server/gcp/cloudfunctions/sdk_roundtrip_test.go @@ -74,6 +74,22 @@ func TestSDKCloudFunctionsCreateGetListDelete(t *testing.T) { t.Fatalf("Name = %q, want suffix /functions/hello", got.Name) } + // The HTTPS trigger URL must be advertised (clients invoke via it). + if got.HttpsTrigger == nil || got.HttpsTrigger.Url == "" { + t.Fatalf("httpsTrigger.url missing: %+v", got.HttpsTrigger) + } + + // generateUploadUrl (first step of a source deploy) must return a URL. + up, err := svc.Projects.Locations.Functions.GenerateUploadUrl(parent, + &cloudfunctions.GenerateUploadUrlRequest{}).Context(ctx).Do() + if err != nil { + t.Fatalf("GenerateUploadUrl: %v", err) + } + + if up.UploadUrl == "" { + t.Fatal("GenerateUploadUrl returned no uploadUrl") + } + listResp, err := svc.Projects.Locations.Functions.List(parent).Context(ctx).Do() if err != nil { t.Fatalf("List: %v", err) diff --git a/server/gcp/cloudlogging/operations.go b/server/gcp/cloudlogging/operations.go index 5a749f1d..72b5ca65 100644 --- a/server/gcp/cloudlogging/operations.go +++ b/server/gcp/cloudlogging/operations.go @@ -2,13 +2,15 @@ package cloudlogging import ( "context" - "github.com/stackshy/cloudemu/v2/services/scope" "net/http" + "sort" + "strings" "time" cerrors "github.com/stackshy/cloudemu/v2/errors" "github.com/stackshy/cloudemu/v2/server/wire/gcprest" logdriver "github.com/stackshy/cloudemu/v2/services/logging/driver" + "github.com/stackshy/cloudemu/v2/services/scope" ) // writeEntries maps WriteLogEntries onto the driver. Cloud Logging creates a log @@ -43,7 +45,7 @@ func (h *Handler) writeEntries(w http.ResponseWriter, r *http.Request) { byLog[logID] = append(byLog[logID], logdriver.LogEvent{ Timestamp: parseTimestamp(e.Timestamp, now), - Message: e.TextPayload, + Message: encodeEntryPayload(e), }) } @@ -103,9 +105,24 @@ func (h *Handler) listEntries(w http.ResponseWriter, r *http.Request) { return } - out := make([]logEntryJSON, 0, len(events)) - for i := range events { - out = append(out, toLogEntryJSON(project, logID, &events[i])) + // Cloud Logging orders by timestamp — ascending by default, descending for + // "timestamp desc". Sort by the entry timestamp rather than assuming the + // driver's insertion order matches (out-of-order writes must still sort). + desc := strings.Contains(strings.ToLower(req.OrderBy), "desc") + + sorted := make([]logdriver.LogEvent, len(events)) + copy(sorted, events) + sort.SliceStable(sorted, func(i, j int) bool { + if desc { + return sorted[i].Timestamp.After(sorted[j].Timestamp) + } + + return sorted[i].Timestamp.Before(sorted[j].Timestamp) + }) + + out := make([]logEntryJSON, 0, len(sorted)) + for i := range sorted { + out = append(out, toLogEntryJSON(project, logID, &sorted[i])) } gcprest.WriteJSON(w, http.StatusOK, listLogEntriesResponse{Entries: out}) diff --git a/server/gcp/cloudlogging/sdk_roundtrip_test.go b/server/gcp/cloudlogging/sdk_roundtrip_test.go index aa0a39c5..b4da537f 100644 --- a/server/gcp/cloudlogging/sdk_roundtrip_test.go +++ b/server/gcp/cloudlogging/sdk_roundtrip_test.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "google.golang.org/api/googleapi" logging "google.golang.org/api/logging/v2" "google.golang.org/api/option" @@ -77,6 +78,60 @@ func TestSDKCloudLoggingWriteAndList(t *testing.T) { } } +// TestSDKCloudLoggingStructuredFields guards the #321 fix: severity, labels and +// jsonPayload round-trip through write→list, and orderBy "timestamp desc" is +// honored. +func TestSDKCloudLoggingStructuredFields(t *testing.T) { + svc := newLoggingService(t) + ctx := context.Background() + + logName := "projects/" + testProject + "/logs/structured" + base := time.Now().UTC().Truncate(time.Millisecond) + + // Write OUT of timestamp order (ERROR is later but written first) so the + // test guards a real timestamp sort, not a mere reversal of insertion order. + if _, err := svc.Entries.Write(&logging.WriteLogEntriesRequest{ + LogName: logName, + Entries: []*logging.LogEntry{ + { + Timestamp: base.Add(time.Second).Format(time.RFC3339Nano), + Severity: "ERROR", + Labels: map[string]string{"component": "api"}, + JsonPayload: googleapi.RawMessage(`{"code":500}`), + }, + {Timestamp: base.Format(time.RFC3339Nano), TextPayload: "first", Severity: "INFO"}, + }, + }).Context(ctx).Do(); err != nil { + t.Fatalf("Entries.Write: %v", err) + } + + resp, err := svc.Entries.List(&logging.ListLogEntriesRequest{ + ResourceNames: []string{"projects/" + testProject}, + Filter: `logName="` + logName + `"`, + OrderBy: "timestamp desc", + }).Context(ctx).Do() + if err != nil { + t.Fatalf("Entries.List: %v", err) + } + + if len(resp.Entries) != 2 { + t.Fatalf("got %d entries, want 2", len(resp.Entries)) + } + + // desc order: the ERROR entry (written second) comes first. + if resp.Entries[0].Severity != "ERROR" { + t.Errorf("first (desc) severity = %q, want ERROR", resp.Entries[0].Severity) + } + + if resp.Entries[0].Labels["component"] != "api" { + t.Errorf("labels did not round-trip: %v", resp.Entries[0].Labels) + } + + if len(resp.Entries[0].JsonPayload) == 0 { + t.Error("jsonPayload did not round-trip") + } +} + func TestSDKCloudLoggingLogsLifecycle(t *testing.T) { svc := newLoggingService(t) ctx := context.Background() diff --git a/server/gcp/cloudlogging/types.go b/server/gcp/cloudlogging/types.go index 6ea38566..ce19505e 100644 --- a/server/gcp/cloudlogging/types.go +++ b/server/gcp/cloudlogging/types.go @@ -1,6 +1,7 @@ package cloudlogging import ( + "encoding/json" "net/url" "strings" "time" @@ -8,15 +9,72 @@ import ( logdriver "github.com/stackshy/cloudemu/v2/services/logging/driver" ) -// logEntryJSON is the subset of the Cloud Logging LogEntry resource we model: -// a text payload plus a timestamp, keyed by logName. The driver has no notion -// of severity or structured payloads, so only textPayload round-trips. +// logEntryJSON is the subset of the Cloud Logging LogEntry resource we model. +// The driver stores only a message string, so the structured fields +// (severity, jsonPayload, labels, insertId) are JSON-enveloped into it on +// write and reconstructed on read — see encode/decodeEntryPayload. type logEntryJSON struct { - LogName string `json:"logName,omitempty"` - Timestamp string `json:"timestamp,omitempty"` - TextPayload string `json:"textPayload,omitempty"` - InsertID string `json:"insertId,omitempty"` - Severity string `json:"severity,omitempty"` + LogName string `json:"logName,omitempty"` + Timestamp string `json:"timestamp,omitempty"` + TextPayload string `json:"textPayload,omitempty"` + JSONPayload map[string]any `json:"jsonPayload,omitempty"` + InsertID string `json:"insertId,omitempty"` + Severity string `json:"severity,omitempty"` + Labels map[string]string `json:"labels,omitempty"` +} + +// entryPayload is the JSON envelope stored in the driver's message string so a +// log entry's structured fields survive a write→read round-trip. +type entryPayload struct { + Text string `json:"t,omitempty"` + JSONPayload map[string]any `json:"j,omitempty"` + Severity string `json:"s,omitempty"` + InsertID string `json:"i,omitempty"` + Labels map[string]string `json:"l,omitempty"` +} + +// entryPayloadPrefix marks a driver message that carries an encoded envelope. +// A message without it is treated as a plain textPayload (backward compatible). +const entryPayloadPrefix = "\x00cloudemu-log\x00" + +func encodeEntryPayload(e *logEntryJSON) string { + // Plain text with no structured fields stays a bare string, so logs written + // by other means still read back naturally. + if e.Severity == "" && e.InsertID == "" && len(e.JSONPayload) == 0 && len(e.Labels) == 0 { + return e.TextPayload + } + + b, err := json.Marshal(entryPayload{ + Text: e.TextPayload, + JSONPayload: e.JSONPayload, + Severity: e.Severity, + InsertID: e.InsertID, + Labels: e.Labels, + }) + if err != nil { + return e.TextPayload + } + + return entryPayloadPrefix + string(b) +} + +func decodeEntryPayload(msg string, out *logEntryJSON) { + if !strings.HasPrefix(msg, entryPayloadPrefix) { + out.TextPayload = msg + return + } + + var p entryPayload + if err := json.Unmarshal([]byte(strings.TrimPrefix(msg, entryPayloadPrefix)), &p); err != nil { + out.TextPayload = msg + return + } + + out.TextPayload = p.Text + out.JSONPayload = p.JSONPayload + out.Severity = p.Severity + out.InsertID = p.InsertID + out.Labels = p.Labels } // writeLogEntriesRequest is the entries:write body. logName/resource may be set @@ -173,9 +231,12 @@ func parseTimestamp(ts string, now time.Time) time.Time { } func toLogEntryJSON(project, logID string, e *logdriver.LogEvent) logEntryJSON { - return logEntryJSON{ - LogName: logNameFor(project, logID), - Timestamp: e.Timestamp.UTC().Format(time.RFC3339Nano), - TextPayload: e.Message, + out := logEntryJSON{ + LogName: logNameFor(project, logID), + Timestamp: e.Timestamp.UTC().Format(time.RFC3339Nano), } + + decodeEntryPayload(e.Message, &out) + + return out } diff --git a/server/gcp/cloudsql/operations.go b/server/gcp/cloudsql/operations.go index 2f38e845..b889a456 100644 --- a/server/gcp/cloudsql/operations.go +++ b/server/gcp/cloudsql/operations.go @@ -204,7 +204,14 @@ func (h *Handler) listBackupRuns(w http.ResponseWriter, r *http.Request, p *sqlP func (h *Handler) getBackupRun(w http.ResponseWriter, r *http.Request, p *sqlPath) { snaps, err := h.db.DescribeSnapshots(r.Context(), []string{p.subName}, p.name) - if err != nil || len(snaps) == 0 { + if err != nil { + // Surface the real backend error rather than masking every failure as + // NOT_FOUND (which would send callers debugging the wrong subsystem). + writeErr(w, err) + return + } + + if len(snaps) == 0 { writeError(w, http.StatusNotFound, "NOT_FOUND", "backup run "+p.subName+" not found") return } diff --git a/server/gcp/compute/handler.go b/server/gcp/compute/handler.go index 35aae8af..830eabfc 100644 --- a/server/gcp/compute/handler.go +++ b/server/gcp/compute/handler.go @@ -230,7 +230,16 @@ func serveOperations(w http.ResponseWriter, r *http.Request, rp gcprest.Resource } if rp.ResourceName == "" { - writeNotImplemented(w, "operations list") + // The mock runs synchronously and retains no pending operations, so a + // list is legitimately empty rather than unimplemented. + host := hostFromRequest(r) + gcprest.WriteJSON(w, http.StatusOK, map[string]any{ + "kind": "compute#operationList", + "id": "projects/" + rp.Project + "/operations", + "items": []any{}, + "selfLink": gcprest.SelfLink(host, rp.Project, rp.Scope, rp.ScopeName, "operations", ""), + }) + return } diff --git a/server/gcp/compute/images.go b/server/gcp/compute/images.go index e59dad1e..b0eecd6b 100644 --- a/server/gcp/compute/images.go +++ b/server/gcp/compute/images.go @@ -54,31 +54,13 @@ func (h *Handler) insertImage(w http.ResponseWriter, r *http.Request, rp gcprest return } - // GCP images can be created from a disk, snapshot, or imported. The - // driver's CreateImage takes an InstanceID — we fake one from any - // existing instance just so the driver lets the create succeed. - insts, err := h.compute.DescribeInstances(r.Context(), nil, nil) - if err != nil { - gcprest.WriteCErr(w, err) - return - } - - instanceID := "" - if len(insts) > 0 { - instanceID = insts[0].ID - } - - if instanceID == "" { - gcprest.WriteError(w, http.StatusBadRequest, "invalid", - "images mock requires at least one running instance to derive the image from") - - return - } - + // GCP images are created from a disk, snapshot, or import — never from a + // source instance (that is EC2's model). Pass an empty InstanceID so the + // driver takes the source-based path; record the source in the description + // so a read reflects what it was built from. cfg := computedriver.ImageConfig{ - InstanceID: instanceID, Name: req.Name, - Description: req.Name, + Description: imageSourceDescription(&req), Tags: mergeImageTags(req.Labels, req.Name), } @@ -148,6 +130,19 @@ func (h *Handler) deleteImage(w http.ResponseWriter, r *http.Request, rp gcprest gcprest.WriteJSON(w, http.StatusOK, op) } +// imageSourceDescription records the source the image was built from so a read +// reflects it. Falls back to the image name when no source was given (import). +func imageSourceDescription(req *imageRequest) string { + switch { + case req.SourceDisk != "": + return "sourceDisk: " + req.SourceDisk + case req.SourceSnapshot != "": + return "sourceSnapshot: " + req.SourceSnapshot + default: + return req.Name + } +} + func findImageByName(ctx context.Context, c computedriver.Compute, name string) (*computedriver.ImageInfo, error) { imgs, err := c.DescribeImages(ctx, nil) if err != nil { diff --git a/server/gcp/compute/instances.go b/server/gcp/compute/instances.go index 121b7ea0..4e7d5ae9 100644 --- a/server/gcp/compute/instances.go +++ b/server/gcp/compute/instances.go @@ -115,7 +115,15 @@ func (h *Handler) deleteInstance(w http.ResponseWriter, r *http.Request, rp gcpr return } - if err := h.compute.TerminateInstances(r.Context(), []string{inst.ID}); err != nil { + // GCP instances.delete removes the resource (a subsequent GET is 404), + // unlike EC2 terminate which leaves a TERMINATED tombstone. Hard-remove + // when the driver supports it; fall back to terminate otherwise. + if remover, ok := h.compute.(instanceRemover); ok { + if err := remover.RemoveInstance(r.Context(), inst.ID); err != nil { + gcprest.WriteCErr(w, err) + return + } + } else if err := h.compute.TerminateInstances(r.Context(), []string{inst.ID}); err != nil { gcprest.WriteCErr(w, err) return } @@ -171,6 +179,12 @@ func (h *Handler) action( gcprest.WriteJSON(w, http.StatusOK, doneOp) } +// instanceRemover is the GCP-local hard-delete capability (removes the +// instance rather than tombstoning it). The GCE provider Mock implements it. +type instanceRemover interface { + RemoveInstance(ctx context.Context, instanceID string) error +} + // findByName looks up an instance by its GCP-tagged name. func findByName(ctx context.Context, c computedriver.Compute, name string) (*computedriver.Instance, error) { instances, err := c.DescribeInstances(ctx, nil, nil) @@ -252,15 +266,33 @@ func toInstanceResponse(inst *computedriver.Instance, rp gcprest.ResourcePath, h name := tagOr(inst.Tags, gcpNameTag, rp.ResourceName) return instanceResponse{ - Kind: "compute#instance", - ID: numericID(inst.ID), - Name: name, - MachineType: gcprest.SelfLink(host, rp.Project, rp.Scope, rp.ScopeName, "machineTypes", inst.InstanceType), - Status: gcpStatusFor(inst.State), - Zone: host + "/compute/v1/projects/" + rp.Project + "/zones/" + rp.ScopeName, - SelfLink: gcprest.SelfLink(host, rp.Project, rp.Scope, rp.ScopeName, "instances", name), - Labels: stripInternalTags(inst.Tags), + Kind: "compute#instance", + ID: numericID(inst.ID), + Name: name, + MachineType: gcprest.SelfLink(host, rp.Project, rp.Scope, rp.ScopeName, "machineTypes", inst.InstanceType), + Status: gcpStatusFor(inst.State), + Zone: host + "/compute/v1/projects/" + rp.Project + "/zones/" + rp.ScopeName, + SelfLink: gcprest.SelfLink(host, rp.Project, rp.Scope, rp.ScopeName, "instances", name), + NetworkInterfaces: instanceNICs(inst), + Labels: stripInternalTags(inst.Tags), + } +} + +// instanceNICs echoes back the network interface the instance was created +// with. The driver stores the subnetwork the client set plus the private IP it +// assigned; a read must return them (real GCP always reports a NIC), otherwise +// a client that sets a subnet reads back an empty interface list. +func instanceNICs(inst *computedriver.Instance) []networkInterface { + if inst.SubnetID == "" && inst.PrivateIP == "" && inst.VPCID == "" { + return nil } + + return []networkInterface{{ + Name: "nic0", + Network: inst.VPCID, + Subnetwork: inst.SubnetID, + NetworkIP: inst.PrivateIP, + }} } // numericID returns a stable uint64-shaped string derived from a driver diff --git a/server/gcp/compute/sdk_roundtrip_test.go b/server/gcp/compute/sdk_roundtrip_test.go index b28865ea..d7be3679 100644 --- a/server/gcp/compute/sdk_roundtrip_test.go +++ b/server/gcp/compute/sdk_roundtrip_test.go @@ -158,6 +158,96 @@ func TestSDKGCEInstanceRoundTrip(t *testing.T) { } } +// TestSDKGCEInstanceNICRoundTrip guards the #321 fix: an instance read must +// echo the network interface it was created with (subnetwork + assigned +// networkIP), not an empty list. +func TestSDKGCEInstanceNICRoundTrip(t *testing.T) { + cloudP := cloudemu.NewGCP() + srv := gcpserver.New(gcpserver.Drivers{Compute: cloudP.GCE}) + + ts := httptest.NewServer(srv) + t.Cleanup(ts.Close) + + client := newSDKInstancesClient(t, ts) + ctx := context.Background() + + insertOp, err := client.Insert(ctx, &computepb.InsertInstanceRequest{ + Project: testProject, Zone: testZone, + InstanceResource: &computepb.Instance{ + Name: ptrStr("nic-vm"), + MachineType: ptrStr("zones/" + testZone + "/machineTypes/n1-standard-1"), + NetworkInterfaces: []*computepb.NetworkInterface{ + {Subnetwork: ptrStr("regions/us-central1/subnetworks/my-subnet")}, + }, + }, + }) + if err != nil { + t.Fatalf("Insert: %v", err) + } + + if err := insertOp.Wait(ctx); err != nil { + t.Fatalf("Insert wait: %v", err) + } + + got, err := client.Get(ctx, &computepb.GetInstanceRequest{ + Project: testProject, Zone: testZone, Instance: "nic-vm", + }) + if err != nil { + t.Fatalf("Get: %v", err) + } + + nics := got.GetNetworkInterfaces() + if len(nics) == 0 { + t.Fatal("read-back instance has no networkInterfaces") + } + + if !strings.HasSuffix(nics[0].GetSubnetwork(), "subnetworks/my-subnet") { + t.Errorf("subnetwork=%q want ...subnetworks/my-subnet", nics[0].GetSubnetwork()) + } + + if nics[0].GetNetworkIP() == "" { + t.Error("networkIP is empty; the mock assigns a private IP on create") + } +} + +// TestSDKGCEImageFromScratch guards the #321 fix: an image create must not +// require a pre-existing instance (GCP images come from disks/snapshots, not +// instances). This creates an image with no instances present. +func TestSDKGCEImageFromScratch(t *testing.T) { + cloudP := cloudemu.NewGCP() + srv := gcpserver.New(gcpserver.Drivers{Compute: cloudP.GCE}) + + ts := httptest.NewServer(srv) + t.Cleanup(ts.Close) + + ctx := context.Background() + imgClient := newImagesSDKClient(t, ts) + + op, err := imgClient.Insert(ctx, &computepb.InsertImageRequest{ + Project: testProject, + ImageResource: &computepb.Image{ + Name: ptrStr("disk-img"), + SourceDisk: ptrStr("zones/" + testZone + "/disks/my-disk"), + }, + }) + if err != nil { + t.Fatalf("Insert: %v", err) + } + + if err := op.Wait(ctx); err != nil { + t.Fatalf("Insert wait (image-from-disk should not need an instance): %v", err) + } + + got, err := imgClient.Get(ctx, &computepb.GetImageRequest{Project: testProject, Image: "disk-img"}) + if err != nil { + t.Fatalf("Get: %v", err) + } + + if got.GetName() != "disk-img" { + t.Errorf("name=%q want disk-img", got.GetName()) + } +} + // ptr helpers — computepb fields are pointers because the protocol uses // proto3-with-presence and the SDK marshalers care about the distinction // between unset and zero-value. diff --git a/server/gcp/compute/types.go b/server/gcp/compute/types.go index 90be1580..ee06de71 100644 --- a/server/gcp/compute/types.go +++ b/server/gcp/compute/types.go @@ -29,8 +29,10 @@ type diskInitializeParams struct { } type networkInterface struct { + Name string `json:"name,omitempty"` Network string `json:"network,omitempty"` Subnetwork string `json:"subnetwork,omitempty"` + NetworkIP string `json:"networkIP,omitempty"` } type tagsBlock struct { diff --git a/server/gcp/eventarc/gapic_lro_test.go b/server/gcp/eventarc/gapic_lro_test.go new file mode 100644 index 00000000..e9c5e942 --- /dev/null +++ b/server/gcp/eventarc/gapic_lro_test.go @@ -0,0 +1,65 @@ +package eventarc_test + +import ( + "context" + "net/http/httptest" + "testing" + + eventarc "cloud.google.com/go/eventarc/apiv1" + "cloud.google.com/go/eventarc/apiv1/eventarcpb" + "google.golang.org/api/option" + + "github.com/stackshy/cloudemu/v2" + gcpserver "github.com/stackshy/cloudemu/v2/server/gcp" +) + +// TestGAPICCreateTriggerWait is the review's #3 check for eventarc: the finding +// targeted the apiv1 GAPIC client's LRO .Wait(), which the raw REST client +// never exercised. CreateTrigger(...).Wait() must resolve (not 404, not a +// missing-@type decode error) and return the created trigger. +func TestGAPICCreateTriggerWait(t *testing.T) { + cloud := cloudemu.NewGCP() + srv := gcpserver.New(gcpserver.DriversFrom(cloud)) + ts := httptest.NewServer(srv) + t.Cleanup(ts.Close) + + ctx := context.Background() + + client, err := eventarc.NewRESTClient(ctx, + option.WithEndpoint(ts.URL), + option.WithoutAuthentication(), + option.WithHTTPClient(ts.Client()), + ) + if err != nil { + t.Fatalf("NewRESTClient: %v", err) + } + + t.Cleanup(func() { _ = client.Close() }) + + op, err := client.CreateTrigger(ctx, &eventarcpb.CreateTriggerRequest{ + Parent: "projects/demo/locations/us-central1", + TriggerId: "gapic-trig", + Trigger: &eventarcpb.Trigger{ + EventFilters: []*eventarcpb.EventFilter{ + {Attribute: "type", Value: "google.cloud.pubsub.topic.v1.messagePublished"}, + }, + Destination: &eventarcpb.Destination{ + Descriptor_: &eventarcpb.Destination_CloudRun{ + CloudRun: &eventarcpb.CloudRun{Service: "svc", Region: "us-central1"}, + }, + }, + }, + }) + if err != nil { + t.Fatalf("CreateTrigger: %v", err) + } + + trig, err := op.Wait(ctx) + if err != nil { + t.Fatalf("op.Wait (the #3 GAPIC LRO fix): %v", err) + } + + if trig == nil || trig.GetName() == "" { + t.Fatalf("Wait returned no trigger: %+v", trig) + } +} diff --git a/server/gcp/eventarc/handler.go b/server/gcp/eventarc/handler.go index 3bb99949..12298304 100644 --- a/server/gcp/eventarc/handler.go +++ b/server/gcp/eventarc/handler.go @@ -14,7 +14,7 @@ // // - Auto-provisioning one event bus per location, named "eventarc-", // the first time a trigger is created there. This is a synthesized -// container with no Eventarc analogue — the SDK never sees it. +// container with no Eventarc analog — the SDK never sees it. // - Mapping each trigger onto a driver rule keyed by the trigger id, with the // trigger's eventFilters serialized into the rule's EventPattern and the // destination folded into a single target so Get/List can round-trip them. @@ -47,9 +47,10 @@ import ( ) const ( - pathPrefix = "/v1/projects/" - locationsSeg = "locations" - triggersSeg = "triggers" + pathPrefix = "/v1/projects/" + locationsSeg = "locations" + triggersSeg = "triggers" + operationsSeg = "operations" ) // minTriggersCollectionParts is the segment count of a triggers collection @@ -68,9 +69,10 @@ func New(b ebdriver.EventBus) *Handler { } type route struct { - project string - location string - trigger string // trigger id; empty for the collection + project string + location string + trigger string // trigger id; empty for the collection + operation string // operation id for an /operations/{op} path } // parseRoute extracts the components of an Eventarc v1 triggers path. @@ -80,14 +82,27 @@ func parseRoute(urlPath string) (route, bool) { } parts := strings.Split(strings.TrimPrefix(urlPath, "/v1/"), "/") - // parts: [projects, {p}, locations, {l}, triggers, {id}?] + // parts: [projects, {p}, locations, {l}, {triggers|operations}, {id}?] if len(parts) < minTriggersCollectionParts || - parts[0] != "projects" || parts[2] != locationsSeg || parts[4] != triggersSeg { + parts[0] != "projects" || parts[2] != locationsSeg { return route{}, false } rt := route{project: parts[1], location: parts[3]} + // LRO polling: GAPIC .Wait() GETs the operation the create/delete returned. + if parts[4] == operationsSeg { + if len(parts) > minTriggersCollectionParts { + rt.operation = parts[5] + } + + return rt, true + } + + if parts[4] != triggersSeg { + return route{}, false + } + if len(parts) > minTriggersCollectionParts { rt.trigger = parts[5] } @@ -110,6 +125,17 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } + if rt.operation != "" { + // Operations complete synchronously; any poll resolves to done, which + // unblocks GAPIC .Wait() callers instead of 404ing. + gcprest.WriteJSON(w, http.StatusOK, operationJSON{ + Name: "projects/" + rt.project + "/locations/" + rt.location + "/operations/" + rt.operation, + Done: true, + }) + + return + } + if rt.trigger == "" { switch r.Method { case http.MethodGet: diff --git a/server/gcp/eventarc/operations.go b/server/gcp/eventarc/operations.go index 10a681af..1b8600f2 100644 --- a/server/gcp/eventarc/operations.go +++ b/server/gcp/eventarc/operations.go @@ -1,6 +1,7 @@ package eventarc import ( + "encoding/json" "net/http" cerrors "github.com/stackshy/cloudemu/v2/errors" @@ -39,6 +40,7 @@ func (h *Handler) createTrigger(w http.ResponseWriter, r *http.Request, rt *rout Name: triggerID, EventBus: bus, EventPattern: encodeEventPattern(body.EventFilters), + Description: encodeTriggerMeta(body.ServiceAccount, body.Labels), }); err != nil { gcprest.WriteCErr(w, err) return @@ -62,7 +64,29 @@ func (h *Handler) createTrigger(w http.ResponseWriter, r *http.Request, rt *rout } gcprest.WriteJSON(w, http.StatusOK, doneOperation(rt, triggerID, - toTriggerJSON(rt.project, rt.location, stored))) + typedResponse(triggerTypeURL, toTriggerJSON(rt.project, rt.location, stored)))) +} + +// triggerTypeURL is the protobuf Any type URL a GAPIC eventarc client expects +// in a done LRO's response so CreateTriggerOperation.Wait() can decode it. +const triggerTypeURL = "type.googleapis.com/google.cloud.eventarc.v1.Trigger" + +// typedResponse renders v as a google.protobuf.Any JSON object (resource fields +// + "@type"); a GAPIC .Wait() can't unmarshal the response without @type. +func typedResponse(typeURL string, v any) map[string]any { + b, err := json.Marshal(v) + if err != nil { + return nil + } + + m := map[string]any{} + if err := json.Unmarshal(b, &m); err != nil { + return nil + } + + m["@type"] = typeURL + + return m } func (h *Handler) getTrigger(w http.ResponseWriter, r *http.Request, rt *route) { diff --git a/server/gcp/eventarc/sdk_roundtrip_test.go b/server/gcp/eventarc/sdk_roundtrip_test.go index 86ec5aaa..1452c7b2 100644 --- a/server/gcp/eventarc/sdk_roundtrip_test.go +++ b/server/gcp/eventarc/sdk_roundtrip_test.go @@ -43,6 +43,51 @@ func parent() string { return "projects/" + testProject + "/locations/" + testLocation } +// TestSDKEventarcOperationAndMetadata guards two #321 fixes: the LRO operation +// endpoint resolves (Operations.Get), and serviceAccount + labels round-trip +// on the trigger instead of being dropped. +func TestSDKEventarcOperationAndMetadata(t *testing.T) { + svc := newEventarcService(t) + ctx := context.Background() + + trigger := &eventarc.Trigger{ + EventFilters: []*eventarc.EventFilter{ + {Attribute: "type", Value: "google.cloud.pubsub.topic.v1.messagePublished"}, + }, + Destination: &eventarc.Destination{CloudRun: &eventarc.CloudRun{Service: "svc", Region: testLocation}}, + ServiceAccount: "runner@demo.iam.gserviceaccount.com", + Labels: map[string]string{"env": "prod"}, + } + + op, err := svc.Projects.Locations.Triggers.Create(parent(), trigger). + TriggerId("meta-trig").Context(ctx).Do() + if err != nil { + t.Fatalf("Triggers.Create: %v", err) + } + + polled, err := svc.Projects.Locations.Operations.Get(op.Name).Context(ctx).Do() + if err != nil { + t.Fatalf("Operations.Get (the #321 LRO route): %v", err) + } + + if !polled.Done { + t.Errorf("polled operation not done: %+v", polled) + } + + got, err := svc.Projects.Locations.Triggers.Get(parent() + "/triggers/meta-trig").Context(ctx).Do() + if err != nil { + t.Fatalf("Triggers.Get: %v", err) + } + + if got.ServiceAccount != "runner@demo.iam.gserviceaccount.com" { + t.Errorf("serviceAccount=%q dropped", got.ServiceAccount) + } + + if got.Labels["env"] != "prod" { + t.Errorf("labels=%v want env=prod", got.Labels) + } +} + func TestSDKEventarcTriggerLifecycle(t *testing.T) { svc := newEventarcService(t) ctx := context.Background() diff --git a/server/gcp/eventarc/types.go b/server/gcp/eventarc/types.go index b8154595..c696692d 100644 --- a/server/gcp/eventarc/types.go +++ b/server/gcp/eventarc/types.go @@ -65,7 +65,7 @@ func triggerResourceName(project, location, id string) string { } // channelName is the synthesized event-bus name backing a location's triggers. -// It has no Eventarc analogue and is never surfaced to the SDK. +// It has no Eventarc analog and is never surfaced to the SDK. func channelName(location string) string { return "eventarc-" + location } @@ -156,11 +156,48 @@ func destinationSummary(dest *destinationJSON) string { // toTriggerJSON converts a driver rule into its Eventarc Trigger element. func toTriggerJSON(project, location string, rule *ebdriver.Rule) triggerJSON { + sa, labels := decodeTriggerMeta(rule.Description) + return triggerJSON{ - Name: triggerResourceName(project, location, rule.Name), - EventFilters: decodeEventPattern(rule.EventPattern), - Destination: destinationFromTargets(rule.Targets), - CreateTime: rule.CreatedAt, - UpdateTime: rule.CreatedAt, + Name: triggerResourceName(project, location, rule.Name), + EventFilters: decodeEventPattern(rule.EventPattern), + Destination: destinationFromTargets(rule.Targets), + ServiceAccount: sa, + Labels: labels, + CreateTime: rule.CreatedAt, + UpdateTime: rule.CreatedAt, } } + +// triggerMeta holds the Eventarc fields the eventbus Rule can't store natively; +// it is JSON-encoded into the rule's Description. +type triggerMeta struct { + ServiceAccount string `json:"serviceAccount,omitempty"` + Labels map[string]string `json:"labels,omitempty"` +} + +func encodeTriggerMeta(sa string, labels map[string]string) string { + if sa == "" && len(labels) == 0 { + return "" + } + + b, err := json.Marshal(triggerMeta{ServiceAccount: sa, Labels: labels}) + if err != nil { + return "" + } + + return string(b) +} + +func decodeTriggerMeta(s string) (serviceAccount string, labels map[string]string) { + if s == "" { + return "", nil + } + + var m triggerMeta + if err := json.Unmarshal([]byte(s), &m); err != nil { + return "", nil + } + + return m.ServiceAccount, m.Labels +} diff --git a/server/gcp/fcm/operations.go b/server/gcp/fcm/operations.go index 361006ae..13196463 100644 --- a/server/gcp/fcm/operations.go +++ b/server/gcp/fcm/operations.go @@ -50,6 +50,23 @@ func (h *Handler) sendMessage(w http.ResponseWriter, r *http.Request, rt route) return } + // FCM requires exactly one target — token, topic, or condition. Reject a + // message that sets more than one (real FCM returns INVALID_ARGUMENT). + targets := 0 + + for _, t := range []string{body.Message.Token, body.Message.Topic, body.Message.Condition} { + if t != "" { + targets++ + } + } + + if targets > 1 { + gcprest.WriteError(w, http.StatusBadRequest, "invalid", + "exactly one of token, topic, condition may be set") + + return + } + if body.ValidateOnly { // Dry run: validate the request only — do NOT auto-create the topic, // publish, or emit metrics. Real FCM returns a fabricated message name. diff --git a/server/gcp/fcm/sdk_roundtrip_test.go b/server/gcp/fcm/sdk_roundtrip_test.go index 3daa71d7..9ecc943f 100644 --- a/server/gcp/fcm/sdk_roundtrip_test.go +++ b/server/gcp/fcm/sdk_roundtrip_test.go @@ -92,4 +92,14 @@ func TestSDKFCMSendErrors(t *testing.T) { if !errors.As(err, &gerr) || gerr.Code != 400 { t.Fatalf("Send(empty): got %v, want 400", err) } + + // A message with more than one target (topic + token) is INVALID_ARGUMENT + // — real FCM allows exactly one of token/topic/condition. + _, err = svc.Projects.Messages.Send("projects/"+testProject, &fcm.SendMessageRequest{ + Message: &fcm.Message{Topic: "news", Token: "device-tok"}, + }).Context(ctx).Do() + + if !errors.As(err, &gerr) || gerr.Code != 400 { + t.Fatalf("Send(multi-target): got %v, want 400", err) + } } diff --git a/server/gcp/firestore/firestore_lifecycle_test.go b/server/gcp/firestore/firestore_lifecycle_test.go index 802bd5ad..7aa237b4 100644 --- a/server/gcp/firestore/firestore_lifecycle_test.go +++ b/server/gcp/firestore/firestore_lifecycle_test.go @@ -292,23 +292,22 @@ func TestDatabaseTypedErrors(t *testing.T) { t.Error("missing doc snapshot should report Exists()==false") } - // Missing collection: never created as a driver table. + // Reading a document in a collection that has never been written is + // NotFound (a read does not create the collection). _, err = client.Collection("ghost").Doc("x").Get(ctx) if code := dbSDKCode(err); code != codes.NotFound { t.Errorf("Get in missing collection: code=%v err=%v, want NotFound", code, err) } - // Writing into a missing collection is also NotFound (tables must be - // pre-created — emulator-specific behavior per survey). - _, err = client.Collection("ghost").Doc("x").Set(ctx, map[string]any{"a": 1}) - if code := dbSDKCode(err); code != codes.NotFound { - t.Errorf("Set in missing collection: code=%v err=%v, want NotFound", code, err) + // Writing into a not-yet-existent collection succeeds — real Firestore + // creates the collection lazily on first write (#321 E2E fix). + if _, err = client.Collection("ghost").Doc("x").Set(ctx, map[string]any{"a": 1}); err != nil { + t.Errorf("Set in new collection: %v, want nil (lazy create)", err) } - // Listing a missing collection surfaces NotFound through the iterator. - _, err = client.Collection("ghost").Documents(ctx).Next() - if code := dbSDKCode(err); code != codes.NotFound { - t.Errorf("List missing collection: code=%v err=%v, want NotFound", code, err) + // After that write the document is readable. + if _, err = client.Collection("ghost").Doc("x").Get(ctx); err != nil { + t.Errorf("Get after lazy-create Set: %v, want nil", err) } // Deleting a missing document is idempotent — no error (matches real diff --git a/server/gcp/firestore/handler.go b/server/gcp/firestore/handler.go index 1cbb0e15..bb0ed8d4 100644 --- a/server/gcp/firestore/handler.go +++ b/server/gcp/firestore/handler.go @@ -13,6 +13,7 @@ package firestore import ( + "context" "encoding/json" "fmt" "net/http" @@ -237,6 +238,8 @@ func (h *Handler) commit(w http.ResponseWriter, r *http.Request, _ string) { } } + h.ensureCollection(r.Context(), p.collection) + if perr := h.db.PutItem(r.Context(), p.collection, item); perr != nil { writeErr(w, perr) return @@ -588,7 +591,9 @@ func parseFirestorePath(path string) (firestorePath, error) { func (h *Handler) createDocument(w http.ResponseWriter, r *http.Request, p firestorePath) { docID := r.URL.Query().Get("documentId") - if docID == "" { + + explicitID := docID != "" + if !explicitID { // Auto-generate an ID; Firestore's default IDs are 20-char IDs but // any string is fine for our purposes. docID = "auto-" + strconv.FormatInt(time.Now().UnixNano(), 10) @@ -600,9 +605,24 @@ func (h *Handler) createDocument(w http.ResponseWriter, r *http.Request, p fires return } + // CreateDocument with an explicit id must fail if that id already exists, + // rather than silently overwriting (real Firestore returns ALREADY_EXISTS). + if explicitID { + if _, err := h.db.GetItem(r.Context(), p.collection, map[string]any{"id": docID}); err == nil { + writeError(w, http.StatusConflict, "ALREADY_EXISTS", + "document "+docID+" already exists") + + return + } + } + item := fieldsToMap(inDoc.Fields) item["id"] = docID + // Firestore creates a collection lazily on first write; the driver requires + // the "table" to exist, so ensure it before writing. + h.ensureCollection(r.Context(), p.collection) + if err := h.db.PutItem(r.Context(), p.collection, item); err != nil { writeErr(w, err) return @@ -611,6 +631,13 @@ func (h *Handler) createDocument(w http.ResponseWriter, r *http.Request, p fires writeJSON(w, http.StatusOK, mapToDocument(item, p, docID)) } +// ensureCollection lazily creates a Firestore collection (driver table keyed on +// the document "id") so a first write doesn't fail with "collection not found". +// An already-exists result is benign. +func (h *Handler) ensureCollection(ctx context.Context, collection string) { + _ = h.db.CreateTable(ctx, dbdriver.TableConfig{Name: collection, PartitionKey: "id"}) +} + func (h *Handler) getDocument(w http.ResponseWriter, r *http.Request, p firestorePath) { item, err := h.db.GetItem(r.Context(), p.collection, map[string]any{"id": p.documentID}) if err != nil { diff --git a/server/gcp/fullserver_test.go b/server/gcp/fullserver_test.go new file mode 100644 index 00000000..c0912d6a --- /dev/null +++ b/server/gcp/fullserver_test.go @@ -0,0 +1,146 @@ +package gcp_test + +import ( + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/stackshy/cloudemu/v2" + gcpserver "github.com/stackshy/cloudemu/v2/server/gcp" +) + +// fullServer boots the complete GCP server with EVERY handler registered (as +// the `cloudemu serve --providers gcp` binary does), so cross-handler dispatch +// collisions surface — the kind single-driver package tests can't catch. +func fullServer(t *testing.T) *httptest.Server { + t.Helper() + + srv := gcpserver.New(gcpserver.DriversFrom(cloudemu.NewGCP())) + ts := httptest.NewServer(srv) + t.Cleanup(ts.Close) + + return ts +} + +func do(t *testing.T, ts *httptest.Server, method, path, body string) (int, string) { + t.Helper() + + var rdr io.Reader + if body != "" { + rdr = strings.NewReader(body) + } + + req, err := http.NewRequest(method, ts.URL+path, rdr) + if err != nil { + t.Fatalf("new request: %v", err) + } + + resp, err := ts.Client().Do(req) + if err != nil { + t.Fatalf("%s %s: %v", method, path, err) + } + defer resp.Body.Close() + + b, _ := io.ReadAll(resp.Body) + + return resp.StatusCode, string(b) +} + +// TestFullServerLROOperationPolling guards the #321 E2E fix: in the full server +// (alloydb/gke register before artifactregistry/eventarc/memorystore and used +// to shadow location operations), a shared LRO handler must resolve every +// location-scoped operation poll to done, not 404. +func TestFullServerLROOperationPolling(t *testing.T) { + ts := fullServer(t) + + // artifactregistry: create returns an op named .../operations/op-r1. + if code, _ := do(t, ts, http.MethodPost, + "/v1/projects/demo/locations/us/repositories?repositoryId=r1", `{"format":"MAVEN"}`); code != http.StatusOK { + t.Fatalf("AR create: %d", code) + } + + if code, body := do(t, ts, http.MethodGet, + "/v1/projects/demo/locations/us/operations/op-r1", ""); code != http.StatusOK || !strings.Contains(body, `"done":true`) { + t.Fatalf("AR op poll: code=%d body=%s (want 200 done:true)", code, body) + } + + // eventarc. + do(t, ts, http.MethodPost, + "/v1/projects/demo/locations/us-central1/triggers?triggerId=t1", + `{"eventFilters":[{"attribute":"type","value":"x"}],"destination":{"cloudRun":{"service":"s","region":"us-central1"}}}`) + + if code, body := do(t, ts, http.MethodGet, + "/v1/projects/demo/locations/us-central1/operations/op-t1", ""); code != http.StatusOK || !strings.Contains(body, `"done":true`) { + t.Fatalf("eventarc op poll: code=%d body=%s", code, body) + } + + // gke (a legitimate location-operations owner) must still resolve. + do(t, ts, http.MethodPost, "/v1/projects/demo/locations/us-central1/clusters", + `{"cluster":{"name":"k1","initialNodeCount":1}}`) + + // GKE's container.Operation uses a `status` field, not the longrunning + // `done` bool — the shared handler must satisfy both. + if code, body := do(t, ts, http.MethodGet, + "/v1/projects/demo/locations/us-central1/operations/operation-00000001", ""); code != http.StatusOK || + !strings.Contains(body, `"status":"DONE"`) { + t.Fatalf("gke op poll: code=%d body=%s (want status DONE)", code, body) + } + + // ...and gke's own cluster GET must not be swallowed by the LRO handler. + if code, _ := do(t, ts, http.MethodGet, + "/v1/projects/demo/locations/us-central1/clusters/k1", ""); code != http.StatusOK { + t.Fatalf("gke cluster GET: %d", code) + } +} + +// TestFullServerFirestoreCreate guards the #321 fix: a document write into a +// not-yet-existent collection auto-creates it (real Firestore), and a duplicate +// explicit id is ALREADY_EXISTS. +func TestFullServerFirestoreCreate(t *testing.T) { + ts := fullServer(t) + + const path = "/v1/projects/demo/databases/(default)/documents/users?documentId=alice" + body := `{"fields":{"name":{"stringValue":"Alice"}}}` + + if code, b := do(t, ts, http.MethodPost, path, body); code != http.StatusOK { + t.Fatalf("create doc in new collection: %d %s", code, b) + } + + if code, _ := do(t, ts, http.MethodPost, path, body); code != http.StatusConflict { + t.Fatalf("duplicate create: %d, want 409", code) + } +} + +// TestFullServerComputeDelete guards the #321 fix: instance delete removes the +// resource (GET after is 404), not a TERMINATED tombstone. +func TestFullServerComputeDelete(t *testing.T) { + ts := fullServer(t) + + const zone = "/compute/v1/projects/demo/zones/us-central1-a/instances" + + do(t, ts, http.MethodPost, zone, + `{"name":"vm1","machineType":"zones/us-central1-a/machineTypes/e2-medium"}`) + + if code, _ := do(t, ts, http.MethodGet, zone+"/vm1", ""); code != http.StatusOK { + t.Fatalf("GET before delete: %d", code) + } + + do(t, ts, http.MethodDelete, zone+"/vm1", "") + + if code, _ := do(t, ts, http.MethodGet, zone+"/vm1", ""); code != http.StatusNotFound { + t.Fatalf("GET after delete: %d, want 404", code) + } +} + +// TestFullServerGCSDoesNotSwallowAPIPaths guards the #321 fix: an unclaimed +// API-version path is NOT misrouted to GCS as a bogus bucket lookup. +func TestFullServerGCSDoesNotSwallowAPIPaths(t *testing.T) { + ts := fullServer(t) + + code, body := do(t, ts, http.MethodGet, "/v1/roles", "") + if strings.Contains(body, "bucket") { + t.Errorf("/v1/roles misrouted to GCS: code=%d body=%s", code, body) + } +} diff --git a/server/gcp/gcp.go b/server/gcp/gcp.go index a3208d9d..1a33d116 100644 --- a/server/gcp/gcp.go +++ b/server/gcp/gcp.go @@ -25,6 +25,7 @@ import ( "github.com/stackshy/cloudemu/v2/server/gcp/gke" "github.com/stackshy/cloudemu/v2/server/gcp/iam" lbsrv "github.com/stackshy/cloudemu/v2/server/gcp/loadbalancer" + "github.com/stackshy/cloudemu/v2/server/gcp/lro" memorystoresrv "github.com/stackshy/cloudemu/v2/server/gcp/memorystore" "github.com/stackshy/cloudemu/v2/server/gcp/monitoring" "github.com/stackshy/cloudemu/v2/server/gcp/networks" @@ -133,6 +134,13 @@ func New(d Drivers) *server.Server { srv := server.New() + // Shared location-operations poller. Registered FIRST so it owns every + // GET /v1/projects/{p}/locations/{l}/operations/{op} uniformly, instead of + // alloydb/gke greedily claiming (and 404ing) operations created by + // artifactregistry, eventarc, memorystore, etc. All emulated ops are + // synchronous, so a done response is always correct. + srv.Register(lro.New()) + if d.Compute != nil { srv.Register(compute.New(d.Compute)) } diff --git a/server/gcp/gcs/gcs_lifecycle_test.go b/server/gcp/gcs/gcs_lifecycle_test.go index 79125101..6a2bc941 100644 --- a/server/gcp/gcs/gcs_lifecycle_test.go +++ b/server/gcp/gcs/gcs_lifecycle_test.go @@ -678,10 +678,9 @@ func TestStorageTrailingBoundaryBytes(t *testing.T) { } } -// TestStorageVersioningSurface documents the provider-specific -// surface: versioning is a driver-level boolean only. The bucket resource -// reports it disabled, and the JSON API exposes no PATCH endpoint, so the -// SDK cannot enable it over HTTP. +// TestStorageVersioningSurface guards the #321 fix: bucket Update (PATCH) is +// now part of the HTTP surface, so enabling versioning over HTTP works and +// round-trips on a subsequent Attrs read. func TestStorageVersioningSurface(t *testing.T) { ctx, client := newStorageClient(t) bkt := mustCreateBucket(t, ctx, client, "e2e-versioning") @@ -695,12 +694,16 @@ func TestStorageVersioningSurface(t *testing.T) { t.Errorf("fresh bucket reports VersioningEnabled=true, want false (survey: default false)") } - // The GCS handler serves only GET/DELETE on /b/{bucket}; bucket Update - // (PATCH) is not part of the HTTP surface — versioning is driver-only. - _, err = bkt.Update(ctx, storage.BucketAttrsToUpdate{VersioningEnabled: true}) - if err == nil { - t.Fatalf("bucket Update(VersioningEnabled) unexpectedly succeeded; HTTP surface was believed to be GET/DELETE only") + if _, err := bkt.Update(ctx, storage.BucketAttrsToUpdate{VersioningEnabled: true}); err != nil { + t.Fatalf("bucket Update(VersioningEnabled) failed (the #321 fix): %v", err) } - t.Logf("bucket Update over HTTP rejected as expected: %v", err) + updated, err := bkt.Attrs(ctx) + if err != nil { + t.Fatalf("bucket Attrs after update: %v", err) + } + + if !updated.VersioningEnabled { + t.Error("VersioningEnabled did not round-trip after Update") + } } diff --git a/server/gcp/gcs/handler.go b/server/gcp/gcs/handler.go index f2ccee73..0fe79eab 100644 --- a/server/gcp/gcs/handler.go +++ b/server/gcp/gcs/handler.go @@ -77,11 +77,50 @@ func (*Handler) Matches(r *http.Request) bool { } // Direct media URLs are /{bucket}/{object}. Two or more path segments - // suffices. + // suffices — but NOT when the first segment is a reserved API prefix + // (v1, v2, sql, compute, …): those are other services' endpoints that no + // earlier handler claimed, and swallowing them here yields a misleading + // "bucket \"v1\" not found" instead of a clean not-implemented/not-found. trimmed := strings.TrimPrefix(p, "/") parts := strings.SplitN(trimmed, "/", pathBucketAndKey) - return len(parts) == pathBucketAndKey && parts[0] != "" && parts[1] != "" + if len(parts) != pathBucketAndKey || parts[0] == "" || parts[1] == "" { + return false + } + + return !isReservedAPIPrefix(parts[0]) +} + +// isReservedAPIPrefix reports whether a first path segment is a Google API +// version/service prefix rather than a plausible bucket name. GCS bucket names +// are lowercase and never collide with these in practice. +func isReservedAPIPrefix(seg string) bool { + switch seg { + case "sql", "compute", "dns", "upload", "storage", "download", "batch", "_cloudemu": + return true + } + + // A whole-segment API version token (v1, v3, v1beta4, v2beta) — but NOT a + // bucket that merely starts that way (e.g. "v2-assets", "v1data"). + return isVersionToken(seg) +} + +// isVersionToken reports whether seg is exactly an API version like v1, v3, +// v1beta4, v2beta — "v" + digits, optionally a beta/alpha qualifier, nothing +// else. A hyphen or other suffix (a real bucket name) is not a version. +func isVersionToken(seg string) bool { + if len(seg) < 2 || seg[0] != 'v' || seg[1] < '0' || seg[1] > '9' { + return false + } + + i := 1 + for i < len(seg) && seg[i] >= '0' && seg[i] <= '9' { + i++ + } + + rest := seg[i:] + + return rest == "" || strings.HasPrefix(rest, "beta") || strings.HasPrefix(rest, "alpha") } // ServeHTTP routes the request based on URL path shape. @@ -146,6 +185,8 @@ func (h *Handler) bucketResource(w http.ResponseWriter, r *http.Request, name st switch r.Method { case http.MethodGet: h.getBucket(w, r, name) + case http.MethodPatch, http.MethodPut: + h.patchBucket(w, r, name) case http.MethodDelete: h.deleteBucket(w, r, name) default: @@ -154,9 +195,7 @@ func (h *Handler) bucketResource(w http.ResponseWriter, r *http.Request, name st } func (h *Handler) createBucket(w http.ResponseWriter, r *http.Request) { - var body struct { - Name string `json:"name"` - } + var body bucketResource if !decodeJSON(w, r, &body) { return @@ -172,14 +211,21 @@ func (h *Handler) createBucket(w http.ResponseWriter, r *http.Request) { return } - writeJSON(w, http.StatusOK, bucketResource{ - Kind: "storage#bucket", - ID: body.Name, - Name: body.Name, - SelfLink: selfLink(r, "/storage/v1/b/"+body.Name), - Location: "US", - TimeCreated: time.Now().UTC().Format(time.RFC3339), - }) + // Persist configuration supplied at create so it round-trips on read. + if len(body.Labels) > 0 { + _ = h.bucket.PutBucketTagging(r.Context(), body.Name, body.Labels) + } + + if body.Versioning != nil && body.Versioning.Enabled { + _ = h.bucket.SetBucketVersioning(r.Context(), body.Name, true) + } + + res := h.bucketView(r, body.Name, time.Now().UTC().Format(time.RFC3339)) + if body.Location != "" { + res.Location = body.Location + } + + writeJSON(w, http.StatusOK, res) } func (h *Handler) listBuckets(w http.ResponseWriter, r *http.Request) { @@ -213,15 +259,7 @@ func (h *Handler) getBucket(w http.ResponseWriter, r *http.Request, name string) for _, b := range buckets { if b.Name == name { - writeJSON(w, http.StatusOK, bucketResource{ - Kind: "storage#bucket", - ID: b.Name, - Name: b.Name, - SelfLink: selfLink(r, "/storage/v1/b/"+b.Name), - Location: "US", - TimeCreated: b.CreatedAt, - }) - + writeJSON(w, http.StatusOK, h.bucketView(r, b.Name, b.CreatedAt)) return } } @@ -229,6 +267,57 @@ func (h *Handler) getBucket(w http.ResponseWriter, r *http.Request, name string) writeError(w, http.StatusNotFound, "notFound", "bucket "+name+" not found") } +// bucketView builds the bucket JSON with its configured versioning + labels +// reflected (real GCS returns these; the driver stores them so a read must +// surface them). +func (h *Handler) bucketView(r *http.Request, name, created string) bucketResource { + res := bucketResource{ + Kind: "storage#bucket", + ID: name, + Name: name, + SelfLink: selfLink(r, "/storage/v1/b/"+name), + Location: "US", + StorageClass: "STANDARD", + TimeCreated: created, + } + + if enabled, err := h.bucket.GetBucketVersioning(r.Context(), name); err == nil && enabled { + res.Versioning = &bucketVersioning{Enabled: true} + } + + if labels, err := h.bucket.GetBucketTagging(r.Context(), name); err == nil && len(labels) > 0 { + res.Labels = labels + } + + return res +} + +// patchBucket applies a bucket configuration update (versioning + labels), +// which real clients set via Buckets.Patch/Update. Without this the driver's +// versioning/label capabilities are unreachable over the wire. +func (h *Handler) patchBucket(w http.ResponseWriter, r *http.Request, name string) { + var body bucketResource + if !decodeJSON(w, r, &body) { + return + } + + if body.Versioning != nil { + if err := h.bucket.SetBucketVersioning(r.Context(), name, body.Versioning.Enabled); err != nil { + writeErr(w, err) + return + } + } + + if body.Labels != nil { + if err := h.bucket.PutBucketTagging(r.Context(), name, body.Labels); err != nil { + writeErr(w, err) + return + } + } + + writeJSON(w, http.StatusOK, h.bucketView(r, name, "")) +} + func (h *Handler) deleteBucket(w http.ResponseWriter, r *http.Request, name string) { if err := h.bucket.DeleteBucket(r.Context(), name); err != nil { writeErr(w, err) diff --git a/server/gcp/gcs/reserved_prefix_test.go b/server/gcp/gcs/reserved_prefix_test.go new file mode 100644 index 00000000..51520623 --- /dev/null +++ b/server/gcp/gcs/reserved_prefix_test.go @@ -0,0 +1,22 @@ +package gcs + +import "testing" + +// TestIsReservedAPIPrefix guards the review fix: API version/service segments +// are reserved (so API paths aren't misrouted to GCS as bucket lookups), but a +// real bucket that merely starts with "v"+digit (e.g. "v2-assets") is not. +func TestIsReservedAPIPrefix(t *testing.T) { + reserved := []string{"v1", "v3", "v1beta4", "v2beta", "sql", "compute", "dns", "upload"} + for _, s := range reserved { + if !isReservedAPIPrefix(s) { + t.Errorf("isReservedAPIPrefix(%q) = false, want true", s) + } + } + + buckets := []string{"v2-assets", "v1data", "my-bucket", "vault", "video", "v"} + for _, s := range buckets { + if isReservedAPIPrefix(s) { + t.Errorf("isReservedAPIPrefix(%q) = true, want false (real bucket name)", s) + } + } +} diff --git a/server/gcp/gcs/types.go b/server/gcp/gcs/types.go index 3092e06f..1950e19c 100644 --- a/server/gcp/gcs/types.go +++ b/server/gcp/gcs/types.go @@ -4,12 +4,19 @@ package gcs // Names map directly to the wire format the SDK expects. type bucketResource struct { - Kind string `json:"kind"` - ID string `json:"id"` - Name string `json:"name"` - SelfLink string `json:"selfLink,omitempty"` - Location string `json:"location,omitempty"` - TimeCreated string `json:"timeCreated,omitempty"` + Kind string `json:"kind"` + ID string `json:"id"` + Name string `json:"name"` + SelfLink string `json:"selfLink,omitempty"` + Location string `json:"location,omitempty"` + StorageClass string `json:"storageClass,omitempty"` + Versioning *bucketVersioning `json:"versioning,omitempty"` + Labels map[string]string `json:"labels,omitempty"` + TimeCreated string `json:"timeCreated,omitempty"` +} + +type bucketVersioning struct { + Enabled bool `json:"enabled"` } type bucketsListResponse struct { diff --git a/server/gcp/gke/sdk_roundtrip_test.go b/server/gcp/gke/sdk_roundtrip_test.go index 64447bc8..30c94037 100644 --- a/server/gcp/gke/sdk_roundtrip_test.go +++ b/server/gcp/gke/sdk_roundtrip_test.go @@ -118,6 +118,8 @@ func TestSDKGKEUpdateAndDelete(t *testing.T) { Update: &container.ClusterUpdate{ DesiredLoggingService: "none", DesiredMonitoringService: "none", + DesiredMasterVersion: "1.31.1-gke.0", + DesiredNodeVersion: "1.31.1-gke.0", }, }).Context(ctx).Do(); err != nil { t.Fatalf("update: %v", err) @@ -138,6 +140,15 @@ func TestSDKGKEUpdateAndDelete(t *testing.T) { t.Fatalf("got logging %q, want none", got.LoggingService) } + // The version upgrade must apply, not stay pinned at the stub version. + if got.CurrentMasterVersion != "1.31.1-gke.0" { + t.Fatalf("got currentMasterVersion %q, want 1.31.1-gke.0", got.CurrentMasterVersion) + } + + if got.CurrentNodeVersion != "1.31.1-gke.0" { + t.Fatalf("got currentNodeVersion %q, want 1.31.1-gke.0", got.CurrentNodeVersion) + } + if got.ResourceLabels["env"] != "test" { t.Fatalf("got label env=%q, want test", got.ResourceLabels["env"]) } diff --git a/server/gcp/gke/types.go b/server/gcp/gke/types.go index e93a0548..a0a9838e 100644 --- a/server/gcp/gke/types.go +++ b/server/gcp/gke/types.go @@ -197,8 +197,8 @@ func toClusterResource(c *gke.Cluster, project, endpoint string, pools []gke.Nod ClusterCaCertificate: k8spki.CertificatePEM(), }, Status: c.Status, - CurrentMasterVer: gke.StubMasterVer, - CurrentNodeVer: gke.StubMasterVer, + CurrentMasterVer: versionOr(c.MasterVersion), + CurrentNodeVer: versionOr(c.NodeVersion), SelfLink: "projects/" + project + "/locations/" + c.Location + "/clusters/" + c.Name, CreateTime: c.CreatedAt.Format("2006-01-02T15:04:05.000Z"), } @@ -210,6 +210,16 @@ func toClusterResource(c *gke.Cluster, project, endpoint string, pools []gke.Nod return out } +// versionOr returns the cluster's applied version, falling back to the stub +// version when none was set (i.e. no upgrade has been requested yet). +func versionOr(v string) string { + if v == "" { + return gke.StubMasterVer + } + + return v +} + func toNodePoolResource(np *gke.NodePool, project string) gkeNodePool { out := gkeNodePool{ Name: np.Name, diff --git a/server/gcp/iam/handler.go b/server/gcp/iam/handler.go index 74707897..26f6eb4d 100644 --- a/server/gcp/iam/handler.go +++ b/server/gcp/iam/handler.go @@ -36,6 +36,7 @@ package iam import ( "net/http" "strings" + "sync" iamdriver "github.com/stackshy/cloudemu/v2/services/iam/driver" ) @@ -48,13 +49,24 @@ const ( ) // Handler serves iam.googleapis.com v1 REST requests against the IAM driver. +// +// Service-account resource policies and the enabled/disabled bit have no place +// in the portable IAM driver, so they're tracked here keyed by SA email. type Handler struct { iam iamdriver.IAM + + mu sync.RWMutex + saPolicy map[string]*iamPolicy // SA email -> resource policy + disabled map[string]bool // SA email -> disabled } // New returns an IAM handler backed by drv. func New(drv iamdriver.IAM) *Handler { - return &Handler{iam: drv} + return &Handler{ + iam: drv, + saPolicy: make(map[string]*iamPolicy), + disabled: make(map[string]bool), + } } // Matches returns true for any /v1/projects/{p}/{serviceAccounts|roles}[/…] @@ -84,6 +96,7 @@ type route struct { name string // SA email or role id, or "" for a collection subKind string // keysSeg, or "" for non-key paths subName string // key id, or "" for the collection + verb string // trailing ":method" (getIamPolicy, signBlob, …), or "" } // parseRoute splits the URL after /v1/projects/. Returns ok=false if the @@ -92,12 +105,21 @@ func parseRoute(urlPath string) (route, bool) { tail := strings.TrimPrefix(urlPath, pathPrefix) tail = strings.TrimRight(tail, "/") + // GCP one-off methods are POSTs to "…/{resource}:method". Split the trailing + // ":method" off the final segment before path splitting (SA emails and role + // ids contain no ':'). + var verb string + if i := strings.LastIndex(tail, ":"); i >= 0 { + verb = tail[i+1:] + tail = tail[:i] + } + parts := strings.Split(tail, "/") if len(parts) < 2 { //nolint:mnd // need at least project + kind return route{}, false } - r := route{project: parts[0], kind: parts[1]} + r := route{project: parts[0], kind: parts[1], verb: verb} if len(parts) >= 3 { //nolint:mnd // optional resource name segment r.name = parts[2] @@ -135,6 +157,13 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // routeServiceAccounts dispatches the /serviceAccounts/* surface. func (h *Handler) routeServiceAccounts(w http.ResponseWriter, r *http.Request, rt *route) { + // One-off ":method" calls (getIamPolicy, signBlob, …) are POSTs on a + // specific service account. + if rt.verb != "" && rt.name != "" { + h.routeSAVerb(w, r, rt) + return + } + switch { // Collection: POST create, GET list. case rt.name == "": diff --git a/server/gcp/iam/operations.go b/server/gcp/iam/operations.go index 546ebe4b..8ab8e737 100644 --- a/server/gcp/iam/operations.go +++ b/server/gcp/iam/operations.go @@ -52,6 +52,11 @@ func (h *Handler) getServiceAccount(w http.ResponseWriter, r *http.Request, proj } sa := saFromUser(user) + + h.mu.RLock() + sa.Disabled = h.disabled[email] + h.mu.RUnlock() + writeJSON(w, toServiceAccountJSON(project, email, &sa)) } diff --git a/server/gcp/iam/sdk_roundtrip_test.go b/server/gcp/iam/sdk_roundtrip_test.go index bfb361a4..13c7efc5 100644 --- a/server/gcp/iam/sdk_roundtrip_test.go +++ b/server/gcp/iam/sdk_roundtrip_test.go @@ -2,6 +2,7 @@ package iam_test import ( "context" + "encoding/base64" "errors" "net/http/httptest" "testing" @@ -215,3 +216,70 @@ func TestSDKGCPIAMNotFoundIsTyped(t *testing.T) { t.Fatalf("got HTTP %d, want 404", apiErr.Code) } } + +// TestSDKGCPIAMServiceAccountVerbs guards the #321 additions: SA-level +// getIamPolicy/setIamPolicy round-trip, signBlob returns a blob, and +// disable/enable toggle the SA's disabled bit. +func TestSDKGCPIAMServiceAccountVerbs(t *testing.T) { + svc := newSDKService(t) + ctx := context.Background() + + parent := "projects/" + testProject + + created, err := svc.Projects.ServiceAccounts.Create(parent, &iamv1.CreateServiceAccountRequest{ + AccountId: "verb-sa", + ServiceAccount: &iamv1.ServiceAccount{DisplayName: "Verb SA"}, + }).Context(ctx).Do() + if err != nil { + t.Fatalf("Create: %v", err) + } + + resource := "projects/" + testProject + "/serviceAccounts/" + created.Email + + // setIamPolicy then getIamPolicy must round-trip the binding. + if _, err := svc.Projects.ServiceAccounts.SetIamPolicy(resource, &iamv1.SetIamPolicyRequest{ + Policy: &iamv1.Policy{ + Bindings: []*iamv1.Binding{{ + Role: "roles/iam.serviceAccountUser", + Members: []string{"user:alice@example.com"}, + }}, + }, + }).Context(ctx).Do(); err != nil { + t.Fatalf("SetIamPolicy: %v", err) + } + + pol, err := svc.Projects.ServiceAccounts.GetIamPolicy(resource).Context(ctx).Do() + if err != nil { + t.Fatalf("GetIamPolicy: %v", err) + } + + if len(pol.Bindings) != 1 || pol.Bindings[0].Role != "roles/iam.serviceAccountUser" { + t.Fatalf("policy did not round-trip: %+v", pol.Bindings) + } + + // signBlob returns a non-empty blob. + sign, err := svc.Projects.ServiceAccounts.SignBlob(resource, &iamv1.SignBlobRequest{ + BytesToSign: base64.StdEncoding.EncodeToString([]byte("hello")), + }).Context(ctx).Do() + if err != nil { + t.Fatalf("SignBlob: %v", err) + } + + if sign.Signature == "" { + t.Error("SignBlob returned empty signature") + } + + // disable then Get shows disabled=true. + if _, err := svc.Projects.ServiceAccounts.Disable(resource, &iamv1.DisableServiceAccountRequest{}).Context(ctx).Do(); err != nil { + t.Fatalf("Disable: %v", err) + } + + got, err := svc.Projects.ServiceAccounts.Get(resource).Context(ctx).Do() + if err != nil { + t.Fatalf("Get after disable: %v", err) + } + + if !got.Disabled { + t.Error("SA not marked disabled after Disable") + } +} diff --git a/server/gcp/iam/types.go b/server/gcp/iam/types.go index e732d6cb..5fdc5c64 100644 --- a/server/gcp/iam/types.go +++ b/server/gcp/iam/types.go @@ -20,6 +20,7 @@ type serviceAccount struct { DisplayName string `json:"displayName,omitempty"` Description string `json:"description,omitempty"` OAuth2ClientID string `json:"oauth2ClientId,omitempty"` + Disabled bool `json:"disabled,omitempty"` Etag string `json:"etag,omitempty"` } diff --git a/server/gcp/iam/verbs.go b/server/gcp/iam/verbs.go new file mode 100644 index 00000000..7175a3ca --- /dev/null +++ b/server/gcp/iam/verbs.go @@ -0,0 +1,177 @@ +package iam + +import ( + "crypto/sha256" + "encoding/base64" + "encoding/json" + "net/http" + "strconv" + "time" +) + +// iamPolicy is the GCP IAM Policy resource returned by getIamPolicy / +// setIamPolicy. Bindings are stored verbatim so a set/get round-trips. +type iamPolicy struct { + Version int `json:"version,omitempty"` + Bindings []policyBinding `json:"bindings,omitempty"` + Etag string `json:"etag,omitempty"` +} + +type policyBinding struct { + Role string `json:"role"` + Members []string `json:"members,omitempty"` +} + +// routeSAVerb dispatches the one-off ":method" service-account calls. All are +// POSTs on a specific SA. +func (h *Handler) routeSAVerb(w http.ResponseWriter, r *http.Request, rt *route) { + if r.Method != http.MethodPost { + writeError(w, http.StatusMethodNotAllowed, "methodNotAllowed", + "method "+rt.verb+" requires POST") + + return + } + + // Confirm the SA exists (all these verbs act on an existing account). + if _, err := h.iam.GetUser(r.Context(), rt.name); err != nil { + writeCErr(w, err) + return + } + + switch rt.verb { + case "getIamPolicy": + h.getSAIamPolicy(w, rt.name) + case "setIamPolicy": + h.setSAIamPolicy(w, r, rt.name) + case "signBlob": + h.signBlob(w, r, rt.name) + case "signJwt": + h.signJwt(w, r, rt.name) + case "generateAccessToken": + h.generateAccessToken(w, r) + case "enable": + h.setDisabled(w, rt.name, false) + case "disable": + h.setDisabled(w, rt.name, true) + default: + writeError(w, http.StatusNotFound, "notFound", "unknown method: "+rt.verb) + } +} + +func (h *Handler) getSAIamPolicy(w http.ResponseWriter, email string) { + h.mu.RLock() + pol := h.saPolicy[email] + h.mu.RUnlock() + + if pol == nil { + // An SA with no policy yet returns an empty, versioned policy (matching + // real GCP, which never 404s getIamPolicy on an existing resource). + pol = &iamPolicy{Version: 1, Etag: etagFor(email, 0)} + } + + writeJSON(w, pol) +} + +func (h *Handler) setSAIamPolicy(w http.ResponseWriter, r *http.Request, email string) { + var body struct { + Policy iamPolicy `json:"policy"` + } + + if !decodeJSON(w, r, &body) { + return + } + + pol := body.Policy + if pol.Version == 0 { + pol.Version = 1 + } + + pol.Etag = etagFor(email, len(pol.Bindings)) + + h.mu.Lock() + h.saPolicy[email] = &pol + h.mu.Unlock() + + writeJSON(w, &pol) +} + +func (*Handler) signBlob(w http.ResponseWriter, r *http.Request, email string) { + // The iam.googleapis.com signBlob uses bytesToSign/signature (base64). + var body struct { + BytesToSign string `json:"bytesToSign"` + } + + if !decodeJSON(w, r, &body) { + return + } + + // Deterministic non-cryptographic "signature": a hash of the SA + payload. + // Real clients only need a stable, base64 blob back. + sig := sha256.Sum256([]byte(email + ":" + body.BytesToSign)) + + writeJSON(w, map[string]string{ + "keyId": "key-" + email, + "signature": base64.StdEncoding.EncodeToString(sig[:]), + }) +} + +func (*Handler) signJwt(w http.ResponseWriter, r *http.Request, email string) { + var body struct { + Payload string `json:"payload"` // JSON claims string + } + + if !decodeJSON(w, r, &body) { + return + } + + // A JWT-shaped (header.payload.signature) string; not cryptographically + // valid, but structurally what clients expect to parse. + header := base64.RawURLEncoding.EncodeToString([]byte(`{"alg":"RS256","typ":"JWT"}`)) + claims := base64.RawURLEncoding.EncodeToString([]byte(body.Payload)) + sig := sha256.Sum256([]byte(email + body.Payload)) + sigStr := base64.RawURLEncoding.EncodeToString(sig[:]) + + writeJSON(w, map[string]string{ + "keyId": "key-" + email, + "signedJwt": header + "." + claims + "." + sigStr, + }) +} + +func (*Handler) generateAccessToken(w http.ResponseWriter, r *http.Request) { + var body struct { + Scope []string `json:"scope"` + Lifetime string `json:"lifetime"` + } + + _ = decodeJSON(w, r, &body) // request fields are optional for the stub + + expire := time.Now().Add(time.Hour).UTC().Format(time.RFC3339) + + writeJSON(w, map[string]string{ + "accessToken": "ya29.emulated-" + strconv.FormatInt(int64(len(body.Scope)), 10), + "expireTime": expire, + }) +} + +func (h *Handler) setDisabled(w http.ResponseWriter, email string, disabled bool) { + h.mu.Lock() + h.disabled[email] = disabled + h.mu.Unlock() + + writeJSON(w, map[string]any{}) +} + +// etagFor returns a stable etag for a policy state. +func etagFor(email string, n int) string { + return base64.StdEncoding.EncodeToString([]byte(email + ":" + strconv.Itoa(n))) +} + +// decodeJSON decodes a JSON request body, writing a 400 on failure. +func decodeJSON(w http.ResponseWriter, r *http.Request, v any) bool { + if err := json.NewDecoder(r.Body).Decode(v); err != nil { + writeError(w, http.StatusBadRequest, "invalidArgument", "invalid JSON: "+err.Error()) + return false + } + + return true +} diff --git a/server/gcp/loadbalancer/operations.go b/server/gcp/loadbalancer/operations.go index fabaa668..5b6cee74 100644 --- a/server/gcp/loadbalancer/operations.go +++ b/server/gcp/loadbalancer/operations.go @@ -30,6 +30,9 @@ func (h *Handler) insertBackendService(w http.ResponseWriter, r *http.Request, r Name: req.Name, Protocol: req.Protocol, Port: req.Port, + // The driver TargetGroup can't hold these GCP fields, so round-trip them + // through tags rather than dropping them on read. + Tags: backendServiceTags(&req), }); err != nil { gcprest.WriteCErr(w, err) return @@ -231,7 +234,7 @@ func (h *Handler) findLBByName(ctx context.Context, name string) (*lbdriver.LBIn //nolint:gocritic // rp is a request-scoped value func toBackendServiceResponse(tg *lbdriver.TargetGroupInfo, rp gcprest.ResourcePath, host string) backendServiceResponse { - return backendServiceResponse{ + resp := backendServiceResponse{ Kind: "compute#backendService", ID: numericID(tg.ID), Name: tg.Name, @@ -239,6 +242,43 @@ func toBackendServiceResponse(tg *lbdriver.TargetGroupInfo, rp gcprest.ResourceP Port: tg.Port, SelfLink: gcprest.SelfLink(host, rp.Project, gcprest.ScopeGlobal, "", resourceBackendServices, tg.Name), } + + resp.Description = tg.Tags[bsDescriptionTag] + resp.PortName = tg.Tags[bsPortNameTag] + + if hc := tg.Tags[bsHealthChecksTag]; hc != "" { + resp.HealthChecks = strings.Split(hc, ",") + } + + return resp +} + +// Reserved tag keys carry the GCP backend-service fields the driver's target +// group can't model. +const ( + bsDescriptionTag = "cloudemu:gcpBsDescription" + bsPortNameTag = "cloudemu:gcpBsPortName" + bsHealthChecksTag = "cloudemu:gcpBsHealthChecks" +) + +// backendServiceTags folds the GCP-specific backend-service fields into a tag +// map so they round-trip through the driver. +func backendServiceTags(req *backendServiceRequest) map[string]string { + tags := map[string]string{} + + if req.Description != "" { + tags[bsDescriptionTag] = req.Description + } + + if req.PortName != "" { + tags[bsPortNameTag] = req.PortName + } + + if len(req.HealthChecks) > 0 { + tags[bsHealthChecksTag] = strings.Join(req.HealthChecks, ",") + } + + return tags } //nolint:gocritic // rp is a request-scoped value diff --git a/server/gcp/loadbalancer/sdk_roundtrip_test.go b/server/gcp/loadbalancer/sdk_roundtrip_test.go index f33c6be6..ba5ee68a 100644 --- a/server/gcp/loadbalancer/sdk_roundtrip_test.go +++ b/server/gcp/loadbalancer/sdk_roundtrip_test.go @@ -52,9 +52,12 @@ func TestSDKGCPBackendServiceRoundTrip(t *testing.T) { insertOp, err := client.Insert(ctx, &computepb.InsertBackendServiceRequest{ Project: testProject, BackendServiceResource: &computepb.BackendService{ - Name: ptrStr("web-backend"), - Protocol: ptrStr("HTTP"), - Port: func() *int32 { p := int32(80); return &p }(), + Name: ptrStr("web-backend"), + Protocol: ptrStr("HTTP"), + Port: func() *int32 { p := int32(80); return &p }(), + Description: ptrStr("web tier"), + PortName: ptrStr("http"), + HealthChecks: []string{"projects/p1/global/healthChecks/hc1"}, }, }) if err != nil { @@ -81,6 +84,19 @@ func TestSDKGCPBackendServiceRoundTrip(t *testing.T) { t.Fatalf("protocol = %q, want HTTP", got.GetProtocol()) } + // description / portName / healthChecks must round-trip, not be dropped. + if got.GetDescription() != "web tier" { + t.Errorf("description = %q, want 'web tier'", got.GetDescription()) + } + + if got.GetPortName() != "http" { + t.Errorf("portName = %q, want http", got.GetPortName()) + } + + if len(got.GetHealthChecks()) != 1 || got.GetHealthChecks()[0] != "projects/p1/global/healthChecks/hc1" { + t.Errorf("healthChecks = %v, want [.../hc1]", got.GetHealthChecks()) + } + // List. var names []string diff --git a/server/gcp/loadbalancer/types.go b/server/gcp/loadbalancer/types.go index ced9079b..b91c499b 100644 --- a/server/gcp/loadbalancer/types.go +++ b/server/gcp/loadbalancer/types.go @@ -16,13 +16,15 @@ type backendServiceRequest struct { } type backendServiceResponse struct { - Kind string `json:"kind"` - ID string `json:"id"` - Name string `json:"name"` - Description string `json:"description,omitempty"` - Protocol string `json:"protocol,omitempty"` - Port int `json:"port,omitempty"` - SelfLink string `json:"selfLink"` + Kind string `json:"kind"` + ID string `json:"id"` + Name string `json:"name"` + Description string `json:"description,omitempty"` + Protocol string `json:"protocol,omitempty"` + Port int `json:"port,omitempty"` + PortName string `json:"portName,omitempty"` + HealthChecks []string `json:"healthChecks,omitempty"` + SelfLink string `json:"selfLink"` } type backendServiceListResponse struct { diff --git a/server/gcp/lro/handler.go b/server/gcp/lro/handler.go new file mode 100644 index 00000000..6e4b4e6b --- /dev/null +++ b/server/gcp/lro/handler.go @@ -0,0 +1,81 @@ +// Package lro provides a shared long-running-operation poller for GCP +// location-scoped operations: GET /v1/projects/{p}/locations/{l}/operations/{op}. +// +// In real GCP each service exposes its own operations endpoint on its own API +// host (alloydb.googleapis.com, artifactregistry.googleapis.com, …). CloudEmu +// collapses every service onto one HTTP server, so those per-service operation +// paths become indistinguishable by URL alone — whichever handler is registered +// first (alloydb/gke) would greedily answer every location operation poll and +// 404 the ones it didn't create, shadowing artifactregistry, eventarc, +// memorystore, etc. +// +// Every CloudEmu mutation completes synchronously (the create/delete response +// already carries done:true with the result inlined), so an operation poll only +// needs to report completion. This one handler, registered ahead of the +// service handlers, answers all location-scoped operation polls uniformly with +// a done operation — the single "operations host" the collapsed server needs. +package lro + +import ( + "net/http" + "strings" + + "github.com/stackshy/cloudemu/v2/server/wire/gcprest" +) + +const ( + pathPrefix = "/v1/projects/" + locationsSeg = "locations" + operationsSeg = "operations" +) + +// Handler answers GET on location-scoped operation names. +type Handler struct{} + +// New returns the shared location-operations handler. +func New() *Handler { return &Handler{} } + +// Matches claims GET /v1/projects/{p}/locations/{l}/operations/{op}. +func (*Handler) Matches(r *http.Request) bool { + if r.Method != http.MethodGet { + return false + } + + _, _, op, ok := parse(r.URL.Path) + + return ok && op != "" +} + +// ServeHTTP returns a completed operation echoing the polled name. +func (*Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + project, location, op, ok := parse(r.URL.Path) + if !ok { + gcprest.WriteError(w, http.StatusBadRequest, "invalid", "malformed operation path") + return + } + + // Return a superset that satisfies both operation schemas served here: + // google.longrunning.Operation reads `done` (artifactregistry, eventarc, + // memorystore, alloydb), while GKE's container.Operation reads `status`. + gcprest.WriteJSON(w, http.StatusOK, map[string]any{ + "name": "projects/" + project + "/locations/" + location + "/operations/" + op, + "done": true, + "status": "DONE", + }) +} + +// parse splits /v1/projects/{p}/locations/{l}/operations/{op}. +func parse(urlPath string) (project, location, op string, ok bool) { + if len(urlPath) < len(pathPrefix) || urlPath[:len(pathPrefix)] != pathPrefix { + return "", "", "", false + } + + parts := strings.Split(urlPath[len(pathPrefix):], "/") + // [project, locations, {l}, operations, {op}] + const want = 5 + if len(parts) != want || parts[1] != locationsSeg || parts[3] != operationsSeg { + return "", "", "", false + } + + return parts[0], parts[2], parts[4], true +} diff --git a/server/gcp/memorystore/handler.go b/server/gcp/memorystore/handler.go index 03160299..fa4bfedf 100644 --- a/server/gcp/memorystore/handler.go +++ b/server/gcp/memorystore/handler.go @@ -139,6 +139,8 @@ func (h *Handler) serveInstances(w http.ResponseWriter, r *http.Request, rt rout switch r.Method { case http.MethodGet: h.getInstance(w, r, rt) + case http.MethodPatch: + h.patchInstance(w, r, rt) case http.MethodDelete: h.deleteInstance(w, r, rt) default: diff --git a/server/gcp/memorystore/operations.go b/server/gcp/memorystore/operations.go index 7bcd32e5..823d3e2a 100644 --- a/server/gcp/memorystore/operations.go +++ b/server/gcp/memorystore/operations.go @@ -28,7 +28,7 @@ func (h *Handler) createInstance(w http.ResponseWriter, r *http.Request, rt rout Name: instanceID, Engine: "redis", NodeType: body.Tier, - Tags: body.Labels, + Tags: instanceTags(&body, nil), Scope: scope.Scope{Project: rt.project}, }) if err != nil { @@ -80,6 +80,48 @@ func (h *Handler) listInstances(w http.ResponseWriter, r *http.Request, rt route gcprest.WriteJSON(w, http.StatusOK, listInstancesResponse{Instances: out}) } +// patchInstance handles PATCH .../instances/{i} — Update. Real clients change +// memorySizeGb, displayName, tier, and labels here; without it those are stuck +// at their create-time values. +func (h *Handler) patchInstance(w http.ResponseWriter, r *http.Request, rt route) { + existing, err := h.cache.GetCache(r.Context(), rt.name) + if err != nil { + gcprest.WriteCErr(w, err) + return + } + + var body instanceJSON + if !gcprest.DecodeJSON(w, r, &body) { + return + } + + nodeType := existing.NodeType + if body.Tier != "" { + nodeType = body.Tier + } + + updated, err := h.cache.UpdateCache(r.Context(), cachedriver.CacheConfig{ + Name: rt.name, + Engine: "redis", + NodeType: nodeType, + Tags: instanceTags(&body, existing.Tags), + }) + if err != nil { + gcprest.WriteCErr(w, err) + return + } + + inst := toInstanceJSON(rt.project, rt.location, shortInstanceID(updated.Name), updated) + + raw, mErr := json.Marshal(inst) + if mErr != nil { + gcprest.WriteError(w, http.StatusInternalServerError, "internalError", mErr.Error()) + return + } + + gcprest.WriteJSON(w, http.StatusOK, doneOperation(rt.project, rt.location, "update-"+rt.name, raw)) +} + // deleteInstance handles DELETE .../instances/{i} — Delete. The operation // completes inline, so a done=true Operation with an empty response is returned. func (h *Handler) deleteInstance(w http.ResponseWriter, r *http.Request, rt route) { diff --git a/server/gcp/memorystore/sdk_roundtrip_test.go b/server/gcp/memorystore/sdk_roundtrip_test.go index fe3b96fa..4a4c7fee 100644 --- a/server/gcp/memorystore/sdk_roundtrip_test.go +++ b/server/gcp/memorystore/sdk_roundtrip_test.go @@ -53,6 +53,49 @@ func instanceName(id string) string { return parent() + "/instances/" + id } +// TestSDKMemorystoreConfigRoundTrip guards the #321 fixes: memorySizeGb, +// redisVersion and displayName round-trip (not hardcoded), and Update (PATCH) +// applies a new size. +func TestSDKMemorystoreConfigRoundTrip(t *testing.T) { + svc := newRedisService(t) + ctx := context.Background() + + if _, err := svc.Projects.Locations.Instances.Create(parent(), &redis.Instance{ + Tier: "STANDARD_HA", + MemorySizeGb: 5, + RedisVersion: "REDIS_7_0", + DisplayName: "prod cache", + }).InstanceId("big").Context(ctx).Do(); err != nil { + t.Fatalf("Create: %v", err) + } + + got, err := svc.Projects.Locations.Instances.Get(instanceName("big")).Context(ctx).Do() + if err != nil { + t.Fatalf("Get: %v", err) + } + + if got.MemorySizeGb != 5 || got.RedisVersion != "REDIS_7_0" || got.DisplayName != "prod cache" { + t.Fatalf("config did not round-trip: size=%d version=%q display=%q", + got.MemorySizeGb, got.RedisVersion, got.DisplayName) + } + + // Update the size via PATCH. + if _, err := svc.Projects.Locations.Instances.Patch(instanceName("big"), &redis.Instance{ + MemorySizeGb: 8, + }).UpdateMask("memorySizeGb").Context(ctx).Do(); err != nil { + t.Fatalf("Patch (the #321 Update fix): %v", err) + } + + after, err := svc.Projects.Locations.Instances.Get(instanceName("big")).Context(ctx).Do() + if err != nil { + t.Fatalf("Get after patch: %v", err) + } + + if after.MemorySizeGb != 8 { + t.Errorf("after patch memorySizeGb=%d want 8", after.MemorySizeGb) + } +} + func TestSDKMemorystoreLifecycle(t *testing.T) { svc := newRedisService(t) ctx := context.Background() diff --git a/server/gcp/memorystore/types.go b/server/gcp/memorystore/types.go index 6b86851b..c6a19b9d 100644 --- a/server/gcp/memorystore/types.go +++ b/server/gcp/memorystore/types.go @@ -104,18 +104,94 @@ func toInstanceJSON(project, location, instanceID string, info *cachedriver.Cach tier = info.NodeType } + memSize := int64(1) + if v, err := strconv.ParseInt(info.Tags[memorySizeTag], 10, 64); err == nil && v > 0 { + memSize = v + } + + redisVersion := defaultRedisVersion + if v := info.Tags[redisVersionTag]; v != "" { + redisVersion = v + } + return instanceJSON{ - Name: instanceResourceName(project, location, instanceID), - Tier: tier, - MemorySizeGb: 1, - RedisVersion: defaultRedisVersion, - State: stateOrReady(info.Status), - Host: host, - Port: port, - CreateTime: info.CreatedAt, - Labels: info.Tags, - LocationID: location, + Name: instanceResourceName(project, location, instanceID), + DisplayName: info.Tags[displayNameTag], + Tier: tier, + MemorySizeGb: memSize, + RedisVersion: redisVersion, + State: stateOrReady(info.Status), + Host: host, + Port: port, + CreateTime: info.CreatedAt, + Labels: stripReservedTags(info.Tags), + LocationID: location, + ReservedIPRng: info.Tags[reservedIPTag], + } +} + +// Reserved tag keys carry GCP-specific fields the cache driver can't model, so +// they round-trip through the cache's tags. +const ( + memorySizeTag = "cloudemu:gcpMemorySizeGb" + redisVersionTag = "cloudemu:gcpRedisVersion" + displayNameTag = "cloudemu:gcpDisplayName" + reservedIPTag = "cloudemu:gcpReservedIpRange" +) + +// instanceTags folds the GCP-specific request fields into the tag map, layered +// over existing tags so a partial PATCH keeps unspecified values. +func instanceTags(body *instanceJSON, existing map[string]string) map[string]string { + out := make(map[string]string, len(existing)+len(body.Labels)) + + for k, v := range existing { + out[k] = v + } + + for k, v := range body.Labels { + out[k] = v + } + + if body.MemorySizeGb > 0 { + out[memorySizeTag] = strconv.FormatInt(body.MemorySizeGb, 10) + } + + if body.RedisVersion != "" { + out[redisVersionTag] = body.RedisVersion } + + if body.DisplayName != "" { + out[displayNameTag] = body.DisplayName + } + + if body.ReservedIPRng != "" { + out[reservedIPTag] = body.ReservedIPRng + } + + return out +} + +// stripReservedTags returns user labels without cloudemu-internal keys. +func stripReservedTags(in map[string]string) map[string]string { + if len(in) == 0 { + return nil + } + + out := make(map[string]string, len(in)) + + for k, v := range in { + if strings.HasPrefix(k, "cloudemu:") { + continue + } + + out[k] = v + } + + if len(out) == 0 { + return nil + } + + return out } // stateOrReady maps the driver status onto Memorystore's state enum, defaulting diff --git a/server/gcp/monitoring/handler.go b/server/gcp/monitoring/handler.go index 048c0aef..22eeba98 100644 --- a/server/gcp/monitoring/handler.go +++ b/server/gcp/monitoring/handler.go @@ -11,10 +11,12 @@ package monitoring import ( - "context" "encoding/json" "net/http" + "strconv" "strings" + "sync" + "sync/atomic" cerrors "github.com/stackshy/cloudemu/v2/errors" mondriver "github.com/stackshy/cloudemu/v2/services/monitoring/driver" @@ -26,13 +28,22 @@ const ( ) // Handler serves GCP Cloud Monitoring alert-policy REST requests. +// +// The portable monitoring driver models a threshold Alarm, not GCP's richer +// alert-policy shape (conditions, combiner, notificationChannels, userLabels). +// To avoid dropping those on read, the full policy is held here keyed by name; +// the driver alarm is kept as an existence marker. type Handler struct { mon mondriver.Monitoring + + mu sync.RWMutex + policies map[string]alertPolicy // keyed by policy short-name + seq atomic.Uint64 } // New returns a Cloud Monitoring handler. func New(m mondriver.Monitoring) *Handler { - return &Handler{mon: m} + return &Handler{mon: m, policies: make(map[string]alertPolicy)} } // Matches returns true for /v3/projects/.../alertPolicies URLs. @@ -67,6 +78,8 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { switch r.Method { case http.MethodGet: h.getPolicy(w, r, project, name) + case http.MethodPatch, http.MethodPut: + h.patchPolicy(w, r, project, name) case http.MethodDelete: h.deletePolicy(w, r, name) default: @@ -97,9 +110,11 @@ func (h *Handler) createPolicy(w http.ResponseWriter, r *http.Request, project s name := body.DisplayName if name == "" { - name = "policy-" + randID() + name = "policy-" + strconv.FormatUint(h.seq.Add(1), 10) } + // The driver alarm is only an existence marker; the full policy shape lives + // in the handler registry so conditions/combiner/labels round-trip. cfg := mondriver.AlarmConfig{ Name: name, Namespace: "gcp", @@ -116,77 +131,129 @@ func (h *Handler) createPolicy(w http.ResponseWriter, r *http.Request, project s return } - body.Name = "projects/" + project + "/alertPolicies/" + name + body.Name = policyResourceName(project, name) + + h.mu.Lock() + h.policies[name] = body + h.mu.Unlock() writeJSON(w, http.StatusOK, body) } -func (h *Handler) getPolicy(w http.ResponseWriter, r *http.Request, project, name string) { - if err := policyExists(r.Context(), h.mon, name); err != nil { - writeErr(w, err) +func (h *Handler) getPolicy(w http.ResponseWriter, _ *http.Request, project, name string) { + h.mu.RLock() + pol, ok := h.policies[name] + h.mu.RUnlock() + + if !ok { + writeError(w, http.StatusNotFound, "NOT_FOUND", "alertPolicy "+name+" not found") return } - writeJSON(w, http.StatusOK, alertPolicy{ - Name: "projects/" + project + "/alertPolicies/" + name, - DisplayName: name, - Enabled: true, - }) + pol.Name = policyResourceName(project, name) + + writeJSON(w, http.StatusOK, pol) } -func (h *Handler) listPolicies(w http.ResponseWriter, r *http.Request, project string) { - alarms, err := h.mon.DescribeAlarms(r.Context(), nil) - if err != nil { - writeErr(w, err) - return - } +func (h *Handler) listPolicies(w http.ResponseWriter, _ *http.Request, project string) { + h.mu.RLock() + out := alertPoliciesList{AlertPolicies: make([]alertPolicy, 0, len(h.policies))} - out := alertPoliciesList{} - for i := range alarms { - out.AlertPolicies = append(out.AlertPolicies, alertPolicy{ - Name: "projects/" + project + "/alertPolicies/" + alarms[i].Name, - DisplayName: alarms[i].Name, - Enabled: true, - }) + for name := range h.policies { + pol := h.policies[name] + pol.Name = policyResourceName(project, name) + out.AlertPolicies = append(out.AlertPolicies, pol) } + h.mu.RUnlock() writeJSON(w, http.StatusOK, out) } -func (h *Handler) deletePolicy(w http.ResponseWriter, r *http.Request, name string) { - if err := policyExists(r.Context(), h.mon, name); err != nil { - writeErr(w, err) +// patchPolicy applies a partial update. GCP scopes changes by updateMask; the +// pragmatic emulation overwrites any field the caller supplied (non-zero), +// which covers displayName/combiner/enabled/conditions/labels/channels. +func (h *Handler) patchPolicy(w http.ResponseWriter, r *http.Request, project, name string) { + // Decode with a pointer Enabled so an omitted "enabled" is distinguishable + // from an explicit false — a partial PATCH must leave it unchanged, not + // silently disable the policy (real GCP applies only the updateMask paths). + var body struct { + DisplayName string `json:"displayName"` + Combiner string `json:"combiner"` + Conditions []alertCondition `json:"conditions"` + UserLabels map[string]string `json:"userLabels"` + NotificationChannels []string `json:"notificationChannels"` + Enabled *bool `json:"enabled"` + } + + r.Body = http.MaxBytesReader(w, r.Body, maxBodyBytes) + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + writeError(w, http.StatusBadRequest, "INVALID_ARGUMENT", err.Error()) return } - if err := h.mon.DeleteAlarm(r.Context(), name); err != nil { - writeErr(w, err) + h.mu.Lock() + + cur, ok := h.policies[name] + if !ok { + h.mu.Unlock() + writeError(w, http.StatusNotFound, "NOT_FOUND", "alertPolicy "+name+" not found") + return } - writeJSON(w, http.StatusOK, map[string]any{}) + if body.DisplayName != "" { + cur.DisplayName = body.DisplayName + } + + if body.Combiner != "" { + cur.Combiner = body.Combiner + } + + if body.Conditions != nil { + cur.Conditions = body.Conditions + } + + if body.UserLabels != nil { + cur.UserLabels = body.UserLabels + } + + if body.NotificationChannels != nil { + cur.NotificationChannels = body.NotificationChannels + } + + if body.Enabled != nil { + cur.Enabled = *body.Enabled + } + + h.policies[name] = cur + h.mu.Unlock() + + cur.Name = policyResourceName(project, name) + + writeJSON(w, http.StatusOK, cur) } -// policyExists reports whether an alert policy exists by name. -func policyExists(ctx context.Context, m mondriver.Monitoring, name string) error { - alarms, err := m.DescribeAlarms(ctx, nil) - if err != nil { - return err +func (h *Handler) deletePolicy(w http.ResponseWriter, r *http.Request, name string) { + h.mu.Lock() + _, ok := h.policies[name] + delete(h.policies, name) + h.mu.Unlock() + + if !ok { + writeError(w, http.StatusNotFound, "NOT_FOUND", "alertPolicy "+name+" not found") + return } - for i := range alarms { - if alarms[i].Name == name { - return nil - } + if err := h.mon.DeleteAlarm(r.Context(), name); err != nil && !cerrors.IsNotFound(err) { + writeErr(w, err) + return } - return cerrors.Newf(cerrors.NotFound, "alertPolicy %s not found", name) + writeJSON(w, http.StatusOK, map[string]any{}) } -// randID returns a small random identifier for synthesized policy names. -// Stable enough for HTTP-level tests; not cryptographic. -func randID() string { - return "auto" +func policyResourceName(project, name string) string { + return "projects/" + project + "/alertPolicies/" + name } func writeJSON(w http.ResponseWriter, status int, v any) { diff --git a/server/gcp/monitoring/monitoring_test.go b/server/gcp/monitoring/monitoring_test.go index f22bc73b..0b1a660f 100644 --- a/server/gcp/monitoring/monitoring_test.go +++ b/server/gcp/monitoring/monitoring_test.go @@ -91,3 +91,122 @@ func TestMonitoringAlertPolicyCRUD(t *testing.T) { t.Errorf("delete status=%d", delResp.StatusCode) } } + +// TestMonitoringAlertPolicySemantics guards the #321 fix: a policy's +// conditions/combiner/enabled/userLabels must round-trip on Get (not be +// dropped for a hardcoded skeleton), and PATCH must apply. +func TestMonitoringAlertPolicySemantics(t *testing.T) { + cloudP := cloudemu.NewGCP() + srv := gcpserver.New(gcpserver.Drivers{Monitoring: cloudP.CloudMonitoring}) + + ts := httptest.NewServer(srv) + t.Cleanup(ts.Close) + + const collURL = "/v3/projects/p1/alertPolicies" + + create := bytes.NewBufferString(`{ + "displayName": "cpu-alert", + "combiner": "AND", + "enabled": true, + "userLabels": {"team": "sre"}, + "conditions": [{"displayName": "cpu>80"}] + }`) + + resp, err := ts.Client().Post(ts.URL+collURL, "application/json", create) + if err != nil { + t.Fatal(err) + } + resp.Body.Close() + + // Get must reflect what was created. + getResp, err := ts.Client().Get(ts.URL + collURL + "/cpu-alert") + if err != nil { + t.Fatal(err) + } + defer getResp.Body.Close() + + var got map[string]any + _ = json.NewDecoder(getResp.Body).Decode(&got) + + if got["combiner"] != "AND" { + t.Errorf("combiner=%v want AND (dropped on read)", got["combiner"]) + } + + if got["enabled"] != true { + t.Errorf("enabled=%v want true", got["enabled"]) + } + + if ul, _ := got["userLabels"].(map[string]any); ul["team"] != "sre" { + t.Errorf("userLabels=%v want team=sre", got["userLabels"]) + } + + if conds, _ := got["conditions"].([]any); len(conds) != 1 { + t.Errorf("conditions=%v want 1", got["conditions"]) + } + + // PATCH updates the combiner but OMITS enabled — a partial patch must NOT + // silently disable the policy (regression guard for the omitted-field bug). + patch := bytes.NewBufferString(`{"combiner": "OR"}`) + patchReq, _ := http.NewRequest(http.MethodPatch, ts.URL+collURL+"/cpu-alert", patch) + patchReq.Header.Set("Content-Type", "application/json") + + patchResp, err := ts.Client().Do(patchReq) + if err != nil { + t.Fatal(err) + } + defer patchResp.Body.Close() + + var patched map[string]any + _ = json.NewDecoder(patchResp.Body).Decode(&patched) + + if patched["combiner"] != "OR" { + t.Errorf("after PATCH combiner=%v want OR", patched["combiner"]) + } + + if patched["enabled"] != true { + t.Errorf("after PATCH omitting enabled, enabled=%v want true (must not silently disable)", patched["enabled"]) + } +} + +// TestMonitoringNonThresholdCondition guards that a non-threshold condition +// (conditionAbsent) round-trips instead of being dropped. +func TestMonitoringNonThresholdCondition(t *testing.T) { + cloudP := cloudemu.NewGCP() + srv := gcpserver.New(gcpserver.Drivers{Monitoring: cloudP.CloudMonitoring}) + + ts := httptest.NewServer(srv) + t.Cleanup(ts.Close) + + const collURL = "/v3/projects/p1/alertPolicies" + + create := bytes.NewBufferString(`{ + "displayName": "absent-alert", + "combiner": "OR", + "conditions": [{"displayName": "no data", "conditionAbsent": {"duration": "300s"}}] + }`) + + resp, err := ts.Client().Post(ts.URL+collURL, "application/json", create) + if err != nil { + t.Fatal(err) + } + resp.Body.Close() + + getResp, err := ts.Client().Get(ts.URL + collURL + "/absent-alert") + if err != nil { + t.Fatal(err) + } + defer getResp.Body.Close() + + var got map[string]any + _ = json.NewDecoder(getResp.Body).Decode(&got) + + conds, _ := got["conditions"].([]any) + if len(conds) != 1 { + t.Fatalf("conditions=%v want 1", got["conditions"]) + } + + c0, _ := conds[0].(map[string]any) + if _, ok := c0["conditionAbsent"]; !ok { + t.Errorf("conditionAbsent dropped on round-trip: %+v", c0) + } +} diff --git a/server/gcp/monitoring/types.go b/server/gcp/monitoring/types.go index 9b2cdf4b..efb1bea6 100644 --- a/server/gcp/monitoring/types.go +++ b/server/gcp/monitoring/types.go @@ -15,9 +15,17 @@ type alertPolicy struct { MutationRecord any `json:"mutationRecord,omitempty"` } +// alertCondition round-trips every Cloud Monitoring condition variant, not just +// conditionThreshold — conditionAbsent / MQL / PromQL / matchedLog are carried +// verbatim so they survive a create→read cycle instead of being silently dropped. type alertCondition struct { - Name string `json:"name,omitempty"` - DisplayName string `json:"displayName,omitempty"` + Name string `json:"name,omitempty"` + DisplayName string `json:"displayName,omitempty"` + ConditionThreshold any `json:"conditionThreshold,omitempty"` + ConditionAbsent any `json:"conditionAbsent,omitempty"` + ConditionMatchedLog any `json:"conditionMatchedLog,omitempty"` + ConditionMonitoringQueryLanguage any `json:"conditionMonitoringQueryLanguage,omitempty"` + ConditionPrometheusQueryLanguage any `json:"conditionPrometheusQueryLanguage,omitempty"` } type alertPoliciesList struct { diff --git a/server/gcp/networks/handler.go b/server/gcp/networks/handler.go index f898a63a..1b4193b5 100644 --- a/server/gcp/networks/handler.go +++ b/server/gcp/networks/handler.go @@ -23,8 +23,10 @@ package networks import ( "context" + "encoding/json" "net/http" "strconv" + "strings" cerrors "github.com/stackshy/cloudemu/v2/errors" "github.com/stackshy/cloudemu/v2/server/wire/gcprest" @@ -39,7 +41,11 @@ const ( resourceAddresses = "addresses" netNameTag = "cloudemu:gcpNetName" subnetNameTag = "cloudemu:gcpSubnetName" + subnetNetworkTag = "cloudemu:gcpSubnetNet" + autoSubnetTag = "cloudemu:gcpAutoSubnet" firewallNameTag = "cloudemu:gcpFwName" + firewallSpecTag = "cloudemu:gcpFwSpec" + trueValue = "true" ) // Handler serves the GCP networking REST surface. @@ -188,14 +194,26 @@ func (h *Handler) insertNetwork(w http.ResponseWriter, r *http.Request, rp gcpre return } + if _, err := findNetByName(r.Context(), h.net, req.Name); err == nil { + gcprest.WriteError(w, http.StatusConflict, "alreadyExists", + "network "+req.Name+" already exists") + + return + } + cidr := "10.0.0.0/16" if req.IPv4Range != "" { cidr = req.IPv4Range } + tags := map[string]string{netNameTag: req.Name} + if req.AutoCreateSubnetworks != nil && *req.AutoCreateSubnetworks { + tags[autoSubnetTag] = trueValue + } + cfg := netdriver.VPCConfig{ CIDRBlock: cidr, - Tags: map[string]string{netNameTag: req.Name}, + Tags: tags, } if _, err := h.net.CreateVPC(r.Context(), cfg); err != nil { @@ -294,7 +312,7 @@ func (h *Handler) insertSubnetwork(w http.ResponseWriter, r *http.Request, rp gc VPCID: vpcID, CIDRBlock: req.IPCIDRRange, AvailabilityZone: rp.ScopeName, - Tags: map[string]string{subnetNameTag: req.Name}, + Tags: map[string]string{subnetNameTag: req.Name, subnetNetworkTag: lastSegment(req.Network)}, } if _, err := h.net.CreateSubnet(r.Context(), cfg); err != nil { @@ -401,11 +419,20 @@ func (h *Handler) insertFirewall(w http.ResponseWriter, r *http.Request, rp gcpr } } + // The driver's SecurityGroup model can't express GCP's firewall shape + // (allowed/denied/direction/priority/targetTags), so persist the rule spec + // verbatim in a reserved tag and reconstruct it on read. Without this a + // created firewall reads back with no rules. + tags := map[string]string{firewallNameTag: req.Name} + if spec := marshalFirewallSpec(&req); spec != "" { + tags[firewallSpecTag] = spec + } + cfg := netdriver.SecurityGroupConfig{ Name: req.Name, Description: req.Description, VPCID: vpcID, - Tags: map[string]string{firewallNameTag: req.Name}, + Tags: tags, } if _, err := h.net.CreateSecurityGroup(r.Context(), cfg); err != nil { @@ -564,11 +591,21 @@ func toNetworkResponse(info *netdriver.VPCInfo, rp gcprest.ResourcePath, host st ID: numericID(info.ID), Name: name, IPv4Range: info.CIDRBlock, - AutoCreateSubnetworks: false, + AutoCreateSubnetworks: info.Tags[autoSubnetTag] == trueValue, SelfLink: gcprest.SelfLink(host, rp.Project, gcprest.ScopeGlobal, "", "networks", name), } } +// lastSegment returns the final path/URL segment (e.g. a network self-link or +// partial ref reduced to its bare name). +func lastSegment(ref string) string { + if i := strings.LastIndex(ref, "/"); i >= 0 { + return ref[i+1:] + } + + return ref +} + //nolint:gocritic // rp is a request-scoped value func toSubnetworkResponse(info *netdriver.SubnetInfo, rp gcprest.ResourcePath, host string) subnetworkResponse { name := tagOr(info.Tags, subnetNameTag, info.ID) @@ -578,7 +615,7 @@ func toSubnetworkResponse(info *netdriver.SubnetInfo, rp gcprest.ResourcePath, h region = info.AvailabilityZone } - return subnetworkResponse{ + resp := subnetworkResponse{ Kind: "compute#subnetwork", ID: numericID(info.ID), Name: name, @@ -586,19 +623,83 @@ func toSubnetworkResponse(info *netdriver.SubnetInfo, rp gcprest.ResourcePath, h Region: host + "/compute/v1/projects/" + rp.Project + "/regions/" + region, SelfLink: gcprest.SelfLink(host, rp.Project, gcprest.ScopeRegions, region, "subnetworks", name), } + + // Echo the parent network self-link so clients can discover a subnet's + // network (real GCP always returns it). + if net := info.Tags[subnetNetworkTag]; net != "" { + resp.Network = gcprest.SelfLink(host, rp.Project, gcprest.ScopeGlobal, "", "networks", net) + } + + return resp } //nolint:gocritic // rp is a request-scoped value func toFirewallResponse(info *netdriver.SecurityGroupInfo, rp gcprest.ResourcePath, host string) firewallResponse { name := tagOr(info.Tags, firewallNameTag, info.ID) - return firewallResponse{ + resp := firewallResponse{ Kind: "compute#firewall", ID: numericID(info.ID), Name: name, Description: info.Description, SelfLink: gcprest.SelfLink(host, rp.Project, gcprest.ScopeGlobal, "", "firewalls", name), } + + if spec, ok := unmarshalFirewallSpec(info.Tags[firewallSpecTag]); ok { + resp.Network = spec.Network + resp.Priority = spec.Priority + resp.Direction = spec.Direction + resp.Allowed = spec.Allowed + resp.Denied = spec.Denied + resp.SourceRanges = spec.SourceRanges + resp.TargetTags = spec.TargetTags + } + + return resp +} + +// firewallSpec is the GCP firewall rule shape persisted verbatim (as JSON in a +// reserved tag) because the driver's SecurityGroup model can't express it. +type firewallSpec struct { + Network string `json:"network,omitempty"` + Priority int `json:"priority,omitempty"` + Direction string `json:"direction,omitempty"` + Allowed []firewallRule `json:"allowed,omitempty"` + Denied []firewallRule `json:"denied,omitempty"` + SourceRanges []string `json:"sourceRanges,omitempty"` + TargetTags []string `json:"targetTags,omitempty"` +} + +func marshalFirewallSpec(req *firewallRequest) string { + spec := firewallSpec{ + Network: req.Network, + Priority: req.Priority, + Direction: req.Direction, + Allowed: req.Allowed, + Denied: req.Denied, + SourceRanges: req.SourceRanges, + TargetTags: req.TargetTags, + } + + b, err := json.Marshal(spec) + if err != nil { + return "" + } + + return string(b) +} + +func unmarshalFirewallSpec(s string) (firewallSpec, bool) { + if s == "" { + return firewallSpec{}, false + } + + var spec firewallSpec + if err := json.Unmarshal([]byte(s), &spec); err != nil { + return firewallSpec{}, false + } + + return spec, true } func tagOr(m map[string]string, key, fallback string) string { diff --git a/server/gcp/networks/networks_test.go b/server/gcp/networks/networks_test.go index c3dbd852..0172c2d5 100644 --- a/server/gcp/networks/networks_test.go +++ b/server/gcp/networks/networks_test.go @@ -19,6 +19,7 @@ const ( ) func ptrStr(s string) *string { return &s } +func ptrInt32(i int32) *int32 { return &i } func newGCPNetServer(t *testing.T) *httptest.Server { t.Helper() @@ -126,8 +127,12 @@ func TestSDKFirewallRoundTrip(t *testing.T) { Name: ptrStr("fw-1"), Allowed: []*computepb.Allowed{{ IPProtocol: ptrStr("tcp"), - Ports: []string{"80"}, + Ports: []string{"80", "443"}, }}, + SourceRanges: []string{"10.0.0.0/8"}, + Direction: ptrStr("INGRESS"), + Priority: ptrInt32(900), + TargetTags: []string{"web"}, }, }) if err != nil { @@ -149,6 +154,24 @@ func TestSDKFirewallRoundTrip(t *testing.T) { t.Errorf("name=%s want fw-1", got.GetName()) } + // #321: firewall rules must round-trip, not read back empty. + allowed := got.GetAllowed() + if len(allowed) != 1 || allowed[0].GetIPProtocol() != "tcp" || len(allowed[0].GetPorts()) != 2 { + t.Fatalf("allowed did not round-trip: %+v", allowed) + } + + if len(got.GetSourceRanges()) != 1 || got.GetSourceRanges()[0] != "10.0.0.0/8" { + t.Errorf("sourceRanges=%v", got.GetSourceRanges()) + } + + if got.GetDirection() != "INGRESS" || got.GetPriority() != 900 { + t.Errorf("direction=%s priority=%d", got.GetDirection(), got.GetPriority()) + } + + if len(got.GetTargetTags()) != 1 || got.GetTargetTags()[0] != "web" { + t.Errorf("targetTags=%v", got.GetTargetTags()) + } + delOp, err := client.Delete(ctx, &computepb.DeleteFirewallRequest{ Project: testProject, Firewall: "fw-1", }) diff --git a/server/gcp/pubsub/handler.go b/server/gcp/pubsub/handler.go index 47c09f79..89dca9be 100644 --- a/server/gcp/pubsub/handler.go +++ b/server/gcp/pubsub/handler.go @@ -26,9 +26,14 @@ package pubsub import ( "encoding/base64" "encoding/json" + "errors" "fmt" + "io" "net/http" + "sort" "strings" + "sync" + "time" cerrors "github.com/stackshy/cloudemu/v2/errors" mqdriver "github.com/stackshy/cloudemu/v2/services/messagequeue/driver" @@ -49,13 +54,31 @@ const ( ) // Handler serves Pub/Sub v1 REST requests against a messagequeue driver. +// +// The portable messagequeue driver has one queue per topic and no separate +// subscription concept, so subscription identity + metadata (its topic, +// ackDeadline, labels) is tracked here. Messages still live in the topic's +// queue; a subscription resolves to that queue for pull/ack. This lets a +// subscription carry a name distinct from its topic. (Multiple subscriptions +// on one topic share the single underlying queue rather than each getting an +// independent copy — a documented emulator simplification.) type Handler struct { mq mqdriver.MessageQueue + + mu sync.RWMutex + subs map[string]*subMeta // keyed by subscription short-name +} + +// subMeta is the per-subscription metadata the driver can't hold. +type subMeta struct { + topic string // topic short-name whose queue backs this subscription + ackDeadline int + labels map[string]string } // New returns a Pub/Sub handler backed by mq. func New(mq mqdriver.MessageQueue) *Handler { - return &Handler{mq: mq} + return &Handler{mq: mq, subs: make(map[string]*subMeta)} } // Matches accepts /v1/projects/{p}/topics[...] and /v1/projects/{p}/subscriptions[...]. @@ -150,13 +173,31 @@ func (h *Handler) serveCollection(w http.ResponseWriter, r *http.Request, projec writeJSON(w, http.StatusOK, out) case resSubscriptions: - out := listSubscriptionsResponse{Subscriptions: make([]subscription, 0, len(queues))} - for i := range queues { + // List from the subscription registry (not one phantom sub per queue), + // so distinct sub/topic names and their ackDeadline/labels round-trip. + // Emit in sorted name order: Go map iteration is randomized, and the + // repo's list endpoints are deterministic. + h.mu.RLock() + subNames := make([]string, 0, len(h.subs)) + + for subName := range h.subs { + subNames = append(subNames, subName) + } + + sort.Strings(subNames) + + out := listSubscriptionsResponse{Subscriptions: make([]subscription, 0, len(subNames))} + + for _, subName := range subNames { + meta := h.subs[subName] out.Subscriptions = append(out.Subscriptions, subscription{ - Name: subscriptionName(project, queues[i].Name), - Topic: topicName(project, queues[i].Name), + Name: subscriptionName(project, subName), + Topic: topicName(project, meta.topic), + AckDeadlineSeconds: meta.ackDeadline, + Labels: meta.labels, }) } + h.mu.RUnlock() writeJSON(w, http.StatusOK, out) default: @@ -185,8 +226,13 @@ func (h *Handler) serveTopic(w http.ResponseWriter, r *http.Request, project, na } func (h *Handler) createTopic(w http.ResponseWriter, r *http.Request, project, name string) { + // Real Pub/Sub createTopic accepts an empty body, so tolerate EOF/empty + // rather than 400ing on a bodyless request. var body topic - _ = decodeJSON(w, r, &body) // topic body is mostly empty for create; tolerate it + if err := json.NewDecoder(r.Body).Decode(&body); err != nil && !errors.Is(err, io.EOF) { + writeError(w, http.StatusBadRequest, "INVALID_ARGUMENT", "invalid JSON: "+err.Error()) + return + } info, err := h.mq.CreateQueue(r.Context(), mqdriver.QueueConfig{Name: name, Tags: body.Labels}) if err != nil { @@ -301,56 +347,86 @@ func (h *Handler) createSubscription(w http.ResponseWriter, r *http.Request, pro return } - // The driver pairs topic+subscription under a single queue; require the - // subscription name to match a known queue (which represents the topic). - if _, err := h.findQueueByName(r, name); err != nil { - // Auto-create from the topic field if present and matches the sub name. - if subToTopicName(body.Topic) != name { - writeErr(w, err) - return - } - - if _, cerr := h.mq.CreateQueue(r.Context(), mqdriver.QueueConfig{Name: name}); cerr != nil && - !cerrors.IsAlreadyExists(cerr) { - writeErr(w, cerr) - return - } + // The topic (a driver queue) must exist. Its short name may differ from + // the subscription name; default to the subscription name only when the + // caller omitted the topic (tolerant legacy path). + topicShort := subToTopicName(body.Topic) + if topicShort == "" { + topicShort = name } - resp := subscription{ - Name: subscriptionName(project, name), - Topic: topicName(project, name), - AckDeadlineSeconds: body.AckDeadlineSeconds, - Labels: body.Labels, + if _, err := h.findQueueByName(r, topicShort); err != nil { + writeErr(w, err) + return } - if resp.AckDeadlineSeconds == 0 { - resp.AckDeadlineSeconds = 10 + + ackDeadline := body.AckDeadlineSeconds + if ackDeadline == 0 { + ackDeadline = 10 } - writeJSON(w, http.StatusOK, resp) + h.mu.Lock() + h.subs[name] = &subMeta{topic: topicShort, ackDeadline: ackDeadline, labels: body.Labels} + h.mu.Unlock() + + writeJSON(w, http.StatusOK, subscription{ + Name: subscriptionName(project, name), + Topic: topicName(project, topicShort), + AckDeadlineSeconds: ackDeadline, + Labels: body.Labels, + }) } -func (h *Handler) getSubscription(w http.ResponseWriter, r *http.Request, project, name string) { - q, err := h.findQueueByName(r, name) - if err != nil { - writeErr(w, err) +func (h *Handler) getSubscription(w http.ResponseWriter, _ *http.Request, project, name string) { + h.mu.RLock() + meta, ok := h.subs[name] + h.mu.RUnlock() + + if !ok { + writeError(w, http.StatusNotFound, "NOT_FOUND", "subscription "+name+" not found") return } writeJSON(w, http.StatusOK, subscription{ - Name: subscriptionName(project, q.Name), - Topic: topicName(project, q.Name), - AckDeadlineSeconds: 10, + Name: subscriptionName(project, name), + Topic: topicName(project, meta.topic), + AckDeadlineSeconds: meta.ackDeadline, + Labels: meta.labels, }) } -func (*Handler) deleteSubscription(w http.ResponseWriter, _ *http.Request, _ string) { - // In the driver, deleting the subscription would orphan the topic. Treat - // it as a no-op: real Pub/Sub has no operation that's both safe and useful - // here without modeling subscriptions separately. +func (h *Handler) deleteSubscription(w http.ResponseWriter, _ *http.Request, name string) { + h.mu.Lock() + _, ok := h.subs[name] + delete(h.subs, name) + h.mu.Unlock() + + if !ok { + writeError(w, http.StatusNotFound, "NOT_FOUND", "subscription "+name+" not found") + return + } + + // The subscription is removed from the registry; the topic's queue is left + // intact (real Pub/Sub deletes the subscription, not the topic). writeJSON(w, http.StatusOK, map[string]any{}) } +// subscriptionQueue resolves a subscription to the queue that backs it (its +// topic's queue). Falls back to a same-named queue for subscriptions created +// before registration (legacy tolerance). +func (h *Handler) subscriptionQueue(r *http.Request, name string) (*mqdriver.QueueInfo, error) { + h.mu.RLock() + meta, ok := h.subs[name] + h.mu.RUnlock() + + target := name + if ok { + target = meta.topic + } + + return h.findQueueByName(r, target) +} + func (h *Handler) pull(w http.ResponseWriter, r *http.Request, name string) { if r.Method != http.MethodPost { writeError(w, http.StatusMethodNotAllowed, "METHOD_NOT_ALLOWED", "method not allowed") @@ -362,7 +438,7 @@ func (h *Handler) pull(w http.ResponseWriter, r *http.Request, name string) { return } - q, err := h.findQueueByName(r, name) + q, err := h.subscriptionQueue(r, name) if err != nil { writeErr(w, err) return @@ -381,14 +457,20 @@ func (h *Handler) pull(w http.ResponseWriter, r *http.Request, name string) { return } + // Real Pub/Sub always stamps a publishTime; the driver doesn't retain one, + // so approximate with the delivery time (clients that require a non-empty, + // valid RFC3339 timestamp are satisfied). + publishTime := time.Now().UTC().Format(time.RFC3339) + out := pullResponse{ReceivedMessages: make([]receivedMessage, 0, len(msgs))} for i := range msgs { out.ReceivedMessages = append(out.ReceivedMessages, receivedMessage{ AckID: msgs[i].ReceiptHandle, Message: pubsubMessage{ - MessageID: msgs[i].MessageID, - Data: base64.StdEncoding.EncodeToString([]byte(msgs[i].Body)), - Attributes: msgs[i].Attributes, + MessageID: msgs[i].MessageID, + Data: base64.StdEncoding.EncodeToString([]byte(msgs[i].Body)), + Attributes: msgs[i].Attributes, + PublishTime: publishTime, }, }) } @@ -407,7 +489,7 @@ func (h *Handler) acknowledge(w http.ResponseWriter, r *http.Request, name strin return } - q, err := h.findQueueByName(r, name) + q, err := h.subscriptionQueue(r, name) if err != nil { writeErr(w, err) return diff --git a/server/gcp/pubsub/sdk_roundtrip_test.go b/server/gcp/pubsub/sdk_roundtrip_test.go index fab4a9b9..1f503dea 100644 --- a/server/gcp/pubsub/sdk_roundtrip_test.go +++ b/server/gcp/pubsub/sdk_roundtrip_test.go @@ -109,6 +109,84 @@ func TestSDKPubSubPublishPullAck(t *testing.T) { } } +// TestSDKPubSubSubscriptionMetadata guards the #321 fixes: a subscription may +// have a name distinct from its topic, and its ackDeadline + labels must +// round-trip on Get (not be hardcoded). Delete must also be effective. +func TestSDKPubSubSubscriptionMetadata(t *testing.T) { + svc := newSDKService(t) + ctx := context.Background() + + if _, err := svc.Projects.Topics.Create("projects/demo/topics/events", + &pubsubv1.Topic{}).Context(ctx).Do(); err != nil { + t.Fatalf("Topic.Create: %v", err) + } + + // Distinct subscription name (not "events"). + if _, err := svc.Projects.Subscriptions.Create("projects/demo/subscriptions/billing-sub", + &pubsubv1.Subscription{ + Topic: "projects/demo/topics/events", + AckDeadlineSeconds: 45, + Labels: map[string]string{"team": "fin"}, + }).Context(ctx).Do(); err != nil { + t.Fatalf("Subscription.Create (distinct name): %v", err) + } + + got, err := svc.Projects.Subscriptions.Get("projects/demo/subscriptions/billing-sub").Context(ctx).Do() + if err != nil { + t.Fatalf("Subscription.Get: %v", err) + } + + if !strings.HasSuffix(got.Topic, "/topics/events") { + t.Errorf("topic=%q want .../topics/events", got.Topic) + } + + if got.AckDeadlineSeconds != 45 { + t.Errorf("ackDeadlineSeconds=%d want 45", got.AckDeadlineSeconds) + } + + if got.Labels["team"] != "fin" { + t.Errorf("labels=%v want team=fin", got.Labels) + } + + // Second subscription so List order is observable. Created after billing-sub + // but sorts before it — proving List sorts by name rather than echoing + // insertion or map-iteration order. + if _, err := svc.Projects.Subscriptions.Create("projects/demo/subscriptions/analytics-sub", + &pubsubv1.Subscription{Topic: "projects/demo/topics/events"}).Context(ctx).Do(); err != nil { + t.Fatalf("Subscription.Create (second): %v", err) + } + + // List must return the real subscriptions (distinct names + metadata), not a + // phantom one named after the topic queue, in deterministic sorted order. + list, err := svc.Projects.Subscriptions.List("projects/demo").Context(ctx).Do() + if err != nil { + t.Fatalf("Subscriptions.List: %v", err) + } + + if len(list.Subscriptions) != 2 { + t.Fatalf("List returned %d subs, want 2: %+v", len(list.Subscriptions), list.Subscriptions) + } + + if !strings.HasSuffix(list.Subscriptions[0].Name, "/subscriptions/analytics-sub") || + !strings.HasSuffix(list.Subscriptions[1].Name, "/subscriptions/billing-sub") { + t.Fatalf("List not sorted by name: [%q, %q]", + list.Subscriptions[0].Name, list.Subscriptions[1].Name) + } + + ls := list.Subscriptions[1] // billing-sub carries the metadata under test + if !strings.HasSuffix(ls.Topic, "/topics/events") || ls.AckDeadlineSeconds != 45 { + t.Errorf("List sub metadata wrong: topic=%q ackDeadline=%d", ls.Topic, ls.AckDeadlineSeconds) + } + + if _, err := svc.Projects.Subscriptions.Delete("projects/demo/subscriptions/billing-sub").Context(ctx).Do(); err != nil { + t.Fatalf("Subscription.Delete: %v", err) + } + + if _, err := svc.Projects.Subscriptions.Get("projects/demo/subscriptions/billing-sub").Context(ctx).Do(); err == nil { + t.Fatal("Get after Delete returned nil error, want NotFound") + } +} + func TestSDKPubSubPublishToMissingTopic(t *testing.T) { svc := newSDKService(t) diff --git a/server/gcp/pubsub/types.go b/server/gcp/pubsub/types.go index 844911f9..00160efd 100644 --- a/server/gcp/pubsub/types.go +++ b/server/gcp/pubsub/types.go @@ -32,6 +32,7 @@ type pubsubMessage struct { Attributes map[string]string `json:"attributes,omitempty"` OrderingKey string `json:"orderingKey,omitempty"` MessageID string `json:"messageId,omitempty"` + PublishTime string `json:"publishTime,omitempty"` } type publishResponse struct { diff --git a/server/gcp/secretmanager/sdk_roundtrip_test.go b/server/gcp/secretmanager/sdk_roundtrip_test.go index 3cfac489..f49560b8 100644 --- a/server/gcp/secretmanager/sdk_roundtrip_test.go +++ b/server/gcp/secretmanager/sdk_roundtrip_test.go @@ -153,10 +153,10 @@ func TestSDKSecretManagerVersionsAndAccess(t *testing.T) { t.Fatalf("Versions.List: %v", err) } - // The driver seeds an initial version on create, so two AddVersion calls - // yield three versions. - if len(versions.Versions) != 3 { - t.Fatalf("got %d versions, want 3", len(versions.Versions)) + // GCP secrets.create makes an empty container (no seeded version), so two + // AddVersion calls yield exactly two versions — matching real Secret Manager. + if len(versions.Versions) != 2 { + t.Fatalf("got %d versions, want 2", len(versions.Versions)) } } diff --git a/server/gcp/vertexai/endpoints.go b/server/gcp/vertexai/endpoints.go index 97e81700..36b5a2a2 100644 --- a/server/gcp/vertexai/endpoints.go +++ b/server/gcp/vertexai/endpoints.go @@ -74,8 +74,10 @@ func (h *Handler) endpointAction(w http.ResponseWriter, r *http.Request, p *vPat h.deployModel(w, r, p.name) case "undeployModel": h.undeployModel(w, r, p.name) - case actionGenerateContent, actionStreamGenerateContent: - h.endpointGenerateContent(w, r, p.name) + case actionGenerateContent, actionStreamGenerateContent, actionCountTokens: + // Route the real action through so endpoint countTokens works and stream + // requests aren't collapsed to non-streaming. + h.runGenAI(w, r, p.name, p.action) default: writeError(w, http.StatusNotFound, "notFound", "unknown endpoint action: "+p.action) } diff --git a/server/gcp/vertexai/genai.go b/server/gcp/vertexai/genai.go index f9f6a95e..9f092738 100644 --- a/server/gcp/vertexai/genai.go +++ b/server/gcp/vertexai/genai.go @@ -106,10 +106,6 @@ func (h *Handler) servePublishers(w http.ResponseWriter, r *http.Request) { h.runGenAI(w, r, model, action) } -func (h *Handler) endpointGenerateContent(w http.ResponseWriter, r *http.Request, endpoint string) { - h.runGenAI(w, r, endpoint, "generateContent") -} - // runGenAI dispatches generateContent / countTokens for either a publisher // model path or an endpoint resource name. func (h *Handler) runGenAI(w http.ResponseWriter, r *http.Request, model, action string) { @@ -119,7 +115,7 @@ func (h *Handler) runGenAI(w http.ResponseWriter, r *http.Request, model, action } switch action { - case "generateContent": + case actionGenerateContent: resp, err := h.svc.GenerateContent(r.Context(), model, toDriverRequest(req)) if err != nil { writeCErr(w, err) @@ -140,7 +136,7 @@ func (h *Handler) runGenAI(w http.ResponseWriter, r *http.Request, model, action // single-element array so SDK stream decoders iterate it correctly // (a lone object fails array-decoding / yields zero chunks). writeJSON(w, []map[string]any{generateResponseJSON(resp)}) - case "countTokens": + case actionCountTokens: resp, err := h.svc.CountTokens(r.Context(), model, toDriverRequest(req)) if err != nil { writeCErr(w, err) diff --git a/server/gcp/vertexai/handler.go b/server/gcp/vertexai/handler.go index 439b7f9e..67e73607 100644 --- a/server/gcp/vertexai/handler.go +++ b/server/gcp/vertexai/handler.go @@ -40,6 +40,7 @@ const ( actionCancel = "cancel" actionGenerateContent = "generateContent" actionStreamGenerateContent = "streamGenerateContent" + actionCountTokens = "countTokens" ) // vertexCollections are the resource collections this handler serves. Listed