[QT-555] Fix panic when passing an invalid scenario filter (#89)

Fix a panic when we tried to execute scenario sub-commands against a
scenario that has no variants but we've included invalid matrix filters.

Signed-off-by: Ryan Cragun <[email protected]>

Author: ryancragun

internal/flightplan/matrix.go

@@ -99,11 +99,6 @@ func (e Element) Equal(other Element) bool {
 	return true
 }
 
-// NewElementFromProto creates a new Element from a proto filter element
-func NewElementFromProto(p *pb.Scenario_Filter_Element) Element {
-	return NewElement(p.GetKey(), p.GetValue())
-}
-
 func NewVector() *Vector {
 	return &Vector{}
 }
@@ -120,6 +115,14 @@ func (v *Vector) String() string {
 
 // Equal returns true if both Vectors have Equal values and Equal value ordering
 func (v *Vector) Equal(other *Vector) bool {
+	if v == nil && other == nil {
+		return true
+	}
+
+	if other == nil {
+		return false
+	}
+
 	if v.elements == nil && other.elements == nil {
 		return true
 	}
@@ -145,6 +148,14 @@ func (v *Vector) Equal(other *Vector) bool {
 // not be ordered the same. This is useful for Vectors of pairs that do not
 // enforce ordering.
 func (v *Vector) EqualUnordered(other *Vector) bool {
+	if v == nil && other == nil {
+		return true
+	}
+
+	if other == nil {
+		return false
+	}
+
 	if v.elements == nil && other.elements == nil {
 		return true
 	}
@@ -172,6 +183,18 @@ func (v *Vector) EqualUnordered(other *Vector) bool {
 // ContainsUnordered returns a boolean which represent if vector contains the values
 // of another vector.
 func (v *Vector) ContainsUnordered(other *Vector) bool {
+	if v == nil && other == nil {
+		return true
+	}
+
+	if other == nil {
+		return false
+	}
+
+	if len(v.elements) < 1 || len(other.elements) < 1 {
+		return false
+	}
+
 	for oi := range other.elements {
 		match := false
 		for vi := range v.elements {
@@ -384,6 +407,10 @@ func (m *Matrix) CartesianProduct() *Matrix {
 // HasVector returns whether or not a matrix has a vector that exactly matches
 // the elements of another that is given.
 func (m *Matrix) HasVector(other *Vector) bool {
+	if other == nil {
+		return false
+	}
+
 	for _, v := range m.Vectors {
 		if v.Equal(other) {
 			return true
@@ -396,6 +423,10 @@ func (m *Matrix) HasVector(other *Vector) bool {
 // HasVectorUnordered returns whether or not a matrix has a vector whose unordered
 // values match exactly with another that is given.
 func (m *Matrix) HasVectorUnordered(other *Vector) bool {
+	if other == nil {
+		return false
+	}
+
 	for _, v := range m.Vectors {
 		if v.EqualUnordered(other) {
 			return true
@@ -431,6 +462,10 @@ func (m *Matrix) UniqueValues() *Matrix {
 
 // Match determines if Exclude directive matches the vector
 func (ex *Exclude) Match(vec *Vector) bool {
+	if vec == nil {
+		return false
+	}
+
 	switch ex.Mode {
 	case pb.Scenario_Filter_Exclude_MODE_EXACTLY:
 		if vec.Equal(ex.Vector) {
@@ -460,6 +495,10 @@ func (ex *Exclude) Proto() *pb.Scenario_Filter_Exclude {
 
 // FromProto unmarshals a proto Scenario_Filter_Exclude into itself
 func (ex *Exclude) FromProto(pfe *pb.Scenario_Filter_Exclude) {
+	if pfe == nil {
+		return
+	}
+
 	ex.Vector = NewVectorFromProto(pfe.GetVector())
 	ex.Mode = pfe.GetMode()
 }

internal/flightplan/scenario.go

@@ -78,13 +78,21 @@ func (s *Scenario) Ref() *pb.Ref_Scenario {
 
 // FromRef takes a unmarshals a scenario reference into itself
 func (s *Scenario) FromRef(ref *pb.Ref_Scenario) {
+	if ref == nil {
+		return
+	}
+
 	s.Name = ref.GetId().GetName()
 	s.Variants = NewVectorFromProto(ref.GetId().GetVariants())
 }
 
 // Match takes a filter and determines whether or not the scenario matches
 // it.
 func (s *Scenario) Match(filter *ScenarioFilter) bool {
+	if filter == nil {
+		return false
+	}
+
 	if filter.SelectAll {
 		return true
 	}
@@ -94,6 +102,18 @@ func (s *Scenario) Match(filter *ScenarioFilter) bool {
 		return false
 	}
 
+	// If our scenario doesn't have any variants make make sure we don't have
+	// a filter with includes or excludes.
+	if s.Variants == nil || len(s.Variants.elements) == 0 {
+		if filter.Include != nil && len(filter.Include.elements) > 0 {
+			return false
+		}
+
+		if filter.Exclude != nil && len(filter.Exclude) > 0 {
+			return false
+		}
+	}
+
 	// Make sure it matches any includes
 	if filter.Include != nil && len(filter.Include.elements) > 0 {
 		if !s.Variants.ContainsUnordered(filter.Include) {

internal/flightplan/scenario_filter_test.go

@@ -10,7 +10,7 @@ import (
 
 // Test_ScenarioFilter_WithScenarioFilterFromScenarioRef tests filtering a
 // scenario that was created from a scenario reference.
-func Test_ScenarioFilter_WithScenarioFilterFromScenarioRe(t *testing.T) {
+func Test_ScenarioFilter_WithScenarioFilterFromScenarioRef(t *testing.T) {
 	ref := &pb.Ref_Scenario{
 		Id: &pb.Scenario_ID{
 			Name: "foo",
@@ -53,6 +53,7 @@ func Test_ScenarioFilter_ScenariosSelect(t *testing.T) {
 		{Name: "upgrade", Variants: &Vector{elements: []Element{NewElement("backend", "raft"), NewElement("arch", "amd64")}}},
 		{Name: "upgrade", Variants: &Vector{elements: []Element{NewElement("backend", "consul"), NewElement("arch", "arm64")}}},
 		{Name: "upgrade", Variants: &Vector{elements: []Element{NewElement("backend", "consul"), NewElement("arch", "amd64")}}},
+		{Name: "no-variant"},
 	}
 
 	for _, test := range []struct {
@@ -65,7 +66,7 @@ func Test_ScenarioFilter_ScenariosSelect(t *testing.T) {
 			"name only",
 			scenarios,
 			&ScenarioFilter{Name: "upgrade"},
-			scenarios[4:],
+			scenarios[4:8],
 		},
 		{
 			"name no match",
@@ -111,6 +112,15 @@ func Test_ScenarioFilter_ScenariosSelect(t *testing.T) {
 			},
 			[]*Scenario{},
 		},
+		{
+			"variant filter pass to scenario without variants",
+			scenarios,
+			&ScenarioFilter{
+				Name:    "no-variant",
+				Include: &Vector{elements: []Element{NewElement("backend", "raft")}},
+			},
+			[]*Scenario{},
+		},
 	} {
 		t.Run(test.desc, func(t *testing.T) {
 			fp := &FlightPlan{

version/version.go

@@ -14,7 +14,7 @@ var (
 	//
 	// Version must conform to the format expected by github.com/hashicorp/go-version
 	// for tests to work.
-	Version = "0.0.18"
+	Version = "0.0.19"
 
 	// VersionPrerelease is a pre-release marker for the version. If this is ""
 	// (empty string) then it means that it is a final release. Otherwise, this