Skip to content

Commit 48c4c0e

Browse files
ToreMerkelyclaude
andcommitted
fix(cloudrun): align snapshot payload with server's snapshot-examples doc (#4986)
Three changes brings the payload into line with the conventions documented in the server repo's out-snapshot-examples.txt: - Added the top-level "type": "cloud-run" literal that every other env-type report ships explicitly. Without it, the (still-to-come) CloudRunReport model on the server would only accept the URL default; with it, the request is unambiguous. - Renamed the per-artifact "service_name" to "serviceName" (camelCase) matching the doc's K8S/ECS examples. The existing CLI's ECS code uses snake_case, but a new type is better off following the doc. - Dropped the per-artifact "project" and "region" fields. The doc notes extra="forbid" on every Pydantic model, so unilaterally picking custom field names risks 422 once the server defines its CloudRunReport. Project + region are already in the URL and flags, mirroring how ECS reports don't carry account/region per artifact. Verified end-to-end against the hello-world-cli-demo project; payload now matches the doc shape exactly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent be73c6c commit 48c4c0e

5 files changed

Lines changed: 40 additions & 47 deletions

File tree

cmd/kosli/snapshotCloudRun.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ func (o *snapshotCloudRunOptions) run(args []string) error {
106106
}
107107
}
108108

109-
payload := cloudrun.ToEnvRequest(filtered, o.project, o.region)
109+
payload := cloudrun.ToEnvRequest(filtered)
110110

