update by codereview

Signed-off-by: dongjiang <dongjiang1989@126.com>

Author: dongjiang1989

.github/workflows/golangci-lint.yml

@@ -32,8 +32,8 @@ jobs:
         with:
           go-version: ${{ steps.vars.outputs.go_version }}
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@1481404843c368bc19ca9406f87d6e0fc97bdcfd # tag=v7.0.0
+        uses: golangci/golangci-lint-action@4afd733a84b1f43292c63897423277bb7f4313a9 # tag=v8.0.0
         with:
-          version: v2.0.2
-          args: --output.text.print-linter-name=true --output.text.colors=true
+          version: v2.1.6
+          args: --output.text.print-linter-name=true --output.text.colors=true --timeout 10m
           working-directory: ${{matrix.working-directory}}

.golangci.yml

@@ -88,7 +88,10 @@ linters:
         - name: bool-literal-in-expr
         - name: constant-logical-expr
   exclusions:
-    generated: lax
+    generated: strict
+    paths:
+      - zz_generated.*\.go$
+      - .*conversion.*\.go$
     rules:
       - linters:
           - gosec
@@ -158,16 +161,14 @@ linters:
       # If it is decided they will not be addressed they should be moved above this comment.
       - path: (.+)\.go$
         text: Subprocess launch(ed with variable|ing should be audited)
-      - path: (.+)\.go$
+      - linters:
+          - gosec
+        path: (.+)\.go$
         text: (G204|G104|G307)
-      - path: (.+)\.go$
-        text: 'ST1000: at least one file in a package should have a package comment'
-    paths:
-      - zz_generated.*\.go$
-      - .*conversion.*\.go$
-      - third_party$
-      - builtin$
-      - examples$
+      - linters:
+          - staticcheck
+        path: (.+)\.go$
+        text: (ST1000|QF1008)
 issues:
   max-issues-per-linter: 0
   max-same-issues: 0
@@ -176,10 +177,7 @@ formatters:
     - gofmt
     - goimports
   exclusions:
-    generated: lax
+    generated: strict
     paths:
       - zz_generated.*\.go$
       - .*conversion.*\.go$
-      - third_party$
-      - builtin$
-      - examples$

pkg/cache/informer_cache.go

