From 3b73ebeeb36aa1140784c8936b400911e4d127e9 Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Mon, 26 Jan 2026 23:42:11 +0000 Subject: [PATCH 01/10] feat: add a new feature enablement mechanism This PR adds a new feature enablement mechanism to the Go GAPIC generator. The intent is to pass behavior enablement via protoc flags, with support for the old explicit allowlists until we can transition the usage. --- internal/gengapic/client_init_test.go | 7 ++++- internal/gengapic/generator.go | 38 ++++----------------------- internal/gengapic/gengapic.go | 9 +------ internal/gengapic/gengapic_test.go | 4 +++ internal/gengapic/gengrpc.go | 2 +- internal/gengapic/options.go | 35 +++++++++++++++++++++++- internal/gengapic/paging.go | 10 +------ internal/gengapic/paging_test.go | 23 +++++----------- 8 files changed, 59 insertions(+), 69 deletions(-) diff --git a/internal/gengapic/client_init_test.go b/internal/gengapic/client_init_test.go index 1c2993e9b6c..2adf2f535da 100644 --- a/internal/gengapic/client_init_test.go +++ b/internal/gengapic/client_init_test.go @@ -134,7 +134,12 @@ func TestClientOpt(t *testing.T) { {Name: "google.longrunning.Operations"}, }, }, - gRPCServiceConfig: grpcConf}, + gRPCServiceConfig: grpcConf, + // Showcase would enable MTLS if we went through legacy enablements, so add it explicitly here. + featureEnablement: map[featureID]bool{ + EnableMTLSHardBoundTokens: true, + }, + }, } serv := &descriptorpb.ServiceDescriptorProto{ diff --git a/internal/gengapic/generator.go b/internal/gengapic/generator.go index 8e3689b672b..e1efc8231da 100644 --- a/internal/gengapic/generator.go +++ b/internal/gengapic/generator.go @@ -31,39 +31,6 @@ import ( "google.golang.org/protobuf/types/pluginpb" ) -// keyed by proto package name, e.g. "google.cloud.foo.v1". -var enableWrapperTypesForPageSize = map[string]bool{ - "google.cloud.bigquery.v2": true, -} - -var enableOrderedRoutingHeaders = map[string]bool{ - "google.firestore.v1": true, - "google.firestore.admin.v1": true, -} - -var enableMtlsHardBoundTokens = map[string]bool{ - "bigquery.googleapis.com": true, - "cloudasset.googleapis.com": true, - "clouderrorreporting.googleapis.com": true, - "cloudkms.googleapis.com": true, - "cloudresourcemanager.googleapis.com": true, - "cloudtasks.googleapis.com": true, - "cloudtrace.googleapis.com": true, - "dataflow.googleapis.com": true, - "datastore.googleapis.com": true, - "essentialcontacts.googleapis.com": true, - "firestore.googleapis.com": true, - "iam.googleapis.com": true, - "iamcredentials.googleapis.com": true, - "logging.googleapis.com": true, - "monitoring.googleapis.com": true, - "orgpolicy.googleapis.com": true, - "pubsub.googleapis.com": true, - "recommender.googleapis.com": true, - "secretmanager.googleapis.com": true, - "showcase.googleapis.com": true, -} - type generator struct { pt printer.P @@ -131,6 +98,11 @@ func newGenerator(req *pluginpb.CodeGeneratorRequest) (*generator, error) { if err != nil { return nil, err } + // Handle legacy enablement via hardcoded allowlists. + // This logic should be removed when legacy enablement is no longer needed. + processLegacyEnablements(cfg, req) + + // attach config to generator. g.cfg = cfg files := req.GetProtoFile() diff --git a/internal/gengapic/gengapic.go b/internal/gengapic/gengapic.go index fc36fd4407b..2394214e4ca 100644 --- a/internal/gengapic/gengapic.go +++ b/internal/gengapic/gengapic.go @@ -456,14 +456,7 @@ func (g *generator) insertDynamicRequestHeaders(m *descriptorpb.MethodDescriptor g.printf(" routingHeadersMap[%q] = %s", headerName, regexHelper) g.printf("}") } - var orderedHeaders bool - for p, ok := range enableOrderedRoutingHeaders { - if g.descInfo.ParentFile[m].GetPackage() == p && ok { - orderedHeaders = true - break - } - } - if orderedHeaders { + if g.cfg.FeatureEnabled(EnableOrderedRoutingHeaders) { for i := range headers { headerName := headers[i][2] g.printf("if headerValue, ok := routingHeadersMap[%q]; ok {", headerName) diff --git a/internal/gengapic/gengapic_test.go b/internal/gengapic/gengapic_test.go index 0bc148a1061..856a79d41fb 100644 --- a/internal/gengapic/gengapic_test.go +++ b/internal/gengapic/gengapic_test.go @@ -1425,6 +1425,7 @@ func TestInsertDynamicRequestHeaders_Ordering(t *testing.T) { Type: make(map[string]pbinfo.ProtoType), ParentFile: make(map[protoreflect.ProtoMessage]*descriptorpb.FileDescriptorProto), }, + cfg: &generatorConfig{}, } m := &descriptorpb.MethodDescriptorProto{ @@ -1467,6 +1468,9 @@ func TestInsertDynamicRequestHeaders_Ordering(t *testing.T) { for _, tc := range tests { t.Run(tc.pkgName, func(t *testing.T) { g.reset() + g.cfg.featureEnablement = map[featureID]bool{ + EnableOrderedRoutingHeaders: tc.wantOrdered, + } file := &descriptorpb.FileDescriptorProto{ Package: proto.String(tc.pkgName), diff --git a/internal/gengapic/gengrpc.go b/internal/gengapic/gengrpc.go index f09fbcad781..ea93af1247f 100644 --- a/internal/gengapic/gengrpc.go +++ b/internal/gengapic/gengrpc.go @@ -186,7 +186,7 @@ func (g *generator) grpcClientOptions(serv *descriptorpb.ServiceDescriptorProto, p(" internaloption.WithDefaultAudience(%q),", generateDefaultAudience(host)) p(" internaloption.WithDefaultScopes(DefaultAuthScopes()...),") p(" internaloption.EnableJwtWithScope(),") - if _, ok := enableMtlsHardBoundTokens[g.cfg.APIServiceConfig.GetName()]; ok { + if g.cfg.FeatureEnabled(EnableMTLSHardBoundTokens) { p("internaloption.AllowHardBoundTokens(\"MTLS_S2A\"),") } p(" internaloption.EnableNewAuthLibrary(),") diff --git a/internal/gengapic/options.go b/internal/gengapic/options.go index 915f4709597..fd7ff713a1c 100644 --- a/internal/gengapic/options.go +++ b/internal/gengapic/options.go @@ -73,7 +73,8 @@ var SupportedValueArgs map[string]func(string) configOption = map[string]func(st // // Recommendation: avoid adding more of these as they complicate processing. var SupportedPrefixArgs map[string]func(string) configOption = map[string]func(string) configOption{ - "M": withPackageOverride, + "M": withPackageOverride, + "FEAT_": withFeatureEnable, } // Configuration needed to drive the operation of the plugin. @@ -121,6 +122,20 @@ type generatorConfig struct { // Package naming overrides, keyed by filepath. pkgOverrides map[string]string + + // Keep track of what conditional behaviors/experiments are enabled. + featureEnablement map[featureID]bool +} + +// Simplify checking if generator feature is enabled. +func (cfg *generatorConfig) FeatureEnabled(f featureID) bool { + if cfg == nil { + return false + } + if val, ok := cfg.featureEnablement[f]; ok && val { + return true + } + return false } // Signature for configuration arguments. @@ -385,6 +400,24 @@ func withPackageOverride(s string) configOption { } } +// withFeatureEnable handles boolean enablement of generator-level functionality. +// The `FEAT:` prefix is not passed as part of the string, only the remaining substring. +func withFeatureEnable(s string) configOption { + fID := featureID(s) + if _, ok := featureRegistry[fID]; !ok { + return func(cfg *generatorConfig) error { + return fmt.Errorf("no such feature is registered in the generator: %q", s) + } + } + return func(cfg *generatorConfig) error { + if cfg.featureEnablement == nil { + cfg.featureEnablement = make(map[featureID]bool) + } + cfg.featureEnablement[fID] = true + return nil + } +} + // Utility function for stringifying the Transport enum func (t transport) String() string { switch t { diff --git a/internal/gengapic/paging.go b/internal/gengapic/paging.go index 81d499e850f..cad47809a11 100644 --- a/internal/gengapic/paging.go +++ b/internal/gengapic/paging.go @@ -178,14 +178,6 @@ func (g *generator) getPagingFields(m *descriptorpb.MethodDescriptorProto) (repe return nil, nil, nil } - var wrapperTypesAllowed bool - for p, ok := range enableWrapperTypesForPageSize { - if g.descInfo.ParentFile[m].GetPackage() == p && ok { - wrapperTypesAllowed = true - break - } - } - inType := g.descInfo.Type[m.GetInputType()] if inType == nil { return nil, nil, fmt.Errorf("expected %q to be message type, found %T", m.GetInputType(), inType) @@ -206,7 +198,7 @@ func (g *generator) getPagingFields(m *descriptorpb.MethodDescriptorProto) (repe hasPageToken := false for _, f := range inMsg.GetField() { - candidate, needsWrapper := isPageSizeField(f, wrapperTypesAllowed) + candidate, needsWrapper := isPageSizeField(f, g.cfg.FeatureEnabled(EnableWrapperTypesForPageSize)) if candidate { if pageSizeField == nil { pageSizeField = f diff --git a/internal/gengapic/paging_test.go b/internal/gengapic/paging_test.go index 2f0965344e1..9380ffc8d3b 100644 --- a/internal/gengapic/paging_test.go +++ b/internal/gengapic/paging_test.go @@ -125,19 +125,6 @@ func TestPagingField(t *testing.T) { // * Response has a string next_page_token field // * Response has one and only one repeated or map field - // This test manipulates the enableWrapperTypesForPageSize allowlist, so ensure we're - // not tainting state after test completes. - origAllowList := make(map[string]bool) - for k, v := range enableWrapperTypesForPageSize { - origAllowList[k] = v - } - - defer func() { - enableWrapperTypesForPageSize = origAllowList - }() - // Clear the allowlist for this test. - enableWrapperTypesForPageSize = make(map[string]bool) - // Messages validPageSize := &descriptorpb.DescriptorProto{ Name: proto.String("ValidPageSizeRequest"), @@ -501,13 +488,17 @@ func TestPagingField(t *testing.T) { } } - // Re-test, adding the "paging" package to the allowlist. - enableWrapperTypesForPageSize["paging"] = true + // Re-test, enabling the wrapper feature. g, err = newGenerator(&req) if err != nil { t.Fatal(err) } - g.cfg = &generatorConfig{transports: []transport{rest}} + g.cfg = &generatorConfig{ + transports: []transport{rest}, + featureEnablement: map[featureID]bool{ + EnableWrapperTypesForPageSize: true, + }, + } for _, tst := range []struct { mthd *descriptorpb.MethodDescriptorProto sizeField *descriptorpb.FieldDescriptorProto // A nil field means this is not a paged method From d7fd90322d99a0bf332df390bef5fde0a2336e4b Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Mon, 26 Jan 2026 23:43:51 +0000 Subject: [PATCH 02/10] add missing files --- internal/gengapic/feature.go | 129 ++++++++++++++++++++++++++++++ internal/gengapic/feature_test.go | 101 +++++++++++++++++++++++ 2 files changed, 230 insertions(+) create mode 100644 internal/gengapic/feature.go create mode 100644 internal/gengapic/feature_test.go diff --git a/internal/gengapic/feature.go b/internal/gengapic/feature.go new file mode 100644 index 00000000000..ffd7042a244 --- /dev/null +++ b/internal/gengapic/feature.go @@ -0,0 +1,129 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package gengapic + +import "google.golang.org/protobuf/types/pluginpb" + +// We use a custom type for feature ID to clarify that features are setup explicitly. +type featureID string + +// featureInfo contains basic information about features, and provides an extensible mechanism +// for adding more infomation down the line (maturity level, warning about stale experiments, etc) +type featureInfo struct { + // Short summary of feature. + Description string + + // Tracking ID for more information. Github issue, internal b/ issue, etc. + TrackingID string +} + +// Define feature ID strings here. More details about features are kept in the featureRegistry map. +const ( + EnableWrapperTypesForPageSize featureID = "WRAPPER_TYPES_FOR_PAGE_SIZE" + EnableOrderedRoutingHeaders featureID = "ORDERED_ROUTING_HEADERS" + EnableMTLSHardBoundTokens featureID = "MTLS_HARD_BOUND_TOKENS" +) + +// featureRegistry contains the registry of defined features. +// Introducing a new capability to the generator generally starts here, as features +// must be registered to be enabled. This should not be modified at runtime. Those +// who attempt to do so will be given a stern talking to. +var featureRegistry = map[featureID]*featureInfo{ + EnableMTLSHardBoundTokens: { + Description: "support MTLS hard bound tokens", + }, + EnableOrderedRoutingHeaders: { + Description: "Specify that routing headers are emitted in a deterministic fashion. Primarily used for firestore.", + }, + EnableWrapperTypesForPageSize: { + Description: "Allow List RPCs to generator with support for protobuf wrapper types (e.g. Int32Value, etc).", + TrackingID: "b/352331075", + }, +} + +// legacyFeatureEnablementByPackage is temporary bridge functionality. Features should be enabled via protoc flags, but +// to bootstrap without breaking generation we keep the legacy definitions enabled here until we can move +// configuration upstream into tools like librarian/bazel/etc as needed. +var legacyFeatureEnablementByPackage = map[featureID][]string{ + EnableOrderedRoutingHeaders: []string{ + "google.firestore.v1", + "google.firestore.admin.v1", + }, + EnableWrapperTypesForPageSize: []string{ + "google.cloud.bigquery.v2", + }, +} + +// similar to legacyFeatureEnablementByPackage, this is legacy feature enablement using the "name" field from the API +// service config. +var legacyFeatureEnablementByAPIName = map[featureID][]string{ + EnableMTLSHardBoundTokens: []string{ + "bigquery.googleapis.com", + "cloudasset.googleapis.com", + "clouderrorreporting.googleapis.com", + "cloudkms.googleapis.com", + "cloudresourcemanager.googleapis.com", + "cloudtasks.googleapis.com", + "cloudtrace.googleapis.com", + "dataflow.googleapis.com", + "datastore.googleapis.com", + "essentialcontacts.googleapis.com", + "firestore.googleapis.com", + "iam.googleapis.com", + "iamcredentials.googleapis.com", + "logging.googleapis.com", + "monitoring.googleapis.com", + "orgpolicy.googleapis.com", + "pubsub.googleapis.com", + "recommender.googleapis.com", + "secretmanager.googleapis.com", + "showcase.googleapis.com", + }, +} + +// This function consolidates legacy processing of feature enablements. +// Like the associated allowlists, it should go away once librarian and bazel can pass feature enablements directly. +func processLegacyEnablements(cfg *generatorConfig, req *pluginpb.CodeGeneratorRequest) { + // Use the first proto file in the FileDescriptorSet to handle legacy enablement by package name. + if len(req.GetProtoFile()) > 0 { + probePackage := req.GetProtoFile()[0].GetPackage() + for f, packages := range legacyFeatureEnablementByPackage { + for _, v := range packages { + if probePackage == v { // matched + if cfg.featureEnablement == nil { + cfg.featureEnablement = make(map[featureID]bool) + } + cfg.featureEnablement[f] = true + break + } + } + } + } + // Now, process legacy feature enablements based on API name. + if cfg.APIServiceConfig != nil { + probeName := cfg.APIServiceConfig.GetName() + for f, apis := range legacyFeatureEnablementByAPIName { + for _, v := range apis { + if probeName == v { // matched + if cfg.featureEnablement == nil { + cfg.featureEnablement = make(map[featureID]bool) + } + cfg.featureEnablement[f] = true + break + } + } + } + } +} diff --git a/internal/gengapic/feature_test.go b/internal/gengapic/feature_test.go new file mode 100644 index 00000000000..9fb06afcc5b --- /dev/null +++ b/internal/gengapic/feature_test.go @@ -0,0 +1,101 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package gengapic + +import ( + "testing" + + "google.golang.org/genproto/googleapis/api/serviceconfig" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/descriptorpb" + "google.golang.org/protobuf/types/pluginpb" +) + +func TestProcessLegacyEnablements(t *testing.T) { + for _, tc := range []struct { + desc string + protoPkg string + apiName string + wantFeaturesEnabled []featureID + wantFeatureDisabled []featureID + }{ + { + desc: "default", + wantFeatureDisabled: []featureID{ + EnableMTLSHardBoundTokens, + EnableOrderedRoutingHeaders, + EnableWrapperTypesForPageSize, + }, + }, + { + desc: "bigquery package", + protoPkg: "google.cloud.bigquery.v2", + wantFeaturesEnabled: []featureID{EnableWrapperTypesForPageSize}, + }, + { + desc: "bigquery package only", + protoPkg: "google.cloud.bigquery.v2", + wantFeaturesEnabled: []featureID{EnableWrapperTypesForPageSize}, + wantFeatureDisabled: []featureID{EnableOrderedRoutingHeaders, EnableMTLSHardBoundTokens}, + }, + { + desc: "firestore admin package only", + protoPkg: "google.firestore.admin.v1", + wantFeaturesEnabled: []featureID{EnableOrderedRoutingHeaders}, + wantFeatureDisabled: []featureID{EnableWrapperTypesForPageSize, EnableMTLSHardBoundTokens}, + }, + { + desc: "cloud kms api name", + apiName: "cloudkms.googleapis.com", + wantFeaturesEnabled: []featureID{EnableMTLSHardBoundTokens}, + wantFeatureDisabled: []featureID{EnableWrapperTypesForPageSize, EnableOrderedRoutingHeaders}, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + req := &pluginpb.CodeGeneratorRequest{ + Parameter: proto.String("go-gapic-package=foo/bar/baz;baz"), + ProtoFile: []*descriptorpb.FileDescriptorProto{ + { + Package: proto.String("foo"), + }, + }, + } + if tc.protoPkg != "" { + req.ProtoFile[0].Package = proto.String(tc.protoPkg) + } + cfg, err := configFromRequest(req.Parameter) + if err != nil { + t.Fatalf("configFromRequest err: %v", err) + } + if tc.apiName != "" { + cfg.APIServiceConfig = &serviceconfig.Service{ + Name: tc.apiName, + } + } + processLegacyEnablements(cfg, req) + for _, f := range tc.wantFeaturesEnabled { + if !cfg.FeatureEnabled(f) { + t.Errorf("expected feature %q enabled, was not", f) + } + } + for _, f := range tc.wantFeatureDisabled { + if cfg.FeatureEnabled(f) { + t.Errorf("expected feature %q to be disabled, was enabled", f) + } + } + }) + + } +} From a785af93d76034ba2411295bf6114b7011ba2f1e Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Tue, 27 Jan 2026 20:29:41 +0000 Subject: [PATCH 03/10] add tests --- internal/gengapic/options_test.go | 46 +++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/internal/gengapic/options_test.go b/internal/gengapic/options_test.go index ecab047d2c2..83f0a7de050 100644 --- a/internal/gengapic/options_test.go +++ b/internal/gengapic/options_test.go @@ -15,7 +15,6 @@ package gengapic import ( - "reflect" "testing" "github.com/google/go-cmp/cmp" @@ -142,22 +141,41 @@ func TestParseOptions(t *testing.T) { }, expectErr: false, }, + { + // Test empty parameter in the CSV. + param: "go-gapic-package=path/to/imp;pkg,FEAT_INVALID_FEATURE", + expectErr: true, + }, + { + // Test empty parameter in the CSV. + param: "go-gapic-package=path;pkg,FEAT_ORDERED_ROUTING_HEADERS", + expectedCfg: &generatorConfig{ + transports: []transport{grpc}, + pkgPath: "path", + pkgName: "pkg", + outDir: "path", + featureEnablement: map[featureID]bool{ + EnableOrderedRoutingHeaders: true, + }, + }, + }, } { - opts, err := configFromRequest(&tst.param) - if tst.expectErr && err == nil { - t.Errorf("parseOptions(%s) expected error", tst.param) - continue - } + t.Run(tst.param, func(t *testing.T) { + + gotCfg, err := configFromRequest(&tst.param) + if tst.expectErr && err == nil { + t.Fatalf("parseOptions(%s) expected error", tst.param) + } - if !tst.expectErr && err != nil { - t.Errorf("parseOptions(%s) got unexpected error: %v", tst.param, err) - continue - } + if !tst.expectErr && err != nil { + t.Fatalf("parseOptions(%s) got unexpected error: %v", tst.param, err) + } + + if diff := cmp.Diff(gotCfg, tst.expectedCfg, cmp.AllowUnexported(generatorConfig{}, conf.Config{})); diff != "" { + t.Errorf("got(-), want(+):\n%s", diff) + } + }) - if !reflect.DeepEqual(opts, tst.expectedCfg) { - t.Errorf("parseOptions(%s) = %+v, expected %+v", tst.param, opts, tst.expectedCfg) - continue - } } } From 4fa93964b516eac59f96ea947ed0cf6186478c59 Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Tue, 27 Jan 2026 20:58:55 +0000 Subject: [PATCH 04/10] update bazel rules --- internal/gengapic/BUILD.bazel | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/gengapic/BUILD.bazel b/internal/gengapic/BUILD.bazel index 3f27a780b62..b0f10b4e8c7 100644 --- a/internal/gengapic/BUILD.bazel +++ b/internal/gengapic/BUILD.bazel @@ -8,6 +8,7 @@ go_library( "custom_operation.go", "doc_file.go", "example.go", + "feature.go", "generator.go", "gengapic.go", "gengrpc.go", @@ -69,6 +70,7 @@ go_test( "custom_operation_test.go", "doc_file_test.go", "example_test.go", + "feature_test.go", "generator_test.go", "gengapic_test.go", "genrest_test.go", From ca11036fad6f7634825f4414bb092802f9182ed8 Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Wed, 28 Jan 2026 00:49:30 +0000 Subject: [PATCH 05/10] address reviewer feedback --- internal/gengapic/feature.go | 1 + internal/gengapic/options.go | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/gengapic/feature.go b/internal/gengapic/feature.go index ffd7042a244..373047905f5 100644 --- a/internal/gengapic/feature.go +++ b/internal/gengapic/feature.go @@ -43,6 +43,7 @@ const ( var featureRegistry = map[featureID]*featureInfo{ EnableMTLSHardBoundTokens: { Description: "support MTLS hard bound tokens", + TrackingID: "b/327916505", }, EnableOrderedRoutingHeaders: { Description: "Specify that routing headers are emitted in a deterministic fashion. Primarily used for firestore.", diff --git a/internal/gengapic/options.go b/internal/gengapic/options.go index fd7ff713a1c..2de90395f26 100644 --- a/internal/gengapic/options.go +++ b/internal/gengapic/options.go @@ -74,7 +74,7 @@ var SupportedValueArgs map[string]func(string) configOption = map[string]func(st // Recommendation: avoid adding more of these as they complicate processing. var SupportedPrefixArgs map[string]func(string) configOption = map[string]func(string) configOption{ "M": withPackageOverride, - "FEAT_": withFeatureEnable, + "FEAT_": withFeature, } // Configuration needed to drive the operation of the plugin. @@ -400,9 +400,9 @@ func withPackageOverride(s string) configOption { } } -// withFeatureEnable handles boolean enablement of generator-level functionality. +// withFeature handles boolean enablement of generator-level functionality. // The `FEAT:` prefix is not passed as part of the string, only the remaining substring. -func withFeatureEnable(s string) configOption { +func withFeature(s string) configOption { fID := featureID(s) if _, ok := featureRegistry[fID]; !ok { return func(cfg *generatorConfig) error { From a8f555d5babacdbffbad61dc53e718a2e03e6292 Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Wed, 28 Jan 2026 16:57:24 +0000 Subject: [PATCH 06/10] update comment --- internal/gengapic/options.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/gengapic/options.go b/internal/gengapic/options.go index 2de90395f26..bd34e2af34f 100644 --- a/internal/gengapic/options.go +++ b/internal/gengapic/options.go @@ -401,7 +401,7 @@ func withPackageOverride(s string) configOption { } // withFeature handles boolean enablement of generator-level functionality. -// The `FEAT:` prefix is not passed as part of the string, only the remaining substring. +// The `FEAT_` prefix is not passed as part of the string, only the remaining substring. func withFeature(s string) configOption { fID := featureID(s) if _, ok := featureRegistry[fID]; !ok { From d825208a1aa6e1bc77602ca9f565bda6960ccceb Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Wed, 28 Jan 2026 22:35:03 +0000 Subject: [PATCH 07/10] updates based on reviewer feedback --- internal/gengapic/client_init_test.go | 2 +- internal/gengapic/feature.go | 18 +++++++++--------- internal/gengapic/feature_test.go | 25 +++++++++++++------------ internal/gengapic/generator.go | 11 +++++++++++ internal/gengapic/gengapic.go | 2 +- internal/gengapic/gengapic_test.go | 2 +- internal/gengapic/gengrpc.go | 2 +- internal/gengapic/options.go | 17 +++-------------- internal/gengapic/options_test.go | 22 ++++++++++++++++++---- internal/gengapic/paging.go | 2 +- internal/gengapic/paging_test.go | 2 +- 11 files changed, 60 insertions(+), 45 deletions(-) diff --git a/internal/gengapic/client_init_test.go b/internal/gengapic/client_init_test.go index 2adf2f535da..acc15953fb2 100644 --- a/internal/gengapic/client_init_test.go +++ b/internal/gengapic/client_init_test.go @@ -137,7 +137,7 @@ func TestClientOpt(t *testing.T) { gRPCServiceConfig: grpcConf, // Showcase would enable MTLS if we went through legacy enablements, so add it explicitly here. featureEnablement: map[featureID]bool{ - EnableMTLSHardBoundTokens: true, + MTLSHardBoundTokensFeature: true, }, }, } diff --git a/internal/gengapic/feature.go b/internal/gengapic/feature.go index 373047905f5..dc75a0cc4da 100644 --- a/internal/gengapic/feature.go +++ b/internal/gengapic/feature.go @@ -31,9 +31,9 @@ type featureInfo struct { // Define feature ID strings here. More details about features are kept in the featureRegistry map. const ( - EnableWrapperTypesForPageSize featureID = "WRAPPER_TYPES_FOR_PAGE_SIZE" - EnableOrderedRoutingHeaders featureID = "ORDERED_ROUTING_HEADERS" - EnableMTLSHardBoundTokens featureID = "MTLS_HARD_BOUND_TOKENS" + WrapperTypesForPageSizeFeature featureID = "wrapper_types_for_page_size" + OrderedRoutingHeadersFeature featureID = "ordered_routing_headers" + MTLSHardBoundTokensFeature featureID = "mtls_hard_bound_tokens" ) // featureRegistry contains the registry of defined features. @@ -41,14 +41,14 @@ const ( // must be registered to be enabled. This should not be modified at runtime. Those // who attempt to do so will be given a stern talking to. var featureRegistry = map[featureID]*featureInfo{ - EnableMTLSHardBoundTokens: { + MTLSHardBoundTokensFeature: { Description: "support MTLS hard bound tokens", TrackingID: "b/327916505", }, - EnableOrderedRoutingHeaders: { + OrderedRoutingHeadersFeature: { Description: "Specify that routing headers are emitted in a deterministic fashion. Primarily used for firestore.", }, - EnableWrapperTypesForPageSize: { + WrapperTypesForPageSizeFeature: { Description: "Allow List RPCs to generator with support for protobuf wrapper types (e.g. Int32Value, etc).", TrackingID: "b/352331075", }, @@ -58,11 +58,11 @@ var featureRegistry = map[featureID]*featureInfo{ // to bootstrap without breaking generation we keep the legacy definitions enabled here until we can move // configuration upstream into tools like librarian/bazel/etc as needed. var legacyFeatureEnablementByPackage = map[featureID][]string{ - EnableOrderedRoutingHeaders: []string{ + OrderedRoutingHeadersFeature: []string{ "google.firestore.v1", "google.firestore.admin.v1", }, - EnableWrapperTypesForPageSize: []string{ + WrapperTypesForPageSizeFeature: []string{ "google.cloud.bigquery.v2", }, } @@ -70,7 +70,7 @@ var legacyFeatureEnablementByPackage = map[featureID][]string{ // similar to legacyFeatureEnablementByPackage, this is legacy feature enablement using the "name" field from the API // service config. var legacyFeatureEnablementByAPIName = map[featureID][]string{ - EnableMTLSHardBoundTokens: []string{ + MTLSHardBoundTokensFeature: []string{ "bigquery.googleapis.com", "cloudasset.googleapis.com", "clouderrorreporting.googleapis.com", diff --git a/internal/gengapic/feature_test.go b/internal/gengapic/feature_test.go index 9fb06afcc5b..159a6b4ff99 100644 --- a/internal/gengapic/feature_test.go +++ b/internal/gengapic/feature_test.go @@ -34,33 +34,33 @@ func TestProcessLegacyEnablements(t *testing.T) { { desc: "default", wantFeatureDisabled: []featureID{ - EnableMTLSHardBoundTokens, - EnableOrderedRoutingHeaders, - EnableWrapperTypesForPageSize, + MTLSHardBoundTokensFeature, + OrderedRoutingHeadersFeature, + WrapperTypesForPageSizeFeature, }, }, { desc: "bigquery package", protoPkg: "google.cloud.bigquery.v2", - wantFeaturesEnabled: []featureID{EnableWrapperTypesForPageSize}, + wantFeaturesEnabled: []featureID{WrapperTypesForPageSizeFeature}, }, { desc: "bigquery package only", protoPkg: "google.cloud.bigquery.v2", - wantFeaturesEnabled: []featureID{EnableWrapperTypesForPageSize}, - wantFeatureDisabled: []featureID{EnableOrderedRoutingHeaders, EnableMTLSHardBoundTokens}, + wantFeaturesEnabled: []featureID{WrapperTypesForPageSizeFeature}, + wantFeatureDisabled: []featureID{OrderedRoutingHeadersFeature, MTLSHardBoundTokensFeature}, }, { desc: "firestore admin package only", protoPkg: "google.firestore.admin.v1", - wantFeaturesEnabled: []featureID{EnableOrderedRoutingHeaders}, - wantFeatureDisabled: []featureID{EnableWrapperTypesForPageSize, EnableMTLSHardBoundTokens}, + wantFeaturesEnabled: []featureID{OrderedRoutingHeadersFeature}, + wantFeatureDisabled: []featureID{WrapperTypesForPageSizeFeature, MTLSHardBoundTokensFeature}, }, { desc: "cloud kms api name", apiName: "cloudkms.googleapis.com", - wantFeaturesEnabled: []featureID{EnableMTLSHardBoundTokens}, - wantFeatureDisabled: []featureID{EnableWrapperTypesForPageSize, EnableOrderedRoutingHeaders}, + wantFeaturesEnabled: []featureID{MTLSHardBoundTokensFeature}, + wantFeatureDisabled: []featureID{WrapperTypesForPageSizeFeature, OrderedRoutingHeadersFeature}, }, } { t.Run(tc.desc, func(t *testing.T) { @@ -84,14 +84,15 @@ func TestProcessLegacyEnablements(t *testing.T) { Name: tc.apiName, } } + g := &generator{cfg: cfg} processLegacyEnablements(cfg, req) for _, f := range tc.wantFeaturesEnabled { - if !cfg.FeatureEnabled(f) { + if !g.featureEnabled(f) { t.Errorf("expected feature %q enabled, was not", f) } } for _, f := range tc.wantFeatureDisabled { - if cfg.FeatureEnabled(f) { + if g.featureEnabled(f) { t.Errorf("expected feature %q to be disabled, was enabled", f) } } diff --git a/internal/gengapic/generator.go b/internal/gengapic/generator.go index e1efc8231da..2bc372b6cab 100644 --- a/internal/gengapic/generator.go +++ b/internal/gengapic/generator.go @@ -143,6 +143,17 @@ func newGenerator(req *pluginpb.CodeGeneratorRequest) (*generator, error) { return g, nil } +// featureEnabled is a simple boolean checker for probing if a given feature has been enabled. +func (g *generator) featureEnabled(f featureID) bool { + if g.cfg == nil { + return false + } + if val, ok := g.cfg.featureEnablement[f]; ok && val { + return true + } + return false +} + // printf formatted-prints to sb, using the print syntax from fmt package. // // It automatically keeps track of indentation caused by curly-braces. diff --git a/internal/gengapic/gengapic.go b/internal/gengapic/gengapic.go index 2394214e4ca..36fe3ee5fe7 100644 --- a/internal/gengapic/gengapic.go +++ b/internal/gengapic/gengapic.go @@ -456,7 +456,7 @@ func (g *generator) insertDynamicRequestHeaders(m *descriptorpb.MethodDescriptor g.printf(" routingHeadersMap[%q] = %s", headerName, regexHelper) g.printf("}") } - if g.cfg.FeatureEnabled(EnableOrderedRoutingHeaders) { + if g.featureEnabled(OrderedRoutingHeadersFeature) { for i := range headers { headerName := headers[i][2] g.printf("if headerValue, ok := routingHeadersMap[%q]; ok {", headerName) diff --git a/internal/gengapic/gengapic_test.go b/internal/gengapic/gengapic_test.go index 856a79d41fb..d076c8f2957 100644 --- a/internal/gengapic/gengapic_test.go +++ b/internal/gengapic/gengapic_test.go @@ -1469,7 +1469,7 @@ func TestInsertDynamicRequestHeaders_Ordering(t *testing.T) { t.Run(tc.pkgName, func(t *testing.T) { g.reset() g.cfg.featureEnablement = map[featureID]bool{ - EnableOrderedRoutingHeaders: tc.wantOrdered, + OrderedRoutingHeadersFeature: tc.wantOrdered, } file := &descriptorpb.FileDescriptorProto{ diff --git a/internal/gengapic/gengrpc.go b/internal/gengapic/gengrpc.go index ea93af1247f..d0c35da4d75 100644 --- a/internal/gengapic/gengrpc.go +++ b/internal/gengapic/gengrpc.go @@ -186,7 +186,7 @@ func (g *generator) grpcClientOptions(serv *descriptorpb.ServiceDescriptorProto, p(" internaloption.WithDefaultAudience(%q),", generateDefaultAudience(host)) p(" internaloption.WithDefaultScopes(DefaultAuthScopes()...),") p(" internaloption.EnableJwtWithScope(),") - if g.cfg.FeatureEnabled(EnableMTLSHardBoundTokens) { + if g.featureEnabled(MTLSHardBoundTokensFeature) { p("internaloption.AllowHardBoundTokens(\"MTLS_S2A\"),") } p(" internaloption.EnableNewAuthLibrary(),") diff --git a/internal/gengapic/options.go b/internal/gengapic/options.go index bd34e2af34f..74c1267324e 100644 --- a/internal/gengapic/options.go +++ b/internal/gengapic/options.go @@ -73,8 +73,8 @@ var SupportedValueArgs map[string]func(string) configOption = map[string]func(st // // Recommendation: avoid adding more of these as they complicate processing. var SupportedPrefixArgs map[string]func(string) configOption = map[string]func(string) configOption{ - "M": withPackageOverride, - "FEAT_": withFeature, + "M": withPackageOverride, + "F_": withFeature, } // Configuration needed to drive the operation of the plugin. @@ -127,17 +127,6 @@ type generatorConfig struct { featureEnablement map[featureID]bool } -// Simplify checking if generator feature is enabled. -func (cfg *generatorConfig) FeatureEnabled(f featureID) bool { - if cfg == nil { - return false - } - if val, ok := cfg.featureEnablement[f]; ok && val { - return true - } - return false -} - // Signature for configuration arguments. // Errors are part of the signature to allow options to provide validation feedback early. // Config options that return errors should not modify the configuration. @@ -401,7 +390,7 @@ func withPackageOverride(s string) configOption { } // withFeature handles boolean enablement of generator-level functionality. -// The `FEAT_` prefix is not passed as part of the string, only the remaining substring. +// The `feat_` prefix is not passed as part of the string, only the remaining substring. func withFeature(s string) configOption { fID := featureID(s) if _, ok := featureRegistry[fID]; !ok { diff --git a/internal/gengapic/options_test.go b/internal/gengapic/options_test.go index 83f0a7de050..984a74d1d0d 100644 --- a/internal/gengapic/options_test.go +++ b/internal/gengapic/options_test.go @@ -143,19 +143,33 @@ func TestParseOptions(t *testing.T) { }, { // Test empty parameter in the CSV. - param: "go-gapic-package=path/to/imp;pkg,FEAT_INVALID_FEATURE", + param: "go-gapic-package=path/to/imp;pkg,F_invalid_feature", expectErr: true, }, { - // Test empty parameter in the CSV. - param: "go-gapic-package=path;pkg,FEAT_ORDERED_ROUTING_HEADERS", + // single feature enablement + param: "go-gapic-package=path;pkg,F_ordered_routing_headers", + expectedCfg: &generatorConfig{ + transports: []transport{grpc}, + pkgPath: "path", + pkgName: "pkg", + outDir: "path", + featureEnablement: map[featureID]bool{ + OrderedRoutingHeadersFeature: true, + }, + }, + }, + { + // multiple feature enablement + param: "go-gapic-package=path;pkg,F_ordered_routing_headers,F_wrapper_types_for_page_size", expectedCfg: &generatorConfig{ transports: []transport{grpc}, pkgPath: "path", pkgName: "pkg", outDir: "path", featureEnablement: map[featureID]bool{ - EnableOrderedRoutingHeaders: true, + OrderedRoutingHeadersFeature: true, + WrapperTypesForPageSizeFeature: true, }, }, }, diff --git a/internal/gengapic/paging.go b/internal/gengapic/paging.go index cad47809a11..ad0dda1b073 100644 --- a/internal/gengapic/paging.go +++ b/internal/gengapic/paging.go @@ -198,7 +198,7 @@ func (g *generator) getPagingFields(m *descriptorpb.MethodDescriptorProto) (repe hasPageToken := false for _, f := range inMsg.GetField() { - candidate, needsWrapper := isPageSizeField(f, g.cfg.FeatureEnabled(EnableWrapperTypesForPageSize)) + candidate, needsWrapper := isPageSizeField(f, g.featureEnabled(WrapperTypesForPageSizeFeature)) if candidate { if pageSizeField == nil { pageSizeField = f diff --git a/internal/gengapic/paging_test.go b/internal/gengapic/paging_test.go index 9380ffc8d3b..a211d3d5b68 100644 --- a/internal/gengapic/paging_test.go +++ b/internal/gengapic/paging_test.go @@ -496,7 +496,7 @@ func TestPagingField(t *testing.T) { g.cfg = &generatorConfig{ transports: []transport{rest}, featureEnablement: map[featureID]bool{ - EnableWrapperTypesForPageSize: true, + WrapperTypesForPageSizeFeature: true, }, } for _, tst := range []struct { From e0a73d02050cec9e4b7bcfe70f1b1163b7d62d64 Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Wed, 28 Jan 2026 22:35:42 +0000 Subject: [PATCH 08/10] docs --- internal/gengapic/options.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/gengapic/options.go b/internal/gengapic/options.go index 74c1267324e..ff37d0ef06a 100644 --- a/internal/gengapic/options.go +++ b/internal/gengapic/options.go @@ -390,7 +390,7 @@ func withPackageOverride(s string) configOption { } // withFeature handles boolean enablement of generator-level functionality. -// The `feat_` prefix is not passed as part of the string, only the remaining substring. +// The `F_` prefix is not passed as part of the string, only the remaining substring. func withFeature(s string) configOption { fID := featureID(s) if _, ok := featureRegistry[fID]; !ok { From bcc7e87b8ccb81f4e54b2b38fc5232f3aeaac5bb Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Thu, 29 Jan 2026 23:25:25 +0000 Subject: [PATCH 09/10] bool -> struct{} for feature map --- internal/gengapic/client_init_test.go | 4 ++-- internal/gengapic/feature.go | 8 ++++---- internal/gengapic/generator.go | 2 +- internal/gengapic/gengapic_test.go | 5 +++-- internal/gengapic/options.go | 6 +++--- internal/gengapic/options_test.go | 10 +++++----- internal/gengapic/paging_test.go | 4 ++-- 7 files changed, 20 insertions(+), 19 deletions(-) diff --git a/internal/gengapic/client_init_test.go b/internal/gengapic/client_init_test.go index acc15953fb2..e370139415d 100644 --- a/internal/gengapic/client_init_test.go +++ b/internal/gengapic/client_init_test.go @@ -136,8 +136,8 @@ func TestClientOpt(t *testing.T) { }, gRPCServiceConfig: grpcConf, // Showcase would enable MTLS if we went through legacy enablements, so add it explicitly here. - featureEnablement: map[featureID]bool{ - MTLSHardBoundTokensFeature: true, + featureEnablement: map[featureID]struct{}{ + MTLSHardBoundTokensFeature: struct{}{}, }, }, } diff --git a/internal/gengapic/feature.go b/internal/gengapic/feature.go index dc75a0cc4da..59a3d765dbb 100644 --- a/internal/gengapic/feature.go +++ b/internal/gengapic/feature.go @@ -104,9 +104,9 @@ func processLegacyEnablements(cfg *generatorConfig, req *pluginpb.CodeGeneratorR for _, v := range packages { if probePackage == v { // matched if cfg.featureEnablement == nil { - cfg.featureEnablement = make(map[featureID]bool) + cfg.featureEnablement = make(map[featureID]struct{}) } - cfg.featureEnablement[f] = true + cfg.featureEnablement[f] = struct{}{} break } } @@ -119,9 +119,9 @@ func processLegacyEnablements(cfg *generatorConfig, req *pluginpb.CodeGeneratorR for _, v := range apis { if probeName == v { // matched if cfg.featureEnablement == nil { - cfg.featureEnablement = make(map[featureID]bool) + cfg.featureEnablement = make(map[featureID]struct{}) } - cfg.featureEnablement[f] = true + cfg.featureEnablement[f] = struct{}{} break } } diff --git a/internal/gengapic/generator.go b/internal/gengapic/generator.go index 2bc372b6cab..77e42c8a925 100644 --- a/internal/gengapic/generator.go +++ b/internal/gengapic/generator.go @@ -148,7 +148,7 @@ func (g *generator) featureEnabled(f featureID) bool { if g.cfg == nil { return false } - if val, ok := g.cfg.featureEnablement[f]; ok && val { + if _, ok := g.cfg.featureEnablement[f]; ok { return true } return false diff --git a/internal/gengapic/gengapic_test.go b/internal/gengapic/gengapic_test.go index d076c8f2957..607dcf0b5e7 100644 --- a/internal/gengapic/gengapic_test.go +++ b/internal/gengapic/gengapic_test.go @@ -1468,8 +1468,9 @@ func TestInsertDynamicRequestHeaders_Ordering(t *testing.T) { for _, tc := range tests { t.Run(tc.pkgName, func(t *testing.T) { g.reset() - g.cfg.featureEnablement = map[featureID]bool{ - OrderedRoutingHeadersFeature: tc.wantOrdered, + g.cfg.featureEnablement = make(map[featureID]struct{}) + if tc.wantOrdered { + g.cfg.featureEnablement[OrderedRoutingHeadersFeature] = struct{}{} } file := &descriptorpb.FileDescriptorProto{ diff --git a/internal/gengapic/options.go b/internal/gengapic/options.go index ff37d0ef06a..9de14f5c57a 100644 --- a/internal/gengapic/options.go +++ b/internal/gengapic/options.go @@ -124,7 +124,7 @@ type generatorConfig struct { pkgOverrides map[string]string // Keep track of what conditional behaviors/experiments are enabled. - featureEnablement map[featureID]bool + featureEnablement map[featureID]struct{} } // Signature for configuration arguments. @@ -400,9 +400,9 @@ func withFeature(s string) configOption { } return func(cfg *generatorConfig) error { if cfg.featureEnablement == nil { - cfg.featureEnablement = make(map[featureID]bool) + cfg.featureEnablement = make(map[featureID]struct{}) } - cfg.featureEnablement[fID] = true + cfg.featureEnablement[fID] = struct{}{} return nil } } diff --git a/internal/gengapic/options_test.go b/internal/gengapic/options_test.go index 984a74d1d0d..a3f87de62a3 100644 --- a/internal/gengapic/options_test.go +++ b/internal/gengapic/options_test.go @@ -154,8 +154,8 @@ func TestParseOptions(t *testing.T) { pkgPath: "path", pkgName: "pkg", outDir: "path", - featureEnablement: map[featureID]bool{ - OrderedRoutingHeadersFeature: true, + featureEnablement: map[featureID]struct{}{ + OrderedRoutingHeadersFeature: struct{}{}, }, }, }, @@ -167,9 +167,9 @@ func TestParseOptions(t *testing.T) { pkgPath: "path", pkgName: "pkg", outDir: "path", - featureEnablement: map[featureID]bool{ - OrderedRoutingHeadersFeature: true, - WrapperTypesForPageSizeFeature: true, + featureEnablement: map[featureID]struct{}{ + OrderedRoutingHeadersFeature: struct{}{}, + WrapperTypesForPageSizeFeature: struct{}{}, }, }, }, diff --git a/internal/gengapic/paging_test.go b/internal/gengapic/paging_test.go index a211d3d5b68..309bcbeffbf 100644 --- a/internal/gengapic/paging_test.go +++ b/internal/gengapic/paging_test.go @@ -495,8 +495,8 @@ func TestPagingField(t *testing.T) { } g.cfg = &generatorConfig{ transports: []transport{rest}, - featureEnablement: map[featureID]bool{ - WrapperTypesForPageSizeFeature: true, + featureEnablement: map[featureID]struct{}{ + WrapperTypesForPageSizeFeature: struct{}{}, }, } for _, tst := range []struct { From 88944cab091999ae832392bce51ec628cf6cccfc Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Thu, 29 Jan 2026 23:34:32 +0000 Subject: [PATCH 10/10] add bugref --- internal/gengapic/generator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/gengapic/generator.go b/internal/gengapic/generator.go index 77e42c8a925..5addc9718fa 100644 --- a/internal/gengapic/generator.go +++ b/internal/gengapic/generator.go @@ -99,7 +99,7 @@ func newGenerator(req *pluginpb.CodeGeneratorRequest) (*generator, error) { return nil, err } // Handle legacy enablement via hardcoded allowlists. - // This logic should be removed when legacy enablement is no longer needed. + // This logic should be removed when legacy enablement is no longer needed. (b/264668184) processLegacyEnablements(cfg, req) // attach config to generator.