Skip to content

fix(gcp): remove the project from the number call#11

Closed
guyoron1 wants to merge 8 commits into
mainfrom
fix/remove-quota-project
Closed

fix(gcp): remove the project from the number call#11
guyoron1 wants to merge 8 commits into
mainfrom
fix/remove-quota-project

Conversation

@guyoron1

Copy link
Copy Markdown
Owner
  • I found that doing this removed the need to enable cloudresourcemanager permissions on the GCP project.

Mirrored from upstream PR #2231

@guyoron1

Copy link
Copy Markdown
Owner Author

/fs quality

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 15, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:42 PM UTC · Completed 2:48 PM UTC
Commit: 4365b3d · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 15, 2026

Copy link
Copy Markdown

Review

Reason: stale-head

The review agent reviewed commit 69a77055df8ba9c049cc7cf7c8385612b50c2d17 but the PR HEAD is now 9f8e63a7eaedbba14fb03f5412012857fccaa42b. This review was discarded to avoid approving unreviewed code.

Previous run

Review

Reason: stale-head

The review agent reviewed commit ff42f77ff6d16bf9fc0dc282e4041d188b6eea43 but the PR HEAD is now 69a77055df8ba9c049cc7cf7c8385612b50c2d17. This review was discarded to avoid approving unreviewed code.

Previous run (2)

Review

Findings

Critical

  • [compilation error] internal/dispatch/gcf/gcp.go:893 — The diff sets noQuotaClient.QuotaProject = "" but gcp.Client (defined in internal/gcp/client.go) has no QuotaProject field. The struct contains only httpClient *http.Client and tokenFunc func(...). This code will not compile.
    Remediation: Either add a QuotaProject string field to gcp.Client in internal/gcp/client.go and update DoRequest to set the x-goog-user-project header when that field is non-empty, or take a different approach to omitting the quota project header (e.g., a DoRequestWithoutQuotaProject method or passing options to DoRequest).

  • [logic error / no-op mechanism] internal/dispatch/gcf/gcp.go:894 — Even if a QuotaProject field were added to gcp.Client, the DoRequest method (internal/gcp/client.go:70-91) never reads any such field and never sets the x-goog-user-project HTTP header. Clearing the field on the copy would have no effect on the outgoing request — the stated goal of avoiding quota-project attribution would not be achieved.
    Remediation: Modify DoRequest (or add a variant) to conditionally set the x-goog-user-project header based on the QuotaProject field value, so that clearing the field actually changes the wire-level request.

Low

  • [struct-copy-pattern] internal/dispatch/gcf/gcp.go:891 — The shallow copy pattern (noQuotaClient := *c.Client) is unconventional in this file. While safe in this specific case (the shared *http.Client is concurrency-safe and not mutated), consider adding explicit support in gcp.Client for omitting the quota project header rather than copying and mutating the struct.

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

// CRM is a global API — omit x-goog-user-project to avoid requiring
// the API to be enabled on the target project.
noQuotaClient := *c.Client
noQuotaClient.QuotaProject = ""

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[critical] compilation error

The diff sets noQuotaClient.QuotaProject but gcp.Client (defined in internal/gcp/client.go) has no QuotaProject field. The struct contains only httpClient *http.Client and tokenFunc. This code will not compile.

Suggested fix: Add a QuotaProject string field to gcp.Client and update DoRequest to set the x-goog-user-project header when non-empty, or take a different approach.

// the API to be enabled on the target project.
noQuotaClient := *c.Client
noQuotaClient.QuotaProject = ""
resp, err := noQuotaClient.DoRequest(ctx, http.MethodGet, reqURL, "")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[critical] logic error / no-op mechanism

Even if QuotaProject were added to gcp.Client, DoRequest never reads it or sets the x-goog-user-project HTTP header. Clearing the field would have no effect on the outgoing request.

Suggested fix: Modify DoRequest to conditionally set x-goog-user-project based on the QuotaProject field value.


resp, err := c.Client.DoRequest(ctx, http.MethodGet, reqURL, "")
// CRM is a global API — omit x-goog-user-project to avoid requiring
// the API to be enabled on the target project.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] struct-copy-pattern

The shallow copy pattern is unconventional in this file. Consider adding explicit support in gcp.Client for quota-project-free requests.

Suggested fix: Add a dedicated method or option to gcp.Client for quota-project-free requests.

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 15, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:50 PM UTC · Completed 2:59 PM UTC
Commit: 4365b3d · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 15, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:01 PM UTC · Completed 3:12 PM UTC
Commit: 4365b3d · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

/fs-review

QualityFlow and others added 3 commits June 15, 2026 15:16
Generated 7 Go e2e test functions from STD YAML for the
"Remove Quota Project from GCP Project Number Lookup" fix.

Test coverage: 7/7 STD scenarios (TS-GH-11-001 through TS-GH-11-007)
Framework: Go testing + testify
Build tag: e2e

Known issue: gcp.Client lacks QuotaProject field (compilation prerequisite).
Replaces intermediate pipeline artifacts with organized test files.

Total: 1 test files → qf-tests/GH-11/
Jira: GH-11
[skip ci]
@github-actions

Copy link
Copy Markdown

QualityFlow Pipeline Summary

Stage Agent Status
1 STP Builder
2 STP Reviewer
3 STP Refiner
4 STD Builder
5 STD Reviewer
6 STD Refiner
7 Test Generator

Test Output

Language Count Location
Go 1 files qf-tests/GH-11/go/

Issue: GH-11


Generated by QualityFlow

@guyoron1

Copy link
Copy Markdown
Owner Author

Closing — QF infrastructure updated on main, will re-create with improved pipeline.

@guyoron1 guyoron1 closed this Jun 16, 2026
@guyoron1 guyoron1 deleted the fix/remove-quota-project branch June 16, 2026 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant