Merge pull request #138 from Nordix/static-lint

Implement improvements identified by static linting

pkg/engine/clone.go

@@ -193,7 +193,7 @@ func (m *clonePackageMutation) cloneFromGit(ctx context.Context, gitPackage *api
 	}, nil
 }
 
-func (m *clonePackageMutation) cloneFromOci(ctx context.Context, ociPackage *api.OciPackage) (repository.PackageResources, error) {
+func (m *clonePackageMutation) cloneFromOci(_ context.Context, _ *api.OciPackage) (repository.PackageResources, error) {
 	return repository.PackageResources{}, errors.New("clone from OCI is not implemented")
 }
 

pkg/engine/clone_test.go

@@ -115,7 +115,7 @@ func createRepoWithContents(t *testing.T, contentDir string) *gogit.Repository {
 	return repo
 }
 
-func startGitServer(t *testing.T, repo *git.Repo, opts ...git.GitServerOption) string {
+func startGitServer(t *testing.T, repo *git.Repo, _ ...git.GitServerOption) string {
 	key := "default"
 	repos := git.NewStaticRepos()
 	if err := repos.Add(key, repo); err != nil {

pkg/engine/engine.go

@@ -17,6 +17,7 @@ package engine
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"io/fs"
 	"os"
@@ -349,30 +350,6 @@ func ensureUniqueWorkspaceName(obj *api.PackageRevision, existingRevs []reposito
 	return nil
 }
 
-func getPackageRevision(ctx context.Context, repo repository.Repository, name string) (repository.PackageRevision, bool, error) {
-	repoPkgRevs, err := repo.ListPackageRevisions(ctx, repository.ListPackageRevisionFilter{
-		KubeObjectName: name,
-	})
-	if err != nil {
-		return nil, false, err
-	}
-	if len(repoPkgRevs) == 0 {
-		return nil, false, nil
-	}
-	return repoPkgRevs[0], true, nil
-}
-
-// TODO: See if we can use a library here for parsing OCI image urls
-func getBaseImage(image string) string {
-	if s := strings.Split(image, "@sha256:"); len(s) > 1 {
-		return s[0]
-	}
-	if s := strings.Split(image, ":"); len(s) > 1 {
-		return s[0]
-	}
-	return image
-}
-
 func taskTypeOneOf(taskType api.TaskType, oneOf ...api.TaskType) bool {
 	for _, tt := range oneOf {
 		if taskType == tt {
@@ -517,7 +494,7 @@ func (cad *cadEngine) UpdatePackageRevision(ctx context.Context, version string,
 	}
 
 	if newRV != oldObj.GetResourceVersion() {
-		return nil, apierrors.NewConflict(api.Resource("packagerevisions"), oldObj.GetName(), fmt.Errorf(OptimisticLockErrorMsg))
+		return nil, apierrors.NewConflict(api.Resource("packagerevisions"), oldObj.GetName(), errors.New(OptimisticLockErrorMsg))
 	}
 
 	repo, err := cad.cache.OpenRepository(ctx, repositoryObj)
@@ -918,12 +895,12 @@ func (cad *cadEngine) CreatePackage(ctx context.Context, repositoryObj *configap
 }
 
 func (cad *cadEngine) UpdatePackage(ctx context.Context, repositoryObj *configapi.Repository, oldPackage *Package, oldObj, newObj *api.PorchPackage) (*Package, error) {
-	ctx, span := tracer.Start(ctx, "cadEngine::UpdatePackage", trace.WithAttributes())
+	_, span := tracer.Start(ctx, "cadEngine::UpdatePackage", trace.WithAttributes())
 	defer span.End()
 
 	// TODO
 	var pkg *Package
-	return pkg, fmt.Errorf("Updating packages is not yet supported")
+	return pkg, errors.New("updating packages is not yet supported")
 }
 
 func (cad *cadEngine) DeletePackage(ctx context.Context, repositoryObj *configapi.Repository, oldPackage *Package) error {
@@ -957,7 +934,7 @@ func (cad *cadEngine) UpdatePackageResources(ctx context.Context, repositoryObj
 	}
 
 	if newRV != old.GetResourceVersion() {
-		return nil, nil, apierrors.NewConflict(api.Resource("packagerevisionresources"), old.GetName(), fmt.Errorf(OptimisticLockErrorMsg))
+		return nil, nil, apierrors.NewConflict(api.Resource("packagerevisionresources"), old.GetName(), errors.New(OptimisticLockErrorMsg))
 	}
 
 	// Validate package lifecycle. Can only update a draft.
@@ -1222,7 +1199,7 @@ type mutationReplaceResources struct {
 }
 
 func (m *mutationReplaceResources) Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.TaskResult, error) {
-	ctx, span := tracer.Start(ctx, "mutationReplaceResources::Apply", trace.WithAttributes())
+	_, span := tracer.Start(ctx, "mutationReplaceResources::Apply", trace.WithAttributes())
 	defer span.End()
 
 	patch := &api.PackagePatchTaskSpec{}

pkg/engine/mergekey.go

@@ -27,7 +27,7 @@ import (
 // identity of resources in a downstream package with the ones in upstream package
 // This is required to ensure package update is able to merge resources in
 // downstream package with upstream.
-func ensureMergeKey(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, error) {
+func ensureMergeKey(_ context.Context, resources repository.PackageResources) (repository.PackageResources, error) {
 	pr := &packageReader{
 		input: resources,
 		extra: map[string]string{},

pkg/engine/patchgen.go

@@ -53,7 +53,7 @@ type applyPatchMutation struct {
 var _ mutation = &applyPatchMutation{}
 
 func (m *applyPatchMutation) Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.TaskResult, error) {
-	ctx, span := tracer.Start(ctx, "applyPatchMutation:::Apply", trace.WithAttributes())
+	_, span := tracer.Start(ctx, "applyPatchMutation:::Apply", trace.WithAttributes())
 	defer span.End()
 
 	result := repository.PackageResources{
@@ -128,7 +128,7 @@ func (m *applyPatchMutation) Apply(ctx context.Context, resources repository.Pac
 	return result, &api.TaskResult{Task: m.task}, nil
 }
 
-func buildPatchMutation(ctx context.Context, task *api.Task) (mutation, error) {
+func buildPatchMutation(_ context.Context, task *api.Task) (mutation, error) {
 	if task.Patch == nil {
 		return nil, fmt.Errorf("patch not set for task of type %q", task.Type)
 	}

pkg/engine/update.go

@@ -20,15 +20,9 @@ import (
 	"path/filepath"
 
 	"github.com/nephio-project/porch/internal/kpt/util/update"
-	"github.com/nephio-project/porch/pkg/kpt/printer"
 	"github.com/nephio-project/porch/pkg/repository"
 )
 
-// packageUpdater knows how to update a local package given original and upstream package resources.
-type packageUpdater interface {
-	Update(ctx context.Context, localResources, originalResources, upstreamResources repository.PackageResources) (updatedResources repository.PackageResources, err error)
-}
-
 // defaultPackageUpdater implements packageUpdater interface.
 type defaultPackageUpdater struct{}
 
@@ -76,29 +70,24 @@ func (m *defaultPackageUpdater) Update(
 }
 
 // PkgUpdate is a wrapper around `kpt pkg update`, running it against the package in packageDir
-func (m *defaultPackageUpdater) do(ctx context.Context, localPkgDir, originalPkgDir, upstreamPkgDir string) error {
-	// TODO: Printer should be a logr
-	pr := printer.New(os.Stdout, os.Stderr)
-	ctx = printer.WithContext(ctx, pr)
-
-	{
-		relPath := "."
-		localPath := filepath.Join(localPkgDir, relPath)
-		updatedPath := filepath.Join(upstreamPkgDir, relPath)
-		originPath := filepath.Join(originalPkgDir, relPath)
-		isRoot := true
-
-		updateOptions := update.Options{
-			RelPackagePath: relPath,
-			LocalPath:      localPath,
-			UpdatedPath:    updatedPath,
-			OriginPath:     originPath,
-			IsRoot:         isRoot,
-		}
-		updater := update.ResourceMergeUpdater{}
-		if err := updater.Update(updateOptions); err != nil {
-			return err
-		}
+func (m *defaultPackageUpdater) do(_ context.Context, localPkgDir, originalPkgDir, upstreamPkgDir string) error {
+	relPath := "."
+	localPath := filepath.Join(localPkgDir, relPath)
+	updatedPath := filepath.Join(upstreamPkgDir, relPath)
+	originPath := filepath.Join(originalPkgDir, relPath)
+	isRoot := true
+
+	updateOptions := update.Options{
+		RelPackagePath: relPath,
+		LocalPath:      localPath,
+		UpdatedPath:    updatedPath,
+		OriginPath:     originPath,
+		IsRoot:         isRoot,
 	}
+	updater := update.ResourceMergeUpdater{}
+	if err := updater.Update(updateOptions); err != nil {
+		return err
+	}
+
 	return nil
 }