Updated to @rickard-von-essen's code review suggestions, including:

- filter build and error checking in Prepare stage (multiErr created in the original function will be returned to Prepare and appended, so all errors show).
- source_image overrides source_image_filter.
- Doc edit

builder/openstack/builder.go

@@ -87,9 +87,10 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			SSHAgentAuth:         b.config.RunConfig.Comm.SSHAgentAuth,
 		},
 		&StepSourceImageInfo{
-			SourceImage:     b.config.SourceImage,
-			SourceImageName: b.config.SourceImageName,
-			ImageFilters:    b.config.SourceImageFilters,
+			SourceImage:      b.config.SourceImage,
+			SourceImageName:  b.config.SourceImageName,
+			SourceImageOpts:  b.config.SourceImageOpts,
+			SourceMostRecent: b.config.SourceImageFilters.MostRecent,
 		},
 		&StepCreateVolume{
 			UseBlockStorageVolume:  b.config.UseBlockStorageVolume,

builder/openstack/run_config.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 
+	"github.com/gophercloud/gophercloud/openstack/imageservice/v2/images"
 	"github.com/hashicorp/packer/common/uuid"
 	"github.com/hashicorp/packer/helper/communicator"
 	"github.com/hashicorp/packer/template/interpolate"
@@ -21,6 +22,7 @@ type RunConfig struct {
 	SourceImage        string             `mapstructure:"source_image"`
 	SourceImageName    string             `mapstructure:"source_image_name"`
 	SourceImageFilters ImageFilterOptions `mapstructure:"source_image_filter"`
+	SourceImageOpts    images.ListOpts    `mapstructure:""`
 	Flavor             string             `mapstructure:"flavor"`
 	AvailabilityZone   string             `mapstructure:"availability_zone"`
 	RackconnectWait    bool               `mapstructure:"rackconnect_wait"`
@@ -112,5 +114,23 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error {
 		}
 	}
 
+	// if neither ID or image name is provided outside the filter, build the filter
+	if len(c.SourceImage) == 0 && len(c.SourceImageName) == 0 {
+		params := &images.ListOpts{}
+
+		if len(c.SourceImageFilters.Filters) > 0 {
+			filterErrs := buildImageFilters(c.SourceImageFilters.Filters, params)
+			if len(filterErrs.Errors) > 0 {
+				errs = append(errs, filterErrs.Errors...)
+			}
+		}
+
+		if c.SourceImageFilters.MostRecent {
+			applyMostRecent(params)
+		}
+
+		c.SourceImageOpts = *params
+	}
+
 	return errs
 }

builder/openstack/step_source_image_info.go

