Merge pull request #159 from Nordix/address_linting_errors

Address golint errors

.github/workflows/golangci-lint.yaml

@@ -51,4 +51,4 @@ jobs:
         uses: golangci/golangci-lint-action@v6
         with:
           version: v1.62.2
-          args: --timeout=10m --go=${{ env.GO_VERSION }} --exclude-dirs=test --exclude-generated=true --issues-exit-code=0
+          args: --timeout=10m --go=${{ env.GO_VERSION }} --exclude-dirs=test --exclude-generated=true

cmd/porch/main.go

@@ -21,7 +21,6 @@ import (
 	"net/url"
 	"os"
 	"strings"
-	"time"
 
 	"github.com/nephio-project/porch/pkg/cmd/server"
 	"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
@@ -44,7 +43,9 @@ func main() {
 
 func run() int {
 	t := &telemetry{}
-	t.Start()
+	if err := t.Start(); err != nil {
+		klog.Warningf("failed to start telemetry: %v", err)
+	}
 	defer t.Stop()
 
 	http.DefaultTransport = otelhttp.NewTransport(http.DefaultClient.Transport)
@@ -92,12 +93,8 @@ func (t *telemetry) Start() error {
 
 		klog.Infof("tracing to %q", config)
 
-		// See https://github.com/open-telemetry/opentelemetry-go/issues/1484
-		ctx, cancel := context.WithTimeout(ctx, time.Second)
-		defer cancel()
-		conn, err := grpc.DialContext(ctx, endpoint,
+		conn, err := grpc.NewClient(endpoint,
 			grpc.WithTransportCredentials(insecure.NewCredentials()),
-			grpc.WithBlock(),
 		)
 		if err != nil {
 			return fmt.Errorf("failed to create gRPC connection to collector: %w", err)

controllers/main.go

@@ -31,7 +31,7 @@ import (
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 	"k8s.io/client-go/tools/leaderelection/resourcelock"
 	"k8s.io/klog/v2"
-	"k8s.io/klog/v2/klogr"
+	"k8s.io/klog/v2/textlogger"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -141,8 +141,12 @@ func run(ctx context.Context) error {
 			},
 		},
 	}
-
-	ctrl.SetLogger(klogr.New())
+
+	config := textlogger.NewConfig(
+		textlogger.Verbosity(4),
+		textlogger.Output(os.Stdout),
+	)
+	ctrl.SetLogger(textlogger.NewLogger(config))
 
 	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), managerOptions)
 	if err != nil {

controllers/packagevariants/pkg/controllers/packagevariant/injection.go

@@ -119,7 +119,10 @@ func ensureConfigInjection(ctx context.Context,
 		return err
 	}
 
-	setInjectionPointConditionsAndGates(kptfile, injectionPoints)
+	err = setInjectionPointConditionsAndGates(kptfile, injectionPoints)
+	if err != nil {
+		return err
+	}
 
 	prr.Spec.Resources["Kptfile"] = kptfile.String()
 

controllers/packagevariantsets/pkg/controllers/packagevariantset/render_test.go

@@ -25,7 +25,7 @@ import (
 	kptfilev1 "github.com/nephio-project/porch/pkg/kpt/api/kptfile/v1"
 	"github.com/stretchr/testify/require"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/utils/pointer"
+	ptr "k8s.io/utils/ptr"
 	"sigs.k8s.io/yaml"
 )
 
@@ -106,7 +106,7 @@ func TestRenderPackageVariantSpec(t *testing.T) {
 				packageDefault: "p",
 				template: &api.PackageVariantTemplate{
 					Downstream: &api.DownstreamTemplate{
-						Repo: pointer.String("my-repo-2"),
+						Repo: ptr.To("my-repo-2"),
 					},
 				},
 			},
@@ -125,7 +125,7 @@ func TestRenderPackageVariantSpec(t *testing.T) {
 				packageDefault: "p",
 				template: &api.PackageVariantTemplate{
 					Downstream: &api.DownstreamTemplate{
-						Package: pointer.String("new-p"),
+						Package: ptr.To("new-p"),
 					},
 				},
 			},
@@ -224,8 +224,8 @@ func TestRenderPackageVariantSpec(t *testing.T) {
 				packageDefault: "p",
 				template: &api.PackageVariantTemplate{
 					Downstream: &api.DownstreamTemplate{
-						RepoExpr:    pointer.String("'my-repo-2'"),
-						PackageExpr: pointer.String("repoDefault + '-' + packageDefault"),
+						RepoExpr:    ptr.To("'my-repo-2'"),
+						PackageExpr: ptr.To("repoDefault + '-' + packageDefault"),
 					},
 				},
 			},
@@ -244,25 +244,25 @@ func TestRenderPackageVariantSpec(t *testing.T) {
 				packageDefault: "p",
 				template: &api.PackageVariantTemplate{
 					Downstream: &api.DownstreamTemplate{
-						RepoExpr:    pointer.String("'my-repo-2'"),
-						PackageExpr: pointer.String("repoDefault + '-' + packageDefault"),
+						RepoExpr:    ptr.To("'my-repo-2'"),
+						PackageExpr: ptr.To("repoDefault + '-' + packageDefault"),
 					},
 					Labels: map[string]string{
 						"foo":   "bar",
 						"hello": "there",
 					},
 					LabelExprs: []api.MapExpr{
 						{
-							Key:       pointer.String("foo"),
-							ValueExpr: pointer.String("repoDefault"),
+							Key:       ptr.To("foo"),
+							ValueExpr: ptr.To("repoDefault"),
 						},
 						{
-							KeyExpr:   pointer.String("repository.labels['efg']"),
-							ValueExpr: pointer.String("packageDefault + '-' + repository.name"),
+							KeyExpr:   ptr.To("repository.labels['efg']"),
+							ValueExpr: ptr.To("packageDefault + '-' + repository.name"),
 						},
 						{
-							Key:   pointer.String("hello"),
-							Value: pointer.String("goodbye"),
+							Key:   ptr.To("hello"),
+							Value: ptr.To("goodbye"),
 						},
 					},
 					Annotations: map[string]string{
@@ -271,12 +271,12 @@ func TestRenderPackageVariantSpec(t *testing.T) {
 					},
 					AnnotationExprs: []api.MapExpr{
 						{
-							Key:   pointer.String("foo.org/id"),
-							Value: pointer.String("54321"),
+							Key:   ptr.To("foo.org/id"),
+							Value: ptr.To("54321"),
 						},
 						{
-							Key:       pointer.String("bigco.com/team"),
-							ValueExpr: pointer.String("upstream.annotations['bigco.com/team']"),
+							Key:       ptr.To("bigco.com/team"),
+							ValueExpr: ptr.To("upstream.annotations['bigco.com/team']"),
 						},
 					},
 				},
@@ -312,16 +312,16 @@ func TestRenderPackageVariantSpec(t *testing.T) {
 						},
 						DataExprs: []api.MapExpr{
 							{
-								Key:       pointer.String("foo"),
-								ValueExpr: pointer.String("upstream.name"),
+								Key:       ptr.To("foo"),
+								ValueExpr: ptr.To("upstream.name"),
 							},
 							{
-								KeyExpr:   pointer.String("upstream.namespace"),
-								ValueExpr: pointer.String("upstream.name"),
+								KeyExpr:   ptr.To("upstream.namespace"),
+								ValueExpr: ptr.To("upstream.name"),
 							},
 							{
-								KeyExpr: pointer.String("upstream.name"),
-								Value:   pointer.String("foo"),
+								KeyExpr: ptr.To("upstream.name"),
+								Value:   ptr.To("foo"),
 							},
 						},
 						RemoveKeys:     []string{"foobar", "barfoo"},
@@ -354,19 +354,19 @@ func TestRenderPackageVariantSpec(t *testing.T) {
 				template: &api.PackageVariantTemplate{
 					Injectors: []api.InjectionSelectorTemplate{
 						{
-							Group:   pointer.String("kpt.dev"),
-							Version: pointer.String("v1alpha1"),
-							Kind:    pointer.String("Foo"),
-							Name:    pointer.String("bar"),
+							Group:   ptr.To("kpt.dev"),
+							Version: ptr.To("v1alpha1"),
+							Kind:    ptr.To("Foo"),
+							Name:    ptr.To("bar"),
 						},
 						{
-							Group:    pointer.String("kpt.dev"),
-							Version:  pointer.String("v1alpha1"),
-							Kind:     pointer.String("Foo"),
-							NameExpr: pointer.String("repository.labels['abc']"),
+							Group:    ptr.To("kpt.dev"),
+							Version:  ptr.To("v1alpha1"),
+							Kind:     ptr.To("Foo"),
+							NameExpr: ptr.To("repository.labels['abc']"),
 						},
 						{
-							NameExpr: pointer.String("repository.name + '-test'"),
+							NameExpr: ptr.To("repository.name + '-test'"),
 						},
 					},
 				},
@@ -379,15 +379,15 @@ func TestRenderPackageVariantSpec(t *testing.T) {
 				},
 				Injectors: []pkgvarapi.InjectionSelector{
 					{
-						Group:   pointer.String("kpt.dev"),
-						Version: pointer.String("v1alpha1"),
-						Kind:    pointer.String("Foo"),
+						Group:   ptr.To("kpt.dev"),
+						Version: ptr.To("v1alpha1"),
+						Kind:    ptr.To("Foo"),
 						Name:    "bar",
 					},
 					{
-						Group:   pointer.String("kpt.dev"),
-						Version: pointer.String("v1alpha1"),
-						Kind:    pointer.String("Foo"),
+						Group:   ptr.To("kpt.dev"),
+						Version: ptr.To("v1alpha1"),
+						Kind:    ptr.To("Foo"),
 						Name:    "def",
 					},
 					{
@@ -420,12 +420,12 @@ func TestRenderPackageVariantSpec(t *testing.T) {
 								},
 								ConfigMapExprs: []api.MapExpr{
 									{
-										Key:       pointer.String("k1"),
-										ValueExpr: pointer.String("repository.name"),
+										Key:       ptr.To("k1"),
+										ValueExpr: ptr.To("repository.name"),
 									},
 									{
-										KeyExpr: pointer.String("'k3'"),
-										Value:   pointer.String("bar"),
+										KeyExpr: ptr.To("'k3'"),
+										Value:   ptr.To("bar"),
 									},
 								},
 							},
@@ -437,8 +437,8 @@ func TestRenderPackageVariantSpec(t *testing.T) {
 								},
 								ConfigMapExprs: []api.MapExpr{
 									{
-										Key:   pointer.String("k1"),
-										Value: pointer.String("yo"),
+										Key:   ptr.To("k1"),
+										Value: ptr.To("yo"),
 									},
 								},
 							},
@@ -591,16 +591,16 @@ func TestCopyAndOverlayMapExpr(t *testing.T) {
 			inMap: map[string]string{},
 			mapExprs: []api.MapExpr{
 				{
-					Key:   pointer.String("foo"),
-					Value: pointer.String("bar"),
+					Key:   ptr.To("foo"),
+					Value: ptr.To("bar"),
 				},
 				{
-					KeyExpr: pointer.String("repoDefault"),
-					Value:   pointer.String("barbar"),
+					KeyExpr: ptr.To("repoDefault"),
+					Value:   ptr.To("barbar"),
 				},
 				{
-					Key:       pointer.String("bar"),
-					ValueExpr: pointer.String("packageDefault"),
+					Key:       ptr.To("bar"),
+					ValueExpr: ptr.To("packageDefault"),
 				},
 			},
 			expectedResult: map[string]string{
@@ -616,12 +616,12 @@ func TestCopyAndOverlayMapExpr(t *testing.T) {
 			},
 			mapExprs: []api.MapExpr{
 				{
-					Key:   pointer.String("foo"),
-					Value: pointer.String("new-bar"),
+					Key:   ptr.To("foo"),
+					Value: ptr.To("new-bar"),
 				},
 				{
-					Key:   pointer.String("foofoo"),
-					Value: pointer.String("barbar"),
+					Key:   ptr.To("foofoo"),
+					Value: ptr.To("barbar"),
 				},
 			},
 			expectedResult: map[string]string{
@@ -637,12 +637,12 @@ func TestCopyAndOverlayMapExpr(t *testing.T) {
 			},
 			mapExprs: []api.MapExpr{
 				{
-					KeyExpr: pointer.String("'foo'"),
-					Value:   pointer.String("new-bar"),
+					KeyExpr: ptr.To("'foo'"),
+					Value:   ptr.To("new-bar"),
 				},
 				{
-					Key:       pointer.String("bar"),
-					ValueExpr: pointer.String("packageDefault"),
+					Key:       ptr.To("bar"),
+					ValueExpr: ptr.To("packageDefault"),
 				},
 			},
 			expectedResult: map[string]string{

func/client/main.go

@@ -92,7 +92,7 @@ func createResourceList(args []string) ([]byte, error) {
 }
 
 func call(rl []byte) (*pb.EvaluateFunctionResponse, error) {
-	cc, err := grpc.Dial(*addressFlag, grpc.WithTransportCredentials(insecure.NewCredentials()))
+	cc, err := grpc.NewClient(*addressFlag, grpc.WithTransportCredentials(insecure.NewCredentials()))
 	if err != nil {
 		return nil, fmt.Errorf("failed to connect to %s: %w", *addressFlag, err)
 	}

func/internal/podevaluator.go

@@ -295,6 +295,7 @@ func forEachConcurrently(m map[string]string, fn func(k string, v string)) {
 // We must run this method in one single goroutine. Doing it this way simplify
 // design around concurrency.
 func (pcm *podCacheManager) podCacheManager() {
+	//nolint:staticcheck
 	tick := time.Tick(pcm.gcScanInterval)
 	for {
 		select {
@@ -493,7 +494,7 @@ func (pm *podManager) getFuncEvalPodClient(ctx context.Context, image string, tt
 			return nil, fmt.Errorf("pod %s/%s did not have podIP", podKey.Namespace, podKey.Name)
 		}
 		address := net.JoinHostPort(podIP, defaultWrapperServerPort)
-		cc, err := grpc.Dial(address,
+		cc, err := grpc.NewClient(address,
 			grpc.WithTransportCredentials(insecure.NewCredentials()),
 			grpc.WithDefaultCallOptions(
 				grpc.MaxCallRecvMsgSize(pm.maxGrpcMessageSize),

func/internal/podevaluator_podmanager_test.go

@@ -40,7 +40,7 @@ func (f *fakeFunctionEvalServer) Start(ctx context.Context) error {
 
 	server := grpc.NewServer()
 	pb.RegisterFunctionEvaluatorServer(server, f)
-
+    //nolint:errcheck
 	go server.Serve(lis)
 
 	go func() {
@@ -687,7 +687,7 @@ func TestPodManager(t *testing.T) {
 					t.Errorf("Expected to get error, got ready pod")
 				}
 				var pod corev1.Pod
-				if tt.kubeClient.Get(ctx, cc.pod, &pod); err != nil {
+				if err := tt.kubeClient.Get(ctx, cc.pod, &pod); err != nil {
 					t.Errorf("Failed to get pod: %v", err)
 				}
 

internal/kpt/errors/errors.go

@@ -192,7 +192,7 @@ func E(args ...interface{}) error {
 		case error:
 			e.Err = a
 		case string:
-			e.Err = fmt.Errorf(a)
+			e.Err = fmt.Errorf("%s", a)
 		default:
 			panic(fmt.Errorf("unknown type %T for value %v in call to error.E", a, a))
 		}

internal/kpt/fnruntime/imagepullpolicy.go

@@ -53,7 +53,7 @@ func (e *ImagePullPolicy) Set(v string) error {
 			return nil
 		}
 	}
-	return fmt.Errorf("must must be one of " + strings.Join(e.AllStrings(), ", "))
+	return fmt.Errorf("must must be one of %s" , strings.Join(e.AllStrings(), ", "))
 }
 
 func (e *ImagePullPolicy) AllStrings() []string {

internal/kpt/util/man/man.go

@@ -53,7 +53,7 @@ const ManFilename = "README.md"
 func (m Command) Run() error {
 	_, err := exec.LookPath(m.GetExecCmd())
 	if err != nil {
-		return errors.Errorf(m.GetExecCmd() + " not installed")
+		return errors.Errorf("%s not installed", m.GetExecCmd())
 	}
 
 	// lookup the path to the man page