diff --git a/alpha/declcfg/write.go b/alpha/declcfg/write.go index 6a0451a26..002adf6a9 100644 --- a/alpha/declcfg/write.go +++ b/alpha/declcfg/write.go @@ -467,52 +467,70 @@ type encoder interface { Encode(interface{}) error } -func writeToEncoder(cfg DeclarativeConfig, enc encoder) error { - pkgNames := sets.NewString() +func configsByPackage(cfg DeclarativeConfig) (sets.Set[string], map[string]DeclarativeConfig, []Meta) { + pkgNames := sets.New[string]() + byCfg := map[string]DeclarativeConfig{} + var rootOthers []Meta + + add := func(name string, fn func(DeclarativeConfig) DeclarativeConfig) { + if name == "" { + return + } + pkgNames.Insert(name) + byCfg[name] = fn(byCfg[name]) + } - packagesByName := map[string][]Package{} for _, p := range cfg.Packages { - pkgName := p.Name - pkgNames.Insert(pkgName) - packagesByName[pkgName] = append(packagesByName[pkgName], p) + add(p.Name, func(c DeclarativeConfig) DeclarativeConfig { + c.Packages = append(c.Packages, p) + return c + }) } - channelsByPackage := map[string][]Channel{} for _, c := range cfg.Channels { - pkgName := c.Package - pkgNames.Insert(pkgName) - channelsByPackage[pkgName] = append(channelsByPackage[pkgName], c) + add(c.Package, func(dc DeclarativeConfig) DeclarativeConfig { + dc.Channels = append(dc.Channels, c) + return dc + }) } - bundlesByPackage := map[string][]Bundle{} for _, b := range cfg.Bundles { - pkgName := b.Package - pkgNames.Insert(pkgName) - bundlesByPackage[pkgName] = append(bundlesByPackage[pkgName], b) + add(b.Package, func(c DeclarativeConfig) DeclarativeConfig { + c.Bundles = append(c.Bundles, b) + return c + }) } - othersByPackage := map[string][]Meta{} for _, o := range cfg.Others { - pkgName := o.Package - pkgNames.Insert(pkgName) - othersByPackage[pkgName] = append(othersByPackage[pkgName], o) + if o.Package == "" { + rootOthers = append(rootOthers, o) + continue + } + add(o.Package, func(c DeclarativeConfig) DeclarativeConfig { + c.Others = append(c.Others, o) + return c + }) } - deprecationsByPackage := map[string][]Deprecation{} for _, d := range cfg.Deprecations { - pkgName := d.Package - pkgNames.Insert(pkgName) - deprecationsByPackage[pkgName] = append(deprecationsByPackage[pkgName], d) + add(d.Package, func(c DeclarativeConfig) DeclarativeConfig { + c.Deprecations = append(c.Deprecations, d) + return c + }) } - for _, pName := range pkgNames.List() { - if len(pName) == 0 { - continue - } - pkgs := packagesByName[pName] - for _, p := range pkgs { + return pkgNames, byCfg, rootOthers +} + +func writeToEncoder(cfg DeclarativeConfig, enc encoder) error { + pkgNames, byCfg, rootOthers := configsByPackage(cfg) + + for _, pName := range sets.List(pkgNames) { + pkgCfg := byCfg[pName] + + for _, p := range pkgCfg.Packages { if err := enc.Encode(p); err != nil { return err } } - channels := channelsByPackage[pName] + channels := pkgCfg.Channels sort.Slice(channels, func(i, j int) bool { return channels[i].Name < channels[j].Name }) @@ -522,7 +540,7 @@ func writeToEncoder(cfg DeclarativeConfig, enc encoder) error { } } - bundles := bundlesByPackage[pName] + bundles := pkgCfg.Bundles sort.Slice(bundles, func(i, j int) bool { return bundles[i].Name < bundles[j].Name }) @@ -532,7 +550,7 @@ func writeToEncoder(cfg DeclarativeConfig, enc encoder) error { } } - others := othersByPackage[pName] + others := pkgCfg.Others sort.SliceStable(others, func(i, j int) bool { return others[i].Schema < others[j].Schema }) @@ -552,15 +570,14 @@ func writeToEncoder(cfg DeclarativeConfig, enc encoder) error { // Deprecation object for a package, and it would bypass validation if this // function gets called without conversion. // - deprecations := deprecationsByPackage[pName] - for _, d := range deprecations { + for _, d := range pkgCfg.Deprecations { if err := enc.Encode(d); err != nil { return err } } } - for _, o := range othersByPackage[""] { + for _, o := range rootOthers { if err := enc.Encode(o); err != nil { return err } @@ -572,34 +589,35 @@ func writeToEncoder(cfg DeclarativeConfig, enc encoder) error { type WriteFunc func(config DeclarativeConfig, w io.Writer) error func WriteFS(cfg DeclarativeConfig, rootDir string, writeFunc WriteFunc, fileExt string) error { - channelsByPackage := map[string][]Channel{} - for _, c := range cfg.Channels { - channelsByPackage[c.Package] = append(channelsByPackage[c.Package], c) - } - bundlesByPackage := map[string][]Bundle{} - for _, b := range cfg.Bundles { - bundlesByPackage[b.Package] = append(bundlesByPackage[b.Package], b) - } + pkgNames, byCfg, rootOthers := configsByPackage(cfg) if err := os.MkdirAll(rootDir, 0777); err != nil { return err } - for _, p := range cfg.Packages { - fcfg := DeclarativeConfig{ - Packages: []Package{p}, - Channels: channelsByPackage[p.Name], - Bundles: bundlesByPackage[p.Name], + for _, pName := range sets.List(pkgNames) { + if !filepath.IsLocal(pName) { + return fmt.Errorf("invalid package name %q: must be a single local path element", pName) } - pkgDir := filepath.Join(rootDir, p.Name) + pkgDir := filepath.Join(rootDir, pName) if err := os.MkdirAll(pkgDir, 0777); err != nil { return err } filename := filepath.Join(pkgDir, fmt.Sprintf("catalog%s", fileExt)) - if err := writeFile(fcfg, filename, writeFunc); err != nil { + if err := writeFile(byCfg[pName], filename, writeFunc); err != nil { + return err + } + } + + // Others with no package name cannot belong to any package directory; + // write them to a root-level catalog file, consistent with writeToEncoder. + if len(rootOthers) > 0 { + filename := filepath.Join(rootDir, fmt.Sprintf("catalog%s", fileExt)) + if err := writeFile(DeclarativeConfig{Others: rootOthers}, filename, writeFunc); err != nil { return err } } + return nil } diff --git a/alpha/declcfg/write_test.go b/alpha/declcfg/write_test.go index fd3c9fb20..74b5f3be2 100644 --- a/alpha/declcfg/write_test.go +++ b/alpha/declcfg/write_test.go @@ -2,7 +2,10 @@ package declcfg import ( "bytes" + "context" "encoding/json" + "os" + "path/filepath" "testing" "github.com/stretchr/testify/require" @@ -621,3 +624,40 @@ class anakin-dark-anakin.v0.1.0 skipped }) } } + +func TestWriteFS(t *testing.T) { + cfg := buildValidDeclarativeConfig(validDeclarativeConfigSpec{IncludeUnrecognized: true, IncludeDeprecations: true}) + + dir := t.TempDir() + require.NoError(t, WriteFS(cfg, dir, WriteJSON, ".json")) + + // Per-package files must include Others and Deprecations for that package. + anakinData, err := os.ReadFile(filepath.Join(dir, "anakin", "catalog.json")) + require.NoError(t, err) + anakinStr := string(anakinData) + require.Contains(t, anakinStr, `"schema": "custom.3"`, "anakin catalog must contain Others") + require.Contains(t, anakinStr, `"schema": "olm.deprecations"`, "anakin catalog must contain Deprecations") + + bobData, err := os.ReadFile(filepath.Join(dir, "boba-fett", "catalog.json")) + require.NoError(t, err) + bobStr := string(bobData) + require.Contains(t, bobStr, `"schema": "custom.3"`, "boba-fett catalog must contain Others") + + // Others with no package name must appear in a root-level catalog file. + rootData, err := os.ReadFile(filepath.Join(dir, "catalog.json")) + require.NoError(t, err) + rootStr := string(rootData) + require.Contains(t, rootStr, `"schema": "custom.1"`, "root catalog must contain package-less Others") + require.Contains(t, rootStr, `"schema": "custom.2"`, "root catalog must contain package-less Others") + + // Round-trip: loading the written files must reproduce the Others and Deprecations. + // Normalize JSON whitespace in Blob fields before comparing because LoadFS calls + // Meta.UnmarshalJSON, which re-encodes the raw blob through a map[string]interface{} + // and therefore compacts whitespace and normalizes key order. + loaded, err := LoadFS(context.Background(), os.DirFS(dir)) + require.NoError(t, err) + removeJSONWhitespace(&cfg) + removeJSONWhitespace(loaded) + require.ElementsMatch(t, cfg.Others, loaded.Others) + require.ElementsMatch(t, cfg.Deprecations, loaded.Deprecations) +}