Updated filter parser to log all invalid values/fields provided by user

builder/openstack/image_query.go

@@ -38,6 +38,7 @@ func getImageVisibility(s string) (images.ImageVisibility, error) {
 }
 
 // Allows construction of all supported fields from ListOpts
+// The `input` map will be modified but is not reused further in the builder
 func buildImageFilters(input map[string]interface{}, listOpts *images.ListOpts) *packer.MultiError {
 
 	// fill each field in the ListOpts based on tag/type
@@ -50,24 +51,29 @@ func buildImageFilters(input map[string]interface{}, listOpts *images.ListOpts)
 
 		// 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
-			}
 
 			// check that this key was provided by the user, then set the field and have compatible types
 			if val, exists := input[key]; exists {
 
+				// non-settable field
+				if !vField.CanSet() {
+					multiErr.Errors = append(multiErr.Errors, fmt.Errorf("Unsettable field: %s", fieldName))
+
+					// remove key from input filters so we can go over them after
+					delete(input, key)
+					continue
+				}
+
 				switch key {
 				case "owner", "name", "tags":
-
 					if valType := reflect.TypeOf(val); valType != vField.Type() {
 						multiErr.Errors = append(multiErr.Errors,
-							fmt.Errorf("Invalid type '%v' for field %s",
+							fmt.Errorf("Invalid type '%v' for field %s (%s)",
 								valType,
+								key,
 								fieldName,
 							))
-						continue
+						break
 					}
 					vField.Set(reflect.ValueOf(val))
 
@@ -75,18 +81,27 @@ func buildImageFilters(input map[string]interface{}, listOpts *images.ListOpts)
 					visibility, err := getImageVisibility(val.(string))
 					if err != nil {
 						multiErr.Errors = append(multiErr.Errors, err)
-						continue
+						break
 					}
 					vField.Set(reflect.ValueOf(visibility))
 
-				default:
-					multiErr.Errors = append(multiErr.Errors,
-						fmt.Errorf("Unsupported filter key provided: %s", key))
 				}
+
+				// remove key from input filters so we can go over them after
+				delete(input, key)
 			}
 		}
 	}
 
+	// error any invalid filters
+	for key, value := range input {
+		multiErr.Errors = append(multiErr.Errors, fmt.Errorf("Invalid filter: %s: %v (type: %v)",
+			key,
+			value,
+			reflect.TypeOf(value),
+		))
+	}
+
 	// Set defaults for status and member_status
 	listOpts.Status = images.ImageStatusActive
 	listOpts.MemberStatus = images.ImageMemberStatusAccepted

builder/openstack/image_query_test.go

@@ -39,7 +39,13 @@ func TestBuildImageFilter(t *testing.T) {
 		"tags":       []string{"prod", "ready"},
 	}
 
-	buildImageFilters(filters, &testOpts)
+	// copy of original filters to pass to build function
+	passedFilters := make(map[string]interface{})
+	for k, v := range filters {
+		passedFilters[k] = v
+	}
+
+	buildImageFilters(passedFilters, &testOpts)
 
 	if testOpts.Limit != 0 {
 		t.Errorf("Limit was parsed: %d", testOpts.Limit)
@@ -66,6 +72,28 @@ func TestBuildImageFilter(t *testing.T) {
 	}
 }
 
+// This test case confirms that invalid filter input are caught and do not result in a panic
+func TestInvalidFilterInput(t *testing.T) {
+
+	testOpts := images.ListOpts{}
+
+	filters := map[string]interface{}{
+		"tags":          "prod", // supposed to be a []string
+		"owner":         12345,  // supposed to be a string
+		"invalid_field": 0,      // not a valid field in ListOpts
+	}
+
+	numFields := len(filters)
+
+	multiErr := buildImageFilters(filters, &testOpts)
+	if len(multiErr.Errors) != numFields {
+		t.Errorf("Failed to catch all %d invalid types/fields in filters", numFields)
+		for _, err := range multiErr.Errors {
+			t.Log(err.Error())
+		}
+	}
+}
+
 func TestApplyMostRecent(t *testing.T) {
 	testSortOpts := images.ListOpts{
 		Name: "RHEL 7.0",