From db3d2682b54ee0c11bdca26da8188c91341512a6 Mon Sep 17 00:00:00 2001 From: Tom Carrio Date: Wed, 18 Jul 2018 11:43:42 -0400 Subject: [PATCH] Updated allowed filters to tags, visibility, owner, and name. Test cases updated and passed --- builder/openstack/image_query.go | 197 +++++------------- builder/openstack/image_query_test.go | 121 +++-------- .../source/docs/builders/openstack.html.md | 14 +- 3 files changed, 90 insertions(+), 242 deletions(-) diff --git a/builder/openstack/image_query.go b/builder/openstack/image_query.go index ff3a9c249..134aa51b8 100644 --- a/builder/openstack/image_query.go +++ b/builder/openstack/image_query.go @@ -2,40 +2,20 @@ package openstack import ( "fmt" - "reflect" - "strconv" - "strings" - "time" - "github.com/gophercloud/gophercloud/openstack/imageservice/v2/images" - "github.com/hashicorp/go-multierror" "github.com/hashicorp/packer/packer" + "reflect" ) const ( - descendingSort = "desc" - createdAtKey = "created_at" + mostRecentSort = "created_at:desc" ) -// Retrieve the specific ImageDateFilter using the exported const from images -func getDateFilter(s string) (images.ImageDateFilter, error) { - filters := []images.ImageDateFilter{ - images.FilterGT, - images.FilterGTE, - images.FilterLT, - images.FilterLTE, - images.FilterNEQ, - images.FilterEQ, - } - - for _, filter := range filters { - if string(filter) == s { - return filter, nil - } - } - - var badFilter images.ImageDateFilter - return badFilter, fmt.Errorf("No valid ImageDateFilter found for %s", s) +var validFields = map[string]string{ + "Name": "name", + "Visibility": "visibility", + "Owner": "owner", + "Tags": "tags", } // Retrieve the specific ImageVisibility using the exported const from images @@ -54,155 +34,72 @@ func getImageVisibility(s string) (images.ImageVisibility, error) { } var nilVisibility images.ImageVisibility - return nilVisibility, fmt.Errorf("No valid ImageVisilibility found for %s", s) + return nilVisibility, fmt.Errorf("No valid ImageVisibility found for %s", s) } -// Retrieve the specific ImageVisibility using the exported const from images -func getImageStatus(s string) (images.ImageStatus, error) { - activeStatus := images.ImageStatusActive - if string(activeStatus) == s { - return activeStatus, 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 -// type detection to set all fields within a provided ListOpts struct +// Allows construction of all supported fields from ListOpts func buildImageFilters(input map[string]interface{}, listOpts *images.ListOpts) *packer.MultiError { // fill each field in the ListOpts based on tag/type metaOpts := reflect.Indirect(reflect.ValueOf(listOpts)) - multiErr := packer.MultiError{} for i := 0; i < metaOpts.Type().NumField(); i++ { vField := metaOpts.Field(i) - tField := metaOpts.Type().Field(i) - fieldName := tField.Name - key := metaOpts.Type().Field(i).Tag.Get("q") + fieldName := metaOpts.Type().Field(i).Name - // get key from the map and set values if they exist - if val, exists := input[key]; exists && vField.CanSet() { - switch vField.Kind() { - - // Handles integer types used in ListOpts - case reflect.Int64, reflect.Int: - iVal, err := strconv.Atoi(val.(string)) - if err != nil { - multierror.Append(err, multiErr.Errors...) - continue - } - - if vField.Kind() == reflect.Int { - 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.(string)) - if err != nil { - multierror.Append(err, multiErr.Errors...) - continue - } - vField.Set(reflect.ValueOf(iv)) - - case reflect.TypeOf(images.ImageStatus("")): - is, err := getImageStatus(val.(string)) - if err != nil { - multierror.Append(err, multiErr.Errors...) - continue - } - vField.Set(reflect.ValueOf(is)) - } - - default: - multierror.Append( - fmt.Errorf("Unsupported kind %s", vField.Kind()), - multiErr.Errors...) - } - - } else if fieldName == reflect.TypeOf(listOpts.CreatedAtQuery).Name() || - fieldName == reflect.TypeOf(listOpts.UpdatedAtQuery).Name() { - // Handles ImageDateQuery types - - query, err := dateToImageDateQuery(key, val.(string)) - if err != nil { - multierror.Append(err, multiErr.Errors...) + // check the valid fields map and whether we can set this field + if key, exists := validFields[fieldName]; exists { + if !vField.CanSet() { + multiErr.Errors = append(multiErr.Errors, fmt.Errorf("Unsettable field: %s", fieldName)) continue } - vField.Set(reflect.ValueOf(query)) + // check that this key was provided by the user, then set the field and have compatible types + if val, exists := input[key]; exists { - } else if fieldName == reflect.TypeOf(listOpts.Tags).Name() { - // Handles "tags" case and processes as slice of string + switch key { + case "owner", "name", "tags": - if val, exists := input["tags"]; exists && vField.CanSet() { - vField.Set(reflect.ValueOf(val)) + if valType := reflect.TypeOf(val); valType != vField.Type() { + multiErr.Errors = append(multiErr.Errors, + fmt.Errorf("Invalid type '%v' for field %s", + valType, + fieldName, + )) + continue + } + vField.Set(reflect.ValueOf(val)) + + case "visibility": + visibility, err := getImageVisibility(val.(string)) + if err != nil { + multiErr.Errors = append(multiErr.Errors, err) + continue + } + vField.Set(reflect.ValueOf(visibility)) + + default: + multiErr.Errors = append(multiErr.Errors, + fmt.Errorf("Unsupported filter key provided: %s", key)) + } } } } + // Set defaults for status and member_status + listOpts.Status = images.ImageStatusActive + listOpts.MemberStatus = images.ImageMemberStatusAccepted + return &multiErr } // Apply most recent filtering logic to ListOpts where user has filled fields. -// This does not check whether both are filled. Allow OpenStack to determine which to use. -// 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. - // The docs don't seem to mention more than one field being allowed here and how they would be - listOpts.SortDir = descendingSort - listOpts.SortKey = createdAtKey - - // Apply to new sorting property. - if listOpts.Sort != "" { - listOpts.Sort = fmt.Sprintf("%s:%s,%s", createdAtKey, descendingSort, listOpts.Sort) - } else { - listOpts.Sort = fmt.Sprintf("%s:%s", createdAtKey, descendingSort) - } + // Sort isn't supported through our API so there should be no existing values. + // Overwriting .Sort is okay. + listOpts.Sort = mostRecentSort return } - -// Converts a given date entry to ImageDateQuery for use in ListOpts -func dateToImageDateQuery(val string, key string) (*images.ImageDateQuery, error) { - q := new(images.ImageDateQuery) - sep := ":" - entries := strings.Split(val, sep) - - if len(entries) > 3 { - filter, err := getDateFilter(entries[0]) - if err != nil { - return nil, fmt.Errorf("Failed to parse date filter for %s", key) - } else { - q.Filter = filter - } - - 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.\nDate: %s.\nError: %s", - key, - dateSubstr, - err.Error()) - } else { - q.Date = date - } - - return q, nil - } - - return nil, fmt.Errorf("Incorrect date query format for %s", key) -} diff --git a/builder/openstack/image_query_test.go b/builder/openstack/image_query_test.go index accdf7e3d..fd2251b6b 100644 --- a/builder/openstack/image_query_test.go +++ b/builder/openstack/image_query_test.go @@ -7,23 +7,22 @@ import ( "github.com/mitchellh/mapstructure" ) -func TestGetImageFilter(t *testing.T) { - passedExpectedMap := map[string]images.ImageDateFilter{ - "gt": images.FilterGT, - "gte": images.FilterGTE, - "lt": images.FilterLT, - "lte": images.FilterLTE, - "neq": images.FilterNEQ, - "eq": images.FilterEQ, +func TestImageFilterOptionsDecode(t *testing.T) { + opts := ImageFilterOptions{} + input := map[string]interface{}{ + "most_recent": true, + "filters": map[string]interface{}{ + "visibility": "protected", + "tag": []string{"prod", "ready"}, + "name": "ubuntu 16.04", + "owner": "tcarrio", + }, } - - for passed, expected := range passedExpectedMap { - filter, err := getDateFilter(passed) - if err != nil { - t.Errorf("Passed %s, received error: %s", passed, err.Error()) - } else if filter != expected { - t.Errorf("Expected %s, got %s", expected, filter) - } + err := mapstructure.Decode(input, &opts) + if err != nil { + t.Errorf("Did not successfully generate ImageFilterOptions from %v.\nContains %v", input, opts) + } else { + t.Log("Successfully generate ImageFilterOptions.") } } @@ -35,107 +34,51 @@ func TestBuildImageFilter(t *testing.T) { "name": "Ubuntu 16.04", "visibility": "public", "status": "active", - "size_min": "0", + "size_min": "25", "sort": "created_at:desc", "tags": []string{"prod", "ready"}, } multiErr := buildImageFilters(filters, &testOpts) - if multiErr != nil { - for _, err := range multiErr.Errors { - t.Error(err) - } + if len(multiErr.Errors) > 0 { + t.Error(multiErr.Error()) } - if testOpts.Limit != 3 { - t.Errorf("Limit did not parse correctly: %d", testOpts.Limit) + if testOpts.Limit != 0 { + t.Errorf("Limit was parsed: %d", testOpts.Limit) } if testOpts.Name != filters["name"] { - t.Errorf("Name did not parse correctly: %s", filters["name"]) + t.Errorf("Name did not parse correctly: %s", testOpts.Name) } - var visibility images.ImageVisibility = "public" - if testOpts.Visibility != visibility { - t.Errorf("Visibility did not parse correctly") + if testOpts.Visibility != images.ImageVisibilityPublic { + t.Errorf("Visibility did not parse correctly: %v", testOpts.Visibility) } - var imageStatus images.ImageStatus = "active" - if testOpts.Status != imageStatus { + if testOpts.Status != images.ImageStatusActive { t.Errorf("Image status did not parse correctly: %s", testOpts.Status) } if testOpts.SizeMin != 0 { - t.Errorf("Size min did not parse correctly: %s", filters["size_min"]) + t.Errorf("Size min was parsed: %d", testOpts.SizeMin) } - if testOpts.Sort != filters["sort"] { - t.Errorf("Sort did not parse correctly: %s", filters["sort"]) + if len(testOpts.Sort) > 0 { + t.Errorf("Sort was parsed: %s", testOpts.Sort) } } func TestApplyMostRecent(t *testing.T) { - testSortEmptyOpts := images.ListOpts{ - Name: "RHEL 7.0", - SizeMin: 0, + testSortOpts := images.ListOpts{ + Name: "RHEL 7.0", + Tags: []string{"prod", "ready"}, } - testSortFilledOpts := images.ListOpts{ - Name: "Ubuntu 16.04", - SizeMin: 0, - Sort: "tags:ubuntu", - } + applyMostRecent(&testSortOpts) - applyMostRecent(&testSortEmptyOpts) - - if testSortEmptyOpts.Sort != "created_at:desc" { + if testSortOpts.Sort != "created_at:desc" { t.Errorf("Error applying most recent filter: sort") } - - if testSortEmptyOpts.SortDir != "desc" || testSortEmptyOpts.SortKey != "created_at" { - 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{ - {"gt:2012-11-01T22:08:41+00:00", "created_at"}, - } - - for _, test := range tests { - 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": []string{"prod", "ready"}, - "name": "ubuntu 16.04", - }, - } - err := mapstructure.Decode(input, &opts) - if err != nil { - t.Errorf("Did not successfully generate ImageFilterOptions from %v.\nContains %v", input, opts) - } else { - t.Logf("Successfully generate ImageFilterOptions from %v.\nContains %v", input, opts) - } } diff --git a/website/source/docs/builders/openstack.html.md b/website/source/docs/builders/openstack.html.md index bdc70f34f..44f7effb8 100644 --- a/website/source/docs/builders/openstack.html.md +++ b/website/source/docs/builders/openstack.html.md @@ -176,7 +176,7 @@ builder. "name": "ubuntu-16.04*", "visibility": "protected", "owner": "d1a588cf4b0743344508dc145649372d1", - "tag": "prod" + "tag": ["prod", "ready"] }, "most_recent": true } @@ -189,8 +189,16 @@ builder. - `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. - Any filter described in the docs for [ImageService](https://developer.openstack.org/api-ref/image/v2/) - is valid. + Of the filters described in [ImageService](https://developer.openstack.org/api-ref/image/v2/), the following + are valid: + + - name (string) + + - owner (string) + + - tags (slice of strings) + + - visibility (string) - `most_recent` (boolean) - Selects the newest created image when true. This is most useful for selecting a daily distro build.