From 1840462a845a93b14e4ad52cc0151efe1da0bcd2 Mon Sep 17 00:00:00 2001 From: Jingyi Hu Date: Thu, 12 Dec 2024 07:23:14 +0000 Subject: [PATCH 1/4] tool: support config file in codegen --- .../config/bigquerydatatransfer.yaml | 21 ++++++ .../controllerbuilder/pkg/codegen/config.go | 53 +++++++++++++++ .../generatemapper/generatemappercommand.go | 58 ++++++++++++----- .../generatetypes/generatetypescommand.go | 65 ++++++++++++------- .../pkg/options/generateoptions.go | 2 + 5 files changed, 162 insertions(+), 37 deletions(-) create mode 100644 dev/tools/controllerbuilder/config/bigquerydatatransfer.yaml create mode 100644 dev/tools/controllerbuilder/pkg/codegen/config.go diff --git a/dev/tools/controllerbuilder/config/bigquerydatatransfer.yaml b/dev/tools/controllerbuilder/config/bigquerydatatransfer.yaml new file mode 100644 index 0000000000..5d6f8a4f5f --- /dev/null +++ b/dev/tools/controllerbuilder/config/bigquerydatatransfer.yaml @@ -0,0 +1,21 @@ +# Copyright 2024 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 +# +# http://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. + +service: google.cloud.bigquery.datatransfer.v1 +apiVersion: bigquerydatatransfer.cnrm.cloud.google.com/v1beta1 +generateMapper: true +resources: + - kind: BigQueryDataTransferConfig + protoName: TransferConfig + skipScaffoldFiles: true # files were scaffolded using a previous template, making them incompatible with the new scaffolding template. diff --git a/dev/tools/controllerbuilder/pkg/codegen/config.go b/dev/tools/controllerbuilder/pkg/codegen/config.go new file mode 100644 index 0000000000..b6d97838c7 --- /dev/null +++ b/dev/tools/controllerbuilder/pkg/codegen/config.go @@ -0,0 +1,53 @@ +// Copyright 2024 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 +// +// http://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 codegen + +import ( + "fmt" + "os" + + "gopkg.in/yaml.v3" +) + +type ServiceConfig struct { + Service string `yaml:"service"` + APIVersion string `yaml:"apiVersion"` + GenerateMapper bool `yaml:"generateMapper"` + Resources []ResourceConfig `yaml:"resources"` +} + +type ResourceConfig struct { + Kind string `yaml:"kind"` + ProtoName string `yaml:"protoName"` + SkipScaffoldFiles bool `yaml:"skipScaffoldFiles"` +} + +func LoadConfig(configPath string) (*ServiceConfig, error) { + if configPath == "" { + return nil, nil + } + + data, err := os.ReadFile(configPath) + if err != nil { + return nil, fmt.Errorf("failed to read config file: %v", err) + } + + var config ServiceConfig + if err := yaml.Unmarshal(data, &config); err != nil { + return nil, fmt.Errorf("failed to parse config file: %v", err) + } + + return &config, nil +} diff --git a/dev/tools/controllerbuilder/pkg/commands/generatemapper/generatemappercommand.go b/dev/tools/controllerbuilder/pkg/commands/generatemapper/generatemappercommand.go index e780beb871..cdf3d7d7c1 100644 --- a/dev/tools/controllerbuilder/pkg/commands/generatemapper/generatemappercommand.go +++ b/dev/tools/controllerbuilder/pkg/commands/generatemapper/generatemappercommand.go @@ -68,22 +68,10 @@ func BuildCommand(baseOptions *options.GenerateOptions) *cobra.Command { Use: "generate-mapper", Short: "generate mapper functions for a proto service", PreRunE: func(cmd *cobra.Command, args []string) error { - if opt.ServiceName == "" { - return fmt.Errorf("ServiceName is required") - } - if opt.GenerateOptions.ProtoSourcePath == "" { - return fmt.Errorf("ProtoSourcePath is required") - } - if opt.APIGoPackagePath == "" { - return fmt.Errorf("GoPackagePath is required") - } - if opt.OutputMapperDirectory == "" { - return fmt.Errorf("OutputMapperDirectory is required") - } - if opt.APIVersion == "" { - return fmt.Errorf("APIVersion is required") + if err := opt.loadAndApplyConfig(); err != nil { + return err } - return nil + return opt.validate() }, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() @@ -154,3 +142,43 @@ func RunGenerateMapper(ctx context.Context, o *GenerateMapperOptions) error { return nil } + +func (o *GenerateMapperOptions) loadAndApplyConfig() error { + if o.ConfigFilePath == "" { + return nil + } + config, err := codegen.LoadConfig(o.ConfigFilePath) + if err != nil { + return fmt.Errorf("loading service config: %w", err) + } + if config == nil { + return nil + } + + if !config.GenerateMapper { + return fmt.Errorf("mapper generation is disabled for this service in config file %s", o.ConfigFilePath) + } + + o.ServiceName = config.Service + o.APIVersion = config.APIVersion + return nil +} + +func (o *GenerateMapperOptions) validate() error { + if o.ServiceName == "" { + return fmt.Errorf("ServiceName is required") + } + if o.GenerateOptions.ProtoSourcePath == "" { + return fmt.Errorf("ProtoSourcePath is required") + } + if o.APIGoPackagePath == "" { + return fmt.Errorf("GoPackagePath is required") + } + if o.OutputMapperDirectory == "" { + return fmt.Errorf("OutputMapperDirectory is required") + } + if o.APIVersion == "" { + return fmt.Errorf("APIVersion is required") + } + return nil +} diff --git a/dev/tools/controllerbuilder/pkg/commands/generatetypes/generatetypescommand.go b/dev/tools/controllerbuilder/pkg/commands/generatetypes/generatetypescommand.go index 43f5586401..bf0093420e 100644 --- a/dev/tools/controllerbuilder/pkg/commands/generatetypes/generatetypescommand.go +++ b/dev/tools/controllerbuilder/pkg/commands/generatetypes/generatetypescommand.go @@ -19,7 +19,6 @@ import ( "fmt" "os" "strings" - "unicode" "github.com/GoogleCloudPlatform/k8s-config-connector/dev/tools/controllerbuilder/pkg/codegen" "github.com/GoogleCloudPlatform/k8s-config-connector/dev/tools/controllerbuilder/pkg/options" @@ -42,8 +41,9 @@ type GenerateCRDOptions struct { } type Resource struct { - Kind string - ProtoName string + Kind string + ProtoName string + SkipScaffoldFiles bool } type ResourceList []Resource @@ -84,7 +84,7 @@ func (o *GenerateCRDOptions) InitDefaults() error { func (o *GenerateCRDOptions) BindFlags(cmd *cobra.Command) { cmd.Flags().StringVar(&o.OutputAPIDirectory, "output-api", o.OutputAPIDirectory, "base directory for writing APIs") cmd.Flags().Var(&o.Resources, "resource", "the KRM Kind and the equivalent proto resource separated with a colon. e.g. for resource google.storage.v1.Bucket, the flag should be `StorageBucket:Bucket`. Can be specified multiple times.") - cmd.Flags().BoolVar(&o.SkipScaffoldFiles, "skip-scaffold-files", false, "skip generating scaffold files (types, refs, and identity)") + cmd.Flags().BoolVar(&o.SkipScaffoldFiles, "skip-scaffold-files", false, "skip generating scaffold files (types, refs, and identity) for all resources generated by this command") } func BuildCommand(baseOptions *options.GenerateOptions) *cobra.Command { @@ -101,16 +101,10 @@ func BuildCommand(baseOptions *options.GenerateOptions) *cobra.Command { Use: "generate-types", Short: "generate KRM types for a proto service", PreRunE: func(cmd *cobra.Command, args []string) error { - if opt.ServiceName == "" { - return fmt.Errorf("`service` is required") - } - if opt.GenerateOptions.ProtoSourcePath == "" { - return fmt.Errorf("`proto-source-path` is required") - } - if len(opt.Resources) == 0 { - return fmt.Errorf("`--resource` is required") + if err := opt.loadAndApplyConfig(); err != nil { + return err } - return nil + return opt.validate() }, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() @@ -129,8 +123,6 @@ func BuildCommand(baseOptions *options.GenerateOptions) *cobra.Command { func RunGenerateCRD(ctx context.Context, o *GenerateCRDOptions) error { log := klog.FromContext(ctx) - // o.ResourceProtoName = capitalizeFirstRune(o.ResourceProtoName) - gv, err := schema.ParseGroupVersion(o.APIVersion) if err != nil { return fmt.Errorf("APIVersion %q is not valid: %w", o.APIVersion, err) @@ -173,7 +165,8 @@ func RunGenerateCRD(ctx context.Context, o *GenerateCRDOptions) error { return err } - if o.SkipScaffoldFiles { + skipScaffold := o.SkipScaffoldFiles || resource.SkipScaffoldFiles + if skipScaffold { log.Info("skipping scaffolding type, refs and identity files", "resource", resource.ProtoName) } else { kind := resource.Kind @@ -216,11 +209,39 @@ func RunGenerateCRD(ctx context.Context, o *GenerateCRDOptions) error { return nil } -func capitalizeFirstRune(s string) string { - if s == "" { - return s +func (o *GenerateCRDOptions) loadAndApplyConfig() error { + if o.ConfigFilePath == "" { + return nil } - runes := []rune(s) - runes[0] = unicode.ToUpper(runes[0]) - return string(runes) + config, err := codegen.LoadConfig(o.ConfigFilePath) + if err != nil { + return fmt.Errorf("loading service config: %w", err) + } + if config == nil { + return nil + } + + o.ServiceName = config.Service + o.APIVersion = config.APIVersion + for _, res := range config.Resources { + o.Resources = append(o.Resources, Resource{ + Kind: res.Kind, + ProtoName: res.ProtoName, + SkipScaffoldFiles: res.SkipScaffoldFiles, + }) + } + return nil +} + +func (o *GenerateCRDOptions) validate() error { + if o.ServiceName == "" { + return fmt.Errorf("`service` is required") + } + if o.GenerateOptions.ProtoSourcePath == "" { + return fmt.Errorf("`proto-source-path` is required") + } + if len(o.Resources) == 0 { + return fmt.Errorf("`--resource` is required") + } + return nil } diff --git a/dev/tools/controllerbuilder/pkg/options/generateoptions.go b/dev/tools/controllerbuilder/pkg/options/generateoptions.go index 5006796a4b..622c276905 100644 --- a/dev/tools/controllerbuilder/pkg/options/generateoptions.go +++ b/dev/tools/controllerbuilder/pkg/options/generateoptions.go @@ -25,6 +25,7 @@ type GenerateOptions struct { ProtoSourcePath string ServiceName string APIVersion string + ConfigFilePath string } func (o *GenerateOptions) InitDefaults() error { @@ -40,6 +41,7 @@ func (o *GenerateOptions) BindPersistentFlags(cmd *cobra.Command) { cmd.PersistentFlags().StringVar(&o.ProtoSourcePath, "proto-source-path", o.ProtoSourcePath, "path to (compiled) proto for APIs") cmd.PersistentFlags().StringVarP(&o.APIVersion, "api-version", "v", o.APIVersion, "the KRM API version. used to import the KRM API") cmd.PersistentFlags().StringVarP(&o.ServiceName, "service", "s", o.ServiceName, "the GCP service name") + cmd.PersistentFlags().StringVar(&o.ConfigFilePath, "config", "", "path to service config file, the config file will override other flags") } func RepoRoot() (string, error) { From e72782fac980ff3c0249ee7074796e38c3f326db Mon Sep 17 00:00:00 2001 From: Jingyi Hu Date: Mon, 16 Dec 2024 23:17:18 +0000 Subject: [PATCH 2/4] feat: add presubmit validation for generated types --- Makefile | 12 +++++++++++- scripts/validate-prereqs.sh | 15 +++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index e424106a39..9e83ff48d2 100644 --- a/Makefile +++ b/Makefile @@ -391,4 +391,14 @@ BILLING_ACCOUNT ?= 010E8D-490B6B-088E1C operator-e2e-tests: export TEST_ORG_ID=${ORG_ID} export TEST_BILLING_ACCOUNT_ID=${BILLING_ACCOUNT} - cd operator/tests/e2e/ && go test --project-id=${PROJECT_ID} \ No newline at end of file + cd operator/tests/e2e/ && go test --project-id=${PROJECT_ID} + +# Generate Go types for direct resources specified in the config files located under `dev/tools/controllerbuilder/config`. +.PHONY: generate-types +generate-types: + cd dev/tools/controllerbuilder && \ + ./generate-proto.sh && \ + for config in config/*.yaml; do \ + go run . generate-types --config $$config; \ + done + dev/tasks/fix-gofmt diff --git a/scripts/validate-prereqs.sh b/scripts/validate-prereqs.sh index 445b332342..3704b25a05 100755 --- a/scripts/validate-prereqs.sh +++ b/scripts/validate-prereqs.sh @@ -93,3 +93,18 @@ if [[ "${changed_file_count}" != "0" ]] || [[ "${added_reference_doc_file_count} git ls-files --others --exclude-standard scripts/generate-google3-docs/resource-reference/generated/ exit 1 fi + +### This check ensures that the generated Go types for direct resources are not manually modified by accident. +### Ensures that the code generation tools can be safely re-run. +make generate-types +changed_file_count=$(git diff --name-only | wc -l) +if [[ "${changed_file_count}" != "0" ]]; then + echo "Full diff:" + git diff + echo "ERROR: The generated types are outdated. Run 'make generate-types' to update them." + echo "If you need to modify any types, first move them out of the generated file." + echo "Then run 'make generate-types' again to ensure the generated file remains unchanged." + echo "Affected files:" + git diff --name-only + exit 1 +fi \ No newline at end of file From a0c3cf8850b6cd2f979f7afb124785119c7df3d3 Mon Sep 17 00:00:00 2001 From: Jingyi Hu Date: Tue, 17 Dec 2024 00:59:51 +0000 Subject: [PATCH 3/4] chore: add experimental flag required by protoc v3.12.x Example error when running on Github ubuntu-22.04: google/bigtable/admin/v2/instance.proto: This file contains proto3 optional fields, but --experimental_allow_proto3_optional was not set. --- dev/tools/controllerbuilder/generate-proto.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/dev/tools/controllerbuilder/generate-proto.sh b/dev/tools/controllerbuilder/generate-proto.sh index ac0d6434fd..647d07c7be 100755 --- a/dev/tools/controllerbuilder/generate-proto.sh +++ b/dev/tools/controllerbuilder/generate-proto.sh @@ -44,6 +44,7 @@ fi protoc --include_imports --include_source_info \ + --experimental_allow_proto3_optional \ -I ${THIRD_PARTY}/googleapis/ \ -I ${REPO_ROOT}/mockgcp/apis \ ${REPO_ROOT}/mockgcp/apis/mockgcp/cloud/networkconnectivity/*/*.proto \ From 8eb435a8445b3781d174b1fa33c28c8a6656a0fa Mon Sep 17 00:00:00 2001 From: Jingyi Hu Date: Mon, 16 Dec 2024 23:32:13 +0000 Subject: [PATCH 4/4] chore: regenerate for BigQueryDataTransferConfig --- .../bigquerydatatransfer/v1beta1/types.generated.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/apis/bigquerydatatransfer/v1beta1/types.generated.go b/apis/bigquerydatatransfer/v1beta1/types.generated.go index b3054fcc25..d1f3cd0d9f 100644 --- a/apis/bigquerydatatransfer/v1beta1/types.generated.go +++ b/apis/bigquerydatatransfer/v1beta1/types.generated.go @@ -17,6 +17,7 @@ package v1beta1 // +kcc:proto=google.cloud.bigquery.datatransfer.v1.EmailPreferences type EmailPreferences struct { // If true, email notifications will be sent on transfer run failures. + // +kcc:proto:field=google.cloud.bigquery.datatransfer.v1.EmailPreferences.enable_failure_email EnableFailureEmail *bool `json:"enableFailureEmail,omitempty"` } @@ -30,6 +31,7 @@ type ScheduleOptions struct { // will be disabled. The runs can be started on ad-hoc basis using // StartManualTransferRuns API. When automatic scheduling is disabled, the // TransferConfig.schedule field will be ignored. + // +kcc:proto:field=google.cloud.bigquery.datatransfer.v1.ScheduleOptions.disable_auto_scheduling DisableAutoScheduling *bool `json:"disableAutoScheduling,omitempty"` // Specifies time to start scheduling transfer runs. The first run will be @@ -37,12 +39,14 @@ type ScheduleOptions struct { // defined in the schedule string. The start time can be changed at any // moment. The time when a data transfer can be triggered manually is not // limited by this option. + // +kcc:proto:field=google.cloud.bigquery.datatransfer.v1.ScheduleOptions.start_time StartTime *string `json:"startTime,omitempty"` // Defines time to stop scheduling transfer runs. A transfer run cannot be // scheduled at or after the end time. The end time can be changed at any // moment. The time when a data transfer can be triggered manually is not // limited by this option. + // +kcc:proto:field=google.cloud.bigquery.datatransfer.v1.ScheduleOptions.end_time EndTime *string `json:"endTime,omitempty"` } @@ -50,16 +54,19 @@ type ScheduleOptions struct { type ScheduleOptionsV2 struct { // Time based transfer schedule options. This is the default schedule // option. + // +kcc:proto:field=google.cloud.bigquery.datatransfer.v1.ScheduleOptionsV2.time_based_schedule TimeBasedSchedule *TimeBasedSchedule `json:"timeBasedSchedule,omitempty"` // Manual transfer schedule. If set, the transfer run will not be // auto-scheduled by the system, unless the client invokes // StartManualTransferRuns. This is equivalent to // disable_auto_scheduling = true. + // +kcc:proto:field=google.cloud.bigquery.datatransfer.v1.ScheduleOptionsV2.manual_schedule ManualSchedule *ManualSchedule `json:"manualSchedule,omitempty"` // Event driven transfer schedule options. If set, the transfer will be // scheduled upon events arrial. + // +kcc:proto:field=google.cloud.bigquery.datatransfer.v1.ScheduleOptionsV2.event_driven_schedule EventDrivenSchedule *EventDrivenSchedule `json:"eventDrivenSchedule,omitempty"` } @@ -78,23 +85,27 @@ type TimeBasedSchedule struct { // // NOTE: The minimum interval time between recurring transfers depends on the // data source; refer to the documentation for your data source. + // +kcc:proto:field=google.cloud.bigquery.datatransfer.v1.TimeBasedSchedule.schedule Schedule *string `json:"schedule,omitempty"` // Specifies time to start scheduling transfer runs. The first run will be // scheduled at or after the start time according to a recurrence pattern // defined in the schedule string. The start time can be changed at any // moment. + // +kcc:proto:field=google.cloud.bigquery.datatransfer.v1.TimeBasedSchedule.start_time StartTime *string `json:"startTime,omitempty"` // Defines time to stop scheduling transfer runs. A transfer run cannot be // scheduled at or after the end time. The end time can be changed at any // moment. + // +kcc:proto:field=google.cloud.bigquery.datatransfer.v1.TimeBasedSchedule.end_time EndTime *string `json:"endTime,omitempty"` } // +kcc:proto=google.cloud.bigquery.datatransfer.v1.UserInfo type UserInfo struct { // E-mail address of the user. + // +kcc:proto:field=google.cloud.bigquery.datatransfer.v1.UserInfo.email Email *string `json:"email,omitempty"` } @@ -127,8 +138,10 @@ type Any struct { // // Schemes other than `http`, `https` (or the empty scheme) might be // used with implementation specific semantics. + // +kcc:proto:field=google.protobuf.Any.type_url TypeURL *string `json:"typeURL,omitempty"` // Must be a valid serialized protocol buffer of the above specified type. + // +kcc:proto:field=google.protobuf.Any.value Value []byte `json:"value,omitempty"` }