Skip to content

Commit e22d2ab

Browse files
author
Eric Stroczynski
authored
[v0.18.x] deps: bump operator-registry version (#3221) (#3284)
This version of operator-registry contains a fix for a bug that causes validation errors when an existing `annotations.yaml` does not exactly match the default annotations generated by `operator-sdk bundle create`. deps: bump operator-registry cmd/operator-sdk/bundle,internal/scorecard/alpha/tests: the `NewImageValidator` API changed such that the command must create an image registry first before creating a validator.
1 parent 72f19f0 commit e22d2ab

File tree

5 files changed

+84
-34
lines changed

5 files changed

+84
-34
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
entries:
2+
- description: >
3+
Fix bug in `operator-sdk bundle validation` that causes erroneous validation errors when the number of
4+
annotations in an existing `annotations.yaml` does not equal the number of default bundle annotations
5+
by upgrading the `operator-registry` dependency.
6+
kind: bugfix

cmd/operator-sdk/bundle/validate.go

Lines changed: 61 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ import (
2323

2424
apimanifests "github.com/operator-framework/api/pkg/manifests"
2525
apierrors "github.com/operator-framework/api/pkg/validation/errors"
26+
"github.com/operator-framework/operator-registry/pkg/containertools"
27+
registryimage "github.com/operator-framework/operator-registry/pkg/image"
28+
"github.com/operator-framework/operator-registry/pkg/image/execregistry"
2629
registrybundle "github.com/operator-framework/operator-registry/pkg/lib/bundle"
2730
log "github.com/sirupsen/logrus"
2831
"github.com/spf13/cobra"
@@ -96,30 +99,10 @@ To build and validate an image:
9699
return fmt.Errorf("invalid command args: %v", err)
97100
}
98101

99-
// If the argument isn't a directory, assume it's an image.
100-
if isExist(args[0]) {
101-
if c.directory, err = relWd(args[0]); err != nil {
102-
logger.Fatal(err)
103-
}
104-
} else {
105-
c.directory, err = ioutil.TempDir("", "bundle-")
106-
if err != nil {
107-
return err
108-
}
109-
defer func() {
110-
if err = os.RemoveAll(c.directory); err != nil {
111-
logger.Errorf("Error removing temp bundle dir: %v", err)
112-
}
113-
}()
114-
115-
logger.Info("Unpacking image layers")
116-
117-
if err := c.unpackImageIntoDir(args[0], c.directory); err != nil {
118-
logger.Fatalf("Error unpacking image %s: %v", args[0], err)
119-
}
102+
result, err := c.run(logger, args[0])
103+
if err != nil {
104+
logger.Fatal(err)
120105
}
121-
122-
result := c.run()
123106
if err := result.PrintWithFormat(c.outputFormat); err != nil {
124107
logger.Fatal(err)
125108
}
@@ -152,25 +135,59 @@ func (c *bundleValidateCmd) addToFlagSet(fs *pflag.FlagSet) {
152135
fs.StringVarP(&c.imageBuilder, "image-builder", "b", "docker",
153136
"Tool to extract bundle image data. Only used when validating a bundle image. "+
154137
"One of: [docker, podman]")
138+
155139
fs.StringVarP(&c.outputFormat, "output", "o", internal.Text,
156140
"Result format for results. One of: [text, json-alpha1]")
157-
158141
// It is hidden because it is an alpha option
159142
// The idea is the next versions of Operator Registry will return a List of errors
160143
if err := fs.MarkHidden("output"); err != nil {
161144
panic(err)
162145
}
163146
}
164147

165-
func (c bundleValidateCmd) run() (res internal.Result) {
148+
func (c bundleValidateCmd) run(logger *log.Entry, bundle string) (res internal.Result, err error) {
149+
// Create a registry to validate bundle files and optionally unpack the image with.
150+
reg, err := newImageRegistryForTool(logger, c.imageBuilder)
151+
if err != nil {
152+
return res, fmt.Errorf("error creating image registry: %v", err)
153+
}
154+
defer func() {
155+
if err := reg.Destroy(); err != nil {
156+
logger.Errorf("Error destroying image registry: %v", err)
157+
}
158+
}()
159+
160+
// If bundle isn't a directory, assume it's an image.
161+
if isExist(bundle) {
162+
if c.directory, err = relWd(bundle); err != nil {
163+
return res, err
164+
}
165+
} else {
166+
c.directory, err = ioutil.TempDir("", "bundle-")
167+
if err != nil {
168+
return res, err
169+
}
170+
defer func() {
171+
if err = os.RemoveAll(c.directory); err != nil {
172+
logger.Errorf("Error removing temp bundle dir: %v", err)
173+
}
174+
}()
175+
176+
logger.Info("Unpacking image layers")
177+
178+
if err := c.unpackImageIntoDir(reg, bundle, c.directory); err != nil {
179+
return res, fmt.Errorf("error unpacking image %s: %v", bundle, err)
180+
}
181+
}
182+
166183
// Create Result to be outputted
167184
res = internal.NewResult()
168185

169-
logger := log.WithFields(log.Fields{
186+
logger = logger.WithFields(log.Fields{
170187
"bundle-dir": c.directory,
171188
"container-tool": c.imageBuilder,
172189
})
173-
val := registrybundle.NewImageValidator(c.imageBuilder, logger)
190+
val := registrybundle.NewImageValidator(reg, logger)
174191

175192
// Validate bundle format.
176193
if err := val.ValidateBundleFormat(c.directory); err != nil {
@@ -190,16 +207,30 @@ func (c bundleValidateCmd) run() (res internal.Result) {
190207
// from the ValidateBundleContent to add the output(s) into the result
191208
checkResults(results, &res)
192209

193-
return res
210+
return res, nil
211+
}
212+
213+
// newImageRegistryForTool returns an image registry based on what type of image tool is passed.
214+
// If toolStr is empty, a containerd registry is returned.
215+
func newImageRegistryForTool(logger *log.Entry, toolStr string) (reg registryimage.Registry, err error) {
216+
switch toolStr {
217+
case containertools.DockerTool.String():
218+
reg, err = execregistry.NewRegistry(containertools.DockerTool, logger)
219+
case containertools.PodmanTool.String():
220+
reg, err = execregistry.NewRegistry(containertools.PodmanTool, logger)
221+
default:
222+
err = fmt.Errorf("unrecognized image-builder option: %s", toolStr)
223+
}
224+
return reg, err
194225
}
195226

196227
// unpackImageIntoDir writes files in image layers found in image imageTag to dir.
197-
func (c bundleValidateCmd) unpackImageIntoDir(imageTag, dir string) error {
228+
func (c bundleValidateCmd) unpackImageIntoDir(reg registryimage.Registry, imageTag, dir string) error {
198229
logger := log.WithFields(log.Fields{
199230
"bundle-dir": dir,
200231
"container-tool": c.imageBuilder,
201232
})
202-
val := registrybundle.NewImageValidator(c.imageBuilder, logger)
233+
val := registrybundle.NewImageValidator(reg, logger)
203234

204235
return val.PullBundleImage(imageTag, dir)
205236
}

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ require (
1717
github.com/onsi/ginkgo v1.12.0
1818
github.com/onsi/gomega v1.9.0
1919
github.com/operator-framework/api v0.3.7-0.20200602203552-431198de9fc2
20-
github.com/operator-framework/operator-registry v1.12.6-0.20200605115407-01fa069730e2
20+
github.com/operator-framework/operator-registry v1.12.6-0.20200611222234-275301b779f8
2121
github.com/pborman/uuid v1.2.0
2222
github.com/pkg/errors v0.9.1
2323
github.com/prometheus/client_golang v1.5.1

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -735,8 +735,8 @@ github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFSt
735735
github.com/openzipkin/zipkin-go v0.1.6/go.mod h1:QgAqvLzwWbR/WpD4A3cGpPtJrZXNIiJc5AZX7/PBEpw=
736736
github.com/operator-framework/api v0.3.7-0.20200602203552-431198de9fc2 h1:2KtDe3jI6ftXGj5M875WVvv6pBIk4K9DyrwPuE+XfOc=
737737
github.com/operator-framework/api v0.3.7-0.20200602203552-431198de9fc2/go.mod h1:Xbje9x0SHmh0nihE21kpesB38vk3cyxnE6JdDS8Jo1Q=
738-
github.com/operator-framework/operator-registry v1.12.6-0.20200605115407-01fa069730e2 h1:G7helxG2m7xgIsn4KX5qu62mqbgjK9kVUZC3AHiPnOQ=
739-
github.com/operator-framework/operator-registry v1.12.6-0.20200605115407-01fa069730e2/go.mod h1:loVINznYhgBIkmv83kU4yee88RS0BBk+hqOw9r4bhJk=
738+
github.com/operator-framework/operator-registry v1.12.6-0.20200611222234-275301b779f8 h1:F3zzxoBJJANdKMxmSOi5z/HWiVT+gwOdhROkEwDWD2M=
739+
github.com/operator-framework/operator-registry v1.12.6-0.20200611222234-275301b779f8/go.mod h1:loVINznYhgBIkmv83kU4yee88RS0BBk+hqOw9r4bhJk=
740740
github.com/otiai10/copy v1.2.0 h1:HvG945u96iNadPoG2/Ja2+AUJeW5YuFQMixq9yirC+k=
741741
github.com/otiai10/copy v1.2.0/go.mod h1:rrF5dJ5F0t/EWSYODDu4j9/vEeYHMkc8jt0zJChqQWw=
742742
github.com/otiai10/curr v0.0.0-20150429015615-9b4961190c95 h1:+OLn68pqasWca0z5ryit9KGfp3sUsW4Lqg32iRMJyzs=

internal/scorecard/alpha/tests/olm.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ import (
2323
apimanifests "github.com/operator-framework/api/pkg/manifests"
2424
operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
2525
apivalidation "github.com/operator-framework/api/pkg/validation"
26+
"github.com/operator-framework/operator-registry/pkg/containertools"
27+
"github.com/operator-framework/operator-registry/pkg/image/execregistry"
2628
registrybundle "github.com/operator-framework/operator-registry/pkg/lib/bundle"
2729
"github.com/sirupsen/logrus"
2830
apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -61,7 +63,18 @@ func BundleValidationTest(dir string) scapiv1alpha2.ScorecardTestResult {
6163
logrus.SetLevel(logrus.DebugLevel)
6264
logrus.SetOutput(buf)
6365

64-
val := registrybundle.NewImageValidator("", logger)
66+
// Despite NewRegistry appearing to create a docker image registry, all this function does is return a type
67+
// that shells out to the docker client binary. Since BundleValidationTest only ever calls ValidateBundleFormat,
68+
// which does not use the underlying registry, we can use this object as a dummy registry. We shouldn't
69+
// use the containerd registry because that actually creates an underlying registry.
70+
// NB(estroz): previously NewImageValidator constructed a docker registry internally, which is what we've done
71+
// here. However it might be nice to create a mock registry that returns an error if any method is called.
72+
reg, err := execregistry.NewRegistry(containertools.DockerTool, logger)
73+
if err != nil {
74+
// This function should never return an error since it's wrapping the docker client binary in a struct.
75+
logger.Fatalf("Scorecard: this docker registry error should never occur: %v", err)
76+
}
77+
val := registrybundle.NewImageValidator(reg, logger)
6578

6679
// Validate bundle format.
6780
if err := val.ValidateBundleFormat(dir); err != nil {

0 commit comments

Comments
 (0)