Skip to content

Commit

Permalink
chore: handle flattened errors related to whole entities
Browse files Browse the repository at this point in the history
  • Loading branch information
czeslavo committed Oct 11, 2023
1 parent 7cd5218 commit 8e1bc30
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 58 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ Adding a new version? You'll need three changes:

- No more "log.SetLogger(...) was never called..." log entry during shutdown of KIC
[#4738](https://github.com/Kong/kubernetes-ingress-controller/pull/4738)
- When Kong returns a flattened error related to a Kong entity, the entity's type and name
will be included in the message reported in `KongConfigurationApplyFailed` Kubernetes event
generated for it.
[#4813](https://github.com/Kong/kubernetes-ingress-controller/pull/4813)

### Changed

Expand Down
2 changes: 1 addition & 1 deletion internal/dataplane/sendconfig/error_handling_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func TestParseFlatEntityErrors(t *testing.T) {
APIVersion: "v1",
UID: "e7e5c93e-4d56-4cc3-8f4f-ff1fcbe95eb2",
Problems: map[string]string{
"": "failed conditional validation given value of field protocol",
"service:67338dc2-31fd-47b6-85a9-9c11d347d090.httpbin.httpbin.80": "failed conditional validation given value of field protocol",
"path": "value must be null",
},
},
Expand Down
51 changes: 43 additions & 8 deletions internal/dataplane/sendconfig/inmemory_error_handling.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,48 @@ type ConfigErrorFields struct{}

// FlatEntityError represents a single Kong entity with one or more invalid fields.
type FlatEntityError struct {
Name string `json:"entity_name,omitempty" yaml:"entity_name,omitempty"`
ID string `json:"entity_id,omitempty" yaml:"entity_id,omitempty"`
Tags []string `json:"entity_tags,omitempty" yaml:"entity_tags,omitempty"`
Errors []FlatFieldError `json:"errors,omitempty" yaml:"errors,omitempty"`
// Name is the name of the Kong entity.
Name string `json:"entity_name,omitempty" yaml:"entity_name,omitempty"`

// ID is the ID of the Kong entity.
ID string `json:"entity_id,omitempty" yaml:"entity_id,omitempty"`

// Tags are the tags of the Kong entity.
Tags []string `json:"entity_tags,omitempty" yaml:"entity_tags,omitempty"`

// Type is the type of the Kong entity.
Type string `json:"entity_type,omitempty" yaml:"entity_type,omitempty"`

// Errors are the errors associated with the Kong entity.
Errors []FlatError `json:"errors,omitempty" yaml:"errors,omitempty"`
}

// FlatFieldError represents an error for a single field within a Kong entity.
type FlatFieldError struct {
// FlatErrorType tells whether a FlatError is associated with a single field or a whole entity.
type FlatErrorType string

const (
// FlatErrorTypeField is an error associated with a single field.
FlatErrorTypeField FlatErrorType = "field"

// FlatErrorTypeEntity is an error associated with a whole entity.
FlatErrorTypeEntity FlatErrorType = "entity"
)

// FlatError represents an error for a single field within a Kong entity or a whole entity.
type FlatError struct {
// Field is the name of the entity's field that has an error.
// Optional: Field can be empty if the error is associated with the whole entity.
Field string `json:"field,omitempty" yaml:"field,omitempty"`
// Message is the error associated with Field for single-value fields.

// Message is the error associated with Field (for single-value fields) or with a whole entity when Type is "entity".
Message string `json:"message,omitempty" yaml:"message,omitempty"`

// Messages are the errors associated with Field for multi-value fields. The array index in Messages matches the
// array index in the input.
Messages []string `json:"messages,omitempty" yaml:"messages,omitempty"`

// Type tells whether the error is associated with a single field or a whole entity.
Type FlatErrorType `json:"type,omitempty" yaml:"type,omitempty"`
}

// parseFlatEntityErrors takes a Kong /config error response body and parses its "fields.flattened_errors" value
Expand Down Expand Up @@ -81,7 +109,14 @@ func parseFlatEntityErrors(body []byte, log logrus.FieldLogger) ([]ResourceError
continue
}
if len(p.Message) > 0 {
raw.Problems[p.Field] = p.Message
switch p.Type {
case FlatErrorTypeField:
// If the error is associated with a single field, store it in the map under the field name.
raw.Problems[p.Field] = p.Message
case FlatErrorTypeEntity:
// If the error is associated with a whole entity, store it in the map under the entity type and name.
raw.Problems[fmt.Sprintf("%s:%s", ee.Type, ee.Name)] = p.Message
}
}
if len(p.Messages) > 0 {
for i, message := range p.Messages {
Expand Down
8 changes: 4 additions & 4 deletions internal/dataplane/sendconfig/sendconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,14 @@ func resourceErrorsToResourceFailures(resourceErrors []ResourceError, parseErr e
UID: k8stypes.UID(ee.UID),
},
}
for field, problem := range ee.Problems {
log.Debug(fmt.Sprintf("adding failure for %s: %s = %s", ee.Name, field, problem))
for problemSource, problem := range ee.Problems {
log.Debug(fmt.Sprintf("adding failure for %s: %s = %s", ee.Name, problemSource, problem))
resourceFailure, failureCreateErr := failures.NewResourceFailure(
fmt.Sprintf("invalid %s: %s", field, problem),
fmt.Sprintf("invalid %s: %s", problemSource, problem),
&obj,
)
if failureCreateErr != nil {
log.WithError(failureCreateErr).Error("could create resource failure event")
log.WithError(failureCreateErr).Error("could not create resource failure event")
} else {
out = append(out, resourceFailure)
}
Expand Down
57 changes: 31 additions & 26 deletions test/envtest/configerrorevent_envtest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,32 +92,37 @@ func TestConfigErrorEventGeneration(t *testing.T) {
}
t.Logf("got %d events", len(events.Items))

return true && // For the sake of equal indentation
lo.ContainsBy(events.Items, func(e corev1.Event) bool {
return e.Reason == dataplane.KongConfigurationApplyFailedEventReason &&
e.InvolvedObject.Kind == "Ingress" &&
e.InvolvedObject.Name == ingress.Name &&
e.Message == "invalid methods: cannot set 'methods' when 'protocols' is 'grpc' or 'grpcs'"
}) &&
lo.ContainsBy(events.Items, func(e corev1.Event) bool {
return e.Reason == dataplane.KongConfigurationApplyFailedEventReason &&
e.InvolvedObject.Kind == "Service" &&
e.InvolvedObject.Name == service.Name &&
e.Message == "invalid path: value must be null"
}) &&
lo.ContainsBy(events.Items, func(e corev1.Event) bool {
return e.Reason == dataplane.KongConfigurationApplyFailedEventReason &&
e.InvolvedObject.Kind == "Service" &&
e.InvolvedObject.Name == service.Name &&
e.Message == "invalid : failed conditional validation given value of field 'protocol'"
}) &&
lo.ContainsBy(events.Items, func(e corev1.Event) bool {
ok, err := regexp.MatchString(`failed to apply Kong configuration to http://[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+:[0-9]+: failed posting new config to /config: got status code 400`, e.Message)
return e.Reason == dataplane.KongConfigurationApplyFailedEventReason &&
e.InvolvedObject.Kind == "Pod" &&
e.InvolvedObject.Name == podName &&
ok && err == nil
})
matches := make([]bool, 4)
matches[0] = lo.ContainsBy(events.Items, func(e corev1.Event) bool {
return e.Reason == dataplane.KongConfigurationApplyFailedEventReason &&
e.InvolvedObject.Kind == "Ingress" &&
e.InvolvedObject.Name == ingress.Name &&
e.Message == "invalid methods: cannot set 'methods' when 'protocols' is 'grpc' or 'grpcs'"
})
matches[1] = lo.ContainsBy(events.Items, func(e corev1.Event) bool {
return e.Reason == dataplane.KongConfigurationApplyFailedEventReason &&
e.InvolvedObject.Kind == "Service" &&
e.InvolvedObject.Name == service.Name &&
e.Message == "invalid path: value must be null"
})
matches[2] = lo.ContainsBy(events.Items, func(e corev1.Event) bool {
return e.Reason == dataplane.KongConfigurationApplyFailedEventReason &&
e.InvolvedObject.Kind == "Service" &&
e.InvolvedObject.Name == service.Name &&
e.Message == "invalid service:httpbin.httpbin.80: failed conditional validation given value of field 'protocol'"
})
matches[3] = lo.ContainsBy(events.Items, func(e corev1.Event) bool {
ok, err := regexp.MatchString(`failed to apply Kong configuration to http://[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+:[0-9]+: failed posting new config to /config: got status code 400`, e.Message)
return e.Reason == dataplane.KongConfigurationApplyFailedEventReason &&
e.InvolvedObject.Kind == "Pod" &&
e.InvolvedObject.Name == podName &&
ok && err == nil
})
if lo.Count(matches, true) != 4 {
t.Logf("not all events matched: %+v", matches)
return false
}
return true
}, waitTime, tickTime)

t.Log("push failure events recorded successfully")
Expand Down
45 changes: 26 additions & 19 deletions test/integration/config_error_event_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,25 +80,32 @@ func TestConfigErrorEventGeneration(t *testing.T) {
}
t.Logf("got %d events", len(events.Items))

return true && // For the sake of equal indentation
lo.ContainsBy(events.Items, func(e corev1.Event) bool {
return e.Reason == dataplane.KongConfigurationApplyFailedEventReason &&
e.InvolvedObject.Kind == "Ingress" &&
e.InvolvedObject.Name == ingress.Name &&
e.Message == "invalid methods: cannot set 'methods' when 'protocols' is 'grpc' or 'grpcs'"
}) &&
lo.ContainsBy(events.Items, func(e corev1.Event) bool {
return e.Reason == dataplane.KongConfigurationApplyFailedEventReason &&
e.InvolvedObject.Kind == "Service" &&
e.InvolvedObject.Name == service.Name &&
e.Message == "invalid path: value must be null"
}) &&
lo.ContainsBy(events.Items, func(e corev1.Event) bool {
return e.Reason == dataplane.KongConfigurationApplyFailedEventReason &&
e.InvolvedObject.Kind == "Service" &&
e.InvolvedObject.Name == service.Name &&
e.Message == "invalid : failed conditional validation given value of field 'protocol'"
})
matches := make([]bool, 3)
matches[0] = lo.ContainsBy(events.Items, func(e corev1.Event) bool {
return e.Reason == dataplane.KongConfigurationApplyFailedEventReason &&
e.InvolvedObject.Kind == "Ingress" &&
e.InvolvedObject.Name == ingress.Name &&
e.Message == "invalid methods: cannot set 'methods' when 'protocols' is 'grpc' or 'grpcs'"
})
matches[1] = lo.ContainsBy(events.Items, func(e corev1.Event) bool {
return e.Reason == dataplane.KongConfigurationApplyFailedEventReason &&
e.InvolvedObject.Kind == "Service" &&
e.InvolvedObject.Name == service.Name &&
e.Message == "invalid path: value must be null"
})
matches[2] = lo.ContainsBy(events.Items, func(e corev1.Event) bool {
return e.Reason == dataplane.KongConfigurationApplyFailedEventReason &&
e.InvolvedObject.Kind == "Service" &&
e.InvolvedObject.Name == service.Name &&
e.Message == "invalid service:httpbin.httpbin.80: failed conditional validation given value of field 'protocol'"
})

if lo.Count(matches, true) != 3 {
t.Logf("not all events matched: %+v", matches)
return false
}

return true
}, statusWait, waitTick)

t.Log("push failure events recorded successfully")
Expand Down

0 comments on commit 8e1bc30

Please sign in to comment.