Cleanup Code (#2811)

kptdev · Feb 22, 2022 · f1dbc0b · f1dbc0b
1 parent 881b7f0
commit f1dbc0b
Show file tree

Hide file tree

Showing 8 changed files with 16 additions and 17 deletions.
diff --git a/porch/apiserver/pkg/registry/porch/packagerevision.go b/porch/apiserver/pkg/registry/porch/packagerevision.go
@@ -19,7 +19,6 @@ import (
 	"fmt"
 
 	api "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1"
-	"github.com/GoogleContainerTools/kpt/porch/controllers/pkg/apis/porch/v1alpha1"
 	configapi "github.com/GoogleContainerTools/kpt/porch/controllers/pkg/apis/porch/v1alpha1"
 	"github.com/GoogleContainerTools/kpt/porch/repository/pkg/repository"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -185,7 +184,7 @@ func (r *packageRevisions) Update(ctx context.Context, name string, objInfo rest
 		return nil, false, apierrors.NewBadRequest(fmt.Sprintf("invalid name %q", name))
 	}
 
-	var repositoryObj v1alpha1.Repository
+	var repositoryObj configapi.Repository
 	repositoryID := types.NamespacedName{Namespace: ns, Name: nameTokens.RepositoryName}
 	if err := r.coreClient.Get(ctx, repositoryID, &repositoryObj); err != nil {
 		if apierrors.IsNotFound(err) {

diff --git a/porch/apiserver/pkg/registry/porch/packagerevisions_approval.go b/porch/apiserver/pkg/registry/porch/packagerevisions_approval.go
@@ -51,6 +51,6 @@ func (a *packageRevisionsApproval) Get(ctx context.Context, name string, options
 // may allow updates creates the object - they should set the created boolean
 // to true.
 func (a *packageRevisionsApproval) Update(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) {
-	forceAllowCreate = false // do not allow create on update
-	return a.revisions.Update(ctx, name, objInfo, createValidation, updateValidation, forceAllowCreate, options)
+	allowCreate := false // do not allow create on update
+	return a.revisions.Update(ctx, name, objInfo, createValidation, updateValidation, allowCreate, options)
 }
diff --git a/porch/engine/pkg/engine/clone.go b/porch/engine/pkg/engine/clone.go
@@ -61,7 +61,7 @@ func (m *clonePackageMutation) Apply(ctx context.Context, resources repository.P
 }
 
 func (m *clonePackageMutation) cloneFromRegisteredRepository(ctx context.Context) (repository.PackageResources, error) {
-	return repository.PackageResources{}, errors.New("Clone from Registered Repository is not implemented")
+	return repository.PackageResources{}, errors.New("clone from Registered Repository is not implemented")
 }
 
 func (m *clonePackageMutation) cloneFromGit(ctx context.Context, gitPackage *api.GitPackage) (repository.PackageResources, error) {
@@ -118,5 +118,5 @@ func (m *clonePackageMutation) cloneFromGit(ctx context.Context, gitPackage *api
 }
 
 func (m *clonePackageMutation) cloneFromOci(ctx context.Context, ociPackage *api.OciPackage) (repository.PackageResources, error) {
-	return repository.PackageResources{}, errors.New("Clone from OCI is not implemented")
+	return repository.PackageResources{}, errors.New("clone from OCI is not implemented")
 }
diff --git a/porch/engine/pkg/engine/clone_test.go b/porch/engine/pkg/engine/clone_test.go
@@ -47,6 +47,9 @@ func createRepoWithContents(t *testing.T, contentDir string) *gogit.Repository {
 	}
 
 	if err := filepath.Walk(contentDir, func(path string, info fs.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
 		if info.IsDir() {
 			return nil
 		} else if !info.Mode().IsRegular() {

diff --git a/porch/repository/pkg/git/git.go b/porch/repository/pkg/git/git.go
@@ -29,7 +29,6 @@ import (
 	"github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1"
 	configapi "github.com/GoogleContainerTools/kpt/porch/controllers/pkg/apis/porch/v1alpha1"
 	"github.com/GoogleContainerTools/kpt/porch/repository/pkg/repository"
-	"github.com/go-git/go-git/v5"
 	gogit "github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/config"
 	"github.com/go-git/go-git/v5/plumbing"
@@ -258,7 +257,7 @@ func (r *gitRepository) ApprovePackageRevision(ctx context.Context, path, revisi
 
 	newRef := plumbing.NewHashReference(approvedName, oldRef.Hash())
 
-	options := &git.PushOptions{
+	options := &gogit.PushOptions{
 		RemoteName:        "origin",
 		RefSpecs:          []config.RefSpec{},
 		Auth:              auth,

diff --git a/porch/repository/pkg/git/testing.go b/porch/repository/pkg/git/testing.go
@@ -26,7 +26,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/go-git/go-git/v5"
 	gogit "github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/plumbing"
 	"github.com/go-git/go-git/v5/plumbing/filemode"
@@ -448,7 +447,7 @@ func (p *objectWalker) walkObjectTree(hash plumbing.Hash) error {
 	// Fetch the object.
 	obj, err := object.GetObject(p.Storer, hash)
 	if err != nil {
-		return fmt.Errorf("Getting object %s failed: %v", hash, err)
+		return fmt.Errorf("getting object %s failed: %v", hash, err)
 	}
 	// Walk all children depending on object type.
 	switch obj := obj.(type) {
@@ -488,13 +487,13 @@ func (p *objectWalker) walkObjectTree(hash plumbing.Hash) error {
 		return p.walkObjectTree(obj.Target)
 	default:
 		// Error out on unhandled object types.
-		return fmt.Errorf("Unknown object %s %s %T\n", obj.ID(), obj.Type(), obj)
+		return fmt.Errorf("unknown object %s %s %T", obj.ID(), obj.Type(), obj)
 	}
 	return nil
 }
 
 // initRepo is a helper that creates a first commit, ensuring the repo is not empty.
-func initRepo(repo *git.Repository) error {
+func initRepo(repo *gogit.Repository) error {
 	store := repo.Storer
 
 	var objectHash plumbing.Hash

diff --git a/porch/repository/pkg/oci/mutate.go b/porch/repository/pkg/oci/mutate.go
@@ -26,7 +26,6 @@ import (
 	"strconv"
 	"time"
 
-	"github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1"
 	api "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1"
 	"github.com/GoogleContainerTools/kpt/porch/repository/pkg/repository"
 	"github.com/google/go-containerregistry/pkg/gcrane"
@@ -107,7 +106,7 @@ type ociPackageDraft struct {
 
 	parent *ociRepository
 
-	tasks []v1alpha1.Task
+	tasks []api.Task
 
 	base      v1.Image
 	tag       name.Tag

diff --git a/porch/test/git/main.go b/porch/test/git/main.go
@@ -42,7 +42,7 @@ func main() {
 
 func run(dirs []string) error {
 	if len(dirs) != 1 {
-		return fmt.Errorf("Expected one path to Git directory to serve. Got %d", len(dirs))
+		return fmt.Errorf("expected one path to Git directory to serve. Got %d", len(dirs))
 	}
 
 	dir := dirs[0]
@@ -80,8 +80,8 @@ func run(dirs []string) error {
 	address := <-addressChannel
 	fmt.Fprintf(os.Stderr, "Listening on %s\n", address)
 
-	wait := make(chan os.Signal)
-	signal.Notify(wait, os.Interrupt, os.Kill)
+	wait := make(chan os.Signal, 1)
+	signal.Notify(wait, os.Interrupt)
 
 	<-wait