diff --git a/CHANGELOG.md b/CHANGELOG.md index 5165cfb309..b5b9f9a1d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/internal/dataplane/sendconfig/error_handling_test.go b/internal/dataplane/sendconfig/error_handling_test.go index 9eb145765d..22d4c2e49d 100644 --- a/internal/dataplane/sendconfig/error_handling_test.go +++ b/internal/dataplane/sendconfig/error_handling_test.go @@ -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", }, }, diff --git a/internal/dataplane/sendconfig/inmemory_error_handling.go b/internal/dataplane/sendconfig/inmemory_error_handling.go index 1c0dcd45dd..030898ac2c 100644 --- a/internal/dataplane/sendconfig/inmemory_error_handling.go +++ b/internal/dataplane/sendconfig/inmemory_error_handling.go @@ -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 @@ -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 { diff --git a/internal/dataplane/sendconfig/sendconfig.go b/internal/dataplane/sendconfig/sendconfig.go index 247a804885..77895227d9 100644 --- a/internal/dataplane/sendconfig/sendconfig.go +++ b/internal/dataplane/sendconfig/sendconfig.go @@ -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) } diff --git a/test/envtest/configerrorevent_envtest_test.go b/test/envtest/configerrorevent_envtest_test.go index 93b7304e4a..c42e151151 100644 --- a/test/envtest/configerrorevent_envtest_test.go +++ b/test/envtest/configerrorevent_envtest_test.go @@ -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") diff --git a/test/integration/config_error_event_test.go b/test/integration/config_error_event_test.go index 0eb5afefe2..697fc1cbd8 100644 --- a/test/integration/config_error_event_test.go +++ b/test/integration/config_error_event_test.go @@ -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")