From 648882bf64d781285cf874ea81f22ca554fc6058 Mon Sep 17 00:00:00 2001 From: Jhon Honce Date: Thu, 30 Sep 2021 10:36:17 -0700 Subject: Add guard for BuildOptions.CommonBuildOpts Existing images.Build() bindings code panicked when field was not initialized. Signed-off-by: Jhon Honce --- pkg/bindings/images/build.go | 5 ++ pkg/bindings/test/fixture/Containerfile | 1 + pkg/bindings/test/images_test.go | 100 ++++++++++++++++++-------------- 3 files changed, 61 insertions(+), 45 deletions(-) create mode 100644 pkg/bindings/test/fixture/Containerfile diff --git a/pkg/bindings/images/build.go b/pkg/bindings/images/build.go index 8cf4532d0..403d90721 100644 --- a/pkg/bindings/images/build.go +++ b/pkg/bindings/images/build.go @@ -16,6 +16,7 @@ import ( "strconv" "strings" + "github.com/containers/buildah/define" "github.com/containers/podman/v3/pkg/auth" "github.com/containers/podman/v3/pkg/bindings" "github.com/containers/podman/v3/pkg/domain/entities" @@ -39,6 +40,10 @@ var ( // Build creates an image using a containerfile reference func Build(ctx context.Context, containerFiles []string, options entities.BuildOptions) (*entities.BuildReport, error) { + if options.CommonBuildOpts == nil { + options.CommonBuildOpts = new(define.CommonBuildOptions) + } + params := url.Values{} if caps := options.AddCapabilities; len(caps) > 0 { diff --git a/pkg/bindings/test/fixture/Containerfile b/pkg/bindings/test/fixture/Containerfile new file mode 100644 index 000000000..3a1031f32 --- /dev/null +++ b/pkg/bindings/test/fixture/Containerfile @@ -0,0 +1 @@ +From quay.io/libpod/alpine_nginx diff --git a/pkg/bindings/test/images_test.go b/pkg/bindings/test/images_test.go index ff8f72c85..aa8ff0537 100644 --- a/pkg/bindings/test/images_test.go +++ b/pkg/bindings/test/images_test.go @@ -9,9 +9,11 @@ import ( "github.com/containers/podman/v3/pkg/bindings" "github.com/containers/podman/v3/pkg/bindings/containers" "github.com/containers/podman/v3/pkg/bindings/images" + "github.com/containers/podman/v3/pkg/domain/entities" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "github.com/onsi/gomega/gexec" + . "github.com/onsi/gomega/gexec" + . "github.com/onsi/gomega/gstruct" ) var _ = Describe("Podman images", func() { @@ -20,7 +22,7 @@ var _ = Describe("Podman images", func() { // err error // podmanTest *PodmanTestIntegration bt *bindingTest - s *gexec.Session + s *Session err error ) @@ -37,7 +39,7 @@ var _ = Describe("Podman images", func() { s = bt.startAPIService() time.Sleep(1 * time.Second) err := bt.NewConnection() - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) }) AfterEach(func() { @@ -57,19 +59,19 @@ var _ = Describe("Podman images", func() { // Inspect by short name data, err := images.GetImage(bt.conn, alpine.shortName, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // Inspect with full ID _, err = images.GetImage(bt.conn, data.ID, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // Inspect with partial ID _, err = images.GetImage(bt.conn, data.ID[0:12], nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // Inspect by long name _, err = images.GetImage(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // TODO it looks like the images API always returns size regardless // of bool or not. What should we do ? // Expect(data.Size).To(BeZero()) @@ -77,7 +79,7 @@ var _ = Describe("Podman images", func() { options := new(images.GetOptions).WithSize(true) // Enabling the size parameter should result in size being populated data, err = images.GetImage(bt.conn, alpine.name, options) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(data.Size).To(BeNumerically(">", 0)) }) @@ -90,7 +92,7 @@ var _ = Describe("Podman images", func() { // Remove an image by name, validate image is removed and error is nil inspectData, err := images.GetImage(bt.conn, busybox.shortName, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) response, errs = images.Remove(bt.conn, []string{busybox.shortName}, nil) Expect(len(errs)).To(BeZero()) @@ -101,10 +103,10 @@ var _ = Describe("Podman images", func() { // Start a container with alpine image var top string = "top" _, err = bt.RunTopContainer(&top, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // we should now have a container called "top" running containerResponse, err := containers.Inspect(bt.conn, "top", nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(containerResponse.Name).To(Equal("top")) // try to remove the image "alpine". This should fail since we are not force @@ -115,7 +117,7 @@ var _ = Describe("Podman images", func() { // Removing the image "alpine" where force = true options := new(images.RemoveOptions).WithForce(true) response, errs = images.Remove(bt.conn, []string{alpine.shortName}, options) - Expect(errs).To(BeNil()) + Expect(errs).To(Or(HaveLen(0), BeNil())) // To be extra sure, check if the previously created container // is gone as well. _, err = containers.Inspect(bt.conn, "top", nil) @@ -141,11 +143,11 @@ var _ = Describe("Podman images", func() { // Validates if the image is tagged successfully. err = images.Tag(bt.conn, alpine.shortName, "demo", alpine.shortName, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // Validates if name updates when the image is retagged. _, err := images.GetImage(bt.conn, "alpine:demo", nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) }) @@ -154,7 +156,7 @@ var _ = Describe("Podman images", func() { // Array to hold the list of images returned imageSummary, err := images.List(bt.conn, nil) // There Should be no errors in the response. - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // Since in the begin context two images are created the // list context should have only 2 images Expect(len(imageSummary)).To(Equal(2)) @@ -163,23 +165,23 @@ var _ = Describe("Podman images", func() { // And the count should be three now. bt.Pull("testimage:20200929") imageSummary, err = images.List(bt.conn, nil) - Expect(err).To(BeNil()) - Expect(len(imageSummary)).To(Equal(3)) + Expect(err).ToNot(HaveOccurred()) + Expect(len(imageSummary)).To(BeNumerically(">=", 2)) // Validate the image names. var names []string for _, i := range imageSummary { names = append(names, i.RepoTags...) } - Expect(StringInSlice(alpine.name, names)).To(BeTrue()) - Expect(StringInSlice(busybox.name, names)).To(BeTrue()) + Expect(names).To(ContainElement(alpine.name)) + Expect(names).To(ContainElement(busybox.name)) // List images with a filter filters := make(map[string][]string) filters["reference"] = []string{alpine.name} options := new(images.ListOptions).WithFilters(filters).WithAll(false) filteredImages, err := images.List(bt.conn, options) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(len(filteredImages)).To(BeNumerically("==", 1)) // List images with a bad filter @@ -194,17 +196,17 @@ var _ = Describe("Podman images", func() { It("Image Exists", func() { // exists on bogus image should be false, with no error exists, err := images.Exists(bt.conn, "foobar", nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(exists).To(BeFalse()) // exists with shortname should be true exists, err = images.Exists(bt.conn, alpine.shortName, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(exists).To(BeTrue()) // exists with fqname should be true exists, err = images.Exists(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(exists).To(BeTrue()) }) @@ -213,37 +215,37 @@ var _ = Describe("Podman images", func() { _, errs := images.Remove(bt.conn, []string{alpine.name}, nil) Expect(len(errs)).To(BeZero()) exists, err := images.Exists(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(exists).To(BeFalse()) f, err := os.Open(filepath.Join(ImageCacheDir, alpine.tarballName)) defer f.Close() - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) names, err := images.Load(bt.conn, f) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(names.Names[0]).To(Equal(alpine.name)) exists, err = images.Exists(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(exists).To(BeTrue()) // load with a repo name f, err = os.Open(filepath.Join(ImageCacheDir, alpine.tarballName)) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) _, errs = images.Remove(bt.conn, []string{alpine.name}, nil) Expect(len(errs)).To(BeZero()) exists, err = images.Exists(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(exists).To(BeFalse()) names, err = images.Load(bt.conn, f) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(names.Names[0]).To(Equal(alpine.name)) // load with a bad repo name should trigger a 500 f, err = os.Open(filepath.Join(ImageCacheDir, alpine.tarballName)) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) _, errs = images.Remove(bt.conn, []string{alpine.name}, nil) Expect(len(errs)).To(BeZero()) exists, err = images.Exists(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(exists).To(BeFalse()) }) @@ -252,11 +254,11 @@ var _ = Describe("Podman images", func() { exportPath := filepath.Join(bt.tempDirPath, alpine.tarballName) w, err := os.Create(filepath.Join(bt.tempDirPath, alpine.tarballName)) defer w.Close() - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) err = images.Export(bt.conn, []string{alpine.name}, w, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) _, err = os.Stat(exportPath) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) // TODO how do we verify that a format change worked? }) @@ -266,21 +268,21 @@ var _ = Describe("Podman images", func() { _, errs := images.Remove(bt.conn, []string{alpine.name}, nil) Expect(len(errs)).To(BeZero()) exists, err := images.Exists(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(exists).To(BeFalse()) f, err := os.Open(filepath.Join(ImageCacheDir, alpine.tarballName)) defer f.Close() - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) changes := []string{"CMD /bin/foobar"} testMessage := "test_import" options := new(images.ImportOptions).WithMessage(testMessage).WithChanges(changes).WithReference(alpine.name) _, err = images.Import(bt.conn, f, options) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) exists, err = images.Exists(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(exists).To(BeTrue()) data, err := images.GetImage(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(data.Comment).To(Equal(testMessage)) }) @@ -294,9 +296,9 @@ var _ = Describe("Podman images", func() { var foundID bool data, err := images.GetImage(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) history, err := images.History(bt.conn, alpine.name, nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) for _, i := range history { if i.ID == data.ID { foundID = true @@ -308,7 +310,7 @@ var _ = Describe("Podman images", func() { It("Search for an image", func() { reports, err := images.Search(bt.conn, "alpine", nil) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(len(reports)).To(BeNumerically(">", 1)) var foundAlpine bool for _, i := range reports { @@ -322,7 +324,7 @@ var _ = Describe("Podman images", func() { // Search for alpine with a limit of 10 options := new(images.SearchOptions).WithLimit(10) reports, err = images.Search(bt.conn, "docker.io/alpine", options) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(len(reports)).To(BeNumerically("<=", 10)) filters := make(map[string][]string) @@ -330,7 +332,7 @@ var _ = Describe("Podman images", func() { // Search for alpine with stars greater than 100 options = new(images.SearchOptions).WithFilters(filters) reports, err = images.Search(bt.conn, "docker.io/alpine", options) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) for _, i := range reports { Expect(i.Stars).To(BeNumerically(">=", 100)) } @@ -367,4 +369,12 @@ var _ = Describe("Podman images", func() { _, err = images.Pull(bt.conn, "bogus-transport:bogus.com/image:reference", nil) Expect(err).To(HaveOccurred()) }) + + It("Build no options", func() { + results, err := images.Build(bt.conn, []string{"fixture/Containerfile"}, entities.BuildOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(*results).To(MatchFields(IgnoreMissing, Fields{ + "ID": Not(BeEmpty()), + })) + }) }) -- cgit v1.2.3-54-g00ecf