From 63fbe531d92d5c77408c8fcadd87c62661ad7c73 Mon Sep 17 00:00:00 2001 From: Aleksey Dukhovniy Date: Fri, 11 Dec 2020 18:11:29 +0100 Subject: [PATCH] Tarball packages can now contain local dependencies (#1728) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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](https://github.com/kudobuilder/kudo/commit/c237347a175bccc9705abff967590794c6073833) for more information. Fixes #1701 Signed-off-by: Andreas Neumann Co-authored-by: Andreas Neumann --- .../instance/instance_controller.go | 4 +- pkg/controller/instance/resolver_incluster.go | 15 ++- pkg/kudoctl/cmd/install/install.go | 15 ++- pkg/kudoctl/cmd/package_list.go | 16 ++- pkg/kudoctl/cmd/upgrade.go | 14 ++- pkg/kudoctl/packages/reader/parser.go | 3 +- pkg/kudoctl/packages/reader/parser_test.go | 2 - pkg/kudoctl/packages/reader/reader.go | 34 ----- pkg/kudoctl/packages/reader/reader_tar.go | 53 ++++++-- pkg/kudoctl/packages/reader/reader_test.go | 15 ++- pkg/kudoctl/packages/resolver/resolver.go | 119 ++++++++++++------ .../packages/resolver/resolver_incluster.go | 13 +- .../resolver/resolver_incluster_test.go | 2 +- .../packages/resolver/resolver_local.go | 70 ++++++----- .../packages/resolver/resolver_local_test.go | 12 +- .../packages/resolver/resolver_test.go | 8 +- pkg/kudoctl/packages/resolver/resolver_url.go | 30 ++--- pkg/kudoctl/packages/types.go | 15 +++ pkg/kudoctl/resources/dependencies/resolve.go | 108 ++++++++-------- .../resources/dependencies/resolve_test.go | 87 +++++-------- .../testdata/operator-with-dependencies.tgz | Bin 0 -> 1029 bytes .../child-operator/operator.yaml | 1 + .../parent-operator/operator.yaml | 1 - pkg/kudoctl/resources/install/operator.go | 38 ------ .../resources/upgrade/operatorversion_test.go | 1 - pkg/kudoctl/util/repo/digest.go | 2 +- pkg/kudoctl/util/repo/resolver_repo.go | 8 +- .../child-operator/operator.yaml | 1 + .../parent-operator/operator.yaml | 1 - 29 files changed, 366 insertions(+), 322 deletions(-) delete mode 100644 pkg/kudoctl/packages/reader/reader.go create mode 100644 pkg/kudoctl/resources/dependencies/testdata/operator-with-dependencies.tgz diff --git a/pkg/controller/instance/instance_controller.go b/pkg/controller/instance/instance_controller.go index 9bbfa6be0..e07f4d306 100644 --- a/pkg/controller/instance/instance_controller.go +++ b/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"} } diff --git a/pkg/controller/instance/resolver_incluster.go b/pkg/controller/instance/resolver_incluster.go index 09c2ff666..15705b840 100644 --- a/pkg/controller/instance/resolver_incluster.go +++ b/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 } diff --git a/pkg/kudoctl/cmd/install/install.go b/pkg/kudoctl/cmd/install/install.go index 376a67ab9..9b7488253 100644 --- a/pkg/kudoctl/cmd/install/install.go +++ b/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,11 +82,16 @@ 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) @@ -92,7 +99,7 @@ func installOperator(operatorArgument string, options *Options, fs afero.Fs, set 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) diff --git a/pkg/kudoctl/cmd/package_list.go b/pkg/kudoctl/cmd/package_list.go index 1e7920198..b033ad8c9 100644 --- a/pkg/kudoctl/cmd/package_list.go +++ b/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 } diff --git a/pkg/kudoctl/cmd/upgrade.go b/pkg/kudoctl/cmd/upgrade.go index 796ecb271..7aa6fd740 100644 --- a/pkg/kudoctl/cmd/upgrade.go +++ b/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) } diff --git a/pkg/kudoctl/packages/reader/parser.go b/pkg/kudoctl/packages/reader/parser.go index b48ad88a4..6b2ae9736 100644 --- a/pkg/kudoctl/packages/reader/parser.go +++ b/pkg/kudoctl/packages/reader/parser.go @@ -98,7 +98,8 @@ func parsePackageFile(filePath string, fileBytes []byte, currentPackage *package } currentPackage.Params = ¶msFile 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 } diff --git a/pkg/kudoctl/packages/reader/parser_test.go b/pkg/kudoctl/packages/reader/parser_test.go index 0e00909f2..776441628 100644 --- a/pkg/kudoctl/packages/reader/parser_test.go +++ b/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")}, } diff --git a/pkg/kudoctl/packages/reader/reader.go b/pkg/kudoctl/packages/reader/reader.go deleted file mode 100644 index accf1f1e2..000000000 --- a/pkg/kudoctl/packages/reader/reader.go +++ /dev/null @@ -1,34 +0,0 @@ -package reader - -import ( - "fmt" - "strings" - - "github.com/spf13/afero" - - "github.com/kudobuilder/kudo/pkg/kudoctl/clog" - "github.com/kudobuilder/kudo/pkg/kudoctl/packages" -) - -// ReadPackage creates the implementation of the packages based on the path. The expectation is the packages -// is always local. The path can be relative or absolute location of the packages. Order of the discovery is: -// 1. tarball -// 2. dir based -func Read(fs afero.Fs, path string) (*packages.Resources, error) { - // make sure file exists - fi, err := fs.Stat(path) - if err != nil { - return nil, err - } - clog.V(0).Printf("determining package type of %v", path) - - if fi.Mode().IsRegular() && strings.HasSuffix(path, ".tgz") { - clog.V(0).Printf("%v is a tgz package", path) - return ReadTar(fs, path) - } - if fi.IsDir() { - clog.V(0).Printf("%v is a file package", path) - return ResourcesFromDir(fs, path) - } - return nil, fmt.Errorf("unsupported file system format %v. Expect either a *.tgz file or a folder", path) -} diff --git a/pkg/kudoctl/packages/reader/reader_tar.go b/pkg/kudoctl/packages/reader/reader_tar.go index 1276ed21f..9b5573de2 100644 --- a/pkg/kudoctl/packages/reader/reader_tar.go +++ b/pkg/kudoctl/packages/reader/reader_tar.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,7 +75,6 @@ func ParseTgz(r io.Reader) (*packages.Files, error) { tr := tar.NewReader(gzr) - result := newPackageFiles() for { header, err := tr.Next() @@ -60,11 +82,11 @@ func ParseTgz(r io.Reader) (*packages.Files, error) { // 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) } } } diff --git a/pkg/kudoctl/packages/reader/reader_test.go b/pkg/kudoctl/packages/reader/reader_test.go index d9bb4b8e8..22bd60b7e 100644 --- a/pkg/kudoctl/packages/reader/reader_test.go +++ b/pkg/kudoctl/packages/reader/reader_test.go @@ -34,18 +34,17 @@ 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) @@ -53,14 +52,14 @@ func TestReadFileSystemPackage(t *testing.T) { // 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) }) } } diff --git a/pkg/kudoctl/packages/resolver/resolver.go b/pkg/kudoctl/packages/resolver/resolver.go index 198643e14..4cfbef728 100644 --- a/pkg/kudoctl/packages/resolver/resolver.go +++ b/pkg/kudoctl/packages/resolver/resolver.go @@ -2,41 +2,46 @@ package resolver import ( "path/filepath" + "strings" + + "github.com/spf13/afero" "github.com/kudobuilder/kudo/pkg/kudoctl/clog" "github.com/kudobuilder/kudo/pkg/kudoctl/http" "github.com/kudobuilder/kudo/pkg/kudoctl/packages" - "github.com/kudobuilder/kudo/pkg/kudoctl/util/kudo" "github.com/kudobuilder/kudo/pkg/kudoctl/util/repo" ) -// Resolver will try to resolve a given package name to either local tarball, folder, remote url or -// an operator in the remote repository. -type Resolver interface { - Resolve(name string, appVersion string, operatorVersion string) (*packages.Resources, error) -} - // PackageResolver is the source of resolver of operator packages. type PackageResolver struct { - local *LocalResolver - uri *URLResolver + local *LocalHelper + uri *URLHelper repo *repo.Client } -// New creates an operator package resolver for non-repository packages -func New(repo *repo.Client) Resolver { - lf := NewLocal() - uf := NewURL() +// NewPackageResolver creates an operator package resolver for non-repository packages +func NewPackageResolver(repo *repo.Client, workingDir string) packages.Resolver { return &PackageResolver{ - local: lf, - uri: uf, + local: newForFilesystem(afero.NewOsFs(), workingDir), + uri: NewURLHelper(), repo: repo, } } -// NewInClusterResolver returns an initialized InClusterResolver for resolving already installed packages -func NewInClusterResolver(c *kudo.Client, ns string) Resolver { - return &InClusterResolver{c: c, ns: ns} +func (m *PackageResolver) copyWithChangedWorkingDir(workingDir string) packages.Resolver { + return &PackageResolver{ + local: newForFilesystem(m.local.fs, workingDir), + uri: m.uri, + repo: m.repo, + } +} + +func (m *PackageResolver) copyWithChangedFs(fs afero.Fs) packages.Resolver { + return &PackageResolver{ + local: newForFilesystem(fs, "/"), + uri: m.uri, + repo: m.repo, + } } // Resolve provides a one stop to acquire any non-repo packages by trying to look for package files @@ -49,32 +54,72 @@ func NewInClusterResolver(c *kudo.Client, ns string) Resolver { // For local access there is a need to provide absolute or relative path as part of the name argument. `cassandra` without a path // component will resolve to the remote repo. `./cassandra` will resolve to a folder which is expected to have the operator structure on the filesystem. // `../folder/cassandra.tgz` will resolve to the cassandra package tarball on the filesystem. -func (m *PackageResolver) Resolve(name string, appVersion string, operatorVersion string) (p *packages.Resources, err error) { - - // Local files/folder have priority - _, err = m.local.fs.Stat(name) - // force local operators usage to be either absolute or express a relative path - // or put another way, a name can NOT be mistaken to be the name of a local folder - if filepath.Base(name) != name && err == nil { - var abs string - abs, err = filepath.Abs(name) - if err != nil { - return nil, err - } +func (m *PackageResolver) Resolve(name string, appVersion string, operatorVersion string) (*packages.PackageScope, error) { + + // if the path to a child dependency is a relative one, we construct the absolute path for the + // resolver by combining the absolute path for the operator directory with the relative dependency path + // a relative dependency path must begin with either './' or '../' + isRelativePackage := strings.HasPrefix(name, "./") || strings.HasPrefix(name, "../") + if isRelativePackage { + name = filepath.Join(m.local.directory, name) + } + + clog.V(2).Printf("determining package type of %v", name) + + // 1. local files/folder have priority + abs, err := m.local.LocalPackagePath(name) + + // LocalPackagePath returns an error if name isn't a local file/folder and does not indicate other errors + if err == nil { clog.V(2).Printf("local operator discovered: %v", abs) - p, err = m.local.Resolve(name, appVersion, operatorVersion) - return + + var res *packages.Resources + if strings.HasSuffix(abs, ".tgz") { + out := afero.NewMemMapFs() + res, err = m.local.ResolveTar(out, abs) + if err == nil { + return &packages.PackageScope{ + Resources: res, + DependenciesResolver: m.copyWithChangedFs(out), + }, nil + } + } else { + res, err = m.local.ResolveDir(abs) + if err == nil { + return &packages.PackageScope{ + Resources: res, + DependenciesResolver: m.copyWithChangedWorkingDir(abs), + }, nil + } + } + + return nil, err } + // 2. next are tarball URLs clog.V(3).Printf("no local operator discovered, looking for http") if http.IsValidURL(name) { clog.V(3).Printf("operator using http protocol for %v", name) - p, err = m.uri.Resolve(name, appVersion, operatorVersion) - return + out := afero.NewMemMapFs() + res, err := m.uri.ResolveURL(out, name) + if err == nil { + return &packages.PackageScope{ + Resources: res, + DependenciesResolver: m.copyWithChangedFs(out), + }, nil + } + return nil, err } + // 3. try the repo as the last clog.V(3).Printf("no http discovered, looking for repository") - p, err = m.repo.Resolve(name, appVersion, operatorVersion) - - return + out := afero.NewMemMapFs() + res, err := m.repo.Resolve(out, name, appVersion, operatorVersion) + if err == nil { + return &packages.PackageScope{ + Resources: res, + DependenciesResolver: m.copyWithChangedFs(out), + }, nil + } + return nil, err } diff --git a/pkg/kudoctl/packages/resolver/resolver_incluster.go b/pkg/kudoctl/packages/resolver/resolver_incluster.go index e36f51948..7cfa42bc4 100644 --- a/pkg/kudoctl/packages/resolver/resolver_incluster.go +++ b/pkg/kudoctl/packages/resolver/resolver_incluster.go @@ -15,7 +15,12 @@ type InClusterResolver struct { ns string } -func (r InClusterResolver) Resolve(name string, appVersion string, operatorVersion string) (*packages.Resources, error) { +// NewInClusterResolver returns an initialized InClusterResolver for resolving already installed packages +func NewInClusterResolver(c *kudo.Client, ns string) packages.Resolver { + return &InClusterResolver{c: c, ns: ns} +} + +func (r InClusterResolver) Resolve(name string, appVersion string, operatorVersion string) (*packages.PackageScope, error) { // Fetch all OVs ovList, err := r.c.ListOperatorVersions(r.ns) if err != nil { @@ -47,9 +52,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: convert.BuildInstanceResource(name, operatorVersion, appVersion), - }, nil + } + + return &packages.PackageScope{Resources: res, DependenciesResolver: r}, nil } diff --git a/pkg/kudoctl/packages/resolver/resolver_incluster_test.go b/pkg/kudoctl/packages/resolver/resolver_incluster_test.go index 491d7c8c0..34b680969 100644 --- a/pkg/kudoctl/packages/resolver/resolver_incluster_test.go +++ b/pkg/kudoctl/packages/resolver/resolver_incluster_test.go @@ -163,7 +163,7 @@ func TestInClusterResolver_Resolve(t *testing.T) { return } - assert.Equal(t, got.OperatorVersion.Spec.Version, tt.wantOperatorVersion, "got: %v", got.OperatorVersion) + assert.Equal(t, got.Resources.OperatorVersion.Spec.Version, tt.wantOperatorVersion, "got: %v", got.Resources.OperatorVersion) } }) } diff --git a/pkg/kudoctl/packages/resolver/resolver_local.go b/pkg/kudoctl/packages/resolver/resolver_local.go index 721ac3c94..99e2b395e 100644 --- a/pkg/kudoctl/packages/resolver/resolver_local.go +++ b/pkg/kudoctl/packages/resolver/resolver_local.go @@ -2,50 +2,62 @@ package resolver import ( "fmt" + "path/filepath" "strings" "github.com/spf13/afero" - "github.com/kudobuilder/kudo/pkg/kudoctl/clog" "github.com/kudobuilder/kudo/pkg/kudoctl/packages" "github.com/kudobuilder/kudo/pkg/kudoctl/packages/reader" ) -// LocalResolver will find local operator packages: folders or tgz -type LocalResolver struct { - fs afero.Fs +// LocalHelper will find local operator packages: folders or tgz +type LocalHelper struct { + fs afero.Fs + directory string } -// Resolve a local package. The path can be relative or absolute location of the packages. -// Order of the discovery is: -// 1. tarball -// 2. dir based -func (f *LocalResolver) Resolve(name string, appVersion string, operatorVersion string) (*packages.Resources, error) { - // make sure file exists - _, err := f.fs.Stat(name) - if err != nil { - return nil, err +// newForFilesystem creates a resolver with an fs and a working directory +func newForFilesystem(fs afero.Fs, dir string) *LocalHelper { + return &LocalHelper{ + fs: fs, + directory: dir, } +} - // make sure file exists - fi, err := f.fs.Stat(name) - if err != nil { - return nil, err +func (f *LocalHelper) LocalPackagePath(path string) (string, error) { + fi, err := f.fs.Stat(path) + + // force local operators usage to be either absolute or express a relative path + // or put another way, a name can NOT be mistaken to be the name of a local folder + if filepath.Base(path) != path && err == nil { + var abs string + abs, err = filepath.Abs(path) + if err != nil { + return "", err + } + + // expecting either a folder or .tgz file + if fi.IsDir() || (fi.Mode().IsRegular() && strings.HasSuffix(abs, ".tgz")) { + return abs, nil + } } - clog.V(1).Printf("determining package type of %v", name) + return "", fmt.Errorf("%s is not a valid local package path", path) +} - if fi.Mode().IsRegular() && strings.HasSuffix(name, ".tgz") { - clog.V(1).Printf("%v is a local tgz package", name) - return reader.ReadTar(f.fs, name) - } - if fi.IsDir() { - clog.V(1).Printf("%v is a local file package", name) - return reader.ResourcesFromDir(f.fs, name) +func (f *LocalHelper) ResolveTar(out afero.Fs, path string) (*packages.Resources, error) { + abs, err := f.LocalPackagePath(path) + if strings.HasSuffix(abs, ".tgz") && err == nil { + return reader.ResourcesFromTar(f.fs, out, path) } - return nil, fmt.Errorf("unsupported file system format %v. Expect either a *.tgz file or a folder", name) + + return nil, fmt.Errorf("unsupported file system format %v. Expect a *.tgz file", abs) } -// NewLocal creates a resolver for local operator package -func NewLocal() *LocalResolver { - return &LocalResolver{fs: afero.NewOsFs()} +func (f *LocalHelper) ResolveDir(path string) (*packages.Resources, error) { + abs, err := f.LocalPackagePath(path) + if err == nil { + return reader.ResourcesFromDir(f.fs, path) + } + return nil, fmt.Errorf("unsupported file system format %v. Expect a folder", abs) } diff --git a/pkg/kudoctl/packages/resolver/resolver_local_test.go b/pkg/kudoctl/packages/resolver/resolver_local_test.go index b44bd844e..cb39d0ec6 100644 --- a/pkg/kudoctl/packages/resolver/resolver_local_test.go +++ b/pkg/kudoctl/packages/resolver/resolver_local_test.go @@ -1,14 +1,17 @@ package resolver import ( + "os" "testing" + "github.com/spf13/afero" "github.com/stretchr/testify/assert" ) func TestLocalResolver_GetPackage(t *testing.T) { - f := NewLocal() - pkg, err := f.Resolve("../testdata/zk", "", "") + wd, _ := os.Getwd() + f := newForFilesystem(afero.NewOsFs(), wd) + pkg, err := f.ResolveDir("../testdata/zk") if err != nil { t.Errorf("PackageResolver.Resolve() error = %v", err) return @@ -18,7 +21,8 @@ func TestLocalResolver_GetPackage(t *testing.T) { } func TestLocalFinder_Failure(t *testing.T) { - f := NewLocal() - _, err := f.Resolve("../testdata/zk-bad", "", "") + wd, _ := os.Getwd() + f := newForFilesystem(afero.NewOsFs(), wd) + _, err := f.ResolveDir("../testdata/zk-bad") assert.Errorf(t, err, "should have errored on bad folder name") } diff --git a/pkg/kudoctl/packages/resolver/resolver_test.go b/pkg/kudoctl/packages/resolver/resolver_test.go index 6ace3fffd..6d13a481f 100644 --- a/pkg/kudoctl/packages/resolver/resolver_test.go +++ b/pkg/kudoctl/packages/resolver/resolver_test.go @@ -1,14 +1,18 @@ package resolver import ( + "os" "testing" + "github.com/spf13/afero" "github.com/stretchr/testify/assert" ) func TestManager_GetPackage(t *testing.T) { + wd, _ := os.Getwd() + m := &PackageResolver{ - local: NewLocal(), + local: newForFilesystem(afero.NewOsFs(), wd), uri: nil, } pr, err := m.Resolve("../testdata/zk", "", "") @@ -17,5 +21,5 @@ func TestManager_GetPackage(t *testing.T) { return } - assert.EqualValues(t, "zookeeper", pr.Operator.Name) + assert.EqualValues(t, "zookeeper", pr.Resources.Operator.Name) } diff --git a/pkg/kudoctl/packages/resolver/resolver_url.go b/pkg/kudoctl/packages/resolver/resolver_url.go index 4f4612fcc..70a6d0544 100644 --- a/pkg/kudoctl/packages/resolver/resolver_url.go +++ b/pkg/kudoctl/packages/resolver/resolver_url.go @@ -4,6 +4,8 @@ import ( "bytes" "fmt" + "github.com/spf13/afero" + "github.com/kudobuilder/kudo/pkg/kudoctl/clog" "github.com/kudobuilder/kudo/pkg/kudoctl/http" "github.com/kudobuilder/kudo/pkg/kudoctl/packages" @@ -11,22 +13,22 @@ import ( "github.com/kudobuilder/kudo/pkg/kudoctl/packages/reader" ) -// URLResolver will resolve a packages from a url -type URLResolver struct { +// URLHelper will resolve a packages from a url +type URLHelper struct { client http.Client } -// Resolve returns a package for the provided url -func (f *URLResolver) Resolve(name string, appVersion string, operatorVersion string) (*packages.Resources, error) { - // check to see if name is url - if !http.IsValidURL(name) { - return nil, fmt.Errorf("resolver: url %v invalid", name) +// ResolveURL returns a package for the provided url +func (f *URLHelper) ResolveURL(out afero.Fs, url string) (*packages.Resources, error) { + // check to see if url is url + if !http.IsValidURL(url) { + return nil, fmt.Errorf("resolver: url %v invalid", url) } - buf, err := f.getPackageByURL(name) + buf, err := f.getPackageByURL(url) if err != nil { return nil, err } - files, err := reader.ParseTgz(buf) + files, err := reader.PackageFilesFromTar(out, buf) if err != nil { return nil, err } @@ -36,12 +38,12 @@ func (f *URLResolver) Resolve(name string, appVersion string, operatorVersion st return nil, err } - clog.V(0).Printf("%v is a remote tgz package", name) + clog.V(0).Printf("%v is a remote .tgz package", url) return resources, nil } -func (f *URLResolver) getPackageByURL(url string) (*bytes.Buffer, error) { +func (f *URLHelper) getPackageByURL(url string) (*bytes.Buffer, error) { resp, err := f.client.Get(url) if err != nil { return nil, fmt.Errorf("resolver: unable to get get reader from url %v", url) @@ -50,11 +52,11 @@ func (f *URLResolver) getPackageByURL(url string) (*bytes.Buffer, error) { return resp, nil } -// NewURL creates an instance of a URLResolver -func NewURL() *URLResolver { +// NewURLHelper creates an instance of a URLHelper +func NewURLHelper() *URLHelper { client := http.NewClient() - return &URLResolver{ + return &URLHelper{ client: *client, } } diff --git a/pkg/kudoctl/packages/types.go b/pkg/kudoctl/packages/types.go index f2a04da21..158c55d5d 100644 --- a/pkg/kudoctl/packages/types.go +++ b/pkg/kudoctl/packages/types.go @@ -10,6 +10,21 @@ const ( APIVersion = "kudo.dev/v1beta1" ) +// Resolver will try to resolve a given package name to either local tarball, folder, remote url or +// an operator in the remote repository. +type Resolver interface { + Resolve(name string, appVersion string, operatorVersion string) (*PackageScope, error) +} + +// PackageScope type provides operator resources together with a "scope-aware" dependency resolver. +// For example, a dependency like `./child-operator` in an local directory needs the host system to be resolved, but when +// found in a tarball, it needs the contents of the tarball because the dependency is not allowed to "escape" into the +// host system. +type PackageScope struct { + Resources *Resources + DependenciesResolver Resolver +} + // Resources is collection of CRDs that are used when installing operator // during installation, package format is converted to this structure type Resources struct { diff --git a/pkg/kudoctl/resources/dependencies/resolve.go b/pkg/kudoctl/resources/dependencies/resolve.go index 4278b6001..a80589cff 100644 --- a/pkg/kudoctl/resources/dependencies/resolve.go +++ b/pkg/kudoctl/resources/dependencies/resolve.go @@ -2,10 +2,7 @@ package dependencies import ( "fmt" - "path/filepath" - "strings" - "github.com/spf13/afero" "github.com/thoas/go-funk" "github.com/yourbasic/graph" @@ -13,7 +10,6 @@ import ( engtask "github.com/kudobuilder/kudo/pkg/engine/task" "github.com/kudobuilder/kudo/pkg/kudoctl/clog" "github.com/kudobuilder/kudo/pkg/kudoctl/packages" - pkgresolver "github.com/kudobuilder/kudo/pkg/kudoctl/packages/resolver" ) // dependencyGraph is modeled after 'graph.Mutable' but allows to add vertices. @@ -48,8 +44,6 @@ func (g *dependencyGraph) Visit(v int, do func(w int, c int64) bool) bool { type Dependency struct { packages.Resources - - PackageName string } // Resolve resolves all dependencies of an OperatorVersion. Dependencies are resolved recursively. @@ -58,7 +52,7 @@ type Dependency struct { // operator in a directory. In that case all its relative dependencies (if any exist) will be relative // to the operator directory (the one with `operator.yaml` file). // See github.com/kudobuilder/kudo/issues/1701 for additional context. -func Resolve(operatorArgument string, operatorVersion *kudoapi.OperatorVersion, resolver pkgresolver.Resolver) ([]Dependency, error) { +func Resolve(operatorVersion *kudoapi.OperatorVersion, resolver packages.Resolver) ([]Dependency, error) { root := packages.Resources{ OperatorVersion: operatorVersion, } @@ -72,10 +66,7 @@ func Resolve(operatorArgument string, operatorVersion *kudoapi.OperatorVersion, edges: []map[int]struct{}{{}}, } - // Here we only care whether the path is absolute or not so we can ignore the error - operatorDir, _ := operatorAbsPath(operatorArgument) - - if err := dependencyWalk(&dependencies, &g, root, 0, resolver, operatorDir); err != nil { + if err := dependencyWalk(&dependencies, &g, &root, 0, resolver); err != nil { return nil, err } @@ -86,10 +77,9 @@ func Resolve(operatorArgument string, operatorVersion *kudoapi.OperatorVersion, func dependencyWalk( dependencies *[]Dependency, g *dependencyGraph, - parent packages.Resources, + parent *packages.Resources, parentIndex int, - resolver pkgresolver.Resolver, - operatorDirectory *string) error { + resolver packages.Resolver) error { //nolint:errcheck childrenTasks := funk.Filter(parent.OperatorVersion.Spec.Tasks, func(task kudoapi.Task) bool { return task.Kind == engtask.KudoOperatorTaskKind @@ -98,23 +88,7 @@ func dependencyWalk( for _, childTask := range childrenTasks { childPackageName := childTask.Spec.KudoOperatorTaskSpec.Package - // if the path to a child dependency is a relative one, we construct the absolute path for the - // resolver by combining the absolute path for the operator directory with the relative dependency path - // a relative dependency path must begin with either './' or '../' - isRelativePackage := strings.HasPrefix(childPackageName, "./") || strings.HasPrefix(childPackageName, "../") - if isRelativePackage { - if operatorDirectory == nil { - return fmt.Errorf("a dependency with a relative path %q is only allowed in a parent %v when it is a local directory", childPackageName, parent.OperatorVersion.FullyQualifiedName()) - } - childDirectory, err := operatorAbsPath(filepath.Join(*operatorDirectory, childPackageName)) - if err != nil { - return fmt.Errorf("local dependency %s of the parent %s has an invalid path: %v", fullyQualifiedName(childTask.Spec.KudoOperatorTaskSpec), parent.OperatorVersion.FullyQualifiedName(), err) - } - childPackageName = *childDirectory - - } - - childRes, err := resolver.Resolve( + childResolved, err := resolver.Resolve( childPackageName, childTask.Spec.KudoOperatorTaskSpec.AppVersion, childTask.Spec.KudoOperatorTaskSpec.OperatorVersion) @@ -123,15 +97,42 @@ func dependencyWalk( "failed to resolve package %s, dependency of package %s: %v", fullyQualifiedName(childTask.Spec.KudoOperatorTaskSpec), parent.OperatorVersion.FullyQualifiedName(), err) } + // after resolving the dependency we update the KudoOperatorTask.Spec definition with the resolved + // operator name, version and app version so that a definition like: + // --- + // - name: deploy-child + // kind: KudoOperator + // spec: + // package: "../child-operator" + // + // will be updated as: + // --- + // - name: deploy-child + // kind: KudoOperator + // spec: + // appVersion: 3.2.1 + // operatorVersion: 0.0.1 + // package: child + // + // so that the KUDO controller to be able to grab the right 'OperatorVersion' resources from the cluster + // when the task is executed. + err = updateResolvedKudoOperatorTask(childTask.Name, + parent.OperatorVersion, + childResolved.Resources.Operator.Name, + childResolved.Resources.OperatorVersion.Spec.Version, + childResolved.Resources.OperatorVersion.Spec.AppVersion) + if err != nil { + return err + } + childDependency := Dependency{ - Resources: *childRes, - PackageName: childTask.Spec.KudoOperatorTaskSpec.Package, + Resources: *childResolved.Resources, } newPackage := false childIndex := indexOf(dependencies, &childDependency) if childIndex == -1 { - clog.V(2).Printf("Adding new dependency %s", childRes.OperatorVersion.FullyQualifiedName()) + clog.V(2).Printf("Adding new dependency %s", childResolved.Resources.OperatorVersion.FullyQualifiedName()) newPackage = true *dependencies = append(*dependencies, childDependency) @@ -146,16 +147,12 @@ func dependencyWalk( if !graph.Acyclic(g) { return fmt.Errorf( - "cyclic package dependency found when adding package %s -> %s", parent.OperatorVersion.FullyQualifiedName(), childRes.OperatorVersion.FullyQualifiedName()) + "cyclic package dependency found when adding package %s -> %s", parent.OperatorVersion.FullyQualifiedName(), childResolved.Resources.OperatorVersion.FullyQualifiedName()) } // We only need to walk the dependencies if the package is new if newPackage { - var childOperatorDirectory *string - if isRelativePackage { - childOperatorDirectory = &childPackageName - } - if err := dependencyWalk(dependencies, g, *childRes, childIndex, resolver, childOperatorDirectory); err != nil { + if err := dependencyWalk(dependencies, g, childResolved.Resources, childIndex, childResolved.DependenciesResolver); err != nil { return err } } @@ -164,6 +161,21 @@ func dependencyWalk( return nil } +// updateResolvedKudoOperatorTask method updates all 'KudoOperatorTasks' of an OperatorVersion by setting their 'Package' and +// 'OperatorVersion' fields to the already resolved packages. This is done for the KUDO controller to be able to grab +// the right 'OperatorVersion' resources from the cluster when the corresponding task is executed. +func updateResolvedKudoOperatorTask(taskName string, parent *kudoapi.OperatorVersion, operatorName, operatorVersion, appVersion string) error { + for i, tt := range parent.Spec.Tasks { + if tt.Name == taskName { + parent.Spec.Tasks[i].Spec.KudoOperatorTaskSpec.Package = operatorName + parent.Spec.Tasks[i].Spec.KudoOperatorTaskSpec.OperatorVersion = operatorVersion + parent.Spec.Tasks[i].Spec.KudoOperatorTaskSpec.AppVersion = appVersion + return nil + } + } + return fmt.Errorf("failed to update resolved task %s of the operator %s with resolved data", taskName, parent.FullyQualifiedName()) +} + // indexOf method searches for the dependency in dependencies that has the same // OperatorVersion/AppVersion (using EqualOperatorVersion method) and returns // its index or -1 if not found. @@ -189,19 +201,3 @@ func fullyQualifiedName(kt kudoapi.KudoOperatorTaskSpec) string { return fmt.Sprintf("Operator: %q, OperatorVersion: %q, AppVersion %q", kt.Package, operatorVersion, appVersion) } - -func operatorAbsPath(path string) (*string, error) { - fs := afero.NewOsFs() - - ok, err := afero.IsDir(fs, path) - // force local operators usage to be either absolute or express a relative path - // or put another way, a name can NOT be mistaken to be the name of a local folder - if ok && filepath.Base(path) != path { - abs, err := filepath.Abs(path) - if err == nil { - return &abs, nil - } - return nil, fmt.Errorf("failed to detect an absolute path for %q: %v", path, err) - } - return nil, fmt.Errorf("%q is not a valid directory: %v", path, err) -} diff --git a/pkg/kudoctl/resources/dependencies/resolve_test.go b/pkg/kudoctl/resources/dependencies/resolve_test.go index a5a066cb0..d4417d729 100644 --- a/pkg/kudoctl/resources/dependencies/resolve_test.go +++ b/pkg/kudoctl/resources/dependencies/resolve_test.go @@ -2,6 +2,7 @@ package dependencies import ( "fmt" + "os" "testing" "github.com/stretchr/testify/assert" @@ -22,12 +23,13 @@ type nameResolver struct { func (resolver nameResolver) Resolve( name string, appVersion string, - operatorVersion string) (*packages.Resources, error) { + operatorVersion string) (*packages.PackageScope, error) { for _, pr := range resolver.Prs { + pr := pr if pr.Operator.Name == name && (operatorVersion == "" || pr.OperatorVersionString() == operatorVersion) && (appVersion == "" || pr.AppVersionString() == appVersion) { - return &pr, nil + return &packages.PackageScope{Resources: &pr, DependenciesResolver: resolver}, nil } } @@ -222,8 +224,7 @@ func TestResolve(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { resolver := nameResolver{tt.prs} - operatorArg := tt.prs[0].OperatorVersion.Name - got, err := Resolve(operatorArg, tt.prs[0].OperatorVersion, resolver) + got, err := Resolve(tt.prs[0].OperatorVersion, resolver) assert.Equal(t, err == nil, tt.wantErr == "") @@ -242,64 +243,40 @@ func TestResolve(t *testing.T) { } } -func Test_isLocalDirPackage(t *testing.T) { +func TestResolveLocalDependencies(t *testing.T) { tests := []struct { - name string - path string - wantNilPath bool - wantErr bool + path string }{ - { - name: "./", - path: "./", - wantNilPath: false, - wantErr: false, - }, - { - name: "../install", - path: "../install", - wantNilPath: false, - wantErr: false, - }, - { - name: "./some-fake-path", - path: "./some-fake-path", - wantNilPath: true, - wantErr: true, - }, - { - name: "./", - path: "./resolve_test.go", - wantNilPath: true, - wantErr: true, - }, + {path: "./testdata/operator-with-dependencies/parent-operator"}, + {path: "./testdata/operator-with-dependencies.tgz"}, } + + wd, _ := os.Getwd() + + var resolver = pkgresolver.NewPackageResolver(nil, wd) + for _, tt := range tests { tt := tt - t.Run(tt.name, func(t *testing.T) { - absPath, err := operatorAbsPath(tt.path) - assert.Equal(t, tt.wantErr, err != nil) - assert.Equal(t, tt.wantNilPath, absPath == nil) - }) - } -} - -func TestResolveLocalDependencies(t *testing.T) { - var resolver = pkgresolver.NewLocal() - var operatorArgument = "./testdata/operator-with-dependencies/parent-operator" + t.Run(tt.path, func(t *testing.T) { + pr, err := resolver.Resolve(tt.path, "", "") + assert.NoError(t, err, "failed to resolve operator package for %s", tt.path) + assert.NotNil(t, pr, "failed to resolve operator %s", tt.path) - pr, err := resolver.Resolve(operatorArgument, "", "") - assert.NoError(t, err, "failed to resolve operator package for %s", operatorArgument) + dependencies, err := Resolve(pr.Resources.OperatorVersion, pr.DependenciesResolver) + assert.Equal(t, "child", pr.Resources.OperatorVersion.Spec.Tasks[0].Spec.KudoOperatorTaskSpec.Package, "expecting the KudoOperatorTask to have resolved child operator name") + assert.Equal(t, "0.0.1", pr.Resources.OperatorVersion.Spec.Tasks[0].Spec.KudoOperatorTaskSpec.OperatorVersion, "expecting the KudoOperatorTask to have resolved child operator version") + assert.Equal(t, "3.2.1", pr.Resources.OperatorVersion.Spec.Tasks[0].Spec.KudoOperatorTaskSpec.AppVersion, "expecting the KudoOperatorTask to have resolved child operator app version") - dependencies, err := Resolve(operatorArgument, pr.OperatorVersion, resolver) - assert.NoError(t, err, "failed to resolve dependencies for %s", operatorArgument) - assert.Equal(t, len(dependencies), 1, "expecting to find child-operator dependency") + assert.NoError(t, err, "failed to resolve dependencies for %s", tt.path) + assert.Equal(t, len(dependencies), 1, "expecting to find child-operator dependency") - assert.NotNil(t, dependencies[0].Operator, "expecting to find child-operator OperatorVersion resource") - assert.NotNil(t, dependencies[0].OperatorVersion, "expecting to find child-operator OperatorVersion resource") - assert.NotNil(t, dependencies[0].Instance, "expecting to find child-operator OperatorVersion resource") + assert.NotNil(t, dependencies[0].Operator, "expecting to find child-operator OperatorVersion resource") + assert.NotNil(t, dependencies[0].OperatorVersion, "expecting to find child-operator OperatorVersion resource") + assert.NotNil(t, dependencies[0].Instance, "expecting to find child-operator OperatorVersion resource") - assert.Equal(t, dependencies[0].Operator.Name, "child") - assert.Equal(t, dependencies[0].OperatorVersion.Name, kudoapi.OperatorVersionName("child", "", "0.0.1")) - assert.Equal(t, dependencies[0].Instance.Name, kudoapi.OperatorInstanceName("child")) + assert.Equal(t, dependencies[0].Operator.Name, "child") + assert.Equal(t, dependencies[0].OperatorVersion.Name, kudoapi.OperatorVersionName("child", "3.2.1", "0.0.1")) + assert.Equal(t, dependencies[0].Instance.Name, kudoapi.OperatorInstanceName("child")) + }) + } } diff --git a/pkg/kudoctl/resources/dependencies/testdata/operator-with-dependencies.tgz b/pkg/kudoctl/resources/dependencies/testdata/operator-with-dependencies.tgz new file mode 100644 index 0000000000000000000000000000000000000000..d23ccdf81268089155fe014f7e9193b2e6773b59 GIT binary patch literal 1029 zcmV+g1p50QiwFQ3N0eUx1MQnjkLorQ$DQdcMyXV>?9yV7@gomFZH2DpN|m~3rK!4Y zJ;VbMA&C-adg*Fb{apQMbrJ`1fq76(nBKuZDf%alZEVc3&%p^Ag3iGh`>qQs^=ie` zsZ|3B^BosEjQT#rgt{&R(Cxly02I08nQ96Tisd4?kHhuhd@9@B-4pfber)s$0S$lQ zpMDfP5ZMr2-~4_flT5twXQ=PPbDSr}pShdyXUtRM?@>$u{PfE0ILMigKOMuB3|HcN z;;CV9>3ED08IMN9bD90tjWcu(>dfl5JZ=BP!Pwcce}^(<|G2XjwK#M8S6Cm4fB?`_ zr&pj>@b}@YSNhoFqN%GsR2=~rfCQvKfb6C1P{%i51{NR$(P!m3km@`O@>z77B+(Il zGZ+j8gTe4PRxMXwzil%N2E+f5a(;kXr&=Fswe^nmd;FgaX#8(57~V0~<@L?= z2e1hU_9)j{sE$8$o`0Hxk2nGs^&f`cf^9yib*lBDR^tp93KZ;$SRnLETp6{Cd|5L#5cK@IHKJ}T49FL4gV}*GD;N62dv-&O1*+0d* z_V4(Vnf?D$z|-(Ik>z0$PvEi$5)_F0;XSz(k`uf7X$YzAPizPWFjiL+7>G1V)+zz2 z(|neNsjQ0snuYVZ$_wQ}=u9w;c&u~Y@gkBFNUBR&=WQx`>u=$FX|MlDDl#sU?6v2D zm#+WS^FM?s`}eEof6V&dJ?-_sdj8KR$YE*%_6T#$`v2}homu^s=j`7nWXJwp>S43~ zpModX|9ZXeD>p#IvTrxPludy?Mg-x$y)14;7Ar>~-`paIxCq-T9>!7~J1)duA|3?E zymAGE$`eiCck86crqu9SOxP}f|S$)&n2=IaT}MHcd?N=z3# z7kN`@)1b1;I;mZ*^rfOmM6FvQ-4t$D(DJVuHoa~$_^j{$ar}3`fG%DK!7qRRgDT4} z?cedt`yVG^Z~Qw52ak{ce2@8N{yzmK{yT5>?7xF>@QC=Yp8qLT9-xW;PQh;c=Z;6n zXU=%!QbtYu_wL!V{|>^z=i|R!`!8Jrv;Logm&AY7FXV9ervkvf&C_(7#t<#j-aaI3 z1%ubcYPH^r0)r%0nPpZ8o$x1*<;P4pC3if^#TOU}n)%vbFdV_ZYWlay07L))pM@NG literal 0 HcmV?d00001 diff --git a/pkg/kudoctl/resources/dependencies/testdata/operator-with-dependencies/child-operator/operator.yaml b/pkg/kudoctl/resources/dependencies/testdata/operator-with-dependencies/child-operator/operator.yaml index 3854b4260..065d2720f 100644 --- a/pkg/kudoctl/resources/dependencies/testdata/operator-with-dependencies/child-operator/operator.yaml +++ b/pkg/kudoctl/resources/dependencies/testdata/operator-with-dependencies/child-operator/operator.yaml @@ -1,6 +1,7 @@ apiVersion: kudo.dev/v1beta1 name: "child" operatorVersion: "0.0.1" +appVersion: "3.2.1" kubernetesVersion: 1.15.0 maintainers: - name: zen-dog diff --git a/pkg/kudoctl/resources/dependencies/testdata/operator-with-dependencies/parent-operator/operator.yaml b/pkg/kudoctl/resources/dependencies/testdata/operator-with-dependencies/parent-operator/operator.yaml index 3632f6818..480baca24 100644 --- a/pkg/kudoctl/resources/dependencies/testdata/operator-with-dependencies/parent-operator/operator.yaml +++ b/pkg/kudoctl/resources/dependencies/testdata/operator-with-dependencies/parent-operator/operator.yaml @@ -11,7 +11,6 @@ tasks: kind: KudoOperator spec: package: "../child-operator" - operatorVersion: 0.0.1 plans: deploy: diff --git a/pkg/kudoctl/resources/install/operator.go b/pkg/kudoctl/resources/install/operator.go index d6f6cfbc6..07546d6b0 100644 --- a/pkg/kudoctl/resources/install/operator.go +++ b/pkg/kudoctl/resources/install/operator.go @@ -6,7 +6,6 @@ import ( "github.com/thoas/go-funk" kudoapi "github.com/kudobuilder/kudo/pkg/apis/kudo/v1beta1" - engtask "github.com/kudobuilder/kudo/pkg/engine/task" "github.com/kudobuilder/kudo/pkg/kudoctl/clog" deps "github.com/kudobuilder/kudo/pkg/kudoctl/resources/dependencies" "github.com/kudobuilder/kudo/pkg/kudoctl/util/kudo" @@ -67,18 +66,6 @@ func OperatorAndOperatorVersion( } func installDependencies(client *kudo.Client, ov *kudoapi.OperatorVersion, dependencies []deps.Dependency) error { - // The KUDO controller will create Instances for the dependencies. For this - // it needs to resolve the dependencies again from 'KudoOperatorTaskSpec'. - // But it cannot resolve packages like the CLI, because it may - // not have access to the referenced local files or URLs. - // It can however resolve the OperatorVersion from the name of the operator - // dependency. For this, we overwrite the 'Package' field describing - // dependencies in 'KudoOperatorTaskSpec' with the operator name of the - // dependency and the 'OperatorVersion' with the corresponding version. - // This has to be done for the operator to upgrade as well as in all of its new dependencies. - - updateKudoOperatorTasks(dependencies, ov) - for _, dependency := range dependencies { dependency.Operator.SetNamespace(ov.Namespace) dependency.OperatorVersion.SetNamespace(ov.Namespace) @@ -104,8 +91,6 @@ func installDependencies(client *kudo.Client, ov *kudoapi.OperatorVersion, depen } if !funk.ContainsString(installed, dependency.OperatorVersion.Spec.Version) { - updateKudoOperatorTasks(dependencies, dependency.OperatorVersion) - if _, err := client.InstallOperatorVersionObjToCluster( dependency.OperatorVersion, dependency.OperatorVersion.Namespace); err != nil { @@ -124,26 +109,3 @@ func installDependencies(client *kudo.Client, ov *kudoapi.OperatorVersion, depen return nil } - -// updateKudoOperatorTasks method updates all 'KudoOperatorTasks' of an OperatorVersion by setting their 'Package' and -// 'OperatorVersion' fields to the already resolved packages. This is done for the KUDO controller to be able to grab -// the right 'OperatorVersion' resources from the cluster when the corresponding task is executed. -func updateKudoOperatorTasks( - dependencies []deps.Dependency, operatorVersion *kudoapi.OperatorVersion) { - tasks := operatorVersion.Spec.Tasks - - for i := range tasks { - if tasks[i].Kind == engtask.KudoOperatorTaskKind { - for _, dependency := range dependencies { - if tasks[i].Spec.KudoOperatorTaskSpec.Package == dependency.PackageName { - tasks[i].Spec.KudoOperatorTaskSpec.Package = dependency.Operator.Name - tasks[i].Spec.KudoOperatorTaskSpec.OperatorVersion = dependency.OperatorVersion.Spec.Version - tasks[i].Spec.KudoOperatorTaskSpec.AppVersion = dependency.OperatorVersion.Spec.AppVersion - break - } - } - } - } - - operatorVersion.Spec.Tasks = tasks -} diff --git a/pkg/kudoctl/resources/upgrade/operatorversion_test.go b/pkg/kudoctl/resources/upgrade/operatorversion_test.go index 06d55e828..052c445fd 100644 --- a/pkg/kudoctl/resources/upgrade/operatorversion_test.go +++ b/pkg/kudoctl/resources/upgrade/operatorversion_test.go @@ -181,7 +181,6 @@ func Test_UpgradeOperatorVersionWithDependency(t *testing.T) { }, }, }, - PackageName: "dependency", } c := kudo.NewClientFromK8s(fake.NewSimpleClientset(), kubefake.NewSimpleClientset()) diff --git a/pkg/kudoctl/util/repo/digest.go b/pkg/kudoctl/util/repo/digest.go index b7ecca489..39da8ffcd 100644 --- a/pkg/kudoctl/util/repo/digest.go +++ b/pkg/kudoctl/util/repo/digest.go @@ -65,7 +65,7 @@ func pathToOperator(fs afero.Fs, path string) (pfd *PackageFilesDigest, err erro return nil, err } - files, err := reader.ParseTgz(bytes.NewBuffer(b)) + files, err := reader.PackageFilesFromTar(afero.NewMemMapFs(), bytes.NewBuffer(b)) pfd = &PackageFilesDigest{ files, digest, diff --git a/pkg/kudoctl/util/repo/resolver_repo.go b/pkg/kudoctl/util/repo/resolver_repo.go index 77e037208..8e24514a8 100644 --- a/pkg/kudoctl/util/repo/resolver_repo.go +++ b/pkg/kudoctl/util/repo/resolver_repo.go @@ -6,6 +6,8 @@ import ( "sort" "strings" + "github.com/spf13/afero" + "github.com/kudobuilder/kudo/pkg/kudoctl/clog" "github.com/kudobuilder/kudo/pkg/kudoctl/packages" "github.com/kudobuilder/kudo/pkg/kudoctl/packages/convert" @@ -35,15 +37,15 @@ func (b EntrySummaries) Less(x, y int) bool { } // Resolve returns a Package for a passed package name and optional version. This is an implementation -// of the Resolver interface located in packages/resolver/resolver.go -func (c *Client) Resolve(name string, appVersion string, operatorVersion string) (*packages.Resources, error) { +// of the DependenciesResolver interface located in packages/resolver/resolver.go +func (c *Client) Resolve(out afero.Fs, name string, appVersion string, operatorVersion string) (*packages.Resources, error) { buf, err := c.GetPackageBytes(name, appVersion, operatorVersion) if err != nil { return nil, err } clog.V(2).Printf("%v is a repository package from %v", name, c.Config) - files, err := reader.ParseTgz(buf) + files, err := reader.PackageFilesFromTar(out, buf) if err != nil { return nil, err } diff --git a/test/integration/operator-with-dependencies/child-operator/operator.yaml b/test/integration/operator-with-dependencies/child-operator/operator.yaml index 3854b4260..065d2720f 100644 --- a/test/integration/operator-with-dependencies/child-operator/operator.yaml +++ b/test/integration/operator-with-dependencies/child-operator/operator.yaml @@ -1,6 +1,7 @@ apiVersion: kudo.dev/v1beta1 name: "child" operatorVersion: "0.0.1" +appVersion: "3.2.1" kubernetesVersion: 1.15.0 maintainers: - name: zen-dog diff --git a/test/integration/operator-with-dependencies/parent-operator/operator.yaml b/test/integration/operator-with-dependencies/parent-operator/operator.yaml index 3632f6818..480baca24 100644 --- a/test/integration/operator-with-dependencies/parent-operator/operator.yaml +++ b/test/integration/operator-with-dependencies/parent-operator/operator.yaml @@ -11,7 +11,6 @@ tasks: kind: KudoOperator spec: package: "../child-operator" - operatorVersion: 0.0.1 plans: deploy: