Skip to content

Commit ae562d8

Browse files
committed
review updates from #1986
Signed-off-by: grokspawn <jordan@nimblewidget.com>
1 parent 9a9b88d commit ae562d8

8 files changed

Lines changed: 80 additions & 108 deletions

File tree

AGENTS.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -105,13 +105,13 @@ opm render quay.io/my-namespace/my-bundle:v1.0.0 quay.io/my-namespace/my-bundle:
105105
### 3. Working with Declarative Config
106106
```go
107107
// Load declarative config
108-
cfg, err := declcfg.LoadFS(os.DirFS("path/to/catalog"))
108+
cfg, err := declcfg.LoadFS(ctx, os.DirFS("path/to/catalog"))
109109

110-
// Convert to model
111-
model, err := declcfg.ConvertToModel(cfg)
110+
// Validate the config
111+
err = declcfg.Validate(*cfg)
112112

113-
// Write declarative config
114-
err = declcfg.Write(cfg, "output.yaml")
113+
// Write declarative config to filesystem
114+
err = declcfg.WriteFS(*cfg, "output-dir", declcfg.WriteYAML, ".yaml")
115115
```
116116

117117
### 4. Serving Catalog Content with `opm serve`

alpha/action/list.go

Lines changed: 2 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func getDisplayName(pkg declcfg.Package, channels []declcfg.Channel, bundles []d
7272
}
7373

7474
// Find the head bundle
75-
head, err := findChannelHead(defaultCh.Entries)
75+
head, err := declcfg.FindChannelHead(defaultCh.Entries)
7676
if err != nil {
7777
return ""
7878
}
@@ -96,41 +96,6 @@ func getDisplayName(pkg declcfg.Package, channels []declcfg.Channel, bundles []d
9696
return csv.Spec.DisplayName
9797
}
9898

