Add "apply-to" field to parameter definition.

Add a check that a required parameter is filled, even with an action specific list.

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>

pkg/action/action.go

@@ -51,6 +51,18 @@ func getImageMap(b *bundle.Bundle) ([]byte, error) {
 	return json.Marshal(imgs)
 }
 
+func appliesToAction(action string, parameter bundle.ParameterDefinition) bool {
+	if len(parameter.ApplyTo) == 0 {
+		return true
+	}
+	for _, act := range parameter.ApplyTo {
+		if action == act {
+			return true
+		}
+	}
+	return false
+}
+
 func opFromClaim(action string, stateless bool, c *claim.Claim, ii bundle.InvocationImage, creds credentials.Set, w io.Writer) (*driver.Operation, error) {
 	env, files, err := creds.Expand(c.Bundle, stateless)
 	if err != nil {
@@ -67,6 +79,9 @@ func opFromClaim(action string, stateless bool, c *claim.Claim, ii bundle.Invoca
 	for k, param := range c.Bundle.Parameters {
 		rawval, ok := c.Parameters[k]
 		if !ok {
+			if param.Required && appliesToAction(action, param) {
+				return nil, fmt.Errorf("missing required parameter %q for action %q", k, action)
+			}
 			continue
 		}
 		value := fmt.Sprintf("%v", rawval)

pkg/action/action_test.go

@@ -144,6 +144,69 @@ func TestOpFromClaim_UndefinedParams(t *testing.T) {
 	assert.Error(t, err)
 }
 
+func TestOpFromClaim_MissingRequiredParameter(t *testing.T) {
+	now := time.Now()
+	b := mockBundle()
+	b.Parameters["param_one"] = bundle.ParameterDefinition{Required: true}
+
+	c := &claim.Claim{
+		Created:  now,
+		Modified: now,
+		Name:     "name",
+		Revision: "revision",
+		Bundle:   b,
+		Parameters: map[string]interface{}{
+			"param_two":   "twoval",
+			"param_three": "threeval",
+		},
+	}
+	invocImage := c.Bundle.InvocationImages[0]
+
+	// missing required parameter fails
+	_, err := opFromClaim(claim.ActionInstall, notStateless, c, invocImage, mockSet, os.Stdout)
+	assert.EqualError(t, err, `missing required parameter "param_one" for action "install"`)
+
+	// fill the missing parameter
+	c.Parameters["param_one"] = "oneval"
+	_, err = opFromClaim(claim.ActionInstall, notStateless, c, invocImage, mockSet, os.Stdout)
+	assert.Nil(t, err)
+}
+
+func TestOpFromClaim_MissingRequiredParamSpecificToAction(t *testing.T) {
+	now := time.Now()
+	b := mockBundle()
+	// Add a required parameter only defined for the test action
+	b.Parameters["param_test"] = bundle.ParameterDefinition{
+		ApplyTo:  []string{"test"},
+		Required: true,
+	}
+	c := &claim.Claim{
+		Created:  now,
+		Modified: now,
+		Name:     "name",
+		Revision: "revision",
+		Bundle:   b,
+		Parameters: map[string]interface{}{
+			"param_one":   "oneval",
+			"param_two":   "twoval",
+			"param_three": "threeval",
+		},
+	}
+	invocImage := c.Bundle.InvocationImages[0]
+
+	// calling install action without the test required parameter for test action is ok
+	_, err := opFromClaim(claim.ActionInstall, notStateless, c, invocImage, mockSet, os.Stdout)
+	assert.Nil(t, err)
+
+	// test action needs the required parameter
+	_, err = opFromClaim("test", notStateless, c, invocImage, mockSet, os.Stdout)
+	assert.EqualError(t, err, `missing required parameter "param_test" for action "test"`)
+
+	c.Parameters["param_test"] = "only for test action"
+	_, err = opFromClaim("test", notStateless, c, invocImage, mockSet, os.Stdout)
+	assert.Nil(t, err)
+}
+
 func TestSelectInvocationImage_EmptyInvocationImages(t *testing.T) {
 	c := &claim.Claim{
 		Bundle: &bundle.Bundle{},

pkg/bundle/parameters.go

@@ -18,7 +18,8 @@ type ParameterDefinition struct {
 	MinLength     *int              `json:"minLength,omitempty" mapstructure:"minLength,omitempty"`
 	MaxLength     *int              `json:"maxLength,omitempty" mapstructure:"maxLength,omitempty"`
 	Metadata      ParameterMetadata `json:"metadata,omitempty" mapstructure:"metadata,omitempty"`
-	Destination   *Location         `json:"destination,omitemtpty" mapstructure:"destination,omitempty"`
+	Destination   *Location         `json:"destination,omitempty" mapstructure:"destination,omitempty"`
+	ApplyTo       []string          `json:"apply-to,omitempty" mapstructure:"apply-to,omitempty"`
 }
 
 // ParameterMetadata contains metadata for a parameter definition.

pkg/bundle/parameters_test.go

@@ -39,6 +39,9 @@ func TestCanReadParameterDefinition(t *testing.T) {
 	minLength := 300
 	maxLength := 400
 	description := "some description"
+	action0 := "action0"
+	action1 := "action1"
+
 	json := fmt.Sprintf(`{
 		"parameters": {
 			"test": {
@@ -51,12 +54,14 @@ func TestCanReadParameterDefinition(t *testing.T) {
 				"maxLength": %d,
 				"metadata": {
 					"description": "%s"
-				}
+				},
+				"apply-to": [ "%s", "%s" ]
 			}
 		}
 	}`,
 		dataType, defaultValue, allowedValues0, allowedValues1,
-		minValue, maxValue, minLength, maxLength, description)
+		minValue, maxValue, minLength, maxLength, description,
+		action0, action1)
 
 	definitions, err := Unmarshal([]byte(json))
 	if err != nil {
@@ -94,6 +99,15 @@ func TestCanReadParameterDefinition(t *testing.T) {
 	if p.Metadata.Description != description {
 		t.Errorf("Expected description '%s' but got '%s'", description, p.Metadata.Description)
 	}
+	if len(p.ApplyTo) != 2 {
+		t.Errorf("Expected 2 apply-to actions but got %d", len(p.ApplyTo))
+	}
+	if p.ApplyTo[0] != action0 {
+		t.Errorf("Expected action '%s' but got '%s'", action0, p.ApplyTo[0])
+	}
+	if p.ApplyTo[1] != action1 {
+		t.Errorf("Expected action '%s' but got '%s'", action1, p.ApplyTo[1])
+	}
 }
 
 func valueTestJSON(jsonRepresentation string) []byte {