Skip to content

Commit

Permalink
Add guard for BuildOptions.CommonBuildOpts
Browse files Browse the repository at this point in the history 
Existing images.Build() bindings code panicked when field was not
initialized.

Signed-off-by: Jhon Honce <[email protected]>
  • Loading branch information
@jwhonce @mheon
jwhonce authored and mheon committed Oct 19, 2021
1 parent c6be714 commit 62d12a2
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 45 deletions.
5 changes: 5 additions & 0 deletions pkg/bindings/images/build.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions pkg/bindings/test/fixture/Containerfile
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
From quay.io/libpod/alpine_nginx
100 changes: 55 additions & 45 deletions pkg/bindings/test/images_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -20,7 +22,7 @@ var _ = Describe("Podman images", func() {
// err error
// podmanTest *PodmanTestIntegration
bt *bindingTest
s *gexec.Session
s *Session
err error
)

Expand All @@ -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() {
Expand All @@ -57,27 +59,27 @@ 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())

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))
})

Expand All @@ -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())

Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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())

})

Expand All @@ -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))
Expand All @@ -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
Expand All @@ -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())
})

Expand All @@ -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())
})

Expand All @@ -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?
})
Expand All @@ -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))

})
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -322,15 +324,15 @@ 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)
filters["stars"] = []string{"100"}
// 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))
}
Expand Down Expand Up @@ -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()),
}))
})
})

0 comments on commit 62d12a2

Please sign in to comment.