99-
// findChannelHead finds the head bundle of a channel by analyzing the replaces chain.
100-
func findChannelHead(entries []declcfg.ChannelEntry) (string, error) {
101-
if len(entries) == 0 {
102-
return "", fmt.Errorf("channel has no entries")
103-
}
104-
105-
// Build a map of bundles that are replaced
106-
replaced := make(map[string]bool)
107-
for _, entry := range entries {
108-
if entry.Replaces != "" {
109-
replaced[entry.Replaces] = true
110-
}
111-
for _, skip := range entry.Skips {
112-
replaced[skip] = true
113-
}
114-
}
115-
116-
// Find bundles that are not replaced by anything
117-
var heads []string
118-
for _, entry := range entries {
119-
if !replaced[entry.Name] {
120-
heads = append(heads, entry.Name)
121-
}
122-
}
123-
124-
if len(heads) == 0 {
125-
return "", fmt.Errorf("channel has circular replaces chain, no head found")
126-
}
127-
if len(heads) > 1 {
128-
return "", fmt.Errorf("channel has multiple heads: %v", heads)
129-
}
130-
131-
return heads[0], nil
132-
}
133-
13499
type ListChannels struct {
135100
IndexReference string
136101
PackageName string
@@ -168,7 +133,7 @@ func (r *ListChannelsResult) WriteColumns(w io.Writer) error {
168133
}
169134
for _, ch := range r.Channels {
170135
headStr := ""
171-
head, err := findChannelHead(ch.Entries)
136+
head, err := declcfg.FindChannelHead(ch.Entries)
172137
if err != nil {
173138
headStr = fmt.Sprintf("ERROR: %s", err)
174139
} else {

alpha/declcfg/api_conversions.go

Lines changed: 29 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,24 +18,36 @@ import (
1818
)
1919

2020
// ConvertBundleToAPIBundle converts a declcfg.Bundle to an api.Bundle.
21-
// The pkg and channels parameters provide context needed for the conversion.
22-
func ConvertBundleToAPIBundle(b Bundle, pkg Package, channels []Channel) (*api.Bundle, error) {
21+
// The pkg, channels, and deprecations parameters provide context needed for the conversion.
22+
// api.Bundle can only represent one channel, so the first matching channel is used.
23+
func ConvertBundleToAPIBundle(b Bundle, pkg Package, channels []Channel, deprecations []Deprecation) (*api.Bundle, error) {
2324
props, err := parseProperties(b.Properties)
2425
if err != nil {
2526
return nil, fmt.Errorf("parse properties: %v", err)
2627
}
2728

2829
csvJSON := generateCSVJSON(b, pkg, props)
30+
objects := b.Objects
31+
if len(objects) == 0 && csvJSON != "" {
32+
objects = []string{csvJSON}
33+
}
2934

30-
// Find which channel this bundle belongs to
31-
channelName := ""
35+
// Find which channel this bundle belongs to and get its entry data.
36+
// api.Bundle can only represent one channel, so use the first match.
37+
var channelName string
38+
var replaces string
39+
var skips []string
40+
var skipRange string
3241
for _, ch := range channels {
3342
if ch.Package != b.Package {
3443
continue
3544
}
3645
for _, entry := range ch.Entries {
3746
if entry.Name == b.Name {
3847
channelName = ch.Name
48+
replaces = entry.Replaces
49+
skips = entry.Skips
50+
skipRange = entry.SkipRange
3951
break
4052
}
4153
}
@@ -44,22 +56,21 @@ func ConvertBundleToAPIBundle(b Bundle, pkg Package, channels []Channel) (*api.B
4456
}
4557
}
4658

47-
// Get replaces and skips from channel entry
48-
var replaces string
49-
var skips []string
50-
var skipRange string
51-
for _, ch := range channels {
52-
if ch.Package != b.Package {
59+
// Find bundle deprecation
60+
var deprecation *api.Deprecation
61+
for _, d := range deprecations {
62+
if d.Package != b.Package {
5363
continue
5464
}
55-
for _, entry := range ch.Entries {
56-
if entry.Name == b.Name {
57-
replaces = entry.Replaces
58-
skips = entry.Skips
59-
skipRange = entry.SkipRange
65+
for _, entry := range d.Entries {
66+
if entry.Reference.Schema == SchemaBundle && entry.Reference.Name == b.Name {
67+
deprecation = &api.Deprecation{Message: entry.Message}
6068
break
6169
}
6270
}
71+
if deprecation != nil {
72+
break
73+
}
6374
}
6475

6576
apiDeps, err := convertPropertiesToAPIDependencies(b.Properties)
@@ -81,7 +92,8 @@ func ConvertBundleToAPIBundle(b Bundle, pkg Package, channels []Channel) (*api.B
8192
Replaces: replaces,
8293
Skips: skips,
8394
CsvJson: csvJSON,
84-
Object: b.Objects,
95+
Object: objects,
96+
Deprecation: deprecation,
8597
}, nil
8698
}
8799

@@ -95,11 +107,7 @@ func generateCSVJSON(b Bundle, pkg Package, props *property.Properties) string {
95107
if err != nil {
96108
return b.CsvJSON
97109
}
98-
csvJSON := string(csvData)
99-
if len(b.Objects) == 0 {
100-
b.Objects = []string{csvJSON}
101-
}
102-
return csvJSON
110+
return string(csvData)
103111
}
104112

105113
func buildCSV(b Bundle, pkg Package, props *property.Properties) *v1alpha1.ClusterServiceVersion {

alpha/declcfg/declcfg.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,40 @@ type ChannelEntry struct {
6161
SkipRange string `json:"skipRange,omitempty"`
6262
}
6363

64+
// FindChannelHead returns the name of the head entry in the channel — the entry
65+
// that is not replaced or skipped by any other entry.
66+
func FindChannelHead(entries []ChannelEntry) (string, error) {
67+
if len(entries) == 0 {
68+
return "", fmt.Errorf("channel has no entries")
69+
}
70+
71+
replaced := make(map[string]bool)
72+
for _, entry := range entries {
73+
if entry.Replaces != "" {
74+
replaced[entry.Replaces] = true
75+
}
76+
for _, skip := range entry.Skips {
77+
replaced[skip] = true
78+
}
79+
}
80+
81+
var heads []string
82+
for _, entry := range entries {
83+
if !replaced[entry.Name] {
84+
heads = append(heads, entry.Name)
85+
}
86+
}
87+
88+
if len(heads) == 0 {
89+
return "", fmt.Errorf("channel has circular replaces chain, no head found")
90+
}
91+
if len(heads) > 1 {
92+
return "", fmt.Errorf("channel has multiple heads: %v", heads)
93+
}
94+
95+
return heads[0], nil
96+
}
97+
6498
// Bundle specifies all metadata and data of a bundle object.
6599
// Top-level fields are the source of truth, i.e. not CSV values.
66100
//

pkg/cache/cache.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -428,13 +428,14 @@ func (c *cache) processPackage(ctx context.Context, reader io.Reader) (packageIn
428428
}
429429
}
430430

431-
apiBundle, err := declcfg.ConvertBundleToAPIBundle(b, *pkg, relevantChannels)
432-
if err != nil {
433-
return nil, fmt.Errorf("convert bundle %q: %v", b.Name, err)
434-
}
435-
436-
// Store the bundle for each channel it appears in
431+
// Convert and store the bundle for each channel it appears in.
432+
// Each channel may have different replaces/skips for the same bundle,
433+
// so we convert once per channel.
437434
for _, ch := range relevantChannels {
435+
apiBundle, err := declcfg.ConvertBundleToAPIBundle(b, *pkg, []declcfg.Channel{ch}, pkgFbc.Deprecations)
436+
if err != nil {
437+
return nil, fmt.Errorf("convert bundle %q: %v", b.Name, err)
438+
}
438439
if err := c.backend.PutBundle(ctx, bundleKey{b.Package, ch.Name, b.Name}, apiBundle); err != nil {
439440
return nil, fmt.Errorf("store bundle %q: %v", b.Name, err)
440441
}

pkg/cache/json_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func TestJSON_StableDigest(t *testing.T) {
2828
//
2929
// If validFS needs to change DO NOT CHANGE the json cache implementation
3030
// in the same pull request.
31-
require.Equal(t, "976864aa02f6a581", actualDigest)
31+
require.Equal(t, "9adad9ff6cf54e4f", actualDigest)
3232
}
3333

3434
func TestJSON_CheckIntegrity(t *testing.T) {

pkg/cache/pkgs.go

Lines changed: 1 addition & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ func packagesFromDeclcfg(cfg declcfg.DeclarativeConfig) (map[string]cPkg, error)
228228
}
229229

230230
// Find the head of this channel
231-
head, err := findChannelHead(ch.Entries)
231+
head, err := declcfg.FindChannelHead(ch.Entries)
232232
if err != nil {
233233
return nil, fmt.Errorf("find head for channel %q in package %q: %v", ch.Name, ch.Package, err)
234234
}
@@ -283,42 +283,6 @@ func packagesFromDeclcfg(cfg declcfg.DeclarativeConfig) (map[string]cPkg, error)
283283
return pkgs, nil
284284
}
285285

286-
// findChannelHead finds the head bundle of a channel by analyzing the replaces chain.
287-
// The head is the bundle that is not replaced by any other bundle in the channel.
288-
func findChannelHead(entries []declcfg.ChannelEntry) (string, error) {
289-
if len(entries) == 0 {
290-
return "", fmt.Errorf("channel has no entries")
291-
}
292-
293-
// Build a map of bundles that are replaced
294-
replaced := make(map[string]bool)
295-
for _, entry := range entries {
296-
if entry.Replaces != "" {
297-
replaced[entry.Replaces] = true
298-
}
299-
for _, skip := range entry.Skips {
300-
replaced[skip] = true
301-
}
302-
}
303-
304-
// Find bundles that are not replaced by anything
305-
var heads []string
306-
for _, entry := range entries {
307-
if !replaced[entry.Name] {
308-
heads = append(heads, entry.Name)
309-
}
310-
}
311-
312-
if len(heads) == 0 {
313-
return "", fmt.Errorf("channel has circular replaces chain, no head found")
314-
}
315-
if len(heads) > 1 {
316-
return "", fmt.Errorf("channel has multiple heads: %v", heads)
317-
}
318-
319-
return heads[0], nil
320-
}
321-
322286
func bundleReplaces(b cBundle, name string) bool {
323287
if b.Replaces == name {
324288
return true

pkg/cache/pogrebv1_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func TestPogrebV1_StableDigest(t *testing.T) {
2828
//
2929
// If validFS needs to change DO NOT CHANGE the json cache implementation
3030
// in the same pull request.
31-
require.Equal(t, "3cc6655887a2d5ce", actualDigest)
31+
require.Equal(t, "485a767449dd66d4", actualDigest)
3232
}
3333

3434
func TestPogrebV1_CheckIntegrity(t *testing.T) {

0 commit comments

Comments
 (0)