@@ -180,7 +180,7 @@ func (ic *informerCache) GetInformer(ctx context.Context, obj client.Object, opt
 
 func (ic *informerCache) getInformerForKind(ctx context.Context, gvk schema.GroupVersionKind, obj runtime.Object) (bool, *internal.Cache, error) {
 	if ic.readerFailOnMissingInformer {
-		cache, started, ok := ic.Peek(gvk, obj)
+		cache, started, ok := ic.Informers.Peek(gvk, obj)
 		if !ok {
 			return false, nil, &ErrResourceNotCached{GVK: gvk}
 		}
@@ -197,7 +197,7 @@ func (ic *informerCache) RemoveInformer(_ context.Context, obj client.Object) er
 		return err
 	}
 
-	ic.Remove(gvk, obj)
+	ic.Informers.Remove(gvk, obj)
 	return nil
 }
 

pkg/client/apiutil/restmapper_wb_test.go

@@ -209,6 +209,6 @@ type fakeAggregatedDiscoveryClient struct {
 }
 
 func (f *fakeAggregatedDiscoveryClient) GroupsAndMaybeResources() (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, map[schema.GroupVersion]error, error) {
-	groupList, err := f.ServerGroups()
+	groupList, err := f.DiscoveryInterface.ServerGroups()
 	return groupList, nil, nil, err
 }

pkg/client/client.go

@@ -450,7 +450,7 @@ type SubResourceUpdateOptions struct {
 
 // ApplyToSubResourceUpdate updates the configuration on the given create options
 func (uo *SubResourceUpdateOptions) ApplyToSubResourceUpdate(o *SubResourceUpdateOptions) {
-	uo.ApplyToUpdate(&o.UpdateOptions)
+	uo.UpdateOptions.ApplyToUpdate(&o.UpdateOptions)
 	if uo.SubResourceBody != nil {
 		o.SubResourceBody = uo.SubResourceBody
 	}
@@ -507,7 +507,7 @@ func (co *SubResourceCreateOptions) ApplyOptions(opts []SubResourceCreateOption)
 
 // ApplyToSubResourceCreate applies the the configuration on the given create options.
 func (co *SubResourceCreateOptions) ApplyToSubResourceCreate(o *SubResourceCreateOptions) {
-	co.ApplyToCreate(&co.CreateOptions)
+	co.CreateOptions.ApplyToCreate(&co.CreateOptions)
 }
 
 // SubResourcePatchOptions holds all possible configurations for a subresource patch
@@ -528,7 +528,7 @@ func (po *SubResourcePatchOptions) ApplyOptions(opts []SubResourcePatchOption) *
 
 // ApplyToSubResourcePatch applies the configuration on the given patch options.
 func (po *SubResourcePatchOptions) ApplyToSubResourcePatch(o *SubResourcePatchOptions) {
-	po.ApplyToPatch(&o.PatchOptions)
+	po.PatchOptions.ApplyToPatch(&o.PatchOptions)
 	if po.SubResourceBody != nil {
 		o.SubResourceBody = po.SubResourceBody
 	}

pkg/client/client_test.go

@@ -1698,7 +1698,7 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
 				dep2Name := dep2.Name
 
 				By("deleting Deployments")
-				err = cl.DeleteAllOf(context.TODO(), dep, client.InNamespace(ns), client.MatchingLabels(dep.Labels))
+				err = cl.DeleteAllOf(context.TODO(), dep, client.InNamespace(ns), client.MatchingLabels(dep.ObjectMeta.Labels))
 				Expect(err).NotTo(HaveOccurred())
 
 				By("validating the Deployment no longer exists")
@@ -1804,7 +1804,7 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
 					Kind:    "Deployment",
 					Version: "v1",
 				})
-				err = cl.DeleteAllOf(context.TODO(), u, client.InNamespace(ns), client.MatchingLabels(dep.Labels))
+				err = cl.DeleteAllOf(context.TODO(), u, client.InNamespace(ns), client.MatchingLabels(dep.ObjectMeta.Labels))
 				Expect(err).NotTo(HaveOccurred())
 
 				By("validating the Deployment no longer exists")
@@ -1884,7 +1884,7 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
 
 				By("deleting Deployments")
 				metaObj := metaOnlyFromObj(dep, scheme)
-				err = cl.DeleteAllOf(context.TODO(), metaObj, client.InNamespace(ns), client.MatchingLabels(dep.Labels))
+				err = cl.DeleteAllOf(context.TODO(), metaObj, client.InNamespace(ns), client.MatchingLabels(dep.ObjectMeta.Labels))
 				Expect(err).NotTo(HaveOccurred())
 
 				By("validating the Deployment no longer exists")

pkg/client/fake/client.go

@@ -411,9 +411,9 @@ func (t versionedTracker) Patch(gvr schema.GroupVersionResource, obj runtime.Obj
 		return err
 	}
 
-	isStatus := bytes.Contains(debug.Stack(), []byte("sigs.k8s.io/controller-runtime/pkg/client/fake.(*fakeSubResourceClient).statusPatch"))
 	// We apply patches using a client-go reaction that ends up calling the trackers Patch. As we can't change
 	// that reaction, we use the callstack to figure out if this originated from the status client.
+	isStatus := bytes.Contains(debug.Stack(), []byte("sigs.k8s.io/controller-runtime/pkg/client/fake.(*fakeSubResourceClient).statusPatch"))
 
 	obj, err = t.updateObject(gvr, obj, ns, isStatus, false, patchOptions.DryRun)
 	if err != nil {
@@ -444,7 +444,7 @@ func (t versionedTracker) updateObject(gvr schema.GroupVersionResource, obj runt
 		return nil, err
 	}
 
-	oldObject, err := t.Get(gvr, ns, accessor.GetName())
+	oldObject, err := t.ObjectTracker.Get(gvr, ns, accessor.GetName())
 	if err != nil {
 		// If the resource is not found and the resource allows create on update, issue a
 		// create instead.
@@ -515,7 +515,7 @@ func (t versionedTracker) updateObject(gvr schema.GroupVersionResource, obj runt
 	}
 
 	if !accessor.GetDeletionTimestamp().IsZero() && len(accessor.GetFinalizers()) == 0 {
-		return nil, t.Delete(gvr, accessor.GetNamespace(), accessor.GetName(), metav1.DeleteOptions{DryRun: dryRun})
+		return nil, t.ObjectTracker.Delete(gvr, accessor.GetNamespace(), accessor.GetName(), metav1.DeleteOptions{DryRun: dryRun})
 	}
 	return convertFromUnstructuredIfNecessary(t.scheme, obj)
 }

pkg/client/metadata_client.go

@@ -77,7 +77,7 @@ func (mc *metadataClient) DeleteAllOf(ctx context.Context, obj Object, opts ...D
 	deleteAllOfOpts := DeleteAllOfOptions{}
 	deleteAllOfOpts.ApplyOptions(opts)
 
-	resInt, err := mc.getResourceInterface(metadata.GroupVersionKind(), deleteAllOfOpts.Namespace)
+	resInt, err := mc.getResourceInterface(metadata.GroupVersionKind(), deleteAllOfOpts.ListOptions.Namespace)
 	if err != nil {
 		return err
 	}

pkg/client/watch.go

@@ -67,7 +67,7 @@ func (w *watchingClient) metadataWatch(ctx context.Context, obj *metav1.PartialO
 
 	listOpts := w.listOpts(opts...)
 
-	resInt, err := w.metadataClient.getResourceInterface(gvk, listOpts.Namespace)
+	resInt, err := w.client.metadataClient.getResourceInterface(gvk, listOpts.Namespace)
 	if err != nil {
 		return nil, err
 	}
@@ -76,7 +76,7 @@ func (w *watchingClient) metadataWatch(ctx context.Context, obj *metav1.PartialO
 }
 
 func (w *watchingClient) unstructuredWatch(ctx context.Context, obj runtime.Unstructured, opts ...ListOption) (watch.Interface, error) {
-	r, err := w.unstructuredClient.resources.getResource(obj)
+	r, err := w.client.unstructuredClient.resources.getResource(obj)
 	if err != nil {
 		return nil, err
 	}
@@ -91,7 +91,7 @@ func (w *watchingClient) unstructuredWatch(ctx context.Context, obj runtime.Unst
 }
 
 func (w *watchingClient) typedWatch(ctx context.Context, obj ObjectList, opts ...ListOption) (watch.Interface, error) {
-	r, err := w.typedClient.resources.getResource(obj)
+	r, err := w.client.typedClient.resources.getResource(obj)
 	if err != nil {
 		return nil, err
 	}

pkg/controller/controllerutil/example_test.go

@@ -41,7 +41,7 @@ func ExampleCreateOrUpdate() {
 	op, err := controllerutil.CreateOrUpdate(context.TODO(), c, deploy, func() error {
 		// Deployment selector is immutable so we set this value only if
 		// a new object is going to be created
-		if deploy.CreationTimestamp.IsZero() {
+		if deploy.ObjectMeta.CreationTimestamp.IsZero() {
 			deploy.Spec.Selector = &metav1.LabelSelector{
 				MatchLabels: map[string]string{"foo": "bar"},
 			}

pkg/envtest/komega/default_test.go

@@ -38,8 +38,8 @@ func TestDefaultList(t *testing.T) {
 	g.Expect(list.Items).To(HaveLen(1))
 	depl := exampleDeployment()
 	g.Expect(list.Items[0]).To(And(
-		HaveField("ObjectMeta.Name", Equal(depl.Name)),
-		HaveField("ObjectMeta.Namespace", Equal(depl.Namespace)),
+		HaveField("ObjectMeta.Name", Equal(depl.ObjectMeta.Name)),
+		HaveField("ObjectMeta.Namespace", Equal(depl.ObjectMeta.Namespace)),
 	))
 }
 

pkg/envtest/komega/komega_test.go

@@ -60,8 +60,8 @@ func TestList(t *testing.T) {
 	g.Expect(list.Items).To(HaveLen(1))
 	depl := exampleDeployment()
 	g.Expect(list.Items[0]).To(And(
-		HaveField("ObjectMeta.Name", Equal(depl.Name)),
-		HaveField("ObjectMeta.Namespace", Equal(depl.Namespace)),
+		HaveField("ObjectMeta.Name", Equal(depl.ObjectMeta.Name)),
+		HaveField("ObjectMeta.Namespace", Equal(depl.ObjectMeta.Namespace)),
 	))
 }
 

pkg/internal/controller/controller.go

@@ -403,16 +403,16 @@ func (p *priorityQueueWrapper[request]) AddWithOpts(opts priorityqueue.AddOpts,
 	for _, item := range items {
 		switch {
 		case opts.RateLimited:
-			p.AddRateLimited(item)
+			p.TypedRateLimitingInterface.AddRateLimited(item)
 		case opts.After > 0:
-			p.AddAfter(item, opts.After)
+			p.TypedRateLimitingInterface.AddAfter(item, opts.After)
 		default:
-			p.Add(item)
+			p.TypedRateLimitingInterface.Add(item)
 		}
 	}
 }
 
 func (p *priorityQueueWrapper[request]) GetWithPriority() (request, int, bool) {
-	item, shutdown := p.Get()
+	item, shutdown := p.TypedRateLimitingInterface.Get()
 	return item, 0, shutdown
 }

pkg/internal/testing/controlplane/apiserver.go

@@ -200,23 +200,23 @@ func (s *APIServer) configurePorts() error {
 
 	// Secure: SecurePort, SecureServing
 	if s.SecurePort != 0 {
-		s.Port = strconv.Itoa(s.SecurePort)
+		s.SecureServing.Port = strconv.Itoa(s.SecurePort)
 		// if we don't have an address, try the insecure address, and otherwise
 		// default to loopback.
-		if s.Address == "" {
+		if s.SecureServing.Address == "" {
 			if s.InsecureServing != nil {
-				s.Address = s.InsecureServing.Address
+				s.SecureServing.Address = s.InsecureServing.Address
 			} else {
-				s.Address = "127.0.0.1"
+				s.SecureServing.Address = "127.0.0.1"
 			}
 		}
-	} else if s.Port == "" || s.Address == "" {
+	} else if s.SecureServing.Port == "" || s.SecureServing.Address == "" {
 		port, host, err := addr.Suggest("")
 		if err != nil {
 			return fmt.Errorf("unable to provision unused secure port: %w", err)
 		}
-		s.Port = strconv.Itoa(port)
-		s.Address = host
+		s.SecureServing.Port = strconv.Itoa(port)
+		s.SecureServing.Address = host
 		s.SecurePort = port
 	}
 
@@ -260,12 +260,12 @@ func (s *APIServer) setProcessState() error {
 		return err
 	}
 
-	if s.Authn == nil {
+	if s.SecureServing.Authn == nil {
 		authn, err := NewCertAuthn()
 		if err != nil {
 			return err
 		}
-		s.Authn = authn
+		s.SecureServing.Authn = authn
 	}
 
 	if err := s.Authn.Configure(s.CertDir, s.Configure()); err != nil {
@@ -337,11 +337,11 @@ func (s *APIServer) defaultArgs() map[string][]string {
 		"disable-admission-plugins": {"ServiceAccount"},
 		"cert-dir":                  {s.CertDir},
 		"authorization-mode":        {"RBAC"},
-		"secure-port":               {s.Port},
+		"secure-port":               {s.SecureServing.Port},
 		// NB(directxman12): previously we didn't set the bind address for the secure
 		// port.  It *shouldn't* make a difference unless people are doing something really
 		// funky, but if you start to get bug reports look here ;-)
-		"bind-address": {s.Address},
+		"bind-address": {s.SecureServing.Address},
 
 		// required on 1.20+, fine to leave on for <1.20
 		"service-account-issuer":           {s.SecureServing.URL("https", "/").String()},
@@ -391,7 +391,7 @@ func (s *APIServer) populateAPIServerCerts() error {
 		return err
 	}
 
-	s.CA = ca.CA.CertBytes()
+	s.SecureServing.CA = ca.CA.CertBytes()
 
 	// service account signing files too
 	saCA, err := certs.NewTinyCA()

pkg/internal/testing/controlplane/plane.go

@@ -168,11 +168,11 @@ func (u *AuthenticatedUser) Config() *rest.Config {
 	if u.cfgIsComplete {
 		return u.cfg
 	}
-	if len(u.plane.APIServer.CA) == 0 {
+	if len(u.plane.APIServer.SecureServing.CA) == 0 {
 		panic("the API server has not yet been started, please do that before accessing connection details")
 	}
 
-	u.cfg.CAData = u.plane.APIServer.CA
+	u.cfg.CAData = u.plane.APIServer.SecureServing.CA
 	u.cfg.Host = u.plane.APIServer.SecureServing.URL("https", "/").String()
 	u.cfgIsComplete = true
 	return u.cfg
@@ -232,14 +232,14 @@ func (u *AuthenticatedUser) Kubectl() (*KubeCtl, error) {
 // guaranteed to be callable both before and after Start has been called (but, as noted in the
 // AuthenticatedUser docs, the given user objects are only valid after Start has been called).
 func (f *ControlPlane) AddUser(user User, baseConfig *rest.Config) (*AuthenticatedUser, error) {
-	if f.GetAPIServer().Authn == nil {
+	if f.GetAPIServer().SecureServing.Authn == nil {
 		return nil, fmt.Errorf("no API server authentication is configured yet.  The API server defaults one when Start is called, did you mean to use that?")
 	}
 
 	if baseConfig == nil {
 		baseConfig = &rest.Config{}
 	}
-	cfg, err := f.GetAPIServer().AddUser(user, baseConfig)
+	cfg, err := f.GetAPIServer().SecureServing.AddUser(user, baseConfig)
 	if err != nil {
 		return nil, err
 	}

pkg/webhook/admission/response.go

@@ -119,6 +119,6 @@ func validationResponseFromStatus(allowed bool, status metav1.Status) Response {
 // WithWarnings adds the given warnings to the Response.
 // If any warnings were already given, they will not be overwritten.
 func (r Response) WithWarnings(warnings ...string) Response {
-	r.Warnings = append(r.Warnings, warnings...)
+	r.AdmissionResponse.Warnings = append(r.AdmissionResponse.Warnings, warnings...)
 	return r
 }

pkg/webhook/authentication/response.go

@@ -57,7 +57,7 @@ func ReviewResponse(authenticated bool, user authenticationv1.UserInfo, err stri
 		},
 	}
 	if len(err) > 0 {
-		resp.Status.Error = err
+		resp.TokenReview.Status.Error = err
 	}
 	return resp
 }

tools/setup-envtest/env/env.go

@@ -366,7 +366,7 @@ func (e *Env) PrintInfo(printFmt PrintFormat) {
 		fmt.Fprintf(e.Out, "Version: %s\n", e.Version)
 		fmt.Fprintf(e.Out, "OS/Arch: %s\n", e.Platform)
 		if e.Platform.Hash != nil {
-			fmt.Fprintf(e.Out, "%s: %s\n", e.Platform.Type, e.Platform.Value)
+			fmt.Fprintf(e.Out, "%s: %s\n", e.Platform.Hash.Type, e.Platform.Hash.Value)
 		}
 		fmt.Fprintf(e.Out, "Path: %s\n", path)
 	case PrintPath: