Respect Registration Directory (#3017)

If a sub-directory is specified at repository registration, the repository will
only include package revisions in the specified directory. Likewise, it will
only create packages in the specified subdirectory.

In order to maintain stable resource names within the repo (regardless of the
registration parameters), the package names are unaffected by the directory
projection.

Author: martinmaly

porch/repository/pkg/git/dir.go

@@ -0,0 +1,33 @@
+// Copyright 2022 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package git
+
+import "strings"
+
+// Determines whether a package specified by a path is in a directory given.
+func packageInDirectory(pkg, dir string) bool {
+	if dir == "" {
+		return true
+	}
+	if strings.HasPrefix(pkg, dir) {
+		if len(pkg) == len(dir) {
+			return true
+		}
+		if pkg[len(dir)] == '/' {
+			return true
+		}
+	}
+	return false
+}

porch/repository/pkg/git/dir_test.go

@@ -0,0 +1,68 @@
+// Copyright 2022 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package git
+
+import "testing"
+
+func TestPackageInDirectory(t *testing.T) {
+	for _, tc := range []struct {
+		pkg, dir string
+		want     bool
+	}{
+		{
+			pkg:  "root/nested",
+			dir:  "",
+			want: true,
+		}, {
+			pkg:  "catalog/package",
+			dir:  "cat",
+			want: false,
+		},
+		{
+			pkg:  "catalog/package",
+			dir:  "catalog",
+			want: true,
+		},
+		{
+			pkg:  "catalog/package/nested",
+			dir:  "catalog/packages",
+			want: false,
+		},
+		{
+			pkg:  "catalog/package/nested",
+			dir:  "catalog/package",
+			want: true,
+		},
+		{
+			pkg:  "catalog/package/nested",
+			dir:  "catalog/package/nest",
+			want: false,
+		},
+		{
+			pkg:  "catalog/package/nested",
+			dir:  "catalog/package/nested",
+			want: true,
+		},
+		{
+			pkg:  "catalog/package/nested",
+			dir:  "catalog/package/nested/even-more",
+			want: false,
+		},
+	} {
+		if got, want := packageInDirectory(tc.pkg, tc.dir), tc.want; got != want {
+			t.Errorf("packageInDirectory(%q, %q): got %t, want %t", tc.pkg, tc.dir, got, want)
+		}
+	}
+}

porch/repository/pkg/git/git.go

@@ -100,6 +100,7 @@ func OpenRepository(ctx context.Context, name, namespace string, spec *configapi
 		namespace:          namespace,
 		repo:               repo,
 		branch:             branch,
+		directory:          strings.Trim(spec.Directory, "/"),
 		secret:             spec.SecretRef.Name,
 		credentialResolver: opts.CredentialResolver,
 		userInfoProvider:   opts.UserInfoProvider,
@@ -117,6 +118,7 @@ type gitRepository struct {
 	namespace          string     // Repository resource namespace
 	secret             string     // Name of the k8s Secret resource containing credentials
 	branch             BranchName // The main branch from repository registration (defaults to 'main' if unspecified)
+	directory          string     // Directory within the repository where to look for packages.
 	repo               *git.Repository
 	cachedCredentials  transport.AuthMethod
 	credentialResolver repository.CredentialResolver
@@ -184,6 +186,10 @@ func (r *gitRepository) ListPackageRevisions(ctx context.Context) ([]repository.
 }
 
 func (r *gitRepository) CreatePackageRevision(ctx context.Context, obj *v1alpha1.PackageRevision) (repository.PackageDraft, error) {
+	if !packageInDirectory(obj.Spec.PackageName, r.directory) {
+		return nil, fmt.Errorf("cannot create %s outside of repository directory %q", obj.Spec.PackageName, r.directory)
+	}
+
 	var base plumbing.Hash
 	refName := r.branch.RefInLocal()
 	switch main, err := r.repo.Reference(refName, true); {
@@ -345,6 +351,10 @@ func (r *gitRepository) GetPackage(version, path string) (repository.PackageRevi
 }
 
 func (r *gitRepository) loadPackageRevision(version, path string, hash plumbing.Hash) (repository.PackageRevision, kptfilev1.GitLock, error) {
+	if !packageInDirectory(path, r.directory) {
+		return nil, kptfilev1.GitLock{}, fmt.Errorf("cannot find package %s@%s; package is not under the Repository.spec.directory", path, version)
+	}
+
 	git := r.repo
 
 	origin, err := git.Remote("origin")
@@ -402,6 +412,16 @@ func (r *gitRepository) discoverFinalizedPackages(ref *plumbing.Reference) ([]re
 		return nil, err
 	}
 
+	var result []repository.PackageRevision
+
+	// Recurse into the specified directory
+	if r.directory != "" {
+		tree, err = tree.Tree(r.directory)
+		if err == object.ErrDirectoryNotFound {
+			return result, nil
+		}
+	}
+
 	var revision string
 	if rev, ok := getBranchNameInLocalRepo(ref.Name()); ok {
 		revision = rev
@@ -412,8 +432,7 @@ func (r *gitRepository) discoverFinalizedPackages(ref *plumbing.Reference) ([]re
 		return nil, fmt.Errorf("cannot determine revision from ref: %q", rev)
 	}
 
-	var result []repository.PackageRevision
-	if err := discoverPackagesInTree(git, tree, "", func(dir string, tree, kptfile plumbing.Hash) error {
+	if err := discoverPackagesInTree(git, tree, r.directory, func(dir string, tree, kptfile plumbing.Hash) error {
 		result = append(result, &gitPackageRevision{
 			parent:   r,
 			path:     dir,
@@ -467,6 +486,11 @@ func (r *gitRepository) loadDraft(ref *plumbing.Reference) (*gitPackageRevision,
 		return nil, err
 	}
 
+	// Only load drafts in the directory specified at repository registration.
+	if !packageInDirectory(name, r.directory) {
+		return nil, nil
+	}
+
 	commit, err := r.repo.CommitObject(ref.Hash())
 	if err != nil {
 		return nil, fmt.Errorf("cannot resolve draft branch to commit (corrupted repository?): %w", err)
@@ -546,6 +570,10 @@ func (r *gitRepository) loadTaggedPackages(tag *plumbing.Reference) ([]repositor
 	// tag=<package path>/version
 	path, revision := name[:slash], name[slash+1:]
 
+	if !packageInDirectory(path, r.directory) {
+		return nil, nil
+	}
+
 	commit, err := r.repo.CommitObject(tag.Hash())
 	if err != nil {
 		return nil, fmt.Errorf("cannot resolve tag %q to commit (corrupted repository?): %w", name, err)

porch/repository/pkg/git/git_test.go

@@ -932,3 +932,80 @@ func TestNested(t *testing.T) {
 		t.Errorf("Discovered packages differ: (-want,+got): %s", cmp.Diff(want, got))
 	}
 }
+
+func createPackageRevisionMap(revisions []repository.PackageRevision) map[string]bool {
+	result := map[string]bool{}
+	for _, pr := range revisions {
+		key := pr.Key()
+		result[fmt.Sprintf("%s/%s", key.Package, key.Revision)] = true
+	}
+	return result
+}
+
+func sliceToSet(s []string) map[string]bool {
+	result := map[string]bool{}
+	for _, v := range s {
+		result[v] = true
+	}
+	return result
+}
+
+func TestNestedDirectories(t *testing.T) {
+	ctx := context.Background()
+
+	for _, tc := range []struct {
+		directory string
+		packages  []string
+	}{
+		{
+			directory: "sample",
+			packages:  []string{"sample/v1", "sample/v2", "sample/main"},
+		},
+		{
+			directory: "nonexistent",
+			packages:  []string{},
+		},
+		{
+			directory: "catalog/gcp",
+			packages: []string{
+				"catalog/gcp/cloud-sql/v1",
+				"catalog/gcp/spanner/v1",
+				"catalog/gcp/bucket/v2",
+				"catalog/gcp/bucket/v1",
+				"catalog/gcp/bucket/main",
+			},
+		},
+	} {
+		t.Run(tc.directory, func(t *testing.T) {
+			tempdir := t.TempDir()
+			tarfile := filepath.Join("testdata", "nested-repository.tar")
+			_, address := ServeGitRepository(t, tarfile, tempdir)
+
+			const (
+				repositoryName = "directory"
+				namespace      = "default"
+			)
+
+			git, err := OpenRepository(ctx, repositoryName, namespace, &configapi.GitRepository{
+				Repo:      address,
+				Branch:    "main",
+				Directory: tc.directory,
+			}, tempdir, GitRepositoryOptions{})
+			if err != nil {
+				t.Fatalf("Failed to open Git repository loaded from %q with directory %q: %v", tarfile, tc.directory, err)
+			}
+
+			revisions, err := git.ListPackageRevisions(ctx)
+			if err != nil {
+				t.Fatalf("Failed to list packages from %q: %v", tarfile, err)
+			}
+
+			got := createPackageRevisionMap(revisions)
+			want := sliceToSet(tc.packages)
+
+			if !cmp.Equal(want, got) {
+				t.Errorf("Packages rooted in %q; Unexpected result (-want,+got): %s", tc.directory, cmp.Diff(want, got))
+			}
+		})
+	}
+}