Tarball packages can now contain local dependencies (#1728)

* Tarball packages can now contain local dependencies

Summary:
Currently, KUDO allows local dependencies like `package: ../child` or `../child.tgz` however, only if the parent operator is *also* in a local directory. This PR adds the possibility for a packaged operator (a tarball) to contain all the dependencies it needs. While, the preferred way is to keep the individual operators in the repo, sometimes a self-contained operator package (like an uber-jar) is more desirable e.g.:
- an air-gaped environment needs a functioning repo before operator installation which significantly increases the overall deployment overhead
- some dependency operators might be heavily customized official ones, which makes it hard to contribute them back to the official repo

To keep the packages backward-compatible, we continue to expect the parent operator at the top-level of the tarball, so that all the dependencies must share the same base path and be at least one level deeper e.g.:

```
.
└── child
│   └── operator.yaml
└── operator.yaml
```

A dependency operator (`child` above) can have its dependencies which are always relative to the `operator.yaml` which declares them. See [this commit](c237347) for more information.

Fixes #1701

Signed-off-by: Andreas Neumann <[email protected]>
Co-authored-by: Andreas Neumann <[email protected]>

Author: zen-dog

pkg/controller/instance/instance_controller.go

@@ -327,9 +327,9 @@ func (r *Reconciler) resolveDependencies(i *kudoapi.Instance, ov *kudoapi.Operat
 	if i.IsChildInstance() {
 		return nil
 	}
-	resolver := &InClusterResolver{ns: ov.Namespace, c: r.Client}
+	resolver := NewInClusterResolver(r.Client, ov.Namespace)
 
-	_, err := dependencies.Resolve(ov.Name, ov, resolver)
+	_, err := dependencies.Resolve(ov, resolver)
 	if err != nil {
 		return engine.ExecutionError{Err: fmt.Errorf("%w%v", engine.ErrFatalExecution, err), EventName: "CircularDependency"}
 	}

pkg/controller/instance/resolver_incluster.go

@@ -21,7 +21,14 @@ type InClusterResolver struct {
 	ns string
 }
 
-func (r InClusterResolver) Resolve(name string, appVersion string, operatorVersion string) (*packages.Resources, error) {
+func NewInClusterResolver(client client.Client, ns string) *InClusterResolver {
+	return &InClusterResolver{
+		c:  client,
+		ns: ns,
+	}
+}
+
+func (r InClusterResolver) Resolve(name string, appVersion string, operatorVersion string) (*packages.PackageScope, error) {
 	ovn := kudoapi.OperatorVersionName(name, appVersion, operatorVersion)
 
 	ov, err := kudoapi.GetOperatorVersionByName(ovn, r.ns, r.c)
@@ -34,9 +41,11 @@ func (r InClusterResolver) Resolve(name string, appVersion string, operatorVersi
 		return nil, fmt.Errorf("failed to resolve operator %s/%s", r.ns, name)
 	}
 
-	return &packages.Resources{
+	res := &packages.Resources{
 		Operator:        o,
 		OperatorVersion: ov,
 		Instance:        nil,
-	}, nil
+	}
+
+	return &packages.PackageScope{Resources: res, DependenciesResolver: r}, nil
 }

pkg/kudoctl/cmd/install/install.go

@@ -2,12 +2,14 @@ package install
 
 import (
 	"fmt"
+	"os"
 	"time"
 
 	"github.com/spf13/afero"
 
 	"github.com/kudobuilder/kudo/pkg/kudoctl/clog"
 	"github.com/kudobuilder/kudo/pkg/kudoctl/env"
+	"github.com/kudobuilder/kudo/pkg/kudoctl/packages"
 	"github.com/kudobuilder/kudo/pkg/kudoctl/packages/install"
 	pkgresolver "github.com/kudobuilder/kudo/pkg/kudoctl/packages/resolver"
 	deps "github.com/kudobuilder/kudo/pkg/kudoctl/resources/dependencies"
@@ -80,19 +82,24 @@ func installOperator(operatorArgument string, options *Options, fs afero.Fs, set
 
 	clog.V(3).Printf("getting operator package")
 
-	var resolver pkgresolver.Resolver
+	var resolver packages.Resolver
 	if options.InCluster {
 		resolver = pkgresolver.NewInClusterResolver(kudoClient, settings.Namespace)
 	} else {
-		resolver = pkgresolver.New(repoClient)
+		wd, err := os.Getwd()
+		if err != nil {
+			return fmt.Errorf("failed to get current working directory: %v", err)
+		}
+
+		resolver = pkgresolver.NewPackageResolver(repoClient, wd)
 	}
 
 	pr, err := resolver.Resolve(operatorArgument, options.AppVersion, options.OperatorVersion)
 	if err != nil {
 		return fmt.Errorf("failed to resolve operator package for: %s %w", operatorArgument, err)
 	}
 
-	dependencies, err := deps.Resolve(operatorArgument, pr.OperatorVersion, resolver)
+	dependencies, err := deps.Resolve(pr.Resources.OperatorVersion, pr.DependenciesResolver)
 	if err != nil {
 		return err
 	}
@@ -111,7 +118,7 @@ func installOperator(operatorArgument string, options *Options, fs afero.Fs, set
 		kudoClient,
 		options.InstanceName,
 		settings.Namespace,
-		*pr,
+		*pr.Resources,
 		options.Parameters,
 		dependencies,
 		installOpts)

pkg/kudoctl/cmd/package_list.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"fmt"
 	"io"
+	"os"
 
 	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
@@ -27,19 +28,19 @@ For list commands, the argument passed represents an operator.   That representa
 `
 
 const packageListExamples = `  # show list of parameters from local operator folder   
-  kubectl kudo package list parameters local-folder
+  kubectl kudo package list parameters ./local-folder
 
   # show list of parameters from zookeeper (where zookeeper is name of package in KUDO repository)
   kubectl kudo package list parameters zookeeper
 
   # show list of tasks from local operator folder
-  kubectl kudo package list tasks local-folder
+  kubectl kudo package list tasks ./local-folder
 
   # show list of tasks from zookeeper (where zookeeper is name of package in KUDO repository)
   kubectl kudo package list tasks zookeeper
 
   # show list of plans from local operator folder
-  kubectl kudo package list plans local-folder
+  kubectl kudo package list plans ./local-folder
 
   # show plans from zookeeper (where zookeeper is name of package in KUDO repository)
   kubectl kudo package list plans zookeeper
@@ -69,10 +70,15 @@ func packageDiscovery(fs afero.Fs, settings *env.Settings, repoName, pathOrName,
 	clog.V(3).Printf("repository used %s", repository)
 
 	clog.V(3).Printf("getting package pkg files for %v with version: %v_%v", pathOrName, appVersion, operatorVersion)
-	resolver := pkgresolver.New(repository)
+	wd, err := os.Getwd()
+	if err != nil {
+		return nil, fmt.Errorf("failed to get current working directory: %v", err)
+	}
+
+	resolver := pkgresolver.NewPackageResolver(repository, wd)
 	pr, err := resolver.Resolve(pathOrName, appVersion, operatorVersion)
 	if err != nil {
 		return nil, fmt.Errorf("failed to resolve package files for operator: %s: %w", pathOrName, err)
 	}
-	return pr, nil
+	return pr.Resources, nil
 }

pkg/kudoctl/cmd/upgrade.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"errors"
 	"fmt"
+	"os"
 
 	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
@@ -103,18 +104,23 @@ func runUpgrade(args []string, options *options, fs afero.Fs, settings *env.Sett
 		return fmt.Errorf("could not build operator repository: %w", err)
 	}
 
-	resolver := pkgresolver.New(repository)
+	wd, err := os.Getwd()
+	if err != nil {
+		return fmt.Errorf("failed to get current working directory: %v", err)
+	}
+
+	resolver := pkgresolver.NewPackageResolver(repository, wd)
 	pr, err := resolver.Resolve(packageToUpgrade, options.AppVersion, options.OperatorVersion)
 	if err != nil {
 		return fmt.Errorf("failed to resolve operator package for: %s: %w", packageToUpgrade, err)
 	}
 
-	pr.OperatorVersion.SetNamespace(settings.Namespace)
+	pr.Resources.OperatorVersion.SetNamespace(settings.Namespace)
 
-	dependencies, err := deps.Resolve(packageToUpgrade, pr.OperatorVersion, resolver)
+	dependencies, err := deps.Resolve(pr.Resources.OperatorVersion, pr.DependenciesResolver)
 	if err != nil {
 		return err
 	}
 
-	return upgrade.OperatorVersion(kc, pr.OperatorVersion, options.InstanceName, options.Parameters, dependencies)
+	return upgrade.OperatorVersion(kc, pr.Resources.OperatorVersion, options.InstanceName, options.Parameters, dependencies)
 }

pkg/kudoctl/packages/reader/parser.go

@@ -98,7 +98,8 @@ func parsePackageFile(filePath string, fileBytes []byte, currentPackage *package
 		}
 		currentPackage.Params = &paramsFile
 	default:
-		return fmt.Errorf("unexpected file when reading package from filesystem: %s", filePath)
+		// we ignore unexpected files as they might belong to a dependency operator
+		return nil
 	}
 	return nil
 }

pkg/kudoctl/packages/reader/parser_test.go

@@ -43,8 +43,6 @@ func TestParsePackageFile(t *testing.T) {
 		{filePath: "templates/some/nested/template2.yaml", isTemplate: true, fileContent: "not-empty"},
 		{filePath: "./templates/some-template.yaml", isTemplate: true, fileContent: "not-empty"},
 		{filePath: "./templates/with/subdirectory/some-template.yaml", isTemplate: true, fileContent: "not-empty"},
-		{filePath: "templates_without_path.yaml", expectedError: errors.New("unexpected file when reading package from filesystem: templates_without_path.yaml"), fileContent: "not-empty"},
-		{filePath: "invalid.yaml", expectedError: errors.New("unexpected file when reading package from filesystem: invalid.yaml")},
 		{filePath: "operator.yaml", isOperator: true, expectedError: errors.New("failed to parse yaml into valid operator operator.yaml")},
 	}
 

pkg/kudoctl/packages/reader/reader.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"path"
 
 	"github.com/spf13/afero"
 
@@ -15,16 +16,19 @@ import (
 	"github.com/kudobuilder/kudo/pkg/kudoctl/packages/convert"
 )
 
-func ReadTar(fs afero.Fs, path string) (*packages.Resources, error) {
+// ResourcesFromTar extracts files from the tar provides by he `inFs` and the `path` and converts
+// them to resources. All the extracted files are saved in the `outFs` for later use (e.g. searching
+// for local dependencies)
+func ResourcesFromTar(in afero.Fs, out afero.Fs, path string) (*packages.Resources, error) {
 	// 1. read the tarball
-	b, err := afero.ReadFile(fs, path)
+	b, err := afero.ReadFile(in, path)
 	if err != nil {
 		return nil, err
 	}
 	buf := bytes.NewBuffer(b)
 
-	// 2. ParseTgz tar files
-	files, err := ParseTgz(buf)
+	// 2. extract and parse tar files
+	files, err := PackageFilesFromTar(out, buf)
 	if err != nil {
 		return nil, fmt.Errorf("while parsing package files from %s: %v", path, err)
 	}
@@ -38,11 +42,30 @@ func ReadTar(fs afero.Fs, path string) (*packages.Resources, error) {
 	return resources, nil
 }
 
-func ParseTgz(r io.Reader) (*packages.Files, error) {
-	gzr, err := gzip.NewReader(r)
+// PackageFilesFromTar extracts a tgz archive held by passed reader and returns the package files.
+// Additionally, all the files are saved in the `out` Fs (in  the root `/` folder).
+func PackageFilesFromTar(out afero.Fs, r io.Reader) (*packages.Files, error) {
+	err := ExtractTar(out, r)
 	if err != nil {
 		return nil, err
 	}
+
+	pf, err := PackageFilesFromDir(out, "/")
+	return pf, err
+}
+
+// ExtractTar extract a tgz archive into the given filesystem. This is a generic extract method
+// so no package parsing is performed.
+// *Note:* all file paths are prepended by `/` and are extracted into the root of the passed Fs.
+// By default tar strips out the leading slash, but leaves `./` when packing a folder and doesn't
+// add it when packing a file so that depending on how it was packed the same file might have a path
+// like `templates/foo.yaml` or `./templates/foo.yaml`. Since we're extracting into the empty MemFs,
+// we can avoid the inconsistency and just extract into the root.
+func ExtractTar(out afero.Fs, r io.Reader) error {
+	gzr, err := gzip.NewReader(r)
+	if err != nil {
+		return err
+	}
 	defer func() {
 		err := gzr.Close()
 		if err != nil {
@@ -52,19 +75,18 @@ func ParseTgz(r io.Reader) (*packages.Files, error) {
 
 	tr := tar.NewReader(gzr)
 
-	result := newPackageFiles()
 	for {
 		header, err := tr.Next()
 
 		switch {
 
 		// if no more files are found return
 		case err == io.EOF:
-			return &result, nil
+			return nil
 
 		// return any other error
 		case err != nil:
-			return nil, err
+			return err
 
 		// if the header is nil, just skip it (not sure how this happens)
 		case header == nil:
@@ -74,18 +96,23 @@ func ParseTgz(r io.Reader) (*packages.Files, error) {
 		// check the file type
 		switch header.Typeflag {
 
+		// there are no folders in the tar, only files with nested file names e.g. `templates/foo.yaml` ¯\_(ツ)_/¯
 		case tar.TypeDir:
-			// we don't need to handle folders, files have folder name in their names and that should be enough
+			clog.Printf("Tar file contained directory. Did not expect this: %s", header.Name)
+			continue
 
 		case tar.TypeReg:
+			// read the file
 			buf, err := ioutil.ReadAll(tr)
 			if err != nil {
-				return nil, fmt.Errorf("while reading file %s from package tarball: %v", header.Name, err)
+				return fmt.Errorf("while reading file %s from package tarball: %v", header.Name, err)
 			}
 
-			err = parsePackageFile(header.Name, buf, &result)
+			// copy over contents. the files are extracted into the root of the passed Fs
+			// nolint:gosec
+			err = afero.WriteFile(out, path.Join("/", header.Name), buf, 0644)
 			if err != nil {
-				return nil, err
+				return fmt.Errorf("while writing file %s: %v", header.Name, err)
 			}
 		}
 	}

pkg/kudoctl/packages/reader/reader_tar.go

@@ -34,33 +34,32 @@ func TestReadFileSystemPackage(t *testing.T) {
 
 		t.Run(fmt.Sprintf("%s-from-%s", tt.name, tt.path), func(t *testing.T) {
 			var err error
-			var pr *packages.Resources
+			var got *packages.Resources
 
 			if strings.HasSuffix(tt.path, ".tgz") {
-				pr, err = ReadTar(fs, tt.path)
+				got, err = ResourcesFromTar(fs, afero.NewMemMapFs(), tt.path)
 			} else {
-				pr, err = ResourcesFromDir(fs, tt.path)
+				got, err = ResourcesFromDir(fs, tt.path)
 			}
 
 			assert.NoError(t, err, "unexpected error while reading the package")
-			actual := pr
 
-			actual.Instance.ObjectMeta.Name = tt.instanceName
+			got.Instance.ObjectMeta.Name = tt.instanceName
 			golden, err := loadResourcesFromPath(tt.goldenFiles)
 			if err != nil {
 				t.Errorf("Found unexpected error when loading golden files: %v", err)
 			}
 
 			// we need to sort here because current yaml parsing is not preserving the order of fields
 			// at the same time, the deep library we use for equality does not support ignoring order
-			sort.Slice(actual.OperatorVersion.Spec.Parameters, func(i, j int) bool {
-				return actual.OperatorVersion.Spec.Parameters[i].Name < actual.OperatorVersion.Spec.Parameters[j].Name
+			sort.Slice(got.OperatorVersion.Spec.Parameters, func(i, j int) bool {
+				return got.OperatorVersion.Spec.Parameters[i].Name < got.OperatorVersion.Spec.Parameters[j].Name
 			})
 			sort.Slice(golden.OperatorVersion.Spec.Parameters, func(i, j int) bool {
 				return golden.OperatorVersion.Spec.Parameters[i].Name < golden.OperatorVersion.Spec.Parameters[j].Name
 			})
 
-			assert.Equal(t, golden, actual)
+			assert.Equal(t, golden, got)
 		})
 	}
 }