@@ -12,9 +12,10 @@ import (
 )
 
 type StepSourceImageInfo struct {
-	SourceImage     string
-	SourceImageName string
-	ImageFilters    ImageFilterOptions
+	SourceImage      string
+	SourceImageName  string
+	SourceImageOpts  images.ListOpts
+	SourceMostRecent bool
 }
 
 type ImageFilterOptions struct {
@@ -28,38 +29,9 @@ func (s *StepSourceImageInfo) Run(_ context.Context, state multistep.StateBag) m
 
 	client, err := config.imageV2Client()
 
-	// if an ID is provided we skip the filter since that will return a single or no image
-	if s.SourceImage != "" {
-		state.Put("source_image", s.SourceImage)
-		return multistep.ActionContinue
-	}
-
-	params := &images.ListOpts{}
-
-	// build ListOpts from filters
-	if len(s.ImageFilters.Filters) > 0 {
-		errs := buildImageFilters(s.ImageFilters.Filters, params)
-		if len(errs.Errors) > 0 {
-			err := fmt.Errorf("Errors encountered in filter parsing.\n%s" + errs.Error())
-			state.Put("error", err)
-			ui.Error(errs.Error())
-			return multistep.ActionHalt
-		}
-	}
-
-	// override the "name" provided in filters if "s.SourceImageName" was filled
-	if s.SourceImageName != "" {
-		params.Name = s.SourceImageName
-	}
-
-	// apply "most_recent" logic to the sort fields and allow OpenStack to return the latest qualified image
-	if s.ImageFilters.MostRecent {
-		applyMostRecent(params)
-	}
-
-	log.Printf("Using Image Filters %v", params)
+	log.Printf("Using Image Filters %v", s.SourceImageOpts)
 	image := &images.Image{}
-	err = images.List(client, params).EachPage(func(page pagination.Page) (bool, error) {
+	err = images.List(client, s.SourceImageOpts).EachPage(func(page pagination.Page) (bool, error) {
 		i, err := images.ExtractImages(page)
 		if err != nil {
 			return false, err
@@ -67,18 +39,18 @@ func (s *StepSourceImageInfo) Run(_ context.Context, state multistep.StateBag) m
 
 		switch len(i) {
 		case 0:
-			return false, fmt.Errorf("No image was found matching filters: %v", params)
+			return false, fmt.Errorf("No image was found matching filters: %v", s.SourceImageOpts)
 		case 1:
 			*image = i[0]
 			return true, nil
 		default:
-			if s.ImageFilters.MostRecent {
+			if s.SourceMostRecent {
 				*image = i[0]
 				return true, nil
 			}
 			return false, fmt.Errorf(
 				"Your query returned more than one result. Please try a more specific search, or set most_recent to true. Search filters: %v",
-				params)
+				s.SourceImageOpts)
 		}
 	})
 

website/source/docs/builders/openstack.html.md

@@ -70,6 +70,11 @@ builder.
     is an alternative way of providing `source_image` and only either of them
     can be specified.
 
+-   `source_image_filter` (map) - The search filters for determining the base
+    image to use. This is an alternative way of providing `source_image` and
+    only one of these methods can be used. `source_image` will override the
+    filters.
+
 -   `username` or `user_id` (string) - The username or id used to connect to
     the OpenStack service. If not specified, Packer will use the environment
     variable `OS_USERNAME` or `OS_USERID`, if set. This is not required if
@@ -176,7 +181,7 @@ builder.
                 "name": "ubuntu-16.04*",
                 "visibility": "protected",
                 "owner": "d1a588cf4b0743344508dc145649372d1",
-                "tag": ["prod", "ready"]
+                "tags": ["prod", "ready"]
             },
             "most_recent": true
         }
@@ -185,7 +190,8 @@ builder.
 
     This selects the most recent production Ubuntu 16.04 shared to you by the given owner.
     NOTE: This will fail unless *exactly* one image is returned, or `most_recent` is set to true.
-    In the example, `most_recent` will cause this to succeed by selecting the newest image.
+    In the example of multiple returned images, `most_recent` will cause this to succeed by selecting
+    the newest image of the returned images.
 
     -   `filters` (map of strings) - filters used to select a `source_image`.
         NOTE: This will fail unless *exactly* one image is returned, or `most_recent` is set to true.
@@ -196,18 +202,16 @@ builder.
 
         - owner (string)
 
-        - tags (slice of strings)
+        - tags (array of strings)
 
         - visibility (string)
 
     -   `most_recent` (boolean) - Selects the newest created image when true.
         This is most useful for selecting a daily distro build.
 
-    You may set this in place of `source_image` or in conjunction with it. If you
-    set this in conjunction with `source_image`, the `source_image` will be added to
-    the filter. The provided `source_image` must meet all of the filtering criteria
-    provided in `source_image_filter`; this pins the image returned by the filter,
-    but will cause Packer to fail if the `source_image` does not exist.
+    You may set use this in place of `source_image` If `source_image_filter` is provided
+    alongside `source_image`, the `source_image` will override the filter. The filter
+    will not be used in this case.
 
 -   `ssh_interface` (string) - The type of interface to connect via SSH. Values
     useful for Rackspace are "public" or "private", and the default behavior is