Merge pull request GoogleCloudPlatform#1264 from acpana/acpana/c6

chore:lint: second round of revive

Author: google-oss-prow[bot]

.golangci.yaml

@@ -38,7 +38,7 @@ linters:
     # - importas
     # - ineffassign
     # - misspell
-    # - revive # replacement for golint
+    - revive # replacement for golint
     # - staticcheck
     # - typecheck
     # - unconvert

config/tests/samples/create/harness.go

@@ -82,7 +82,7 @@ var httpRoundTripperKey httpRoundTripperKeyType
 
 // NewHarnessWithManager builds a Harness for an existing manager.
 // deprecated: Prefer NewHarness, which can construct a manager and mock gcp etc.
-func NewHarnessWithManager(t *testing.T, ctx context.Context, mgr manager.Manager) *Harness {
+func NewHarnessWithManager(ctx context.Context, t *testing.T, mgr manager.Manager) *Harness {
 	h := &Harness{
 		T:      t,
 		Ctx:    ctx,
@@ -91,7 +91,7 @@ func NewHarnessWithManager(t *testing.T, ctx context.Context, mgr manager.Manage
 	return h
 }
 
-func NewHarness(t *testing.T, ctx context.Context) *Harness {
+func NewHarness(ctx context.Context, t *testing.T) *Harness {
 	ctx, ctxCancel := context.WithCancel(ctx)
 	t.Cleanup(func() {
 		ctxCancel()
@@ -117,7 +117,7 @@ func NewHarness(t *testing.T, ctx context.Context) *Harness {
 	kccConfig.ManagerOptions.NewClient = nocache.NoCacheClientFunc
 	kccConfig.StateIntoSpecDefaultValue = k8s.StateIntoSpecDefaultValueV1Beta1
 
-	var webhooks []cnrmwebhook.WebhookConfig
+	var webhooks []cnrmwebhook.Config
 
 	loadCRDs := true
 	if targetKube := os.Getenv("E2E_KUBE_TARGET"); targetKube == "envtest" {
@@ -450,14 +450,14 @@ func MaybeSkip(t *testing.T, name string, resources []*unstructured.Unstructured
 			case schema.GroupKind{Group: "tags.cnrm.cloud.google.com", Kind: "TagsTagKey"}:
 
 			default:
-				t.Skipf("gk %v not suppported by mock gcp; skipping", gvk.GroupKind())
+				t.Skipf("gk %v not suppported by mock gcp %v; skipping", gvk.GroupKind(), name)
 			}
 		}
 	}
 }
 
-func (t *Harness) waitForCRDReady(obj client.Object) {
-	logger := log.FromContext(t.Ctx)
+func (h *Harness) waitForCRDReady(obj client.Object) {
+	logger := log.FromContext(h.Ctx)
 
 	apiVersion, kind := obj.GetObjectKind().GroupVersionKind().ToAPIVersionAndKind()
 	name := obj.GetName()
@@ -469,11 +469,11 @@ func (t *Harness) waitForCRDReady(obj client.Object) {
 		u.SetAPIVersion(apiVersion)
 		u.SetKind(kind)
 		logger.V(2).Info("Testing to see if resource is ready", "kind", kind, "id", id)
-		if err := t.GetClient().Get(t.Ctx, id, u); err != nil {
+		if err := h.GetClient().Get(h.Ctx, id, u); err != nil {
 			logger.Info("Error getting resource", "kind", kind, "id", id, "error", err)
 			return false, err
 		}
-		objectStatus := dynamic.GetObjectStatus(t.T, u)
+		objectStatus := dynamic.GetObjectStatus(h.T, u)
 		// CRDs do not have observedGeneration
 		for _, condition := range objectStatus.Conditions {
 			if condition.Type == "Established" && condition.Status == "True" {
@@ -485,7 +485,7 @@ func (t *Harness) waitForCRDReady(obj client.Object) {
 		logger.V(2).Info("CRD is not ready", "kind", kind, "id", id, "conditions", objectStatus.Conditions)
 		return false, nil
 	}); err != nil {
-		t.Errorf("error while polling for ready on %v %v: %v", kind, id, err)
+		h.Errorf("error while polling for ready on %v %v: %v", kind, id, err)
 		return
 	}
 }

config/tests/samples/create/samples.go

@@ -60,7 +60,7 @@ func networksInSampleCount(sample Sample) int {
 	count := 0
 	for _, r := range sample.Resources {
 		if r.GetKind() == "ComputeNetwork" {
-			count += 1
+			count++
 		}
 	}
 	return count
@@ -85,7 +85,7 @@ func getNamespaces(resources []*unstructured.Unstructured) []string {
 	return namespaces.List()
 }
 
-type CreateDeleteTestOptions struct {
+type CreateDeleteTestOptions struct { //nolint:revive
 	// Create is the set of objects to create
 	Create []*unstructured.Unstructured
 

config/tests/samples/create/samples_test.go

@@ -257,7 +257,7 @@ func TestAll(t *testing.T) {
 
 			ctx := context.TODO()
 
-			h := NewHarnessWithManager(t, ctx, mgr)
+			h := NewHarnessWithManager(ctx, t, mgr)
 			SetupNamespacesAndApplyDefaults(h, s.Resources, project)
 
 			networkCount := int64(networksInSampleCount(s))

config/tests/servicemapping/servicemapping_test.go

@@ -590,13 +590,13 @@ func TestMustHaveIDTemplateOrServerGeneratedId(t *testing.T) {
 			rc := rc
 			t.Run(rc.Kind, func(t *testing.T) {
 				t.Parallel()
-				assertIDTemplateOrServerGeneratedId(t, rc)
+				assertIDTemplateOrServerGeneratedID(t, rc)
 			})
 		}
 	}
 }
 
-func assertIDTemplateOrServerGeneratedId(t *testing.T, rc v1alpha1.ResourceConfig) {
+func assertIDTemplateOrServerGeneratedID(t *testing.T, rc v1alpha1.ResourceConfig) {
 	if rc.IDTemplate == "" && rc.ServerGeneratedIDField == "" {
 		t.Fatalf("resource kind '%v' with name '%v' has neither id template or server generated ID defined: at least one must be present", rc.Kind, rc.Name)
 	}
@@ -625,10 +625,10 @@ func TestIDTemplate(t *testing.T) {
 
 					// The idTemplate should contain either the user-specified
 					// ID or the server-generated ID.
-					if (IDTemplateContainsMetadataName(t, rc) &&
-						!IDTemplateContainsServerGeneratedIDField(t, rc)) ||
-						(!IDTemplateContainsMetadataName(t, rc) &&
-							IDTemplateContainsServerGeneratedIDField(t, rc)) {
+					if (IDTemplateContainsMetadataName(rc) &&
+						!IDTemplateContainsServerGeneratedIDField(rc)) ||
+						(!IDTemplateContainsMetadataName(rc) &&
+							IDTemplateContainsServerGeneratedIDField(rc)) {
 						return
 					}
 
@@ -642,12 +642,12 @@ func TestIDTemplate(t *testing.T) {
 	}
 }
 
-func IDTemplateContainsMetadataName(t *testing.T, rc v1alpha1.ResourceConfig) bool {
+func IDTemplateContainsMetadataName(rc v1alpha1.ResourceConfig) bool {
 	return strings.Contains(rc.IDTemplate,
 		fmt.Sprintf("{{%v}}", rc.MetadataMapping.Name))
 }
 
-func IDTemplateContainsServerGeneratedIDField(t *testing.T, rc v1alpha1.ResourceConfig) bool {
+func IDTemplateContainsServerGeneratedIDField(rc v1alpha1.ResourceConfig) bool {
 	return strings.Contains(rc.IDTemplate,
 		fmt.Sprintf("{{%v}}", rc.ServerGeneratedIDField))
 }
@@ -1261,11 +1261,11 @@ func TestStorageVersionIsSetAndValidIFFV1alpha1ToV1beta1IsSet(t *testing.T) {
 						"must be %v or %v", r.Name, k8s.KCCAPIVersionV1Alpha1,
 						k8s.KCCAPIVersion)
 					continue
-				} else {
-					t.Errorf("Resource config %v has `v1alpha1ToV1beta1: "+
-						"true` but doesn't have a `storageVersion`", r.Name)
-					continue
 				}
+
+				t.Errorf("Resource config %v has `v1alpha1ToV1beta1: "+
+					"true` but doesn't have a `storageVersion`", r.Name)
+				continue
 			}
 			if hasStorageVersion {
 				t.Errorf("Resource config %v has `storageVersion` set "+

operator/pkg/controllers/configconnector/configconnector_controller_test.go

@@ -280,7 +280,7 @@ func TestHandleConfigConnectorCreate(t *testing.T) {
 				}
 			}
 
-			if err := handleLifecycles(t, ctx, r, tc.cc, m); err != nil {
+			if err := handleLifecycles(ctx, t, r, tc.cc, m); err != nil {
 				t.Fatalf("unexpected error: %v", err)
 			}
 			expectedObjs := tc.resultsFunc(t, c)
@@ -451,7 +451,7 @@ func TestHandleConfigConnectorDelete(t *testing.T) {
 			}
 			if len(tc.cccs) > 0 {
 				// Expect that the first attempt returns an error.
-				if err := handleLifecycles(t, ctx, r, tc.cc, m); err == nil {
+				if err := handleLifecycles(ctx, t, r, tc.cc, m); err == nil {
 					t.Fatalf("expect to have an error because the controller manager pod per namespace is not deleted, but got nil")
 				}
 				// Simulate that CCC controller kicks in and deletes the controller manager pod.
@@ -463,7 +463,7 @@ func TestHandleConfigConnectorDelete(t *testing.T) {
 					}
 				}
 			}
-			if err := handleLifecycles(t, ctx, r, tc.cc, m); err != nil {
+			if err := handleLifecycles(ctx, t, r, tc.cc, m); err != nil {
 				t.Fatalf("unexpected error: %v", err)
 			}
 			expectedObjs := tc.resultsFunc(t, c)
@@ -776,7 +776,7 @@ func TestConfigConnectorUpdate(t *testing.T) {
 
 			m := testcontroller.ParseObjects(ctx, t, tc.manifest)
 			if tc.cc.GetMode() == "namespaced" {
-				if err := handleLifecycles(t, ctx, r, tc.updatedCc, m); err == nil {
+				if err := handleLifecycles(ctx, t, r, tc.updatedCc, m); err == nil {
 					t.Fatalf("got nil, but want to have an error because the controller manager pod per namespace is not deleted")
 				}
 				// Simulate that CCC controller kicks in and deletes the controller manager pod.
@@ -788,7 +788,7 @@ func TestConfigConnectorUpdate(t *testing.T) {
 					}
 				}
 			}
-			if err := handleLifecycles(t, ctx, r, tc.updatedCc, m); err != nil {
+			if err := handleLifecycles(ctx, t, r, tc.updatedCc, m); err != nil {
 				t.Fatalf("unexpected error: %v", err)
 			}
 			expectedObjs := tc.resultsFunc(t, c)
@@ -825,8 +825,7 @@ type testCaseStruct struct {
 	resultsFunc    func(t *testing.T, c client.Client) []string
 }
 
-func handleLifecycles(t *testing.T, ctx context.Context,
-	r *Reconciler, cc *corev1beta1.ConfigConnector, m *manifest.Objects) error {
+func handleLifecycles(ctx context.Context, t *testing.T, r *Reconciler, cc *corev1beta1.ConfigConnector, m *manifest.Objects) error {
 	t.Helper()
 
 	fn := r.transformForClusterMode()