Merge pull request #1668 from carolynvs/fix-explain-outputs

Fix applyTo check for outputs in porter explain

pkg/manifest/manifest.go

@@ -14,6 +14,7 @@ import (
 	"get.porter.sh/porter/pkg/yaml"
 	"github.com/Masterminds/semver/v3"
 	"github.com/cbroglie/mustache"
+	"github.com/cnabio/cnab-go/bundle"
 	"github.com/cnabio/cnab-go/bundle/definition"
 	"github.com/cnabio/cnab-go/claim"
 	"github.com/docker/distribution/reference"
@@ -309,6 +310,8 @@ func (pd *ParameterDefinitions) UnmarshalYAML(unmarshal func(interface{}) error)
 	return nil
 }
 
+var _ bundle.Scoped = &ParameterDefinition{}
+
 // ParameterDefinition defines a single parameter for a CNAB bundle
 type ParameterDefinition struct {
 	Name      string          `yaml:"name"`
@@ -323,6 +326,10 @@ type ParameterDefinition struct {
 	definition.Schema `yaml:",inline"`
 }
 
+func (pd *ParameterDefinition) GetApplyTo() []string {
+	return pd.ApplyTo
+}
+
 func (pd *ParameterDefinition) Validate() error {
 	var result *multierror.Error
 
@@ -367,15 +374,7 @@ func (pd *ParameterDefinition) DeepCopy() *ParameterDefinition {
 // AppliesTo returns a boolean value specifying whether or not
 // the Parameter applies to the provided action
 func (pd *ParameterDefinition) AppliesTo(action string) bool {
-	if len(pd.ApplyTo) == 0 {
-		return true
-	}
-	for _, act := range pd.ApplyTo {
-		if action == act {
-			return true
-		}
-	}
-	return false
+	return bundle.AppliesTo(pd, action)
 }
 
 // exemptFromInstall returns true if a parameter definition:

pkg/porter/explain.go

@@ -224,7 +224,7 @@ func generatePrintable(bun bundle.Bundle, action string) (*PrintableBundle, erro
 		pc.Required = v.Required
 		pc.ApplyTo = generateApplyToString(v.ApplyTo)
 
-		if action == "" || v.AppliesTo(action) {
+		if shouldIncludeInExplainOutput(&v, action) {
 			creds = append(creds, pc)
 		}
 	}
@@ -250,7 +250,7 @@ func generatePrintable(bun bundle.Bundle, action string) (*PrintableBundle, erro
 		pp.Required = v.Required
 		pp.Description = v.Description
 
-		if action == "" || v.AppliesTo(action) {
+		if shouldIncludeInExplainOutput(&v, action) {
 			params = append(params, pp)
 		}
 	}
@@ -271,7 +271,7 @@ func generatePrintable(bun bundle.Bundle, action string) (*PrintableBundle, erro
 		po.ApplyTo = generateApplyToString(v.ApplyTo)
 		po.Description = v.Description
 
-		if v.AppliesTo(action) {
+		if shouldIncludeInExplainOutput(&v, action) {
 			outputs = append(outputs, po)
 		}
 	}
@@ -300,6 +300,16 @@ func generatePrintable(bun bundle.Bundle, action string) (*PrintableBundle, erro
 	return &pb, nil
 }
 
+// shouldIncludeInExplainOutput determine if a scoped item such as a credential, parameter or output
+// should be included in the explain output.
+func shouldIncludeInExplainOutput(scoped bundle.Scoped, action string) bool {
+	if action == "" {
+		return true
+	}
+
+	return bundle.AppliesTo(scoped, action)
+}
+
 func generateApplyToString(appliesTo []string) string {
 	if len(appliesTo) == 0 {
 		return "All Actions"

pkg/porter/explain_test.go

@@ -319,6 +319,10 @@ func TestExplain_generatePrintableBundleOutputs(t *testing.T) {
 			"debug": {
 				Definition: "string",
 			},
+			"someoutput": {
+				Definition: "string",
+				ApplyTo: []string{"install"},
+			},
 		},
 		Custom: map[string]interface{}{
 			"sh.porter": map[string]interface{}{
@@ -330,14 +334,32 @@ func TestExplain_generatePrintableBundleOutputs(t *testing.T) {
 	}
 
 	pb, err := generatePrintable(bun, "")
-	assert.NoError(t, err)
+	require.NoError(t, err)
+
+	require.Equal(t, 2, len(pb.Outputs), "expected someoutput to be included because the action is unset")
+	debugOutput := pb.Outputs[0]
+	assert.Equal(t, "string", fmt.Sprintf("%v", debugOutput.Type))
+	assert.Equal(t, "debug", debugOutput.Name)
+	assert.Equal(t, "All Actions", debugOutput.ApplyTo)
+
+	someOutput := pb.Outputs[1]
+	assert.Equal(t, "string", fmt.Sprintf("%v", someOutput.Type))
+	assert.Equal(t, "someoutput", someOutput.Name)
+	assert.Equal(t, "install", someOutput.ApplyTo)
 
-	assert.Equal(t, 1, len(pb.Outputs))
-	d := pb.Outputs[0]
-	assert.Equal(t, "string", fmt.Sprintf("%v", d.Type))
 	assert.Equal(t, 0, len(pb.Parameters))
 	assert.Equal(t, 0, len(pb.Credentials))
 	assert.Equal(t, 0, len(pb.Actions))
+
+	// Check outputs for install action
+	pb, err = generatePrintable(bun, "install")
+	require.NoError(t, err)
+	assert.Equal(t, 2, len(pb.Outputs),"expected someoutput to be included")
+
+	// Check outputs for upgrade action action (someoutput doesn't apply)
+	pb, err = generatePrintable(bun, "upgrade")
+	require.NoError(t, err)
+	assert.Equal(t, 1, len(pb.Outputs), "expected someoutput to be excluded by its applyTo")
 }
 
 func TestExplain_generatePrintableBundleCreds(t *testing.T) {