From 836c7422a4d2c75161edd0446c4242446699f233 Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Thu, 22 Jul 2021 18:23:26 -0700 Subject: [PATCH] feat(opm): include arbitrary packages, channels, and versions in diffs Signed-off-by: Eric Stroczynski --- alpha/action/diff.go | 85 ++++++++++++++++ alpha/action/diff_test.go | 160 +++++++++++++++++++++++++++--- alpha/declcfg/diff.go | 29 +++++- alpha/declcfg/diff_include.go | 181 +++++++++++++++++++++++++++++++++- cmd/opm/alpha/diff/cmd.go | 49 +++++++-- 5 files changed, 479 insertions(+), 25 deletions(-) diff --git a/alpha/action/diff.go b/alpha/action/diff.go index 4f7427f95..9bd5d7501 100644 --- a/alpha/action/diff.go +++ b/alpha/action/diff.go @@ -4,8 +4,12 @@ import ( "context" "errors" "fmt" + "io" + "github.com/blang/semver" "github.com/sirupsen/logrus" + utilerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/apimachinery/pkg/util/yaml" "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/model" @@ -21,6 +25,8 @@ type Diff struct { // of bundles included in the diff if true. SkipDependencies bool + IncludeConfig DiffIncludeConfig + Logger *logrus.Entry } @@ -65,6 +71,7 @@ func (a Diff) Run(ctx context.Context) (*declcfg.DeclarativeConfig, error) { g := &declcfg.DiffGenerator{ Logger: a.Logger, SkipDependencies: a.SkipDependencies, + Includer: convertIncludeConfigToIncluder(a.IncludeConfig), } diffModel, err := g.Run(oldModel, newModel) if err != nil { @@ -81,3 +88,81 @@ func (p Diff) validate() error { } return nil } + +// DiffIncludeConfig configures Diff.Run() to include a set of packages, +// channels, and/or bundle versions in the output DeclarativeConfig. +// These override other diff mechanisms. For example, if running in +// heads-only mode but package "foo" channel "stable" is specified, +// the entire "stable" channel (all channel bundles) is added to the output. +type DiffIncludeConfig struct { + // Packages to include. + Packages []DiffIncludePackage `json:"packages" yaml:"packages"` +} + +// DiffIncludePackage contains a name (required) and channels and/or versions +// (optional) to include in the diff. The full package is only included if no channels +// or versions are specified. +type DiffIncludePackage struct { + // Name of package. + Name string `json:"name" yaml:"name"` + // Channels to include. + Channels []DiffIncludeChannel `json:"channels,omitempty" yaml:"channels,omitempty"` + // Versions to include. All channels containing these versions + // are parsed for an upgrade graph. + Versions []semver.Version `json:"versions,omitempty" yaml:"versions,omitempty"` +} + +// DiffIncludeChannel contains a name (required) and versions (optional) +// to include in the diff. The full channel is only included if no versions are specified. +type DiffIncludeChannel struct { + // Name of channel. + Name string `json:"name" yaml:"name"` + // Versions to include. + Versions []semver.Version `json:"versions,omitempty" yaml:"versions,omitempty"` +} + +// LoadDiffIncludeConfig loads a (YAML or JSON) DiffIncludeConfig from r. +func LoadDiffIncludeConfig(r io.Reader) (c DiffIncludeConfig, err error) { + dec := yaml.NewYAMLOrJSONDecoder(r, 8) + if err := dec.Decode(&c); err != nil { + return DiffIncludeConfig{}, err + } + + if len(c.Packages) == 0 { + return c, fmt.Errorf("must specify at least one package in include config") + } + + var errs []error + for pkgI, pkg := range c.Packages { + if pkg.Name == "" { + errs = append(errs, fmt.Errorf("package at index %v requires a name", pkgI)) + continue + } + for chI, ch := range pkg.Channels { + if ch.Name == "" { + errs = append(errs, fmt.Errorf("package %s: channel at index %v requires a name", pkg.Name, chI)) + continue + } + } + } + return c, utilerrors.NewAggregate(errs) +} + +func convertIncludeConfigToIncluder(c DiffIncludeConfig) (includer declcfg.DiffIncluder) { + includer.Packages = make([]declcfg.DiffIncludePackage, len(c.Packages)) + for pkgI, cpkg := range c.Packages { + pkg := &includer.Packages[pkgI] + pkg.Name = cpkg.Name + pkg.AllChannels.Versions = cpkg.Versions + + if len(cpkg.Channels) != 0 { + pkg.Channels = make([]declcfg.DiffIncludeChannel, len(cpkg.Channels)) + for chI, cch := range cpkg.Channels { + ch := &pkg.Channels[chI] + ch.Name = cch.Name + ch.Versions = cch.Versions + } + } + } + return includer +} diff --git a/alpha/action/diff_test.go b/alpha/action/diff_test.go index 4ab09caf2..fc7f9fb64 100644 --- a/alpha/action/diff_test.go +++ b/alpha/action/diff_test.go @@ -1,6 +1,7 @@ -package action_test +package action import ( + "bytes" "context" "embed" "errors" @@ -10,10 +11,10 @@ import ( "strings" "testing" + "github.com/blang/semver" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/operator-framework/operator-registry/alpha/action" "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/pkg/containertools" "github.com/operator-framework/operator-registry/pkg/image" @@ -23,7 +24,7 @@ import ( func TestDiff(t *testing.T) { type spec struct { name string - diff action.Diff + diff Diff expectedCfg *declcfg.DeclarativeConfig assertion require.ErrorAssertionFunc } @@ -34,7 +35,7 @@ func TestDiff(t *testing.T) { specs := []spec{ { name: "Success/Latest", - diff: action.Diff{ + diff: Diff{ Registry: registry, OldRefs: []string{filepath.Join("testdata", "index-declcfgs", "old")}, NewRefs: []string{filepath.Join("testdata", "index-declcfgs", "latest")}, @@ -44,7 +45,7 @@ func TestDiff(t *testing.T) { }, { name: "Success/HeadsOnly", - diff: action.Diff{ + diff: Diff{ Registry: registry, NewRefs: []string{filepath.Join("testdata", "index-declcfgs", "latest")}, }, @@ -53,7 +54,7 @@ func TestDiff(t *testing.T) { }, { name: "Fail/NewBundleImage", - diff: action.Diff{ + diff: Diff{ Registry: registry, NewRefs: []string{"test.registry/foo-operator/foo-bundle:v0.1.0"}, }, @@ -61,14 +62,14 @@ func TestDiff(t *testing.T) { if !assert.Error(t, err) { require.Fail(t, "expected an error") } - if !errors.Is(err, action.ErrNotAllowed) { - require.Fail(t, "err is not action.ErrNotAllowed", err) + if !errors.Is(err, ErrNotAllowed) { + require.Fail(t, "err is not ErrNotAllowed", err) } }, }, { name: "Fail/OldBundleImage", - diff: action.Diff{ + diff: Diff{ Registry: registry, OldRefs: []string{"test.registry/foo-operator/foo-bundle:v0.1.0"}, NewRefs: []string{filepath.Join("testdata", "index-declcfgs", "latest")}, @@ -77,8 +78,8 @@ func TestDiff(t *testing.T) { if !assert.Error(t, err) { require.Fail(t, "expected an error") } - if !errors.Is(err, action.ErrNotAllowed) { - require.Fail(t, "err is not action.ErrNotAllowed", err) + if !errors.Is(err, ErrNotAllowed) { + require.Fail(t, "err is not ErrNotAllowed", err) } }, }, @@ -93,6 +94,143 @@ func TestDiff(t *testing.T) { } } +func TestLoadDiffIncludeConfig(t *testing.T) { + type spec struct { + name string + input string + expectedCfg DiffIncludeConfig + expectedIncluder declcfg.DiffIncluder + assertion require.ErrorAssertionFunc + } + + specs := []spec{ + { + name: "Success/Basic", + input: ` +packages: +- name: foo +`, + expectedCfg: DiffIncludeConfig{ + Packages: []DiffIncludePackage{{Name: "foo"}}, + }, + expectedIncluder: declcfg.DiffIncluder{ + Packages: []declcfg.DiffIncludePackage{{Name: "foo"}}, + }, + assertion: require.NoError, + }, + { + name: "Success/MultiPackage", + input: ` +packages: +- name: foo + channels: + - name: stable + versions: + - 0.1.0 + - 0.2.0 + versions: + - 1.0.0 +- name: bar + channels: + - name: stable + versions: + - 0.1.0 + versions: + - 1.0.0 +`, + expectedCfg: DiffIncludeConfig{ + Packages: []DiffIncludePackage{ + { + Name: "foo", + Channels: []DiffIncludeChannel{ + {Name: "stable", Versions: []semver.Version{ + semver.MustParse("0.1.0"), + semver.MustParse("0.2.0"), + }}, + }, + Versions: []semver.Version{semver.MustParse("1.0.0")}, + }, + { + Name: "bar", + Channels: []DiffIncludeChannel{ + {Name: "stable", Versions: []semver.Version{ + semver.MustParse("0.1.0"), + }}, + }, + Versions: []semver.Version{semver.MustParse("1.0.0")}, + }, + }, + }, + expectedIncluder: declcfg.DiffIncluder{ + Packages: []declcfg.DiffIncludePackage{ + { + Name: "foo", + Channels: []declcfg.DiffIncludeChannel{ + {Name: "stable", Versions: []semver.Version{ + semver.MustParse("0.1.0"), + semver.MustParse("0.2.0"), + }}, + }, + AllChannels: declcfg.DiffIncludeChannel{ + Versions: []semver.Version{semver.MustParse("1.0.0")}, + }, + }, + { + Name: "bar", + Channels: []declcfg.DiffIncludeChannel{ + {Name: "stable", Versions: []semver.Version{ + semver.MustParse("0.1.0"), + }}, + }, + AllChannels: declcfg.DiffIncludeChannel{ + Versions: []semver.Version{semver.MustParse("1.0.0")}, + }, + }, + }, + }, + assertion: require.NoError, + }, + { + name: "Fail/Empty", + input: ``, + assertion: require.Error, + }, + { + name: "Fail/NoPackageName", + input: ` +packages: +- channels: + - name: stable + versions: + - 0.1.0 +`, + assertion: require.Error, + }, + { + name: "Fail/NoChannelName", + input: ` +packages: +- name: foo + channels: + - versions: + - 0.1.0 +`, + assertion: require.Error, + }, + } + + for _, s := range specs { + t.Run(s.name, func(t *testing.T) { + actualCfg, err := LoadDiffIncludeConfig(bytes.NewBufferString(s.input)) + s.assertion(t, err) + if err == nil { + require.Equal(t, s.expectedCfg, actualCfg) + require.Equal(t, s.expectedIncluder, convertIncludeConfigToIncluder(actualCfg)) + } + }) + } +} + var ( //go:embed testdata/foo-bundle-v0.1.0/manifests/* //go:embed testdata/foo-bundle-v0.1.0/metadata/* diff --git a/alpha/declcfg/diff.go b/alpha/declcfg/diff.go index c4f78d660..35b06aed3 100644 --- a/alpha/declcfg/diff.go +++ b/alpha/declcfg/diff.go @@ -21,6 +21,8 @@ type DiffGenerator struct { // SkipDependencies directs Run() to not include dependencies // of bundles included in the diff if true. SkipDependencies bool + // Includer for adding catalog objects to Run() output. + Includer DiffIncluder initOnce sync.Once } @@ -45,15 +47,31 @@ func (g *DiffGenerator) Run(oldModel, newModel model.Model) (model.Model, error) // load by package. outputModel := model.Model{} + + // Add included packages/channels/bundles from newModel to outputModel. + // Code further down handles packages and channels included in outputModel. + g.Includer.Logger = g.Logger + if err := g.Includer.Run(newModel, outputModel); err != nil { + return nil, err + } + if len(oldModel) == 0 { // Heads-only mode. // Make shallow copies of packages and channels that are only // filled with channel heads. for _, newPkg := range newModel { - outputPkg := copyPackageNoChannels(newPkg) - outputModel[outputPkg.Name] = outputPkg + // This package may have been created in the include step. + outputPkg, pkgIncluded := outputModel[newPkg.Name] + if !pkgIncluded { + outputPkg = copyPackageNoChannels(newPkg) + outputModel[outputPkg.Name] = outputPkg + } for _, newCh := range newPkg.Channels { + if _, chIncluded := outputPkg.Channels[newCh.Name]; chIncluded { + // Head (and other bundles) were added in the include step. + continue + } outputCh := copyChannelNoBundles(newCh, outputPkg) outputPkg.Channels[outputCh.Name] = outputCh head, err := newCh.Head() @@ -71,6 +89,11 @@ func (g *DiffGenerator) Run(oldModel, newModel model.Model) (model.Model, error) // which is more succinct than by addition and potentially // more memory efficient. for _, newPkg := range newModel { + if _, pkgIncluded := outputModel[newPkg.Name]; pkgIncluded { + // The user has specified the state they want this package to have in the diff + // via an inclusion entry, so the package created above should not be changed. + continue + } outputModel[newPkg.Name] = copyPackage(newPkg) } @@ -274,7 +297,6 @@ func getBundles(m model.Model) (bundles []*model.Bundle) { for _, pkg := range m { for _, ch := range pkg.Channels { for _, b := range ch.Bundles { - b := b bundles = append(bundles, b) } } @@ -332,7 +354,6 @@ func getBundlesThatProvide(pkg *model.Package, reqGVKs map[property.GVK]struct{} bundlesProvidingGVK := make(map[property.GVK][]*model.Bundle) for _, ch := range pkg.Channels { for _, b := range ch.Bundles { - b := b for _, gvk := range b.PropertiesP.GVKs { if _, hasGVK := reqGVKs[gvk]; hasGVK { bundlesProvidingGVK[gvk] = append(bundlesProvidingGVK[gvk], b) diff --git a/alpha/declcfg/diff_include.go b/alpha/declcfg/diff_include.go index 14a4d5c89..457fe0e94 100644 --- a/alpha/declcfg/diff_include.go +++ b/alpha/declcfg/diff_include.go @@ -1,14 +1,193 @@ package declcfg import ( + "fmt" + + "github.com/blang/semver" + "github.com/sirupsen/logrus" + utilerrors "k8s.io/apimachinery/pkg/util/errors" + "github.com/operator-framework/operator-registry/alpha/model" ) +// DiffIncluder knows how to add packages, channels, and bundles +// from a source to a destination model.Model. +type DiffIncluder struct { + // Packages to add. + Packages []DiffIncludePackage + Logger *logrus.Entry +} + +// DiffIncludePackage specifies a package, and optionally channels +// or a set of bundles from all channels (wrapped by a DiffIncludeChannel), +// to include. +type DiffIncludePackage struct { + // Name of package. + Name string + // Channels in package. + Channels []DiffIncludeChannel + // AllChannels contains bundle versions in package. + // Upgrade graphs from all channels in the named package containing a version + // from this field are included. + AllChannels DiffIncludeChannel +} + +// DiffIncludeChannel specifies a channel, and optionally bundle versions to include. +type DiffIncludeChannel struct { + // Name of channel. + Name string + // Versions of bundles. + Versions []semver.Version +} + +// Run adds all packages and channels in DiffIncluder with matching names +// directly, and all versions plus their upgrade graphs to channel heads, +// from newModel to outputModel. +func (i DiffIncluder) Run(newModel, outputModel model.Model) error { + var includeErrs []error + for _, ipkg := range i.Packages { + pkgLog := i.Logger.WithField("package", ipkg.Name) + includeErrs = append(includeErrs, ipkg.includeNewInOutputModel(newModel, outputModel, pkgLog)...) + } + if len(includeErrs) != 0 { + return fmt.Errorf("error including items:\n%v", utilerrors.NewAggregate(includeErrs)) + } + return nil +} + +// includeNewInOutputModel adds all packages, channels, and versions (bundles) +// specified by ipkg that exist in newModel to outputModel. Any package, channel, +// or version in ipkg not satisfied by newModel is an error. +func (ipkg DiffIncludePackage) includeNewInOutputModel(newModel, outputModel model.Model, logger *logrus.Entry) (ierrs []error) { + + newPkg, newHasPkg := newModel[ipkg.Name] + if !newHasPkg { + ierrs = append(ierrs, fmt.Errorf("[package=%q] package does not exist in new model", ipkg.Name)) + return ierrs + } + pkgLog := logger.WithField("package", newPkg.Name) + + // No channels or versions were specified, meaning "include the full package". + if len(ipkg.Channels) == 0 && len(ipkg.AllChannels.Versions) == 0 { + outputModel[ipkg.Name] = newPkg + return nil + } + + outputPkg := copyPackageNoChannels(newPkg) + outputModel[outputPkg.Name] = outputPkg + + // Add all channels to ipkg.Channels if versions were specified to include across all channels. + // skipMissingVerForChannels's value for a channel will be true IFF at least one version is specified, + // since some other channel may contain that version. + skipMissingVerForChannels := map[string]bool{} + if len(ipkg.AllChannels.Versions) != 0 { + for newChName := range newPkg.Channels { + ipkg.Channels = append(ipkg.Channels, DiffIncludeChannel{ + Name: newChName, + Versions: ipkg.AllChannels.Versions, + }) + skipMissingVerForChannels[newChName] = true + } + } + + for _, ich := range ipkg.Channels { + newCh, pkgHasCh := newPkg.Channels[ich.Name] + if !pkgHasCh { + ierrs = append(ierrs, fmt.Errorf("[package=%q channel=%q] channel does not exist in new model", newPkg.Name, ich.Name)) + continue + } + chLog := pkgLog.WithField("channel", newCh.Name) + + bundles, err := getBundlesForVersions(newCh, ich.Versions, chLog, skipMissingVerForChannels[newCh.Name]) + if err != nil { + ierrs = append(ierrs, fmt.Errorf("[package=%q channel=%q] %v", newPkg.Name, newCh.Name, err)) + continue + } + outputCh := copyChannelNoBundles(newCh, outputPkg) + outputPkg.Channels[outputCh.Name] = outputCh + for _, b := range bundles { + tb := copyBundle(b, outputCh, outputPkg) + outputCh.Bundles[tb.Name] = tb + } + } + + return ierrs +} + +// getBundlesForVersions returns all bundles matching a version in vers +// and their upgrade graph(s) to ch.Head(). +// If skipMissingVersions is true, versions not satisfied by bundles in ch +// will not result in errors. +func getBundlesForVersions(ch *model.Channel, vers []semver.Version, logger *logrus.Entry, skipMissingVersions bool) (bundles []*model.Bundle, err error) { + + // Short circuit when no versions were specified, meaning "include the whole channel". + if len(vers) == 0 { + for _, b := range ch.Bundles { + bundles = append(bundles, b) + } + return bundles, nil + } + + // Add every bundle directly satisfying a version to bundles. + versionsToInclude := make(map[string]struct{}, len(vers)) + for _, ver := range vers { + versionsToInclude[ver.String()] = struct{}{} + } + for _, b := range ch.Bundles { + if _, includeBundle := versionsToInclude[b.Version.String()]; includeBundle { + bundles = append(bundles, b) + } + } + + // Some version was not satisfied by this channel. + if len(bundles) != len(versionsToInclude) && !skipMissingVersions { + for _, b := range bundles { + delete(versionsToInclude, b.Version.String()) + } + verStrs := []string{} + for verStr := range versionsToInclude { + verStrs = append(verStrs, verStr) + } + return nil, fmt.Errorf("versions do not exist in channel: %+q", verStrs) + } + + // Fill in the upgrade graph between each bundle and head. + // Regardless of semver order, this step needs to be performed + // for each included bundle because there might be leaf nodes + // in the upgrade graph for a particular version not captured + // by any other fill due to skips not being honored here. + head, err := ch.Head() + if err != nil { + return nil, err + } + graph := makeUpgradeGraph(ch) + bundleSet := map[string]*model.Bundle{} + for _, ib := range bundles { + if _, addedBundle := bundleSet[ib.Name]; addedBundle { + // A prior graph traverse already included this bundle. + continue + } + intersectingBundles, intersectionFound := findIntersectingBundles(ch, ib, head, graph) + if !intersectionFound { + logger.Debugf("channel head %q not reachable from bundle %q, adding without upgrade graph", head.Name, ib.Name) + bundleSet[ib.Name] = ib + } + + for _, rb := range intersectingBundles { + bundleSet[rb.Name] = rb + } + } + for _, b := range bundleSet { + bundles = append(bundles, b) + } + + return bundles, nil +} + // makeUpgradeGraph creates a DAG of bundles with map key Bundle.Replaces. func makeUpgradeGraph(ch *model.Channel) map[string][]*model.Bundle { graph := map[string][]*model.Bundle{} for _, b := range ch.Bundles { - b := b if b.Replaces != "" { graph[b.Replaces] = append(graph[b.Replaces], b) } diff --git a/cmd/opm/alpha/diff/cmd.go b/cmd/opm/alpha/diff/cmd.go index f1a83f759..4d8e0cc39 100644 --- a/cmd/opm/alpha/diff/cmd.go +++ b/cmd/opm/alpha/diff/cmd.go @@ -24,9 +24,10 @@ const ( ) type diff struct { - oldRefs []string - newRefs []string - skipDeps bool + oldRefs []string + newRefs []string + skipDeps bool + includeFile string output string caFile string @@ -62,11 +63,6 @@ case they are included. Dependencies provided by some catalog unknown to if that catalog is not serving these dependencies at runtime. Dependency inclusion can be turned off with --no-deps, although this is not recommended unless you are certain some in-cluster catalog satisfies all dependencies. - -NOTE: for now, if any dependency exists, the entire dependency's package is added to the diff. -In the future, these packages will be pruned such that only the latest dependencies -satisfying a package version range or GVK, and their upgrade graph(s) to their latest -channel head(s), are included in the diff. `), Example: templates.Examples(` # Create a directory for your declarative config diff. @@ -81,6 +77,23 @@ opm alpha diff registry.org/my-catalog:abc123 registry.org/my-catalog:def456 -o # Create a new catalog from the heads of an existing catalog. opm alpha diff registry.org/my-catalog:def456 -o yaml > my-catalog-index/index.yaml +# OR: +# Create a heads-only catalog, but include all of package "foo", package "bar" channel "stable", +# and package "baz" channel "alpha" version "0.2.0-alpha.0" (and its upgrade graph) in the diff. +cat < include.yaml +packages: +- name: foo +- name: bar + channels: + - name: stable +- name: baz + channels: + - name: alpha + versions: + - 0.2.0-alpha.0 +EOF +opm alpha diff registry.org/my-catalog:def456 -i include.yaml -o yaml > my-catalog-index/index.yaml + # FINALLY: # Build an index image containing the diff-ed declarative config, # then tag and push it. @@ -102,7 +115,10 @@ docker push registry.org/my-catalog:diff-latest cmd.Flags().BoolVar(&a.skipDeps, "skip-deps", false, "do not include bundle dependencies in the output catalog") cmd.Flags().StringVarP(&a.output, "output", "o", "yaml", "Output format (json|yaml)") - cmd.Flags().StringVarP(&a.caFile, "ca-file", "", "", "the root Certificates to use with this command") + cmd.Flags().StringVar(&a.caFile, "ca-file", "", "the root Certificates to use with this command") + cmd.Flags().StringVarP(&a.includeFile, "include-file", "i", "", "YAML defining packages, "+ + "channels, and/or versions to include in the diff from the new refs. Upgrade graphs "+ + "from individual versions to their channel's head are added to the diff") cmd.Flags().BoolVar(&a.debug, "debug", false, "enable debug logging") return cmd @@ -140,6 +156,20 @@ func (a *diff) addFunc(cmd *cobra.Command, args []string) error { } }() + f, err := os.Open(a.includeFile) + if err != nil { + a.logger.Fatalf("error opening include file: %v", err) + } + defer func() { + if cerr := f.Close(); cerr != nil { + a.logger.Error(cerr) + } + }() + includeConfig, err := action.LoadDiffIncludeConfig(f) + if err != nil { + a.logger.Fatalf("error loading include file: %v", err) + } + ctx, cancel := context.WithTimeout(context.TODO(), timeout) defer cancel() @@ -148,6 +178,7 @@ func (a *diff) addFunc(cmd *cobra.Command, args []string) error { OldRefs: a.oldRefs, NewRefs: a.newRefs, SkipDependencies: a.skipDeps, + IncludeConfig: includeConfig, Logger: a.logger, } cfg, err := diff.Run(ctx)