Skip to content

Commit 03ab15f

Browse files
rashmigottipatirashmigottipati
committed
bugfix and additional tests
Signed-off-by: Rashmi Gottipati <[email protected]>
1 parent 7ed36fd commit 03ab15f

File tree

3 files changed

+96
-48
lines changed

3 files changed

+96
-48
lines changed

‎internal/cmd/internal/olmv1/catalog_update.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,15 @@ func NewCatalogUpdateCmd(cfg *action.Configuration) *cobra.Command {
2323
Args: cobra.ExactArgs(1),
2424
Run: func(cmd *cobra.Command, args []string) {
2525
i.CatalogName = args[0]
26-
i.Priority = &priority
27-
i.PollIntervalMinutes = &pollInterval
28-
i.Labels = labels
26+
if cmd.Flags().Changed("priority") {
27+
i.Priority = &priority
28+
}
29+
if cmd.Flags().Changed("source-poll-interval-minutes") {
30+
i.PollIntervalMinutes = &pollInterval
31+
}
32+
if cmd.Flags().Changed("labels") {
33+
i.Labels = labels
34+
}
2935
_, err := i.Run(cmd.Context())
3036
if err != nil {
3137
log.Fatalf("failed to update catalog: %v", err)
@@ -38,6 +44,7 @@ func NewCatalogUpdateCmd(cfg *action.Configuration) *cobra.Command {
3844
cmd.Flags().StringToStringVar(&labels, "labels", map[string]string{}, "labels that will be added to the catalog")
3945
cmd.Flags().StringVar(&i.AvailabilityMode, "availability-mode", "", "available means that the catalog should be active and serving data")
4046
cmd.Flags().StringVar(&i.ImageRef, "image", "", "Image reference for the catalog source. Leave unset to retain the current image.")
47+
cmd.Flags().BoolVar(&i.IgnoreUnset, "ignore-unset", true, "when enabled, any unset flag value will not be changed. Disabling means that for each unset value a default will be used instead")
4148

4249
return cmd
4350
}

‎internal/pkg/v1/action/catalog_update.go

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ type CatalogUpdate struct {
2121
Labels map[string]string
2222
AvailabilityMode string
2323
ImageRef string
24+
IgnoreUnset bool
2425

2526
Logf func(string, ...interface{})
2627
}
@@ -52,13 +53,15 @@ func (cu *CatalogUpdate) Run(ctx context.Context) (*olmv1.ClusterCatalog, error)
5253
return nil, fmt.Errorf("invalid image reference: %q, it must be a valid image reference format", cu.ImageRef)
5354
}
5455

55-
cu.setDefaults(catalog)
56+
cu.setDefaults(&catalog)
5657

5758
cu.setUpdatedCatalog(&catalog)
5859
if err := cu.config.Client.Update(ctx, &catalog); err != nil {
5960
return nil, err
6061
}
6162

63+
cu.Logf("Updating catalog %q in namespace %q", cu.CatalogName, cu.config.Namespace)
64+
6265
return &catalog, nil
6366
}
6467

@@ -67,11 +70,10 @@ func (cu *CatalogUpdate) setUpdatedCatalog(catalog *olmv1.ClusterCatalog) {
6770
if existingLabels == nil {
6871
existingLabels = make(map[string]string)
6972
}
70-
7173
if cu.Labels != nil {
7274
for k, v := range cu.Labels {
7375
if v == "" {
74-
delete(existingLabels, k) // remove keys with empty values
76+
delete(existingLabels, k)
7577
} else {
7678
existingLabels[k] = v
7779
}
@@ -83,32 +85,39 @@ func (cu *CatalogUpdate) setUpdatedCatalog(catalog *olmv1.ClusterCatalog) {
8385
catalog.Spec.Priority = *cu.Priority
8486
}
8587

86-
if catalog.Spec.Source.Image != nil {
87-
if cu.PollIntervalMinutes != nil {
88-
// Set PollIntervalMinutes to the value if it's not nil
89-
catalog.Spec.Source.Image.PollIntervalMinutes = cu.PollIntervalMinutes
90-
} else {
91-
// If it's nil, explicitly unset it
88+
if catalog.Spec.Source.Image == nil {
89+
catalog.Spec.Source.Image = &olmv1.ImageSource{}
90+
}
91+
92+
if cu.PollIntervalMinutes != nil {
93+
if *cu.PollIntervalMinutes == 0 || *cu.PollIntervalMinutes == -1 {
9294
catalog.Spec.Source.Image.PollIntervalMinutes = nil
95+
} else {
96+
catalog.Spec.Source.Image.PollIntervalMinutes = cu.PollIntervalMinutes
9397
}
98+
}
9499

95-
if cu.ImageRef != "" {
96-
catalog.Spec.Source.Image.Ref = cu.ImageRef
97-
}
100+
if cu.ImageRef != "" {
101+
catalog.Spec.Source.Image.Ref = cu.ImageRef
98102
}
99103

100104
if cu.AvailabilityMode != "" {
101105
catalog.Spec.AvailabilityMode = olmv1.AvailabilityMode(cu.AvailabilityMode)
102106
}
103107
}
104108

105-
func (cu *CatalogUpdate) setDefaults(catalog olmv1.ClusterCatalog) {
109+
func (cu *CatalogUpdate) setDefaults(catalog *olmv1.ClusterCatalog) {
110+
if !cu.IgnoreUnset {
111+
return
112+
}
113+
106114
catalogSrc := catalog.Spec.Source
107115

108-
if cu.PollIntervalMinutes != nil && (*cu.PollIntervalMinutes == 0 || *cu.PollIntervalMinutes == -1) {
109-
cu.PollIntervalMinutes = nil
110-
} else if cu.PollIntervalMinutes == nil && catalogSrc.Image != nil && catalogSrc.Image.PollIntervalMinutes != nil {
111-
// Only default if user didn’t explicitly set anything
116+
if cu.Priority == nil {
117+
cu.Priority = &catalog.Spec.Priority
118+
}
119+
120+
if cu.PollIntervalMinutes == nil && catalogSrc.Image != nil && catalogSrc.Image.PollIntervalMinutes != nil {
112121
cu.PollIntervalMinutes = catalogSrc.Image.PollIntervalMinutes
113122
}
114123

@@ -118,15 +127,13 @@ func (cu *CatalogUpdate) setDefaults(catalog olmv1.ClusterCatalog) {
118127
if cu.AvailabilityMode == "" {
119128
cu.AvailabilityMode = string(catalog.Spec.AvailabilityMode)
120129
}
121-
if cu.Priority == nil {
122-
cu.Priority = &catalog.Spec.Priority
123-
}
124130
if len(cu.Labels) == 0 {
125131
cu.Labels = catalog.Labels
126132
}
127133
}
128134

129135
func isValidImageRef(imageRef string) bool {
130-
re := regexp.MustCompile(`^[a-zA-Z0-9-_\.]+(?:\/[a-zA-Z0-9-_\.]+)+(:[a-zA-Z0-9-_\.]+)?$`)
131-
return re.MatchString(imageRef)
136+
var imageRefRegex = regexp.MustCompile(`^([a-z0-9]+(\.[a-z0-9]+)*(:[0-9]+)?/)?[a-z0-9-_]+(/[a-z0-9-_]+)*(:[a-zA-Z0-9_\.-]+)?(@sha256:[a-fA-F0-9]{64})?$`)
137+
138+
return imageRefRegex.MatchString(imageRef)
132139
}

‎internal/pkg/v1/action/catalog_update_test.go

Lines changed: 57 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -160,29 +160,6 @@ var _ = Describe("CatalogUpdate", func() {
160160
Expect(err.Error()).To(ContainSubstring("invalid image reference"))
161161
})
162162

163-
It("preserves existing catalog values if Priority and PollIntervalMinutes are nil", func() {
164-
testCatalog := buildCatalog(
165-
"test",
166-
withCatalogSourceType(olmv1.SourceTypeImage),
167-
withCatalogPollInterval(pointerToInt(15)),
168-
withCatalogSourcePriority(pointerToInt32(10)),
169-
withCatalogImageRef("quay.io/myrepo/image"),
170-
)
171-
172-
cfg := setupEnv(testCatalog)
173-
174-
updater := internalaction.NewCatalogUpdate(&cfg)
175-
updater.CatalogName = "test"
176-
updater.Priority = nil
177-
updater.PollIntervalMinutes = nil
178-
179-
catalog, err := updater.Run(context.TODO())
180-
181-
Expect(err).NotTo(HaveOccurred())
182-
Expect(catalog.Spec.Priority).To(Equal(int32(10)))
183-
Expect(*catalog.Spec.Source.Image.PollIntervalMinutes).To(Equal(15))
184-
})
185-
186163
It("removes labels with empty values and merges the rest", func() {
187164
initial := map[string]string{"foo": "bar", "remove": "yes"}
188165
testCatalog := buildCatalog(
@@ -224,6 +201,63 @@ var _ = Describe("CatalogUpdate", func() {
224201
Expect(catalog.Labels).To(Equal(map[string]string{"retain": "this"}))
225202
})
226203

204+
It("preserves priority and poll interval when ignoreUnset flag is true and flags not explicitly set", func() {
205+
testCatalog := buildCatalog(
206+
"test",
207+
withCatalogSourceType(olmv1.SourceTypeImage),
208+
withCatalogPollInterval(pointerToInt(10)),
209+
withCatalogSourcePriority(pointerToInt32(3)),
210+
withCatalogImageRef("quay.io/myrepo/image"),
211+
withCatalogLabels(map[string]string{"foo": "bar"}),
212+
)
213+
214+
cfg := setupEnv(testCatalog)
215+
216+
updater := internalaction.NewCatalogUpdate(&cfg)
217+
updater.CatalogName = "test"
218+
updater.IgnoreUnset = true
219+
220+
catalog, err := updater.Run(context.TODO())
221+
Expect(err).NotTo(HaveOccurred())
222+
Expect(catalog).NotTo(BeNil())
223+
224+
Expect(catalog.Spec.Priority).To(Equal(int32(3)))
225+
Expect(catalog.Spec.Source.Image).NotTo(BeNil())
226+
Expect(catalog.Labels).To(Equal(map[string]string{"foo": "bar"}))
227+
Expect(catalog.Spec.Source.Image.PollIntervalMinutes).NotTo(BeNil())
228+
Expect(*catalog.Spec.Source.Image.PollIntervalMinutes).To(Equal(10))
229+
230+
})
231+
232+
It("resets priority and poll interval when ignoreUnset is false and flags are nil", func() {
233+
testCatalog := buildCatalog(
234+
"test",
235+
withCatalogSourceType(olmv1.SourceTypeImage),
236+
withCatalogPollInterval(pointerToInt(10)),
237+
withCatalogSourcePriority(pointerToInt32(3)),
238+
withCatalogImageRef("quay.io/myrepo/image"),
239+
)
240+
241+
cfg := setupEnv(testCatalog)
242+
243+
updater := internalaction.NewCatalogUpdate(&cfg)
244+
updater.CatalogName = "test"
245+
updater.IgnoreUnset = false
246+
updater.Priority = nil
247+
updater.PollIntervalMinutes = nil
248+
updater.ImageRef = ""
249+
updater.AvailabilityMode = ""
250+
251+
catalog, err := updater.Run(context.TODO())
252+
Expect(err).NotTo(HaveOccurred())
253+
254+
Expect(catalog.Spec.Priority).To(Equal(int32(3)))
255+
Expect(catalog.Spec.Source.Image.Ref).To(Equal("quay.io/myrepo/image"))
256+
Expect(string(catalog.Spec.AvailabilityMode)).To(BeEmpty())
257+
Expect(catalog.Spec.Source.Image.PollIntervalMinutes).ToNot(BeNil())
258+
Expect(*catalog.Spec.Source.Image.PollIntervalMinutes).To(Equal(10))
259+
260+
})
227261
})
228262

229263
func pointerToInt32(i int32) *int32 {

0 commit comments

Comments
 (0)