Fixed step interface, added step to builder, passing all tests for new image_query.go

builder/openstack/builder.go

@@ -86,6 +86,11 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			PrivateKeyFile:       b.config.RunConfig.Comm.SSHPrivateKey,
 			SSHAgentAuth:         b.config.RunConfig.Comm.SSHAgentAuth,
 		},
+		&StepSourceImageInfo{
+			SourceImage:     b.config.SourceImage,
+			SourceImageName: b.config.SourceImageName,
+			ImageFilters:    b.config.SourceImageFilters,
+		},
 		&StepCreateVolume{
 			UseBlockStorageVolume:  b.config.UseBlockStorageVolume,
 			SourceImage:            b.config.SourceImage,

builder/openstack/image_query.go

@@ -3,13 +3,13 @@ package openstack
 import (
 	"fmt"
 	"reflect"
+	"strconv"
 	"strings"
 	"time"
-	"strconv"
 
-	"github.com/hashicorp/packer/packer"
-	"github.com/hashicorp/go-multierror"
 	"github.com/gophercloud/gophercloud/openstack/imageservice/v2/images"
+	"github.com/hashicorp/go-multierror"
+	"github.com/hashicorp/packer/packer"
 )
 
 const (
@@ -35,7 +35,47 @@ func getDateFilter(s string) (images.ImageDateFilter, error) {
 	}
 
 	var badFilter images.ImageDateFilter
-	return badFilter, fmt.Errorf("No ImageDateFilter found for %s", s)
+	return badFilter, fmt.Errorf("No valid ImageDateFilter found for %s", s)
+}
+
+// Retrieve the specific ImageVisibility using the exported const from images
+func getImageVisibility(s string) (images.ImageVisibility, error) {
+	visibilities := [...]images.ImageVisibility{
+		images.ImageVisibilityPublic,
+		images.ImageVisibilityPrivate,
+		images.ImageVisibilityCommunity,
+		images.ImageVisibilityShared,
+	}
+
+	for _, visibility := range visibilities {
+		if string(visibility) == s {
+			return visibility, nil
+		}
+	}
+
+	var nilVisibility images.ImageVisibility
+	return nilVisibility, fmt.Errorf("No valid ImageVisilibility found for %s", s)
+}
+
+// Retrieve the specific ImageVisibility using the exported const from images
+func getImageStatus(s string) (images.ImageStatus, error) {
+	statuses := [...]images.ImageStatus{
+		images.ImageStatusActive,
+		images.ImageStatusDeactivated,
+		images.ImageStatusDeleted,
+		images.ImageStatusPendingDelete,
+		images.ImageStatusQueued,
+		images.ImageStatusSaving,
+	}
+
+	for _, status := range statuses {
+		if string(status) == s {
+			return status, nil
+		}
+	}
+
+	var nilStatus images.ImageStatus
+	return nilStatus, fmt.Errorf("No valid ImageVisilibility found for %s", s)
 }
 
 // Allows construction of all fields from ListOpts using the "q" tags and
@@ -43,7 +83,7 @@ func getDateFilter(s string) (images.ImageDateFilter, error) {
 func buildImageFilters(input map[string]string, listOpts *images.ListOpts) *packer.MultiError {
 
 	// fill each field in the ListOpts based on tag/type
-	metaOpts := reflect.ValueOf(listOpts).Elem()
+	metaOpts := reflect.Indirect(reflect.ValueOf(listOpts))
 
 	multiErr := packer.MultiError{}
 
@@ -57,17 +97,46 @@ func buildImageFilters(input map[string]string, listOpts *images.ListOpts) *pack
 		if val, exists := input[key]; exists && vField.CanSet() {
 			switch vField.Kind() {
 
-			case reflect.Int64:
+			// Handles integer types used in ListOpts
+			case reflect.Int64, reflect.Int:
 				iVal, err := strconv.Atoi(val)
 				if err != nil {
 					multierror.Append(err, multiErr.Errors...)
-				} else {
+					continue
-					vField.Set(reflect.ValueOf(iVal))
 				}
 
-			case reflect.String:
+				if vField.Kind() == reflect.Int {
-				vField.Set(reflect.ValueOf(val))
+					vField.Set(reflect.ValueOf(iVal))
+				} else {
+					var i64Val int64
+					i64Val = int64(iVal)
+					vField.Set(reflect.ValueOf(i64Val))
+				}
 
+			// Handles string and types using string
+			case reflect.String:
+				switch vField.Type() {
+				default:
+					vField.Set(reflect.ValueOf(val))
+
+				case reflect.TypeOf(images.ImageVisibility("")):
+					iv, err := getImageVisibility(val)
+					if err != nil {
+						multierror.Append(err, multiErr.Errors...)
+						continue
+					}
+					vField.Set(reflect.ValueOf(iv))
+
+				case reflect.TypeOf(images.ImageStatus("")):
+					is, err := getImageStatus(val)
+					if err != nil {
+						multierror.Append(err, multiErr.Errors...)
+						continue
+					}
+					vField.Set(reflect.ValueOf(is))
+				}
+
+			// Generates slice of strings for Tags
 			case reflect.Slice:
 				typeOfSlice := reflect.TypeOf(vField).Elem()
 				fieldArray := reflect.MakeSlice(reflect.SliceOf(typeOfSlice), 0, 0)
@@ -80,18 +149,21 @@ func buildImageFilters(input map[string]string, listOpts *images.ListOpts) *pack
 
 			default:
 				multierror.Append(
-					fmt.Errorf("Unsupported struct type %s", vField.Type().Name),
+					fmt.Errorf("Unsupported kind %s", vField.Kind()),
 					multiErr.Errors...)
 			}
 
-		} else if fieldName == reflect.TypeOf(images.ListOpts{}.CreatedAtQuery).Name() ||
+			// Handles ImageDateQuery types
-			fieldName == reflect.TypeOf(images.ListOpts{}.UpdatedAtQuery).Name() {
+		} else if fieldName == reflect.TypeOf(listOpts.CreatedAtQuery).Name() ||
+			fieldName == reflect.TypeOf(listOpts.UpdatedAtQuery).Name() {
+
 			// get ImageDateQuery from string and set to this field
-			query, err := dateToImageDateQuery(&key, &val)
+			query, err := dateToImageDateQuery(key, val)
 			if err != nil {
 				multierror.Append(err, multiErr.Errors...)
 				continue
 			}
+
 			vField.Set(reflect.ValueOf(query))
 		}
 	}
@@ -104,13 +176,12 @@ func buildImageFilters(input map[string]string, listOpts *images.ListOpts) *pack
 // It is suggested that users use the newest sort field
 // See https://developer.openstack.org/api-ref/image/v2/
 func applyMostRecent(listOpts *images.ListOpts) {
-	// apply to old sorting properties if user used them. This overwrites previous values?
+	// Apply to old sorting properties if user used them. This overwrites previous values.
-	if listOpts.SortDir == "" && listOpts.SortKey != "" {
+	// The docs don't seem to mention more than one field being allowed here and how they would be
-		listOpts.SortDir = descendingSort
+	listOpts.SortDir = descendingSort
-		listOpts.SortKey = createdAtKey
+	listOpts.SortKey = createdAtKey
-	}
 
-	// apply to new sorting property
+	// Apply to new sorting property.
 	if listOpts.Sort != "" {
 		listOpts.Sort = fmt.Sprintf("%s:%s,%s", createdAtKey, descendingSort, listOpts.Sort)
 	} else {
@@ -121,10 +192,10 @@ func applyMostRecent(listOpts *images.ListOpts) {
 }
 
 // Converts a given date entry to ImageDateQuery for use in ListOpts
-func dateToImageDateQuery(val *string, key *string) (*images.ImageDateQuery, error) {
+func dateToImageDateQuery(val string, key string) (*images.ImageDateQuery, error) {
 	q := new(images.ImageDateQuery)
 	sep := ":"
-	entries := strings.Split(*val, sep)
+	entries := strings.Split(val, sep)
 
 	if len(entries) > 3 {
 		filter, err := getDateFilter(entries[0])
@@ -134,9 +205,13 @@ func dateToImageDateQuery(val *string, key *string) (*images.ImageDateQuery, err
 			q.Filter = filter
 		}
 
-		date, err := time.Parse((*val)[len(entries[0]):], time.RFC3339)
+		dateSubstr := val[len(entries[0])+1:]
+		date, err := time.Parse(time.RFC3339, dateSubstr)
 		if err != nil {
-			return nil, fmt.Errorf("Failed to parse date format for %s", key)
+			return nil, fmt.Errorf("Failed to parse date format for %s.\nDate: %s.\nError: %s",
+				key,
+				dateSubstr,
+				err.Error())
 		} else {
 			q.Date = date
 		}

builder/openstack/image_query_test.go

@@ -2,17 +2,19 @@ package openstack
 
 import (
 	"testing"
+
 	"github.com/gophercloud/gophercloud/openstack/imageservice/v2/images"
+	"github.com/mitchellh/mapstructure"
 )
 
 func TestGetImageFilter(t *testing.T) {
 	passedExpectedMap := map[string]images.ImageDateFilter{
-		"gt":   images.FilterGT,
+		"gt":  images.FilterGT,
-		"gte":  images.FilterGTE,
+		"gte": images.FilterGTE,
-		"lt":   images.FilterLT,
+		"lt":  images.FilterLT,
-		"lte":  images.FilterLTE,
+		"lte": images.FilterLTE,
-		"neq":  images.FilterNEQ,
+		"neq": images.FilterNEQ,
-		"eq":   images.FilterEQ,
+		"eq":  images.FilterEQ,
 	}
 
 	for passed, expected := range passedExpectedMap {
@@ -29,12 +31,12 @@ func TestBuildImageFilter(t *testing.T) {
 	testOpts := images.ListOpts{}
 
 	filters := map[string]string{
-		"limit": "3",
+		"limit":      "3",
-		"name": "Ubuntu 16.04",
+		"name":       "Ubuntu 16.04",
 		"visibility": "public",
-		"image_status": "active",
+		"status":     "active",
-		"size_min": "0",
+		"size_min":   "0",
-		"sort": "created_at:desc",
+		"sort":       "created_at:desc",
 	}
 
 	multiErr := buildImageFilters(filters, &testOpts)
@@ -46,11 +48,11 @@ func TestBuildImageFilter(t *testing.T) {
 	}
 
 	if testOpts.Limit != 3 {
-		t.Errorf("Limit did not parse correctly")
+		t.Errorf("Limit did not parse correctly: %d", testOpts.Limit)
 	}
 
 	if testOpts.Name != filters["name"] {
-		t.Errorf("Name did not parse correctly")
+		t.Errorf("Name did not parse correctly: %")
 	}
 
 	var visibility images.ImageVisibility = "public"
@@ -60,7 +62,7 @@ func TestBuildImageFilter(t *testing.T) {
 
 	var imageStatus images.ImageStatus = "active"
 	if testOpts.Status != imageStatus {
-		t.Errorf("Image status did not parse correctly")
+		t.Errorf("Image status did not parse correctly: %s", testOpts.Status)
 	}
 
 	if testOpts.SizeMin != 0 {
@@ -73,30 +75,64 @@ func TestBuildImageFilter(t *testing.T) {
 }
 
 func TestApplyMostRecent(t *testing.T) {
-	testOpts := images.ListOpts{
+	testSortEmptyOpts := images.ListOpts{
-		Name: "RHEL 7.0",
+		Name:    "RHEL 7.0",
 		SizeMin: 0,
 	}
 
-	applyMostRecent(&testOpts)
+	testSortFilledOpts := images.ListOpts{
+		Name:    "Ubuntu 16.04",
+		SizeMin: 0,
+		Sort:    "tags:ubuntu",
+	}
 
-	if testOpts.Sort != "created_at:desc" {
+	applyMostRecent(&testSortEmptyOpts)
+
+	if testSortEmptyOpts.Sort != "created_at:desc" {
 		t.Errorf("Error applying most recent filter: sort")
 	}
 
-	if testOpts.SortDir != "desc" || testOpts.SortKey != "created_at" {
+	if testSortEmptyOpts.SortDir != "desc" || testSortEmptyOpts.SortKey != "created_at" {
-		t.Errorf("Error applying most recent filter: sort_dir/sort_key")
+		t.Errorf("Error applying most recent filter: sort_dir/sort_key:\n{sort_dir: %s, sort_key: %s}",
+			testSortEmptyOpts.SortDir, testSortEmptyOpts.SortKey)
+	}
+
+	applyMostRecent(&testSortFilledOpts)
+
+	if testSortFilledOpts.Sort != "created_at:desc,tags:ubuntu" {
+		t.Errorf("Error applying most recent filter: sort")
+	}
+
+	if testSortFilledOpts.SortDir != "desc" || testSortFilledOpts.SortKey != "created_at" {
+		t.Errorf("Error applying most recent filter: sort_dir/sort_key:\n{sort_dir: %s, sort_key: %s}",
+			testSortFilledOpts.SortDir, testSortFilledOpts.SortKey)
 	}
 }
 
 func TestDateToImageDateQuery(t *testing.T) {
 	tests := [][2]string{
-		{"2006-01-02T15:04:05Z07:00", "created_at"},
+		{"gt:2012-11-01T22:08:41+00:00", "created_at"},
 	}
 
 	for _, test := range tests {
-		if _, err := dateToImageDateQuery(&test[0], &test[1]); err != nil {
+		if _, err := dateToImageDateQuery(test[0], test[1]); err != nil {
 			t.Error(err)
 		}
 	}
-}
+}
+
+func TestImageFilterOptionsDecode(t *testing.T) {
+	opts := ImageFilterOptions{}
+	input := map[string]interface{}{
+		"most_recent": true,
+		"filters": map[string]interface{}{
+			"visibility": "protected",
+			"tag":        "prod",
+			"name":       "ubuntu 16.04",
+		},
+	}
+	err := mapstructure.Decode(input, &opts)
+	if err != nil {
+		t.Error("Did not successfully generate ImageFilterOptions from %v. Contains %v", input, opts)
+	}
+}

builder/openstack/run_config.go

@@ -18,21 +18,22 @@ type RunConfig struct {
 	SSHInterface         string              `mapstructure:"ssh_interface"`
 	SSHIPVersion         string              `mapstructure:"ssh_ip_version"`
 
-	SourceImage       string            `mapstructure:"source_image"`
+	SourceImage        string             `mapstructure:"source_image"`
-	SourceImageName   string            `mapstructure:"source_image_name"`
+	SourceImageName    string             `mapstructure:"source_image_name"`
-	Flavor            string            `mapstructure:"flavor"`
+	SourceImageFilters ImageFilterOptions `mapstructure:"source_image_filter"`
-	AvailabilityZone  string            `mapstructure:"availability_zone"`
+	Flavor             string             `mapstructure:"flavor"`
-	RackconnectWait   bool              `mapstructure:"rackconnect_wait"`
+	AvailabilityZone   string             `mapstructure:"availability_zone"`
-	FloatingIPNetwork string            `mapstructure:"floating_ip_network"`
+	RackconnectWait    bool               `mapstructure:"rackconnect_wait"`
-	FloatingIP        string            `mapstructure:"floating_ip"`
+	FloatingIPNetwork  string             `mapstructure:"floating_ip_network"`
-	ReuseIPs          bool              `mapstructure:"reuse_ips"`
+	FloatingIP         string             `mapstructure:"floating_ip"`
-	SecurityGroups    []string          `mapstructure:"security_groups"`
+	ReuseIPs           bool               `mapstructure:"reuse_ips"`
-	Networks          []string          `mapstructure:"networks"`
+	SecurityGroups     []string           `mapstructure:"security_groups"`
-	Ports             []string          `mapstructure:"ports"`
+	Networks           []string           `mapstructure:"networks"`
-	UserData          string            `mapstructure:"user_data"`
+	Ports              []string           `mapstructure:"ports"`
-	UserDataFile      string            `mapstructure:"user_data_file"`
+	UserData           string             `mapstructure:"user_data"`
-	InstanceName      string            `mapstructure:"instance_name"`
+	UserDataFile       string             `mapstructure:"user_data_file"`
-	InstanceMetadata  map[string]string `mapstructure:"instance_metadata"`
+	InstanceName       string             `mapstructure:"instance_name"`
+	InstanceMetadata   map[string]string  `mapstructure:"instance_metadata"`
 
 	ConfigDrive bool `mapstructure:"config_drive"`
 
@@ -75,8 +76,8 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error {
 		}
 	}
 
-	if c.SourceImage == "" && c.SourceImageName == "" {
+	if c.SourceImage == "" && c.SourceImageName == "" && c.SourceImageFilters.Filters != nil {
-		errs = append(errs, errors.New("Either a source_image or a source_image_`name must be specified"))
+		errs = append(errs, errors.New("Either a source_image, a source_image_name, or  must be specified"))
 	} else if len(c.SourceImage) > 0 && len(c.SourceImageName) > 0 {
 		errs = append(errs, errors.New("Only a source_image or a source_image_name can be specified, not both."))
 	}

builder/openstack/step_source_image_info.go

@@ -1,26 +1,25 @@
 package openstack
 
 import (
-	"log"
-	"fmt"
 	"context"
+	"fmt"
+	"log"
 
-
-	"github.com/hashicorp/packer/packer"
-	"github.com/hashicorp/packer/helper/multistep"
 	"github.com/gophercloud/gophercloud/openstack/imageservice/v2/images"
 	"github.com/gophercloud/gophercloud/pagination"
+	"github.com/hashicorp/packer/helper/multistep"
+	"github.com/hashicorp/packer/packer"
 )
 
 type StepSourceImageInfo struct {
-	SourceImage        string
+	SourceImage     string
-	SourceImageName    string
+	SourceImageName string
-	ImageFilters       ImageFilterOptions
+	ImageFilters    ImageFilterOptions
 }
 
 type ImageFilterOptions struct {
-	Filters      map[string]string
+	Filters    map[string]string `mapstructure:"filters"`
-	MostRecent   bool                    `mapstructure:"most_recent"`
+	MostRecent bool              `mapstructure:"most_recent"`
 }
 
 func (s *StepSourceImageInfo) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
@@ -54,7 +53,7 @@ func (s *StepSourceImageInfo) Run(_ context.Context, state multistep.StateBag) m
 
 	log.Printf("Using Image Filters %v", params)
 	image := &images.Image{}
-	err = images.List(client, params).EachPage(func (page pagination.Page) (bool, error) {
+	err = images.List(client, params).EachPage(func(page pagination.Page) (bool, error) {
 		i, err := images.ExtractImages(page)
 		if err != nil {
 			return false, err
@@ -62,12 +61,14 @@ 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", )
+			return false, fmt.Errorf("No image was found matching filters: %v", params)
 		case 1:
 			*image = i[0]
 			return true, nil
 		default:
-			return false, fmt.Errorf("Your query returned more than one result. Please try a more specific search, or set most_recent to true.")
+			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)
 		}
 
 		return true, nil
@@ -84,4 +85,8 @@ func (s *StepSourceImageInfo) Run(_ context.Context, state multistep.StateBag) m
 
 	state.Put("source_image", image)
 	return multistep.ActionContinue
-}
+}
+
+func (s *StepSourceImageInfo) Cleanup(state multistep.StateBag) {
+	// No cleanup required for backout
+}