Normalize project name by replacing underscore with dash #1724
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
📝 WalkthroughWalkthroughThis pull request adds project name normalization to sanitize underscores to hyphens, updates GCP Cloud Build tag parsing to support underscores in service names, introduces DNS zone name sanitization utilities with deterministic hashing, and threads normalized project names through CLI commands and BYOC query paths. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)level=warning msg="[linters_context] running gomodguard failed: unable to read module file go.mod: current working directory must have a go.mod file: if you are not using go modules it is suggested to disable this linter" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
lionello
reviewed
Dec 18, 2025
Member
lionello
left a comment
lionello
left a comment
There was a problem hiding this comment.
was expecting something generic, but this is GCP specific
Contributor
Author
The GCP specific parts are actually not related to the normalization, more of supporting it. The main changes are in the compose folder, which changes/normalizes the project name before sending it to deploy/tail/subscribe, which are provider agnostic. |
edwardrf
force-pushed
the
edw/gcp-build-tag-project-name-underscore
branch
from
6bbf18b to
ae7e827
Compare
Contributor
Author
|
Convert to ready for review for code rabbit to review. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pkg/cli/client/byoc/gcp/query.go (1)
158-164: AddCloudBuildLogQuery skips SafeLabelValue normalization for stack, project, etag, and services—inconsistent with all other query functions.AddCloudBuildLogQuery (lines 158–163) does not apply
gcp.SafeLabelValue()tostack,project,etag, orservices, unlikeAddJobLogQuery,AddServiceLogQuery, and other query functions that consistently apply it. This creates two problems:
If
stackorprojectparameters contain characters that SafeLabelValue would normalize (e.g., underscores → dashes), they will be used raw in the regex pattern while actual Cloud Build labels would have been normalized differently, causing query mismatches.Services are joined directly without normalization, while
servicesPattern()used elsewhere appliesSafeLabelValueto each service. This inconsistency means the build_tags regex may not match actual labels if services contain uppercase or special characters.Apply
SafeLabelValue()tostack,project, andetagparameters, and useservicesPattern(services)instead ofstrings.Join(services, "|")to maintain consistency with other query functions.
♻️ Duplicate comments (4)
src/pkg/cli/client/byoc/gcp/testdata/with_etag_and_since.query (1)
9-22: Same verification needed as in with_project.query.The build_tags pattern here also includes underscores in the character class
[a-zA-Z0-9-_]. Please verify this is consistent with project name normalization strategy.src/pkg/cli/client/byoc/gcp/testdata/with_etag_equal_cd_exec.query (1)
12-22: Same verification needed as in with_project.query.This testdata file has the same character class update allowing underscores.
src/pkg/cli/client/byoc/gcp/testdata/with_cd_exec.query (1)
12-22: Same verification needed as in with_project.query.Character class includes underscore in build_tags pattern.
src/pkg/cli/client/byoc/gcp/testdata/with_logtype_all.query (1)
9-19: Same verification needed as in with_project.query.Build_tags pattern includes underscore in character class.
🧹 Nitpick comments (4)
src/pkg/clouds/gcp/cloudbuild_test.go (1)
7-47: Consider adding negative test cases for Parse() error handling.The current tests cover the happy path well. Consider adding test cases for invalid inputs to verify error handling in
Parse():
- Empty string:
""- Fewer than 3 parts:
"proj_svc"or"single"- Empty parts:
"stack1__svc_123"🔎 Example additional test cases
func TestBuildTagParseErrors(t *testing.T) { tests := []struct { name string input string }{ {name: "empty string", input: ""}, {name: "single part", input: "single"}, {name: "two parts", input: "proj_svc"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { var bt BuildTag if err := bt.Parse(tt.input); err == nil { t.Errorf("Parse(%q) expected error, got nil", tt.input) } }) } }src/pkg/cli/client/byoc/gcp/byoc.go (2)
560-562: Stale TODO comment can be removed.The TODO on line 560 says "update stack (1st param) to b.PulumiStack" but the code on lines 561-562 already does this. Consider removing the outdated comment.
Proposed fix
- // TODO: update stack (1st param) to b.PulumiStack subscribeStream.AddJobStatusUpdate(b.PulumiStack, req.Project, req.Etag, req.Services) subscribeStream.AddServiceStatusUpdate(b.PulumiStack, req.Project, req.Etag, req.Services)
806-817: Remove stale TODO comments.Both TODO comments on lines 806 and 813 reference updating the stack parameter to
b.PulumiStack, but the code already does this.Proposed fix
- // Logs TODO: update stack (1st param) to b.PulumiStack query.AddJobLogQuery(b.PulumiStack, req.Project, req.Etag, req.Services) // Kaniko OR CD logs query.AddServiceLogQuery(b.PulumiStack, req.Project, req.Etag, req.Services) // Cloudrun service logs query.AddCloudBuildLogQuery(b.PulumiStack, req.Project, req.Etag, req.Services) // CloudBuild logs query.AddSince(since) query.AddUntil(until) - // Service status updates TODO: update stack (1st param) to b.PulumiStack + // Service status updates query.AddJobStatusUpdateRequestQuery(b.PulumiStack, req.Project, req.Etag, req.Services)src/pkg/clouds/gcp/dns.go (1)
107-109: Redundant condition afterstrings.Trim.After
strings.Trim(safe, "-")on line 106, the string cannot start with a hyphen (it can only be empty or start with an alphanumeric). Thesafe[0] == '-'check is dead code.Proposed fix
safe = strings.Trim(safe, "-") // Rule 3, 4: trim hyphens from start and end - if len(safe) == 0 || safe[0] == '-' { + if len(safe) == 0 { safe = "zone" + safe }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/pkg/cli/client/byoc/gcp/byoc.gosrc/pkg/cli/client/byoc/gcp/query.gosrc/pkg/cli/client/byoc/gcp/testdata/with_cd_exec.querysrc/pkg/cli/client/byoc/gcp/testdata/with_etag.querysrc/pkg/cli/client/byoc/gcp/testdata/with_etag_and_since.querysrc/pkg/cli/client/byoc/gcp/testdata/with_etag_equal_cd_exec.querysrc/pkg/cli/client/byoc/gcp/testdata/with_everything.querysrc/pkg/cli/client/byoc/gcp/testdata/with_logtype_all.querysrc/pkg/cli/client/byoc/gcp/testdata/with_logtype_build.querysrc/pkg/cli/client/byoc/gcp/testdata/with_logtype_run.querysrc/pkg/cli/client/byoc/gcp/testdata/with_project.querysrc/pkg/cli/client/byoc/gcp/testdata/with_project_since_and_until.querysrc/pkg/cli/compose/fixup.gosrc/pkg/cli/composeUp.gosrc/pkg/cli/subscribe.gosrc/pkg/cli/tail.gosrc/pkg/clouds/gcp/cloudbuild.gosrc/pkg/clouds/gcp/cloudbuild_test.gosrc/pkg/clouds/gcp/dns.gosrc/pkg/clouds/gcp/dns_test.go
🧰 Additional context used
🧬 Code graph analysis (7)
src/pkg/cli/subscribe.go (3)
src/protos/io/defang/v1/fabric.pb.go (3)
SubscribeRequest(4134-4141)SubscribeRequest(4154-4154)SubscribeRequest(4169-4171)src/pkg/cli/compose/loader.go (2)
Project(25-25)Services(29-29)src/pkg/cli/compose/fixup.go (1)
NormalizeProjectName(484-486)
src/pkg/cli/tail.go (1)
src/pkg/cli/compose/loader.go (1)
Project(25-25)
src/pkg/cli/compose/fixup.go (1)
src/pkg/term/colorizer.go (1)
Debugf(298-300)
src/pkg/clouds/gcp/dns_test.go (1)
src/pkg/clouds/gcp/dns.go (1)
SafeZoneName(103-114)
src/pkg/cli/composeUp.go (4)
src/pkg/cli/compose/loader.go (1)
Project(25-25)src/pkg/cli/client/provider.go (1)
PrepareDomainDelegationRequest(25-29)src/pkg/cli/preview.go (1)
Preview(11-19)src/pkg/cli/compose/context.go (2)
UploadModePreview(35-35)UploadModeEstimate(36-36)
src/pkg/cli/client/byoc/gcp/byoc.go (1)
src/pkg/clouds/gcp/dns.go (1)
SafeZoneName(103-114)
src/pkg/clouds/gcp/cloudbuild_test.go (1)
src/pkg/clouds/gcp/cloudbuild.go (1)
BuildTag(13-18)
🔇 Additional comments (20)
src/pkg/clouds/gcp/cloudbuild.go (2)
20-26: LGTM - String() formatting is correct.The
String()method correctly formats the tag with underscore delimiters and handles both cases (with/without Stack) for backward compatibility.
28-46: The parsing logic is correct; project name normalization is enforced by ProjectIDFromName().The invariant that project names contain no underscores is not implicit—it is actively enforced by
ProjectIDFromName()(src/pkg/clouds/gcp/project.go:49), which replaces underscores with hyphens before any project ID is used. This normalization aligns with GCP's own constraint that project IDs cannot contain underscores.The backward-compatibility concern about ambiguous 4-part tags with underscores in the project name cannot occur in practice, as both the code and GCP itself prevent underscores in project identifiers. Legacy tags would also have been created through the same normalized project IDs, so no parsing ambiguity exists.
The design is sound.
src/pkg/cli/subscribe.go (1)
8-8: Project name normalization applied correctly.The addition of
compose.NormalizeProjectNameensures that project names containing underscores are properly normalized before being passed to the Subscribe API, aligning with the PR objective.Also applies to: 33-33
src/pkg/cli/client/byoc/gcp/testdata/with_etag.query (1)
9-9: Test data updated to reflect query pattern changes.The updates to the test query correctly reflect:
- Addition of
defang-stack = "beta"label filtering across resource types- Expansion of the build_tags regex to allow underscores:
[a-zA-Z0-9-_]instead of[a-zA-Z0-9-]These changes align with the broader normalization and query handling updates in the PR.
Also applies to: 13-13, 16-16, 20-20
src/pkg/cli/client/byoc/gcp/testdata/with_logtype_run.query (1)
9-9: Test data consistently updated.The addition of
defang-stack = "beta"labels aligns with the query pattern updates across the testdata suite.Also applies to: 12-12
src/pkg/cli/client/byoc/gcp/testdata/with_logtype_build.query (1)
9-9: Test data consistently updated.The changes properly reflect the updated query patterns with
defang-stack = "beta"labels and underscore support in build_tags regex.Also applies to: 12-12
src/pkg/cli/client/byoc/gcp/testdata/with_everything.query (1)
12-12: Comprehensive test data updated correctly.All resource types (cloud_run_job, build, cloud_run_revision, gce_instance) have been updated with the appropriate label filters and regex patterns, maintaining consistency across the test suite.
Also applies to: 17-17, 20-20, 25-25
src/pkg/cli/client/byoc/gcp/testdata/with_project_since_and_until.query (1)
9-9: Test data properly synchronized with code changes.The query patterns correctly incorporate the
defang-stack = "beta"labels and updated build_tags regex across all resource types.Also applies to: 13-13, 16-16, 20-20
src/pkg/cli/composeUp.go (2)
92-92: Correctly using normalized project name for external provider calls.The switch from
project.NametofixedProject.Nameensures that all provider API calls (GetDelegateSubdomainZone, DeployRequest, PrepareDomainDelegation, DelegateSubdomainZone) receive the normalized project name aftercompose.FixupServicesprocesses it. This aligns with the PR objective to handle project names with underscores.Also applies to: 103-103, 111-111, 136-136
156-156: Verify if storing original project name in deployment records is intentional.Line 156 uses
project.Name(original, non-normalized) when recording the deployment, while lines 92, 103, 111, and 136 usefixedProject.Name(normalized) for provider API calls. This creates an inconsistency where provider systems see the normalized name while deployment history records preserve the original name.This appears to be intentional—the comment on line 71 states "Do not modify the original project, because the caller needs it for debugging." However, this design could complicate correlation between deployment records (original name) and provider operations (normalized name). Confirm whether this is desired behavior for historical accuracy or if deployment records should also use the normalized name for consistency with provider identifiers.
src/pkg/cli/tail.go (2)
17-17: LGTM! Import added for project name normalization.The compose package import is needed to call
NormalizeProjectName.
211-211: LGTM! Project name normalization applied consistently.The normalization aligns with the
FixupServicesbehavior and ensures consistent project identifiers in tail requests.src/pkg/clouds/gcp/dns_test.go (1)
9-88: LGTM! Comprehensive test coverage for SafeZoneName.The test suite thoroughly validates all DNS zone name sanitization rules:
- Character normalization
- Length constraints
- Leading/trailing character requirements
- Empty input handling
The table-driven approach with custom validators is appropriate for this validation logic.
src/pkg/cli/compose/fixup.go (2)
8-8: LGTM! Import added for project name normalization.The regexp package is needed for the
safeProjectNameREregex pattern.
33-37: LGTM! Project name normalization applied early in the workflow.The normalization is correctly applied before loading config and fixing up services. The debug logging is helpful for tracking when normalization occurs.
src/pkg/cli/client/byoc/gcp/testdata/with_project.query (1)
9-21: The build_tags regex pattern is correct as-is.The middle segment
[a-zA-Z0-9-_]{1,63}correctly represents service names, which may contain underscores. Project names are a separate segment that have been normalized to contain only dashes (underscores replaced viaNormalizeProjectName()). The underscore inclusion is intentional: see theBuildTag.Parse()method at src/pkg/clouds/gcp/cloudbuild.go:42, which explicitly joins service names (parts 2 through n-1) with underscores to support service names that contain underscores.src/pkg/cli/client/byoc/gcp/byoc.go (2)
601-606: LGTM!Consistent propagation of
b.PulumiStackto log stream methods.
705-705: LGTM!The switch to
gcp.SafeZoneNamecorrectly applies GCP-specific DNS zone naming requirements (lowercase, max 63 chars, starts with letter, etc.) rather than the more genericdns.SafeLabel.src/pkg/clouds/gcp/dns.go (2)
116-125: LGTM!The
hashTrimfunction correctly preserves as much of the original name as possible while appending a deterministic hash suffix to handle overflow.
102-114: Overall implementation is sound.The
SafeZoneNamefunction correctly implements GCP DNS zone naming constraints. Aside from the minor issues noted above, the logic properly handles lowercase conversion, character sanitization, prefix requirements, and length limits with deterministic hashing.
Comment on lines
+482
to
+486
| var safeProjectNameRE = regexp.MustCompile(`[^A-Za-z0-9-]+`) | ||
|
|
||
| func NormalizeProjectName(name string) string { | ||
| return safeProjectNameRE.ReplaceAllString(name, "-") | ||
| } |
Contributor
There was a problem hiding this comment.
Handle edge cases in NormalizeProjectName.
The current implementation can produce invalid project names in edge cases:
- Input
"_test"→"-test"(leading hyphen) - Input
"test_"→"test-"(trailing hyphen) - Input
"___"→"-"(just a hyphen) - Input
""→""(empty string)
These could cause downstream failures in GCP APIs. Consider adding safeguards similar to SafeZoneName:
- Trim leading/trailing hyphens
- Ensure non-empty output (e.g., prefix with "project-" if empty)
- Optionally enforce starting with a letter
🔎 Proposed enhancement with edge case handling
+var safeProjectNameRE = regexp.MustCompile(`[^A-Za-z0-9-]+`)
+
func NormalizeProjectName(name string) string {
- return safeProjectNameRE.ReplaceAllString(name, "-")
+ normalized := safeProjectNameRE.ReplaceAllString(name, "-")
+ normalized = strings.Trim(normalized, "-")
+ if normalized == "" {
+ return "project"
+ }
+ return normalized
}Import strings at the top if not already present.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| var safeProjectNameRE = regexp.MustCompile(`[^A-Za-z0-9-]+`) | |
| func NormalizeProjectName(name string) string { | |
| return safeProjectNameRE.ReplaceAllString(name, "-") | |
| } | |
| var safeProjectNameRE = regexp.MustCompile(`[^A-Za-z0-9-]+`) | |
| func NormalizeProjectName(name string) string { | |
| normalized := safeProjectNameRE.ReplaceAllString(name, "-") | |
| normalized = strings.Trim(normalized, "-") | |
| if normalized == "" { | |
| return "project" | |
| } | |
| return normalized | |
| } |
🤖 Prompt for AI Agents
In src/pkg/cli/compose/fixup.go around lines 482 to 486, NormalizeProjectName
can emit invalid names (leading/trailing hyphens, single hyphen or empty
string). Update the function to: replace invalid chars as now, then trim leading
and trailing hyphens, if the result is empty set it to a safe default like
"project-" + original cleaned value or simply "project", and optionally ensure
the first character is a letter (prepend "project" if it does not start with
A-Z/a-z). Import strings if needed for Trim/HasPrefix operations.
Member
There was a problem hiding this comment.
This seems like a good suggestion. Let's write some test cases for this function and cover the suggested cases. I vaguely remember that repeated hyphens may also not be permitted. Is that true?
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GCP project name constraints requirements
💡 Result:
Short answer — two different fields:
-
Project name (human-readable): editable anytime, need not be unique; no strict universal character set beyond UI/product-specific variants — avoid sensitive/PII. (cloud.google.com)
-
Project ID (the actual identifier with constraints): required to be globally unique and permanent. Requirements: 6–30 characters; lowercase letters, numbers, and hyphens only; must start with a letter; cannot end with a hyphen; cannot be already used (including deleted projects); cannot contain restricted strings such as "google" or "ssl". (Project ID is generated from the name by default and is recommended to be used as provided.) (cloud.google.com)
Notes:
- Some Google products/distributions may document slightly different project-name character rules (e.g., Distributed Cloud variants); those are product-specific. (cloud.google.com)
- Avoid putting secrets/PII in name or ID because IDs appear in resource paths and URLs. (cloud.google.com)
References:
- Google Cloud — Creating and managing projects (Resource Manager). (cloud.google.com)
- Google Distributed Cloud project naming (product-specific examples). (cloud.google.com)
Citations:
- 1: https://cloud.google.com/resource-manager/docs/creating-managing-projects?utm_source=openai
- 2: https://cloud.google.com/resource-manager/docs/creating-managing-projects?utm_source=openai
- 3: https://cloud.google.com/distributed-cloud/hosted/docs/latest/appliance/platform/pa-user/create-a-project?utm_source=openai
- 4: https://cloud.google.com/resource-manager/docs/creating-managing-projects?utm_source=openai
- 5: https://cloud.google.com/resource-manager/docs/creating-managing-projects?utm_source=openai
- 6: https://cloud.google.com/distributed-cloud/hosted/docs/latest/appliance/platform/pa-user/create-a-project?utm_source=openai
🏁 Script executed:
#!/bin/bash
# Create a comprehensive test file for NormalizeProjectName
cat > /tmp/fixup_test.go << 'EOF'
package compose
import (
"testing"
)
func TestNormalizeProjectName(t *testing.T) {
tests := []struct {
name string
input string
expected string
}{
{
name: "simple alphanumeric",
input: "myproject",
expected: "myproject",
},
{
name: "with valid hyphens",
input: "my-project-123",
expected: "my-project-123",
},
{
name: "underscore to hyphen",
input: "my_project",
expected: "my-project",
},
{
name: "multiple underscores",
input: "my__project",
expected: "my-project",
},
{
name: "leading underscore",
input: "_test",
expected: "test",
},
{
name: "trailing underscore",
input: "test_",
expected: "test",
},
{
name: "leading and trailing underscores",
input: "_test_",
expected: "test",
},
{
name: "only underscores",
input: "___",
expected: "project",
},
{
name: "empty string",
input: "",
expected: "project",
},
{
name: "special characters",
input: "my@project#test",
expected: "my-project-test",
},
{
name: "mixed special characters and underscores",
input: "my_project@123",
expected: "my-project-123",
},
{
name: "spaces",
input: "my project",
expected: "my-project",
},
{
name: "dots",
input: "my.project.test",
expected: "my-project-test",
},
{
name: "consecutive special characters",
input: "my___project!!!",
expected: "my-project",
},
{
name: "starts with number",
input: "123project",
expected: "123project",
},
{
name: "starts with hyphen after normalization",
input: "-test",
expected: "test",
},
{
name: "ends with hyphen after normalization",
input: "test-",
expected: "test",
},
{
name: "only special characters",
input: "!!!@@@",
expected: "project",
},
{
name: "unicode characters",
input: "my项目test",
expected: "my-test",
},
{
name: "real world example from issue",
input: "raphaeltm-prod1_zaoconnect_project",
expected: "raphaeltm-prod1-zaoconnect-project",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := NormalizeProjectName(tt.input)
if result != tt.expected {
t.Errorf("NormalizeProjectName(%q) = %q, want %q", tt.input, result, tt.expected)
}
// Additional validation: result should not have leading/trailing hyphens
if len(result) > 0 {
if result[0] == '-' {
t.Errorf("NormalizeProjectName(%q) = %q starts with hyphen", tt.input, result)
}
if result[len(result)-1] == '-' {
t.Errorf("NormalizeProjectName(%q) = %q ends with hyphen", tt.input, result)
}
}
// Result should not be empty
if result == "" {
t.Errorf("NormalizeProjectName(%q) returned empty string", tt.input)
}
})
}
}
func TestNormalizeProjectNameRepeatedHyphens(t *testing.T) {
tests := []struct {
name string
input string
expected string
}{
{
name: "double hyphen",
input: "my--project",
expected: "my-project",
},
{
name: "triple hyphen",
input: "my---project",
expected: "my-project",
},
{
name: "multiple double hyphens",
input: "my--project--test",
expected: "my-project-test",
},
{
name: "from consecutive special chars",
input: "my___project",
expected: "my-project",
},
{
name: "mixed underscores and special chars",
input: "my_@_project",
expected: "my-project",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := NormalizeProjectName(tt.input)
if result != tt.expected {
t.Errorf("NormalizeProjectName(%q) = %q, want %q", tt.input, result, tt.expected)
}
// Verify no consecutive hyphens remain
if len(result) > 1 {
for i := 0; i < len(result)-1; i++ {
if result[i] == '-' && result[i+1] == '-' {
t.Errorf("NormalizeProjectName(%q) = %q contains consecutive hyphens", tt.input, result)
}
}
}
})
}
}
EOF
cat /tmp/fixup_test.go
Length of output: 4114
@jordanstephens, great question! Based on GCP Project ID constraints (6-30 characters, lowercase letters/numbers/hyphens only, must start with a letter, cannot end with a hyphen), repeated hyphens aren't explicitly forbidden, but they're poor practice and should be collapsed.
More importantly, the current implementation has gaps:
- Consecutive hyphens from input aren't collapsed (e.g.,
"my--project"stays as-is) - Consecutive special chars are collapsed (e.g.,
"my__project"→"my-project"✓) - Leading/trailing hyphens aren't trimmed
- Empty results aren't handled
- Must start with letter per GCP isn't enforced
Here's a comprehensive test file covering all edge cases:
package compose
import (
"testing"
)
func TestNormalizeProjectName(t *testing.T) {
tests := []struct {
name string
input string
expected string
}{
{
name: "simple alphanumeric",
input: "myproject",
expected: "myproject",
},
{
name: "with valid hyphens",
input: "my-project-123",
expected: "my-project-123",
},
{
name: "underscore to hyphen",
input: "my_project",
expected: "my-project",
},
{
name: "multiple underscores",
input: "my__project",
expected: "my-project",
},
{
name: "leading underscore",
input: "_test",
expected: "test",
},
{
name: "trailing underscore",
input: "test_",
expected: "test",
},
{
name: "leading and trailing underscores",
input: "_test_",
expected: "test",
},
{
name: "only underscores",
input: "___",
expected: "project",
},
{
name: "empty string",
input: "",
expected: "project",
},
{
name: "special characters",
input: "my@project#test",
expected: "my-project-test",
},
{
name: "mixed special characters and underscores",
input: "my_project@123",
expected: "my-project-123",
},
{
name: "spaces",
input: "my project",
expected: "my-project",
},
{
name: "dots",
input: "my.project.test",
expected: "my-project-test",
},
{
name: "consecutive special characters",
input: "my___project!!!",
expected: "my-project",
},
{
name: "starts with number",
input: "123project",
expected: "123project",
},
{
name: "starts with hyphen after normalization",
input: "-test",
expected: "test",
},
{
name: "ends with hyphen after normalization",
input: "test-",
expected: "test",
},
{
name: "only special characters",
input: "!!!@@@",
expected: "project",
},
{
name: "unicode characters",
input: "my项目test",
expected: "my-test",
},
{
name: "real world example from issue",
input: "raphaeltm-prod1_zaoconnect_project",
expected: "raphaeltm-prod1-zaoconnect-project",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := NormalizeProjectName(tt.input)
if result != tt.expected {
t.Errorf("NormalizeProjectName(%q) = %q, want %q", tt.input, result, tt.expected)
}
// Additional validation: result should not have leading/trailing hyphens
if len(result) > 0 {
if result[0] == '-' {
t.Errorf("NormalizeProjectName(%q) = %q starts with hyphen", tt.input, result)
}
if result[len(result)-1] == '-' {
t.Errorf("NormalizeProjectName(%q) = %q ends with hyphen", tt.input, result)
}
}
// Result should not be empty
if result == "" {
t.Errorf("NormalizeProjectName(%q) returned empty string", tt.input)
}
})
}
}
func TestNormalizeProjectNameRepeatedHyphens(t *testing.T) {
tests := []struct {
name string
input string
expected string
}{
{
name: "double hyphen",
input: "my--project",
expected: "my-project",
},
{
name: "triple hyphen",
input: "my---project",
expected: "my-project",
},
{
name: "multiple double hyphens",
input: "my--project--test",
expected: "my-project-test",
},
{
name: "from consecutive special chars",
input: "my___project",
expected: "my-project",
},
{
name: "mixed underscores and special chars",
input: "my_@_project",
expected: "my-project",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := NormalizeProjectName(tt.input)
if result != tt.expected {
t.Errorf("NormalizeProjectName(%q) = %q, want %q", tt.input, result, tt.expected)
}
// Verify no consecutive hyphens remain
if len(result) > 1 {
for i := 0; i < len(result)-1; i++ {
if result[i] == '-' && result[i+1] == '-' {
t.Errorf("NormalizeProjectName(%q) = %q contains consecutive hyphens", tt.input, result)
}
}
}
})
}
}And here's the enhanced implementation to handle all these cases:
var safeProjectNameRE = regexp.MustCompile(`[^A-Za-z0-9-]+`)
var repeatedHyphensRE = regexp.MustCompile(`-+`)
func NormalizeProjectName(name string) string {
// Replace non-allowed characters with hyphens
normalized := safeProjectNameRE.ReplaceAllString(name, "-")
// Collapse consecutive hyphens into single hyphen
normalized = repeatedHyphensRE.ReplaceAllString(normalized, "-")
// Trim leading and trailing hyphens
normalized = strings.Trim(normalized, "-")
// Return default if empty
if normalized == "" {
return "project"
}
return normalized
}The key improvements:
- ✅ Collapses repeated hyphens with
repeatedHyphensRE - ✅ Trims leading/trailing hyphens
- ✅ Provides safe default for empty input
- ✅ Handles all edge cases from the real-world issue
Note: I didn't enforce "must start with letter" since that might be too restrictive for project names (vs. GCP Project IDs), but you can add that if needed.
🧬 Code Graph Analysis Results
[src/pkg/cli/compose/fixup.go] — Summary of relevant code for user comment
-
Summary: NormalizeProjectName and related regular expression used to sanitize project names by replacing sequences of non-allowed characters with a single hyphen, ensuring the resulting name contains only letters, numbers, and hyphens.
-
Entities:
- safeProjectNameRE: a compiled regular expression that matches any run of characters not in [A-Za-z0-9-].
- NormalizeProjectName(name string) string: function that returns a sanitized version of the input name by replacing all matches of safeProjectNameRE with "-".
-
Behavior details:
- Parameters:
- name: the input project name to normalize.
- Returns:
- A string where any consecutive group of non-alphanumeric, non-hyphen characters in the input is replaced by a single hyphen.
- Error handling:
- None (pure string transformation; does not raise errors).
- Important implementation notes:
- Hyphens already present in the input are preserved (they are allowed characters).
- Sequences of disallowed characters are collapsed to a single hyphen.
- This function is used to derive project.Name normalization and is referenced in FixupServices when normalizing the project name.
- Parameters:
🧠 Learnings used
Learnt from: lionello
Repo: DefangLabs/defang PR: 1734
File: src/cmd/cli/command/commands.go:1220-1226
Timestamp: 2025-12-25T04:38:40.356Z
Learning: In the Defang CLI codebase (src/cmd/cli/command/commands.go), empty project names (from inputs like "/stack" or when --project-name is omitted) are acceptable and intentional behavior, as they work similarly to not providing the --project-name flag at all.
Comment on lines
+42
to
+44
| if parsed != tt.bt { | ||
| t.Errorf("Parse() = %+v, want %+v", parsed, tt.want) | ||
| } |
Contributor
There was a problem hiding this comment.
Bug: Wrong variable in error message.
The error message references tt.want (a string) instead of tt.bt (the expected BuildTag struct). This would produce a misleading error message if the test fails.
🔎 Proposed fix
if parsed != tt.bt {
- t.Errorf("Parse() = %+v, want %+v", parsed, tt.want)
+ t.Errorf("Parse() = %+v, want %+v", parsed, tt.bt)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if parsed != tt.bt { | |
| t.Errorf("Parse() = %+v, want %+v", parsed, tt.want) | |
| } | |
| if parsed != tt.bt { | |
| t.Errorf("Parse() = %+v, want %+v", parsed, tt.bt) | |
| } |
🤖 Prompt for AI Agents
In src/pkg/clouds/gcp/cloudbuild_test.go around lines 42 to 44, the test's error
message prints tt.want (a string) instead of the expected tt.bt (the BuildTag
struct), producing misleading output on failure; update the t.Errorf call so it
uses tt.bt as the expected value (and keep the %v/%+v format consistent with
parsed and tt.bt) so the error message shows the actual expected BuildTag struct
rather than the string.
Comment on lines
+133
to
+138
| if len(hashBase36) > length { | ||
| return hashBase36[:length] | ||
| } | ||
| // if the hash is too short, pad with leading zeros | ||
| return fmt.Sprintf("%0*s", length, hashBase36) | ||
| } |
Contributor
There was a problem hiding this comment.
Bug: %0*s does not zero-pad strings.
In Go's fmt package, the 0 flag only works for numeric format verbs. Using %0*s will pad with spaces, not zeros. If hashBase36 is short (e.g., "0" for a zero hash), the result would be " 0" (space-padded), which violates DNS zone naming rules.
Proposed fix
func hashn(str string, length int) string {
hash := sha256.New()
hash.Write([]byte(str))
hashInt := binary.LittleEndian.Uint64(hash.Sum(nil)[:8])
hashBase36 := strconv.FormatUint(hashInt, 36) // base 36 string
// truncate if the hash is too long
if len(hashBase36) > length {
return hashBase36[:length]
}
// if the hash is too short, pad with leading zeros
- return fmt.Sprintf("%0*s", length, hashBase36)
+ return strings.Repeat("0", length-len(hashBase36)) + hashBase36
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if len(hashBase36) > length { | |
| return hashBase36[:length] | |
| } | |
| // if the hash is too short, pad with leading zeros | |
| return fmt.Sprintf("%0*s", length, hashBase36) | |
| } | |
| if len(hashBase36) > length { | |
| return hashBase36[:length] | |
| } | |
| // if the hash is too short, pad with leading zeros | |
| return strings.Repeat("0", length-len(hashBase36)) + hashBase36 | |
| } |
🤖 Prompt for AI Agents
In src/pkg/clouds/gcp/dns.go around lines 133 to 138, the code uses
fmt.Sprintf("%0*s", length, hashBase36) which does not zero-pad strings; replace
this with explicit zero-padding by prefixing the hash with strings.Repeat("0",
length-len(hashBase36)) + hashBase36 when len(hashBase36) < length, and add the
strings import if missing.
Member
There was a problem hiding this comment.
I don't see a test case that covers this. Let's add one
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# View the existing dns_test.go file to understand test structure
cat src/pkg/clouds/gcp/dns_test.go
Length of output: 2120
@jordanstephens, great catch! Here's a test case that covers the zero-padding scenario when the hash is shorter than 6 characters:
{
name: "hash trimming produces valid output without spaces",
input: strings.Repeat("a", 70), // Long enough to trigger hashTrim
validate: func(t *testing.T, output string) {
if len(output) != 63 {
t.Fatalf("expected length 63, got %d", len(output))
}
// Verify no spaces in output (would indicate improper padding)
if strings.Contains(output, " ") {
t.Fatalf("output contains spaces from improper padding: %q", output)
}
// Verify only valid characters (a-z, 0-9, -)
matched, _ := regexp.MatchString(`^[a-z0-9-]+$`, output)
if !matched {
t.Fatalf("output contains invalid characters: %q", output)
}
},
},This test should be added to the tests slice in the TestSafeZoneName function. It will:
- Trigger the
hashTrimpath by providing input longer than 63 characters - Verify the output is exactly 63 characters (as required)
- Check for spaces in the output, which would appear with the buggy
fmt.Sprintf("%0*s", ...)implementation when the hash needs padding - Ensure all characters are valid per DNS zone naming rules
With the current buggy code, if the hash happens to be short, this test would fail due to space padding. After applying the fix suggested by CodeRabbit, the test should pass consistently.
🧬 Code Graph Analysis Results
[src/pkg/clouds/gcp/project.go] (lines 98-101)
Gcp struct {
Region string
ProjectId string
}
[src/pkg/clouds/gcp/project.go] (lines 15-15)
ProjectId string
[src/pkg/clouds/gcp/artifactregistry.go] (lines 51-61)
func IsNotFound(err error) bool {
if grpcErr, ok := status.FromError(err); ok {
if grpcErr.Code() == codes.NotFound {
return true
}
if grpcErr.Code() == codes.Unknown && strings.HasSuffix(grpcErr.Message(), "notFound") {
return true
}
}
return false
}
| subscribeStream.AddJobExecutionUpdate(path.Base(b.cdExecution)) | ||
| } | ||
|
|
||
| // TODO: update stack (1st param) to b.PulumiStack |
Member
There was a problem hiding this comment.
Is this TODO done now?
Suggested change
| // TODO: update stack (1st param) to b.PulumiStack |
src/pkg/cli/client/byoc/gcp/byoc.go
Outdated
| query.AddJobExecutionQuery(path.Base(b.cdExecution)) | ||
| } | ||
|
|
||
| // Logs TODO: update stack (1st param) to b.PulumiStack |
Member
There was a problem hiding this comment.
Suggested change
| // Logs TODO: update stack (1st param) to b.PulumiStack |
src/pkg/cli/client/byoc/gcp/byoc.go
Outdated
| query.AddSince(since) | ||
| query.AddUntil(until) | ||
|
|
||
| // Service status updates TODO: update stack (1st param) to b.PulumiStack |
Member
There was a problem hiding this comment.
Suggested change
| // Service status updates TODO: update stack (1st param) to b.PulumiStack |
| func (q *Query) AddCloudBuildLogQuery(stack, project, etag string, services []string) { | ||
| query := `resource.type="build"` | ||
|
|
||
| // FIXME: Support stack |
Member
There was a problem hiding this comment.
What is necessary to support stacks?
Comment on lines
+482
to
+486
| var safeProjectNameRE = regexp.MustCompile(`[^A-Za-z0-9-]+`) | ||
|
|
||
| func NormalizeProjectName(name string) string { | ||
| return safeProjectNameRE.ReplaceAllString(name, "-") | ||
| } |
Member
There was a problem hiding this comment.
This seems like a good suggestion. Let's write some test cases for this function and cover the suggested cases. I vaguely remember that repeated hyphens may also not be permitted. Is that true?
|
|
||
| // Assume "services" are normalized service names | ||
| subscribeRequest := defangv1.SubscribeRequest{Project: projectName, Etag: etag, Services: services} | ||
| subscribeRequest := defangv1.SubscribeRequest{Project: compose.NormalizeProjectName(projectName), Etag: etag, Services: services} |
Member
There was a problem hiding this comment.
Should we push this responsibility into the provider.Subscribe function so callers do not have to remember to make the project name safe on each invocation?
Comment on lines
-210
to
+211
| Project: projectName, | ||
| Project: compose.NormalizeProjectName(projectName), // Matching the FixupServices behavior |
Member
There was a problem hiding this comment.
Same here, I think provider.QueryLogs should be responsible for normalizing the project name in the incoming request.
Comment on lines
+107
to
+109
| if len(safe) == 0 || safe[0] == '-' { | ||
| safe = "zone" + safe | ||
| } |
Member
There was a problem hiding this comment.
If len(safe) == 0 doesn't that mean we have a larger problem somewhere? Should we panic or fail more loudly?
Comment on lines
+133
to
+138
| if len(hashBase36) > length { | ||
| return hashBase36[:length] | ||
| } | ||
| // if the hash is too short, pad with leading zeros | ||
| return fmt.Sprintf("%0*s", length, hashBase36) | ||
| } |
Member
There was a problem hiding this comment.
I don't see a test case that covers this. Let's add one
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pkg/cli/tail.go (1)
136-136: Normalize project name for GetService consistency with TailRequestThe
GetServicecall on line 136 uses the rawprojectName, while theTailRequeston line 211 normalizes it withcompose.NormalizeProjectName(projectName). The comment on line 211 explicitly states "Matching the FixupServices behavior". Since the backend expects normalized project names (where special characters like underscores are converted to hyphens), the two calls will query different project identifiers when the project name contains special characters, potentially causing false "Service does not exist (yet)" warnings.Apply the same normalization to the GetService call:
if _, err := provider.GetService(ctx, &defangv1.GetRequest{Project: compose.NormalizeProjectName(projectName), Name: service}); err != nil {
🤖 Fix all issues with AI agents
In @src/pkg/cli/client/byoc/gcp/testdata/with_cd_exec.query:
- Around line 14-16: The character class in the regex used for labels.build_tags
is ordered incorrectly causing an unintended range; update the pattern
labels.build_tags =~ "beta__[a-zA-Z0-9-_]{1,63}_" to move the hyphen to the end
so it becomes labels.build_tags =~ "beta__[a-zA-Z0-9_-]{1,63}_" to avoid
matching characters between '-' and '_' and preserve the intended allowed set.
In @src/pkg/cli/client/byoc/gcp/testdata/with_logtype_build.query:
- Line 12: The character class in the regex "[a-zA-Z0-9-_]" creates an
accidental range; update occurrences to put the hyphen at the end so it becomes
"[a-zA-Z0-9_-]". Replace the pattern in the test data (with_logtype_build.query)
and the corresponding production regex in query.go (the build tags /
labels.build_tags pattern) so both use "[a-zA-Z0-9_-]" to restrict matches to
alphanumerics, underscore and hyphen only.
🧹 Nitpick comments (2)
src/pkg/cli/compose/fixup.go (1)
488-492: Consider handling edge cases in project name normalization.The current regex
[^A-Za-z0-9-]+replaces non-alphanumeric/hyphen characters with a single hyphen, but doesn't handle several edge cases that could produce invalid GCP identifiers:
- Multiple consecutive underscores:
"my__project"→"my--project"(consecutive hyphens)- Leading underscores:
"_project"→"-project"(leading hyphen)- Trailing underscores:
"project_"→"project-"(trailing hyphen)GCP identifiers (project IDs, DNS labels, Cloud Build tags) typically must not start or end with hyphens, and consecutive hyphens may be invalid in some contexts.
♻️ Proposed enhancement to handle edge cases
-var safeProjectNameRE = regexp.MustCompile(`[^A-Za-z0-9-]+`) +var safeProjectNameRE = regexp.MustCompile(`[^A-Za-z0-9]+`) func NormalizeProjectName(name string) string { - return safeProjectNameRE.ReplaceAllString(name, "-") + // Replace non-alphanumeric characters with hyphens + normalized := safeProjectNameRE.ReplaceAllString(name, "-") + // Remove leading and trailing hyphens + normalized = strings.Trim(normalized, "-") + // Collapse consecutive hyphens + normalized = regexp.MustCompile(`-+`).ReplaceAllString(normalized, "-") + return normalized }Note: This preserves existing hyphens in the name while cleaning up the edges and preventing consecutive hyphens from normalization.
src/pkg/clouds/gcp/cloudbuild.go (1)
81-81: Fix grammatical error in comment.The comment has a subject-verb agreement issue: "Project names has been normalized" should be "Project names have been normalized" (plural) or "Project name has been normalized" (singular).
📝 Proposed fix
- bt.Project = parts[1] // Project names has been normalized to not contain underscores + bt.Project = parts[1] // Project names have been normalized to not contain underscores
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/pkg/cli/client/byoc/gcp/byoc.gosrc/pkg/cli/client/byoc/gcp/query.gosrc/pkg/cli/client/byoc/gcp/testdata/with_cd_exec.querysrc/pkg/cli/client/byoc/gcp/testdata/with_etag.querysrc/pkg/cli/client/byoc/gcp/testdata/with_etag_and_since.querysrc/pkg/cli/client/byoc/gcp/testdata/with_etag_equal_cd_exec.querysrc/pkg/cli/client/byoc/gcp/testdata/with_everything.querysrc/pkg/cli/client/byoc/gcp/testdata/with_logtype_all.querysrc/pkg/cli/client/byoc/gcp/testdata/with_logtype_build.querysrc/pkg/cli/client/byoc/gcp/testdata/with_project.querysrc/pkg/cli/client/byoc/gcp/testdata/with_project_since_and_until.querysrc/pkg/cli/compose/fixup.gosrc/pkg/cli/composeUp.gosrc/pkg/cli/tail.gosrc/pkg/clouds/gcp/cloudbuild.gosrc/pkg/clouds/gcp/cloudbuild_test.go
🚧 Files skipped from review as they are similar to previous changes (11)
- src/pkg/cli/client/byoc/gcp/query.go
- src/pkg/cli/composeUp.go
- src/pkg/cli/client/byoc/gcp/testdata/with_project.query
- src/pkg/cli/client/byoc/gcp/testdata/with_etag_equal_cd_exec.query
- src/pkg/cli/client/byoc/gcp/byoc.go
- src/pkg/cli/client/byoc/gcp/testdata/with_project_since_and_until.query
- src/pkg/cli/client/byoc/gcp/testdata/with_logtype_all.query
- src/pkg/clouds/gcp/cloudbuild_test.go
- src/pkg/cli/client/byoc/gcp/testdata/with_etag.query
- src/pkg/cli/client/byoc/gcp/testdata/with_etag_and_since.query
- src/pkg/cli/client/byoc/gcp/testdata/with_everything.query
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2026-01-07T00:34:13.131Z
Learnt from: lionello
Repo: DefangLabs/defang PR: 1742
File: src/pkg/cli/composeDown.go:14-18
Timestamp: 2026-01-07T00:34:13.131Z
Learning: In Defang's Defang CLI, CdCommandDown performs refresh + destroy, while CdCommandDestroy performs destroy only (no refresh). Update ComposeDown (src/pkg/cli/composeDown.go) to call CdCommandDestroy to perform destruction without refreshing. This ensures the intended semantics are preserved when tearing down compositions; avoid using CdCommandDown in ComposeDown unless a refresh is explicitly desired. Verify that ComposeDown's destroy path does not trigger a refresh side effect from CdCommandDown and that tests cover both pathways if they exist.
Applied to files:
src/pkg/cli/tail.gosrc/pkg/clouds/gcp/cloudbuild.gosrc/pkg/cli/compose/fixup.go
📚 Learning: 2026-01-09T20:12:21.986Z
Learnt from: edwardrf
Repo: DefangLabs/defang PR: 1747
File: src/pkg/cli/client/byoc/gcp/byoc.go:30-30
Timestamp: 2026-01-09T20:12:21.986Z
Learning: In Go files, recognize and accept the import path go.yaml.in/yaml/v3 as the maintained fork of the YAML library. Do not flag this import as incorrect; this fork supersedes the archived gopkg.in/yaml.v3 path. If you encounter this or similar forked import paths, treat them as valid Go imports and do not raise review flags.
Applied to files:
src/pkg/cli/tail.gosrc/pkg/clouds/gcp/cloudbuild.gosrc/pkg/cli/compose/fixup.go
📚 Learning: 2026-01-09T20:18:59.234Z
Learnt from: edwardrf
Repo: DefangLabs/defang PR: 1747
File: src/pkg/clouds/gcp/cloudbuild.go:62-86
Timestamp: 2026-01-09T20:18:59.234Z
Learning: In src/pkg/clouds/gcp/cloudbuild.go, enforce BuildTag.Parse to fail for any tag format that is not strictly controlled: require that tags have exactly 3–4 underscore-separated parts and that the tag is DefangCDBuildTag (or a clearly defined equivalent type/constant). This tag format check should be performed during parsing to catch unexpected, potentially erroneous tags, since build tags are only created in two controlled paths: (1) running CD in Cloud Build via CLI, and (2) building images by CD. Add unit tests covering valid cases (defang CD build tags) and invalid cases (wrong number of parts, non-DefangCDBuildTag), and fail fast with a descriptive error message when the constraint is violated.
Applied to files:
src/pkg/clouds/gcp/cloudbuild.go
📚 Learning: 2026-01-06T20:32:17.173Z
Learnt from: edwardrf
Repo: DefangLabs/defang PR: 1747
File: src/pkg/clouds/gcp/cloudbuild.go:120-123
Timestamp: 2026-01-06T20:32:17.173Z
Learning: In src/pkg/clouds/gcp/cloudbuild.go, the YAML unmarshaling into cloudbuildpb.BuildStep works because CloudBuildStep (from byoc.go) marshals to YAML with lowercase keys (name, args, env, entrypoint) that match the protobuf BuildStep exported field names via case-insensitive matching in yaml.Unmarshal. This is an intentional intermediate-struct pattern.
Applied to files:
src/pkg/clouds/gcp/cloudbuild.go
📚 Learning: 2025-12-25T04:38:40.356Z
Learnt from: lionello
Repo: DefangLabs/defang PR: 1734
File: src/cmd/cli/command/commands.go:1220-1226
Timestamp: 2025-12-25T04:38:40.356Z
Learning: In the Defang CLI codebase (src/cmd/cli/command/commands.go), empty project names (from inputs like "/stack" or when --project-name is omitted) are acceptable and intentional behavior, as they work similarly to not providing the --project-name flag at all.
Applied to files:
src/pkg/cli/compose/fixup.go
📚 Learning: 2026-01-09T20:18:59.234Z
Learnt from: edwardrf
Repo: DefangLabs/defang PR: 1747
File: src/pkg/clouds/gcp/cloudbuild.go:62-86
Timestamp: 2026-01-09T20:18:59.234Z
Learning: In src/pkg/clouds/gcp/cloudbuild.go, BuildTag.Parse should fail on unexpected tag formats (tags that don't have 3-4 underscore-separated parts or aren't DefangCDBuildTag) because build tags are strictly controlled and only created in two places: (1) running CD in cloudbuild by CLI, and (2) building images by CD. Unexpected tags indicate an error case.
Applied to files:
src/pkg/cli/compose/fixup.gosrc/pkg/cli/client/byoc/gcp/testdata/with_cd_exec.querysrc/pkg/cli/client/byoc/gcp/testdata/with_logtype_build.query
📚 Learning: 2026-01-07T03:07:48.228Z
Learnt from: edwardrf
Repo: DefangLabs/defang PR: 1747
File: src/pkg/cli/client/byoc/gcp/byoc.go:448-450
Timestamp: 2026-01-07T03:07:48.228Z
Learning: In src/pkg/cli/client/byoc/gcp/byoc.go, the GetDeploymentStatus method intentionally does not pre-validate b.cdExecution before calling b.driver.GetBuildStatus. If b.cdExecution is empty, it represents an error state that will be surfaced by the GCP API as an "invalid operation name" error, which is the intended behavior.
Applied to files:
src/pkg/cli/client/byoc/gcp/testdata/with_cd_exec.query
🧬 Code graph analysis (2)
src/pkg/cli/tail.go (2)
src/pkg/cli/compose/loader.go (1)
Project(25-25)src/pkg/cli/compose/fixup.go (1)
NormalizeProjectName(490-492)
src/pkg/cli/compose/fixup.go (1)
src/pkg/term/colorizer.go (1)
Debugf(298-300)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (go)
- GitHub Check: go-test
🔇 Additional comments (4)
src/pkg/cli/tail.go (1)
17-17: LGTM! Project name normalization properly applied.The tail request now uses the normalized project name, ensuring consistency with the compose-level normalization introduced in this PR.
Also applies to: 211-211
src/pkg/cli/compose/fixup.go (1)
33-37: LGTM! Normalization applied at the right point in the flow.The normalization occurs after provider-specific fixups and before config loading, ensuring that downstream operations (config lookup, build context uploads, etc.) all use the sanitized project name. The debug logging helps track when normalization occurs.
src/pkg/clouds/gcp/cloudbuild.go (2)
56-60: LGTM! Backward compatibility clearly documented.The comment on line 57 clearly indicates the backward-compatible format when Stack is empty, which aligns with the parsing logic that handles both 3-part and 4+-part tag formats.
70-84: Verify that relaxed tag format validation is intentional.The parsing logic now accepts tags with 3 or more underscore-separated parts (changed from exactly 3 or 4), allowing service names with arbitrary underscores by joining the middle segments. While this supports the PR objective of handling underscores in service names, it conflicts with the learnings that recommend strict validation:
"BuildTag.Parse should fail on unexpected tag formats (tags that don't have 3-4 underscore-separated parts or aren't DefangCDBuildTag)"
The new logic permits tags like
stack_project_service_with_many_underscores_etag(6+ parts), which may be valid if project names are guaranteed to be normalized (no underscores) and stacks follow the same constraint.Based on learnings, strict validation was recommended to catch errors early since build tags are created in controlled code paths. Confirm that:
- Relaxing validation to support arbitrary-length service names with underscores is intentional and safe
- The assumption that Stack and Project never contain underscores holds (enforced by normalization)
- This won't inadvertently accept malformed tags from unexpected sources
Comment on lines
14
to
16
| resource.type="build" | ||
| labels.build_tags =~ "beta__[a-zA-Z0-9-]{1,63}_" | ||
| labels.build_tags =~ "beta__[a-zA-Z0-9-_]{1,63}_" | ||
| -labels.build_step="MAIN" |
Contributor
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
# First, let's find and examine the BuildTag.Parse function mentioned in the learning
find . -type f -name "*.go" | xargs rg -l "BuildTag" | head -20Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
./src/pkg/clouds/gcp/cloudbuild_test.go
./src/pkg/clouds/gcp/cloudbuild.go
./src/pkg/cli/client/byoc/gcp/stream.go
./src/pkg/cli/client/byoc/gcp/byoc_test.go
Script executed:
# Look at the test data file to understand its context
cat -n src/pkg/cli/client/byoc/gcp/testdata/with_cd_exec.queryRepository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
1 (
2 logName=~"logs/run.googleapis.com%2F(stdout|stderr)$" OR
3 logName="projects/test-project/logs/cloudbuild" OR
4 logName="projects/test-project/logs/cos_containers" OR
5 logName="projects/test-project/logs/docker-logs"
6 ) AND (
7 (
8 resource.type = "cloud_run_job"
9 labels."run.googleapis.com/execution_name" = "test-execution-id"
10 ) OR (
11 resource.type = "cloud_run_job"
12 labels."defang-stack" = "beta"
13 ) OR (
14 resource.type="build"
15 labels.build_tags =~ "beta__[a-zA-Z0-9-_]{1,63}_"
16 -labels.build_step="MAIN"
17 ) OR (
18 resource.type="cloud_run_revision"
19 labels."defang-stack" = "beta"
20 ) OR (
21 resource.type="gce_instance"
22 labels."defang-stack" = "beta"
23 )
24 )
Script executed:
# Check git history to see what the original regex was
cd . && git log -p --follow -- "src/pkg/cli/client/byoc/gcp/testdata/with_cd_exec.query" | head -100Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
Script executed:
# Find BuildTag definition and Parse method
fd -t f "*.go" | xargs rg -A 30 "func.*BuildTag.*Parse|type.*BuildTag" | head -100Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
src/pkg/clouds/gcp/cloudbuild.go:type BuildTag struct {
src/pkg/clouds/gcp/cloudbuild.go- Stack string
src/pkg/clouds/gcp/cloudbuild.go- Project string
src/pkg/clouds/gcp/cloudbuild.go- Service string
src/pkg/clouds/gcp/cloudbuild.go- Etag string
src/pkg/clouds/gcp/cloudbuild.go- IsDefangCD bool
src/pkg/clouds/gcp/cloudbuild.go-}
src/pkg/clouds/gcp/cloudbuild.go-
src/pkg/clouds/gcp/cloudbuild.go-func (bt BuildTag) String() string {
src/pkg/clouds/gcp/cloudbuild.go- if bt.Stack == "" {
src/pkg/clouds/gcp/cloudbuild.go- return fmt.Sprintf("%s_%s_%s", bt.Project, bt.Service, bt.Etag) // Backward compatibility
src/pkg/clouds/gcp/cloudbuild.go- } else {
src/pkg/clouds/gcp/cloudbuild.go- return fmt.Sprintf("%s_%s_%s_%s", bt.Stack, bt.Project, bt.Service, bt.Etag)
src/pkg/clouds/gcp/cloudbuild.go- }
src/pkg/clouds/gcp/cloudbuild.go-}
src/pkg/clouds/gcp/cloudbuild.go-
src/pkg/clouds/gcp/cloudbuild.go:func (bt *BuildTag) Parse(tags []string) error {
src/pkg/clouds/gcp/cloudbuild.go- for _, tag := range tags {
src/pkg/clouds/gcp/cloudbuild.go- if tag == DefangCDBuildTag {
src/pkg/clouds/gcp/cloudbuild.go- bt.IsDefangCD = true
src/pkg/clouds/gcp/cloudbuild.go- continue
src/pkg/clouds/gcp/cloudbuild.go- }
src/pkg/clouds/gcp/cloudbuild.go- parts := strings.Split(tag, "_")
src/pkg/clouds/gcp/cloudbuild.go- if len(parts) < 3 {
src/pkg/clouds/gcp/cloudbuild.go- return fmt.Errorf("invalid cloudbuild build tags value: %q", tag)
src/pkg/clouds/gcp/cloudbuild.go- }
src/pkg/clouds/gcp/cloudbuild.go-
src/pkg/clouds/gcp/cloudbuild.go- if n := len(parts); n == 3 { // Backward compatibility
src/pkg/clouds/gcp/cloudbuild.go- bt.Stack = ""
src/pkg/clouds/gcp/cloudbuild.go- bt.Project = parts[0]
src/pkg/clouds/gcp/cloudbuild.go- bt.Service = parts[1]
src/pkg/clouds/gcp/cloudbuild.go- bt.Etag = parts[2]
src/pkg/clouds/gcp/cloudbuild.go- } else {
src/pkg/clouds/gcp/cloudbuild.go- bt.Stack = parts[0]
src/pkg/clouds/gcp/cloudbuild.go- bt.Project = parts[1] // Project names has been normalized to not contain underscores
src/pkg/clouds/gcp/cloudbuild.go- bt.Service = strings.Join(parts[2:n-1], "_") // Service names may contain underscores, so join all parts except last which is the etag
src/pkg/clouds/gcp/cloudbuild.go- bt.Etag = parts[n-1]
src/pkg/clouds/gcp/cloudbuild.go- }
src/pkg/clouds/gcp/cloudbuild.go- }
src/pkg/clouds/gcp/cloudbuild.go- return nil
src/pkg/clouds/gcp/cloudbuild.go-}
src/pkg/clouds/gcp/cloudbuild.go-
src/pkg/clouds/gcp/cloudbuild.go-func (gcp Gcp) GetBuildInfo(ctx context.Context, buildId string) (*BuildTag, error) {
src/pkg/clouds/gcp/cloudbuild.go- client, err := cloudbuild.NewClient(ctx)
src/pkg/clouds/gcp/cloudbuild.go- if err != nil {
src/pkg/clouds/gcp/cloudbuild.go- return nil, fmt.Errorf("failed to create cloudbuild client: %w", err)
src/pkg/clouds/gcp/cloudbuild.go- }
stderr:
[fd error]: regex parse error:
*.go
^
error: repetition operator missing expression
Note: You can use the '--fixed-strings' option to search for a literal string instead of a regular expression. Alternatively, you can also use the '--glob' option to match on a glob pattern.
Script executed:
# Search for underscore/dash normalization
rg -i "underscore|dash|normalize" src/pkg/clouds/gcp/ -A 2 -B 2Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
src/pkg/clouds/gcp/cloudbuild_test.go- },
src/pkg/clouds/gcp/cloudbuild_test.go- {
src/pkg/clouds/gcp/cloudbuild_test.go: name: "service name with underscores",
src/pkg/clouds/gcp/cloudbuild_test.go- bt: BuildTag{Stack: "stack1", Project: "my-proj-name", Service: "svc_name", Etag: "123"},
src/pkg/clouds/gcp/cloudbuild_test.go- want: "stack1_my-proj-name_svc_name_123",
--
src/pkg/clouds/gcp/cloudbuild.go- } else {
src/pkg/clouds/gcp/cloudbuild.go- bt.Stack = parts[0]
src/pkg/clouds/gcp/cloudbuild.go: bt.Project = parts[1] // Project names has been normalized to not contain underscores
src/pkg/clouds/gcp/cloudbuild.go: bt.Service = strings.Join(parts[2:n-1], "_") // Service names may contain underscores, so join all parts except last which is the etag
src/pkg/clouds/gcp/cloudbuild.go- bt.Etag = parts[n-1]
src/pkg/clouds/gcp/cloudbuild.go- }
--
src/pkg/clouds/gcp/project.go-//
src/pkg/clouds/gcp/project.go-// Compose Project Name:
src/pkg/clouds/gcp/project.go:// - must contain only lowercase letters, decimal digits, dashes, and underscores,
src/pkg/clouds/gcp/project.go-// - and must begin with a lowercase letter or decimal digit
src/pkg/clouds/gcp/project.go-//
src/pkg/clouds/gcp/project.go-// Differences:
src/pkg/clouds/gcp/project.go:// - Project ID cannot contain underscores
src/pkg/clouds/gcp/project.go-// - Project ID cannot start with a digit
src/pkg/clouds/gcp/project.go-// - Project ID cannot end with a hyphen
--
src/pkg/clouds/gcp/project.go- // Sanity step: Remove any illegal characters
src/pkg/clouds/gcp/project.go- id = regexp.MustCompile(`[^a-z0-9-]`).ReplaceAllString(id, "")
src/pkg/clouds/gcp/project.go: // Project ID cannot contain underscores
src/pkg/clouds/gcp/project.go- id = strings.ReplaceAll(id, "_", "-")
src/pkg/clouds/gcp/project.go- // Project ID cannot start with a digit
--
src/pkg/clouds/gcp/label.go-//
src/pkg/clouds/gcp/label.go-// GCP Label value requirement:
src/pkg/clouds/gcp/label.go:// - The value can only contain lowercase letters, numeric characters, underscores and dashes.
src/pkg/clouds/gcp/label.go-// - The value can be at most 63 characters long.
src/pkg/clouds/gcp/label.go-// - International characters are allowed.
Script executed:
# Check where build tags are created
rg -A 5 "BuildTag{" src/pkg/clouds/gcp/cloudbuild.goRepository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 2
stderr:
regex parse error:
BuildTag{
^
error: unclosed counted repetition
Script executed:
# Look for normalization in the codebase
rg "ReplaceAll.*_.*-|strings.Replace.*_" src/pkg/ -B 3 -A 3Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
src/pkg/clouds/gcp/project.go- // Sanity step: Remove any illegal characters
src/pkg/clouds/gcp/project.go- id = regexp.MustCompile(`[^a-z0-9-]`).ReplaceAllString(id, "")
src/pkg/clouds/gcp/project.go- // Project ID cannot contain underscores
src/pkg/clouds/gcp/project.go: id = strings.ReplaceAll(id, "_", "-")
src/pkg/clouds/gcp/project.go- // Project ID cannot start with a digit
src/pkg/clouds/gcp/project.go- if id[0] >= '0' && id[0] <= '9' {
src/pkg/clouds/gcp/project.go- id = "p-" + id
--
src/pkg/cli/compose/dockerfile_validator_test.go- for _, tt := range tests {
src/pkg/cli/compose/dockerfile_validator_test.go- t.Run(tt.name, func(t *testing.T) {
src/pkg/cli/compose/dockerfile_validator_test.go- // Create a test Dockerfile
src/pkg/cli/compose/dockerfile_validator_test.go: dockerfilePath := filepath.Join(tmpDir, "Dockerfile."+strings.ReplaceAll(tt.name, " ", "_"))
src/pkg/cli/compose/dockerfile_validator_test.go- err := os.WriteFile(dockerfilePath, []byte(tt.dockerfile), 0644)
src/pkg/cli/compose/dockerfile_validator_test.go- if err != nil {
src/pkg/cli/compose/dockerfile_validator_test.go- t.Fatalf("Failed to create test Dockerfile: %v", err)
--
src/pkg/cli/client/byoc/baseclient.go-// getServiceLabel returns a DNS-safe label for the given service
src/pkg/cli/client/byoc/baseclient.go-func getServiceLabel(serviceName string) string {
src/pkg/cli/client/byoc/baseclient.go- // Technically DNS names can have underscores, but these are reserved for SRV records and some systems have issues with them.
src/pkg/cli/client/byoc/baseclient.go: return dns.SafeLabel(strings.ReplaceAll(serviceName, "_", "-"))
src/pkg/cli/client/byoc/baseclient.go-}
src/pkg/cli/client/byoc/baseclient.go-
src/pkg/cli/client/byoc/baseclient.go-func (b *ByocBaseClient) ServicePrivateDNS(serviceName string) string {
Script executed:
# Find test cases or examples of actual build tag format
rg "beta.*__" . -r "test" --type-list | head -20Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stderr:
error: The argument '<PATTERN>' cannot be used with '--type-list'
USAGE:
rg [OPTIONS] PATTERN [PATH ...]
rg [OPTIONS] -e PATTERN ... [PATH ...]
rg [OPTIONS] -f PATTERNFILE ... [PATH ...]
rg [OPTIONS] --files [PATH ...]
rg [OPTIONS] --type-list
command | rg [OPTIONS] PATTERN
rg [OPTIONS] --help
rg [OPTIONS] --version
For more information try --help
Script executed:
# Check git history differently
git log --oneline -- "src/pkg/cli/client/byoc/gcp/testdata/with_cd_exec.query" 2>/dev/null || echo "No git history"Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
bee70a7 Merge branch 'main' into edw/gcp-build-tag-project-name-underscore
Script executed:
# Search for examples of build tag matching in test files
rg "beta_" src/pkg/ --type go -B 2 -A 2Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 1
Script executed:
# Look for the pattern with double underscore
rg "beta__" . -B 3 -A 3Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
./src/pkg/cli/client/byoc/gcp/testdata/with_logtype_build.query- labels."defang-stack" = "beta"
./src/pkg/cli/client/byoc/gcp/testdata/with_logtype_build.query-) OR (
./src/pkg/cli/client/byoc/gcp/testdata/with_logtype_build.query- resource.type="build"
./src/pkg/cli/client/byoc/gcp/testdata/with_logtype_build.query: labels.build_tags =~ "beta__[a-zA-Z0-9-_]{1,63}_"
./src/pkg/cli/client/byoc/gcp/testdata/with_logtype_build.query- -labels.build_step="MAIN"
./src/pkg/cli/client/byoc/gcp/testdata/with_logtype_build.query-)
./src/pkg/cli/client/byoc/gcp/testdata/with_logtype_build.query-)
--
./src/pkg/cli/client/byoc/gcp/testdata/with_logtype_all.query- labels."defang-stack" = "beta"
./src/pkg/cli/client/byoc/gcp/testdata/with_logtype_all.query-) OR (
./src/pkg/cli/client/byoc/gcp/testdata/with_logtype_all.query- resource.type="build"
./src/pkg/cli/client/byoc/gcp/testdata/with_logtype_all.query: labels.build_tags =~ "beta__[a-zA-Z0-9-_]{1,63}_"
./src/pkg/cli/client/byoc/gcp/testdata/with_logtype_all.query- -labels.build_step="MAIN"
./src/pkg/cli/client/byoc/gcp/testdata/with_logtype_all.query-) OR (
./src/pkg/cli/client/byoc/gcp/testdata/with_logtype_all.query- resource.type="cloud_run_revision"
--
./src/pkg/cli/client/byoc/gcp/testdata/with_etag_equal_cd_exec.query- labels."defang-stack" = "beta"
./src/pkg/cli/client/byoc/gcp/testdata/with_etag_equal_cd_exec.query-) OR (
./src/pkg/cli/client/byoc/gcp/testdata/with_etag_equal_cd_exec.query- resource.type="build"
./src/pkg/cli/client/byoc/gcp/testdata/with_etag_equal_cd_exec.query: labels.build_tags =~ "beta__[a-zA-Z0-9-_]{1,63}_"
./src/pkg/cli/client/byoc/gcp/testdata/with_etag_equal_cd_exec.query- -labels.build_step="MAIN"
./src/pkg/cli/client/byoc/gcp/testdata/with_etag_equal_cd_exec.query-) OR (
./src/pkg/cli/client/byoc/gcp/testdata/with_etag_equal_cd_exec.query- resource.type="cloud_run_revision"
--
./src/pkg/cli/client/byoc/gcp/testdata/with_cd_exec.query- labels."defang-stack" = "beta"
./src/pkg/cli/client/byoc/gcp/testdata/with_cd_exec.query-) OR (
./src/pkg/cli/client/byoc/gcp/testdata/with_cd_exec.query- resource.type="build"
./src/pkg/cli/client/byoc/gcp/testdata/with_cd_exec.query: labels.build_tags =~ "beta__[a-zA-Z0-9-_]{1,63}_"
./src/pkg/cli/client/byoc/gcp/testdata/with_cd_exec.query- -labels.build_step="MAIN"
./src/pkg/cli/client/byoc/gcp/testdata/with_cd_exec.query-) OR (
./src/pkg/cli/client/byoc/gcp/testdata/with_cd_exec.query- resource.type="cloud_run_revision"
--
./src/pkg/cli/client/byoc/gcp/testdata/with_etag.query- labels."defang-etag" = "test-etag"
./src/pkg/cli/client/byoc/gcp/testdata/with_etag.query-) OR (
./src/pkg/cli/client/byoc/gcp/testdata/with_etag.query- resource.type="build"
./src/pkg/cli/client/byoc/gcp/testdata/with_etag.query: labels.build_tags =~ "beta__[a-zA-Z0-9-_]{1,63}_test-etag"
./src/pkg/cli/client/byoc/gcp/testdata/with_etag.query- -labels.build_step="MAIN"
./src/pkg/cli/client/byoc/gcp/testdata/with_etag.query-) OR (
./src/pkg/cli/client/byoc/gcp/testdata/with_etag.query- resource.type="cloud_run_revision"
--
./src/pkg/cli/client/byoc/gcp/testdata/with_etag_and_since.query- labels."defang-etag" = "test-etag"
./src/pkg/cli/client/byoc/gcp/testdata/with_etag_and_since.query-) OR (
./src/pkg/cli/client/byoc/gcp/testdata/with_etag_and_since.query- resource.type="build"
./src/pkg/cli/client/byoc/gcp/testdata/with_etag_and_since.query: labels.build_tags =~ "beta__[a-zA-Z0-9-_]{1,63}_test-etag"
./src/pkg/cli/client/byoc/gcp/testdata/with_etag_and_since.query- -labels.build_step="MAIN"
./src/pkg/cli/client/byoc/gcp/testdata/with_etag_and_since.query-) OR (
./src/pkg/cli/client/byoc/gcp/testdata/with_etag_and_since.query- resource.type="cloud_run_revision"
Script executed:
# Check if there are examples of actual build tag values in tests
rg "stack.*project.*service.*etag|BuildTag" src/pkg/clouds/gcp/cloudbuild_test.go -A 5 -B 2Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
)
func TestBuildTagString(t *testing.T) {
tests := []struct {
name string
bt BuildTag
want string
}{
{
name: "with stack",
bt: BuildTag{Stack: "stack1", Project: "proj", Service: "svc", Etag: "123"},
want: "stack1_proj_svc_123",
},
{
name: "without stack",
bt: BuildTag{Project: "proj", Service: "svc", Etag: "123"},
want: "proj_svc_123",
},
{
name: "service name with underscores",
bt: BuildTag{Stack: "stack1", Project: "my-proj-name", Service: "svc_name", Etag: "123"},
want: "stack1_my-proj-name_svc_name_123",
},
}
for _, tt := range tests {
--
t.Errorf("String() = %q, want %q", tagStr, tt.want)
}
var parsed BuildTag
err := parsed.Parse([]string{tagStr})
if err != nil {
t.Fatalf("Parse() returned error: %v", err)
}
Script executed:
# Look at the query generation code to understand the double underscore
rg -B 10 -A 10 "build_tags.*=~" src/pkg/cli/client/byoc/gcp/ --type goRepository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
src/pkg/cli/client/byoc/gcp/query.go-
src/pkg/cli/client/byoc/gcp/query.go-func (q *Query) AddCloudBuildLogQuery(stack, project, etag string, services []string) {
src/pkg/cli/client/byoc/gcp/query.go- query := `resource.type="build"`
src/pkg/cli/client/byoc/gcp/query.go-
src/pkg/cli/client/byoc/gcp/query.go- // FIXME: Support stack
src/pkg/cli/client/byoc/gcp/query.go- servicesRegex := `[a-zA-Z0-9-_]{1,63}`
src/pkg/cli/client/byoc/gcp/query.go- if len(services) > 0 {
src/pkg/cli/client/byoc/gcp/query.go- servicesRegex = fmt.Sprintf("(%v)", strings.Join(services, "|")) // Cloud build labels allows upper case letters
src/pkg/cli/client/byoc/gcp/query.go- }
src/pkg/cli/client/byoc/gcp/query.go- query += fmt.Sprintf(`
src/pkg/cli/client/byoc/gcp/query.go:labels.build_tags =~ "%v_%v_%v_%v"`, stack, project, servicesRegex, etag)
src/pkg/cli/client/byoc/gcp/query.go- query += `
src/pkg/cli/client/byoc/gcp/query.go--labels.build_step="MAIN"` // Exclude main build step logs (like "FETCHSOURCE"/"PUSH"/"DONE") to reduce noise
src/pkg/cli/client/byoc/gcp/query.go-
src/pkg/cli/client/byoc/gcp/query.go- q.AddQuery(query)
src/pkg/cli/client/byoc/gcp/query.go-}
src/pkg/cli/client/byoc/gcp/query.go-
src/pkg/cli/client/byoc/gcp/query.go-func (q *Query) AddCloudBuildActivityQuery() {
src/pkg/cli/client/byoc/gcp/query.go- query := `resource.type="build"
src/pkg/cli/client/byoc/gcp/query.go-logName=~"logs/cloudaudit.googleapis.com%2Factivity$"`
src/pkg/cli/client/byoc/gcp/query.go- q.AddQuery(query)
Script executed:
# Check test file for query generation tests
rg "with_cd_exec|build_tags" src/pkg/cli/client/byoc/gcp/*test*.go -B 5 -A 5Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
src/pkg/cli/client/byoc/gcp/byoc_test.go- }},
src/pkg/cli/client/byoc/gcp/byoc_test.go- {name: "with_logtype_all", req: &defangv1.TailRequest{
src/pkg/cli/client/byoc/gcp/byoc_test.go- Pattern: "error",
src/pkg/cli/client/byoc/gcp/byoc_test.go- LogType: uint32(logs.LogTypeAll),
src/pkg/cli/client/byoc/gcp/byoc_test.go- }},
src/pkg/cli/client/byoc/gcp/byoc_test.go: {name: "with_cd_exec", req: &defangv1.TailRequest{
src/pkg/cli/client/byoc/gcp/byoc_test.go- LogType: uint32(logs.LogTypeAll),
src/pkg/cli/client/byoc/gcp/byoc_test.go- },
src/pkg/cli/client/byoc/gcp/byoc_test.go- cdExecution: "test-execution-id",
src/pkg/cli/client/byoc/gcp/byoc_test.go- },
src/pkg/cli/client/byoc/gcp/byoc_test.go- {name: "with_etag", req: &defangv1.TailRequest{
Script executed:
# Look for where these query files are generated or used
rg "with_cd_exec\|with_logtype" src/pkg/cli/client/byoc/gcp/ --type go -B 3 -A 3Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 1
Script executed:
# Look at how the test uses these query files
cat -n src/pkg/cli/client/byoc/gcp/byoc_test.go | head -150Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
1 package gcp
2
3 import (
4 "context"
5 "encoding/base64"
6 "io"
7 "testing"
8 "time"
9
10 "cloud.google.com/go/logging/apiv2/loggingpb"
11 "github.com/DefangLabs/defang/src/pkg"
12 "github.com/DefangLabs/defang/src/pkg/clouds/gcp"
13 "github.com/DefangLabs/defang/src/pkg/logs"
14 defangv1 "github.com/DefangLabs/defang/src/protos/io/defang/v1"
15 "google.golang.org/protobuf/types/known/timestamppb"
16 )
17
18 func TestSetUpCD(t *testing.T) {
19 t.Skip("skipping test")
20 ctx := t.Context()
21 b := NewByocProvider(ctx, "testTenantID", "")
22 account, err := b.AccountInfo(ctx)
23 if err != nil {
24 t.Errorf("AccountInfo() error = %v, want nil", err)
25 }
26 t.Logf("account: %+v", account)
27 if err := b.SetUpCD(ctx); err != nil {
28 t.Errorf("SetUpCD() error = %v, want nil", err)
29 }
30
31 payload := base64.StdEncoding.EncodeToString([]byte(`services:
32 nginx:
33 image: nginx:1-alpine
34 ports:
35 - "8080:80"
36 `))
37 cmd := cdCommand{
38 project: "testproj",
39 command: []string{"up", payload},
40 }
41
42 if op, err := b.runCdCommand(ctx, cmd); err != nil {
43 t.Errorf("CdCommand() error = %v, want nil", err)
44 } else {
45 t.Logf("CdCommand() = %v", op)
46 }
47 }
48
49 type MockGcpLogsClient struct {
50 lister gcp.Lister
51 tailer gcp.Tailer
52 }
53
54 func (m MockGcpLogsClient) ListLogEntries(ctx context.Context, query string, order gcp.Order) (gcp.Lister, error) {
55 return m.lister, nil
56 }
57
58 func (m MockGcpLogsClient) NewTailer(ctx context.Context) (gcp.Tailer, error) {
59 return m.tailer, nil
60 }
61 func (m MockGcpLogsClient) GetExecutionEnv(ctx context.Context, executionName string) (map[string]string, error) {
62 return nil, nil
63 }
64 func (m MockGcpLogsClient) GetProjectID() gcp.ProjectId {
65 return "test-project"
66 }
67 func (m MockGcpLogsClient) GetBuildInfo(ctx context.Context, buildId string) (*gcp.BuildTag, error) {
68 return &gcp.BuildTag{
69 Stack: "test-stack",
70 Project: "test-project",
71 Service: "test-service",
72 Etag: "test-etag",
73 }, nil
74 }
75
76 type MockGcpLoggingLister struct {
77 logEntries []*loggingpb.LogEntry
78 }
79
80 func (m *MockGcpLoggingLister) Next() (*loggingpb.LogEntry, error) {
81 if len(m.logEntries) > 0 {
82 entry := m.logEntries[0]
83 m.logEntries = m.logEntries[1:]
84 return entry, nil
85 }
86 return nil, io.EOF
87 }
88
89 type MockGcpLoggingTailer struct {
90 MockGcpLoggingLister
91 }
92
93 func (m *MockGcpLoggingTailer) Close() error {
94 return nil
95 }
96
97 func (m *MockGcpLoggingTailer) Start(ctx context.Context, query string) error {
98 return nil
99 }
100
101 func (m *MockGcpLoggingTailer) Next(ctx context.Context) (*loggingpb.LogEntry, error) {
102 return m.MockGcpLoggingLister.Next()
103 }
104
105 func TestGetCDExecutionContext(t *testing.T) {
106 tests := []struct {
107 name string
108 listEntries []loggingpb.LogEntry
109 tailEntries []loggingpb.LogEntry
110 }{
111 {name: "no entries"},
112 {name: "with only list entries",
113 listEntries: []loggingpb.LogEntry{
114 {Payload: &loggingpb.LogEntry_TextPayload{TextPayload: "log entry 1 from lister"}},
115 {Payload: &loggingpb.LogEntry_TextPayload{TextPayload: "log entry 2 from lister"}},
116 },
117 },
118 {name: "with only tail entries",
119 tailEntries: []loggingpb.LogEntry{
120 {Payload: &loggingpb.LogEntry_TextPayload{TextPayload: "log entry 1 from tailer"}},
121 {Payload: &loggingpb.LogEntry_TextPayload{TextPayload: "log entry 2 from tailer"}},
122 },
123 },
124 {name: "with both list and tail entries",
125 listEntries: []loggingpb.LogEntry{
126 {Payload: &loggingpb.LogEntry_TextPayload{TextPayload: "log entry 1 from lister"}},
127 {Payload: &loggingpb.LogEntry_TextPayload{TextPayload: "log entry 2 from lister"}},
128 },
129 tailEntries: []loggingpb.LogEntry{
130 {Payload: &loggingpb.LogEntry_TextPayload{TextPayload: "log entry 1 from tailer"}},
131 {Payload: &loggingpb.LogEntry_TextPayload{TextPayload: "log entry 2 from tailer"}},
132 },
133 },
134 }
135
136 ctx := t.Context()
137
138 for _, tt := range tests {
139 t.Run(tt.name, func(t *testing.T) {
140 b := NewByocProvider(ctx, "testTenantID", "")
141
142 driver := &MockGcpLogsClient{
143 lister: &MockGcpLoggingLister{},
144 tailer: &MockGcpLoggingTailer{},
145 }
146 newCtx, err := b.getCDExecutionContext(ctx, driver, &defangv1.TailRequest{})
147 if err != nil {
148 t.Errorf("getCDExecutionContext() error = %v, want nil", err)
149 }
150 if newCtx == ctx {
Script executed:
# Check if these are golden files being compared
rg "testdata|golden|with_cd_exec" src/pkg/cli/client/byoc/gcp/byoc_test.go -B 5 -A 10 | head -80Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
}},
{name: "with_logtype_all", req: &defangv1.TailRequest{
Pattern: "error",
LogType: uint32(logs.LogTypeAll),
}},
{name: "with_cd_exec", req: &defangv1.TailRequest{
LogType: uint32(logs.LogTypeAll),
},
cdExecution: "test-execution-id",
},
{name: "with_etag", req: &defangv1.TailRequest{
LogType: uint32(logs.LogTypeAll),
Etag: "test-etag",
}},
{name: "with_etag_and_since", req: &defangv1.TailRequest{
Since: timestamppb.New(time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)),
--
if !ok {
t.Fatalf("getLogStream() returned wrong type, want *gcp.LogStream")
}
query := logStream.GetQuery()
if err := pkg.Compare([]byte(query), "testdata/"+tt.name+".query"); err != nil {
t.Errorf("getLogStream() query mismatch: %v", err)
}
})
}
}
Script executed:
# Look at the actual test structure
ast-grep --pattern 'func Test.*($) {
$$$
}' src/pkg/cli/client/byoc/gcp/byoc_test.goRepository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 1
Script executed:
# Look for where AddCloudBuildLogQuery is called with CD execution
rg "AddCloudBuildLogQuery" src/pkg/cli/client/byoc/gcp/ -B 5 -A 5 --type goRepository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
src/pkg/cli/client/byoc/gcp/query.go- }
src/pkg/cli/client/byoc/gcp/query.go-
src/pkg/cli/client/byoc/gcp/query.go- q.AddQuery(query)
src/pkg/cli/client/byoc/gcp/query.go-}
src/pkg/cli/client/byoc/gcp/query.go-
src/pkg/cli/client/byoc/gcp/query.go:func (q *Query) AddCloudBuildLogQuery(stack, project, etag string, services []string) {
src/pkg/cli/client/byoc/gcp/query.go- query := `resource.type="build"`
src/pkg/cli/client/byoc/gcp/query.go-
src/pkg/cli/client/byoc/gcp/query.go- // FIXME: Support stack
src/pkg/cli/client/byoc/gcp/query.go- servicesRegex := `[a-zA-Z0-9-_]{1,63}`
src/pkg/cli/client/byoc/gcp/query.go- if len(services) > 0 {
--
src/pkg/cli/client/byoc/gcp/byoc.go- query.AddJobExecutionQuery(path.Base(b.cdExecution))
src/pkg/cli/client/byoc/gcp/byoc.go- }
src/pkg/cli/client/byoc/gcp/byoc.go-
src/pkg/cli/client/byoc/gcp/byoc.go- query.AddJobLogQuery(b.PulumiStack, req.Project, req.Etag, req.Services) // Kaniko OR CD logs
src/pkg/cli/client/byoc/gcp/byoc.go- query.AddServiceLogQuery(b.PulumiStack, req.Project, req.Etag, req.Services) // Cloudrun service logs
src/pkg/cli/client/byoc/gcp/byoc.go: query.AddCloudBuildLogQuery(b.PulumiStack, req.Project, req.Etag, req.Services) // CloudBuild logs
src/pkg/cli/client/byoc/gcp/byoc.go- query.AddSince(since)
src/pkg/cli/client/byoc/gcp/byoc.go- query.AddUntil(until)
src/pkg/cli/client/byoc/gcp/byoc.go-
src/pkg/cli/client/byoc/gcp/byoc.go- // Service status updates
src/pkg/cli/client/byoc/gcp/byoc.go- query.AddJobStatusUpdateRequestQuery(b.PulumiStack, req.Project, req.Etag, req.Services)
--
src/pkg/cli/client/byoc/gcp/stream.go- s.query.AddServiceLogQuery(stack, project, etag, services)
src/pkg/cli/client/byoc/gcp/stream.go- s.query.AddComputeEngineLogQuery(stack, project, etag, services)
src/pkg/cli/client/byoc/gcp/stream.go-}
src/pkg/cli/client/byoc/gcp/stream.go-
src/pkg/cli/client/byoc/gcp/stream.go-func (s *LogStream) AddCloudBuildLog(stack, project, etag string, services []string) {
src/pkg/cli/client/byoc/gcp/stream.go: s.query.AddCloudBuildLogQuery(stack, project, etag, services)
src/pkg/cli/client/byoc/gcp/stream.go-}
src/pkg/cli/client/byoc/gcp/stream.go-
src/pkg/cli/client/byoc/gcp/stream.go-func (s *LogStream) AddSince(start time.Time) {
src/pkg/cli/client/byoc/gcp/stream.go- s.query.AddSince(start)
src/pkg/cli/client/byoc/gcp/stream.go-}
Script executed:
# Check how cdExecution is used in getLogStream
rg "cdExecution|getLogStream" src/pkg/cli/client/byoc/gcp/stream.go -B 3 -A 10Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
s.query.AddCloudBuildActivityQuery()
}
var cdExecutionNamePattern = regexp.MustCompile(`^defang-cd-[a-z0-9]{5}$`)
func getLogEntryParser(ctx context.Context, gcpClient GcpLogsClient) func(entry *loggingpb.LogEntry) ([]*defangv1.TailResponse, error) {
envCache := make(map[string]map[string]string)
cdStarted := false
return func(entry *loggingpb.LogEntry) ([]*defangv1.TailResponse, error) {
if entry == nil {
return nil, nil
}
msg := entry.GetTextPayload()
--
envCache[executionName] = env
}
if cdExecutionNamePattern.MatchString(executionName) { // Special CD case
serviceName = "cd"
} else {
serviceName = env["DEFANG_SERVICE"]
}
// use kaniko build job environment to get etag
etag = env["DEFANG_ETAG"]
host = "pulumi" // Hardcoded to match end condition detector in cmd/cli/command/compose.go
} else if len(buildTags) > 0 {
var bt gcp.BuildTag
--
// CD job
executionName := path.Base(auditLog.GetResourceName())
if cdExecutionNamePattern.MatchString(executionName) {
if auditLog.GetStatus().GetCode() != 0 {
return nil, client.ErrDeploymentFailed{Message: auditLog.GetStatus().GetMessage()}
}
cdSuccess = true
// Report all ready services when CD is successful, prevents cli deploy stop before cd is done
return getReadyServicesCompletedResps(auditLog.GetStatus().GetMessage()), nil // Ignore success cd status when we are waiting for service status
} else {
term.Warnf("unexpected execution name in audit log : %v", executionName)
return nil, nil
}
Script executed:
# Look at stream.go to understand query construction
cat -n src/pkg/cli/client/byoc/gcp/stream.goRepository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
1 package gcp
2
3 import (
4 "context"
5 "errors"
6 "fmt"
7 "io"
8 "path"
9 "regexp"
10 "strings"
11 "time"
12
13 "cloud.google.com/go/logging/apiv2/loggingpb"
14 "github.com/DefangLabs/defang/src/pkg/cli/client"
15 "github.com/DefangLabs/defang/src/pkg/clouds/gcp"
16 "github.com/DefangLabs/defang/src/pkg/term"
17 defangv1 "github.com/DefangLabs/defang/src/protos/io/defang/v1"
18 auditpb "google.golang.org/genproto/googleapis/cloud/audit"
19 "google.golang.org/grpc/codes"
20 "google.golang.org/grpc/status"
21 "google.golang.org/protobuf/types/known/structpb"
22
23 logtype "google.golang.org/genproto/googleapis/logging/type"
24 )
25
26 type LogParser[T any] func(*loggingpb.LogEntry) ([]*T, error)
27 type LogFilter[T any] func(entry T) T
28
29 type GcpLogsClient interface {
30 ListLogEntries(ctx context.Context, query string, order gcp.Order) (gcp.Lister, error)
31 NewTailer(ctx context.Context) (gcp.Tailer, error)
32 GetExecutionEnv(ctx context.Context, executionName string) (map[string]string, error)
33 GetProjectID() gcp.ProjectId
34 GetBuildInfo(ctx context.Context, buildId string) (*gcp.BuildTag, error)
35 }
36
37 type ServerStream[T any] struct {
38 ctx context.Context
39 gcpLogsClient GcpLogsClient
40 parse LogParser[T]
41 filters []LogFilter[*T]
42 query *Query
43 tailer gcp.Tailer
44
45 lastResp *T
46 lastErr error
47 respCh chan *T
48 errCh chan error
49 cancel func()
50 }
51
52 func NewServerStream[T any](ctx context.Context, gcpLogsClient GcpLogsClient, parse LogParser[T], filters ...LogFilter[*T]) (*ServerStream[T], error) {
53 tailer, err := gcpLogsClient.NewTailer(ctx)
54 if err != nil {
55 return nil, err
56 }
57 streamCtx, cancel := context.WithCancel(ctx)
58 return &ServerStream[T]{
59 ctx: streamCtx,
60 gcpLogsClient: gcpLogsClient,
61 parse: parse,
62 filters: filters,
63 tailer: tailer,
64
65 respCh: make(chan *T),
66 errCh: make(chan error),
67 cancel: cancel,
68 }, nil
69 }
70
71 func (s *ServerStream[T]) Close() error {
72 s.cancel()
73 s.tailer.Close() // Close the grpc connection
74 return nil
75 }
76
77 func (s *ServerStream[T]) Receive() bool {
78 select {
79 case s.lastResp = <-s.respCh:
80 return true
81 case err := <-s.errCh:
82 if context.Cause(s.ctx) == io.EOF {
83 s.lastErr = nil
84 } else if errors.Is(err, io.EOF) {
85 s.lastErr = nil
86 } else if isContextCanceledError(err) {
87 s.lastErr = context.Cause(s.ctx)
88 } else {
89 s.lastErr = err
90 }
91 return false
92 }
93 }
94
95 func isContextCanceledError(err error) bool {
96 if errors.Is(err, context.Canceled) {
97 return true
98 }
99 if st, ok := status.FromError(err); ok {
100 return st.Code() == codes.Canceled
101 }
102 return false
103 }
104
105 func (s *ServerStream[T]) StartFollow(start time.Time) {
106 query := s.query.GetQuery()
107 term.Debugf("Query and tail logs since %v with query: \n%v", start, query)
108 go func() {
109 // Only query older logs if start time is more than 10ms ago
110 if !start.IsZero() && start.Unix() > 0 && time.Since(start) > 10*time.Millisecond {
111 s.queryHead(query, 0)
112 }
113
114 // Start tailing logs after all older logs are processed
115 if err := s.tailer.Start(s.ctx, query); err != nil {
116 s.errCh <- err
117 return
118 }
119 for {
120 entry, err := s.tailer.Next(s.ctx)
121 if err != nil {
122 s.errCh <- err
123 return
124 }
125 resps, err := s.parseAndFilter(entry)
126 if err != nil {
127 s.errCh <- err
128 return
129 }
130 for _, resp := range resps {
131 s.respCh <- resp
132 }
133 }
134 }()
135 }
136
137 func (s *ServerStream[T]) StartHead(limit int32) {
138 query := s.query.GetQuery()
139 term.Debugf("Query logs with query: \n%v", query)
140 go func() {
141 s.queryHead(query, limit)
142 }()
143 }
144
145 func (s *ServerStream[T]) StartTail(limit int32) {
146 query := s.query.GetQuery()
147 term.Debugf("Query logs with query: \n%v", query)
148 go func() {
149 s.queryTail(query, limit)
150 }()
151 }
152
153 func (s *ServerStream[T]) queryHead(query string, limit int32) {
154 lister, err := s.gcpLogsClient.ListLogEntries(s.ctx, query, gcp.OrderAscending)
155 if err != nil {
156 s.errCh <- err
157 return
158 }
159 if limit == 0 {
160 err = s.listToChannel(lister)
161 if err != nil && !errors.Is(err, io.EOF) { // Ignore EOF for listing older logs, to proceed to tailing
162 s.errCh <- err
163 return
164 }
165 } else {
166 buffer, err := s.listToBuffer(lister, limit)
167 if err != nil {
168 s.errCh <- err
169 }
170 for i := range buffer {
171 s.respCh <- buffer[i]
172 }
173 s.errCh <- io.EOF
174 }
175 }
176
177 func (s *ServerStream[T]) queryTail(query string, limit int32) {
178 lister, err := s.gcpLogsClient.ListLogEntries(s.ctx, query, gcp.OrderDescending)
179 if err != nil {
180 s.errCh <- err
181 return
182 }
183 if limit == 0 {
184 err = s.listToChannel(lister)
185 if err != nil {
186 s.errCh <- err
187 return
188 }
189 } else {
190 buffer, err := s.listToBuffer(lister, limit)
191 if err != nil {
192 s.errCh <- err
193 }
194 // iterate over the buffer in reverse order to send the oldest resps first
195 for i := len(buffer) - 1; i >= 0; i-- {
196 s.respCh <- buffer[i]
197 }
198 s.errCh <- io.EOF
199 }
200 }
201
202 func (s *ServerStream[T]) listToBuffer(lister gcp.Lister, limit int32) ([]*T, error) {
203 received := 0
204 buffer := make([]*T, 0, limit)
205 for range limit {
206 entry, err := lister.Next()
207 if err != nil {
208 if errors.Is(err, io.EOF) {
209 return buffer, nil
210 }
211 return nil, err
212 }
213 resps, err := s.parseAndFilter(entry)
214 if err != nil {
215 return nil, err
216 }
217 buffer = append(buffer, resps...)
218 received += len(resps)
219 }
220 return buffer, nil
221 }
222
223 func (s *ServerStream[T]) listToChannel(lister gcp.Lister) error {
224 for {
225 entry, err := lister.Next()
226 if err != nil {
227 return err
228 }
229 resps, err := s.parseAndFilter(entry)
230 if err != nil {
231 return err
232 }
233 for _, resp := range resps {
234 s.respCh <- resp
235 }
236 }
237 }
238
239 func (s *ServerStream[T]) parseAndFilter(entry *loggingpb.LogEntry) ([]*T, error) {
240 resps, err := s.parse(entry)
241 if err != nil {
242 return nil, err
243 }
244 newResps := make([]*T, 0, len(resps))
245 for _, resp := range resps {
246 include := true
247 for _, f := range s.filters {
248 if resp = f(resp); resp == nil {
249 include = false
250 break
251 }
252 }
253 if include {
254 newResps = append(newResps, resp)
255 }
256 }
257 return newResps, nil
258 }
259
260 func (s *ServerStream[T]) AddCustomQuery(query string) {
261 s.query.AddQuery(query)
262 }
263
264 func (s *ServerStream[T]) GetQuery() string {
265 return s.query.GetQuery()
266 }
267
268 func (s *ServerStream[T]) Err() error {
269 return s.lastErr
270 }
271
272 func (s *ServerStream[T]) Msg() *T {
273 return s.lastResp
274 }
275
276 type LogStream struct {
277 *ServerStream[defangv1.TailResponse]
278 }
279
280 func NewLogStream(ctx context.Context, gcpLogsClient GcpLogsClient, services []string) (*LogStream, error) {
281 restoreServiceName := getServiceNameRestorer(services, gcp.SafeLabelValue,
282 func(entry *defangv1.TailResponse) string { return entry.Service },
283 func(entry *defangv1.TailResponse, name string) *defangv1.TailResponse {
284 entry.Service = name
285 return entry
286 })
287
288 ss, err := NewServerStream(ctx, gcpLogsClient, getLogEntryParser(ctx, gcpLogsClient), restoreServiceName)
289 if err != nil {
290 return nil, err
291 }
292
293 ss.query = NewLogQuery(gcpLogsClient.GetProjectID())
294 return &LogStream{ServerStream: ss}, nil
295 }
296
297 func (s *LogStream) AddJobExecutionLog(executionName string) {
298 s.query.AddJobExecutionQuery(executionName)
299 }
300
301 func (s *LogStream) AddJobLog(stack, project, etag string, services []string) {
302 s.query.AddJobLogQuery(stack, project, etag, services)
303 }
304
305 func (s *LogStream) AddServiceLog(stack, project, etag string, services []string) {
306 s.query.AddServiceLogQuery(stack, project, etag, services)
307 s.query.AddComputeEngineLogQuery(stack, project, etag, services)
308 }
309
310 func (s *LogStream) AddCloudBuildLog(stack, project, etag string, services []string) {
311 s.query.AddCloudBuildLogQuery(stack, project, etag, services)
312 }
313
314 func (s *LogStream) AddSince(start time.Time) {
315 s.query.AddSince(start)
316 }
317
318 func (s *LogStream) AddUntil(end time.Time) {
319 s.query.AddUntil(end)
320 }
321
322 func (s *LogStream) AddFilter(filter string) {
323 s.query.AddFilter(filter)
324 }
325
326 type SubscribeStream struct {
327 *ServerStream[defangv1.SubscribeResponse]
328 }
329
330 func getServiceNameRestorer[T any](services []string, encode func(string) string, extract func(T) string, update func(T, string) T) func(T) T {
331 mapping := make(map[string]string, len(services))
332 for _, service := range services {
333 mapping[encode(service)] = service
334 }
335 return func(entry T) T {
336 name := extract(entry)
337 if restored, ok := mapping[name]; ok {
338 name = restored
339 }
340 return update(entry, name)
341 }
342 }
343
344 func NewSubscribeStream(ctx context.Context, driver GcpLogsClient, waitForCD bool, etag string, services []string, filters ...LogFilter[*defangv1.SubscribeResponse]) (*SubscribeStream, error) {
345 filters = append(filters, getServiceNameRestorer(services, gcp.SafeLabelValue,
346 func(entry *defangv1.SubscribeResponse) string { return entry.Name },
347 func(entry *defangv1.SubscribeResponse, name string) *defangv1.SubscribeResponse {
348 entry.Name = name
349 return entry
350 }),
351 )
352
353 ss, err := NewServerStream(ctx, driver, getActivityParser(ctx, driver, waitForCD, etag), filters...)
354 if err != nil {
355 return nil, err
356 }
357 ss.query = NewSubscribeQuery()
358 return &SubscribeStream{ServerStream: ss}, nil
359 }
360
361 func (s *SubscribeStream) AddJobExecutionUpdate(executionName string) {
362 s.query.AddJobExecutionUpdateQuery(executionName)
363 }
364
365 func (s *SubscribeStream) AddJobStatusUpdate(stack, project, etag string, services []string) {
366 s.query.AddJobStatusUpdateRequestQuery(stack, project, etag, services)
367 s.query.AddJobStatusUpdateResponseQuery(stack, project, etag, services)
368 }
369
370 func (s *SubscribeStream) AddServiceStatusUpdate(stack, project, etag string, services []string) {
371 s.query.AddServiceStatusRequestUpdate(stack, project, etag, services)
372 s.query.AddServiceStatusReponseUpdate(stack, project, etag, services)
373 s.query.AddComputeEngineInstanceGroupInsertOrPatch(stack, project, etag, services)
374 s.query.AddComputeEngineInstanceGroupAddInstances()
375 s.query.AddCloudBuildActivityQuery()
376 }
377
378 var cdExecutionNamePattern = regexp.MustCompile(`^defang-cd-[a-z0-9]{5}$`)
379
380 func getLogEntryParser(ctx context.Context, gcpClient GcpLogsClient) func(entry *loggingpb.LogEntry) ([]*defangv1.TailResponse, error) {
381 envCache := make(map[string]map[string]string)
382 cdStarted := false
383 return func(entry *loggingpb.LogEntry) ([]*defangv1.TailResponse, error) {
384 if entry == nil {
385 return nil, nil
386 }
387
388 msg := entry.GetTextPayload()
389 if msg == "" && entry.GetJsonPayload() != nil {
390 msg = entry.GetJsonPayload().GetFields()["message"].GetStringValue()
391 }
392 var stderr bool
393 if entry.LogName != "" {
394 stderr = strings.HasSuffix(entry.LogName, "run.googleapis.com%2Fstderr")
395 } else if entry.GetJsonPayload() != nil && entry.GetJsonPayload().GetFields()["cos.googleapis.com/stream"] != nil {
396 stderr = entry.GetJsonPayload().GetFields()["cos.googleapis.com/stream"].GetStringValue() == "stderr"
397 }
398 if strings.Contains(strings.ToLower(msg), "error:") {
399 stderr = true
400 }
401
402 var serviceName, etag, host string
403 var buildTags []string
404 serviceName = entry.Labels["defang-service"]
405 executionName := entry.Labels["run.googleapis.com/execution_name"]
406 if entry.Labels["build_tags"] != "" {
407 buildTags = strings.Split(entry.Labels["build_tags"], ",")
408 }
409 // Log from service
410 if serviceName != "" {
411 etag = entry.Labels["defang-etag"]
412 host = entry.Labels["instanceId"] // cloudrun instance
413 if host == "" {
414 host = entry.Resource.Labels["instance_id"] // compute engine instance
415 }
416 if len(host) > 8 {
417 host = host[:8]
418 }
419 // kaniko build job
420 if regexp.MustCompile(`-build-[a-z0-9]{7}-[a-z0-9]{8}$`).MatchString(executionName) {
421 serviceName += "-image"
422 }
423 } else if executionName != "" {
424 env, ok := envCache[executionName]
425 if !ok {
426 var err error
427 env, err = gcpClient.GetExecutionEnv(ctx, executionName)
428 if err != nil {
429 return nil, fmt.Errorf("failed to get execution environment variables: %w", err)
430 }
431 envCache[executionName] = env
432 }
433
434 if cdExecutionNamePattern.MatchString(executionName) { // Special CD case
435 serviceName = "cd"
436 } else {
437 serviceName = env["DEFANG_SERVICE"]
438 }
439
440 // use kaniko build job environment to get etag
441 etag = env["DEFANG_ETAG"]
442 host = "pulumi" // Hardcoded to match end condition detector in cmd/cli/command/compose.go
443 } else if len(buildTags) > 0 {
444 var bt gcp.BuildTag
445 if err := bt.Parse(buildTags); err != nil {
446 return nil, err
447 }
448 serviceName = bt.Service
449 etag = bt.Etag
450 host = "cloudbuild"
451 if bt.IsDefangCD {
452 host = "pulumi"
453 }
454 // HACK: Detect cd start from cloudbuild logs to skip the cloud build image pulling logs
455 // " ** " or "Defang: " could come first in the log message when cd starts
456 if strings.HasPrefix(msg, " ** ") || strings.HasPrefix(msg, "Defang: ") {
457 cdStarted = true
458 }
459 if !cdStarted {
460 return nil, nil // Skip cloudbuild logs (like pulling cd image) before cd started
461 }
462 } else {
463 var err error
464 _, msg, err = LogEntryToString(entry)
465 if err != nil {
466 return nil, err
467 }
468 }
469
470 return []*defangv1.TailResponse{
471 {
472 Service: serviceName,
473 Etag: etag,
474 Entries: []*defangv1.LogEntry{
475 {
476 Message: msg,
477 Timestamp: entry.Timestamp,
478 Etag: etag,
479 Service: serviceName,
480 Host: host,
481 Stderr: stderr,
482 },
483 },
484 },
485 }, nil
486 }
487 }
488
489 const defangCD = "#defang-cd" // Special service name for CD, # is used to avoid conflict with service names
490
491 func getActivityParser(ctx context.Context, gcpLogsClient GcpLogsClient, waitForCD bool, etag string) func(entry *loggingpb.LogEntry) ([]*defangv1.SubscribeResponse, error) {
492 cdSuccess := false
493 readyServices := make(map[string]string)
494
495 computeEngineRootTriggers := make(map[string]string)
496
497 getReadyServicesCompletedResps := func(cdStatus string) []*defangv1.SubscribeResponse {
498 resps := make([]*defangv1.SubscribeResponse, 0, len(readyServices))
499 for serviceName, status := range readyServices {
500 resps = append(resps, &defangv1.SubscribeResponse{
501 Name: serviceName,
502 State: defangv1.ServiceState_DEPLOYMENT_COMPLETED,
503 Status: status,
504 })
505 }
506 resps = append(resps, &defangv1.SubscribeResponse{
507 Name: defangCD,
508 State: defangv1.ServiceState_DEPLOYMENT_COMPLETED,
509 Status: cdStatus,
510 })
511 return resps
512 }
513
514 return func(entry *loggingpb.LogEntry) ([]*defangv1.SubscribeResponse, error) {
515 if entry == nil {
516 return nil, nil
517 }
518
519 if entry.GetProtoPayload().GetTypeUrl() != "type.googleapis.com/google.cloud.audit.AuditLog" {
520 term.Warnf("unexpected log entry type : %v", entry.GetProtoPayload().GetTypeUrl())
521 return nil, nil
522 }
523
524 auditLog := new(auditpb.AuditLog)
525 if err := entry.GetProtoPayload().UnmarshalTo(auditLog); err != nil {
526 term.Warnf("failed to unmarshal audit log : %v", err)
527 return nil, nil
528 }
529
530 switch entry.Resource.Type {
531 case "cloud_run_revision": // Service status
532 if request := auditLog.GetRequest(); request != nil { // Activity log: service update requests
533 serviceName := GetValueInStruct(request, "service.template.labels.defang-service")
534 return []*defangv1.SubscribeResponse{{
535 Name: serviceName,
536 State: defangv1.ServiceState_DEPLOYMENT_PENDING,
537 Status: GetValueInStruct(request, "methodName"),
538 }}, nil
539 } else if response := auditLog.GetResponse(); response != nil { // System log: service status update
540 serviceName := GetValueInStruct(response, "spec.template.metadata.labels.defang-service")
541 status := auditLog.GetStatus()
542 if status == nil {
543 return nil, errors.New("missing status in audit log for service " + serviceName)
544 }
545 var state defangv1.ServiceState
546 if status.GetCode() == 0 {
547 if cdSuccess || !waitForCD {
548 state = defangv1.ServiceState_DEPLOYMENT_COMPLETED
549 } else {
550 state = defangv1.ServiceState_DEPLOYMENT_PENDING // Report later
551 readyServices[serviceName] = status.GetMessage()
552 }
553 } else {
554 state = defangv1.ServiceState_DEPLOYMENT_FAILED
555 }
556 return []*defangv1.SubscribeResponse{{
557 Name: serviceName,
558 State: state,
559 Status: status.GetMessage(),
560 }}, nil
561 } else {
562 term.Warnf("missing request and response in audit log for service %v", path.Base(auditLog.GetResourceName()))
563 return nil, nil
564 }
565
566 // etag is at protoPayload.response.spec.template.metadata.labels.defang-etag
567 // etag := getValueInStruct(auditLog.GetResponse(), "spec.template.metadata.labels.defang-etag") // etag not needed
568 // service.spec.template.metadata.labels."defang-service"
569
570 case "cloud_run_job": // Job execution update
571 // Kaniko job
572 if request := auditLog.GetRequest(); request != nil { // Acitivity log: job creation
573 serviceName := GetValueInStruct(request, "job.template.labels.defang-service")
574 if serviceName != "" {
575 return []*defangv1.SubscribeResponse{{
576 Name: serviceName,
577 State: defangv1.ServiceState_BUILD_ACTIVATING,
578 Status: "Building job creating",
579 }}, nil
580 }
581 } else if response := auditLog.GetResponse(); response != nil { // System log: job status update
582 serviceName := GetValueInStruct(response, "spec.template.metadata.labels.defang-service")
583 status := auditLog.GetStatus()
584 if status == nil {
585 term.Warnf("missing status in audit log for job %v", path.Base(auditLog.GetResourceName()))
586 return nil, nil
587 }
588 var state defangv1.ServiceState
589 if status.GetCode() == 0 {
590 state = defangv1.ServiceState_BUILD_STOPPING
591 } else {
592 state = defangv1.ServiceState_DEPLOYMENT_FAILED
593 }
594 if serviceName != "" {
595 return []*defangv1.SubscribeResponse{{
596 Name: serviceName,
597 State: state,
598 Status: status.GetMessage(),
599 }}, nil
600 }
601 }
602
603 // CD job
604 executionName := path.Base(auditLog.GetResourceName())
605 if cdExecutionNamePattern.MatchString(executionName) {
606 if auditLog.GetStatus().GetCode() != 0 {
607 return nil, client.ErrDeploymentFailed{Message: auditLog.GetStatus().GetMessage()}
608 }
609 cdSuccess = true
610 // Report all ready services when CD is successful, prevents cli deploy stop before cd is done
611 return getReadyServicesCompletedResps(auditLog.GetStatus().GetMessage()), nil // Ignore success cd status when we are waiting for service status
612 } else {
613 term.Warnf("unexpected execution name in audit log : %v", executionName)
614 return nil, nil
615 }
616 case "gce_instance_group_manager": // Compute engine update start
617 request := auditLog.GetRequest()
618 if request == nil {
619 term.Warnf("missing request in audit log for instance group manager %v", path.Base(auditLog.GetResourceName()))
620 return nil, nil
621 }
622 labels := GetListInStruct(request, "allInstancesConfig.properties.labels")
623 if labels == nil {
624 term.Warnf("missing labels in audit log for instance group manager %v", path.Base(auditLog.GetResourceName()))
625 return nil, nil
626 }
627 // Find the service name from the labels
628 serviceName := ""
629 for _, label := range labels {
630 fields := label.GetStructValue().GetFields()
631 if fields["key"].GetStringValue() == "defang-service" {
632 serviceName = fields["value"].GetStringValue()
633 break
634 }
635 }
636 if serviceName == "" {
637 term.Warnf("missing defang-service label in audit log for instance group manager %v", path.Base(auditLog.GetResourceName()))
638 return nil, nil
639 }
640 rootTriggerId := entry.GetLabels()["compute.googleapis.com/root_trigger_id"]
641 if rootTriggerId == "" {
642 term.Warnf("missing root_trigger_id in audit log for instance group manager %v", path.Base(auditLog.GetResourceName()))
643 } else {
644 computeEngineRootTriggers[rootTriggerId] = serviceName
645 }
646 return []*defangv1.SubscribeResponse{{
647 Name: serviceName,
648 State: defangv1.ServiceState_DEPLOYMENT_PENDING,
649 Status: auditLog.GetResponse().GetFields()["status"].GetStringValue(),
650 }}, nil
651 case "gce_instance_group": // Compute engine update end
652 // TODO: Better handle of multiple instance group insert events for the same service where more than 1 replica is created, all of them would have 100% for progress and DONE as status
653 rootTriggerId := entry.GetLabels()["compute.googleapis.com/root_trigger_id"]
654 serviceName, ok := computeEngineRootTriggers[rootTriggerId]
655 if !ok {
656 term.Debugf("ignored root trigger id %v for instance group insert", rootTriggerId)
657 return nil, nil
658 }
659 response := auditLog.GetResponse()
660 if response == nil {
661 term.Warnf("missing response in audit log for instance group %v", path.Base(auditLog.GetResourceName()))
662 return nil, nil
663 }
664 status := response.GetFields()["status"].GetStringValue()
665 var state defangv1.ServiceState
666 switch status {
667 case "DONE":
668 state = defangv1.ServiceState_DEPLOYMENT_COMPLETED
669 case "RUNNING":
670 state = defangv1.ServiceState_DEPLOYMENT_PENDING
671 default:
672 state = defangv1.ServiceState_DEPLOYMENT_FAILED
673 }
674 return []*defangv1.SubscribeResponse{{
675 Name: serviceName,
676 State: state,
677 Status: status,
678 }}, nil
679 // TODO: Add cloud build activities for building status update
680 case "build": // Cloudbuild events
681 buildId := entry.Resource.Labels["build_id"]
682 if buildId == "" {
683 return nil, nil // Ignore activities without build id
684 }
685 bt, err := gcpLogsClient.GetBuildInfo(ctx, buildId) // TODO: Cache the build IDs?
686 if err != nil {
687 term.Warnf("failed to get build tag for build %v: %v", buildId, err)
688 return nil, nil
689 }
690
691 if etag != "" && bt.Etag != etag {
692 return nil, nil
693 }
694
695 if bt.IsDefangCD {
696 if !entry.Operation.Last { // Ignore non-final cloud build event for CD
697 return nil, nil
698 }
699 // When cloud build fails, the last log message is an error message
700 if entry.Severity == logtype.LogSeverity_ERROR {
701 msg := ""
702 if auditLog.GetStatus() != nil {
703 msg = auditLog.GetStatus().GetMessage()
704 }
705 return nil, client.ErrDeploymentFailed{Message: msg}
706 }
707
708 cdSuccess = true
709 status := ""
710 if auditLog.GetStatus() != nil {
711 status = auditLog.GetStatus().String()
712 }
713 return getReadyServicesCompletedResps(status), nil
714 } else {
715 var state defangv1.ServiceState
716 status := ""
717 if entry.Operation.First {
718 state = defangv1.ServiceState_BUILD_ACTIVATING
719 } else if entry.Operation.Last {
720 if entry.Severity == logtype.LogSeverity_ERROR {
721 state = defangv1.ServiceState_BUILD_FAILED
722 if auditLog.GetStatus() != nil {
723 status = auditLog.GetStatus().String()
724 }
725 } else {
726 state = defangv1.ServiceState_BUILD_STOPPING
727 }
728 } else {
729 state = defangv1.ServiceState_BUILD_RUNNING
730 }
731 if status == "" {
732 status = state.String()
733 }
734 return []*defangv1.SubscribeResponse{{
735 Name: bt.Service,
736 State: state,
737 Status: status,
738 }}, nil
739 }
740 default:
741 term.Warnf("unexpected resource type : %v", entry.Resource.Type)
742 return nil, nil
743 }
744 }
745 }
746
747 // Extract a string value from a nested structpb.Struct
748 func GetValueInStruct(s *structpb.Struct, path string) string {
749 keys := strings.Split(path, ".")
750 for len(keys) > 0 {
751 if s == nil {
752 return ""
753 }
754 key := keys[0]
755 field := s.GetFields()[key]
756 if s = field.GetStructValue(); s == nil {
757 return field.GetStringValue()
758 }
759 keys = keys[1:]
760 }
761 return ""
762 }
763
764 func GetListInStruct(s *structpb.Struct, path string) []*structpb.Value {
765 keys := strings.Split(path, ".")
766 for len(keys) > 0 {
767 if s == nil {
768 return nil
769 }
770 key := keys[0]
771 field := s.GetFields()[key]
772 if s = field.GetStructValue(); s == nil {
773 return field.GetListValue().Values
774 }
775 keys = keys[1:]
776 }
777 return nil
778 }
Script executed:
# Check what parameters are passed in the test
cat -n src/pkg/cli/client/byoc/gcp/byoc.go | grep -A 30 "getLogStream"Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
554 return b.getLogStream(ctx, b.driver, req)
555 }
556
557 func (b *ByocGcp) getLogStream(ctx context.Context, gcpLogsClient GcpLogsClient, req *defangv1.TailRequest) (client.ServerStream[defangv1.TailResponse], error) {
558 logStream, err := NewLogStream(ctx, gcpLogsClient, req.Services)
559 if err != nil {
560 return nil, err
561 }
562
563 if req.Since.IsValid() {
564 logStream.AddSince(req.Since.AsTime())
565 }
566 if req.Until.IsValid() {
567 logStream.AddUntil(req.Until.AsTime())
568 }
569 etag := req.Etag
570 if etag == b.cdExecution { // Do not pass the cd execution name as etag
571 etag = ""
572 }
573 if logs.LogType(req.LogType).Has(logs.LogTypeBuild) {
574 execName := path.Base(b.cdExecution)
575 if execName == "." {
576 execName = ""
577 }
578 logStream.AddJobExecutionLog(execName) // CD log when there is an execution name
579 logStream.AddJobLog(b.PulumiStack, req.Project, etag, req.Services) // Kaniko or CD logs when there is no execution name
580 logStream.AddCloudBuildLog(b.PulumiStack, req.Project, etag, req.Services) // CloudBuild logs
581 }
582 if logs.LogType(req.LogType).Has(logs.LogTypeRun) {
583 logStream.AddServiceLog(b.PulumiStack, req.Project, etag, req.Services) // Service logs
584 }
585 logStream.AddFilter(req.Pattern)
586 if req.Follow {
587 logStream.StartFollow(req.Since.AsTime())
Script executed:
# Look at how PulumiStack is set
rg "PulumiStack" src/pkg/cli/client/byoc/gcp/ -B 3 -A 3Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
src/pkg/cli/client/byoc/gcp/byoc.go- "PULUMI_COPILOT": "false",
src/pkg/cli/client/byoc/gcp/byoc.go- "PULUMI_SKIP_UPDATE_CHECK": "true",
src/pkg/cli/client/byoc/gcp/byoc.go- "REGION": b.driver.Region,
src/pkg/cli/client/byoc/gcp/byoc.go: "STACK": b.PulumiStack,
src/pkg/cli/client/byoc/gcp/byoc.go- pulumiBackendKey: pulumiBackendValue, // TODO: make secret
src/pkg/cli/client/byoc/gcp/byoc.go- }
src/pkg/cli/client/byoc/gcp/byoc.go-
--
src/pkg/cli/client/byoc/gcp/byoc.go- Steps: string(steps),
src/pkg/cli/client/byoc/gcp/byoc.go- ServiceAccount: &b.cdServiceAccount,
src/pkg/cli/client/byoc/gcp/byoc.go- Tags: []string{
src/pkg/cli/client/byoc/gcp/byoc.go: fmt.Sprintf("%v_%v_%v_%v", b.PulumiStack, cmd.project, "cd", cmd.etag), // For cd logs, consistent with cloud build tagging
src/pkg/cli/client/byoc/gcp/byoc.go- "defang-cd", // To indicate this is the actual cd service
src/pkg/cli/client/byoc/gcp/byoc.go- },
src/pkg/cli/client/byoc/gcp/byoc.go- })
--
src/pkg/cli/client/byoc/gcp/byoc.go- subscribeStream.AddJobExecutionUpdate(path.Base(b.cdExecution))
src/pkg/cli/client/byoc/gcp/byoc.go- }
src/pkg/cli/client/byoc/gcp/byoc.go-
src/pkg/cli/client/byoc/gcp/byoc.go: // TODO: update stack (1st param) to b.PulumiStack
src/pkg/cli/client/byoc/gcp/byoc.go: subscribeStream.AddJobStatusUpdate(b.PulumiStack, req.Project, req.Etag, req.Services)
src/pkg/cli/client/byoc/gcp/byoc.go: subscribeStream.AddServiceStatusUpdate(b.PulumiStack, req.Project, req.Etag, req.Services)
src/pkg/cli/client/byoc/gcp/byoc.go- subscribeStream.StartFollow(time.Now())
src/pkg/cli/client/byoc/gcp/byoc.go- return subscribeStream, nil
src/pkg/cli/client/byoc/gcp/byoc.go-}
--
src/pkg/cli/client/byoc/gcp/byoc.go- execName = ""
src/pkg/cli/client/byoc/gcp/byoc.go- }
src/pkg/cli/client/byoc/gcp/byoc.go- logStream.AddJobExecutionLog(execName) // CD log when there is an execution name
src/pkg/cli/client/byoc/gcp/byoc.go: logStream.AddJobLog(b.PulumiStack, req.Project, etag, req.Services) // Kaniko or CD logs when there is no execution name
src/pkg/cli/client/byoc/gcp/byoc.go: logStream.AddCloudBuildLog(b.PulumiStack, req.Project, etag, req.Services) // CloudBuild logs
src/pkg/cli/client/byoc/gcp/byoc.go- }
src/pkg/cli/client/byoc/gcp/byoc.go- if logs.LogType(req.LogType).Has(logs.LogTypeRun) {
src/pkg/cli/client/byoc/gcp/byoc.go: logStream.AddServiceLog(b.PulumiStack, req.Project, etag, req.Services) // Service logs
src/pkg/cli/client/byoc/gcp/byoc.go- }
src/pkg/cli/client/byoc/gcp/byoc.go- logStream.AddFilter(req.Pattern)
src/pkg/cli/client/byoc/gcp/byoc.go- if req.Follow {
--
src/pkg/cli/client/byoc/gcp/byoc.go- query.AddJobExecutionQuery(path.Base(b.cdExecution))
src/pkg/cli/client/byoc/gcp/byoc.go- }
src/pkg/cli/client/byoc/gcp/byoc.go-
src/pkg/cli/client/byoc/gcp/byoc.go: query.AddJobLogQuery(b.PulumiStack, req.Project, req.Etag, req.Services) // Kaniko OR CD logs
src/pkg/cli/client/byoc/gcp/byoc.go: query.AddServiceLogQuery(b.PulumiStack, req.Project, req.Etag, req.Services) // Cloudrun service logs
src/pkg/cli/client/byoc/gcp/byoc.go: query.AddCloudBuildLogQuery(b.PulumiStack, req.Project, req.Etag, req.Services) // CloudBuild logs
src/pkg/cli/client/byoc/gcp/byoc.go- query.AddSince(since)
src/pkg/cli/client/byoc/gcp/byoc.go- query.AddUntil(until)
src/pkg/cli/client/byoc/gcp/byoc.go-
src/pkg/cli/client/byoc/gcp/byoc.go- // Service status updates
src/pkg/cli/client/byoc/gcp/byoc.go: query.AddJobStatusUpdateRequestQuery(b.PulumiStack, req.Project, req.Etag, req.Services)
src/pkg/cli/client/byoc/gcp/byoc.go: query.AddJobStatusUpdateResponseQuery(b.PulumiStack, req.Project, req.Etag, req.Services)
src/pkg/cli/client/byoc/gcp/byoc.go: query.AddServiceStatusRequestUpdate(b.PulumiStack, req.Project, req.Etag, req.Services)
src/pkg/cli/client/byoc/gcp/byoc.go: query.AddServiceStatusReponseUpdate(b.PulumiStack, req.Project, req.Etag, req.Services)
src/pkg/cli/client/byoc/gcp/byoc.go-
src/pkg/cli/client/byoc/gcp/byoc.go- return query.GetQuery()
src/pkg/cli/client/byoc/gcp/byoc.go-}
--
src/pkg/cli/client/byoc/gcp/byoc.go- if b.Prefix != "" {
src/pkg/cli/client/byoc/gcp/byoc.go- parts = []string{b.Prefix}
src/pkg/cli/client/byoc/gcp/byoc.go- }
src/pkg/cli/client/byoc/gcp/byoc.go: return strings.Join(append(parts, projectName, b.PulumiStack, name), "_") // same as fullDefangResourceName in gcpcd/up.go
src/pkg/cli/client/byoc/gcp/byoc.go-}
src/pkg/cli/client/byoc/gcp/byoc.go-
src/pkg/cli/client/byoc/gcp/byoc.go-func (*ByocGcp) GetPrivateDomain(projectName string) string {
Script executed:
# Check what RE2 does with [a-zA-Z0-9-_] char classes
python3 << 'EOF'
import re
# RE2 is Go's regex engine, which uses standard char class rules
# Let's test what [a-zA-Z0-9-_] actually matches
pattern1 = r'[a-zA-Z0-9-_]'
pattern2 = r'[a-zA-Z0-9_-]'
test_strings = [
'a', 'A', '0', '9', '-', '_', '.', ':',
chr(46), # .
chr(47), # /
chr(45), # -
chr(46), # .
chr(95), # _
]
print("Pattern [a-zA-Z0-9-_]:")
for s in test_strings:
if re.match(pattern1, s):
print(f" Matches: {repr(s)} (ord={ord(s)})")
print("\nPattern [a-zA-Z0-9_-]:")
for s in test_strings:
if re.match(pattern2, s):
print(f" Matches: {repr(s)} (ord={ord(s)})")
# Check if - in the middle creates a range
print("\nChecking if '-_' creates a range:")
print(f" '-' is ord {ord('-')}, '_' is ord {ord('_')}")
print(f" Characters between - (45) and _ (95):")
print(f" {[chr(i) for i in range(45, 96)]}")
EOFRepository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
Pattern [a-zA-Z0-9-_]:
Matches: 'a' (ord=97)
Matches: 'A' (ord=65)
Matches: '0' (ord=48)
Matches: '9' (ord=57)
Matches: '-' (ord=45)
Matches: '_' (ord=95)
Matches: '-' (ord=45)
Matches: '_' (ord=95)
Pattern [a-zA-Z0-9_-]:
Matches: 'a' (ord=97)
Matches: 'A' (ord=65)
Matches: '0' (ord=48)
Matches: '9' (ord=57)
Matches: '-' (ord=45)
Matches: '_' (ord=95)
Matches: '-' (ord=45)
Matches: '_' (ord=95)
Checking if '-_' creates a range:
'-' is ord 45, '_' is ord 95
Characters between - (45) and _ (95):
['-', '.', '/', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', ':', ';', '<', '=', '>', '?', '@', 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', '[', '\\', ']', '^', '_']
Script executed:
# Find the test case definition for with_cd_exec
rg -A 10 '"with_cd_exec"' src/pkg/cli/client/byoc/gcp/byoc_test.go | head -30Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
{name: "with_cd_exec", req: &defangv1.TailRequest{
LogType: uint32(logs.LogTypeAll),
},
cdExecution: "test-execution-id",
},
{name: "with_etag", req: &defangv1.TailRequest{
LogType: uint32(logs.LogTypeAll),
Etag: "test-etag",
}},
{name: "with_etag_and_since", req: &defangv1.TailRequest{
Since: timestamppb.New(time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)),
Script executed:
# Check the getLogStream test to see what values are used
rg -B 5 "with_cd_exec.*query" src/pkg/cli/client/byoc/gcp/byoc_test.go -A 2Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 1
Script executed:
# Look more carefully at the test structure and what gets passed to AddCloudBuildLogQuery
cat -n src/pkg/cli/client/byoc/gcp/byoc_test.go | grep -A 50 "TestGetLogStream"Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
163 func TestGetLogStream(t *testing.T) {
164 tests := []struct {
165 name string
166 req *defangv1.TailRequest
167 cdExecution string
168 }{
169 // TODO: use golang 1.25 synctest to avoid needing a fixed Since in every test case
170 {name: "no_args", req: &defangv1.TailRequest{}},
171 {name: "since", req: &defangv1.TailRequest{Since: timestamppb.New(time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC))}},
172 {name: "since_and_until", req: &defangv1.TailRequest{
173 Since: timestamppb.New(time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)),
174 Until: timestamppb.New(time.Date(2024, 1, 2, 0, 0, 0, 0, time.UTC)),
175 }},
176 {name: "with_pattern", req: &defangv1.TailRequest{
177 Pattern: "error",
178 }},
179 {name: "with_pattern_since_and_until", req: &defangv1.TailRequest{
180 Pattern: "error",
181 Since: timestamppb.New(time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)),
182 Until: timestamppb.New(time.Date(2024, 1, 2, 0, 0, 0, 0, time.UTC)),
183 }},
184 {name: "with_project", req: &defangv1.TailRequest{
185 Project: "test-project",
186 LogType: uint32(logs.LogTypeAll),
187 }},
188 {name: "with_project_since_and_until", req: &defangv1.TailRequest{
189 Project: "test-project",
190 Since: timestamppb.New(time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)),
191 Until: timestamppb.New(time.Date(2024, 1, 2, 0, 0, 0, 0, time.UTC)),
192 LogType: uint32(logs.LogTypeAll),
193 }},
194 {name: "with_logtype_build", req: &defangv1.TailRequest{
195 LogType: uint32(logs.LogTypeBuild),
196 }},
197 {name: "with_logtype_run", req: &defangv1.TailRequest{
198 LogType: uint32(logs.LogTypeRun),
199 }},
200 {name: "with_logtype_all", req: &defangv1.TailRequest{
201 Pattern: "error",
202 LogType: uint32(logs.LogTypeAll),
203 }},
204 {name: "with_cd_exec", req: &defangv1.TailRequest{
205 LogType: uint32(logs.LogTypeAll),
206 },
207 cdExecution: "test-execution-id",
208 },
209 {name: "with_etag", req: &defangv1.TailRequest{
210 LogType: uint32(logs.LogTypeAll),
211 Etag: "test-etag",
212 }},
213 {name: "with_etag_and_since", req: &defangv1.TailRequest{
Script executed:
# Now let me check how the test is executed - what are the test parameters for b.PulumiStack and project
rg -A 100 "func TestGetLogStream" src/pkg/cli/client/byoc/gcp/byoc_test.go | grep -A 30 "for _, tt := range"Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
b := NewByocProvider(ctx, "testTenantID", "")
b.cdExecution = tt.cdExecution
driver := &MockGcpLogsClient{
lister: &MockGcpLoggingLister{},
tailer: &MockGcpLoggingTailer{},
}
stream, err := b.getLogStream(ctx, driver, tt.req)
if err != nil {
t.Errorf("getLogStream() error = %v, want nil", err)
}
if stream == nil {
t.Errorf("getLogStream() returned nil tailer, want non-nil")
}
logStream, ok := stream.(*LogStream)
if !ok {
t.Fatalf("getLogStream() returned wrong type, want *gcp.LogStream")
}
query := logStream.GetQuery()
if err := pkg.Compare([]byte(query), "testdata/"+tt.name+".query"); err != nil {
Script executed:
# Check what NewByocProvider sets for PulumiStack in the test
rg -B 5 "NewByocProvider" src/pkg/cli/client/byoc/gcp/byoc_test.go | head -30Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
)
func TestSetUpCD(t *testing.T) {
t.Skip("skipping test")
ctx := t.Context()
b := NewByocProvider(ctx, "testTenantID", "")
--
ctx := t.Context()
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
b := NewByocProvider(ctx, "testTenantID", "")
--
ctx := t.Context()
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
b := NewByocProvider(ctx, "testTenantID", "")
Script executed:
# Find NewByocProvider implementation and what it sets PulumiStack to
rg -A 20 "func NewByocProvider" src/pkg/cli/client/byoc/gcp/ --type go | head -60Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
src/pkg/cli/client/byoc/gcp/byoc.go:func NewByocProvider(ctx context.Context, tenantName types.TenantLabel, stack string) *ByocGcp {
src/pkg/cli/client/byoc/gcp/byoc.go- region := pkg.Getenv("GCP_LOCATION", "us-central1") // Defaults to us-central1 for lower price
src/pkg/cli/client/byoc/gcp/byoc.go- projectId := getGcpProjectID()
src/pkg/cli/client/byoc/gcp/byoc.go- b := &ByocGcp{driver: &gcp.Gcp{Region: region, ProjectId: projectId}}
src/pkg/cli/client/byoc/gcp/byoc.go- b.ByocBaseClient = byoc.NewByocBaseClient(tenantName, b, stack)
src/pkg/cli/client/byoc/gcp/byoc.go- return b
src/pkg/cli/client/byoc/gcp/byoc.go-}
src/pkg/cli/client/byoc/gcp/byoc.go-
src/pkg/cli/client/byoc/gcp/byoc.go-func getGcpProjectID() string {
src/pkg/cli/client/byoc/gcp/byoc.go- projectId, ok := os.LookupEnv("GCP_PROJECT_ID")
src/pkg/cli/client/byoc/gcp/byoc.go- if !ok {
src/pkg/cli/client/byoc/gcp/byoc.go- projectId = os.Getenv("CLOUDSDK_CORE_PROJECT")
src/pkg/cli/client/byoc/gcp/byoc.go- }
src/pkg/cli/client/byoc/gcp/byoc.go- return projectId
src/pkg/cli/client/byoc/gcp/byoc.go-}
src/pkg/cli/client/byoc/gcp/byoc.go-
src/pkg/cli/client/byoc/gcp/byoc.go-func (b *ByocGcp) SetUpCD(ctx context.Context) error {
src/pkg/cli/client/byoc/gcp/byoc.go- if b.setupDone {
src/pkg/cli/client/byoc/gcp/byoc.go- return nil
src/pkg/cli/client/byoc/gcp/byoc.go- }
src/pkg/cli/client/byoc/gcp/byoc.go- // TODO: Handle project creation flow
Script executed:
# Check if PulumiStack is set to a default value like "beta"
rg "PulumiStack.*=" src/pkg/cli/client/byoc/gcp/byoc.go | head -20Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
Script executed:
# Find NewByocBaseClient and see what it does with the stack parameter
rg -A 30 "func NewByocBaseClient" src/pkg/cli/client/byoc/ --type goRepository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
src/pkg/cli/client/byoc/baseclient.go:func NewByocBaseClient(tenantLabel types.TenantLabel, backend ProjectBackend, stack string) *ByocBaseClient {
src/pkg/cli/client/byoc/baseclient.go- if stack == "" {
src/pkg/cli/client/byoc/baseclient.go- stack = stacks.DefaultBeta // backwards compat
src/pkg/cli/client/byoc/baseclient.go- }
src/pkg/cli/client/byoc/baseclient.go- return &ByocBaseClient{
src/pkg/cli/client/byoc/baseclient.go- Prefix: pkg.Getenv("DEFANG_PREFIX", "Defang"), // prefix for all resources created by Defang
src/pkg/cli/client/byoc/baseclient.go- PulumiStack: pkg.Getenv("DEFANG_SUFFIX", stack),
src/pkg/cli/client/byoc/baseclient.go- TenantLabel: tenantLabel,
src/pkg/cli/client/byoc/baseclient.go- projectBackend: backend,
src/pkg/cli/client/byoc/baseclient.go- }
src/pkg/cli/client/byoc/baseclient.go-}
src/pkg/cli/client/byoc/baseclient.go-
src/pkg/cli/client/byoc/baseclient.go-func (b *ByocBaseClient) GetStackName() string {
src/pkg/cli/client/byoc/baseclient.go- return b.PulumiStack
src/pkg/cli/client/byoc/baseclient.go-}
src/pkg/cli/client/byoc/baseclient.go-
src/pkg/cli/client/byoc/baseclient.go-func (b *ByocBaseClient) Debug(context.Context, *defangv1.DebugRequest) (*defangv1.DebugResponse, error) {
src/pkg/cli/client/byoc/baseclient.go- return nil, client.ErrNotImplemented("AI debugging is not yet supported for BYOC")
src/pkg/cli/client/byoc/baseclient.go-}
src/pkg/cli/client/byoc/baseclient.go-
src/pkg/cli/client/byoc/baseclient.go-func (b *ByocBaseClient) SetCanIUseConfig(quotas *defangv1.CanIUseResponse) {
src/pkg/cli/client/byoc/baseclient.go- b.CanIUseConfig.AllowGPU = quotas.Gpu
src/pkg/cli/client/byoc/baseclient.go- b.CanIUseConfig.AllowScaling = quotas.AllowScaling
src/pkg/cli/client/byoc/baseclient.go- b.CanIUseConfig.CDImage = quotas.CdImage
src/pkg/cli/client/byoc/baseclient.go- b.CanIUseConfig.PulumiVersion = quotas.PulumiVersion
src/pkg/cli/client/byoc/baseclient.go-}
src/pkg/cli/client/byoc/baseclient.go-
src/pkg/cli/client/byoc/baseclient.go-// getServiceLabel returns a DNS-safe label for the given service
src/pkg/cli/client/byoc/baseclient.go-func getServiceLabel(serviceName string) string {
src/pkg/cli/client/byoc/baseclient.go- // Technically DNS names can have underscores, but these are reserved for SRV records and some systems have issues with them.
src/pkg/cli/client/byoc/baseclient.go- return dns.SafeLabel(strings.ReplaceAll(serviceName, "_", "-"))
Script executed:
# Check if there's a default stack value
rg "PulumiStack.*:=|PulumiStack.*=" src/pkg/cli/client/byoc/ --type go -B 2 -A 2Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
src/pkg/cli/client/byoc/baseclient.go- // place of "beta", so that Fabric will treat these stacks as if there
src/pkg/cli/client/byoc/baseclient.go- // was no explicit stack.
src/pkg/cli/client/byoc/baseclient.go: if b.PulumiStack == stacks.DefaultBeta {
src/pkg/cli/client/byoc/baseclient.go- return ""
src/pkg/cli/client/byoc/baseclient.go- }
Script executed:
# Look at ByocBaseClient structure
rg "type ByocBaseClient struct" src/pkg/cli/client/byoc/ -A 20 --type goRepository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
src/pkg/cli/client/byoc/baseclient.go:type ByocBaseClient struct {
src/pkg/cli/client/byoc/baseclient.go- client.RetryDelayer
src/pkg/cli/client/byoc/baseclient.go-
src/pkg/cli/client/byoc/baseclient.go- Prefix string
src/pkg/cli/client/byoc/baseclient.go- PulumiStack string
src/pkg/cli/client/byoc/baseclient.go- SetupDone bool
src/pkg/cli/client/byoc/baseclient.go- TenantLabel types.TenantLabel
src/pkg/cli/client/byoc/baseclient.go- CanIUseConfig
src/pkg/cli/client/byoc/baseclient.go-
src/pkg/cli/client/byoc/baseclient.go- projectBackend ProjectBackend
src/pkg/cli/client/byoc/baseclient.go-}
src/pkg/cli/client/byoc/baseclient.go-
src/pkg/cli/client/byoc/baseclient.go-func NewByocBaseClient(tenantLabel types.TenantLabel, backend ProjectBackend, stack string) *ByocBaseClient {
src/pkg/cli/client/byoc/baseclient.go- if stack == "" {
src/pkg/cli/client/byoc/baseclient.go- stack = stacks.DefaultBeta // backwards compat
src/pkg/cli/client/byoc/baseclient.go- }
src/pkg/cli/client/byoc/baseclient.go- return &ByocBaseClient{
src/pkg/cli/client/byoc/baseclient.go- Prefix: pkg.Getenv("DEFANG_PREFIX", "Defang"), // prefix for all resources created by Defang
src/pkg/cli/client/byoc/baseclient.go- PulumiStack: pkg.Getenv("DEFANG_SUFFIX", stack),
src/pkg/cli/client/byoc/baseclient.go- TenantLabel: tenantLabel,
src/pkg/cli/client/byoc/baseclient.go- projectBackend: backend,
Script executed:
# Check what stacks.DefaultBeta is
rg "DefaultBeta" src/ --type go -B 2 -A 2Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
src/pkg/stacks/stacks.go-
src/pkg/stacks/stacks.go-const (
src/pkg/stacks/stacks.go: DefaultBeta = "beta"
src/pkg/stacks/stacks.go- Directory = ".defang"
src/pkg/stacks/stacks.go-)
--
src/pkg/stacks/selector.go-func printStacksInfoMessage(stacks []string) {
src/pkg/stacks/selector.go- // If there is a stack named "beta", print an informational message about it
src/pkg/stacks/selector.go: betaExists := slices.Contains(stacks, DefaultBeta)
src/pkg/stacks/selector.go- if betaExists {
src/pkg/stacks/selector.go- infoLine := "This project was deployed with an implicit Stack called 'beta' before Stacks were introduced."
--
src/pkg/stacks/manager.go- stackName := deployment.GetStack()
src/pkg/stacks/manager.go- if stackName == "" {
src/pkg/stacks/manager.go: stackName = DefaultBeta
src/pkg/stacks/manager.go- }
src/pkg/stacks/manager.go- var providerID client.ProviderID
--
src/cmd/cli/command/compose.go- }
src/cmd/cli/command/compose.go- if stackName == "" {
src/cmd/cli/command/compose.go: stackName = stacks.DefaultBeta
src/cmd/cli/command/compose.go- _, err := stacks.Create(stacks.StackParameters{
src/cmd/cli/command/compose.go- Name: stackName,
--
src/cmd/cli/command/commands.go- if global.NonInteractive && stack.Provider == client.ProviderAuto {
src/cmd/cli/command/commands.go- whence = "non-interactive default"
src/cmd/cli/command/commands.go: stack.Name = stacks.DefaultBeta
src/cmd/cli/command/commands.go- stack.Provider = client.ProviderDefang
src/cmd/cli/command/commands.go- return stack, whence, nil
--
src/pkg/cli/client/byoc/baseclient.go-func NewByocBaseClient(tenantLabel types.TenantLabel, backend ProjectBackend, stack string) *ByocBaseClient {
src/pkg/cli/client/byoc/baseclient.go- if stack == "" {
src/pkg/cli/client/byoc/baseclient.go: stack = stacks.DefaultBeta // backwards compat
src/pkg/cli/client/byoc/baseclient.go- }
src/pkg/cli/client/byoc/baseclient.go- return &ByocBaseClient{
--
src/pkg/cli/client/byoc/baseclient.go- // place of "beta", so that Fabric will treat these stacks as if there
src/pkg/cli/client/byoc/baseclient.go- // was no explicit stack.
src/pkg/cli/client/byoc/baseclient.go: if b.PulumiStack == stacks.DefaultBeta {
src/pkg/cli/client/byoc/baseclient.go- return ""
src/pkg/cli/client/byoc/baseclient.go- }
Script executed:
# Now let me trace through what happens when AddCloudBuildLogQuery is called with empty project
python3 << 'EOF'
# Simulate what happens in AddCloudBuildLogQuery when project is empty
stack = "beta"
project = "" # Empty when not provided in test
etag = "" # Empty when not provided in test
servicesRegex = "[a-zA-Z0-9-_]{1,63}"
# The format string "%v_%v_%v_%v" with empty strings:
query = f'labels.build_tags =~ "{stack}_{project}_{servicesRegex}_{etag}"'
print("Generated query:")
print(query)
# This explains the double underscore!
EOFRepository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
Generated query:
labels.build_tags =~ "beta__[a-zA-Z0-9-_]{1,63}_"
Script executed:
# Verify the test doesn't pass a project
rg -A 5 '"with_cd_exec"' src/pkg/cli/client/byoc/gcp/byoc_test.goRepository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
{name: "with_cd_exec", req: &defangv1.TailRequest{
LogType: uint32(logs.LogTypeAll),
},
cdExecution: "test-execution-id",
},
{name: "with_etag", req: &defangv1.TailRequest{
Script executed:
# Check if there's any documentation or tests about the RE2 char class behavior
rg "RE2|Cloud Logging.*regex" src/ --type go -B 2 -A 2 | head -40Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
Script executed:
# The key insight: verify the context of when this regex is used
cat -n src/pkg/cli/client/byoc/gcp/query.go | sed -n '40,60p'Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
40 buf.WriteString("\n)\n)")
41 }
42 return buf.String()
43 }
44
45 func NewLogQuery(projectId gcp.ProjectId) *Query {
46 return NewQuery(fmt.Sprintf(`(
47 logName=~"logs/run.googleapis.com%%2F(stdout|stderr)$" OR
48 logName="projects/%[1]s/logs/cloudbuild" OR
49 logName="projects/%[1]s/logs/cos_containers" OR
50 logName="projects/%[1]s/logs/docker-logs"
51 )`, projectId))
52 }
53
54 func NewSubscribeQuery() *Query {
55 return NewQuery(`(
56 protoPayload.serviceName="run.googleapis.com" OR
57 protoPayload.serviceName="compute.googleapis.com" OR
58 protoPayload.serviceName="cloudbuild.googleapis.com"
59 )`)
60 }
Script executed:
# Verify the actual character class in the code generation
cat -n src/pkg/cli/client/byoc/gcp/query.go | sed -n '70,85p'Repository: DefangLabs/defang
Repository: DefangLabs/defang
Exit code: 0
stdout:
70 labels."run.googleapis.com/execution_name" = %q`, executionName)
71 }
72
73 q.AddQuery(query)
74 }
75
76 func (q *Query) AddJobLogQuery(stack, project, etag string, services []string) {
77 query := `resource.type = "cloud_run_job"`
78
79 if stack != "" {
80 query += fmt.Sprintf(`
81 labels."defang-stack" = %q`, gcp.SafeLabelValue(stack))
82 }
83
84 if project != "" {
85 query += fmt.Sprintf(`
Fix character class ordering to avoid unintended range matching.
The hyphen in [a-zA-Z0-9-_] creates a range from - (ASCII 45) to _ (ASCII 95), matching characters like ., /, :, etc. that shouldn't appear in service names. Move the hyphen to the end: [a-zA-Z0-9_-].
The double underscore (beta__...) is intentional—it occurs when the project parameter is empty in the query format.
Proposed fix
- labels.build_tags =~ "beta__[a-zA-Z0-9-_]{1,63}_"
+ labels.build_tags =~ "beta__[a-zA-Z0-9_-]{1,63}_"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| resource.type="build" | |
| labels.build_tags =~ "beta__[a-zA-Z0-9-]{1,63}_" | |
| labels.build_tags =~ "beta__[a-zA-Z0-9-_]{1,63}_" | |
| -labels.build_step="MAIN" | |
| resource.type="build" | |
| labels.build_tags =~ "beta__[a-zA-Z0-9_-]{1,63}_" | |
| -labels.build_step="MAIN" |
🤖 Prompt for AI Agents
In @src/pkg/cli/client/byoc/gcp/testdata/with_cd_exec.query around lines 14 -
16, The character class in the regex used for labels.build_tags is ordered
incorrectly causing an unintended range; update the pattern labels.build_tags =~
"beta__[a-zA-Z0-9-_]{1,63}_" to move the hyphen to the end so it becomes
labels.build_tags =~ "beta__[a-zA-Z0-9_-]{1,63}_" to avoid matching characters
between '-' and '_' and preserve the intended allowed set.
| ) OR ( | ||
| resource.type="build" | ||
| labels.build_tags =~ "beta__[a-zA-Z0-9-]{1,63}_" | ||
| labels.build_tags =~ "beta__[a-zA-Z0-9-_]{1,63}_" |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the test file and check its content
find . -name "with_logtype_build.query" -type f | head -5Repository: DefangLabs/defang
Length of output: 124
🏁 Script executed:
# Search for query.go in the codebase
find . -name "query.go" -path "*/byoc/gcp/*" -type fRepository: DefangLabs/defang
Length of output: 99
🏁 Script executed:
# Look for the pattern in production code that generates these queries
rg -l "build_tags" --type go --type txt | head -10Repository: DefangLabs/defang
Length of output: 135
🏁 Script executed:
# Read the test file to see the context and line 12
cat -n ./src/pkg/cli/client/byoc/gcp/testdata/with_logtype_build.query | head -20Repository: DefangLabs/defang
Length of output: 571
🏁 Script executed:
# Read query.go to find the regex pattern construction
cat -n ./src/pkg/cli/client/byoc/gcp/query.go | head -150Repository: DefangLabs/defang
Length of output: 4370
🏁 Script executed:
# Continue reading query.go to find the AddBuildLogQuery or similar method
sed -n '150,250p' ./src/pkg/cli/client/byoc/gcp/query.go | cat -nRepository: DefangLabs/defang
Length of output: 4133
🏁 Script executed:
# Search for where the build_tags pattern is defined
rg -n "build_tags" ./src/pkg/cli/client/byoc/gcp/query.go -A 3 -B 3Repository: DefangLabs/defang
Length of output: 427
🏁 Script executed:
# Verify the regex issue by checking what characters are actually matched
# Create a test to show the unintended range
python3 << 'EOF'
import re
# The problematic pattern
pattern = r'[a-zA-Z0-9-_]'
regex = re.compile(pattern)
# Test various characters
test_chars = list(range(48, 96)) + [95] # ASCII 48-95
matched = []
for code in test_chars:
char = chr(code)
if regex.match(char):
matched.append((code, char, repr(char)))
print("Characters matched by [a-zA-Z0-9-_]:")
for code, char, repr_char in matched:
if char in '0123456789':
print(f" {code}: {repr_char} (digit - INTENDED)")
elif char in 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ':
print(f" {code}: {repr_char} (letter - INTENDED)")
elif char in '-_':
print(f" {code}: {repr_char} (dash/underscore - INTENDED)")
else:
print(f" {code}: {repr_char} (UNINTENDED - in range 57-95)")
print("\n\nCharacters matched by corrected [a-zA-Z0-9_-]:")
pattern_fixed = r'[a-zA-Z0-9_-]'
regex_fixed = re.compile(pattern_fixed)
for code in test_chars:
char = chr(code)
if regex_fixed.match(char):
matched_fixed = (code, char, repr(char))
if char not in [c[1] for c in matched]:
print(f" {code}: {repr(char)} (NOW EXCLUDED - UNINTENDED)")
EOFRepository: DefangLabs/defang
Length of output: 147
Critical: Fix hyphen placement in character class to prevent unintended character matches.
The character class [a-zA-Z0-9-_] with the hyphen positioned between 9 and _ creates an unintended range from ASCII 57 to 95, which matches characters beyond the intended set, including :, ;, <, =, >, ?, @, [, \, ], ^. This violates the strict build tag format requirement (3-4 underscore-separated parts).
Move the hyphen to the end: [a-zA-Z0-9_-]
This pattern appears in both the test data (line 12) and the production code (query.go:159).
🤖 Prompt for AI Agents
In @src/pkg/cli/client/byoc/gcp/testdata/with_logtype_build.query at line 12,
The character class in the regex "[a-zA-Z0-9-_]" creates an accidental range;
update occurrences to put the hyphen at the end so it becomes "[a-zA-Z0-9_-]".
Replace the pattern in the test data (with_logtype_build.query) and the
corresponding production regex in query.go (the build tags / labels.build_tags
pattern) so both use "[a-zA-Z0-9_-]" to restrict matches to alphanumerics,
underscore and hyphen only.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
To allow handling of project names with underscore in it in GCP provider
Linked Issues
Fixes https://github.com/DefangLabs/defang-mvp/issues/2533
Checklist
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.