Skip to content

Commit 07dd3ac

Browse files
fix: track if there is no service config (#3281)
In our code we're assuming if there is no service config that we should automatically derive it, this is incorrect, sometimes a service config does not exist: example [orgpolicy](https://github.com/googleapis/googleapis/tree/master/google/cloud/orgpolicy/v1). We need to track when migrating from sidekick if the service config does not exist, and if it doesn't we shouldn't try automatically derive. Fix #3273 --------- Signed-off-by: ldetmer <[email protected]> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 parent c0f68e9 commit 07dd3ac

File tree

7 files changed

+96
-13
lines changed

7 files changed

+96
-13
lines changed

devtools/cmd/migrate-sidekick/main.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -590,8 +590,15 @@ func buildConfig(libraries map[string]*config.Library, defaults *config.Config)
590590
for _, lib := range libraries {
591591
// Get the API path for this library
592592
apiPath := ""
593+
serviceConfigDoesNotExist := false
593594
if len(lib.Channels) > 0 {
594595
apiPath = lib.Channels[0].Path
596+
for _, ch := range lib.Channels {
597+
if ch.ServiceConfig == "" {
598+
ch.ServiceConfigDoesNotExist = true
599+
serviceConfigDoesNotExist = true
600+
}
601+
}
595602
}
596603

597604
// Derive expected library name from API path
@@ -603,11 +610,8 @@ func buildConfig(libraries map[string]*config.Library, defaults *config.Config)
603610
lib.Rust.GenerateSetterSamples != "" || lib.Rust.GenerateRpcSamples ||
604611
len(lib.Rust.PackageDependencies) > 0 || len(lib.Rust.PaginationOverrides) > 0 ||
605612
lib.Rust.NameOverrides != ""))
606-
// Only include in libraries section if:
607-
// 1. Name doesn't match expected naming convention (name override)
608-
// 2. Library has extra configuration
609-
// 3. Library spans multiple APIs
610-
if !nameMatchesConvention || hasExtraConfig || len(lib.Channels) > 1 {
613+
// Only include in libraries section if specific data needs to be retained
614+
if !nameMatchesConvention || hasExtraConfig || len(lib.Channels) > 1 || serviceConfigDoesNotExist {
611615
libCopy := *lib
612616
libList = append(libList, &libCopy)
613617
}

devtools/cmd/migrate-sidekick/main_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,56 @@ func TestBuildConfig(t *testing.T) {
561561
},
562562
},
563563
},
564+
{
565+
name: "service does not exist",
566+
defaults: &config.Config{},
567+
libraries: map[string]*config.Library{
568+
"google-cloud-security-publicca-v1": {
569+
Name: "google-cloud-security-publicca-v1",
570+
Channels: []*config.Channel{
571+
{
572+
Path: "google/cloud/security/publicca/v1",
573+
},
574+
},
575+
Version: "1.1.0",
576+
CopyrightYear: "2025",
577+
Rust: &config.RustCrate{
578+
RustDefault: config.RustDefault{
579+
DisabledRustdocWarnings: []string{"bare_urls", "broken_intra_doc_links", "redundant_explicit_links"},
580+
GenerateSetterSamples: "true",
581+
},
582+
PerServiceFeatures: true,
583+
GenerateRpcSamples: true,
584+
NameOverrides: ".google.cloud.security/publicca.v1.Storage=StorageControl",
585+
},
586+
},
587+
},
588+
want: &config.Config{
589+
Libraries: []*config.Library{
590+
{
591+
Name: "google-cloud-security-publicca-v1",
592+
Channels: []*config.Channel{
593+
{
594+
ServiceConfigDoesNotExist: true,
595+
Path: "google/cloud/security/publicca/v1",
596+
ServiceConfig: "",
597+
},
598+
},
599+
Version: "1.1.0",
600+
CopyrightYear: "2025",
601+
Rust: &config.RustCrate{
602+
RustDefault: config.RustDefault{
603+
DisabledRustdocWarnings: []string{"bare_urls", "broken_intra_doc_links", "redundant_explicit_links"},
604+
GenerateSetterSamples: "true",
605+
},
606+
PerServiceFeatures: true,
607+
GenerateRpcSamples: true,
608+
NameOverrides: ".google.cloud.security/publicca.v1.Storage=StorageControl",
609+
},
610+
},
611+
},
612+
},
613+
},
564614
} {
565615
t.Run(test.name, func(t *testing.T) {
566616
got := buildConfig(test.libraries, test.defaults)

internal/config/config.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,4 +209,7 @@ type Channel struct {
209209

210210
// ServiceConfig is the path to the service config file.
211211
ServiceConfig string `yaml:"service_config,omitempty"`
212+
213+
// ServiceConfigDoesNotExist specifies if there is no service config for this channel.
214+
ServiceConfigDoesNotExist bool `yaml:"service_config_does_not_exist,omitempty"`
212215
}

internal/librarian/generate.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ func prepareLibrary(language string, lib *config.Library, defaults *config.Defau
266266
if ch.Path == "" {
267267
ch.Path = deriveChannelPath(language, lib)
268268
}
269-
if ch.ServiceConfig == "" {
269+
if ch.ServiceConfig == "" && !ch.ServiceConfigDoesNotExist {
270270
ch.ServiceConfig = deriveServiceConfig(ch.Path)
271271
}
272272
}

internal/librarian/generate_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,14 @@ func TestPrepareLibrary(t *testing.T) {
279279
output: "src/storage",
280280
wantOutput: "src/storage",
281281
},
282+
{
283+
name: "rust lib without service config does not derive service config",
284+
language: "rust",
285+
channels: []*config.Channel{{ServiceConfigDoesNotExist: true}},
286+
wantOutput: "src/generated/cloud/test/v1",
287+
wantChannelPath: "google/cloud/test/v1",
288+
wantServiceConfig: "",
289+
},
282290
} {
283291
t.Run(test.name, func(t *testing.T) {
284292
lib := &config.Library{

internal/librarian/tidy.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ func RunTidy() error {
6464
}
6565
}
6666
lib.Channels = slices.DeleteFunc(lib.Channels, func(ch *config.Channel) bool {
67-
return ch.Path == "" && ch.ServiceConfig == ""
67+
return ch.Path == "" && ch.ServiceConfig == "" && !ch.ServiceConfigDoesNotExist
6868
})
6969

7070
tidyLanguageConfig(lib, cfg.Language)

internal/librarian/tidy_test.go

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -129,12 +129,13 @@ libraries:
129129

130130
func TestTidy_DerivableFields(t *testing.T) {
131131
for _, test := range []struct {
132-
name string
133-
configContent string
134-
wantPath string
135-
wantSvcCfg string
136-
wantNumLibs int
137-
wantNumChnls int
132+
name string
133+
configContent string
134+
wantPath string
135+
wantSvcCfg string
136+
wantNumLibs int
137+
wantNumChnls int
138+
wantServiceConfigDne bool
138139
}{
139140
{
140141
name: "derivable fields removed",
@@ -205,6 +206,20 @@ libraries:
205206
wantNumLibs: 1,
206207
wantNumChnls: 1,
207208
},
209+
{
210+
name: "channel remains if service config does not exist",
211+
configContent: `
212+
libraries:
213+
- name: google-cloud-accessapproval-v1
214+
channels:
215+
- service_config_does_not_exist: true
216+
`,
217+
wantPath: "",
218+
wantSvcCfg: "",
219+
wantNumLibs: 1,
220+
wantNumChnls: 1,
221+
wantServiceConfigDne: true,
222+
},
208223
} {
209224
t.Run(test.name, func(t *testing.T) {
210225
tempDir := t.TempDir()
@@ -238,6 +253,9 @@ libraries:
238253
if ch.ServiceConfig != test.wantSvcCfg {
239254
t.Errorf("service_config should be %s, got %q", test.wantSvcCfg, ch.ServiceConfig)
240255
}
256+
if ch.ServiceConfigDoesNotExist != test.wantServiceConfigDne {
257+
t.Errorf("service_config_does_not_exist should be %t, got %t", test.wantServiceConfigDne, ch.ServiceConfigDoesNotExist)
258+
}
241259
}
242260
})
243261
}

0 commit comments

Comments
 (0)