From 1a91f400405f872f232e08abc344b7810089fa90 Mon Sep 17 00:00:00 2001 From: CMGS Date: Mon, 7 Sep 2026 23:39:44 +0900 Subject: [PATCH 1/2] review: cmp.Or for zero-value fallbacks The asl cmpor analyzer rewrites the if/return and if/assign zero-value fallbacks into cmp.Or where the fallback is call-free. --- cmd/sandbox-operator/tls.go | 13 ++++--------- controllers/sandbox_controller_test.go | 4 +--- .../controllers/sandboxclaim_controller_test.go | 4 +--- .../controllers/sandboxtemplate_controller.go | 9 +++------ pkg/e2bcompat/server.go | 13 ++++--------- pkg/podruntime/cocoon.go | 5 ++--- pkg/scale/apiserver/lifecycle_storage.go | 5 ++--- pkg/scale/apiserver/storage.go | 5 ++--- pkg/scale/poolkey.go | 4 +--- pkg/scale/sandboxstore_impl.go | 9 ++------- 10 files changed, 22 insertions(+), 49 deletions(-) diff --git a/cmd/sandbox-operator/tls.go b/cmd/sandbox-operator/tls.go index b4f84a4..4f49c0f 100644 --- a/cmd/sandbox-operator/tls.go +++ b/cmd/sandbox-operator/tls.go @@ -15,6 +15,7 @@ package main import ( + "cmp" "context" "crypto/ecdsa" "crypto/elliptic" @@ -235,21 +236,15 @@ func patchCRDs(ctx context.Context, c client.Client, caPEM []byte, serviceName, original := crd.DeepCopy() webhook := crd.Spec.Conversion.Webhook - if webhook == nil { - webhook = &apiextensionsv1.WebhookConversion{} - } + webhook = cmp.Or(webhook, &apiextensionsv1.WebhookConversion{}) if len(webhook.ConversionReviewVersions) == 0 { webhook.ConversionReviewVersions = []string{"v1", "v1beta1"} } - if webhook.ClientConfig == nil { - webhook.ClientConfig = &apiextensionsv1.WebhookClientConfig{} - } + webhook.ClientConfig = cmp.Or(webhook.ClientConfig, &apiextensionsv1.WebhookClientConfig{}) - if webhook.ClientConfig.Service == nil { - webhook.ClientConfig.Service = &apiextensionsv1.ServiceReference{} - } + webhook.ClientConfig.Service = cmp.Or(webhook.ClientConfig.Service, &apiextensionsv1.ServiceReference{}) webhook.ClientConfig.Service.Name = serviceName webhook.ClientConfig.Service.Namespace = namespace diff --git a/controllers/sandbox_controller_test.go b/controllers/sandbox_controller_test.go index c1e272f..dd2bb75 100644 --- a/controllers/sandbox_controller_test.go +++ b/controllers/sandbox_controller_test.go @@ -966,9 +966,7 @@ func TestReconcile(t *testing.T) { } reconcileCount := tc.reconcileCount - if reconcileCount == 0 { - reconcileCount = 1 - } + reconcileCount = cmp.Or(reconcileCount, 1) var err error for i := 0; i < reconcileCount; i++ { _, err = r.Reconcile(t.Context(), ctrl.Request{ diff --git a/extensions/controllers/sandboxclaim_controller_test.go b/extensions/controllers/sandboxclaim_controller_test.go index a84e9bc..71e9ed3 100644 --- a/extensions/controllers/sandboxclaim_controller_test.go +++ b/extensions/controllers/sandboxclaim_controller_test.go @@ -992,9 +992,7 @@ func TestSandboxClaimReconcile(t *testing.T) { scheme := newScheme(t) claimToUse := tc.claimToReconcile - if claimToUse == nil { - claimToUse = claim - } + claimToUse = cmp.Or(claimToUse, claim) allObjects := append(slices.Clone(tc.existingObjects), claimToUse) client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(allObjects...).WithStatusSubresource(claimToUse).Build() diff --git a/extensions/controllers/sandboxtemplate_controller.go b/extensions/controllers/sandboxtemplate_controller.go index 049d2f9..9fe351b 100644 --- a/extensions/controllers/sandboxtemplate_controller.go +++ b/extensions/controllers/sandboxtemplate_controller.go @@ -15,6 +15,7 @@ package controllers import ( + "cmp" "context" "fmt" @@ -82,9 +83,7 @@ func (r *SandboxTemplateReconciler) Reconcile(ctx context.Context, req ctrl.Requ npNamespace := template.Namespace management := template.Spec.NetworkPolicyManagement - if management == "" { - management = extensionsv1beta1.NetworkPolicyManagementManaged - } + management = cmp.Or(management, extensionsv1beta1.NetworkPolicyManagementManaged) if management == extensionsv1beta1.NetworkPolicyManagementUnmanaged { return ctrl.Result{}, r.dropManagedNetworkPolicy(ctx, template, npName, npNamespace) @@ -209,9 +208,7 @@ func (r *SandboxTemplateReconciler) dropManagedNetworkPolicy(ctx context.Context // routerNamespace is the namespace the sandbox-router runs in (the operator // install namespace); ingress is admitted only from that namespace. func buildDefaultNetworkPolicySpec(templateName, routerNamespace string) networkingv1.NetworkPolicySpec { - if routerNamespace == "" { - routerNamespace = defaultRouterNamespace - } + routerNamespace = cmp.Or(routerNamespace, defaultRouterNamespace) peers := []networkingv1.NetworkPolicyPeer{ { NamespaceSelector: &metav1.LabelSelector{ diff --git a/pkg/e2bcompat/server.go b/pkg/e2bcompat/server.go index 91e2250..81ea6b4 100644 --- a/pkg/e2bcompat/server.go +++ b/pkg/e2bcompat/server.go @@ -20,6 +20,7 @@ package e2bcompat import ( + "cmp" "crypto/subtle" "errors" "fmt" @@ -106,15 +107,9 @@ func NewServer(store scale.SandboxStore, opts Options) (*Server, error) { if !ok { return nil, errors.New("e2bcompat: store does not implement scale.ClaimIDResolver") } - if opts.Namespace == "" { - opts.Namespace = "default" - } - if opts.EnvdVersion == "" { - opts.EnvdVersion = DefaultEnvdVersion - } - if opts.SizeClass == "" { - opts.SizeClass = scale.SizeClassSmall - } + opts.Namespace = cmp.Or(opts.Namespace, "default") + opts.EnvdVersion = cmp.Or(opts.EnvdVersion, DefaultEnvdVersion) + opts.SizeClass = cmp.Or(opts.SizeClass, scale.SizeClassSmall) keys := make(map[string]struct{}, len(opts.APIKeys)) for _, k := range opts.APIKeys { if k = strings.TrimSpace(k); k != "" { diff --git a/pkg/podruntime/cocoon.go b/pkg/podruntime/cocoon.go index 98d3dc2..6616087 100644 --- a/pkg/podruntime/cocoon.go +++ b/pkg/podruntime/cocoon.go @@ -2,6 +2,7 @@ package podruntime import ( + "cmp" "context" "crypto/sha256" "fmt" @@ -207,9 +208,7 @@ func stableVMName(namespace, name string) string { } } prefix := strings.Trim(normalized.String(), "-") - if prefix == "" { - prefix = "sandbox" - } + prefix = cmp.Or(prefix, "sandbox") digest := fmt.Sprintf("%x", sha256.Sum256([]byte(namespace+"/"+name)))[:8] const maxPrefix = 63 - 1 - 8 if len(prefix) > maxPrefix { diff --git a/pkg/scale/apiserver/lifecycle_storage.go b/pkg/scale/apiserver/lifecycle_storage.go index 330cdd1..1d98efc 100644 --- a/pkg/scale/apiserver/lifecycle_storage.go +++ b/pkg/scale/apiserver/lifecycle_storage.go @@ -1,6 +1,7 @@ package apiserver import ( + "cmp" "context" "fmt" @@ -121,9 +122,7 @@ func NewSandboxForkREST(store scale.SandboxStore) rest.Storage { return nil, apierrors.NewBadRequest(fmt.Sprintf("expected SandboxForkOptions, got %T", obj)) } count := int(opts.Count) - if count == 0 { - count = 1 - } + count = cmp.Or(count, 1) if count < 0 { return nil, apierrors.NewBadRequest(fmt.Sprintf("count must be >= 1, got %d", count)) } diff --git a/pkg/scale/apiserver/storage.go b/pkg/scale/apiserver/storage.go index 76580c5..756e889 100644 --- a/pkg/scale/apiserver/storage.go +++ b/pkg/scale/apiserver/storage.go @@ -1,6 +1,7 @@ package apiserver import ( + "cmp" "context" "fmt" "math" @@ -114,9 +115,7 @@ func (r *sandboxREST) Create(ctx context.Context, obj runtime.Object, createVali // Identity: namespace from the request path, name (honoring generateName) from // the submitted object. namespace := genericapirequest.NamespaceValue(ctx) - if namespace == "" { - namespace = sb.Namespace - } + namespace = cmp.Or(namespace, sb.Namespace) name := sb.Name if name == "" && sb.GenerateName != "" { name = names.SimpleNameGenerator.GenerateName(sb.GenerateName) diff --git a/pkg/scale/poolkey.go b/pkg/scale/poolkey.go index 970c3fb..01499e9 100644 --- a/pkg/scale/poolkey.go +++ b/pkg/scale/poolkey.go @@ -39,9 +39,7 @@ func PoolKeyFor(containers []corev1.Container, net string) PoolKey { if len(containers) > 0 { template = containers[0].Image } - if net == "" { - net = NetDefault - } + net = cmp.Or(net, NetDefault) return PoolKey{Template: template, Net: net, Size: SizeClassForContainers(containers)} } diff --git a/pkg/scale/sandboxstore_impl.go b/pkg/scale/sandboxstore_impl.go index ba29e7d..2d19118 100644 --- a/pkg/scale/sandboxstore_impl.go +++ b/pkg/scale/sandboxstore_impl.go @@ -656,9 +656,7 @@ type ssaInventoryApplier struct { // NewSSAInventoryApplier returns the default server-side-apply InventoryApplier. func NewSSAInventoryApplier(c client.Client, fieldOwner string) InventoryApplier { - if fieldOwner == "" { - fieldOwner = "cocoon-node-inventory-publisher" - } + fieldOwner = cmp.Or(fieldOwner, "cocoon-node-inventory-publisher") return &ssaInventoryApplier{c: c, fieldOwner: fieldOwner} } @@ -980,10 +978,7 @@ func readyStatus(phase string) metav1.ConditionStatus { } func readyReason(phase string) string { - if phase == "" { - return "Unknown" - } - return phase + return cmp.Or(phase, "Unknown") } // resourceVersionFor derives a deterministic, content-sensitive ResourceVersion From 37840dad52089577e82365f15ef7d91c0f4118cc Mon Sep 17 00:00:00 2001 From: CMGS Date: Mon, 7 Sep 2026 23:43:03 +0900 Subject: [PATCH 2/2] review: keep the two if-fallbacks in tests that import go-cmp cmp.Or cannot be spelled where cmp is go-cmp; the analyzer now skips such files (asl 4th fix), and these two test sites stay as they were. --- controllers/sandbox_controller_test.go | 4 +++- extensions/controllers/sandboxclaim_controller_test.go | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/controllers/sandbox_controller_test.go b/controllers/sandbox_controller_test.go index dd2bb75..c1e272f 100644 --- a/controllers/sandbox_controller_test.go +++ b/controllers/sandbox_controller_test.go @@ -966,7 +966,9 @@ func TestReconcile(t *testing.T) { } reconcileCount := tc.reconcileCount - reconcileCount = cmp.Or(reconcileCount, 1) + if reconcileCount == 0 { + reconcileCount = 1 + } var err error for i := 0; i < reconcileCount; i++ { _, err = r.Reconcile(t.Context(), ctrl.Request{ diff --git a/extensions/controllers/sandboxclaim_controller_test.go b/extensions/controllers/sandboxclaim_controller_test.go index 71e9ed3..a84e9bc 100644 --- a/extensions/controllers/sandboxclaim_controller_test.go +++ b/extensions/controllers/sandboxclaim_controller_test.go @@ -992,7 +992,9 @@ func TestSandboxClaimReconcile(t *testing.T) { scheme := newScheme(t) claimToUse := tc.claimToReconcile - claimToUse = cmp.Or(claimToUse, claim) + if claimToUse == nil { + claimToUse = claim + } allObjects := append(slices.Clone(tc.existingObjects), claimToUse) client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(allObjects...).WithStatusSubresource(claimToUse).Build()