diff --git a/apis/networkservices/v1alpha1/generate.sh b/apis/networkservices/v1alpha1/generate.sh new file mode 100755 index 00000000000..fbb48302c1b --- /dev/null +++ b/apis/networkservices/v1alpha1/generate.sh @@ -0,0 +1,35 @@ +#!/bin/bash +# Copyright 2025 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. + +set -o errexit +set -o nounset +set -o pipefail + +REPO_ROOT="$(git rev-parse --show-toplevel)" +cd ${REPO_ROOT}/dev/tools/controllerbuilder + +go run . generate-types \ + --service google.cloud.networkservices.v1 \ + --api-version "networkservices.cnrm.cloud.google.com/v1alpha1" \ + --resource NetworkServicesServiceBinding:ServiceBinding + +go run . generate-mapper \ + --service google.cloud.networkservices.v1 \ + --api-version "networkservices.cnrm.cloud.google.com/v1alpha1" + +cd ${REPO_ROOT} +dev/tasks/generate-crds + +go run -mod=readonly golang.org/x/tools/cmd/goimports@latest -w pkg/controller/direct/networkservices/ diff --git a/apis/networkservices/v1alpha1/types.generated.go b/apis/networkservices/v1alpha1/types.generated.go index c880469b476..8309cb2a5ed 100644 --- a/apis/networkservices/v1alpha1/types.generated.go +++ b/apis/networkservices/v1alpha1/types.generated.go @@ -12,4 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. +// +generated:types +// krm.group: networkservices.cnrm.cloud.google.com +// krm.version: v1alpha1 +// proto.service: google.cloud.networkservices.v1 +// resource: NetworkServicesServiceBinding:ServiceBinding + package v1alpha1 diff --git a/docs/ai/unify-api-mapper-fuzzer.md b/docs/ai/unify-api-mapper-fuzzer.md new file mode 100644 index 00000000000..ffb42676a0b --- /dev/null +++ b/docs/ai/unify-api-mapper-fuzzer.md @@ -0,0 +1,111 @@ +# How to Unify API Mappers and Fuzzers for Existing Resources + +This guide provides a step-by-step process for refactoring and unifying the API, mapper, and fuzzer for existing resources. The goal is to leverage the auto-generation tools as much as possible, reduce manual code, and ensure consistency across different API versions. + +## 1. Start with `generate.sh` + +The unification process begins with the `generate.sh` script located in the `apis//` directory (e.g., `apis/dataproc/v1alpha1/`). This script is the source of truth for the auto-generation process. + +### Verify and Correct `generate.sh` + +1. **Check for Existence**: Ensure a `generate.sh` script exists for each version of the service you are unifying (e.g., `v1alpha1`, `v1beta1`). If it doesn't exist, create one using a known good example like `apis/bigquerybiglake/v1alpha1/generate.sh` as a template. + +2. **Verify Resource List**: Open the `generate.sh` script and check the `--resource` flags in the `go run . generate-types` command. This list should accurately reflect all the resources defined within that API version directory. Add or remove `--resource` flags as necessary. + +3. **Check Output Directory**: Ensure the final `go run ... goimports` command is writing to the correct controller directory, which should be `pkg/controller/direct//`. + +A corrected `generate.sh` for a `v1alpha1` service might look like this: + +```bash +#!/bin/bash +# ... (license header) +set -o errexit +set -o nounset +set -o pipefail + +REPO_ROOT="$(git rev-parse --show-toplevel)" +cd ${REPO_ROOT}/dev/tools/controllerbuilder + +go run . generate-types \ + --service google.cloud.myservice.v1 \ + --api-version "myservice.cnrm.cloud.google.com/v1alpha1" \ + --resource MyResourceOne:ResourceOne \ + --resource MyResourceTwo:ResourceTwo + +go run . generate-mapper \ + --service google.cloud.myservice.v1 \ + --api-version "myservice.cnrm.cloud.google.com/v1alpha1" + +cd ${REPO_ROOT} +dev/tasks/generate-crds + +go run -mod=readonly golang.org/x/tools/cmd/goimports@latest -w pkg/controller/direct/myservice/ +``` + +## 2. Rely on Auto-Generated Mappers + +The primary goal of unification is to remove as much manual mapping logic as possible and rely on the code generated by `generate-mapper`. + +1. **Run `generate.sh`**: Execute the script for each API version you are unifying. This will create or update the `types.mapper.go` file in the controller directory (`pkg/controller/direct//`). + +2. **Identify Manual Mappers**: Look for manually written mapper functions in files like `_mapper.go`. These are the functions you want to replace. + +3. **Replace with Generated Functions**: Update your controller and fuzzer code to call the new, auto-generated functions from `types.mapper.go` instead of the old manual ones. + +4. **Delete Old Code**: Once all references to the manual mapper functions have been removed, you can delete the `_mapper.go` file or remove the now-unused functions from it. + +## 3. Handling Multi-Version Services + +When a service has multiple API versions (e.g., `v1alpha1` and `v1beta1`), the mapper needs to handle potential differences in the resource schemas. + +### Use the `--multiversion` Flag + +For the newer version of the API (e.g., `v1beta1`), add the `--multiversion` flag to the `generate-mapper` command in its `generate.sh` script. + +Example for `apis/myservice/v1beta1/generate.sh`: +```bash +# ... +go run . generate-mapper \ + --multiversion \ + --service google.cloud.myservice.v1 \ + --api-version "myservice.cnrm.cloud.google.com/v1beta1" +# ... +``` + +When this flag is used, the generator will create version-specific mapping functions to avoid conflicts. The function names will have the version injected into their name, for example: +* `MyResourceSpec_v1beta1_ToProto(...)` +* `MyResourceSpec_v1alpha1_FromProto(...)` + +**Note**: If the `--multiversion` flag is *not* used (typically for services with only one version), the generated function names will **not** have a version in them (e.g., `MyResourceSpec_ToProto(...)`). + +## 4. Update Fuzzer and Controller Code + +After re-generating the mappers, you must update your fuzzer and controller files to use the correct function names. + +1. **Ensure Fuzzer File Exists**: Ensure each CRD being unified has a corresponding `_fuzzer.go` file. If one does not exist, create it by copying from a similar resource and adapting it. + +2. **Update `*_fuzzer.go`**: Go to `pkg/controller/direct//_fuzzer.go` and update the `fuzztesting.NewKRMTypedFuzzer` call to use the new generated function names. + + * If you used `--multiversion`, the names will be version-specific. + * If you did not use `--multiversion`, the names will be generic. + + **Example with `--multiversion`:** + ```go + // For the v1beta1 fuzzer + f := fuzztesting.NewKRMTypedFuzzer(&pb.MyResource{}, + MyResourceSpec_v1beta1_FromProto, MyResourceSpec_v1beta1_ToProto, + MyResourceObservedState_v1beta1_FromProto, MyResourceObservedState_v1beta1_ToProto, + ) + ``` + + **Example without `--multiversion`:** + ```go + f := fuzztesting.NewKRMTypedFuzzer(&pb.MyResource{}, + MyResourceSpec_FromProto, MyResourceSpec_ToProto, + MyResourceObservedState_FromProto, MyResourceObservedState_ToProto, + ) + ``` + +3. **Update `*_controller.go`**: Search for any remaining usages of the old function names in your controller logic (`pkg/controller/direct//_controller.go`) and update them to point to the new generated functions. + +By following these steps, you can systematically unify the API definitions, mappers, and fuzzers for existing resources, leading to a more maintainable and consistent codebase. diff --git a/pkg/controller/direct/networkservices/mapper.generated.go b/pkg/controller/direct/networkservices/mapper.generated.go index 23f323294fd..6b77482d5c6 100644 --- a/pkg/controller/direct/networkservices/mapper.generated.go +++ b/pkg/controller/direct/networkservices/mapper.generated.go @@ -22,6 +22,7 @@ package networkservices import ( pb "cloud.google.com/go/networkservices/apiv1/networkservicespb" krm "github.com/GoogleCloudPlatform/k8s-config-connector/apis/networkservices/v1alpha1" + krmservicedirectoryv1alpha1 "github.com/GoogleCloudPlatform/k8s-config-connector/apis/servicedirectory/v1alpha1" "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/controller/direct" ) @@ -33,6 +34,7 @@ func NetworkServicesServiceBindingObservedState_FromProto(mapCtx *direct.MapCont // MISSING: Name out.CreateTime = direct.StringTimestamp_FromProto(mapCtx, in.GetCreateTime()) out.UpdateTime = direct.StringTimestamp_FromProto(mapCtx, in.GetUpdateTime()) + // MISSING: ServiceID return out } func NetworkServicesServiceBindingObservedState_ToProto(mapCtx *direct.MapContext, in *krm.NetworkServicesServiceBindingObservedState) *pb.ServiceBinding { @@ -43,5 +45,34 @@ func NetworkServicesServiceBindingObservedState_ToProto(mapCtx *direct.MapContex // MISSING: Name out.CreateTime = direct.StringTimestamp_ToProto(mapCtx, in.CreateTime) out.UpdateTime = direct.StringTimestamp_ToProto(mapCtx, in.UpdateTime) + // MISSING: ServiceID + return out +} +func NetworkServicesServiceBindingSpec_FromProto(mapCtx *direct.MapContext, in *pb.ServiceBinding) *krm.NetworkServicesServiceBindingSpec { + if in == nil { + return nil + } + out := &krm.NetworkServicesServiceBindingSpec{} + // MISSING: Name + out.Description = direct.LazyPtr(in.GetDescription()) + if in.GetService() != "" { + out.ServiceRef = &krmservicedirectoryv1alpha1.ServiceDirectoryServiceRef{External: in.GetService()} + } + // MISSING: ServiceID + out.Labels = in.Labels + return out +} +func NetworkServicesServiceBindingSpec_ToProto(mapCtx *direct.MapContext, in *krm.NetworkServicesServiceBindingSpec) *pb.ServiceBinding { + if in == nil { + return nil + } + out := &pb.ServiceBinding{} + // MISSING: Name + out.Description = direct.ValueOf(in.Description) + if in.ServiceRef != nil { + out.Service = in.ServiceRef.External + } + // MISSING: ServiceID + out.Labels = in.Labels return out } diff --git a/pkg/controller/direct/networkservices/sevicebinding_mappings.go b/pkg/controller/direct/networkservices/sevicebinding_mappings.go deleted file mode 100644 index e1ed8a97692..00000000000 --- a/pkg/controller/direct/networkservices/sevicebinding_mappings.go +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright 2025 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. - -// +generated:mapper -// krm.group: networkservices.cnrm.cloud.google.com -// krm.version: v1alpha1 -// proto.service: google.cloud.networkservices.v1 - -package networkservices - -import ( - pb "cloud.google.com/go/networkservices/apiv1/networkservicespb" - krm "github.com/GoogleCloudPlatform/k8s-config-connector/apis/networkservices/v1alpha1" - v1alpha1 "github.com/GoogleCloudPlatform/k8s-config-connector/apis/servicedirectory/v1alpha1" - "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/controller/direct" -) - -func NetworkServicesServiceBindingSpec_FromProto(mapCtx *direct.MapContext, in *pb.ServiceBinding) *krm.NetworkServicesServiceBindingSpec { - if in == nil { - return nil - } - out := &krm.NetworkServicesServiceBindingSpec{} - // MISSING: Name - out.Description = direct.LazyPtr(in.GetDescription()) - if in.Service != "" { - out.ServiceRef = &v1alpha1.ServiceDirectoryServiceRef{ - External: in.Service, - } - } - out.Labels = in.Labels - return out -} -func NetworkServicesServiceBindingSpec_ToProto(mapCtx *direct.MapContext, in *krm.NetworkServicesServiceBindingSpec) *pb.ServiceBinding { - if in == nil { - return nil - } - out := &pb.ServiceBinding{} - // MISSING: Name - out.Description = direct.ValueOf(in.Description) - if in.ServiceRef != nil { - out.Service = in.ServiceRef.External - } - out.Labels = in.Labels - return out -}