Add guard for BuildOptions.CommonBuildOpts

Existing images.Build() bindings code panicked when field was not
initialized.

Signed-off-by: Jhon Honce <jhonce@redhat.com>

3 files changed, 61 insertions, 45 deletions

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()),
+		}))
+	})
 })