From 2f84da087e9c0a802327761c628c15b0f6fa56ed Mon Sep 17 00:00:00 2001 From: Jeff Cantrill Date: Tue, 16 Jul 2024 17:14:06 -0400 Subject: [PATCH] LOG-5816: Refactor http validation to obs api --- .../validate_http_content_type_headers.go | 35 -------- ...validate_http_content_type_headers_test.go | 80 ------------------- .../clusterlogforwarder/validations.go | 1 - .../observability/outputs/validate.go | 2 + .../validate_http_content_type_headers.go | 30 +++++++ ...validate_http_content_type_headers_test.go | 61 ++++++++++++++ 6 files changed, 93 insertions(+), 116 deletions(-) delete mode 100644 internal/validations/clusterlogforwarder/validate_http_content_type_headers.go delete mode 100644 internal/validations/clusterlogforwarder/validate_http_content_type_headers_test.go create mode 100644 internal/validations/observability/outputs/validate_http_content_type_headers.go create mode 100644 internal/validations/observability/outputs/validate_http_content_type_headers_test.go diff --git a/internal/validations/clusterlogforwarder/validate_http_content_type_headers.go b/internal/validations/clusterlogforwarder/validate_http_content_type_headers.go deleted file mode 100644 index dbfc502110..0000000000 --- a/internal/validations/clusterlogforwarder/validate_http_content_type_headers.go +++ /dev/null @@ -1,35 +0,0 @@ -package clusterlogforwarder - -import ( - log "github.com/ViaQ/logerr/v2/log/static" - "github.com/openshift/cluster-logging-operator/api/logging/v1" - "github.com/openshift/cluster-logging-operator/internal/validations/errors" - "reflect" - "sigs.k8s.io/controller-runtime/pkg/client" - "strings" -) - -var validContentTypes = map[string]string{ - "application/json": "json", - "application/x-ndjson": "ndjson", -} - -// validateHttpContentTypeHeaders will validate Content-Type header in Http Output -// valid content-type are: "application/json" and "application/x-ndjson" -// was introduced in https://github.com/openshift/cluster-logging-operator/pull/1924 -// for https://issues.redhat.com/browse/LOG-3784 -func validateHttpContentTypeHeaders(clf v1.ClusterLogForwarder, k8sClient client.Client, extras map[string]bool) (error, *v1.ClusterLogForwarderStatus) { - for i, output := range clf.Spec.Outputs { - _, output := i, output // Don't bind range variable. - if output.Type == v1.OutputTypeHttp && output.Http != nil { - if contentType, found := output.Http.Headers["Content-Type"]; found && validContentTypes[strings.ToLower(contentType)] == "" { - validKeys := reflect.ValueOf(validContentTypes).MapKeys() - log.V(3).Info("validateHttpContentTypeHeaders failed", "reason", "not valid content type set in headers", - "content type", contentType, "supported types: ", validKeys) - return errors.NewValidationError("not valid content type set in headers: %s , supported types: %s", - contentType, validKeys), nil - } - } - } - return nil, nil -} diff --git a/internal/validations/clusterlogforwarder/validate_http_content_type_headers_test.go b/internal/validations/clusterlogforwarder/validate_http_content_type_headers_test.go deleted file mode 100644 index b1b54cb08d..0000000000 --- a/internal/validations/clusterlogforwarder/validate_http_content_type_headers_test.go +++ /dev/null @@ -1,80 +0,0 @@ -package clusterlogforwarder - -import ( - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - "github.com/openshift/cluster-logging-operator/api/logging/v1" -) - -var _ = Describe("[internal][validations] ClusterLogForwarder will validate Content-Type header in Http Output", func() { - var ( - clf *v1.ClusterLogForwarder - http *v1.Http - ) - BeforeEach(func() { - http = &v1.Http{} - clf = &v1.ClusterLogForwarder{ - Spec: v1.ClusterLogForwarderSpec{ - Outputs: []v1.OutputSpec{ - { - Name: "httpOutput", - Type: v1.OutputTypeHttp, - OutputTypeSpec: v1.OutputTypeSpec{ - Http: http, - }, - }, - }, - }, - } - }) - - Context("#validateHttpContentTypeHeaders", func() { - - It("should pass validation with empty headers", func() { - Expect(validateHttpContentTypeHeaders(*clf, nil, nil)).To(Succeed()) - }) - It("should pass validation when not Content Type header", func() { - clf.Spec.Outputs[0].Http.Headers = map[string]string{ - "Accept": "application/json", - } - Expect(validateHttpContentTypeHeaders(*clf, nil, nil)).To(Succeed()) - }) - It("should pass validation when the Content Type header is application/json", func() { - clf.Spec.Outputs[0].Http.Headers = map[string]string{ - "Content-Type": "application/json", - } - Expect(validateHttpContentTypeHeaders(*clf, nil, nil)).To(Succeed()) - }) - It("should pass validation when the Content Type header is application/x-ndjson", func() { - clf.Spec.Outputs[0].Http.Headers = map[string]string{ - "Content-Type": "application/x-ndjson", - } - Expect(validateHttpContentTypeHeaders(*clf, nil, nil)).To(Succeed()) - }) - It("should fail validation when not valid content types", func() { - clf.Spec.Outputs[0].Http.Headers = map[string]string{ - "Content-Type": "application/x-www-form-urlencoded", - } - Expect(validateHttpContentTypeHeaders(*clf, nil, nil)).To(Not(Succeed())) - }) - It("should pass validation when not Http Output", func() { - notHttpClf := &v1.ClusterLogForwarder{ - Spec: v1.ClusterLogForwarderSpec{ - Outputs: []v1.OutputSpec{ - { - Name: "esOutput", - Type: v1.OutputTypeElasticsearch, - OutputTypeSpec: v1.OutputTypeSpec{ - Elasticsearch: &v1.Elasticsearch{}, - }, - }, - }, - }, - } - clf.Spec.Outputs[0].Http.Headers = map[string]string{ - "Content-Type": "application/x-www-form-urlencoded", - } - Expect(validateHttpContentTypeHeaders(*notHttpClf, nil, nil)).To(Succeed()) - }) - }) -}) diff --git a/internal/validations/clusterlogforwarder/validations.go b/internal/validations/clusterlogforwarder/validations.go index 825c96431c..32f8ce83ab 100644 --- a/internal/validations/clusterlogforwarder/validations.go +++ b/internal/validations/clusterlogforwarder/validations.go @@ -27,6 +27,5 @@ var validations = []func(clf v1.ClusterLogForwarder, k8sClient client.Client, ex ValidateInputsOutputsPipelines, validateJsonParsingToElasticsearch, validateUrlAccordingToTls, - validateHttpContentTypeHeaders, validateAnnotations, } diff --git a/internal/validations/observability/outputs/validate.go b/internal/validations/observability/outputs/validate.go index 378f61eb40..322ad6ee60 100644 --- a/internal/validations/observability/outputs/validate.go +++ b/internal/validations/observability/outputs/validate.go @@ -20,6 +20,8 @@ func Validate(context internalcontext.ForwarderContext) { switch out.Type { case obsv1.OutputTypeCloudwatch: messages = append(messages, ValidateCloudWatchAuth(out)...) + case obsv1.OutputTypeHTTP: + messages = append(messages, validateHttpContentTypeHeaders(out)...) case obsv1.OutputTypeOTLP: messages = append(messages, ValidateOtlpAnnotation(context)...) } diff --git a/internal/validations/observability/outputs/validate_http_content_type_headers.go b/internal/validations/observability/outputs/validate_http_content_type_headers.go new file mode 100644 index 0000000000..d2a15c8dab --- /dev/null +++ b/internal/validations/observability/outputs/validate_http_content_type_headers.go @@ -0,0 +1,30 @@ +package outputs + +import ( + "fmt" + log "github.com/ViaQ/logerr/v2/log/static" + obs "github.com/openshift/cluster-logging-operator/api/observability/v1" + "reflect" + "strings" +) + +var validContentTypes = map[string]string{ + "application/json": "json", + "application/x-ndjson": "ndjson", +} + +// validateHttpContentTypeHeaders will validate Content-Type header in Http Output +// valid content-type are: "application/json" and "application/x-ndjson" +// was introduced in https://github.com/openshift/cluster-logging-operator/pull/1924 +// for https://issues.redhat.com/browse/LOG-3784 +func validateHttpContentTypeHeaders(output obs.OutputSpec) (results []string) { + if output.Type == obs.OutputTypeHTTP && output.HTTP != nil { + if contentType, found := output.HTTP.Headers["Content-Type"]; found && validContentTypes[strings.ToLower(contentType)] == "" { + validKeys := reflect.ValueOf(validContentTypes).MapKeys() + log.V(3).Info("validateHttpContentTypeHeaders failed", "reason", "not valid content type set in headers", + "content type", contentType, "supported types: ", validKeys) + results = append(results, fmt.Sprintf("not valid content type set in headers: %s , supported types: %s", contentType, validKeys)) + } + } + return results +} diff --git a/internal/validations/observability/outputs/validate_http_content_type_headers_test.go b/internal/validations/observability/outputs/validate_http_content_type_headers_test.go new file mode 100644 index 0000000000..561184e7ad --- /dev/null +++ b/internal/validations/observability/outputs/validate_http_content_type_headers_test.go @@ -0,0 +1,61 @@ +package outputs + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "github.com/openshift/cluster-logging-operator/api/observability/v1" +) + +var _ = Describe("[internal][validations] ClusterLogForwarder will validate Content-Type header in HTTP Output", func() { + var ( + http *v1.HTTP + spec v1.OutputSpec + ) + BeforeEach(func() { + http = &v1.HTTP{} + spec = v1.OutputSpec{ + Name: "httpOutput", + Type: v1.OutputTypeHTTP, + HTTP: http, + } + }) + + Context("#validateHttpContentTypeHeaders", func() { + + It("should pass validation with empty headers", func() { + Expect(validateHttpContentTypeHeaders(spec)).To(BeEmpty()) + }) + It("should pass validation when not Content Type header", func() { + spec.HTTP.Headers = map[string]string{ + "Accept": "application/json", + } + Expect(validateHttpContentTypeHeaders(spec)).To(BeEmpty()) + }) + It("should pass validation when the Content Type header is application/json", func() { + spec.HTTP.Headers = map[string]string{ + "Content-Type": "application/json", + } + Expect(validateHttpContentTypeHeaders(spec)).To(BeEmpty()) + }) + It("should pass validation when the Content Type header is application/x-ndjson", func() { + spec.HTTP.Headers = map[string]string{ + "Content-Type": "application/x-ndjson", + } + Expect(validateHttpContentTypeHeaders(spec)).To(BeEmpty()) + }) + It("should fail validation when not valid content types", func() { + spec.HTTP.Headers = map[string]string{ + "Content-Type": "application/x-www-form-urlencoded", + } + Expect(validateHttpContentTypeHeaders(spec)).ToNot(BeEmpty()) + }) + It("should pass validation when not HTTP Output", func() { + spec = v1.OutputSpec{ + Name: "esOutput", + Type: v1.OutputTypeElasticsearch, + Elasticsearch: &v1.Elasticsearch{}, + } + Expect(validateHttpContentTypeHeaders(spec)).To(BeEmpty()) + }) + }) +})