Skip to content

Commit

Permalink
Added tests for checking condition status and panic conditions for
Browse files Browse the repository at this point in the history
validating combinations, added dummy code for fsm store
  • Loading branch information
jm96441n committed Apr 18, 2023
1 parent c777004 commit eb62bef
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 9 deletions.
8 changes: 2 additions & 6 deletions agent/consul/fsm_data_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,11 @@ func (f *FSMDataStore) Update(entry structs.ConfigEntry) error {
// UpdateStatus takes a config entry, an error, and updates the status field as needed in the FSM state
func (f *FSMDataStore) UpdateStatus(entry structs.ControlledConfigEntry, err error) error {
if err == nil {
//TODO additional status messages for success?
// TODO additional status messages for success?
return nil
}
status := structs.Status{
Conditions: []structs.Condition{{

Status: err.Error() + ": Accepted == false",
},
},
Conditions: entry.GetStatus().Conditions,
}
entry.SetStatus(status)
return f.Update(entry)
Expand Down
19 changes: 16 additions & 3 deletions agent/structs/config_entry_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,10 @@ func NewGatewayCondition(name GatewayConditionType, status ConditionStatus, reas
}

func validateGatewayConfigReason(name GatewayConditionType, status ConditionStatus, reason GatewayConditionReason) error {
if err := checkConditionStatus(status); err != nil {
return err
}

reasons, ok := validGatewayConditionReasonsMapping[name]
if !ok {
return fmt.Errorf("unrecognized GatewayConditionType %q", name)
Expand Down Expand Up @@ -636,9 +640,9 @@ var validRouteConditionReasonsMapping = map[RouteConditionType]map[ConditionStat
}

func checkRouteConditionReason(name RouteConditionType, status ConditionStatus, reason RouteConditionReason) error {
// if err := checkConditionStatus(status); err != nil {
// return err
// }
if err := checkConditionStatus(status); err != nil {
return err
}

reasons, ok := validRouteConditionReasonsMapping[name]
if !ok {
Expand All @@ -657,6 +661,15 @@ func checkRouteConditionReason(name RouteConditionType, status ConditionStatus,
return nil
}

func checkConditionStatus(status ConditionStatus) error {
switch status {
case ConditionStatusTrue, ConditionStatusFalse, ConditionStatusUnknown:
return nil
default:
return fmt.Errorf("unrecognized condition status: %q", status)
}
}

func ptrTo[T any](val T) *T {
return &val
}
65 changes: 65 additions & 0 deletions agent/structs/config_entry_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,13 @@ func TestNewGatewayInvalidCombinationsCausePanic(t *testing.T) {
message: "still not working",
ref: ResourceReference{Name: "name", Kind: "httproute"},
},
"pass something other than a condition status": {
status: ConditionStatus("hello"),
reason: GatewayReasonInvalid,
condType: GatewayConditionResolvedRefs,
message: "still not working",
ref: ResourceReference{Name: "name", Kind: "httproute"},
},
}

for name, tc := range testCases {
Expand Down Expand Up @@ -309,3 +316,61 @@ func TestNewRouteConditionWithValidCombinations(t *testing.T) {
})
}
}

func TestNewRouteInvalidCombinationsCausePanic(t *testing.T) {
// This is not an exhaustive list of all invalid combinations, just a few to confirm
testCases := map[string]struct {
status ConditionStatus
reason RouteConditionReason
condType RouteConditionType
message string
ref ResourceReference
}{
"reason and condition type are valid but status is not": {
status: ConditionStatusTrue,
reason: RouteReasonNotAllowedByListeners,
condType: RouteConditionAccepted,
message: "almost there",
ref: ResourceReference{Name: "name", Kind: "httproute"},
},
"reason and status are valid but condition type is not": {
status: ConditionStatusFalse,
reason: RouteReasonRefNotPermitted,
condType: RouteConditionBound,
message: "not quite",
ref: ResourceReference{Name: "name", Kind: "httproute"},
},
"condition type and status are valid but status is not": {
status: ConditionStatusUnknown,
reason: RouteReasonBound,
condType: RouteConditionBound,
message: "still not working",
ref: ResourceReference{Name: "name", Kind: "httproute"},
},
"all are invalid": {
status: ConditionStatusUnknown,
reason: RouteReasonGatewayNotFound,
condType: RouteConditionResolvedRefs,
message: "still not working",
ref: ResourceReference{Name: "name", Kind: "httproute"},
},
"pass something other than a condition status": {
status: ConditionStatus("hello"),
reason: RouteReasonAccepted,
condType: RouteConditionAccepted,
message: "still not working",
ref: ResourceReference{Name: "name", Kind: "httproute"},
},
}

for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
defer func() {
if r := recover(); r == nil {
t.Errorf("Expected combination %+v to be invalid", tc)
}
}()
_ = NewRouteCondition(tc.condType, tc.status, tc.reason, tc.message, tc.ref)
})
}
}

0 comments on commit eb62bef

Please sign in to comment.