111111
reqParams := &requests.RequestParams{
112112
Method: http.MethodPut,

cmd/kosli/snapshotCloudRun_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ func (suite *SnapshotCloudRunTestSuite) TestSnapshotCloudRunCmd() {
106106
{
107107
name: "snapshot cloud-run dry-runs the report URL and payload built from the GCP client",
108108
cmd: fmt.Sprintf(`snapshot cloud-run %s --project proj-x --region europe-west1 %s`, suite.envName, suite.defaultKosliArguments),
109-
goldenRegex: `(?s)THIS IS A DRY-RUN.*report/cloud-run.*"service_name": "alpha".*"service_name": "beta"`,
109+
goldenRegex: `(?s)THIS IS A DRY-RUN.*report/cloud-run.*"type": "cloud-run".*"serviceName": "alpha".*"serviceName": "beta"`,
110110
},
111111
{
112112
wantError: true,
@@ -150,26 +150,26 @@ func (suite *SnapshotCloudRunTestSuite) runFilteredCmd(filterArgs string) string
150150

151151
func (suite *SnapshotCloudRunTestSuite) TestSnapshotCloudRunFilter_Services() {
152152
out := suite.runFilteredCmd("--services alpha")
153-
require.Contains(suite.T(), out, `"service_name": "alpha"`)
154-
require.NotContains(suite.T(), out, `"service_name": "beta"`)
153+
require.Contains(suite.T(), out, `"serviceName": "alpha"`)
154+
require.NotContains(suite.T(), out, `"serviceName": "beta"`)
155155
}
156156

157157
func (suite *SnapshotCloudRunTestSuite) TestSnapshotCloudRunFilter_ServicesRegex() {
158158
out := suite.runFilteredCmd(`--services-regex "^al"`)
159-
require.Contains(suite.T(), out, `"service_name": "alpha"`)
160-
require.NotContains(suite.T(), out, `"service_name": "beta"`)
159+
require.Contains(suite.T(), out, `"serviceName": "alpha"`)
160+
require.NotContains(suite.T(), out, `"serviceName": "beta"`)
161161
}
162162

163163
func (suite *SnapshotCloudRunTestSuite) TestSnapshotCloudRunFilter_Exclude() {
164164
out := suite.runFilteredCmd("--exclude alpha")
165-
require.NotContains(suite.T(), out, `"service_name": "alpha"`)
166-
require.Contains(suite.T(), out, `"service_name": "beta"`)
165+
require.NotContains(suite.T(), out, `"serviceName": "alpha"`)
166+
require.Contains(suite.T(), out, `"serviceName": "beta"`)
167167
}
168168

169169
func (suite *SnapshotCloudRunTestSuite) TestSnapshotCloudRunFilter_ExcludeRegex() {
170170
out := suite.runFilteredCmd(`--exclude-regex "^al"`)
171-
require.NotContains(suite.T(), out, `"service_name": "alpha"`)
172-
require.Contains(suite.T(), out, `"service_name": "beta"`)
171+
require.NotContains(suite.T(), out, `"serviceName": "alpha"`)
172+
require.Contains(suite.T(), out, `"serviceName": "beta"`)
173173
}
174174

175175
// TestSnapshotCloudRunCmd_UnauthenticatedReturnsFriendlyError verifies that a

docs/handover/2026-04-28-4986-google-cloud-run-1.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ GCP test environment (provisioned, ADC already configured on this machine):
3434
- Package named `internal/cloudrun` (not `gcprun`) to mirror the user-facing command name `snapshot cloud-run`. If GCP integrations expand later we'll either rename or split into `internal/gcp/run`.
3535
- `internal/cloudrun` reports all revisions referenced in `service.traffic[]` regardless of percent (including 0%). Trade-off: matches the user's framing of "running or could run" without dragging in retired revisions that aren't currently configured for traffic; canary 90/10 splits surface both revisions naturally.
3636
- Digest extraction follows the ECS pattern (`internal/aws/aws.go:670-693`): use a `@sha256:` substring if present, else leave the digest empty rather than calling Artifact Registry. Registry-lookup mode (analogous to Azure's `--digests-source acr`) is deferred until customers ask for it.
37-
- Wire payload mirrors `EcsEnvRequest` plus `project` and `region` fields per artifact (so each row is self-describing). Endpoint name is `report/cloud-run` (kebab-case, parallels `report/azure-apps`). Server-side endpoint does not yet exist; the forced dry-run means no network call is made.
37+
- Wire payload follows the server's `out-snapshot-examples.txt` reference: top-level `{"type": "cloud-run", "artifacts": [...]}` with camelCase per-artifact fields (`revisionName`, `serviceName`, `digests`, `creationTimestamp`). Endpoint is `report/cloud-run` (kebab-case, parallels `report/azure-apps`). Server-side endpoint does not yet exist; the forced dry-run means no network call is made. Initial design (Slice 3) added `project`/`region` per artifact mirroring ECS's `cluster_name`; reverted in Slice 3.5 because the doc specifies `extra="forbid"` on every Pydantic model and project/region are derivable from the URL + flags.
3838
- Command depends on a local `cloudRunLister` interface and a package-level `newCloudRunClient` variable so tests can substitute a stub without touching ADC. The seam stays in `cmd/kosli/snapshotCloudRun.go` rather than being exposed from `internal/cloudrun` — keeps the public package surface minimal.
3939
- Error classification (`Classify`) lives in `internal/cloudrun` (GCP knowledge belongs to the package) but is *applied* at the command layer, not inside `Client.ListServices`. Why: applying it inside `ListServices` would double-wrap real-call errors when the command also classified them, and bypass the friendly path entirely for stub-driven tests. Calling it once at the command boundary covers both real and stub error sources.
4040

@@ -46,7 +46,8 @@ Slice plan (each slice is a separate, independently-mergeable branch):
4646

4747
- [x] **Slice 1 (this branch):** Skeleton command — `cmd/kosli/snapshotCloudRun.go` (Hidden, forced dry-run, stub `RunE`), register in `snapshot.go`, arg/flag validation tests. Done 2026-04-28: 5 cmdTestCase tests passing, `make lint` clean, hidden from `snapshot --help` but reachable directly.
4848
- [x] **Slice 2:** Internal `internal/cloudrun` package — wraps `cloud.google.com/go/run/apiv2` to list services in project+region; unit-tested with a fake. Done 2026-04-28: `Client.ListServices` returns `Service{Name, URI, Revisions}` with one `Revision{Name, Digests, CreatedAt}` per traffic-configured revision (any percent including 0%, with `LATEST` resolved via `LatestReadyRevision` and dupes removed). Digest extraction mirrors the ECS fallback (`@sha256:` parse, else empty string). 9 unit tests passing.
49-
- [x] **Slice 3:** End-to-end happy path — wire the package into `RunE`, build the snapshot payload, POST to the server `cloud-run` endpoint (still dry-run only). Done 2026-04-28: command now calls `cloudrun.New` + `ListServices`, builds an `EnvRequest` via `ToEnvRequest(services, project, region)`, and submits PUT `report/cloud-run` via `kosliClient.Do` (dry-run forced, so no network call leaves the client). Tested against the real `hello-world-cli-demo` GCP project — emits a digest-pinned artifact for the running `hello-world` service.
49+
- [x] **Slice 3:** End-to-end happy path — wire the package into `RunE`, build the snapshot payload, POST to the server `cloud-run` endpoint (still dry-run only). Done 2026-04-28: command now calls `cloudrun.New` + `ListServices`, builds an `EnvRequest` via `ToEnvRequest(services)`, and submits PUT `report/cloud-run` via `kosliClient.Do` (dry-run forced, so no network call leaves the client). Tested against the real `hello-world-cli-demo` GCP project — emits a digest-pinned artifact for the running `hello-world` service.
50+
- [x] **Slice 3.5:** Align payload with the server's snapshot-examples doc. Done 2026-04-28: added top-level `"type": "cloud-run"`; renamed `service_name``serviceName` (camelCase, matching the doc's K8S/ECS examples); dropped per-artifact `project` and `region` (would be rejected by `extra="forbid"` once the server defines a `CloudRunReport` model — they're identifiable from the URL + flags anyway, like ECS region).
5051
- [x] **Slice 4:** Filtering flags — `--services`, `--services-regex`, `--exclude`, `--exclude-regex`. Done 2026-04-28: backed by `filters.ResourceFilterOptions` (same struct ECS uses); 4 mutex pairs validated in `PreRunE`. Filter is applied in the command after `cloudrun.ListServices` returns — services excluded by name still cost their revision-fetch round-trips. If that becomes a bottleneck, push the filter into `cloudrun.ListServices` so excluded services skip the per-revision API calls.
5152
- [x] **Slice 5:** ~~Multi-revision / traffic splitting — handle services with multiple active revisions and services with no active revisions.~~ Dropped 2026-04-28: multi-revision (traffic splitting), `LATEST` resolution, and dedup were all completed in Slice 2; the only remaining edge case (services with no active revisions emitting a placeholder artifact) was deferred until the server-side wire contract is defined, since picking the format unilaterally now risks rework. Re-open as a small slice once the server contract lands.
5253
- [x] **Slice 6:** Auth error UX — clear messages for ADC / `GOOGLE_APPLICATION_CREDENTIALS` failures and for missing project/region. Done 2026-04-28: `cloudrun.Classify(err, project, region)` maps gRPC `Unauthenticated` → ADC advice, `PermissionDenied``roles/run.viewer` advice, `NotFound` → "project or region not found"; other codes pass through. Auth message names all three credential sources (env var, `gcloud auth application-default login`, GCE/GKE metadata server / Workload Identity) since the production deployment is a GKE cron job. `cloudrun.New(ctx)` errors get a generic "GCP client setup failed" prefix.

internal/cloudrun/payload.go

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,41 @@
11
package cloudrun
22

3+
// reportType is the literal sent in the top-level "type" field, mirroring the
4+
// "K8S", "ECS", "azure-apps", … values used by the other env-type reports.
5+
const reportType = "cloud-run"
6+
37
// EnvRequest is the PUT body sent to the Kosli "report/cloud-run" endpoint.
4-
// It mirrors the shape of EcsEnvRequest in internal/aws.
8+
// Field naming mirrors the conventions documented in the server's
9+
// out-snapshot-examples.txt (top-level "type" + "artifacts", camelCase
10+
// per-artifact fields).
511
type EnvRequest struct {
12+
Type string `json:"type"`
613
Artifacts []*RevisionData `json:"artifacts"`
714
}
815

916
// RevisionData represents one Cloud Run revision in the snapshot payload.
1017
// One artifact is emitted per revision in each service's traffic configuration.
1118
type RevisionData struct {
1219
RevisionName string `json:"revisionName"`
13-
Service string `json:"service_name,omitempty"`
14-
Project string `json:"project,omitempty"`
15-
Region string `json:"region,omitempty"`
20+
ServiceName string `json:"serviceName,omitempty"`
1621
Digests map[string]string `json:"digests"`
1722
CreatedAt int64 `json:"creationTimestamp"`
1823
}
1924

2025
// ToEnvRequest flattens services into a list of revision artifacts. Services
2126
// with no revisions contribute nothing, mirroring the ECS behaviour of
2227
// services with no running tasks.
23-
func ToEnvRequest(services []Service, project, region string) *EnvRequest {
28+
func ToEnvRequest(services []Service) *EnvRequest {
2429
artifacts := []*RevisionData{}
2530
for _, svc := range services {
2631
for _, rev := range svc.Revisions {
2732
artifacts = append(artifacts, &RevisionData{
2833
RevisionName: rev.Name,
29-
Service: svc.Name,
30-
Project: project,
31-
Region: region,
34+
ServiceName: svc.Name,
3235
Digests: rev.Digests,
3336
CreatedAt: rev.CreatedAt.Unix(),
3437
})
3538
}
3639
}
37-
return &EnvRequest{Artifacts: artifacts}
40+
return &EnvRequest{Type: reportType, Artifacts: artifacts}
3841
}

internal/cloudrun/payload_test.go

Lines changed: 15 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,13 @@ import (
77
"github.com/stretchr/testify/require"
88
)
99

10+
func TestToEnvRequest_TypeIsCloudRun(t *testing.T) {
11+
got := ToEnvRequest(nil)
12+
require.Equal(t, "cloud-run", got.Type)
13+
}
14+
1015
func TestToEnvRequest_EmptyInput(t *testing.T) {
11-
got := ToEnvRequest(nil, "p", "r")
16+
got := ToEnvRequest(nil)
1217
require.NotNil(t, got)
1318
require.Empty(t, got.Artifacts)
1419
}
@@ -29,14 +34,12 @@ func TestToEnvRequest_SingleServiceSingleRevision(t *testing.T) {
2934
},
3035
}
3136

32-
got := ToEnvRequest(services, "proj-1", "europe-west1")
37+
got := ToEnvRequest(services)
3338
require.Len(t, got.Artifacts, 1)
3439

3540
art := got.Artifacts[0]
3641
require.Equal(t, "svc-a-rev1", art.RevisionName)
37-
require.Equal(t, "svc-a", art.Service)
38-
require.Equal(t, "proj-1", art.Project)
39-
require.Equal(t, "europe-west1", art.Region)
42+
require.Equal(t, "svc-a", art.ServiceName)
4043
require.Equal(t, map[string]string{"img@sha256:aaa": "aaa"}, art.Digests)
4144
require.Equal(t, created.Unix(), art.CreatedAt)
4245
}
@@ -58,15 +61,15 @@ func TestToEnvRequest_MultipleServicesMultipleRevisions(t *testing.T) {
5861
},
5962
}
6063

61-
got := ToEnvRequest(services, "proj-1", "europe-west1")
64+
got := ToEnvRequest(services)
6265
require.Len(t, got.Artifacts, 3)
6366

6467
revisionNames := []string{got.Artifacts[0].RevisionName, got.Artifacts[1].RevisionName, got.Artifacts[2].RevisionName}
6568
require.Equal(t, []string{"a-rev1", "a-rev2", "b-rev1"}, revisionNames)
6669

67-
require.Equal(t, "svc-a", got.Artifacts[0].Service)
68-
require.Equal(t, "svc-a", got.Artifacts[1].Service)
69-
require.Equal(t, "svc-b", got.Artifacts[2].Service)
70+
require.Equal(t, "svc-a", got.Artifacts[0].ServiceName)
71+
require.Equal(t, "svc-a", got.Artifacts[1].ServiceName)
72+
require.Equal(t, "svc-b", got.Artifacts[2].ServiceName)
7073
}
7174

7275
func TestToEnvRequest_ServiceWithNoRevisionsContributesNothing(t *testing.T) {
@@ -80,31 +83,17 @@ func TestToEnvRequest_ServiceWithNoRevisionsContributesNothing(t *testing.T) {
8083
},
8184
}
8285

83-
got := ToEnvRequest(services, "p", "r")
86+
got := ToEnvRequest(services)
8487
require.Len(t, got.Artifacts, 1)
85-
require.Equal(t, "svc-a", got.Artifacts[0].Service)
86-
}
87-
88-
func TestToEnvRequest_ProjectAndRegionOnEveryArtifact(t *testing.T) {
89-
services := []Service{
90-
{Name: "a", Revisions: []Revision{{Name: "a1"}}},
91-
{Name: "b", Revisions: []Revision{{Name: "b1"}}},
92-
}
93-
94-
got := ToEnvRequest(services, "proj-x", "us-central1")
95-
96-
for _, art := range got.Artifacts {
97-
require.Equal(t, "proj-x", art.Project)
98-
require.Equal(t, "us-central1", art.Region)
99-
}
88+
require.Equal(t, "svc-a", got.Artifacts[0].ServiceName)
10089
}
10190

10291
func TestToEnvRequest_ZeroCreatedAtSerialisesAsZero(t *testing.T) {
10392
services := []Service{
10493
{Name: "svc", Revisions: []Revision{{Name: "rev"}}},
10594
}
10695

107-
got := ToEnvRequest(services, "p", "r")
96+
got := ToEnvRequest(services)
10897
require.Len(t, got.Artifacts, 1)
10998
require.Equal(t, time.Time{}.Unix(), got.Artifacts[0].CreatedAt)
11099
}

0 commit comments

Comments
 (0)