From e5209016193acd3e8923319c66b4c5f7f56efa50 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Mon, 5 Jan 2026 15:04:49 +0100 Subject: [PATCH 01/36] Add log type include filter --- .../egress/syslog/filtering_drain_writer.go | 74 +++++++++++++++++-- src/pkg/egress/syslog/syslog_connector.go | 1 + src/pkg/ingress/bindings/binding_config.go | 43 +++++++++++ .../ingress/bindings/binding_config_test.go | 19 +++++ 4 files changed, 129 insertions(+), 8 deletions(-) diff --git a/src/pkg/egress/syslog/filtering_drain_writer.go b/src/pkg/egress/syslog/filtering_drain_writer.go index db6d56d90..65a0e26eb 100644 --- a/src/pkg/egress/syslog/filtering_drain_writer.go +++ b/src/pkg/egress/syslog/filtering_drain_writer.go @@ -2,6 +2,7 @@ package syslog import ( "errors" + "strings" "code.cloudfoundry.org/go-loggregator/v10/rpc/loggregator_v2" "code.cloudfoundry.org/loggregator-agent-release/src/pkg/egress" @@ -18,6 +19,32 @@ const ( LOGS_AND_METRICS ) +type LogType int + +// Ordered as in https://docs.cloudfoundry.org/devguide/deploy-apps/streaming-logs.html#format +const ( + API LogType = iota + STG + RTR + LGR + APP + SSH + CELL +) + +var logTypePrefixes = map[string]LogType{ + "API": API, + "STG": STG, + "RTR": RTR, + "LGR": LGR, + "APP": APP, + "SSH": SSH, + "CELL": CELL, +} + +// A set of LogTypes for efficient membership checking +type LogTypeSet map[LogType]struct{} + type FilteringDrainWriter struct { binding Binding writer egress.Writer @@ -50,7 +77,13 @@ func (w *FilteringDrainWriter) Write(env *loggregator_v2.Envelope) error { } } if env.GetLog() != nil { - if sendsLogs(w.binding.DrainData) { + // Default to sending logs if no source_type tag is present + value, ok := env.GetTags()["source_type"] + if !ok { + // TODO Log + value = "" + } + if sendsLogs(w.binding.DrainData, w.binding.LogFilter, value) { return w.writer.Write(env) } } @@ -63,17 +96,42 @@ func (w *FilteringDrainWriter) Write(env *loggregator_v2.Envelope) error { return nil } -func sendsLogs(drainData DrainData) bool { - switch drainData { - case LOGS: +func shouldIncludeLog(logFilter *LogTypeSet, sourceTypeTag string) bool { + // Empty filter or missing source type means no filtering + if logFilter == nil || sourceTypeTag == "" { return true - case LOGS_AND_METRICS: - return true - case LOGS_NO_EVENTS: + } + + // Find the first "/" to extract prefix + idx := strings.IndexByte(sourceTypeTag, '/') + prefix := sourceTypeTag + if idx != -1 { + prefix = sourceTypeTag[:idx] + } + + // Prefer map lookup over switch for performance + logType, known := logTypePrefixes[prefix] + if !known { + // Unknown log type, default to not filtering + // TODO log return true - default: + } + + _, exists := (*logFilter)[logType] + return exists +} + +func sendsLogs(drainData DrainData, logFilter *LogTypeSet, sourceTypeTag string) bool { + // TODO ALL appears to be special? + if drainData != LOGS && drainData != LOGS_AND_METRICS && drainData != LOGS_NO_EVENTS { return false } + + if shouldIncludeLog(logFilter, sourceTypeTag) { + return true + } + + return false } func sendsMetrics(drainData DrainData) bool { diff --git a/src/pkg/egress/syslog/syslog_connector.go b/src/pkg/egress/syslog/syslog_connector.go index 66134f24c..d46b580fc 100644 --- a/src/pkg/egress/syslog/syslog_connector.go +++ b/src/pkg/egress/syslog/syslog_connector.go @@ -19,6 +19,7 @@ type Binding struct { DrainData DrainData `json:"type,omitempty"` OmitMetadata bool InternalTls bool + LogFilter *LogTypeSet } type Drain struct { diff --git a/src/pkg/ingress/bindings/binding_config.go b/src/pkg/ingress/bindings/binding_config.go index 9a1931916..6d2614f9b 100644 --- a/src/pkg/ingress/bindings/binding_config.go +++ b/src/pkg/ingress/bindings/binding_config.go @@ -2,6 +2,7 @@ package bindings import ( "net/url" + "strings" "code.cloudfoundry.org/loggregator-agent-release/src/pkg/binding" "code.cloudfoundry.org/loggregator-agent-release/src/pkg/egress/syslog" @@ -35,6 +36,7 @@ func (d *DrainParamParser) FetchBindings() ([]syslog.Binding, error) { b.OmitMetadata = getOmitMetadata(urlParsed, d.defaultDrainMetadata) b.InternalTls = getInternalTLS(urlParsed) b.DrainData = getBindingType(urlParsed) + b.LogFilter = getLogFilter(urlParsed) processed = append(processed, b) } @@ -84,6 +86,47 @@ func getBindingType(u *url.URL) syslog.DrainData { return drainData } +// Parse HTML query parameter into a Set of LogTypes +func NewLogTypeSet(logTypeList string) *syslog.LogTypeSet { + if logTypeList == "" { + set := make(syslog.LogTypeSet) + return &set + } + + logTypes := strings.Split(logTypeList, ",") + set := make(syslog.LogTypeSet, len(logTypes)) + + for _, logType := range logTypes { + logType = strings.ToLower(strings.TrimSpace(logType)) + var t syslog.LogType + switch logType { + case "app": + t = syslog.APP + case "stg": + t = syslog.STG + case "rtr": + t = syslog.RTR + case "api": + t = syslog.API + case "lgr": + t = syslog.LGR + case "ssh": + t = syslog.SSH + case "cell": + t = syslog.CELL + default: + // TODO Log unknown log type + } + set[t] = struct{}{} + } + + return &set +} + +func getLogFilter(u *url.URL) *syslog.LogTypeSet { + return NewLogTypeSet(u.Query().Get("include-log-types")) +} + func getRemoveMetadataQuery(u *url.URL) string { q := u.Query().Get("disable-metadata") if q == "" { diff --git a/src/pkg/ingress/bindings/binding_config_test.go b/src/pkg/ingress/bindings/binding_config_test.go index ef9146e74..702a98eb7 100644 --- a/src/pkg/ingress/bindings/binding_config_test.go +++ b/src/pkg/ingress/bindings/binding_config_test.go @@ -88,6 +88,25 @@ var _ = Describe("Drain Param Config", func() { Expect(configedBindings[4].DrainData).To(Equal(syslog.ALL)) }) + It("sets drain filter appropriately'", func() { + bs := []syslog.Binding{ + {Drain: syslog.Drain{Url: "https://test.org/drain"}}, + {Drain: syslog.Drain{Url: "https://test.org/drain?include-log-types=app"}}, + {Drain: syslog.Drain{Url: "https://test.org/drain?include-log-types=app,stg,cell"}}, + // {Drain: syslog.Drain{Url: "https://test.org/drain?exclude-log-types=rtr,cell,stg"}}, + // {Drain: syslog.Drain{Url: "https://test.org/drain?exclude-log-types=rtr"}}, + } + f := newStubFetcher(bs, nil) + wf := bindings.NewDrainParamParser(f, true) + + configedBindings, _ := wf.FetchBindings() + Expect(configedBindings[0].LogFilter).To(Equal(bindings.NewLogTypeSet(""))) // Empty map defaults to all types + Expect(configedBindings[1].LogFilter).To(Equal(bindings.NewLogTypeSet("app"))) + Expect(configedBindings[2].LogFilter).To(Equal(bindings.NewLogTypeSet("app,stg,cell"))) + // Expect(configedBindings[3].LogFilter).To(Equal(bindings.NewLogTypeSet("api,lgr,app,ssh"))) + // Expect(configedBindings[4].LogFilter).To(Equal(bindings.NewLogTypeSet("api,stg,lgr,app,ssh,cell"))) + }) + It("sets drain data for old parameter appropriately'", func() { bs := []syslog.Binding{ {Drain: syslog.Drain{Url: "https://test.org/drain?drain-type=metrics"}}, From 6084cff5022d2c430a4b3dcb3f2d2a699fc4d1a3 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Mon, 5 Jan 2026 17:35:21 +0100 Subject: [PATCH 02/36] Support exclude and include --- src/pkg/ingress/bindings/binding_config.go | 29 +++++++++++++++++-- .../ingress/bindings/binding_config_test.go | 22 +++++++++----- 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/src/pkg/ingress/bindings/binding_config.go b/src/pkg/ingress/bindings/binding_config.go index 6d2614f9b..6cdbb75c6 100644 --- a/src/pkg/ingress/bindings/binding_config.go +++ b/src/pkg/ingress/bindings/binding_config.go @@ -87,7 +87,7 @@ func getBindingType(u *url.URL) syslog.DrainData { } // Parse HTML query parameter into a Set of LogTypes -func NewLogTypeSet(logTypeList string) *syslog.LogTypeSet { +func NewLogTypeSet(logTypeList string, isExclude bool) *syslog.LogTypeSet { if logTypeList == "" { set := make(syslog.LogTypeSet) return &set @@ -116,15 +116,40 @@ func NewLogTypeSet(logTypeList string) *syslog.LogTypeSet { t = syslog.CELL default: // TODO Log unknown log type + continue } set[t] = struct{}{} } + if isExclude { + // Invert the set + fullSet := make(syslog.LogTypeSet) + allLogTypes := []syslog.LogType{syslog.API, syslog.STG, syslog.RTR, syslog.LGR, syslog.APP, syslog.SSH, syslog.CELL} + for _, t := range allLogTypes { + fullSet[t] = struct{}{} + } + + for t := range set { + delete(fullSet, t) + } + return &fullSet + } + return &set } func getLogFilter(u *url.URL) *syslog.LogTypeSet { - return NewLogTypeSet(u.Query().Get("include-log-types")) + includeLogTypes := u.Query().Get("include-log-types") + excludeLogTypes := u.Query().Get("exclude-log-types") + + if excludeLogTypes != "" && includeLogTypes != "" { + // TODO return errors.New("include-log-types and exclude-log-types can not be used at the same time") + } else if excludeLogTypes != "" { + return NewLogTypeSet(excludeLogTypes, true) + } else if includeLogTypes != "" { + return NewLogTypeSet(includeLogTypes, false) + } + return NewLogTypeSet("", false) } func getRemoveMetadataQuery(u *url.URL) string { diff --git a/src/pkg/ingress/bindings/binding_config_test.go b/src/pkg/ingress/bindings/binding_config_test.go index 702a98eb7..139951e26 100644 --- a/src/pkg/ingress/bindings/binding_config_test.go +++ b/src/pkg/ingress/bindings/binding_config_test.go @@ -93,18 +93,18 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain"}}, {Drain: syslog.Drain{Url: "https://test.org/drain?include-log-types=app"}}, {Drain: syslog.Drain{Url: "https://test.org/drain?include-log-types=app,stg,cell"}}, - // {Drain: syslog.Drain{Url: "https://test.org/drain?exclude-log-types=rtr,cell,stg"}}, - // {Drain: syslog.Drain{Url: "https://test.org/drain?exclude-log-types=rtr"}}, + {Drain: syslog.Drain{Url: "https://test.org/drain?exclude-log-types=rtr,cell,stg"}}, + {Drain: syslog.Drain{Url: "https://test.org/drain?exclude-log-types=rtr"}}, } f := newStubFetcher(bs, nil) wf := bindings.NewDrainParamParser(f, true) configedBindings, _ := wf.FetchBindings() - Expect(configedBindings[0].LogFilter).To(Equal(bindings.NewLogTypeSet(""))) // Empty map defaults to all types - Expect(configedBindings[1].LogFilter).To(Equal(bindings.NewLogTypeSet("app"))) - Expect(configedBindings[2].LogFilter).To(Equal(bindings.NewLogTypeSet("app,stg,cell"))) - // Expect(configedBindings[3].LogFilter).To(Equal(bindings.NewLogTypeSet("api,lgr,app,ssh"))) - // Expect(configedBindings[4].LogFilter).To(Equal(bindings.NewLogTypeSet("api,stg,lgr,app,ssh,cell"))) + Expect(configedBindings[0].LogFilter).To(Equal(NewLogTypeSet())) // Empty map defaults to all types + Expect(configedBindings[1].LogFilter).To(Equal(NewLogTypeSet(syslog.APP))) + Expect(configedBindings[2].LogFilter).To(Equal(NewLogTypeSet(syslog.APP, syslog.STG, syslog.CELL))) + Expect(configedBindings[3].LogFilter).To(Equal(NewLogTypeSet(syslog.API, syslog.LGR, syslog.APP, syslog.SSH))) + Expect(configedBindings[4].LogFilter).To(Equal(NewLogTypeSet(syslog.API, syslog.STG, syslog.LGR, syslog.APP, syslog.SSH, syslog.CELL))) }) It("sets drain data for old parameter appropriately'", func() { @@ -170,3 +170,11 @@ func (f *stubFetcher) FetchBindings() ([]syslog.Binding, error) { func (f *stubFetcher) DrainLimit() int { return -1 } + +func NewLogTypeSet(logTypes ...syslog.LogType) *syslog.LogTypeSet { + set := make(syslog.LogTypeSet, len(logTypes)) + for _, t := range logTypes { + set[t] = struct{}{} + } + return &set +} From 13eb748baf43fd9eb1915da98ccf470d4e3841a0 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Mon, 5 Jan 2026 17:43:39 +0100 Subject: [PATCH 03/36] Error when using both include and exclude types --- src/pkg/ingress/bindings/binding_config.go | 16 ++++++++++------ src/pkg/ingress/bindings/binding_config_test.go | 12 ++++++++++++ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/pkg/ingress/bindings/binding_config.go b/src/pkg/ingress/bindings/binding_config.go index 6cdbb75c6..8ab5dbe4c 100644 --- a/src/pkg/ingress/bindings/binding_config.go +++ b/src/pkg/ingress/bindings/binding_config.go @@ -1,6 +1,7 @@ package bindings import ( + "errors" "net/url" "strings" @@ -36,7 +37,10 @@ func (d *DrainParamParser) FetchBindings() ([]syslog.Binding, error) { b.OmitMetadata = getOmitMetadata(urlParsed, d.defaultDrainMetadata) b.InternalTls = getInternalTLS(urlParsed) b.DrainData = getBindingType(urlParsed) - b.LogFilter = getLogFilter(urlParsed) + b.LogFilter, err = getLogFilter(urlParsed) + if err != nil { + return nil, err + } processed = append(processed, b) } @@ -138,18 +142,18 @@ func NewLogTypeSet(logTypeList string, isExclude bool) *syslog.LogTypeSet { return &set } -func getLogFilter(u *url.URL) *syslog.LogTypeSet { +func getLogFilter(u *url.URL) (*syslog.LogTypeSet, error) { includeLogTypes := u.Query().Get("include-log-types") excludeLogTypes := u.Query().Get("exclude-log-types") if excludeLogTypes != "" && includeLogTypes != "" { - // TODO return errors.New("include-log-types and exclude-log-types can not be used at the same time") + return nil, errors.New("include-log-types and exclude-log-types can not be used at the same time") } else if excludeLogTypes != "" { - return NewLogTypeSet(excludeLogTypes, true) + return NewLogTypeSet(excludeLogTypes, true), nil } else if includeLogTypes != "" { - return NewLogTypeSet(includeLogTypes, false) + return NewLogTypeSet(includeLogTypes, false), nil } - return NewLogTypeSet("", false) + return NewLogTypeSet("", false), nil } func getRemoveMetadataQuery(u *url.URL) string { diff --git a/src/pkg/ingress/bindings/binding_config_test.go b/src/pkg/ingress/bindings/binding_config_test.go index 139951e26..76b5c6133 100644 --- a/src/pkg/ingress/bindings/binding_config_test.go +++ b/src/pkg/ingress/bindings/binding_config_test.go @@ -107,6 +107,18 @@ var _ = Describe("Drain Param Config", func() { Expect(configedBindings[4].LogFilter).To(Equal(NewLogTypeSet(syslog.API, syslog.STG, syslog.LGR, syslog.APP, syslog.SSH, syslog.CELL))) }) + It("returns an error when both include-log-types and exclude-log-types are specified", func() { + bs := []syslog.Binding{ + {Drain: syslog.Drain{Url: "https://test.org/drain?include-log-types=app&exclude-log-types=rtr"}}, + } + f := newStubFetcher(bs, nil) + wf := bindings.NewDrainParamParser(f, true) + + configedBindings, err := wf.FetchBindings() + Expect(err).To(HaveOccurred()) + Expect(configedBindings).To(HaveLen(0)) + }) + It("sets drain data for old parameter appropriately'", func() { bs := []syslog.Binding{ {Drain: syslog.Drain{Url: "https://test.org/drain?drain-type=metrics"}}, From 0e81cde23948ee2673043d38a3584c730c3c1e9e Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Thu, 8 Jan 2026 09:42:42 +0100 Subject: [PATCH 04/36] Introduce logging to binding config --- src/cmd/syslog-agent/app/syslog_agent.go | 4 +- .../egress/syslog/filtering_drain_writer.go | 12 ++-- src/pkg/ingress/bindings/binding_config.go | 23 +++++--- .../ingress/bindings/binding_config_test.go | 56 +++++++++++++++---- 4 files changed, 69 insertions(+), 26 deletions(-) diff --git a/src/cmd/syslog-agent/app/syslog_agent.go b/src/cmd/syslog-agent/app/syslog_agent.go index d200ea359..ffe2df27f 100644 --- a/src/cmd/syslog-agent/app/syslog_agent.go +++ b/src/cmd/syslog-agent/app/syslog_agent.go @@ -107,13 +107,13 @@ func NewSyslogAgent( cfg.WarnOnInvalidDrains, l, ) - cupsFetcher = bindings.NewDrainParamParser(cupsFetcher, cfg.DefaultDrainMetadata) + cupsFetcher = bindings.NewDrainParamParser(cupsFetcher, cfg.DefaultDrainMetadata, l) } aggregateFetcher := bindings.NewAggregateDrainFetcher(cfg.AggregateDrainURLs, cacheClient) bindingManager := binding.NewManager( cupsFetcher, - bindings.NewDrainParamParser(aggregateFetcher, cfg.DefaultDrainMetadata), + bindings.NewDrainParamParser(aggregateFetcher, cfg.DefaultDrainMetadata, l), connector, m, cfg.Cache.PollingInterval, diff --git a/src/pkg/egress/syslog/filtering_drain_writer.go b/src/pkg/egress/syslog/filtering_drain_writer.go index 65a0e26eb..275c4fcca 100644 --- a/src/pkg/egress/syslog/filtering_drain_writer.go +++ b/src/pkg/egress/syslog/filtering_drain_writer.go @@ -19,9 +19,10 @@ const ( LOGS_AND_METRICS ) +// LogType defines the log types used within Cloud Foundry +// Their order in the code is as documented in https://docs.cloudfoundry.org/devguide/deploy-apps/streaming-logs.html#format type LogType int -// Ordered as in https://docs.cloudfoundry.org/devguide/deploy-apps/streaming-logs.html#format const ( API LogType = iota STG @@ -32,6 +33,7 @@ const ( CELL ) +// logTypePrefixes maps string prefixes to LogType values for efficient lookup var logTypePrefixes = map[string]LogType{ "API": API, "STG": STG, @@ -42,7 +44,7 @@ var logTypePrefixes = map[string]LogType{ "CELL": CELL, } -// A set of LogTypes for efficient membership checking +// LogTypeSet is a set of LogTypes for efficient membership checking type LogTypeSet map[LogType]struct{} type FilteringDrainWriter struct { @@ -80,7 +82,8 @@ func (w *FilteringDrainWriter) Write(env *loggregator_v2.Envelope) error { // Default to sending logs if no source_type tag is present value, ok := env.GetTags()["source_type"] if !ok { - // TODO Log + // TODO add unit test for case where source_type tag is missing + // source_type tag is missing, default to sending logs value = "" } if sendsLogs(w.binding.DrainData, w.binding.LogFilter, value) { @@ -96,6 +99,7 @@ func (w *FilteringDrainWriter) Write(env *loggregator_v2.Envelope) error { return nil } +// shouldIncludeLog determines if a log with the given sourceTypeTag should be forwarded func shouldIncludeLog(logFilter *LogTypeSet, sourceTypeTag string) bool { // Empty filter or missing source type means no filtering if logFilter == nil || sourceTypeTag == "" { @@ -113,7 +117,7 @@ func shouldIncludeLog(logFilter *LogTypeSet, sourceTypeTag string) bool { logType, known := logTypePrefixes[prefix] if !known { // Unknown log type, default to not filtering - // TODO log + // TODO unit test return true } diff --git a/src/pkg/ingress/bindings/binding_config.go b/src/pkg/ingress/bindings/binding_config.go index 8ab5dbe4c..c3d023d3a 100644 --- a/src/pkg/ingress/bindings/binding_config.go +++ b/src/pkg/ingress/bindings/binding_config.go @@ -2,6 +2,7 @@ package bindings import ( "errors" + "log" "net/url" "strings" @@ -12,12 +13,14 @@ import ( type DrainParamParser struct { fetcher binding.Fetcher defaultDrainMetadata bool + log *log.Logger } -func NewDrainParamParser(f binding.Fetcher, defaultDrainMetadata bool) *DrainParamParser { +func NewDrainParamParser(f binding.Fetcher, defaultDrainMetadata bool, l *log.Logger) *DrainParamParser { return &DrainParamParser{ fetcher: f, defaultDrainMetadata: defaultDrainMetadata, + log: l, } } @@ -37,7 +40,7 @@ func (d *DrainParamParser) FetchBindings() ([]syslog.Binding, error) { b.OmitMetadata = getOmitMetadata(urlParsed, d.defaultDrainMetadata) b.InternalTls = getInternalTLS(urlParsed) b.DrainData = getBindingType(urlParsed) - b.LogFilter, err = getLogFilter(urlParsed) + b.LogFilter, err = d.getLogFilter(urlParsed) if err != nil { return nil, err } @@ -90,8 +93,8 @@ func getBindingType(u *url.URL) syslog.DrainData { return drainData } -// Parse HTML query parameter into a Set of LogTypes -func NewLogTypeSet(logTypeList string, isExclude bool) *syslog.LogTypeSet { +// NewLogTypeSet parses an HTML query parameter into a Set of LogTypes +func (d *DrainParamParser) NewLogTypeSet(logTypeList string, isExclude bool) *syslog.LogTypeSet { if logTypeList == "" { set := make(syslog.LogTypeSet) return &set @@ -119,7 +122,9 @@ func NewLogTypeSet(logTypeList string, isExclude bool) *syslog.LogTypeSet { case "cell": t = syslog.CELL default: - // TODO Log unknown log type + // Unknown log type, skip it + // TODO add unit test for unknown log type + d.log.Printf("Unknown log type '%s' in log type filter, ignoring", logType) continue } set[t] = struct{}{} @@ -142,18 +147,18 @@ func NewLogTypeSet(logTypeList string, isExclude bool) *syslog.LogTypeSet { return &set } -func getLogFilter(u *url.URL) (*syslog.LogTypeSet, error) { +func (d *DrainParamParser) getLogFilter(u *url.URL) (*syslog.LogTypeSet, error) { includeLogTypes := u.Query().Get("include-log-types") excludeLogTypes := u.Query().Get("exclude-log-types") if excludeLogTypes != "" && includeLogTypes != "" { return nil, errors.New("include-log-types and exclude-log-types can not be used at the same time") } else if excludeLogTypes != "" { - return NewLogTypeSet(excludeLogTypes, true), nil + return d.NewLogTypeSet(excludeLogTypes, true), nil } else if includeLogTypes != "" { - return NewLogTypeSet(includeLogTypes, false), nil + return d.NewLogTypeSet(includeLogTypes, false), nil } - return NewLogTypeSet("", false), nil + return d.NewLogTypeSet("", false), nil } func getRemoveMetadataQuery(u *url.URL) string { diff --git a/src/pkg/ingress/bindings/binding_config_test.go b/src/pkg/ingress/bindings/binding_config_test.go index 76b5c6133..97edd0eb1 100644 --- a/src/pkg/ingress/bindings/binding_config_test.go +++ b/src/pkg/ingress/bindings/binding_config_test.go @@ -2,6 +2,8 @@ package bindings_test import ( "errors" + "log" + "strings" "code.cloudfoundry.org/loggregator-agent-release/src/pkg/egress/syslog" "code.cloudfoundry.org/loggregator-agent-release/src/pkg/ingress/bindings" @@ -10,12 +12,15 @@ import ( ) var _ = Describe("Drain Param Config", func() { + var ( + logger = log.New(GinkgoWriter, "", 0) + ) It("sets OmitMetadata to false if the drain doesn't contain 'disable-metadata=true'", func() { bs := []syslog.Binding{ {Drain: syslog.Drain{Url: "https://test.org/drain"}}, } f := newStubFetcher(bs, nil) - wf := bindings.NewDrainParamParser(f, true) + wf := bindings.NewDrainParamParser(f, true, logger) configedBindings, _ := wf.FetchBindings() Expect(configedBindings[0].OmitMetadata).To(BeFalse()) @@ -27,7 +32,7 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain?omit-metadata=true"}}, } f := newStubFetcher(bs, nil) - wf := bindings.NewDrainParamParser(f, true) + wf := bindings.NewDrainParamParser(f, true, logger) configedBindings, _ := wf.FetchBindings() Expect(configedBindings[0].OmitMetadata).To(BeTrue()) @@ -39,7 +44,7 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain"}}, } f := newStubFetcher(bs, nil) - wf := bindings.NewDrainParamParser(f, false) + wf := bindings.NewDrainParamParser(f, false, logger) configedBindings, _ := wf.FetchBindings() Expect(configedBindings[0].OmitMetadata).To(BeTrue()) @@ -51,7 +56,7 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain?omit-metadata=false"}}, } f := newStubFetcher(bs, nil) - wf := bindings.NewDrainParamParser(f, false) + wf := bindings.NewDrainParamParser(f, false, logger) configedBindings, _ := wf.FetchBindings() Expect(configedBindings[0].OmitMetadata).To(BeFalse()) @@ -63,7 +68,7 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain?ssl-strict-internal=true"}}, } f := newStubFetcher(bs, nil) - wf := bindings.NewDrainParamParser(f, true) + wf := bindings.NewDrainParamParser(f, true, logger) configedBindings, _ := wf.FetchBindings() Expect(configedBindings[0].InternalTls).To(BeTrue()) @@ -78,7 +83,7 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain?drain-data=all"}}, } f := newStubFetcher(bs, nil) - wf := bindings.NewDrainParamParser(f, true) + wf := bindings.NewDrainParamParser(f, true, logger) configedBindings, _ := wf.FetchBindings() Expect(configedBindings[0].DrainData).To(Equal(syslog.LOGS)) @@ -97,7 +102,7 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain?exclude-log-types=rtr"}}, } f := newStubFetcher(bs, nil) - wf := bindings.NewDrainParamParser(f, true) + wf := bindings.NewDrainParamParser(f, true, logger) configedBindings, _ := wf.FetchBindings() Expect(configedBindings[0].LogFilter).To(Equal(NewLogTypeSet())) // Empty map defaults to all types @@ -112,13 +117,42 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain?include-log-types=app&exclude-log-types=rtr"}}, } f := newStubFetcher(bs, nil) - wf := bindings.NewDrainParamParser(f, true) + wf := bindings.NewDrainParamParser(f, true, logger) configedBindings, err := wf.FetchBindings() Expect(err).To(HaveOccurred()) Expect(configedBindings).To(HaveLen(0)) }) + It("logs a warning when an unknown log type is provided", func() { + var logOutput strings.Builder + testLogger := log.New(&logOutput, "", log.LstdFlags) + parser := bindings.NewDrainParamParser(newStubFetcher(nil, nil), false, testLogger) + + result := parser.NewLogTypeSet("app,unknown,rtr", false) + + // Should only contain APP and RTR, not the unknown type + Expect(result).To(Equal(NewLogTypeSet(syslog.APP, syslog.RTR))) + + // Should have logged a warning + Expect(logOutput.String()).To(ContainSubstring("ignoring")) + }) + + It("handles unknown log types in exclude mode", func() { + var logOutput strings.Builder + testLogger := log.New(&logOutput, "", log.LstdFlags) + parser := bindings.NewDrainParamParser(newStubFetcher(nil, nil), false, testLogger) + + result := parser.NewLogTypeSet("rtr,unknown", true) + + // Should exclude only RTR (unknown type is ignored) + expectedSet := NewLogTypeSet(syslog.API, syslog.STG, syslog.LGR, syslog.APP, syslog.SSH, syslog.CELL) + Expect(result).To(Equal(expectedSet)) + + // Should have logged a warning + Expect(logOutput.String()).To(ContainSubstring("ignoring")) + }) + It("sets drain data for old parameter appropriately'", func() { bs := []syslog.Binding{ {Drain: syslog.Drain{Url: "https://test.org/drain?drain-type=metrics"}}, @@ -128,7 +162,7 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain?include-metrics-deprecated=true"}}, } f := newStubFetcher(bs, nil) - wf := bindings.NewDrainParamParser(f, true) + wf := bindings.NewDrainParamParser(f, true, logger) configedBindings, _ := wf.FetchBindings() Expect(configedBindings[0].DrainData).To(Equal(syslog.METRICS)) @@ -145,7 +179,7 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain?omit-metadata=true"}}, } f := newStubFetcher(bs, nil) - wf := bindings.NewDrainParamParser(f, true) + wf := bindings.NewDrainParamParser(f, true, logger) configedBindings, err := wf.FetchBindings() Expect(err).ToNot(HaveOccurred()) @@ -156,7 +190,7 @@ var _ = Describe("Drain Param Config", func() { It("Returns a error when fetching fails", func() { f := newStubFetcher(nil, errors.New("Ahhh an error")) - wf := bindings.NewDrainParamParser(f, true) + wf := bindings.NewDrainParamParser(f, true, logger) _, err := wf.FetchBindings() Expect(err).To(MatchError("Ahhh an error")) From a7855b60b5daad48058c89f1950ddf3e4ae8df47 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Thu, 8 Jan 2026 10:08:17 +0100 Subject: [PATCH 05/36] Add some more unit tests --- .../egress/syslog/filtering_drain_writer.go | 6 +-- .../syslog/filtering_drain_writer_test.go | 54 +++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/src/pkg/egress/syslog/filtering_drain_writer.go b/src/pkg/egress/syslog/filtering_drain_writer.go index 275c4fcca..c5ec9d53d 100644 --- a/src/pkg/egress/syslog/filtering_drain_writer.go +++ b/src/pkg/egress/syslog/filtering_drain_writer.go @@ -79,11 +79,9 @@ func (w *FilteringDrainWriter) Write(env *loggregator_v2.Envelope) error { } } if env.GetLog() != nil { - // Default to sending logs if no source_type tag is present value, ok := env.GetTags()["source_type"] if !ok { - // TODO add unit test for case where source_type tag is missing - // source_type tag is missing, default to sending logs + // Default to sending logs if no source_type tag is present value = "" } if sendsLogs(w.binding.DrainData, w.binding.LogFilter, value) { @@ -117,7 +115,6 @@ func shouldIncludeLog(logFilter *LogTypeSet, sourceTypeTag string) bool { logType, known := logTypePrefixes[prefix] if !known { // Unknown log type, default to not filtering - // TODO unit test return true } @@ -126,7 +123,6 @@ func shouldIncludeLog(logFilter *LogTypeSet, sourceTypeTag string) bool { } func sendsLogs(drainData DrainData, logFilter *LogTypeSet, sourceTypeTag string) bool { - // TODO ALL appears to be special? if drainData != LOGS && drainData != LOGS_AND_METRICS && drainData != LOGS_NO_EVENTS { return false } diff --git a/src/pkg/egress/syslog/filtering_drain_writer_test.go b/src/pkg/egress/syslog/filtering_drain_writer_test.go index 020e0b157..c2ef0a564 100644 --- a/src/pkg/egress/syslog/filtering_drain_writer_test.go +++ b/src/pkg/egress/syslog/filtering_drain_writer_test.go @@ -53,6 +53,60 @@ var _ = Describe("Filtering Drain Writer", func() { _, err := syslog.NewFilteringDrainWriter(binding, &fakeWriter{}) Expect(err).To(HaveOccurred()) }) + + It("sends logs when source_type tag is missing", func() { + binding := syslog.Binding{ + DrainData: syslog.LOGS, + LogFilter: nil, + } + fakeWriter := &fakeWriter{} + drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) + Expect(err).NotTo(HaveOccurred()) + + envelope := &loggregator_v2.Envelope{ + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{ + Payload: []byte("test log"), + }, + }, + Tags: map[string]string{ + // source_type tag is intentionally missing + }, + } + + err = drainWriter.Write(envelope) + + Expect(err).NotTo(HaveOccurred()) + Expect(fakeWriter.received).To(Equal(1)) + }) + + It("sends logs with unknown source_type prefix when filter is set", func() { + appFilter := syslog.LogTypeSet{syslog.APP: struct{}{}} + binding := syslog.Binding{ + DrainData: syslog.LOGS, + LogFilter: &appFilter, + } + fakeWriter := &fakeWriter{} + drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) + Expect(err).NotTo(HaveOccurred()) + + envelope := &loggregator_v2.Envelope{ + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{ + Payload: []byte("test log"), + }, + }, + Tags: map[string]string{ + "source_type": "UNKNOWN/some/path", + }, + } + + err = drainWriter.Write(envelope) + + // Should send the log because unknown types default to being included + Expect(err).NotTo(HaveOccurred()) + Expect(fakeWriter.received).To(Equal(1)) + }) }) type fakeWriter struct { From 23e92ed5f56f4a17b629e0aef53469db661a03e1 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Mon, 12 Jan 2026 08:55:24 +0100 Subject: [PATCH 06/36] Fix wrong word --- src/pkg/ingress/bindings/binding_config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pkg/ingress/bindings/binding_config.go b/src/pkg/ingress/bindings/binding_config.go index c3d023d3a..9f3bd03c1 100644 --- a/src/pkg/ingress/bindings/binding_config.go +++ b/src/pkg/ingress/bindings/binding_config.go @@ -93,7 +93,7 @@ func getBindingType(u *url.URL) syslog.DrainData { return drainData } -// NewLogTypeSet parses an HTML query parameter into a Set of LogTypes +// NewLogTypeSet parses a URL query parameter into a Set of LogTypes func (d *DrainParamParser) NewLogTypeSet(logTypeList string, isExclude bool) *syslog.LogTypeSet { if logTypeList == "" { set := make(syslog.LogTypeSet) From 553cd46f8a561107182c389eb2c7c0e5e20ff994 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Mon, 12 Jan 2026 15:52:10 +0100 Subject: [PATCH 07/36] Create LogType as string, move to package --- .../egress/syslog/filtering_drain_writer.go | 35 +----------- src/pkg/egress/syslog/log.go | 55 +++++++++++++++++++ src/pkg/ingress/bindings/binding_config.go | 39 +++++-------- 3 files changed, 72 insertions(+), 57 deletions(-) create mode 100644 src/pkg/egress/syslog/log.go diff --git a/src/pkg/egress/syslog/filtering_drain_writer.go b/src/pkg/egress/syslog/filtering_drain_writer.go index c5ec9d53d..8eaaa46bb 100644 --- a/src/pkg/egress/syslog/filtering_drain_writer.go +++ b/src/pkg/egress/syslog/filtering_drain_writer.go @@ -19,34 +19,6 @@ const ( LOGS_AND_METRICS ) -// LogType defines the log types used within Cloud Foundry -// Their order in the code is as documented in https://docs.cloudfoundry.org/devguide/deploy-apps/streaming-logs.html#format -type LogType int - -const ( - API LogType = iota - STG - RTR - LGR - APP - SSH - CELL -) - -// logTypePrefixes maps string prefixes to LogType values for efficient lookup -var logTypePrefixes = map[string]LogType{ - "API": API, - "STG": STG, - "RTR": RTR, - "LGR": LGR, - "APP": APP, - "SSH": SSH, - "CELL": CELL, -} - -// LogTypeSet is a set of LogTypes for efficient membership checking -type LogTypeSet map[LogType]struct{} - type FilteringDrainWriter struct { binding Binding writer egress.Writer @@ -112,14 +84,13 @@ func shouldIncludeLog(logFilter *LogTypeSet, sourceTypeTag string) bool { } // Prefer map lookup over switch for performance - logType, known := logTypePrefixes[prefix] - if !known { + logType := LogType(prefix) + if !logType.IsValid() { // Unknown log type, default to not filtering return true } - _, exists := (*logFilter)[logType] - return exists + return logFilter.Contains(logType) } func sendsLogs(drainData DrainData, logFilter *LogTypeSet, sourceTypeTag string) bool { diff --git a/src/pkg/egress/syslog/log.go b/src/pkg/egress/syslog/log.go new file mode 100644 index 000000000..2e94f6b6c --- /dev/null +++ b/src/pkg/egress/syslog/log.go @@ -0,0 +1,55 @@ +package syslog + +// LogType defines the log types used within Cloud Foundry +// Their order in the code is as documented in https://docs.cloudfoundry.org/devguide/deploy-apps/streaming-logs.html#format +type LogType string + +const ( + API LogType = "API" + STG LogType = "STG" + RTR LogType = "RTR" + LGR LogType = "LGR" + APP LogType = "APP" + SSH LogType = "SSH" + CELL LogType = "CELL" +) + +// validLogTypes contains LogType prefixes for efficient lookup +var validLogTypes = map[LogType]struct{}{ + API: {}, + STG: {}, + RTR: {}, + LGR: {}, + APP: {}, + SSH: {}, + CELL: {}, +} + +// IsValid checks if the provided LogType is valid +func (lt LogType) IsValid() bool { + _, ok := validLogTypes[lt] + return ok +} + +// AllLogTypes returns all valid log types +func AllLogTypes() []LogType { + types := make([]LogType, 0, len(validLogTypes)) + for t := range validLogTypes { + types = append(types, t) + } + return types +} + +// LogTypeSet is a set of LogTypes for efficient membership checking +type LogTypeSet map[LogType]struct{} + +// Add adds a LogType to the set +func (s LogTypeSet) Add(lt LogType) { + s[lt] = struct{}{} +} + +// Contains checks if the set contains a LogType +func (s LogTypeSet) Contains(lt LogType) bool { + _, exists := s[lt] + return exists +} diff --git a/src/pkg/ingress/bindings/binding_config.go b/src/pkg/ingress/bindings/binding_config.go index 9f3bd03c1..c9d7bf59c 100644 --- a/src/pkg/ingress/bindings/binding_config.go +++ b/src/pkg/ingress/bindings/binding_config.go @@ -93,6 +93,12 @@ func getBindingType(u *url.URL) syslog.DrainData { return drainData } +// parseLogType parses a string into a LogType value +func parseLogType(s string) (syslog.LogType, bool) { + lt := syslog.LogType(strings.ToUpper(s)) + return lt, lt.IsValid() +} + // NewLogTypeSet parses a URL query parameter into a Set of LogTypes func (d *DrainParamParser) NewLogTypeSet(logTypeList string, isExclude bool) *syslog.LogTypeSet { if logTypeList == "" { @@ -104,38 +110,21 @@ func (d *DrainParamParser) NewLogTypeSet(logTypeList string, isExclude bool) *sy set := make(syslog.LogTypeSet, len(logTypes)) for _, logType := range logTypes { - logType = strings.ToLower(strings.TrimSpace(logType)) - var t syslog.LogType - switch logType { - case "app": - t = syslog.APP - case "stg": - t = syslog.STG - case "rtr": - t = syslog.RTR - case "api": - t = syslog.API - case "lgr": - t = syslog.LGR - case "ssh": - t = syslog.SSH - case "cell": - t = syslog.CELL - default: - // Unknown log type, skip it - // TODO add unit test for unknown log type + logType = strings.TrimSpace(logType) + t, ok := parseLogType(logType) + if !ok { d.log.Printf("Unknown log type '%s' in log type filter, ignoring", logType) continue } - set[t] = struct{}{} + set.Add(t) } if isExclude { - // Invert the set + // Invert the set - include all types except those in the set fullSet := make(syslog.LogTypeSet) - allLogTypes := []syslog.LogType{syslog.API, syslog.STG, syslog.RTR, syslog.LGR, syslog.APP, syslog.SSH, syslog.CELL} - for _, t := range allLogTypes { - fullSet[t] = struct{}{} + + for _, t := range syslog.AllLogTypes() { + fullSet.Add(t) } for t := range set { From 27cf2beb5585f39c7b8d092fa184de5f50c5a4bc Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Mon, 12 Jan 2026 16:59:06 +0100 Subject: [PATCH 08/36] Prefix log type constants with LOG_ --- .../syslog/filtering_drain_writer_test.go | 2 +- src/pkg/egress/syslog/log.go | 28 +++++++++---------- .../ingress/bindings/binding_config_test.go | 12 ++++---- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/pkg/egress/syslog/filtering_drain_writer_test.go b/src/pkg/egress/syslog/filtering_drain_writer_test.go index c2ef0a564..f5c0e6c42 100644 --- a/src/pkg/egress/syslog/filtering_drain_writer_test.go +++ b/src/pkg/egress/syslog/filtering_drain_writer_test.go @@ -81,7 +81,7 @@ var _ = Describe("Filtering Drain Writer", func() { }) It("sends logs with unknown source_type prefix when filter is set", func() { - appFilter := syslog.LogTypeSet{syslog.APP: struct{}{}} + appFilter := syslog.LogTypeSet{syslog.LOG_APP: struct{}{}} binding := syslog.Binding{ DrainData: syslog.LOGS, LogFilter: &appFilter, diff --git a/src/pkg/egress/syslog/log.go b/src/pkg/egress/syslog/log.go index 2e94f6b6c..1e0338da1 100644 --- a/src/pkg/egress/syslog/log.go +++ b/src/pkg/egress/syslog/log.go @@ -5,24 +5,24 @@ package syslog type LogType string const ( - API LogType = "API" - STG LogType = "STG" - RTR LogType = "RTR" - LGR LogType = "LGR" - APP LogType = "APP" - SSH LogType = "SSH" - CELL LogType = "CELL" + LOG_API LogType = "API" + LOG_STG LogType = "STG" + LOG_RTR LogType = "RTR" + LOG_LGR LogType = "LGR" + LOG_APP LogType = "APP" + LOG_SSH LogType = "SSH" + LOG_CELL LogType = "CELL" ) // validLogTypes contains LogType prefixes for efficient lookup var validLogTypes = map[LogType]struct{}{ - API: {}, - STG: {}, - RTR: {}, - LGR: {}, - APP: {}, - SSH: {}, - CELL: {}, + LOG_API: {}, + LOG_STG: {}, + LOG_RTR: {}, + LOG_LGR: {}, + LOG_APP: {}, + LOG_SSH: {}, + LOG_CELL: {}, } // IsValid checks if the provided LogType is valid diff --git a/src/pkg/ingress/bindings/binding_config_test.go b/src/pkg/ingress/bindings/binding_config_test.go index 97edd0eb1..0a2ba9138 100644 --- a/src/pkg/ingress/bindings/binding_config_test.go +++ b/src/pkg/ingress/bindings/binding_config_test.go @@ -106,10 +106,10 @@ var _ = Describe("Drain Param Config", func() { configedBindings, _ := wf.FetchBindings() Expect(configedBindings[0].LogFilter).To(Equal(NewLogTypeSet())) // Empty map defaults to all types - Expect(configedBindings[1].LogFilter).To(Equal(NewLogTypeSet(syslog.APP))) - Expect(configedBindings[2].LogFilter).To(Equal(NewLogTypeSet(syslog.APP, syslog.STG, syslog.CELL))) - Expect(configedBindings[3].LogFilter).To(Equal(NewLogTypeSet(syslog.API, syslog.LGR, syslog.APP, syslog.SSH))) - Expect(configedBindings[4].LogFilter).To(Equal(NewLogTypeSet(syslog.API, syslog.STG, syslog.LGR, syslog.APP, syslog.SSH, syslog.CELL))) + Expect(configedBindings[1].LogFilter).To(Equal(NewLogTypeSet(syslog.LOG_APP))) + Expect(configedBindings[2].LogFilter).To(Equal(NewLogTypeSet(syslog.LOG_APP, syslog.LOG_STG, syslog.LOG_CELL))) + Expect(configedBindings[3].LogFilter).To(Equal(NewLogTypeSet(syslog.LOG_API, syslog.LOG_LGR, syslog.LOG_APP, syslog.LOG_SSH))) + Expect(configedBindings[4].LogFilter).To(Equal(NewLogTypeSet(syslog.LOG_API, syslog.LOG_STG, syslog.LOG_LGR, syslog.LOG_APP, syslog.LOG_SSH, syslog.LOG_CELL))) }) It("returns an error when both include-log-types and exclude-log-types are specified", func() { @@ -132,7 +132,7 @@ var _ = Describe("Drain Param Config", func() { result := parser.NewLogTypeSet("app,unknown,rtr", false) // Should only contain APP and RTR, not the unknown type - Expect(result).To(Equal(NewLogTypeSet(syslog.APP, syslog.RTR))) + Expect(result).To(Equal(NewLogTypeSet(syslog.LOG_APP, syslog.LOG_RTR))) // Should have logged a warning Expect(logOutput.String()).To(ContainSubstring("ignoring")) @@ -146,7 +146,7 @@ var _ = Describe("Drain Param Config", func() { result := parser.NewLogTypeSet("rtr,unknown", true) // Should exclude only RTR (unknown type is ignored) - expectedSet := NewLogTypeSet(syslog.API, syslog.STG, syslog.LGR, syslog.APP, syslog.SSH, syslog.CELL) + expectedSet := NewLogTypeSet(syslog.LOG_API, syslog.LOG_STG, syslog.LOG_LGR, syslog.LOG_APP, syslog.LOG_SSH, syslog.LOG_CELL) Expect(result).To(Equal(expectedSet)) // Should have logged a warning From a05a6306d0d46288657f155c2b52d4548643d8b7 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Tue, 13 Jan 2026 10:11:49 +0100 Subject: [PATCH 09/36] Rename value to sourceType --- src/pkg/egress/syslog/filtering_drain_writer.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pkg/egress/syslog/filtering_drain_writer.go b/src/pkg/egress/syslog/filtering_drain_writer.go index 8eaaa46bb..79e136c09 100644 --- a/src/pkg/egress/syslog/filtering_drain_writer.go +++ b/src/pkg/egress/syslog/filtering_drain_writer.go @@ -51,12 +51,12 @@ func (w *FilteringDrainWriter) Write(env *loggregator_v2.Envelope) error { } } if env.GetLog() != nil { - value, ok := env.GetTags()["source_type"] + sourceType, ok := env.GetTags()["source_type"] if !ok { // Default to sending logs if no source_type tag is present - value = "" + sourceType = "" } - if sendsLogs(w.binding.DrainData, w.binding.LogFilter, value) { + if sendsLogs(w.binding.DrainData, w.binding.LogFilter, sourceType) { return w.writer.Write(env) } } From 100a721cfdb417aa531c40d656ef0190286fe3f7 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Tue, 13 Jan 2026 11:17:59 +0100 Subject: [PATCH 10/36] Only emit one warning on wrong drain config --- src/pkg/ingress/bindings/binding_config.go | 7 ++++++- src/pkg/ingress/bindings/binding_config_test.go | 14 ++++++++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/pkg/ingress/bindings/binding_config.go b/src/pkg/ingress/bindings/binding_config.go index c9d7bf59c..dc5b480cf 100644 --- a/src/pkg/ingress/bindings/binding_config.go +++ b/src/pkg/ingress/bindings/binding_config.go @@ -108,17 +108,22 @@ func (d *DrainParamParser) NewLogTypeSet(logTypeList string, isExclude bool) *sy logTypes := strings.Split(logTypeList, ",") set := make(syslog.LogTypeSet, len(logTypes)) + var unknownTypes []string for _, logType := range logTypes { logType = strings.TrimSpace(logType) t, ok := parseLogType(logType) if !ok { - d.log.Printf("Unknown log type '%s' in log type filter, ignoring", logType) + unknownTypes = append(unknownTypes, logType) continue } set.Add(t) } + if len(unknownTypes) > 0 { + d.log.Printf("Unknown log types '%s' in log type filter, ignoring", strings.Join(unknownTypes, ", ")) + } + if isExclude { // Invert the set - include all types except those in the set fullSet := make(syslog.LogTypeSet) diff --git a/src/pkg/ingress/bindings/binding_config_test.go b/src/pkg/ingress/bindings/binding_config_test.go index 0a2ba9138..a202e1e40 100644 --- a/src/pkg/ingress/bindings/binding_config_test.go +++ b/src/pkg/ingress/bindings/binding_config_test.go @@ -124,18 +124,24 @@ var _ = Describe("Drain Param Config", func() { Expect(configedBindings).To(HaveLen(0)) }) - It("logs a warning when an unknown log type is provided", func() { + It("logs a single warning when multiple unknown log types are provided", func() { var logOutput strings.Builder testLogger := log.New(&logOutput, "", log.LstdFlags) parser := bindings.NewDrainParamParser(newStubFetcher(nil, nil), false, testLogger) - result := parser.NewLogTypeSet("app,unknown,rtr", false) + result := parser.NewLogTypeSet("app,unknown,invalid,rtr", false) // Should only contain APP and RTR, not the unknown type Expect(result).To(Equal(NewLogTypeSet(syslog.LOG_APP, syslog.LOG_RTR))) - // Should have logged a warning - Expect(logOutput.String()).To(ContainSubstring("ignoring")) + // Should have logged exactly one warning containing all unknown types + output := logOutput.String() + Expect(output).To(ContainSubstring("unknown")) + Expect(output).To(ContainSubstring("invalid")) + Expect(output).To(ContainSubstring("ignoring")) + + // Verify it's a single log line (only one newline) + Expect(strings.Count(output, "\n")).To(Equal(1)) }) It("handles unknown log types in exclude mode", func() { From d009752ca62374d253ac4e7f591089ca4ac5be69 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Tue, 13 Jan 2026 12:12:05 +0100 Subject: [PATCH 11/36] Rename DrainParameterParser in test --- .../ingress/bindings/binding_config_test.go | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/pkg/ingress/bindings/binding_config_test.go b/src/pkg/ingress/bindings/binding_config_test.go index a202e1e40..6f28378dd 100644 --- a/src/pkg/ingress/bindings/binding_config_test.go +++ b/src/pkg/ingress/bindings/binding_config_test.go @@ -20,9 +20,9 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain"}}, } f := newStubFetcher(bs, nil) - wf := bindings.NewDrainParamParser(f, true, logger) + dp := bindings.NewDrainParamParser(f, true, logger) - configedBindings, _ := wf.FetchBindings() + configedBindings, _ := dp.FetchBindings() Expect(configedBindings[0].OmitMetadata).To(BeFalse()) }) @@ -32,9 +32,9 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain?omit-metadata=true"}}, } f := newStubFetcher(bs, nil) - wf := bindings.NewDrainParamParser(f, true, logger) + dp := bindings.NewDrainParamParser(f, true, logger) - configedBindings, _ := wf.FetchBindings() + configedBindings, _ := dp.FetchBindings() Expect(configedBindings[0].OmitMetadata).To(BeTrue()) Expect(configedBindings[1].OmitMetadata).To(BeTrue()) }) @@ -44,9 +44,9 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain"}}, } f := newStubFetcher(bs, nil) - wf := bindings.NewDrainParamParser(f, false, logger) + dp := bindings.NewDrainParamParser(f, false, logger) - configedBindings, _ := wf.FetchBindings() + configedBindings, _ := dp.FetchBindings() Expect(configedBindings[0].OmitMetadata).To(BeTrue()) }) @@ -56,9 +56,9 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain?omit-metadata=false"}}, } f := newStubFetcher(bs, nil) - wf := bindings.NewDrainParamParser(f, false, logger) + dp := bindings.NewDrainParamParser(f, false, logger) - configedBindings, _ := wf.FetchBindings() + configedBindings, _ := dp.FetchBindings() Expect(configedBindings[0].OmitMetadata).To(BeFalse()) Expect(configedBindings[1].OmitMetadata).To(BeFalse()) }) @@ -68,9 +68,9 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain?ssl-strict-internal=true"}}, } f := newStubFetcher(bs, nil) - wf := bindings.NewDrainParamParser(f, true, logger) + dp := bindings.NewDrainParamParser(f, true, logger) - configedBindings, _ := wf.FetchBindings() + configedBindings, _ := dp.FetchBindings() Expect(configedBindings[0].InternalTls).To(BeTrue()) }) @@ -83,9 +83,9 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain?drain-data=all"}}, } f := newStubFetcher(bs, nil) - wf := bindings.NewDrainParamParser(f, true, logger) + dp := bindings.NewDrainParamParser(f, true, logger) - configedBindings, _ := wf.FetchBindings() + configedBindings, _ := dp.FetchBindings() Expect(configedBindings[0].DrainData).To(Equal(syslog.LOGS)) Expect(configedBindings[1].DrainData).To(Equal(syslog.LOGS)) Expect(configedBindings[2].DrainData).To(Equal(syslog.METRICS)) @@ -102,9 +102,9 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain?exclude-log-types=rtr"}}, } f := newStubFetcher(bs, nil) - wf := bindings.NewDrainParamParser(f, true, logger) + dp := bindings.NewDrainParamParser(f, true, logger) - configedBindings, _ := wf.FetchBindings() + configedBindings, _ := dp.FetchBindings() Expect(configedBindings[0].LogFilter).To(Equal(NewLogTypeSet())) // Empty map defaults to all types Expect(configedBindings[1].LogFilter).To(Equal(NewLogTypeSet(syslog.LOG_APP))) Expect(configedBindings[2].LogFilter).To(Equal(NewLogTypeSet(syslog.LOG_APP, syslog.LOG_STG, syslog.LOG_CELL))) @@ -117,9 +117,9 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain?include-log-types=app&exclude-log-types=rtr"}}, } f := newStubFetcher(bs, nil) - wf := bindings.NewDrainParamParser(f, true, logger) + dp := bindings.NewDrainParamParser(f, true, logger) - configedBindings, err := wf.FetchBindings() + configedBindings, err := dp.FetchBindings() Expect(err).To(HaveOccurred()) Expect(configedBindings).To(HaveLen(0)) }) @@ -168,9 +168,9 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain?include-metrics-deprecated=true"}}, } f := newStubFetcher(bs, nil) - wf := bindings.NewDrainParamParser(f, true, logger) + dp := bindings.NewDrainParamParser(f, true, logger) - configedBindings, _ := wf.FetchBindings() + configedBindings, _ := dp.FetchBindings() Expect(configedBindings[0].DrainData).To(Equal(syslog.METRICS)) Expect(configedBindings[1].DrainData).To(Equal(syslog.LOGS_NO_EVENTS)) Expect(configedBindings[2].DrainData).To(Equal(syslog.LOGS)) @@ -185,9 +185,9 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain?omit-metadata=true"}}, } f := newStubFetcher(bs, nil) - wf := bindings.NewDrainParamParser(f, true, logger) + dp := bindings.NewDrainParamParser(f, true, logger) - configedBindings, err := wf.FetchBindings() + configedBindings, err := dp.FetchBindings() Expect(err).ToNot(HaveOccurred()) Expect(configedBindings).To(HaveLen(2)) Expect(configedBindings[0].Drain).To(Equal(syslog.Drain{Url: "https://test.org/drain?disable-metadata=true"})) @@ -196,9 +196,9 @@ var _ = Describe("Drain Param Config", func() { It("Returns a error when fetching fails", func() { f := newStubFetcher(nil, errors.New("Ahhh an error")) - wf := bindings.NewDrainParamParser(f, true, logger) + dp := bindings.NewDrainParamParser(f, true, logger) - _, err := wf.FetchBindings() + _, err := dp.FetchBindings() Expect(err).To(MatchError("Ahhh an error")) }) }) From 455328059fbc81172206fe4b04b0297e1bc17301 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Tue, 13 Jan 2026 12:43:26 +0100 Subject: [PATCH 12/36] Add tests for the include and exclude filters --- .../syslog/filtering_drain_writer_test.go | 85 ++++++++++++++++++- 1 file changed, 84 insertions(+), 1 deletion(-) diff --git a/src/pkg/egress/syslog/filtering_drain_writer_test.go b/src/pkg/egress/syslog/filtering_drain_writer_test.go index f5c0e6c42..561e5b764 100644 --- a/src/pkg/egress/syslog/filtering_drain_writer_test.go +++ b/src/pkg/egress/syslog/filtering_drain_writer_test.go @@ -57,7 +57,6 @@ var _ = Describe("Filtering Drain Writer", func() { It("sends logs when source_type tag is missing", func() { binding := syslog.Binding{ DrainData: syslog.LOGS, - LogFilter: nil, } fakeWriter := &fakeWriter{} drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) @@ -80,6 +79,90 @@ var _ = Describe("Filtering Drain Writer", func() { Expect(fakeWriter.received).To(Equal(1)) }) + It("filters logs based on include filter - includes only APP logs", func() { + appFilter := syslog.LogTypeSet{syslog.LOG_APP: struct{}{}} + binding := syslog.Binding{ + DrainData: syslog.LOGS, + LogFilter: &appFilter, + } + fakeWriter := &fakeWriter{} + drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) + Expect(err).NotTo(HaveOccurred()) + + envelopes := []*loggregator_v2.Envelope{ + { + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{Payload: []byte("app log")}, + }, + Tags: map[string]string{"source_type": "APP/PROC/WEB/0"}, + }, + { + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{Payload: []byte("rtr log")}, + }, + Tags: map[string]string{"source_type": "RTR/1"}, + }, + { + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{Payload: []byte("stg log")}, + }, + Tags: map[string]string{"source_type": "STG/0"}, + }, + } + + for _, envelope := range envelopes { + err = drainWriter.Write(envelope) + Expect(err).NotTo(HaveOccurred()) + } + + // Only APP log should be sent + Expect(fakeWriter.received).To(Equal(1)) + }) + + It("filters logs based on exclude filter - excludes RTR logs", func() { + // Include APP and STG, effectively excluding RTR + includeFilter := syslog.LogTypeSet{ + syslog.LOG_APP: struct{}{}, + syslog.LOG_STG: struct{}{}, + } + binding := syslog.Binding{ + DrainData: syslog.LOGS, + LogFilter: &includeFilter, + } + fakeWriter := &fakeWriter{} + drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) + Expect(err).NotTo(HaveOccurred()) + + envelopes := []*loggregator_v2.Envelope{ + { + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{Payload: []byte("app log")}, + }, + Tags: map[string]string{"source_type": "APP/PROC/WEB/0"}, + }, + { + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{Payload: []byte("rtr log")}, + }, + Tags: map[string]string{"source_type": "RTR/1"}, + }, + { + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{Payload: []byte("stg log")}, + }, + Tags: map[string]string{"source_type": "STG/0"}, + }, + } + + for _, envelope := range envelopes { + err = drainWriter.Write(envelope) + Expect(err).NotTo(HaveOccurred()) + } + + // APP and STG logs should be sent, RTR should be filtered out + Expect(fakeWriter.received).To(Equal(2)) + }) + It("sends logs with unknown source_type prefix when filter is set", func() { appFilter := syslog.LogTypeSet{syslog.LOG_APP: struct{}{}} binding := syslog.Binding{ From c83295039c78bb101827cb92629ce30833adc6b0 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Mon, 19 Jan 2026 16:04:21 +0100 Subject: [PATCH 13/36] Move new log filter test to table test --- .../ingress/bindings/binding_config_test.go | 57 ++++++++++++++----- 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/src/pkg/ingress/bindings/binding_config_test.go b/src/pkg/ingress/bindings/binding_config_test.go index 6f28378dd..05d65405b 100644 --- a/src/pkg/ingress/bindings/binding_config_test.go +++ b/src/pkg/ingress/bindings/binding_config_test.go @@ -93,23 +93,50 @@ var _ = Describe("Drain Param Config", func() { Expect(configedBindings[4].DrainData).To(Equal(syslog.ALL)) }) - It("sets drain filter appropriately'", func() { - bs := []syslog.Binding{ - {Drain: syslog.Drain{Url: "https://test.org/drain"}}, - {Drain: syslog.Drain{Url: "https://test.org/drain?include-log-types=app"}}, - {Drain: syslog.Drain{Url: "https://test.org/drain?include-log-types=app,stg,cell"}}, - {Drain: syslog.Drain{Url: "https://test.org/drain?exclude-log-types=rtr,cell,stg"}}, - {Drain: syslog.Drain{Url: "https://test.org/drain?exclude-log-types=rtr"}}, + It("sets drain filter appropriately", func() { + testCases := []struct { + name string + url string + expected *syslog.LogTypeSet + }{ + { + name: "empty drain URL defaults to all types", + url: "https://test.org/drain", + expected: NewLogTypeSet(), + }, + { + name: "include-log-types=app", + url: "https://test.org/drain?include-log-types=app", + expected: NewLogTypeSet(syslog.LOG_APP), + }, + { + name: "include-log-types=app,stg,cell", + url: "https://test.org/drain?include-log-types=app,stg,cell", + expected: NewLogTypeSet(syslog.LOG_APP, syslog.LOG_STG, syslog.LOG_CELL), + }, + { + name: "exclude-log-types=rtr,cell,stg", + url: "https://test.org/drain?exclude-log-types=rtr,cell,stg", + expected: NewLogTypeSet(syslog.LOG_API, syslog.LOG_LGR, syslog.LOG_APP, syslog.LOG_SSH), + }, + { + name: "exclude-log-types=rtr", + url: "https://test.org/drain?exclude-log-types=rtr", + expected: NewLogTypeSet(syslog.LOG_API, syslog.LOG_STG, syslog.LOG_LGR, syslog.LOG_APP, syslog.LOG_SSH, syslog.LOG_CELL), + }, } - f := newStubFetcher(bs, nil) - dp := bindings.NewDrainParamParser(f, true, logger) - configedBindings, _ := dp.FetchBindings() - Expect(configedBindings[0].LogFilter).To(Equal(NewLogTypeSet())) // Empty map defaults to all types - Expect(configedBindings[1].LogFilter).To(Equal(NewLogTypeSet(syslog.LOG_APP))) - Expect(configedBindings[2].LogFilter).To(Equal(NewLogTypeSet(syslog.LOG_APP, syslog.LOG_STG, syslog.LOG_CELL))) - Expect(configedBindings[3].LogFilter).To(Equal(NewLogTypeSet(syslog.LOG_API, syslog.LOG_LGR, syslog.LOG_APP, syslog.LOG_SSH))) - Expect(configedBindings[4].LogFilter).To(Equal(NewLogTypeSet(syslog.LOG_API, syslog.LOG_STG, syslog.LOG_LGR, syslog.LOG_APP, syslog.LOG_SSH, syslog.LOG_CELL))) + for _, tc := range testCases { + By(tc.name) + bs := []syslog.Binding{ + {Drain: syslog.Drain{Url: tc.url}}, + } + f := newStubFetcher(bs, nil) + dp := bindings.NewDrainParamParser(f, true, logger) + + configedBindings, _ := dp.FetchBindings() + Expect(configedBindings[0].LogFilter).To(Equal(tc.expected), "failed for case: %s", tc.name) + } }) It("returns an error when both include-log-types and exclude-log-types are specified", func() { From 9b7d458b5b33958f661f1a48834e2ce46c3de328 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Mon, 19 Jan 2026 16:21:43 +0100 Subject: [PATCH 14/36] Move next test to table --- .../ingress/bindings/binding_config_test.go | 55 ++++++++++++++----- 1 file changed, 41 insertions(+), 14 deletions(-) diff --git a/src/pkg/ingress/bindings/binding_config_test.go b/src/pkg/ingress/bindings/binding_config_test.go index 05d65405b..42ba797cb 100644 --- a/src/pkg/ingress/bindings/binding_config_test.go +++ b/src/pkg/ingress/bindings/binding_config_test.go @@ -75,22 +75,49 @@ var _ = Describe("Drain Param Config", func() { }) It("sets drain data appropriately'", func() { - bs := []syslog.Binding{ - {Drain: syslog.Drain{Url: "https://test.org/drain"}}, - {Drain: syslog.Drain{Url: "https://test.org/drain?drain-data=logs"}}, - {Drain: syslog.Drain{Url: "https://test.org/drain?drain-data=metrics"}}, - {Drain: syslog.Drain{Url: "https://test.org/drain?drain-data=traces"}}, - {Drain: syslog.Drain{Url: "https://test.org/drain?drain-data=all"}}, + testCases := []struct { + name string + url string + expected syslog.DrainData + }{ + { + name: "no drain-data parameter defaults to logs", + url: "https://test.org/drain", + expected: syslog.LOGS, + }, + { + name: "drain-data=logs", + url: "https://test.org/drain?drain-data=logs", + expected: syslog.LOGS, + }, + { + name: "drain-data=metrics", + url: "https://test.org/drain?drain-data=metrics", + expected: syslog.METRICS, + }, + { + name: "drain-data=traces", + url: "https://test.org/drain?drain-data=traces", + expected: syslog.TRACES, + }, + { + name: "drain-data=all", + url: "https://test.org/drain?drain-data=all", + expected: syslog.ALL, + }, } - f := newStubFetcher(bs, nil) - dp := bindings.NewDrainParamParser(f, true, logger) - configedBindings, _ := dp.FetchBindings() - Expect(configedBindings[0].DrainData).To(Equal(syslog.LOGS)) - Expect(configedBindings[1].DrainData).To(Equal(syslog.LOGS)) - Expect(configedBindings[2].DrainData).To(Equal(syslog.METRICS)) - Expect(configedBindings[3].DrainData).To(Equal(syslog.TRACES)) - Expect(configedBindings[4].DrainData).To(Equal(syslog.ALL)) + for _, tc := range testCases { + By(tc.name) + bs := []syslog.Binding{ + {Drain: syslog.Drain{Url: tc.url}}, + } + f := newStubFetcher(bs, nil) + dp := bindings.NewDrainParamParser(f, true, logger) + + configedBindings, _ := dp.FetchBindings() + Expect(configedBindings[0].DrainData).To(Equal(tc.expected)) + } }) It("sets drain filter appropriately", func() { From ba2b7ab8f35c4e8e8dc7dedc726cffc5175f832e Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Mon, 19 Jan 2026 16:25:59 +0100 Subject: [PATCH 15/36] Move drain data for old parameter test to table --- .../ingress/bindings/binding_config_test.go | 55 ++++++++++++++----- 1 file changed, 41 insertions(+), 14 deletions(-) diff --git a/src/pkg/ingress/bindings/binding_config_test.go b/src/pkg/ingress/bindings/binding_config_test.go index 42ba797cb..1ea09198c 100644 --- a/src/pkg/ingress/bindings/binding_config_test.go +++ b/src/pkg/ingress/bindings/binding_config_test.go @@ -214,22 +214,49 @@ var _ = Describe("Drain Param Config", func() { }) It("sets drain data for old parameter appropriately'", func() { - bs := []syslog.Binding{ - {Drain: syslog.Drain{Url: "https://test.org/drain?drain-type=metrics"}}, - {Drain: syslog.Drain{Url: "https://test.org/drain?drain-type=logs"}}, - {Drain: syslog.Drain{Url: "https://test.org/drain"}}, - {Drain: syslog.Drain{Url: "https://test.org/drain?drain-type=all"}}, - {Drain: syslog.Drain{Url: "https://test.org/drain?include-metrics-deprecated=true"}}, + testCases := []struct { + name string + url string + expected syslog.DrainData + }{ + { + name: "drain-type=metrics", + url: "https://test.org/drain?drain-type=metrics", + expected: syslog.METRICS, + }, + { + name: "drain-type=logs", + url: "https://test.org/drain?drain-type=logs", + expected: syslog.LOGS_NO_EVENTS, + }, + { + name: "no drain-type parameter", + url: "https://test.org/drain", + expected: syslog.LOGS, + }, + { + name: "drain-type=all", + url: "https://test.org/drain?drain-type=all", + expected: syslog.LOGS_AND_METRICS, + }, + { + name: "include-metrics-deprecated=true", + url: "https://test.org/drain?include-metrics-deprecated=true", + expected: syslog.ALL, + }, } - f := newStubFetcher(bs, nil) - dp := bindings.NewDrainParamParser(f, true, logger) - configedBindings, _ := dp.FetchBindings() - Expect(configedBindings[0].DrainData).To(Equal(syslog.METRICS)) - Expect(configedBindings[1].DrainData).To(Equal(syslog.LOGS_NO_EVENTS)) - Expect(configedBindings[2].DrainData).To(Equal(syslog.LOGS)) - Expect(configedBindings[3].DrainData).To(Equal(syslog.LOGS_AND_METRICS)) - Expect(configedBindings[4].DrainData).To(Equal(syslog.ALL)) + for _, tc := range testCases { + By(tc.name) + bs := []syslog.Binding{ + {Drain: syslog.Drain{Url: tc.url}}, + } + f := newStubFetcher(bs, nil) + dp := bindings.NewDrainParamParser(f, true, logger) + + configedBindings, _ := dp.FetchBindings() + Expect(configedBindings[0].DrainData).To(Equal(tc.expected)) + } }) It("omits bindings with bad Drain URLs", func() { From f5aa7bcc673668701c4acce1dcef794f82178429 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Tue, 3 Feb 2026 13:19:20 +0100 Subject: [PATCH 16/36] Move log filter validation to binding_fetcher --- src/cmd/syslog-agent/app/syslog_agent.go | 4 +- src/pkg/egress/syslog/log.go | 8 ++ src/pkg/ingress/bindings/binding_config.go | 56 +++----- .../ingress/bindings/binding_config_test.go | 75 ++-------- .../bindings/filtered_binding_fetcher.go | 53 +++++++ .../bindings/filtered_binding_fetcher_test.go | 129 ++++++++++++++++++ 6 files changed, 221 insertions(+), 104 deletions(-) diff --git a/src/cmd/syslog-agent/app/syslog_agent.go b/src/cmd/syslog-agent/app/syslog_agent.go index ffe2df27f..d200ea359 100644 --- a/src/cmd/syslog-agent/app/syslog_agent.go +++ b/src/cmd/syslog-agent/app/syslog_agent.go @@ -107,13 +107,13 @@ func NewSyslogAgent( cfg.WarnOnInvalidDrains, l, ) - cupsFetcher = bindings.NewDrainParamParser(cupsFetcher, cfg.DefaultDrainMetadata, l) + cupsFetcher = bindings.NewDrainParamParser(cupsFetcher, cfg.DefaultDrainMetadata) } aggregateFetcher := bindings.NewAggregateDrainFetcher(cfg.AggregateDrainURLs, cacheClient) bindingManager := binding.NewManager( cupsFetcher, - bindings.NewDrainParamParser(aggregateFetcher, cfg.DefaultDrainMetadata, l), + bindings.NewDrainParamParser(aggregateFetcher, cfg.DefaultDrainMetadata), connector, m, cfg.Cache.PollingInterval, diff --git a/src/pkg/egress/syslog/log.go b/src/pkg/egress/syslog/log.go index 1e0338da1..e41cf75b1 100644 --- a/src/pkg/egress/syslog/log.go +++ b/src/pkg/egress/syslog/log.go @@ -1,5 +1,7 @@ package syslog +import "strings" + // LogType defines the log types used within Cloud Foundry // Their order in the code is as documented in https://docs.cloudfoundry.org/devguide/deploy-apps/streaming-logs.html#format type LogType string @@ -53,3 +55,9 @@ func (s LogTypeSet) Contains(lt LogType) bool { _, exists := s[lt] return exists } + +// ParseLogType parses a string into a LogType value +func ParseLogType(s string) (LogType, bool) { + lt := LogType(strings.ToUpper(s)) + return lt, lt.IsValid() +} diff --git a/src/pkg/ingress/bindings/binding_config.go b/src/pkg/ingress/bindings/binding_config.go index dc5b480cf..e36acd1ae 100644 --- a/src/pkg/ingress/bindings/binding_config.go +++ b/src/pkg/ingress/bindings/binding_config.go @@ -1,8 +1,6 @@ package bindings import ( - "errors" - "log" "net/url" "strings" @@ -13,14 +11,12 @@ import ( type DrainParamParser struct { fetcher binding.Fetcher defaultDrainMetadata bool - log *log.Logger } -func NewDrainParamParser(f binding.Fetcher, defaultDrainMetadata bool, l *log.Logger) *DrainParamParser { +func NewDrainParamParser(f binding.Fetcher, defaultDrainMetadata bool) *DrainParamParser { return &DrainParamParser{ fetcher: f, defaultDrainMetadata: defaultDrainMetadata, - log: l, } } @@ -40,10 +36,7 @@ func (d *DrainParamParser) FetchBindings() ([]syslog.Binding, error) { b.OmitMetadata = getOmitMetadata(urlParsed, d.defaultDrainMetadata) b.InternalTls = getInternalTLS(urlParsed) b.DrainData = getBindingType(urlParsed) - b.LogFilter, err = d.getLogFilter(urlParsed) - if err != nil { - return nil, err - } + b.LogFilter = d.getLogFilter(urlParsed) processed = append(processed, b) } @@ -93,37 +86,34 @@ func getBindingType(u *url.URL) syslog.DrainData { return drainData } -// parseLogType parses a string into a LogType value -func parseLogType(s string) (syslog.LogType, bool) { - lt := syslog.LogType(strings.ToUpper(s)) - return lt, lt.IsValid() +func (d *DrainParamParser) getLogFilter(u *url.URL) *syslog.LogTypeSet { + includeLogTypes := u.Query().Get("include-log-types") + excludeLogTypes := u.Query().Get("exclude-log-types") + + if excludeLogTypes != "" { + return d.NewLogTypeSet(excludeLogTypes, true) + } else if includeLogTypes != "" { + return d.NewLogTypeSet(includeLogTypes, false) + } + return nil } -// NewLogTypeSet parses a URL query parameter into a Set of LogTypes +// NewLogTypeSet parses a URL query parameter into a Set of LogTypes. +// logTypeList is assumed to be a comma-separated list of valid log types. func (d *DrainParamParser) NewLogTypeSet(logTypeList string, isExclude bool) *syslog.LogTypeSet { if logTypeList == "" { - set := make(syslog.LogTypeSet) - return &set + return nil } logTypes := strings.Split(logTypeList, ",") set := make(syslog.LogTypeSet, len(logTypes)) - var unknownTypes []string for _, logType := range logTypes { logType = strings.TrimSpace(logType) - t, ok := parseLogType(logType) - if !ok { - unknownTypes = append(unknownTypes, logType) - continue - } + t, _ := syslog.ParseLogType(logType) set.Add(t) } - if len(unknownTypes) > 0 { - d.log.Printf("Unknown log types '%s' in log type filter, ignoring", strings.Join(unknownTypes, ", ")) - } - if isExclude { // Invert the set - include all types except those in the set fullSet := make(syslog.LogTypeSet) @@ -141,20 +131,6 @@ func (d *DrainParamParser) NewLogTypeSet(logTypeList string, isExclude bool) *sy return &set } -func (d *DrainParamParser) getLogFilter(u *url.URL) (*syslog.LogTypeSet, error) { - includeLogTypes := u.Query().Get("include-log-types") - excludeLogTypes := u.Query().Get("exclude-log-types") - - if excludeLogTypes != "" && includeLogTypes != "" { - return nil, errors.New("include-log-types and exclude-log-types can not be used at the same time") - } else if excludeLogTypes != "" { - return d.NewLogTypeSet(excludeLogTypes, true), nil - } else if includeLogTypes != "" { - return d.NewLogTypeSet(includeLogTypes, false), nil - } - return d.NewLogTypeSet("", false), nil -} - func getRemoveMetadataQuery(u *url.URL) string { q := u.Query().Get("disable-metadata") if q == "" { diff --git a/src/pkg/ingress/bindings/binding_config_test.go b/src/pkg/ingress/bindings/binding_config_test.go index 1ea09198c..2af8df173 100644 --- a/src/pkg/ingress/bindings/binding_config_test.go +++ b/src/pkg/ingress/bindings/binding_config_test.go @@ -2,8 +2,6 @@ package bindings_test import ( "errors" - "log" - "strings" "code.cloudfoundry.org/loggregator-agent-release/src/pkg/egress/syslog" "code.cloudfoundry.org/loggregator-agent-release/src/pkg/ingress/bindings" @@ -12,15 +10,12 @@ import ( ) var _ = Describe("Drain Param Config", func() { - var ( - logger = log.New(GinkgoWriter, "", 0) - ) It("sets OmitMetadata to false if the drain doesn't contain 'disable-metadata=true'", func() { bs := []syslog.Binding{ {Drain: syslog.Drain{Url: "https://test.org/drain"}}, } f := newStubFetcher(bs, nil) - dp := bindings.NewDrainParamParser(f, true, logger) + dp := bindings.NewDrainParamParser(f, true) configedBindings, _ := dp.FetchBindings() Expect(configedBindings[0].OmitMetadata).To(BeFalse()) @@ -32,7 +27,7 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain?omit-metadata=true"}}, } f := newStubFetcher(bs, nil) - dp := bindings.NewDrainParamParser(f, true, logger) + dp := bindings.NewDrainParamParser(f, true) configedBindings, _ := dp.FetchBindings() Expect(configedBindings[0].OmitMetadata).To(BeTrue()) @@ -44,7 +39,7 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain"}}, } f := newStubFetcher(bs, nil) - dp := bindings.NewDrainParamParser(f, false, logger) + dp := bindings.NewDrainParamParser(f, false) configedBindings, _ := dp.FetchBindings() Expect(configedBindings[0].OmitMetadata).To(BeTrue()) @@ -56,7 +51,7 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain?omit-metadata=false"}}, } f := newStubFetcher(bs, nil) - dp := bindings.NewDrainParamParser(f, false, logger) + dp := bindings.NewDrainParamParser(f, false) configedBindings, _ := dp.FetchBindings() Expect(configedBindings[0].OmitMetadata).To(BeFalse()) @@ -68,7 +63,7 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain?ssl-strict-internal=true"}}, } f := newStubFetcher(bs, nil) - dp := bindings.NewDrainParamParser(f, true, logger) + dp := bindings.NewDrainParamParser(f, true) configedBindings, _ := dp.FetchBindings() Expect(configedBindings[0].InternalTls).To(BeTrue()) @@ -113,7 +108,7 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: tc.url}}, } f := newStubFetcher(bs, nil) - dp := bindings.NewDrainParamParser(f, true, logger) + dp := bindings.NewDrainParamParser(f, true) configedBindings, _ := dp.FetchBindings() Expect(configedBindings[0].DrainData).To(Equal(tc.expected)) @@ -159,60 +154,13 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: tc.url}}, } f := newStubFetcher(bs, nil) - dp := bindings.NewDrainParamParser(f, true, logger) + dp := bindings.NewDrainParamParser(f, true) configedBindings, _ := dp.FetchBindings() Expect(configedBindings[0].LogFilter).To(Equal(tc.expected), "failed for case: %s", tc.name) } }) - It("returns an error when both include-log-types and exclude-log-types are specified", func() { - bs := []syslog.Binding{ - {Drain: syslog.Drain{Url: "https://test.org/drain?include-log-types=app&exclude-log-types=rtr"}}, - } - f := newStubFetcher(bs, nil) - dp := bindings.NewDrainParamParser(f, true, logger) - - configedBindings, err := dp.FetchBindings() - Expect(err).To(HaveOccurred()) - Expect(configedBindings).To(HaveLen(0)) - }) - - It("logs a single warning when multiple unknown log types are provided", func() { - var logOutput strings.Builder - testLogger := log.New(&logOutput, "", log.LstdFlags) - parser := bindings.NewDrainParamParser(newStubFetcher(nil, nil), false, testLogger) - - result := parser.NewLogTypeSet("app,unknown,invalid,rtr", false) - - // Should only contain APP and RTR, not the unknown type - Expect(result).To(Equal(NewLogTypeSet(syslog.LOG_APP, syslog.LOG_RTR))) - - // Should have logged exactly one warning containing all unknown types - output := logOutput.String() - Expect(output).To(ContainSubstring("unknown")) - Expect(output).To(ContainSubstring("invalid")) - Expect(output).To(ContainSubstring("ignoring")) - - // Verify it's a single log line (only one newline) - Expect(strings.Count(output, "\n")).To(Equal(1)) - }) - - It("handles unknown log types in exclude mode", func() { - var logOutput strings.Builder - testLogger := log.New(&logOutput, "", log.LstdFlags) - parser := bindings.NewDrainParamParser(newStubFetcher(nil, nil), false, testLogger) - - result := parser.NewLogTypeSet("rtr,unknown", true) - - // Should exclude only RTR (unknown type is ignored) - expectedSet := NewLogTypeSet(syslog.LOG_API, syslog.LOG_STG, syslog.LOG_LGR, syslog.LOG_APP, syslog.LOG_SSH, syslog.LOG_CELL) - Expect(result).To(Equal(expectedSet)) - - // Should have logged a warning - Expect(logOutput.String()).To(ContainSubstring("ignoring")) - }) - It("sets drain data for old parameter appropriately'", func() { testCases := []struct { name string @@ -252,7 +200,7 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: tc.url}}, } f := newStubFetcher(bs, nil) - dp := bindings.NewDrainParamParser(f, true, logger) + dp := bindings.NewDrainParamParser(f, true) configedBindings, _ := dp.FetchBindings() Expect(configedBindings[0].DrainData).To(Equal(tc.expected)) @@ -266,7 +214,7 @@ var _ = Describe("Drain Param Config", func() { {Drain: syslog.Drain{Url: "https://test.org/drain?omit-metadata=true"}}, } f := newStubFetcher(bs, nil) - dp := bindings.NewDrainParamParser(f, true, logger) + dp := bindings.NewDrainParamParser(f, true) configedBindings, err := dp.FetchBindings() Expect(err).ToNot(HaveOccurred()) @@ -277,7 +225,7 @@ var _ = Describe("Drain Param Config", func() { It("Returns a error when fetching fails", func() { f := newStubFetcher(nil, errors.New("Ahhh an error")) - dp := bindings.NewDrainParamParser(f, true, logger) + dp := bindings.NewDrainParamParser(f, true) _, err := dp.FetchBindings() Expect(err).To(MatchError("Ahhh an error")) @@ -305,6 +253,9 @@ func (f *stubFetcher) DrainLimit() int { } func NewLogTypeSet(logTypes ...syslog.LogType) *syslog.LogTypeSet { + if len(logTypes) == 0 { + return nil + } set := make(syslog.LogTypeSet, len(logTypes)) for _, t := range logTypes { set[t] = struct{}{} diff --git a/src/pkg/ingress/bindings/filtered_binding_fetcher.go b/src/pkg/ingress/bindings/filtered_binding_fetcher.go index c9a08f0a3..068ab01f9 100644 --- a/src/pkg/ingress/bindings/filtered_binding_fetcher.go +++ b/src/pkg/ingress/bindings/filtered_binding_fetcher.go @@ -3,6 +3,7 @@ package bindings import ( "log" "net/url" + "strings" "time" "code.cloudfoundry.org/loggregator-agent-release/src/pkg/binding" @@ -69,6 +70,19 @@ func (f *FilteredBindingFetcher) FetchBindings() ([]syslog.Binding, error) { continue } + if invalidLogFilter(u) { + invalidDrains += 1 + f.printWarning("include-log-types and exclude-log-types cannot be used at the same time in syslog drain url %s for application %s", anonymousUrl.String(), b.AppId) + continue + } + + logTypes := getUnknownLogTypes(u.Query()) + if logTypes != nil { + invalidDrains += 1 + f.printWarning("Unknown log types '%s' in log type filter in syslog drain url %s for application %s", strings.Join(logTypes, ", "), anonymousUrl.String(), b.AppId) + continue + } + _, exists := f.failedHostsCache.Get(u.Host) if exists { invalidDrains += 1 @@ -99,6 +113,45 @@ func (f *FilteredBindingFetcher) FetchBindings() ([]syslog.Binding, error) { } +// invalidLogFilter checks if both include-log-types and exclude-log-types +func invalidLogFilter(u *url.URL) bool { + includeLogTypes := u.Query().Get("include-log-types") + excludeLogTypes := u.Query().Get("exclude-log-types") + if excludeLogTypes != "" && includeLogTypes != "" { + return true + } + return false +} + +// assumes only one of include-log-types or exclude-log-types is set +func getUnknownLogTypes(u url.Values) []string { + var logTypeList string + includeLogTypes := u.Get("include-log-types") + excludeLogTypes := u.Get("exclude-log-types") + + if includeLogTypes != "" { + logTypeList = includeLogTypes + } else if excludeLogTypes != "" { + logTypeList = excludeLogTypes + } else { + return nil + } + + logTypes := strings.Split(logTypeList, ",") + var unknownTypes []string + + for _, logType := range logTypes { + logType = strings.TrimSpace(logType) + _, ok := syslog.ParseLogType(logType) + if !ok { + unknownTypes = append(unknownTypes, logType) + continue + } + } + + return unknownTypes +} + func (f FilteredBindingFetcher) printWarning(format string, v ...any) { if f.warn { f.logger.Printf(format, v...) diff --git a/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go b/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go index 767b9fdc9..f8586474c 100644 --- a/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go +++ b/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go @@ -239,6 +239,135 @@ var _ = Describe("FilteredBindingFetcher", func() { }) }) + Context("when both include-log-types and exclude-log-types are specified", func() { + var logBuffer bytes.Buffer + var warn bool + var mockic *bindingsfakes.FakeIPChecker + + BeforeEach(func() { + logBuffer = bytes.Buffer{} + log.SetOutput(&logBuffer) + warn = true + mockic = &bindingsfakes.FakeIPChecker{} + mockic.ResolveAddrReturns(net.ParseIP("10.10.10.10"), nil) + mockic.CheckBlacklistReturns(nil) + }) + + JustBeforeEach(func() { + input := []syslog.Binding{ + {AppId: "app-id", Hostname: "we.dont.care", Drain: syslog.Drain{Url: "https://test.org/drain?include-log-types=app&exclude-log-types=rtr"}}, + } + filter = bindings.NewFilteredBindingFetcher( + mockic, + &SpyBindingReader{bindings: input}, + metrics, + warn, + log, + ) + }) + + It("ignores the drain", func() { + actual, err := filter.FetchBindings() + + Expect(err).ToNot(HaveOccurred()) + Expect(actual).To(HaveLen(0)) + Expect(logBuffer.String()).Should(MatchRegexp("include-log-types and exclude-log-types cannot be used at the same time")) + Expect(metrics.GetMetric("invalid_drains", map[string]string{"unit": "total"}).Value()).To(Equal(1.0)) + + }) + + Context("when configured not to warn", func() { + BeforeEach(func() { + warn = false + }) + It("doesn't log the conflicting filters warning", func() { + _, err := filter.FetchBindings() + Expect(err).ToNot(HaveOccurred()) + Expect(logBuffer.String()).ToNot(MatchRegexp("include-log-types and exclude-log-types cannot be used at the same time")) + }) + }) + }) + + Context("when unknown log types are provided", func() { + var logBuffer bytes.Buffer + var warn bool + var mockic *bindingsfakes.FakeIPChecker + + BeforeEach(func() { + logBuffer = bytes.Buffer{} + log.SetOutput(&logBuffer) + warn = true + mockic = &bindingsfakes.FakeIPChecker{} + mockic.ResolveAddrReturns(net.ParseIP("10.10.10.10"), nil) + mockic.CheckBlacklistReturns(nil) + }) + + It("logs a warning and ignores the drain in include mode", func() { + input := []syslog.Binding{ + {AppId: "app-id", Hostname: "we.dont.care", Drain: syslog.Drain{Url: "https://test.org/drain?include-log-types=app,unknown,invalid,rtr"}}, + } + filter = bindings.NewFilteredBindingFetcher( + mockic, + &SpyBindingReader{bindings: input}, + metrics, + warn, + log, + ) + + actual, err := filter.FetchBindings() + + Expect(err).ToNot(HaveOccurred()) + Expect(actual).To(HaveLen(0)) + Expect(logBuffer.String()).Should(MatchRegexp("Unknown log types")) + Expect(logBuffer.String()).Should(MatchRegexp("unknown")) + Expect(logBuffer.String()).Should(MatchRegexp("invalid")) + Expect(metrics.GetMetric("invalid_drains", map[string]string{"unit": "total"}).Value()).To(Equal(1.0)) + }) + + It("logs a warning and ignores the drain in exclude mode", func() { + input := []syslog.Binding{ + {AppId: "app-id", Hostname: "we.dont.care", Drain: syslog.Drain{Url: "https://test.org/drain?exclude-log-types=rtr,unknown"}}, + } + filter = bindings.NewFilteredBindingFetcher( + mockic, + &SpyBindingReader{bindings: input}, + metrics, + warn, + log, + ) + + actual, err := filter.FetchBindings() + + Expect(err).ToNot(HaveOccurred()) + Expect(actual).To(HaveLen(0)) + Expect(logBuffer.String()).Should(MatchRegexp("Unknown log types")) + Expect(logBuffer.String()).Should(MatchRegexp("unknown")) + Expect(metrics.GetMetric("invalid_drains", map[string]string{"unit": "total"}).Value()).To(Equal(1.0)) + }) + + Context("when configured not to warn", func() { + BeforeEach(func() { + warn = false + }) + It("doesn't log the warning", func() { + input := []syslog.Binding{ + {AppId: "app-id", Hostname: "we.dont.care", Drain: syslog.Drain{Url: "https://test.org/drain?include-log-types=app,unknown,rtr"}}, + } + filter = bindings.NewFilteredBindingFetcher( + mockic, + &SpyBindingReader{bindings: input}, + metrics, + warn, + log, + ) + + _, err := filter.FetchBindings() + Expect(err).ToNot(HaveOccurred()) + Expect(logBuffer.String()).ToNot(MatchRegexp("Unknown log types")) + }) + }) + }) + Context("when the syslog drain has been blacklisted", func() { var logBuffer bytes.Buffer var warn bool From da09b15225a7aa0b13f34dab58cbb9c8e783dd21 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Mon, 2 Feb 2026 17:54:52 +0100 Subject: [PATCH 17/36] Dereference pointer to make an actual copy I noticed that as my changes depend on `RawQuery` not being empty, but actually they were. --- src/pkg/ingress/bindings/filtered_binding_fetcher.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pkg/ingress/bindings/filtered_binding_fetcher.go b/src/pkg/ingress/bindings/filtered_binding_fetcher.go index 068ab01f9..45c65aa2c 100644 --- a/src/pkg/ingress/bindings/filtered_binding_fetcher.go +++ b/src/pkg/ingress/bindings/filtered_binding_fetcher.go @@ -56,7 +56,7 @@ func (f *FilteredBindingFetcher) FetchBindings() ([]syslog.Binding, error) { continue } - anonymousUrl := u + anonymousUrl := *u anonymousUrl.User = nil anonymousUrl.RawQuery = "" From 9d442dafc746e1f86a5aede8e7c22da1e7f3d41d Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Wed, 11 Feb 2026 09:07:26 +0100 Subject: [PATCH 18/36] Rename log type to source type where possible --- .../egress/syslog/filtering_drain_writer.go | 8 +-- .../syslog/filtering_drain_writer_test.go | 10 +-- src/pkg/egress/syslog/log.go | 63 ------------------ src/pkg/egress/syslog/source.go | 64 +++++++++++++++++++ src/pkg/egress/syslog/syslog_connector.go | 2 +- src/pkg/ingress/bindings/binding_config.go | 30 ++++----- .../ingress/bindings/binding_config_test.go | 32 +++++----- .../bindings/filtered_binding_fetcher.go | 32 +++++----- .../bindings/filtered_binding_fetcher_test.go | 22 +++---- 9 files changed, 132 insertions(+), 131 deletions(-) delete mode 100644 src/pkg/egress/syslog/log.go create mode 100644 src/pkg/egress/syslog/source.go diff --git a/src/pkg/egress/syslog/filtering_drain_writer.go b/src/pkg/egress/syslog/filtering_drain_writer.go index 79e136c09..247520929 100644 --- a/src/pkg/egress/syslog/filtering_drain_writer.go +++ b/src/pkg/egress/syslog/filtering_drain_writer.go @@ -70,7 +70,7 @@ func (w *FilteringDrainWriter) Write(env *loggregator_v2.Envelope) error { } // shouldIncludeLog determines if a log with the given sourceTypeTag should be forwarded -func shouldIncludeLog(logFilter *LogTypeSet, sourceTypeTag string) bool { +func shouldIncludeLog(logFilter *SourceTypeSet, sourceTypeTag string) bool { // Empty filter or missing source type means no filtering if logFilter == nil || sourceTypeTag == "" { return true @@ -84,16 +84,16 @@ func shouldIncludeLog(logFilter *LogTypeSet, sourceTypeTag string) bool { } // Prefer map lookup over switch for performance - logType := LogType(prefix) + logType := SourceType(prefix) if !logType.IsValid() { - // Unknown log type, default to not filtering + // Unknown source type, default to not filtering return true } return logFilter.Contains(logType) } -func sendsLogs(drainData DrainData, logFilter *LogTypeSet, sourceTypeTag string) bool { +func sendsLogs(drainData DrainData, logFilter *SourceTypeSet, sourceTypeTag string) bool { if drainData != LOGS && drainData != LOGS_AND_METRICS && drainData != LOGS_NO_EVENTS { return false } diff --git a/src/pkg/egress/syslog/filtering_drain_writer_test.go b/src/pkg/egress/syslog/filtering_drain_writer_test.go index 561e5b764..696290cf3 100644 --- a/src/pkg/egress/syslog/filtering_drain_writer_test.go +++ b/src/pkg/egress/syslog/filtering_drain_writer_test.go @@ -80,7 +80,7 @@ var _ = Describe("Filtering Drain Writer", func() { }) It("filters logs based on include filter - includes only APP logs", func() { - appFilter := syslog.LogTypeSet{syslog.LOG_APP: struct{}{}} + appFilter := syslog.SourceTypeSet{syslog.SOURCE_APP: struct{}{}} binding := syslog.Binding{ DrainData: syslog.LOGS, LogFilter: &appFilter, @@ -121,9 +121,9 @@ var _ = Describe("Filtering Drain Writer", func() { It("filters logs based on exclude filter - excludes RTR logs", func() { // Include APP and STG, effectively excluding RTR - includeFilter := syslog.LogTypeSet{ - syslog.LOG_APP: struct{}{}, - syslog.LOG_STG: struct{}{}, + includeFilter := syslog.SourceTypeSet{ + syslog.SOURCE_APP: struct{}{}, + syslog.SOURCE_STG: struct{}{}, } binding := syslog.Binding{ DrainData: syslog.LOGS, @@ -164,7 +164,7 @@ var _ = Describe("Filtering Drain Writer", func() { }) It("sends logs with unknown source_type prefix when filter is set", func() { - appFilter := syslog.LogTypeSet{syslog.LOG_APP: struct{}{}} + appFilter := syslog.SourceTypeSet{syslog.SOURCE_APP: struct{}{}} binding := syslog.Binding{ DrainData: syslog.LOGS, LogFilter: &appFilter, diff --git a/src/pkg/egress/syslog/log.go b/src/pkg/egress/syslog/log.go deleted file mode 100644 index e41cf75b1..000000000 --- a/src/pkg/egress/syslog/log.go +++ /dev/null @@ -1,63 +0,0 @@ -package syslog - -import "strings" - -// LogType defines the log types used within Cloud Foundry -// Their order in the code is as documented in https://docs.cloudfoundry.org/devguide/deploy-apps/streaming-logs.html#format -type LogType string - -const ( - LOG_API LogType = "API" - LOG_STG LogType = "STG" - LOG_RTR LogType = "RTR" - LOG_LGR LogType = "LGR" - LOG_APP LogType = "APP" - LOG_SSH LogType = "SSH" - LOG_CELL LogType = "CELL" -) - -// validLogTypes contains LogType prefixes for efficient lookup -var validLogTypes = map[LogType]struct{}{ - LOG_API: {}, - LOG_STG: {}, - LOG_RTR: {}, - LOG_LGR: {}, - LOG_APP: {}, - LOG_SSH: {}, - LOG_CELL: {}, -} - -// IsValid checks if the provided LogType is valid -func (lt LogType) IsValid() bool { - _, ok := validLogTypes[lt] - return ok -} - -// AllLogTypes returns all valid log types -func AllLogTypes() []LogType { - types := make([]LogType, 0, len(validLogTypes)) - for t := range validLogTypes { - types = append(types, t) - } - return types -} - -// LogTypeSet is a set of LogTypes for efficient membership checking -type LogTypeSet map[LogType]struct{} - -// Add adds a LogType to the set -func (s LogTypeSet) Add(lt LogType) { - s[lt] = struct{}{} -} - -// Contains checks if the set contains a LogType -func (s LogTypeSet) Contains(lt LogType) bool { - _, exists := s[lt] - return exists -} - -// ParseLogType parses a string into a LogType value -func ParseLogType(s string) (LogType, bool) { - lt := LogType(strings.ToUpper(s)) - return lt, lt.IsValid() -} diff --git a/src/pkg/egress/syslog/source.go b/src/pkg/egress/syslog/source.go new file mode 100644 index 000000000..9128c0626 --- /dev/null +++ b/src/pkg/egress/syslog/source.go @@ -0,0 +1,64 @@ +package syslog + +import "strings" + +// SourceType defines the source types used within Cloud Foundry +// Their order in the code is as documented in https://docs.cloudfoundry.org/devguide/deploy-apps/streaming-logs.html#format +type SourceType string + +const ( + SOURCE_API SourceType = "API" + SOURCE_STG SourceType = "STG" + SOURCE_RTR SourceType = "RTR" + SOURCE_LGR SourceType = "LGR" + SOURCE_APP SourceType = "APP" + SOURCE_SSH SourceType = "SSH" + SOURCE_CELL SourceType = "CELL" + // TODO PROXY missing. Anything else as well? Also I guess there will be new ones in the future? +) + +// validSourceTypes contains SourceType prefixes for efficient lookup +var validSourceTypes = map[SourceType]struct{}{ + SOURCE_API: {}, + SOURCE_STG: {}, + SOURCE_RTR: {}, + SOURCE_LGR: {}, + SOURCE_APP: {}, + SOURCE_SSH: {}, + SOURCE_CELL: {}, +} + +// IsValid checks if the provided SourceType is valid +func (lt SourceType) IsValid() bool { + _, ok := validSourceTypes[lt] + return ok +} + +// AllSourceTypes returns all valid source types +func AllSourceTypes() []SourceType { + types := make([]SourceType, 0, len(validSourceTypes)) + for t := range validSourceTypes { + types = append(types, t) + } + return types +} + +// SourceTypeSet is a set of SourceTypes for efficient membership checking +type SourceTypeSet map[SourceType]struct{} + +// Add adds a SourceType to the set +func (s SourceTypeSet) Add(lt SourceType) { + s[lt] = struct{}{} +} + +// Contains checks if the set contains a SourceType +func (s SourceTypeSet) Contains(lt SourceType) bool { + _, exists := s[lt] + return exists +} + +// ParseSourceType parses a string into a SourceType value +func ParseSourceType(s string) (SourceType, bool) { + lt := SourceType(strings.ToUpper(s)) + return lt, lt.IsValid() +} diff --git a/src/pkg/egress/syslog/syslog_connector.go b/src/pkg/egress/syslog/syslog_connector.go index d46b580fc..8ca6bcd2b 100644 --- a/src/pkg/egress/syslog/syslog_connector.go +++ b/src/pkg/egress/syslog/syslog_connector.go @@ -19,7 +19,7 @@ type Binding struct { DrainData DrainData `json:"type,omitempty"` OmitMetadata bool InternalTls bool - LogFilter *LogTypeSet + LogFilter *SourceTypeSet } type Drain struct { diff --git a/src/pkg/ingress/bindings/binding_config.go b/src/pkg/ingress/bindings/binding_config.go index e36acd1ae..a746261a6 100644 --- a/src/pkg/ingress/bindings/binding_config.go +++ b/src/pkg/ingress/bindings/binding_config.go @@ -86,39 +86,39 @@ func getBindingType(u *url.URL) syslog.DrainData { return drainData } -func (d *DrainParamParser) getLogFilter(u *url.URL) *syslog.LogTypeSet { - includeLogTypes := u.Query().Get("include-log-types") - excludeLogTypes := u.Query().Get("exclude-log-types") - - if excludeLogTypes != "" { - return d.NewLogTypeSet(excludeLogTypes, true) - } else if includeLogTypes != "" { - return d.NewLogTypeSet(includeLogTypes, false) +func (d *DrainParamParser) getLogFilter(u *url.URL) *syslog.SourceTypeSet { + includeSourceTypes := u.Query().Get("include-source-types") + excludeSourceTypes := u.Query().Get("exclude-source-types") + + if excludeSourceTypes != "" { + return d.NewSourceTypeSet(excludeSourceTypes, true) + } else if includeSourceTypes != "" { + return d.NewSourceTypeSet(includeSourceTypes, false) } return nil } -// NewLogTypeSet parses a URL query parameter into a Set of LogTypes. -// logTypeList is assumed to be a comma-separated list of valid log types. -func (d *DrainParamParser) NewLogTypeSet(logTypeList string, isExclude bool) *syslog.LogTypeSet { +// NewSourceTypeSet parses a URL query parameter into a Set of SourceTypes. +// logTypeList is assumed to be a comma-separated list of valid source types. +func (d *DrainParamParser) NewSourceTypeSet(logTypeList string, isExclude bool) *syslog.SourceTypeSet { if logTypeList == "" { return nil } logTypes := strings.Split(logTypeList, ",") - set := make(syslog.LogTypeSet, len(logTypes)) + set := make(syslog.SourceTypeSet, len(logTypes)) for _, logType := range logTypes { logType = strings.TrimSpace(logType) - t, _ := syslog.ParseLogType(logType) + t, _ := syslog.ParseSourceType(logType) set.Add(t) } if isExclude { // Invert the set - include all types except those in the set - fullSet := make(syslog.LogTypeSet) + fullSet := make(syslog.SourceTypeSet) - for _, t := range syslog.AllLogTypes() { + for _, t := range syslog.AllSourceTypes() { fullSet.Add(t) } diff --git a/src/pkg/ingress/bindings/binding_config_test.go b/src/pkg/ingress/bindings/binding_config_test.go index 2af8df173..49b236df4 100644 --- a/src/pkg/ingress/bindings/binding_config_test.go +++ b/src/pkg/ingress/bindings/binding_config_test.go @@ -119,32 +119,32 @@ var _ = Describe("Drain Param Config", func() { testCases := []struct { name string url string - expected *syslog.LogTypeSet + expected *syslog.SourceTypeSet }{ { name: "empty drain URL defaults to all types", url: "https://test.org/drain", - expected: NewLogTypeSet(), + expected: NewSourceTypeSet(), }, { - name: "include-log-types=app", - url: "https://test.org/drain?include-log-types=app", - expected: NewLogTypeSet(syslog.LOG_APP), + name: "include-source-types=app", + url: "https://test.org/drain?include-source-types=app", + expected: NewSourceTypeSet(syslog.SOURCE_APP), }, { - name: "include-log-types=app,stg,cell", - url: "https://test.org/drain?include-log-types=app,stg,cell", - expected: NewLogTypeSet(syslog.LOG_APP, syslog.LOG_STG, syslog.LOG_CELL), + name: "include-source-types=app,stg,cell", + url: "https://test.org/drain?include-source-types=app,stg,cell", + expected: NewSourceTypeSet(syslog.SOURCE_APP, syslog.SOURCE_STG, syslog.SOURCE_CELL), }, { - name: "exclude-log-types=rtr,cell,stg", - url: "https://test.org/drain?exclude-log-types=rtr,cell,stg", - expected: NewLogTypeSet(syslog.LOG_API, syslog.LOG_LGR, syslog.LOG_APP, syslog.LOG_SSH), + name: "exclude-source-types=rtr,cell,stg", + url: "https://test.org/drain?exclude-source-types=rtr,cell,stg", + expected: NewSourceTypeSet(syslog.SOURCE_API, syslog.SOURCE_LGR, syslog.SOURCE_APP, syslog.SOURCE_SSH), }, { - name: "exclude-log-types=rtr", - url: "https://test.org/drain?exclude-log-types=rtr", - expected: NewLogTypeSet(syslog.LOG_API, syslog.LOG_STG, syslog.LOG_LGR, syslog.LOG_APP, syslog.LOG_SSH, syslog.LOG_CELL), + name: "exclude-source-types=rtr", + url: "https://test.org/drain?exclude-source-types=rtr", + expected: NewSourceTypeSet(syslog.SOURCE_API, syslog.SOURCE_STG, syslog.SOURCE_LGR, syslog.SOURCE_APP, syslog.SOURCE_SSH, syslog.SOURCE_CELL), }, } @@ -252,11 +252,11 @@ func (f *stubFetcher) DrainLimit() int { return -1 } -func NewLogTypeSet(logTypes ...syslog.LogType) *syslog.LogTypeSet { +func NewSourceTypeSet(logTypes ...syslog.SourceType) *syslog.SourceTypeSet { if len(logTypes) == 0 { return nil } - set := make(syslog.LogTypeSet, len(logTypes)) + set := make(syslog.SourceTypeSet, len(logTypes)) for _, t := range logTypes { set[t] = struct{}{} } diff --git a/src/pkg/ingress/bindings/filtered_binding_fetcher.go b/src/pkg/ingress/bindings/filtered_binding_fetcher.go index 45c65aa2c..7a8536f02 100644 --- a/src/pkg/ingress/bindings/filtered_binding_fetcher.go +++ b/src/pkg/ingress/bindings/filtered_binding_fetcher.go @@ -72,14 +72,14 @@ func (f *FilteredBindingFetcher) FetchBindings() ([]syslog.Binding, error) { if invalidLogFilter(u) { invalidDrains += 1 - f.printWarning("include-log-types and exclude-log-types cannot be used at the same time in syslog drain url %s for application %s", anonymousUrl.String(), b.AppId) + f.printWarning("include-source-types and exclude-source-types cannot be used at the same time in syslog drain url %s for application %s", anonymousUrl.String(), b.AppId) continue } - logTypes := getUnknownLogTypes(u.Query()) + logTypes := getUnknownSourceTypes(u.Query()) if logTypes != nil { invalidDrains += 1 - f.printWarning("Unknown log types '%s' in log type filter in syslog drain url %s for application %s", strings.Join(logTypes, ", "), anonymousUrl.String(), b.AppId) + f.printWarning("Unknown source types '%s' in source type filter in syslog drain url %s for application %s", strings.Join(logTypes, ", "), anonymousUrl.String(), b.AppId) continue } @@ -113,26 +113,26 @@ func (f *FilteredBindingFetcher) FetchBindings() ([]syslog.Binding, error) { } -// invalidLogFilter checks if both include-log-types and exclude-log-types +// invalidLogFilter checks if both include-source-types and exclude-source-types func invalidLogFilter(u *url.URL) bool { - includeLogTypes := u.Query().Get("include-log-types") - excludeLogTypes := u.Query().Get("exclude-log-types") - if excludeLogTypes != "" && includeLogTypes != "" { + includeSourceTypes := u.Query().Get("include-source-types") + excludeSourceTypes := u.Query().Get("exclude-source-types") + if excludeSourceTypes != "" && includeSourceTypes != "" { return true } return false } -// assumes only one of include-log-types or exclude-log-types is set -func getUnknownLogTypes(u url.Values) []string { +// assumes only one of include-source-types or exclude-source-types is set +func getUnknownSourceTypes(u url.Values) []string { var logTypeList string - includeLogTypes := u.Get("include-log-types") - excludeLogTypes := u.Get("exclude-log-types") + includeSourceTypes := u.Get("include-source-types") + excludeSourceTypes := u.Get("exclude-source-types") - if includeLogTypes != "" { - logTypeList = includeLogTypes - } else if excludeLogTypes != "" { - logTypeList = excludeLogTypes + if includeSourceTypes != "" { + logTypeList = includeSourceTypes + } else if excludeSourceTypes != "" { + logTypeList = excludeSourceTypes } else { return nil } @@ -142,7 +142,7 @@ func getUnknownLogTypes(u url.Values) []string { for _, logType := range logTypes { logType = strings.TrimSpace(logType) - _, ok := syslog.ParseLogType(logType) + _, ok := syslog.ParseSourceType(logType) if !ok { unknownTypes = append(unknownTypes, logType) continue diff --git a/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go b/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go index f8586474c..a0cbf0056 100644 --- a/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go +++ b/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go @@ -239,7 +239,7 @@ var _ = Describe("FilteredBindingFetcher", func() { }) }) - Context("when both include-log-types and exclude-log-types are specified", func() { + Context("when both include-source-types and exclude-source-types are specified", func() { var logBuffer bytes.Buffer var warn bool var mockic *bindingsfakes.FakeIPChecker @@ -255,7 +255,7 @@ var _ = Describe("FilteredBindingFetcher", func() { JustBeforeEach(func() { input := []syslog.Binding{ - {AppId: "app-id", Hostname: "we.dont.care", Drain: syslog.Drain{Url: "https://test.org/drain?include-log-types=app&exclude-log-types=rtr"}}, + {AppId: "app-id", Hostname: "we.dont.care", Drain: syslog.Drain{Url: "https://test.org/drain?include-source-types=app&exclude-source-types=rtr"}}, } filter = bindings.NewFilteredBindingFetcher( mockic, @@ -271,7 +271,7 @@ var _ = Describe("FilteredBindingFetcher", func() { Expect(err).ToNot(HaveOccurred()) Expect(actual).To(HaveLen(0)) - Expect(logBuffer.String()).Should(MatchRegexp("include-log-types and exclude-log-types cannot be used at the same time")) + Expect(logBuffer.String()).Should(MatchRegexp("include-source-types and exclude-source-types cannot be used at the same time")) Expect(metrics.GetMetric("invalid_drains", map[string]string{"unit": "total"}).Value()).To(Equal(1.0)) }) @@ -283,12 +283,12 @@ var _ = Describe("FilteredBindingFetcher", func() { It("doesn't log the conflicting filters warning", func() { _, err := filter.FetchBindings() Expect(err).ToNot(HaveOccurred()) - Expect(logBuffer.String()).ToNot(MatchRegexp("include-log-types and exclude-log-types cannot be used at the same time")) + Expect(logBuffer.String()).ToNot(MatchRegexp("include-source-types and exclude-source-types cannot be used at the same time")) }) }) }) - Context("when unknown log types are provided", func() { + Context("when unknown source types are provided", func() { var logBuffer bytes.Buffer var warn bool var mockic *bindingsfakes.FakeIPChecker @@ -304,7 +304,7 @@ var _ = Describe("FilteredBindingFetcher", func() { It("logs a warning and ignores the drain in include mode", func() { input := []syslog.Binding{ - {AppId: "app-id", Hostname: "we.dont.care", Drain: syslog.Drain{Url: "https://test.org/drain?include-log-types=app,unknown,invalid,rtr"}}, + {AppId: "app-id", Hostname: "we.dont.care", Drain: syslog.Drain{Url: "https://test.org/drain?include-source-types=app,unknown,invalid,rtr"}}, } filter = bindings.NewFilteredBindingFetcher( mockic, @@ -318,7 +318,7 @@ var _ = Describe("FilteredBindingFetcher", func() { Expect(err).ToNot(HaveOccurred()) Expect(actual).To(HaveLen(0)) - Expect(logBuffer.String()).Should(MatchRegexp("Unknown log types")) + Expect(logBuffer.String()).Should(MatchRegexp("Unknown source types")) Expect(logBuffer.String()).Should(MatchRegexp("unknown")) Expect(logBuffer.String()).Should(MatchRegexp("invalid")) Expect(metrics.GetMetric("invalid_drains", map[string]string{"unit": "total"}).Value()).To(Equal(1.0)) @@ -326,7 +326,7 @@ var _ = Describe("FilteredBindingFetcher", func() { It("logs a warning and ignores the drain in exclude mode", func() { input := []syslog.Binding{ - {AppId: "app-id", Hostname: "we.dont.care", Drain: syslog.Drain{Url: "https://test.org/drain?exclude-log-types=rtr,unknown"}}, + {AppId: "app-id", Hostname: "we.dont.care", Drain: syslog.Drain{Url: "https://test.org/drain?exclude-source-types=rtr,unknown"}}, } filter = bindings.NewFilteredBindingFetcher( mockic, @@ -340,7 +340,7 @@ var _ = Describe("FilteredBindingFetcher", func() { Expect(err).ToNot(HaveOccurred()) Expect(actual).To(HaveLen(0)) - Expect(logBuffer.String()).Should(MatchRegexp("Unknown log types")) + Expect(logBuffer.String()).Should(MatchRegexp("Unknown source types")) Expect(logBuffer.String()).Should(MatchRegexp("unknown")) Expect(metrics.GetMetric("invalid_drains", map[string]string{"unit": "total"}).Value()).To(Equal(1.0)) }) @@ -351,7 +351,7 @@ var _ = Describe("FilteredBindingFetcher", func() { }) It("doesn't log the warning", func() { input := []syslog.Binding{ - {AppId: "app-id", Hostname: "we.dont.care", Drain: syslog.Drain{Url: "https://test.org/drain?include-log-types=app,unknown,rtr"}}, + {AppId: "app-id", Hostname: "we.dont.care", Drain: syslog.Drain{Url: "https://test.org/drain?include-source-types=app,unknown,rtr"}}, } filter = bindings.NewFilteredBindingFetcher( mockic, @@ -363,7 +363,7 @@ var _ = Describe("FilteredBindingFetcher", func() { _, err := filter.FetchBindings() Expect(err).ToNot(HaveOccurred()) - Expect(logBuffer.String()).ToNot(MatchRegexp("Unknown log types")) + Expect(logBuffer.String()).ToNot(MatchRegexp("Unknown source types")) }) }) }) From e73379222717b444b925f701c752c309ace8d6d2 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Wed, 11 Feb 2026 14:04:33 +0100 Subject: [PATCH 19/36] Only exclude filter shall pass unknown source type Also rename remaining instances of log type to source type --- .../egress/syslog/filtering_drain_writer.go | 39 +--------- .../syslog/filtering_drain_writer_test.go | 75 +++++++++++-------- src/pkg/egress/syslog/source.go | 64 +++++++++++++++- src/pkg/egress/syslog/syslog_connector.go | 2 +- src/pkg/ingress/bindings/binding_config.go | 40 ++++------ .../ingress/bindings/binding_config_test.go | 23 +++--- .../bindings/filtered_binding_fetcher.go | 22 +++--- 7 files changed, 143 insertions(+), 122 deletions(-) diff --git a/src/pkg/egress/syslog/filtering_drain_writer.go b/src/pkg/egress/syslog/filtering_drain_writer.go index 247520929..57e7bef6b 100644 --- a/src/pkg/egress/syslog/filtering_drain_writer.go +++ b/src/pkg/egress/syslog/filtering_drain_writer.go @@ -2,7 +2,6 @@ package syslog import ( "errors" - "strings" "code.cloudfoundry.org/go-loggregator/v10/rpc/loggregator_v2" "code.cloudfoundry.org/loggregator-agent-release/src/pkg/egress" @@ -51,11 +50,7 @@ func (w *FilteringDrainWriter) Write(env *loggregator_v2.Envelope) error { } } if env.GetLog() != nil { - sourceType, ok := env.GetTags()["source_type"] - if !ok { - // Default to sending logs if no source_type tag is present - sourceType = "" - } + sourceType := env.GetTags()["source_type"] if sendsLogs(w.binding.DrainData, w.binding.LogFilter, sourceType) { return w.writer.Write(env) } @@ -69,40 +64,12 @@ func (w *FilteringDrainWriter) Write(env *loggregator_v2.Envelope) error { return nil } -// shouldIncludeLog determines if a log with the given sourceTypeTag should be forwarded -func shouldIncludeLog(logFilter *SourceTypeSet, sourceTypeTag string) bool { - // Empty filter or missing source type means no filtering - if logFilter == nil || sourceTypeTag == "" { - return true - } - - // Find the first "/" to extract prefix - idx := strings.IndexByte(sourceTypeTag, '/') - prefix := sourceTypeTag - if idx != -1 { - prefix = sourceTypeTag[:idx] - } - - // Prefer map lookup over switch for performance - logType := SourceType(prefix) - if !logType.IsValid() { - // Unknown source type, default to not filtering - return true - } - - return logFilter.Contains(logType) -} - -func sendsLogs(drainData DrainData, logFilter *SourceTypeSet, sourceTypeTag string) bool { +func sendsLogs(drainData DrainData, logFilter *LogFilter, sourceTypeTag string) bool { if drainData != LOGS && drainData != LOGS_AND_METRICS && drainData != LOGS_NO_EVENTS { return false } - if shouldIncludeLog(logFilter, sourceTypeTag) { - return true - } - - return false + return logFilter.ShouldInclude(sourceTypeTag) } func sendsMetrics(drainData DrainData) bool { diff --git a/src/pkg/egress/syslog/filtering_drain_writer_test.go b/src/pkg/egress/syslog/filtering_drain_writer_test.go index 696290cf3..d14fa5481 100644 --- a/src/pkg/egress/syslog/filtering_drain_writer_test.go +++ b/src/pkg/egress/syslog/filtering_drain_writer_test.go @@ -54,36 +54,57 @@ var _ = Describe("Filtering Drain Writer", func() { Expect(err).To(HaveOccurred()) }) - It("sends logs when source_type tag is missing", func() { - binding := syslog.Binding{ - DrainData: syslog.LOGS, - } - fakeWriter := &fakeWriter{} - drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) - Expect(err).NotTo(HaveOccurred()) + Context("when source_type tag is missing", func() { + var envelope *loggregator_v2.Envelope - envelope := &loggregator_v2.Envelope{ - Message: &loggregator_v2.Envelope_Log{ - Log: &loggregator_v2.Log{ - Payload: []byte("test log"), + BeforeEach(func() { + envelope = &loggregator_v2.Envelope{ + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{ + Payload: []byte("test log"), + }, }, - }, - Tags: map[string]string{ - // source_type tag is intentionally missing - }, - } + Tags: map[string]string{ + // source_type tag is intentionally missing + }, + } + }) - err = drainWriter.Write(envelope) + It("omits logs when source type include filter is configured with LOGS", func() { + binding := syslog.Binding{ + DrainData: syslog.LOGS, + LogFilter: syslog.NewLogFilter(syslog.SourceTypeSet{syslog.SOURCE_APP: struct{}{}}, syslog.LogFilterModeInclude), + } + fakeWriter := &fakeWriter{} + drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) + Expect(err).NotTo(HaveOccurred()) - Expect(err).NotTo(HaveOccurred()) - Expect(fakeWriter.received).To(Equal(1)) + err = drainWriter.Write(envelope) + + Expect(err).NotTo(HaveOccurred()) + Expect(fakeWriter.received).To(Equal(0)) + }) + + It("sends logs when source type exclude filter is configured with LOGS", func() { + binding := syslog.Binding{ + DrainData: syslog.LOGS, + LogFilter: syslog.NewLogFilter(syslog.SourceTypeSet{syslog.SOURCE_RTR: struct{}{}}, syslog.LogFilterModeExclude), + } + fakeWriter := &fakeWriter{} + drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) + Expect(err).NotTo(HaveOccurred()) + + err = drainWriter.Write(envelope) + + Expect(err).NotTo(HaveOccurred()) + Expect(fakeWriter.received).To(Equal(1)) + }) }) It("filters logs based on include filter - includes only APP logs", func() { - appFilter := syslog.SourceTypeSet{syslog.SOURCE_APP: struct{}{}} binding := syslog.Binding{ DrainData: syslog.LOGS, - LogFilter: &appFilter, + LogFilter: syslog.NewLogFilter(syslog.SourceTypeSet{syslog.SOURCE_APP: struct{}{}}, syslog.LogFilterModeInclude), } fakeWriter := &fakeWriter{} drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) @@ -120,14 +141,9 @@ var _ = Describe("Filtering Drain Writer", func() { }) It("filters logs based on exclude filter - excludes RTR logs", func() { - // Include APP and STG, effectively excluding RTR - includeFilter := syslog.SourceTypeSet{ - syslog.SOURCE_APP: struct{}{}, - syslog.SOURCE_STG: struct{}{}, - } binding := syslog.Binding{ DrainData: syslog.LOGS, - LogFilter: &includeFilter, + LogFilter: syslog.NewLogFilter(syslog.SourceTypeSet{syslog.SOURCE_RTR: struct{}{}}, syslog.LogFilterModeExclude), } fakeWriter := &fakeWriter{} drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) @@ -164,10 +180,9 @@ var _ = Describe("Filtering Drain Writer", func() { }) It("sends logs with unknown source_type prefix when filter is set", func() { - appFilter := syslog.SourceTypeSet{syslog.SOURCE_APP: struct{}{}} binding := syslog.Binding{ DrainData: syslog.LOGS, - LogFilter: &appFilter, + LogFilter: syslog.NewLogFilter(syslog.SourceTypeSet{syslog.SOURCE_APP: struct{}{}}, syslog.LogFilterModeExclude), } fakeWriter := &fakeWriter{} drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) @@ -186,7 +201,7 @@ var _ = Describe("Filtering Drain Writer", func() { err = drainWriter.Write(envelope) - // Should send the log because unknown types default to being included + // Should send the log because unknown types default to being included for exclude filter Expect(err).NotTo(HaveOccurred()) Expect(fakeWriter.received).To(Equal(1)) }) diff --git a/src/pkg/egress/syslog/source.go b/src/pkg/egress/syslog/source.go index 9128c0626..ed3f9ba07 100644 --- a/src/pkg/egress/syslog/source.go +++ b/src/pkg/egress/syslog/source.go @@ -34,6 +34,12 @@ func (lt SourceType) IsValid() bool { return ok } +// ParseSourceType parses a string into a SourceType value +func ParseSourceType(s string) (SourceType, bool) { + lt := SourceType(strings.ToUpper(s)) + return lt, lt.IsValid() +} + // AllSourceTypes returns all valid source types func AllSourceTypes() []SourceType { types := make([]SourceType, 0, len(validSourceTypes)) @@ -43,6 +49,14 @@ func AllSourceTypes() []SourceType { return types } +// ExtractPrefix extracts the prefix from a source_type tag (e.g., "APP/PROC/WEB/0" -> "APP") +func ExtractPrefix(sourceTypeTag string) string { + if idx := strings.IndexByte(sourceTypeTag, '/'); idx != -1 { + return sourceTypeTag[:idx] + } + return sourceTypeTag +} + // SourceTypeSet is a set of SourceTypes for efficient membership checking type SourceTypeSet map[SourceType]struct{} @@ -57,8 +71,50 @@ func (s SourceTypeSet) Contains(lt SourceType) bool { return exists } -// ParseSourceType parses a string into a SourceType value -func ParseSourceType(s string) (SourceType, bool) { - lt := SourceType(strings.ToUpper(s)) - return lt, lt.IsValid() +// LogFilterMode determines how the log filter should be applied +type LogFilterMode int + +const ( + // LogFilterModeInclude only includes logs matching the specified types (strict) + LogFilterModeInclude LogFilterMode = iota + // LogFilterModeExclude excludes logs matching the specified types (permissive) + LogFilterModeExclude +) + +// LogFilter encapsulates source type filtering configuration +type LogFilter struct { + Types SourceTypeSet + Mode LogFilterMode +} + +// NewLogFilter creates a new LogFilter with the given types and mode +func NewLogFilter(types SourceTypeSet, mode LogFilterMode) *LogFilter { + return &LogFilter{ + Types: types, + Mode: mode, + } +} + +// ShouldInclude determines if a log with the given sourceTypeTag should be forwarded +// Include mode omits missing/unknown source types, exclude mode forwards them +func (f *LogFilter) ShouldInclude(sourceTypeTag string) bool { + if f == nil { + return true + } + + if sourceTypeTag == "" { + return f.Mode == LogFilterModeExclude + } + + prefix := ExtractPrefix(sourceTypeTag) + sourceType := SourceType(prefix) + if !sourceType.IsValid() { + return f.Mode == LogFilterModeExclude + } + + inSet := f.Types.Contains(sourceType) + if f.Mode == LogFilterModeInclude { + return inSet + } + return !inSet } diff --git a/src/pkg/egress/syslog/syslog_connector.go b/src/pkg/egress/syslog/syslog_connector.go index 8ca6bcd2b..9f948bc00 100644 --- a/src/pkg/egress/syslog/syslog_connector.go +++ b/src/pkg/egress/syslog/syslog_connector.go @@ -19,7 +19,7 @@ type Binding struct { DrainData DrainData `json:"type,omitempty"` OmitMetadata bool InternalTls bool - LogFilter *SourceTypeSet + LogFilter *LogFilter } type Drain struct { diff --git a/src/pkg/ingress/bindings/binding_config.go b/src/pkg/ingress/bindings/binding_config.go index a746261a6..4db30fd65 100644 --- a/src/pkg/ingress/bindings/binding_config.go +++ b/src/pkg/ingress/bindings/binding_config.go @@ -86,49 +86,35 @@ func getBindingType(u *url.URL) syslog.DrainData { return drainData } -func (d *DrainParamParser) getLogFilter(u *url.URL) *syslog.SourceTypeSet { +func (d *DrainParamParser) getLogFilter(u *url.URL) *syslog.LogFilter { includeSourceTypes := u.Query().Get("include-source-types") excludeSourceTypes := u.Query().Get("exclude-source-types") if excludeSourceTypes != "" { - return d.NewSourceTypeSet(excludeSourceTypes, true) + return d.newLogFilter(excludeSourceTypes, syslog.LogFilterModeExclude) } else if includeSourceTypes != "" { - return d.NewSourceTypeSet(includeSourceTypes, false) + return d.newLogFilter(includeSourceTypes, syslog.LogFilterModeInclude) } return nil } -// NewSourceTypeSet parses a URL query parameter into a Set of SourceTypes. -// logTypeList is assumed to be a comma-separated list of valid source types. -func (d *DrainParamParser) NewSourceTypeSet(logTypeList string, isExclude bool) *syslog.SourceTypeSet { - if logTypeList == "" { +// newLogFilter parses a URL query parameter into a LogFilter. +// sourceTypeList is assumed to be a comma-separated list of valid source types. +func (d *DrainParamParser) newLogFilter(sourceTypeList string, mode syslog.LogFilterMode) *syslog.LogFilter { + if sourceTypeList == "" { return nil } - logTypes := strings.Split(logTypeList, ",") - set := make(syslog.SourceTypeSet, len(logTypes)) + sourceTypes := strings.Split(sourceTypeList, ",") + set := make(syslog.SourceTypeSet, len(sourceTypes)) - for _, logType := range logTypes { - logType = strings.TrimSpace(logType) - t, _ := syslog.ParseSourceType(logType) + for _, sourceType := range sourceTypes { + sourceType = strings.TrimSpace(sourceType) + t, _ := syslog.ParseSourceType(sourceType) set.Add(t) } - if isExclude { - // Invert the set - include all types except those in the set - fullSet := make(syslog.SourceTypeSet) - - for _, t := range syslog.AllSourceTypes() { - fullSet.Add(t) - } - - for t := range set { - delete(fullSet, t) - } - return &fullSet - } - - return &set + return syslog.NewLogFilter(set, mode) } func getRemoveMetadataQuery(u *url.URL) string { diff --git a/src/pkg/ingress/bindings/binding_config_test.go b/src/pkg/ingress/bindings/binding_config_test.go index 49b236df4..031e6f22f 100644 --- a/src/pkg/ingress/bindings/binding_config_test.go +++ b/src/pkg/ingress/bindings/binding_config_test.go @@ -119,32 +119,32 @@ var _ = Describe("Drain Param Config", func() { testCases := []struct { name string url string - expected *syslog.SourceTypeSet + expected *syslog.LogFilter }{ { name: "empty drain URL defaults to all types", url: "https://test.org/drain", - expected: NewSourceTypeSet(), + expected: nil, }, { name: "include-source-types=app", url: "https://test.org/drain?include-source-types=app", - expected: NewSourceTypeSet(syslog.SOURCE_APP), + expected: NewLogFilter(syslog.LogFilterModeInclude, syslog.SOURCE_APP), }, { name: "include-source-types=app,stg,cell", url: "https://test.org/drain?include-source-types=app,stg,cell", - expected: NewSourceTypeSet(syslog.SOURCE_APP, syslog.SOURCE_STG, syslog.SOURCE_CELL), + expected: NewLogFilter(syslog.LogFilterModeInclude, syslog.SOURCE_APP, syslog.SOURCE_STG, syslog.SOURCE_CELL), }, { name: "exclude-source-types=rtr,cell,stg", url: "https://test.org/drain?exclude-source-types=rtr,cell,stg", - expected: NewSourceTypeSet(syslog.SOURCE_API, syslog.SOURCE_LGR, syslog.SOURCE_APP, syslog.SOURCE_SSH), + expected: NewLogFilter(syslog.LogFilterModeExclude, syslog.SOURCE_RTR, syslog.SOURCE_CELL, syslog.SOURCE_STG), }, { name: "exclude-source-types=rtr", url: "https://test.org/drain?exclude-source-types=rtr", - expected: NewSourceTypeSet(syslog.SOURCE_API, syslog.SOURCE_STG, syslog.SOURCE_LGR, syslog.SOURCE_APP, syslog.SOURCE_SSH, syslog.SOURCE_CELL), + expected: NewLogFilter(syslog.LogFilterModeExclude, syslog.SOURCE_RTR), }, } @@ -252,13 +252,10 @@ func (f *stubFetcher) DrainLimit() int { return -1 } -func NewSourceTypeSet(logTypes ...syslog.SourceType) *syslog.SourceTypeSet { - if len(logTypes) == 0 { - return nil - } - set := make(syslog.SourceTypeSet, len(logTypes)) - for _, t := range logTypes { +func NewLogFilter(mode syslog.LogFilterMode, sourceTypes ...syslog.SourceType) *syslog.LogFilter { + set := make(syslog.SourceTypeSet, len(sourceTypes)) + for _, t := range sourceTypes { set[t] = struct{}{} } - return &set + return syslog.NewLogFilter(set, mode) } diff --git a/src/pkg/ingress/bindings/filtered_binding_fetcher.go b/src/pkg/ingress/bindings/filtered_binding_fetcher.go index 7a8536f02..b0ff9b1e3 100644 --- a/src/pkg/ingress/bindings/filtered_binding_fetcher.go +++ b/src/pkg/ingress/bindings/filtered_binding_fetcher.go @@ -76,10 +76,10 @@ func (f *FilteredBindingFetcher) FetchBindings() ([]syslog.Binding, error) { continue } - logTypes := getUnknownSourceTypes(u.Query()) - if logTypes != nil { + sourceTypes := getUnknownSourceTypes(u.Query()) + if sourceTypes != nil { invalidDrains += 1 - f.printWarning("Unknown source types '%s' in source type filter in syslog drain url %s for application %s", strings.Join(logTypes, ", "), anonymousUrl.String(), b.AppId) + f.printWarning("Unknown source types '%s' in source type filter in syslog drain url %s for application %s", strings.Join(sourceTypes, ", "), anonymousUrl.String(), b.AppId) continue } @@ -125,26 +125,26 @@ func invalidLogFilter(u *url.URL) bool { // assumes only one of include-source-types or exclude-source-types is set func getUnknownSourceTypes(u url.Values) []string { - var logTypeList string + var sourceTypeList string includeSourceTypes := u.Get("include-source-types") excludeSourceTypes := u.Get("exclude-source-types") if includeSourceTypes != "" { - logTypeList = includeSourceTypes + sourceTypeList = includeSourceTypes } else if excludeSourceTypes != "" { - logTypeList = excludeSourceTypes + sourceTypeList = excludeSourceTypes } else { return nil } - logTypes := strings.Split(logTypeList, ",") + sourceTypes := strings.Split(sourceTypeList, ",") var unknownTypes []string - for _, logType := range logTypes { - logType = strings.TrimSpace(logType) - _, ok := syslog.ParseSourceType(logType) + for _, sourceType := range sourceTypes { + sourceType = strings.TrimSpace(sourceType) + _, ok := syslog.ParseSourceType(sourceType) if !ok { - unknownTypes = append(unknownTypes, logType) + unknownTypes = append(unknownTypes, sourceType) continue } } From a1ec8d18e0790d36ae1c4e5d475d96de0ffee4cb Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Wed, 11 Feb 2026 14:08:11 +0100 Subject: [PATCH 20/36] Add SourceType PROXY, HEALTH, SYS and STATS --- src/pkg/egress/syslog/source.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/pkg/egress/syslog/source.go b/src/pkg/egress/syslog/source.go index ed3f9ba07..27da271af 100644 --- a/src/pkg/egress/syslog/source.go +++ b/src/pkg/egress/syslog/source.go @@ -7,14 +7,17 @@ import "strings" type SourceType string const ( - SOURCE_API SourceType = "API" - SOURCE_STG SourceType = "STG" - SOURCE_RTR SourceType = "RTR" - SOURCE_LGR SourceType = "LGR" - SOURCE_APP SourceType = "APP" - SOURCE_SSH SourceType = "SSH" - SOURCE_CELL SourceType = "CELL" - // TODO PROXY missing. Anything else as well? Also I guess there will be new ones in the future? + SOURCE_API SourceType = "API" + SOURCE_STG SourceType = "STG" + SOURCE_RTR SourceType = "RTR" + SOURCE_LGR SourceType = "LGR" + SOURCE_APP SourceType = "APP" + SOURCE_SSH SourceType = "SSH" + SOURCE_CELL SourceType = "CELL" + SOURCE_PROXY SourceType = "PROXY" + SOURCE_HEALTH SourceType = "HEALTH" + SOURCE_SYS SourceType = "SYS" + SOURCE_STATS SourceType = "STATS" ) // validSourceTypes contains SourceType prefixes for efficient lookup From dd4263332bb229e30b9fcb48e460dd4448dc6fc1 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Fri, 20 Feb 2026 10:00:03 +0100 Subject: [PATCH 21/36] Make missing source_type tests clearer --- .../syslog/filtering_drain_writer_test.go | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/src/pkg/egress/syslog/filtering_drain_writer_test.go b/src/pkg/egress/syslog/filtering_drain_writer_test.go index d14fa5481..cd7aaadc8 100644 --- a/src/pkg/egress/syslog/filtering_drain_writer_test.go +++ b/src/pkg/egress/syslog/filtering_drain_writer_test.go @@ -70,7 +70,7 @@ var _ = Describe("Filtering Drain Writer", func() { } }) - It("omits logs when source type include filter is configured with LOGS", func() { + It("filters out the log with missing source_type when include filter is configured", func() { binding := syslog.Binding{ DrainData: syslog.LOGS, LogFilter: syslog.NewLogFilter(syslog.SourceTypeSet{syslog.SOURCE_APP: struct{}{}}, syslog.LogFilterModeInclude), @@ -80,12 +80,21 @@ var _ = Describe("Filtering Drain Writer", func() { Expect(err).NotTo(HaveOccurred()) err = drainWriter.Write(envelope) - Expect(err).NotTo(HaveOccurred()) Expect(fakeWriter.received).To(Equal(0)) + + appEnvelope := &loggregator_v2.Envelope{ + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{Payload: []byte("app log")}, + }, + Tags: map[string]string{"source_type": "APP/PROC/WEB/0"}, + } + err = drainWriter.Write(appEnvelope) + Expect(err).NotTo(HaveOccurred()) + Expect(fakeWriter.received).To(Equal(1)) }) - It("sends logs when source type exclude filter is configured with LOGS", func() { + It("sends the log with missing source_type when exclude filter is configured", func() { binding := syslog.Binding{ DrainData: syslog.LOGS, LogFilter: syslog.NewLogFilter(syslog.SourceTypeSet{syslog.SOURCE_RTR: struct{}{}}, syslog.LogFilterModeExclude), @@ -95,7 +104,16 @@ var _ = Describe("Filtering Drain Writer", func() { Expect(err).NotTo(HaveOccurred()) err = drainWriter.Write(envelope) + Expect(err).NotTo(HaveOccurred()) + Expect(fakeWriter.received).To(Equal(1)) + rtrEnvelope := &loggregator_v2.Envelope{ + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{Payload: []byte("rtr log")}, + }, + Tags: map[string]string{"source_type": "RTR/1"}, + } + err = drainWriter.Write(rtrEnvelope) Expect(err).NotTo(HaveOccurred()) Expect(fakeWriter.received).To(Equal(1)) }) From 9f7a5dfd33bd7117c806a328045f8e5ec8a50842 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Fri, 20 Feb 2026 11:03:25 +0100 Subject: [PATCH 22/36] Add tests for no filtering value configured --- src/pkg/ingress/bindings/binding_config_test.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/pkg/ingress/bindings/binding_config_test.go b/src/pkg/ingress/bindings/binding_config_test.go index 031e6f22f..93daa9250 100644 --- a/src/pkg/ingress/bindings/binding_config_test.go +++ b/src/pkg/ingress/bindings/binding_config_test.go @@ -122,10 +122,20 @@ var _ = Describe("Drain Param Config", func() { expected *syslog.LogFilter }{ { - name: "empty drain URL defaults to all types", + name: "empty drain URL defaults to no filtering", url: "https://test.org/drain", expected: nil, }, + { + name: "include-source-types= defaults to no filtering", + url: "https://test.org/drain?include-source-types=", + expected: nil, + }, + { + name: "exclude-source-types= defaults to no filtering", + url: "https://test.org/drain?exclude-source-types=", + expected: nil, + }, { name: "include-source-types=app", url: "https://test.org/drain?include-source-types=app", From a520a28a85fd15552297aaeaa05a46a57684ea02 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Fri, 20 Feb 2026 11:26:09 +0100 Subject: [PATCH 23/36] Rename source to log source where sensible --- .../syslog/filtering_drain_writer_test.go | 10 +-- .../syslog/{source.go => log_source.go} | 66 +++++++++---------- src/pkg/ingress/bindings/binding_config.go | 2 +- .../ingress/bindings/binding_config_test.go | 12 ++-- 4 files changed, 45 insertions(+), 45 deletions(-) rename src/pkg/egress/syslog/{source.go => log_source.go} (60%) diff --git a/src/pkg/egress/syslog/filtering_drain_writer_test.go b/src/pkg/egress/syslog/filtering_drain_writer_test.go index cd7aaadc8..f73bc56f7 100644 --- a/src/pkg/egress/syslog/filtering_drain_writer_test.go +++ b/src/pkg/egress/syslog/filtering_drain_writer_test.go @@ -73,7 +73,7 @@ var _ = Describe("Filtering Drain Writer", func() { It("filters out the log with missing source_type when include filter is configured", func() { binding := syslog.Binding{ DrainData: syslog.LOGS, - LogFilter: syslog.NewLogFilter(syslog.SourceTypeSet{syslog.SOURCE_APP: struct{}{}}, syslog.LogFilterModeInclude), + LogFilter: syslog.NewLogFilter(syslog.LogSourceTypeSet{syslog.LOG_SOURCE_APP: struct{}{}}, syslog.LogFilterModeInclude), } fakeWriter := &fakeWriter{} drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) @@ -97,7 +97,7 @@ var _ = Describe("Filtering Drain Writer", func() { It("sends the log with missing source_type when exclude filter is configured", func() { binding := syslog.Binding{ DrainData: syslog.LOGS, - LogFilter: syslog.NewLogFilter(syslog.SourceTypeSet{syslog.SOURCE_RTR: struct{}{}}, syslog.LogFilterModeExclude), + LogFilter: syslog.NewLogFilter(syslog.LogSourceTypeSet{syslog.LOG_SOURCE_RTR: struct{}{}}, syslog.LogFilterModeExclude), } fakeWriter := &fakeWriter{} drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) @@ -122,7 +122,7 @@ var _ = Describe("Filtering Drain Writer", func() { It("filters logs based on include filter - includes only APP logs", func() { binding := syslog.Binding{ DrainData: syslog.LOGS, - LogFilter: syslog.NewLogFilter(syslog.SourceTypeSet{syslog.SOURCE_APP: struct{}{}}, syslog.LogFilterModeInclude), + LogFilter: syslog.NewLogFilter(syslog.LogSourceTypeSet{syslog.LOG_SOURCE_APP: struct{}{}}, syslog.LogFilterModeInclude), } fakeWriter := &fakeWriter{} drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) @@ -161,7 +161,7 @@ var _ = Describe("Filtering Drain Writer", func() { It("filters logs based on exclude filter - excludes RTR logs", func() { binding := syslog.Binding{ DrainData: syslog.LOGS, - LogFilter: syslog.NewLogFilter(syslog.SourceTypeSet{syslog.SOURCE_RTR: struct{}{}}, syslog.LogFilterModeExclude), + LogFilter: syslog.NewLogFilter(syslog.LogSourceTypeSet{syslog.LOG_SOURCE_RTR: struct{}{}}, syslog.LogFilterModeExclude), } fakeWriter := &fakeWriter{} drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) @@ -200,7 +200,7 @@ var _ = Describe("Filtering Drain Writer", func() { It("sends logs with unknown source_type prefix when filter is set", func() { binding := syslog.Binding{ DrainData: syslog.LOGS, - LogFilter: syslog.NewLogFilter(syslog.SourceTypeSet{syslog.SOURCE_APP: struct{}{}}, syslog.LogFilterModeExclude), + LogFilter: syslog.NewLogFilter(syslog.LogSourceTypeSet{syslog.LOG_SOURCE_APP: struct{}{}}, syslog.LogFilterModeExclude), } fakeWriter := &fakeWriter{} drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) diff --git a/src/pkg/egress/syslog/source.go b/src/pkg/egress/syslog/log_source.go similarity index 60% rename from src/pkg/egress/syslog/source.go rename to src/pkg/egress/syslog/log_source.go index 27da271af..91229705b 100644 --- a/src/pkg/egress/syslog/source.go +++ b/src/pkg/egress/syslog/log_source.go @@ -2,50 +2,50 @@ package syslog import "strings" -// SourceType defines the source types used within Cloud Foundry +// LogSourceType defines the source types used within Cloud Foundry // Their order in the code is as documented in https://docs.cloudfoundry.org/devguide/deploy-apps/streaming-logs.html#format -type SourceType string +type LogSourceType string const ( - SOURCE_API SourceType = "API" - SOURCE_STG SourceType = "STG" - SOURCE_RTR SourceType = "RTR" - SOURCE_LGR SourceType = "LGR" - SOURCE_APP SourceType = "APP" - SOURCE_SSH SourceType = "SSH" - SOURCE_CELL SourceType = "CELL" - SOURCE_PROXY SourceType = "PROXY" - SOURCE_HEALTH SourceType = "HEALTH" - SOURCE_SYS SourceType = "SYS" - SOURCE_STATS SourceType = "STATS" + LOG_SOURCE_API LogSourceType = "API" + LOG_SOURCE_STG LogSourceType = "STG" + LOG_SOURCE_RTR LogSourceType = "RTR" + LOG_SOURCE_LGR LogSourceType = "LGR" + LOG_SOURCE_APP LogSourceType = "APP" + LOG_SOURCE_SSH LogSourceType = "SSH" + LOG_SOURCE_CELL LogSourceType = "CELL" + LOG_SOURCE_PROXY LogSourceType = "PROXY" + LOG_SOURCE_HEALTH LogSourceType = "HEALTH" + LOG_SOURCE_SYS LogSourceType = "SYS" + LOG_SOURCE_STATS LogSourceType = "STATS" ) // validSourceTypes contains SourceType prefixes for efficient lookup -var validSourceTypes = map[SourceType]struct{}{ - SOURCE_API: {}, - SOURCE_STG: {}, - SOURCE_RTR: {}, - SOURCE_LGR: {}, - SOURCE_APP: {}, - SOURCE_SSH: {}, - SOURCE_CELL: {}, +var validSourceTypes = map[LogSourceType]struct{}{ + LOG_SOURCE_API: {}, + LOG_SOURCE_STG: {}, + LOG_SOURCE_RTR: {}, + LOG_SOURCE_LGR: {}, + LOG_SOURCE_APP: {}, + LOG_SOURCE_SSH: {}, + LOG_SOURCE_CELL: {}, } // IsValid checks if the provided SourceType is valid -func (lt SourceType) IsValid() bool { +func (lt LogSourceType) IsValid() bool { _, ok := validSourceTypes[lt] return ok } // ParseSourceType parses a string into a SourceType value -func ParseSourceType(s string) (SourceType, bool) { - lt := SourceType(strings.ToUpper(s)) +func ParseSourceType(s string) (LogSourceType, bool) { + lt := LogSourceType(strings.ToUpper(s)) return lt, lt.IsValid() } // AllSourceTypes returns all valid source types -func AllSourceTypes() []SourceType { - types := make([]SourceType, 0, len(validSourceTypes)) +func AllSourceTypes() []LogSourceType { + types := make([]LogSourceType, 0, len(validSourceTypes)) for t := range validSourceTypes { types = append(types, t) } @@ -60,16 +60,16 @@ func ExtractPrefix(sourceTypeTag string) string { return sourceTypeTag } -// SourceTypeSet is a set of SourceTypes for efficient membership checking -type SourceTypeSet map[SourceType]struct{} +// LogSourceTypeSet is a set of SourceTypes for efficient membership checking +type LogSourceTypeSet map[LogSourceType]struct{} // Add adds a SourceType to the set -func (s SourceTypeSet) Add(lt SourceType) { +func (s LogSourceTypeSet) Add(lt LogSourceType) { s[lt] = struct{}{} } // Contains checks if the set contains a SourceType -func (s SourceTypeSet) Contains(lt SourceType) bool { +func (s LogSourceTypeSet) Contains(lt LogSourceType) bool { _, exists := s[lt] return exists } @@ -86,12 +86,12 @@ const ( // LogFilter encapsulates source type filtering configuration type LogFilter struct { - Types SourceTypeSet + Types LogSourceTypeSet Mode LogFilterMode } // NewLogFilter creates a new LogFilter with the given types and mode -func NewLogFilter(types SourceTypeSet, mode LogFilterMode) *LogFilter { +func NewLogFilter(types LogSourceTypeSet, mode LogFilterMode) *LogFilter { return &LogFilter{ Types: types, Mode: mode, @@ -110,7 +110,7 @@ func (f *LogFilter) ShouldInclude(sourceTypeTag string) bool { } prefix := ExtractPrefix(sourceTypeTag) - sourceType := SourceType(prefix) + sourceType := LogSourceType(prefix) if !sourceType.IsValid() { return f.Mode == LogFilterModeExclude } diff --git a/src/pkg/ingress/bindings/binding_config.go b/src/pkg/ingress/bindings/binding_config.go index 4db30fd65..03bd55af6 100644 --- a/src/pkg/ingress/bindings/binding_config.go +++ b/src/pkg/ingress/bindings/binding_config.go @@ -106,7 +106,7 @@ func (d *DrainParamParser) newLogFilter(sourceTypeList string, mode syslog.LogFi } sourceTypes := strings.Split(sourceTypeList, ",") - set := make(syslog.SourceTypeSet, len(sourceTypes)) + set := make(syslog.LogSourceTypeSet, len(sourceTypes)) for _, sourceType := range sourceTypes { sourceType = strings.TrimSpace(sourceType) diff --git a/src/pkg/ingress/bindings/binding_config_test.go b/src/pkg/ingress/bindings/binding_config_test.go index 93daa9250..a55417d4d 100644 --- a/src/pkg/ingress/bindings/binding_config_test.go +++ b/src/pkg/ingress/bindings/binding_config_test.go @@ -139,22 +139,22 @@ var _ = Describe("Drain Param Config", func() { { name: "include-source-types=app", url: "https://test.org/drain?include-source-types=app", - expected: NewLogFilter(syslog.LogFilterModeInclude, syslog.SOURCE_APP), + expected: NewLogFilter(syslog.LogFilterModeInclude, syslog.LOG_SOURCE_APP), }, { name: "include-source-types=app,stg,cell", url: "https://test.org/drain?include-source-types=app,stg,cell", - expected: NewLogFilter(syslog.LogFilterModeInclude, syslog.SOURCE_APP, syslog.SOURCE_STG, syslog.SOURCE_CELL), + expected: NewLogFilter(syslog.LogFilterModeInclude, syslog.LOG_SOURCE_APP, syslog.LOG_SOURCE_STG, syslog.LOG_SOURCE_CELL), }, { name: "exclude-source-types=rtr,cell,stg", url: "https://test.org/drain?exclude-source-types=rtr,cell,stg", - expected: NewLogFilter(syslog.LogFilterModeExclude, syslog.SOURCE_RTR, syslog.SOURCE_CELL, syslog.SOURCE_STG), + expected: NewLogFilter(syslog.LogFilterModeExclude, syslog.LOG_SOURCE_RTR, syslog.LOG_SOURCE_CELL, syslog.LOG_SOURCE_STG), }, { name: "exclude-source-types=rtr", url: "https://test.org/drain?exclude-source-types=rtr", - expected: NewLogFilter(syslog.LogFilterModeExclude, syslog.SOURCE_RTR), + expected: NewLogFilter(syslog.LogFilterModeExclude, syslog.LOG_SOURCE_RTR), }, } @@ -262,8 +262,8 @@ func (f *stubFetcher) DrainLimit() int { return -1 } -func NewLogFilter(mode syslog.LogFilterMode, sourceTypes ...syslog.SourceType) *syslog.LogFilter { - set := make(syslog.SourceTypeSet, len(sourceTypes)) +func NewLogFilter(mode syslog.LogFilterMode, sourceTypes ...syslog.LogSourceType) *syslog.LogFilter { + set := make(syslog.LogSourceTypeSet, len(sourceTypes)) for _, t := range sourceTypes { set[t] = struct{}{} } From 7ba8c604563c9ab03517854734c7bb56ccb321fb Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Fri, 20 Feb 2026 11:28:37 +0100 Subject: [PATCH 24/36] Remove unused method --- src/pkg/egress/syslog/log_source.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/pkg/egress/syslog/log_source.go b/src/pkg/egress/syslog/log_source.go index 91229705b..901487ab5 100644 --- a/src/pkg/egress/syslog/log_source.go +++ b/src/pkg/egress/syslog/log_source.go @@ -43,15 +43,6 @@ func ParseSourceType(s string) (LogSourceType, bool) { return lt, lt.IsValid() } -// AllSourceTypes returns all valid source types -func AllSourceTypes() []LogSourceType { - types := make([]LogSourceType, 0, len(validSourceTypes)) - for t := range validSourceTypes { - types = append(types, t) - } - return types -} - // ExtractPrefix extracts the prefix from a source_type tag (e.g., "APP/PROC/WEB/0" -> "APP") func ExtractPrefix(sourceTypeTag string) string { if idx := strings.IndexByte(sourceTypeTag, '/'); idx != -1 { From 4184c3ccdbce74eddb46b63f83ca5c85f4af7cc9 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Fri, 20 Feb 2026 11:51:36 +0100 Subject: [PATCH 25/36] Add -log to uri query param --- src/pkg/ingress/bindings/binding_config.go | 4 ++-- .../ingress/bindings/binding_config_test.go | 24 +++++++++---------- .../bindings/filtered_binding_fetcher.go | 14 +++++------ .../bindings/filtered_binding_fetcher_test.go | 14 +++++------ 4 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/pkg/ingress/bindings/binding_config.go b/src/pkg/ingress/bindings/binding_config.go index 03bd55af6..53a7eb59b 100644 --- a/src/pkg/ingress/bindings/binding_config.go +++ b/src/pkg/ingress/bindings/binding_config.go @@ -87,8 +87,8 @@ func getBindingType(u *url.URL) syslog.DrainData { } func (d *DrainParamParser) getLogFilter(u *url.URL) *syslog.LogFilter { - includeSourceTypes := u.Query().Get("include-source-types") - excludeSourceTypes := u.Query().Get("exclude-source-types") + includeSourceTypes := u.Query().Get("include-log-source-types") + excludeSourceTypes := u.Query().Get("exclude-log-source-types") if excludeSourceTypes != "" { return d.newLogFilter(excludeSourceTypes, syslog.LogFilterModeExclude) diff --git a/src/pkg/ingress/bindings/binding_config_test.go b/src/pkg/ingress/bindings/binding_config_test.go index a55417d4d..57b0b87d8 100644 --- a/src/pkg/ingress/bindings/binding_config_test.go +++ b/src/pkg/ingress/bindings/binding_config_test.go @@ -127,33 +127,33 @@ var _ = Describe("Drain Param Config", func() { expected: nil, }, { - name: "include-source-types= defaults to no filtering", - url: "https://test.org/drain?include-source-types=", + name: "include-log-source-types= defaults to no filtering", + url: "https://test.org/drain?include-log-source-types=", expected: nil, }, { - name: "exclude-source-types= defaults to no filtering", - url: "https://test.org/drain?exclude-source-types=", + name: "exclude-log-source-types= defaults to no filtering", + url: "https://test.org/drain?exclude-log-source-types=", expected: nil, }, { - name: "include-source-types=app", - url: "https://test.org/drain?include-source-types=app", + name: "include-log-source-types=app", + url: "https://test.org/drain?include-log-source-types=app", expected: NewLogFilter(syslog.LogFilterModeInclude, syslog.LOG_SOURCE_APP), }, { - name: "include-source-types=app,stg,cell", - url: "https://test.org/drain?include-source-types=app,stg,cell", + name: "include-log-source-types=app,stg,cell", + url: "https://test.org/drain?include-log-source-types=app,stg,cell", expected: NewLogFilter(syslog.LogFilterModeInclude, syslog.LOG_SOURCE_APP, syslog.LOG_SOURCE_STG, syslog.LOG_SOURCE_CELL), }, { - name: "exclude-source-types=rtr,cell,stg", - url: "https://test.org/drain?exclude-source-types=rtr,cell,stg", + name: "exclude-log-source-types=rtr,cell,stg", + url: "https://test.org/drain?exclude-log-source-types=rtr,cell,stg", expected: NewLogFilter(syslog.LogFilterModeExclude, syslog.LOG_SOURCE_RTR, syslog.LOG_SOURCE_CELL, syslog.LOG_SOURCE_STG), }, { - name: "exclude-source-types=rtr", - url: "https://test.org/drain?exclude-source-types=rtr", + name: "exclude-log-source-types=rtr", + url: "https://test.org/drain?exclude-log-source-types=rtr", expected: NewLogFilter(syslog.LogFilterModeExclude, syslog.LOG_SOURCE_RTR), }, } diff --git a/src/pkg/ingress/bindings/filtered_binding_fetcher.go b/src/pkg/ingress/bindings/filtered_binding_fetcher.go index b0ff9b1e3..3d6ae727c 100644 --- a/src/pkg/ingress/bindings/filtered_binding_fetcher.go +++ b/src/pkg/ingress/bindings/filtered_binding_fetcher.go @@ -72,7 +72,7 @@ func (f *FilteredBindingFetcher) FetchBindings() ([]syslog.Binding, error) { if invalidLogFilter(u) { invalidDrains += 1 - f.printWarning("include-source-types and exclude-source-types cannot be used at the same time in syslog drain url %s for application %s", anonymousUrl.String(), b.AppId) + f.printWarning("include-log-source-types and exclude-log-source-types cannot be used at the same time in syslog drain url %s for application %s", anonymousUrl.String(), b.AppId) continue } @@ -113,21 +113,21 @@ func (f *FilteredBindingFetcher) FetchBindings() ([]syslog.Binding, error) { } -// invalidLogFilter checks if both include-source-types and exclude-source-types +// invalidLogFilter checks if both include-log-source-types and exclude-log-source-types func invalidLogFilter(u *url.URL) bool { - includeSourceTypes := u.Query().Get("include-source-types") - excludeSourceTypes := u.Query().Get("exclude-source-types") + includeSourceTypes := u.Query().Get("include-log-source-types") + excludeSourceTypes := u.Query().Get("exclude-log-source-types") if excludeSourceTypes != "" && includeSourceTypes != "" { return true } return false } -// assumes only one of include-source-types or exclude-source-types is set +// assumes only one of include-log-source-types or exclude-log-source-types is set func getUnknownSourceTypes(u url.Values) []string { var sourceTypeList string - includeSourceTypes := u.Get("include-source-types") - excludeSourceTypes := u.Get("exclude-source-types") + includeSourceTypes := u.Get("include-log-source-types") + excludeSourceTypes := u.Get("exclude-log-source-types") if includeSourceTypes != "" { sourceTypeList = includeSourceTypes diff --git a/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go b/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go index a0cbf0056..95eb6afc3 100644 --- a/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go +++ b/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go @@ -239,7 +239,7 @@ var _ = Describe("FilteredBindingFetcher", func() { }) }) - Context("when both include-source-types and exclude-source-types are specified", func() { + Context("when both include-log-source-types and exclude-log-source-types are specified", func() { var logBuffer bytes.Buffer var warn bool var mockic *bindingsfakes.FakeIPChecker @@ -255,7 +255,7 @@ var _ = Describe("FilteredBindingFetcher", func() { JustBeforeEach(func() { input := []syslog.Binding{ - {AppId: "app-id", Hostname: "we.dont.care", Drain: syslog.Drain{Url: "https://test.org/drain?include-source-types=app&exclude-source-types=rtr"}}, + {AppId: "app-id", Hostname: "we.dont.care", Drain: syslog.Drain{Url: "https://test.org/drain?include-log-source-types=app&exclude-log-source-types=rtr"}}, } filter = bindings.NewFilteredBindingFetcher( mockic, @@ -271,7 +271,7 @@ var _ = Describe("FilteredBindingFetcher", func() { Expect(err).ToNot(HaveOccurred()) Expect(actual).To(HaveLen(0)) - Expect(logBuffer.String()).Should(MatchRegexp("include-source-types and exclude-source-types cannot be used at the same time")) + Expect(logBuffer.String()).Should(MatchRegexp("include-log-source-types and exclude-log-source-types cannot be used at the same time")) Expect(metrics.GetMetric("invalid_drains", map[string]string{"unit": "total"}).Value()).To(Equal(1.0)) }) @@ -283,7 +283,7 @@ var _ = Describe("FilteredBindingFetcher", func() { It("doesn't log the conflicting filters warning", func() { _, err := filter.FetchBindings() Expect(err).ToNot(HaveOccurred()) - Expect(logBuffer.String()).ToNot(MatchRegexp("include-source-types and exclude-source-types cannot be used at the same time")) + Expect(logBuffer.String()).ToNot(MatchRegexp("include-log-source-types and exclude-log-source-types cannot be used at the same time")) }) }) }) @@ -304,7 +304,7 @@ var _ = Describe("FilteredBindingFetcher", func() { It("logs a warning and ignores the drain in include mode", func() { input := []syslog.Binding{ - {AppId: "app-id", Hostname: "we.dont.care", Drain: syslog.Drain{Url: "https://test.org/drain?include-source-types=app,unknown,invalid,rtr"}}, + {AppId: "app-id", Hostname: "we.dont.care", Drain: syslog.Drain{Url: "https://test.org/drain?include-log-source-types=app,unknown,invalid,rtr"}}, } filter = bindings.NewFilteredBindingFetcher( mockic, @@ -326,7 +326,7 @@ var _ = Describe("FilteredBindingFetcher", func() { It("logs a warning and ignores the drain in exclude mode", func() { input := []syslog.Binding{ - {AppId: "app-id", Hostname: "we.dont.care", Drain: syslog.Drain{Url: "https://test.org/drain?exclude-source-types=rtr,unknown"}}, + {AppId: "app-id", Hostname: "we.dont.care", Drain: syslog.Drain{Url: "https://test.org/drain?exclude-log-source-types=rtr,unknown"}}, } filter = bindings.NewFilteredBindingFetcher( mockic, @@ -351,7 +351,7 @@ var _ = Describe("FilteredBindingFetcher", func() { }) It("doesn't log the warning", func() { input := []syslog.Binding{ - {AppId: "app-id", Hostname: "we.dont.care", Drain: syslog.Drain{Url: "https://test.org/drain?include-source-types=app,unknown,rtr"}}, + {AppId: "app-id", Hostname: "we.dont.care", Drain: syslog.Drain{Url: "https://test.org/drain?include-log-source-types=app,unknown,rtr"}}, } filter = bindings.NewFilteredBindingFetcher( mockic, From f1bb2a076df703080d3e9824410348096accd792 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Fri, 20 Feb 2026 13:55:29 +0100 Subject: [PATCH 26/36] No longer accept URI drains with space in values --- src/pkg/ingress/bindings/binding_config.go | 1 - .../bindings/filtered_binding_fetcher.go | 1 - .../bindings/filtered_binding_fetcher_test.go | 20 +++++++++++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/pkg/ingress/bindings/binding_config.go b/src/pkg/ingress/bindings/binding_config.go index 53a7eb59b..4f08fdd71 100644 --- a/src/pkg/ingress/bindings/binding_config.go +++ b/src/pkg/ingress/bindings/binding_config.go @@ -109,7 +109,6 @@ func (d *DrainParamParser) newLogFilter(sourceTypeList string, mode syslog.LogFi set := make(syslog.LogSourceTypeSet, len(sourceTypes)) for _, sourceType := range sourceTypes { - sourceType = strings.TrimSpace(sourceType) t, _ := syslog.ParseSourceType(sourceType) set.Add(t) } diff --git a/src/pkg/ingress/bindings/filtered_binding_fetcher.go b/src/pkg/ingress/bindings/filtered_binding_fetcher.go index 3d6ae727c..44f65dc01 100644 --- a/src/pkg/ingress/bindings/filtered_binding_fetcher.go +++ b/src/pkg/ingress/bindings/filtered_binding_fetcher.go @@ -141,7 +141,6 @@ func getUnknownSourceTypes(u url.Values) []string { var unknownTypes []string for _, sourceType := range sourceTypes { - sourceType = strings.TrimSpace(sourceType) _, ok := syslog.ParseSourceType(sourceType) if !ok { unknownTypes = append(unknownTypes, sourceType) diff --git a/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go b/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go index 95eb6afc3..2953912b7 100644 --- a/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go +++ b/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go @@ -345,6 +345,26 @@ var _ = Describe("FilteredBindingFetcher", func() { Expect(metrics.GetMetric("invalid_drains", map[string]string{"unit": "total"}).Value()).To(Equal(1.0)) }) + It("logs a warning and ignores the drain when source types have spaces", func() { + input := []syslog.Binding{ + {AppId: "app-id", Hostname: "we.dont.care", Drain: syslog.Drain{Url: "https://test.org/drain?include-log-source-types=app, rtr"}}, + } + filter = bindings.NewFilteredBindingFetcher( + mockic, + &SpyBindingReader{bindings: input}, + metrics, + warn, + log, + ) + + actual, err := filter.FetchBindings() + + Expect(err).ToNot(HaveOccurred()) + Expect(actual).To(HaveLen(0)) + Expect(logBuffer.String()).Should(MatchRegexp("Unknown source types")) + Expect(metrics.GetMetric("invalid_drains", map[string]string{"unit": "total"}).Value()).To(Equal(1.0)) + }) + Context("when configured not to warn", func() { BeforeEach(func() { warn = false From 6385a740599127a5af2ae27d7507de7e348c8d27 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Fri, 20 Feb 2026 14:31:15 +0100 Subject: [PATCH 27/36] Move log source type parsing into common function --- src/pkg/egress/syslog/log_source.go | 19 +++++++++++++++++++ src/pkg/ingress/bindings/binding_config.go | 10 +--------- .../bindings/filtered_binding_fetcher.go | 12 +----------- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/pkg/egress/syslog/log_source.go b/src/pkg/egress/syslog/log_source.go index 901487ab5..5e68cf549 100644 --- a/src/pkg/egress/syslog/log_source.go +++ b/src/pkg/egress/syslog/log_source.go @@ -43,6 +43,25 @@ func ParseSourceType(s string) (LogSourceType, bool) { return lt, lt.IsValid() } +// ParseSourceTypeList parses a comma-separated list of source types and returns +// the valid types as a set and any unknown types as a slice. +func ParseSourceTypeList(sourceTypeList string) (LogSourceTypeSet, []string) { + sourceTypes := strings.Split(sourceTypeList, ",") + set := make(LogSourceTypeSet, len(sourceTypes)) + var unknownTypes []string + + for _, sourceType := range sourceTypes { + t, ok := ParseSourceType(sourceType) + if !ok { + unknownTypes = append(unknownTypes, sourceType) + continue + } + set.Add(t) + } + + return set, unknownTypes +} + // ExtractPrefix extracts the prefix from a source_type tag (e.g., "APP/PROC/WEB/0" -> "APP") func ExtractPrefix(sourceTypeTag string) string { if idx := strings.IndexByte(sourceTypeTag, '/'); idx != -1 { diff --git a/src/pkg/ingress/bindings/binding_config.go b/src/pkg/ingress/bindings/binding_config.go index 4f08fdd71..f9f01bc3e 100644 --- a/src/pkg/ingress/bindings/binding_config.go +++ b/src/pkg/ingress/bindings/binding_config.go @@ -2,7 +2,6 @@ package bindings import ( "net/url" - "strings" "code.cloudfoundry.org/loggregator-agent-release/src/pkg/binding" "code.cloudfoundry.org/loggregator-agent-release/src/pkg/egress/syslog" @@ -105,14 +104,7 @@ func (d *DrainParamParser) newLogFilter(sourceTypeList string, mode syslog.LogFi return nil } - sourceTypes := strings.Split(sourceTypeList, ",") - set := make(syslog.LogSourceTypeSet, len(sourceTypes)) - - for _, sourceType := range sourceTypes { - t, _ := syslog.ParseSourceType(sourceType) - set.Add(t) - } - + set, _ := syslog.ParseSourceTypeList(sourceTypeList) return syslog.NewLogFilter(set, mode) } diff --git a/src/pkg/ingress/bindings/filtered_binding_fetcher.go b/src/pkg/ingress/bindings/filtered_binding_fetcher.go index 44f65dc01..4ae176042 100644 --- a/src/pkg/ingress/bindings/filtered_binding_fetcher.go +++ b/src/pkg/ingress/bindings/filtered_binding_fetcher.go @@ -137,17 +137,7 @@ func getUnknownSourceTypes(u url.Values) []string { return nil } - sourceTypes := strings.Split(sourceTypeList, ",") - var unknownTypes []string - - for _, sourceType := range sourceTypes { - _, ok := syslog.ParseSourceType(sourceType) - if !ok { - unknownTypes = append(unknownTypes, sourceType) - continue - } - } - + _, unknownTypes := syslog.ParseSourceTypeList(sourceTypeList) return unknownTypes } From ee5b2b89099c7316972f2ba4bbf6df2f8eb1c797 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Tue, 17 Mar 2026 10:17:44 +0100 Subject: [PATCH 28/36] Group tests where source_type is present --- .../syslog/filtering_drain_writer_test.go | 172 +++++++++--------- 1 file changed, 87 insertions(+), 85 deletions(-) diff --git a/src/pkg/egress/syslog/filtering_drain_writer_test.go b/src/pkg/egress/syslog/filtering_drain_writer_test.go index f73bc56f7..e8e54b33b 100644 --- a/src/pkg/egress/syslog/filtering_drain_writer_test.go +++ b/src/pkg/egress/syslog/filtering_drain_writer_test.go @@ -119,109 +119,111 @@ var _ = Describe("Filtering Drain Writer", func() { }) }) - It("filters logs based on include filter - includes only APP logs", func() { - binding := syslog.Binding{ - DrainData: syslog.LOGS, - LogFilter: syslog.NewLogFilter(syslog.LogSourceTypeSet{syslog.LOG_SOURCE_APP: struct{}{}}, syslog.LogFilterModeInclude), - } - fakeWriter := &fakeWriter{} - drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) - Expect(err).NotTo(HaveOccurred()) + Context("when source_type tag is present", func() { + It("filters logs based on include filter - includes only APP logs", func() { + binding := syslog.Binding{ + DrainData: syslog.LOGS, + LogFilter: syslog.NewLogFilter(syslog.LogSourceTypeSet{syslog.LOG_SOURCE_APP: struct{}{}}, syslog.LogFilterModeInclude), + } + fakeWriter := &fakeWriter{} + drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) + Expect(err).NotTo(HaveOccurred()) - envelopes := []*loggregator_v2.Envelope{ - { - Message: &loggregator_v2.Envelope_Log{ - Log: &loggregator_v2.Log{Payload: []byte("app log")}, + envelopes := []*loggregator_v2.Envelope{ + { + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{Payload: []byte("app log")}, + }, + Tags: map[string]string{"source_type": "APP/PROC/WEB/0"}, }, - Tags: map[string]string{"source_type": "APP/PROC/WEB/0"}, - }, - { - Message: &loggregator_v2.Envelope_Log{ - Log: &loggregator_v2.Log{Payload: []byte("rtr log")}, + { + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{Payload: []byte("rtr log")}, + }, + Tags: map[string]string{"source_type": "RTR/1"}, }, - Tags: map[string]string{"source_type": "RTR/1"}, - }, - { - Message: &loggregator_v2.Envelope_Log{ - Log: &loggregator_v2.Log{Payload: []byte("stg log")}, + { + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{Payload: []byte("stg log")}, + }, + Tags: map[string]string{"source_type": "STG/0"}, }, - Tags: map[string]string{"source_type": "STG/0"}, - }, - } + } - for _, envelope := range envelopes { - err = drainWriter.Write(envelope) - Expect(err).NotTo(HaveOccurred()) - } + for _, envelope := range envelopes { + err = drainWriter.Write(envelope) + Expect(err).NotTo(HaveOccurred()) + } - // Only APP log should be sent - Expect(fakeWriter.received).To(Equal(1)) - }) + // Only APP log should be sent + Expect(fakeWriter.received).To(Equal(1)) + }) - It("filters logs based on exclude filter - excludes RTR logs", func() { - binding := syslog.Binding{ - DrainData: syslog.LOGS, - LogFilter: syslog.NewLogFilter(syslog.LogSourceTypeSet{syslog.LOG_SOURCE_RTR: struct{}{}}, syslog.LogFilterModeExclude), - } - fakeWriter := &fakeWriter{} - drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) - Expect(err).NotTo(HaveOccurred()) + It("filters logs based on exclude filter - excludes RTR logs", func() { + binding := syslog.Binding{ + DrainData: syslog.LOGS, + LogFilter: syslog.NewLogFilter(syslog.LogSourceTypeSet{syslog.LOG_SOURCE_RTR: struct{}{}}, syslog.LogFilterModeExclude), + } + fakeWriter := &fakeWriter{} + drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) + Expect(err).NotTo(HaveOccurred()) - envelopes := []*loggregator_v2.Envelope{ - { - Message: &loggregator_v2.Envelope_Log{ - Log: &loggregator_v2.Log{Payload: []byte("app log")}, + envelopes := []*loggregator_v2.Envelope{ + { + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{Payload: []byte("app log")}, + }, + Tags: map[string]string{"source_type": "APP/PROC/WEB/0"}, }, - Tags: map[string]string{"source_type": "APP/PROC/WEB/0"}, - }, - { - Message: &loggregator_v2.Envelope_Log{ - Log: &loggregator_v2.Log{Payload: []byte("rtr log")}, + { + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{Payload: []byte("rtr log")}, + }, + Tags: map[string]string{"source_type": "RTR/1"}, }, - Tags: map[string]string{"source_type": "RTR/1"}, - }, - { - Message: &loggregator_v2.Envelope_Log{ - Log: &loggregator_v2.Log{Payload: []byte("stg log")}, + { + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{Payload: []byte("stg log")}, + }, + Tags: map[string]string{"source_type": "STG/0"}, }, - Tags: map[string]string{"source_type": "STG/0"}, - }, - } + } - for _, envelope := range envelopes { - err = drainWriter.Write(envelope) - Expect(err).NotTo(HaveOccurred()) - } + for _, envelope := range envelopes { + err = drainWriter.Write(envelope) + Expect(err).NotTo(HaveOccurred()) + } - // APP and STG logs should be sent, RTR should be filtered out - Expect(fakeWriter.received).To(Equal(2)) - }) + // APP and STG logs should be sent, RTR should be filtered out + Expect(fakeWriter.received).To(Equal(2)) + }) - It("sends logs with unknown source_type prefix when filter is set", func() { - binding := syslog.Binding{ - DrainData: syslog.LOGS, - LogFilter: syslog.NewLogFilter(syslog.LogSourceTypeSet{syslog.LOG_SOURCE_APP: struct{}{}}, syslog.LogFilterModeExclude), - } - fakeWriter := &fakeWriter{} - drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) - Expect(err).NotTo(HaveOccurred()) + It("sends logs with unknown source_type prefix when filter is set", func() { + binding := syslog.Binding{ + DrainData: syslog.LOGS, + LogFilter: syslog.NewLogFilter(syslog.LogSourceTypeSet{syslog.LOG_SOURCE_APP: struct{}{}}, syslog.LogFilterModeExclude), + } + fakeWriter := &fakeWriter{} + drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) + Expect(err).NotTo(HaveOccurred()) - envelope := &loggregator_v2.Envelope{ - Message: &loggregator_v2.Envelope_Log{ - Log: &loggregator_v2.Log{ - Payload: []byte("test log"), + envelope := &loggregator_v2.Envelope{ + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{ + Payload: []byte("test log"), + }, }, - }, - Tags: map[string]string{ - "source_type": "UNKNOWN/some/path", - }, - } + Tags: map[string]string{ + "source_type": "UNKNOWN/some/path", + }, + } - err = drainWriter.Write(envelope) + err = drainWriter.Write(envelope) - // Should send the log because unknown types default to being included for exclude filter - Expect(err).NotTo(HaveOccurred()) - Expect(fakeWriter.received).To(Equal(1)) + // Should send the log because unknown types default to being included for exclude filter + Expect(err).NotTo(HaveOccurred()) + Expect(fakeWriter.received).To(Equal(1)) + }) }) }) From efb81ed8924a341787444f176f18f526d030517f Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Thu, 26 Mar 2026 14:59:54 +0100 Subject: [PATCH 29/36] Use new package name in test --- src/pkg/ingress/bindings/filtered_binding_fetcher_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go b/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go index 2953912b7..2ecc51432 100644 --- a/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go +++ b/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go @@ -242,13 +242,13 @@ var _ = Describe("FilteredBindingFetcher", func() { Context("when both include-log-source-types and exclude-log-source-types are specified", func() { var logBuffer bytes.Buffer var warn bool - var mockic *bindingsfakes.FakeIPChecker + var mockic *bindingfakes.FakeIPChecker BeforeEach(func() { logBuffer = bytes.Buffer{} log.SetOutput(&logBuffer) warn = true - mockic = &bindingsfakes.FakeIPChecker{} + mockic = &bindingfakes.FakeIPChecker{} mockic.ResolveAddrReturns(net.ParseIP("10.10.10.10"), nil) mockic.CheckBlacklistReturns(nil) }) @@ -291,13 +291,13 @@ var _ = Describe("FilteredBindingFetcher", func() { Context("when unknown source types are provided", func() { var logBuffer bytes.Buffer var warn bool - var mockic *bindingsfakes.FakeIPChecker + var mockic *bindingfakes.FakeIPChecker BeforeEach(func() { logBuffer = bytes.Buffer{} log.SetOutput(&logBuffer) warn = true - mockic = &bindingsfakes.FakeIPChecker{} + mockic = &bindingfakes.FakeIPChecker{} mockic.ResolveAddrReturns(net.ParseIP("10.10.10.10"), nil) mockic.CheckBlacklistReturns(nil) }) From 6ff68b3341019b02eca0952b9e203eb70a4cbd92 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Thu, 26 Mar 2026 16:26:02 +0100 Subject: [PATCH 30/36] Add filtering validation to poller --- src/pkg/binding/poller.go | 59 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/src/pkg/binding/poller.go b/src/pkg/binding/poller.go index b9f13ec13..f9887d7d1 100644 --- a/src/pkg/binding/poller.go +++ b/src/pkg/binding/poller.go @@ -9,9 +9,11 @@ import ( "net" "net/http" "net/url" + "strings" "time" metrics "code.cloudfoundry.org/go-metric-registry" + "code.cloudfoundry.org/loggregator-agent-release/src/pkg/egress/syslog" v2 "code.cloudfoundry.org/loggregator-agent-release/src/pkg/ingress/v2" "code.cloudfoundry.org/loggregator-agent-release/src/pkg/simplecache" ) @@ -236,9 +238,32 @@ func (bc *bindingChecker) checkBindings(bindings []Binding) []Binding { continue } - _, exists := bc.failedHostsCache.Get(u.Host) - if exists { - bc.rejectBinding(b.Credentials, fmt.Sprintf("Skipped resolve ip address for syslog drain with url %s due to prior failure", anonymousUrl.String()), false) + if invalidLogFilter(u) { + bc.invalidDrains += 1 + if bc.warn { + for _, cred := range b.Credentials { + sendAppLogMessage( + fmt.Sprintf("include-log-source-types and exclude-log-source-types cannot be used at the same time in syslog drain url %s", anonymousUrl.String()), + cred.Apps, + bc.appLogClient, + bc.logger, + ) + } + } + continue + } + + sourceTypes := getUnknownSourceTypes(u.Query()) + if sourceTypes != nil { + bc.invalidDrains += 1 + for _, cred := range b.Credentials { + sendAppLogMessage( + fmt.Sprintf("Unknown source types '%s' in source type filter in syslog drain url %s", strings.Join(sourceTypes, ", "), anonymousUrl.String()), + cred.Apps, + bc.appLogClient, + bc.logger, + ) + } continue } @@ -308,6 +333,34 @@ func invalidScheme(scheme string) bool { return true } +// invalidLogFilter checks if both include-log-source-types and exclude-log-source-types +func invalidLogFilter(u *url.URL) bool { + includeSourceTypes := u.Query().Get("include-log-source-types") + excludeSourceTypes := u.Query().Get("exclude-log-source-types") + if excludeSourceTypes != "" && includeSourceTypes != "" { + return true + } + return false +} + +// assumes only one of include-log-source-types or exclude-log-source-types is set +func getUnknownSourceTypes(u url.Values) []string { + var sourceTypeList string + includeSourceTypes := u.Get("include-log-source-types") + excludeSourceTypes := u.Get("exclude-log-source-types") + + if includeSourceTypes != "" { + sourceTypeList = includeSourceTypes + } else if excludeSourceTypes != "" { + sourceTypeList = excludeSourceTypes + } else { + return nil + } + + _, unknownTypes := syslog.ParseSourceTypeList(sourceTypeList) + return unknownTypes +} + func CalculateBindingCount(bindings []Binding) int { apps := make(map[string]bool) for _, b := range bindings { From 97231b551e47ce674f31681503ccb6ddb7f96f41 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Thu, 26 Mar 2026 16:26:49 +0100 Subject: [PATCH 31/36] Fix cyclomatic complexity by refactoring poller --- src/pkg/binding/poller.go | 28 ++---- src/pkg/binding/poller_test.go | 165 +++++++++++++++++++++++++++++++++ 2 files changed, 173 insertions(+), 20 deletions(-) diff --git a/src/pkg/binding/poller.go b/src/pkg/binding/poller.go index f9887d7d1..19d8672fc 100644 --- a/src/pkg/binding/poller.go +++ b/src/pkg/binding/poller.go @@ -239,31 +239,19 @@ func (bc *bindingChecker) checkBindings(bindings []Binding) []Binding { } if invalidLogFilter(u) { - bc.invalidDrains += 1 - if bc.warn { - for _, cred := range b.Credentials { - sendAppLogMessage( - fmt.Sprintf("include-log-source-types and exclude-log-source-types cannot be used at the same time in syslog drain url %s", anonymousUrl.String()), - cred.Apps, - bc.appLogClient, - bc.logger, - ) - } - } + bc.rejectBinding(b.Credentials, fmt.Sprintf("include-log-source-types and exclude-log-source-types cannot be used at the same time in syslog drain url %s", anonymousUrl.String()), true) continue } sourceTypes := getUnknownSourceTypes(u.Query()) if sourceTypes != nil { - bc.invalidDrains += 1 - for _, cred := range b.Credentials { - sendAppLogMessage( - fmt.Sprintf("Unknown source types '%s' in source type filter in syslog drain url %s", strings.Join(sourceTypes, ", "), anonymousUrl.String()), - cred.Apps, - bc.appLogClient, - bc.logger, - ) - } + bc.rejectBinding(b.Credentials, fmt.Sprintf("Unknown source types '%s' in source type filter in syslog drain url %s", strings.Join(sourceTypes, ", "), anonymousUrl.String()), true) + continue + } + + _, exists := bc.failedHostsCache.Get(u.Host) + if exists { + bc.rejectBinding(b.Credentials, fmt.Sprintf("Skipped resolve ip address for syslog drain with url %s due to prior failure", anonymousUrl.String()), false) continue } diff --git a/src/pkg/binding/poller_test.go b/src/pkg/binding/poller_test.go index d60d1c654..862028ff7 100644 --- a/src/pkg/binding/poller_test.go +++ b/src/pkg/binding/poller_test.go @@ -425,6 +425,7 @@ var _ = Describe("Poller", func() { }, }, } +<<<<<<< HEAD filteredBindings := bndChecker.checkBindings(bindings) @@ -436,6 +437,54 @@ var _ = Describe("Poller", func() { }) It("returns no binding when there is a prior IP checking failure for a URL", func() { +======= + cache := simplecache.New[string, bool](120 * time.Second) + + bc := &bindingChecker{ + logStream: &appLogStream, + checker: &dummyIPChecker{}, + logger: logger, + failedHostsCache: cache, + warn: true, + } + filteredBindings := bc.checkBindings(bindings) + + Expect(filteredBindings).To(BeEmpty()) + Expect(logClient.Message()).To(ContainElement(Equal("No hostname found in syslog drain url syslog:/drain-0"))) + Expect(bc.invalidDrains).To(BeNumerically("==", 0)) + Expect(bc.blacklistedDrains).To(BeNumerically("==", 0)) + }) + + It("returns no binding which contains an invalid scheme in URL", func() { + bindings := []Binding{ + { + Url: "syslog-ssl://drain-0", + Credentials: []Credentials{ + { + Cert: "cert0", Key: "key0", CA: "ca0", Apps: []App{{Hostname: "app-hostname0", AppID: "app-id-0"}}, + }, + }, + }, + } + cache := simplecache.New[string, bool](120 * time.Second) + + bc := &bindingChecker{ + logStream: &appLogStream, + checker: &dummyIPChecker{}, + logger: logger, + failedHostsCache: cache, + warn: true, + } + filteredBindings := bc.checkBindings(bindings) + + Expect(filteredBindings).To(BeEmpty()) + Expect(logClient.Message()).To(ContainElement(Equal("Invalid Scheme for syslog drain url syslog-ssl://drain-0"))) + Expect(bc.invalidDrains).To(BeNumerically("==", 0)) + Expect(bc.blacklistedDrains).To(BeNumerically("==", 0)) + }) + + It("returns no binding with unresolvable URL", func() { +>>>>>>> 7e11944a (Fix cyclomatic complexity by refactoring poller) bindings := []Binding{ { Url: "syslog://syslog-drain-test-37c4f6db-12e2-4206-8bb2-c8d6f440d4d2.example.com", @@ -447,6 +496,7 @@ var _ = Describe("Poller", func() { }, } cache := simplecache.New[string, bool](120 * time.Second) +<<<<<<< HEAD cache.Set("syslog-drain-test-37c4f6db-12e2-4206-8bb2-c8d6f440d4d2.example.com", true) bndChecker.failedHostsCache = cache @@ -478,6 +528,25 @@ var _ = Describe("Poller", func() { Expect(logClient.Message()).To(ContainElement(Equal("Cannot resolve ip address for syslog drain with url syslog://fail_to_resolve_ip"))) Expect(bndChecker.invalidDrains).To(Equal(float64(1))) Expect(bndChecker.blacklistedDrains).To(Equal(float64(0))) +======= + blacklistRanges, _ := blacklist.NewBlacklistRanges( + blacklist.BlacklistRange{Start: "192.168.188.1", End: "192.168.188.255"}, + ) + + bc := &bindingChecker{ + logStream: &appLogStream, + checker: blacklistRanges, + logger: logger, + failedHostsCache: cache, + warn: true, + } + filteredBindings := bc.checkBindings(bindings) + + Expect(filteredBindings).To(BeEmpty()) + Expect(logClient.Message()).To(ContainElement(Equal("Cannot resolve ip address for syslog drain with url syslog://syslog-drain-test-37c4f6db-12e2-4206-8bb2-c8d6f440d4d2.example.com"))) + Expect(bc.invalidDrains).To(BeNumerically("==", 1)) + Expect(bc.blacklistedDrains).To(BeNumerically("==", 0)) +>>>>>>> 7e11944a (Fix cyclomatic complexity by refactoring poller) }) It("returns no binding which has a blacklisted IP", func() { @@ -491,6 +560,7 @@ var _ = Describe("Poller", func() { }, }, } +<<<<<<< HEAD filteredBindings := bndChecker.checkBindings(bindings) @@ -499,6 +569,65 @@ var _ = Describe("Poller", func() { Expect(logClient.Message()).To(ContainElement(Equal("Resolved ip address for syslog drain with url syslog://blacklisted_domain is blacklisted"))) Expect(bndChecker.invalidDrains).To(Equal(float64(1))) Expect(bndChecker.blacklistedDrains).To(Equal(float64(1))) +======= + cache := simplecache.New[string, bool](120 * time.Second) + blacklistRanges, _ := blacklist.NewBlacklistRanges( + blacklist.BlacklistRange{Start: "192.168.188.1", End: "192.168.188.255"}, + ) + + bc := &bindingChecker{ + logStream: &appLogStream, + checker: blacklistRanges, + logger: logger, + failedHostsCache: cache, + warn: true, + } + filteredBindings := bc.checkBindings(bindings) + + Expect(filteredBindings).To(BeEmpty()) + Expect(logClient.Message()).To(ContainElement(Equal("Resolved ip address for syslog drain with url syslog://192.168.188.15 is blacklisted"))) + Expect(bc.invalidDrains).To(BeNumerically("==", 1)) + Expect(bc.blacklistedDrains).To(BeNumerically("==", 1)) + }) + + It("returns no binding when there is a prior IP checking failure for URL", func() { + bindings := []Binding{ + { + Url: "syslog://syslog-drain-test-37c4f6db-12e2-4206-8bb2-c8d6f440d4d2.example.com", + Credentials: []Credentials{ + { + Cert: "cert0", Key: "key0", CA: "ca0", Apps: []App{{Hostname: "app-hostname0", AppID: "app-id-0"}}, + }, + }, + }, + { + Url: "syslog://syslog-drain-test-37c4f6db-12e2-4206-8bb2-c8d6f440d4d2.example.com", + Credentials: []Credentials{ + { + Cert: "cert1", Key: "key1", CA: "ca1", Apps: []App{{Hostname: "app-hostname1", AppID: "app-id-1"}}, + }, + }, + }, + } + cache := simplecache.New[string, bool](120 * time.Second) + blacklistRanges, _ := blacklist.NewBlacklistRanges( + blacklist.BlacklistRange{Start: "192.168.188.1", End: "192.168.188.255"}, + ) + + bc := &bindingChecker{ + logStream: &appLogStream, + checker: blacklistRanges, + logger: logger, + failedHostsCache: cache, + warn: true, + } + filteredBindings := bc.checkBindings(bindings) + + Expect(filteredBindings).To(BeEmpty()) + Expect(logClient.Message()).To(ContainElement(Equal("Skipped resolve ip address for syslog drain with url syslog://syslog-drain-test-37c4f6db-12e2-4206-8bb2-c8d6f440d4d2.example.com due to prior failure"))) + Expect(bc.invalidDrains).To(BeNumerically("==", 2)) + Expect(bc.blacklistedDrains).To(BeNumerically("==", 0)) +>>>>>>> 7e11944a (Fix cyclomatic complexity by refactoring poller) }) It("returns no binding when key pair cannot be loaded", func() { @@ -512,14 +641,32 @@ var _ = Describe("Poller", func() { }, }, } +<<<<<<< HEAD filteredBindings := bndChecker.checkBindings(bindings) +======= + cache := simplecache.New[string, bool](120 * time.Second) + + bc := &bindingChecker{ + logStream: &appLogStream, + checker: &dummyIPChecker{}, + logger: logger, + failedHostsCache: cache, + warn: true, + } + filteredBindings := bc.checkBindings(bindings) +>>>>>>> 7e11944a (Fix cyclomatic complexity by refactoring poller) Expect(filteredBindings).To(BeEmpty()) Expect(logBuffer).Should(gbytes.Say(("failed to load certificate for syslog-tls://drain-0 for app app-id-0"))) Expect(logClient.Message()).To(ContainElement(Equal("failed to load certificate for syslog-tls://drain-0"))) +<<<<<<< HEAD Expect(bndChecker.invalidDrains).To(Equal(float64(1))) Expect(bndChecker.blacklistedDrains).To(Equal(float64(0))) +======= + Expect(bc.invalidDrains).To(BeNumerically("==", 0)) + Expect(bc.blacklistedDrains).To(BeNumerically("==", 0)) +>>>>>>> 7e11944a (Fix cyclomatic complexity by refactoring poller) }) It("returns no binding when CA cannot be loaded", func() { @@ -533,12 +680,26 @@ var _ = Describe("Poller", func() { }, }, } +<<<<<<< HEAD filteredBindings := bndChecker.checkBindings(bindings) +======= + cache := simplecache.New[string, bool](120 * time.Second) + + bc := &bindingChecker{ + logStream: &appLogStream, + checker: &dummyIPChecker{}, + logger: logger, + failedHostsCache: cache, + warn: true, + } + filteredBindings := bc.checkBindings(bindings) +>>>>>>> 7e11944a (Fix cyclomatic complexity by refactoring poller) Expect(filteredBindings).To(BeEmpty()) Expect(logBuffer).Should(gbytes.Say(("failed to load root CA for syslog-tls://drain-0 for app app-id-0"))) Expect(logClient.Message()).To(ContainElement(Equal("failed to load root CA for syslog-tls://drain-0"))) +<<<<<<< HEAD Expect(bndChecker.invalidDrains).To(Equal(float64(1))) Expect(bndChecker.blacklistedDrains).To(Equal(float64(0))) }) @@ -562,6 +723,10 @@ var _ = Describe("Poller", func() { Expect(logClient.Message()).To(BeEmpty()) Expect(bndChecker.invalidDrains).To(Equal(float64(0))) Expect(bndChecker.blacklistedDrains).To(Equal(float64(0))) +======= + Expect(bc.invalidDrains).To(BeNumerically("==", 0)) + Expect(bc.blacklistedDrains).To(BeNumerically("==", 0)) +>>>>>>> 7e11944a (Fix cyclomatic complexity by refactoring poller) }) }) }) From c90ecf3dabc3374d05b457010b9719547f8ebd19 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Thu, 26 Mar 2026 17:29:12 +0100 Subject: [PATCH 32/36] Also move filtering tests to poller --- src/pkg/binding/poller_test.go | 284 ++++++++++++++------------------- 1 file changed, 119 insertions(+), 165 deletions(-) diff --git a/src/pkg/binding/poller_test.go b/src/pkg/binding/poller_test.go index 862028ff7..fff7ce4cb 100644 --- a/src/pkg/binding/poller_test.go +++ b/src/pkg/binding/poller_test.go @@ -425,7 +425,6 @@ var _ = Describe("Poller", func() { }, }, } -<<<<<<< HEAD filteredBindings := bndChecker.checkBindings(bindings) @@ -437,54 +436,6 @@ var _ = Describe("Poller", func() { }) It("returns no binding when there is a prior IP checking failure for a URL", func() { -======= - cache := simplecache.New[string, bool](120 * time.Second) - - bc := &bindingChecker{ - logStream: &appLogStream, - checker: &dummyIPChecker{}, - logger: logger, - failedHostsCache: cache, - warn: true, - } - filteredBindings := bc.checkBindings(bindings) - - Expect(filteredBindings).To(BeEmpty()) - Expect(logClient.Message()).To(ContainElement(Equal("No hostname found in syslog drain url syslog:/drain-0"))) - Expect(bc.invalidDrains).To(BeNumerically("==", 0)) - Expect(bc.blacklistedDrains).To(BeNumerically("==", 0)) - }) - - It("returns no binding which contains an invalid scheme in URL", func() { - bindings := []Binding{ - { - Url: "syslog-ssl://drain-0", - Credentials: []Credentials{ - { - Cert: "cert0", Key: "key0", CA: "ca0", Apps: []App{{Hostname: "app-hostname0", AppID: "app-id-0"}}, - }, - }, - }, - } - cache := simplecache.New[string, bool](120 * time.Second) - - bc := &bindingChecker{ - logStream: &appLogStream, - checker: &dummyIPChecker{}, - logger: logger, - failedHostsCache: cache, - warn: true, - } - filteredBindings := bc.checkBindings(bindings) - - Expect(filteredBindings).To(BeEmpty()) - Expect(logClient.Message()).To(ContainElement(Equal("Invalid Scheme for syslog drain url syslog-ssl://drain-0"))) - Expect(bc.invalidDrains).To(BeNumerically("==", 0)) - Expect(bc.blacklistedDrains).To(BeNumerically("==", 0)) - }) - - It("returns no binding with unresolvable URL", func() { ->>>>>>> 7e11944a (Fix cyclomatic complexity by refactoring poller) bindings := []Binding{ { Url: "syslog://syslog-drain-test-37c4f6db-12e2-4206-8bb2-c8d6f440d4d2.example.com", @@ -496,7 +447,6 @@ var _ = Describe("Poller", func() { }, } cache := simplecache.New[string, bool](120 * time.Second) -<<<<<<< HEAD cache.Set("syslog-drain-test-37c4f6db-12e2-4206-8bb2-c8d6f440d4d2.example.com", true) bndChecker.failedHostsCache = cache @@ -528,25 +478,6 @@ var _ = Describe("Poller", func() { Expect(logClient.Message()).To(ContainElement(Equal("Cannot resolve ip address for syslog drain with url syslog://fail_to_resolve_ip"))) Expect(bndChecker.invalidDrains).To(Equal(float64(1))) Expect(bndChecker.blacklistedDrains).To(Equal(float64(0))) -======= - blacklistRanges, _ := blacklist.NewBlacklistRanges( - blacklist.BlacklistRange{Start: "192.168.188.1", End: "192.168.188.255"}, - ) - - bc := &bindingChecker{ - logStream: &appLogStream, - checker: blacklistRanges, - logger: logger, - failedHostsCache: cache, - warn: true, - } - filteredBindings := bc.checkBindings(bindings) - - Expect(filteredBindings).To(BeEmpty()) - Expect(logClient.Message()).To(ContainElement(Equal("Cannot resolve ip address for syslog drain with url syslog://syslog-drain-test-37c4f6db-12e2-4206-8bb2-c8d6f440d4d2.example.com"))) - Expect(bc.invalidDrains).To(BeNumerically("==", 1)) - Expect(bc.blacklistedDrains).To(BeNumerically("==", 0)) ->>>>>>> 7e11944a (Fix cyclomatic complexity by refactoring poller) }) It("returns no binding which has a blacklisted IP", func() { @@ -560,7 +491,6 @@ var _ = Describe("Poller", func() { }, }, } -<<<<<<< HEAD filteredBindings := bndChecker.checkBindings(bindings) @@ -569,65 +499,6 @@ var _ = Describe("Poller", func() { Expect(logClient.Message()).To(ContainElement(Equal("Resolved ip address for syslog drain with url syslog://blacklisted_domain is blacklisted"))) Expect(bndChecker.invalidDrains).To(Equal(float64(1))) Expect(bndChecker.blacklistedDrains).To(Equal(float64(1))) -======= - cache := simplecache.New[string, bool](120 * time.Second) - blacklistRanges, _ := blacklist.NewBlacklistRanges( - blacklist.BlacklistRange{Start: "192.168.188.1", End: "192.168.188.255"}, - ) - - bc := &bindingChecker{ - logStream: &appLogStream, - checker: blacklistRanges, - logger: logger, - failedHostsCache: cache, - warn: true, - } - filteredBindings := bc.checkBindings(bindings) - - Expect(filteredBindings).To(BeEmpty()) - Expect(logClient.Message()).To(ContainElement(Equal("Resolved ip address for syslog drain with url syslog://192.168.188.15 is blacklisted"))) - Expect(bc.invalidDrains).To(BeNumerically("==", 1)) - Expect(bc.blacklistedDrains).To(BeNumerically("==", 1)) - }) - - It("returns no binding when there is a prior IP checking failure for URL", func() { - bindings := []Binding{ - { - Url: "syslog://syslog-drain-test-37c4f6db-12e2-4206-8bb2-c8d6f440d4d2.example.com", - Credentials: []Credentials{ - { - Cert: "cert0", Key: "key0", CA: "ca0", Apps: []App{{Hostname: "app-hostname0", AppID: "app-id-0"}}, - }, - }, - }, - { - Url: "syslog://syslog-drain-test-37c4f6db-12e2-4206-8bb2-c8d6f440d4d2.example.com", - Credentials: []Credentials{ - { - Cert: "cert1", Key: "key1", CA: "ca1", Apps: []App{{Hostname: "app-hostname1", AppID: "app-id-1"}}, - }, - }, - }, - } - cache := simplecache.New[string, bool](120 * time.Second) - blacklistRanges, _ := blacklist.NewBlacklistRanges( - blacklist.BlacklistRange{Start: "192.168.188.1", End: "192.168.188.255"}, - ) - - bc := &bindingChecker{ - logStream: &appLogStream, - checker: blacklistRanges, - logger: logger, - failedHostsCache: cache, - warn: true, - } - filteredBindings := bc.checkBindings(bindings) - - Expect(filteredBindings).To(BeEmpty()) - Expect(logClient.Message()).To(ContainElement(Equal("Skipped resolve ip address for syslog drain with url syslog://syslog-drain-test-37c4f6db-12e2-4206-8bb2-c8d6f440d4d2.example.com due to prior failure"))) - Expect(bc.invalidDrains).To(BeNumerically("==", 2)) - Expect(bc.blacklistedDrains).To(BeNumerically("==", 0)) ->>>>>>> 7e11944a (Fix cyclomatic complexity by refactoring poller) }) It("returns no binding when key pair cannot be loaded", func() { @@ -641,32 +512,14 @@ var _ = Describe("Poller", func() { }, }, } -<<<<<<< HEAD filteredBindings := bndChecker.checkBindings(bindings) -======= - cache := simplecache.New[string, bool](120 * time.Second) - - bc := &bindingChecker{ - logStream: &appLogStream, - checker: &dummyIPChecker{}, - logger: logger, - failedHostsCache: cache, - warn: true, - } - filteredBindings := bc.checkBindings(bindings) ->>>>>>> 7e11944a (Fix cyclomatic complexity by refactoring poller) Expect(filteredBindings).To(BeEmpty()) Expect(logBuffer).Should(gbytes.Say(("failed to load certificate for syslog-tls://drain-0 for app app-id-0"))) Expect(logClient.Message()).To(ContainElement(Equal("failed to load certificate for syslog-tls://drain-0"))) -<<<<<<< HEAD Expect(bndChecker.invalidDrains).To(Equal(float64(1))) Expect(bndChecker.blacklistedDrains).To(Equal(float64(0))) -======= - Expect(bc.invalidDrains).To(BeNumerically("==", 0)) - Expect(bc.blacklistedDrains).To(BeNumerically("==", 0)) ->>>>>>> 7e11944a (Fix cyclomatic complexity by refactoring poller) }) It("returns no binding when CA cannot be loaded", func() { @@ -680,26 +533,12 @@ var _ = Describe("Poller", func() { }, }, } -<<<<<<< HEAD filteredBindings := bndChecker.checkBindings(bindings) -======= - cache := simplecache.New[string, bool](120 * time.Second) - - bc := &bindingChecker{ - logStream: &appLogStream, - checker: &dummyIPChecker{}, - logger: logger, - failedHostsCache: cache, - warn: true, - } - filteredBindings := bc.checkBindings(bindings) ->>>>>>> 7e11944a (Fix cyclomatic complexity by refactoring poller) Expect(filteredBindings).To(BeEmpty()) Expect(logBuffer).Should(gbytes.Say(("failed to load root CA for syslog-tls://drain-0 for app app-id-0"))) Expect(logClient.Message()).To(ContainElement(Equal("failed to load root CA for syslog-tls://drain-0"))) -<<<<<<< HEAD Expect(bndChecker.invalidDrains).To(Equal(float64(1))) Expect(bndChecker.blacklistedDrains).To(Equal(float64(0))) }) @@ -723,10 +562,125 @@ var _ = Describe("Poller", func() { Expect(logClient.Message()).To(BeEmpty()) Expect(bndChecker.invalidDrains).To(Equal(float64(0))) Expect(bndChecker.blacklistedDrains).To(Equal(float64(0))) -======= - Expect(bc.invalidDrains).To(BeNumerically("==", 0)) - Expect(bc.blacklistedDrains).To(BeNumerically("==", 0)) ->>>>>>> 7e11944a (Fix cyclomatic complexity by refactoring poller) + }) + + Context("when both include-log-source-types and exclude-log-source-types are specified", func() { + It("ignores the drain and counts as invalid", func() { + bindings := []Binding{ + { + Url: "https://test.org/drain?include-log-source-types=app&exclude-log-source-types=rtr", + Credentials: []Credentials{ + { + Apps: []App{{Hostname: "app-hostname0", AppID: "app-id-0"}}, + }, + }, + }, + } + + filteredBindings := bndChecker.checkBindings(bindings) + + Expect(filteredBindings).To(BeEmpty()) + Expect(logClient.Message()).To(ContainElement(MatchRegexp("include-log-source-types and exclude-log-source-types cannot be used at the same time"))) + Expect(bndChecker.invalidDrains).To(BeNumerically("==", 1)) + Expect(bndChecker.blacklistedDrains).To(BeNumerically("==", 0)) + }) + + It("doesn't log the conflicting filters warning when warn is false", func() { + bndChecker.warn = false + bindings := []Binding{ + { + Url: "https://test.org/drain?include-log-source-types=app&exclude-log-source-types=rtr", + Credentials: []Credentials{ + { + Apps: []App{{Hostname: "app-hostname0", AppID: "app-id-0"}}, + }, + }, + }, + } + bndChecker.checkBindings(bindings) + + for _, msg := range logClient.Message() { + Expect(msg).ToNot(MatchRegexp("include-log-source-types and exclude-log-source-types cannot be used at the same time")) + } + }) + }) + + Context("when unknown source types are provided", func() { + It("logs a warning and ignores the drain in include mode", func() { + bindings := []Binding{ + { + Url: "https://test.org/drain?include-log-source-types=app,unknown,invalid,rtr", + Credentials: []Credentials{ + { + Apps: []App{{Hostname: "app-hostname0", AppID: "app-id-0"}}, + }, + }, + }, + } + filteredBindings := bndChecker.checkBindings(bindings) + + Expect(filteredBindings).To(BeEmpty()) + Expect(logClient.Message()).To(ContainElement(MatchRegexp("Unknown source types"))) + Expect(logClient.Message()).To(ContainElement(MatchRegexp("unknown"))) + Expect(logClient.Message()).To(ContainElement(MatchRegexp("invalid"))) + Expect(bndChecker.invalidDrains).To(BeNumerically("==", 1)) + }) + + It("logs a warning and ignores the drain in exclude mode", func() { + bindings := []Binding{ + { + Url: "https://test.org/drain?exclude-log-source-types=rtr,unknown", + Credentials: []Credentials{ + { + Apps: []App{{Hostname: "app-hostname0", AppID: "app-id-0"}}, + }, + }, + }, + } + filteredBindings := bndChecker.checkBindings(bindings) + + Expect(filteredBindings).To(BeEmpty()) + Expect(logClient.Message()).To(ContainElement(MatchRegexp("Unknown source types"))) + Expect(logClient.Message()).To(ContainElement(MatchRegexp("unknown"))) + Expect(bndChecker.invalidDrains).To(BeNumerically("==", 1)) + }) + + It("logs a warning and ignores the drain when source types have spaces", func() { + bindings := []Binding{ + { + Url: "https://test.org/drain?include-log-source-types=app, rtr", + Credentials: []Credentials{ + { + Apps: []App{{Hostname: "app-hostname0", AppID: "app-id-0"}}, + }, + }, + }, + } + filteredBindings := bndChecker.checkBindings(bindings) + + Expect(filteredBindings).To(BeEmpty()) + Expect(logClient.Message()).To(ContainElement(MatchRegexp("Unknown source types"))) + Expect(bndChecker.invalidDrains).To(BeNumerically("==", 1)) + }) + + It("doesn't log the warning when warn is false", func() { + bndChecker.warn = false + bindings := []Binding{ + { + Url: "https://test.org/drain?include-log-source-types=app,unknown,rtr", + Credentials: []Credentials{ + { + Apps: []App{{Hostname: "app-hostname0", AppID: "app-id-0"}}, + }, + }, + }, + } + bndChecker.checkBindings(bindings) + + for _, msg := range logClient.Message() { + Expect(msg).ToNot(MatchRegexp("Unknown source types")) + } + }) }) }) }) From 86b5879e6964b19ab2bc74091ef3320fe876d3fb Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Fri, 10 Jul 2026 17:22:12 +0200 Subject: [PATCH 33/36] Follow refactoring of FilterebBindingFetcher --- .../ingress/bindings/filtered_binding_fetcher_test.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go b/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go index 2ecc51432..703a3183f 100644 --- a/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go +++ b/src/pkg/ingress/bindings/filtered_binding_fetcher_test.go @@ -260,7 +260,6 @@ var _ = Describe("FilteredBindingFetcher", func() { filter = bindings.NewFilteredBindingFetcher( mockic, &SpyBindingReader{bindings: input}, - metrics, warn, log, ) @@ -272,7 +271,6 @@ var _ = Describe("FilteredBindingFetcher", func() { Expect(err).ToNot(HaveOccurred()) Expect(actual).To(HaveLen(0)) Expect(logBuffer.String()).Should(MatchRegexp("include-log-source-types and exclude-log-source-types cannot be used at the same time")) - Expect(metrics.GetMetric("invalid_drains", map[string]string{"unit": "total"}).Value()).To(Equal(1.0)) }) @@ -309,7 +307,6 @@ var _ = Describe("FilteredBindingFetcher", func() { filter = bindings.NewFilteredBindingFetcher( mockic, &SpyBindingReader{bindings: input}, - metrics, warn, log, ) @@ -321,7 +318,6 @@ var _ = Describe("FilteredBindingFetcher", func() { Expect(logBuffer.String()).Should(MatchRegexp("Unknown source types")) Expect(logBuffer.String()).Should(MatchRegexp("unknown")) Expect(logBuffer.String()).Should(MatchRegexp("invalid")) - Expect(metrics.GetMetric("invalid_drains", map[string]string{"unit": "total"}).Value()).To(Equal(1.0)) }) It("logs a warning and ignores the drain in exclude mode", func() { @@ -331,7 +327,6 @@ var _ = Describe("FilteredBindingFetcher", func() { filter = bindings.NewFilteredBindingFetcher( mockic, &SpyBindingReader{bindings: input}, - metrics, warn, log, ) @@ -342,7 +337,6 @@ var _ = Describe("FilteredBindingFetcher", func() { Expect(actual).To(HaveLen(0)) Expect(logBuffer.String()).Should(MatchRegexp("Unknown source types")) Expect(logBuffer.String()).Should(MatchRegexp("unknown")) - Expect(metrics.GetMetric("invalid_drains", map[string]string{"unit": "total"}).Value()).To(Equal(1.0)) }) It("logs a warning and ignores the drain when source types have spaces", func() { @@ -352,7 +346,6 @@ var _ = Describe("FilteredBindingFetcher", func() { filter = bindings.NewFilteredBindingFetcher( mockic, &SpyBindingReader{bindings: input}, - metrics, warn, log, ) @@ -362,7 +355,6 @@ var _ = Describe("FilteredBindingFetcher", func() { Expect(err).ToNot(HaveOccurred()) Expect(actual).To(HaveLen(0)) Expect(logBuffer.String()).Should(MatchRegexp("Unknown source types")) - Expect(metrics.GetMetric("invalid_drains", map[string]string{"unit": "total"}).Value()).To(Equal(1.0)) }) Context("when configured not to warn", func() { @@ -376,7 +368,6 @@ var _ = Describe("FilteredBindingFetcher", func() { filter = bindings.NewFilteredBindingFetcher( mockic, &SpyBindingReader{bindings: input}, - metrics, warn, log, ) From af1453a7f76d5937e5d75e9d944a18d7f202c9c3 Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Fri, 17 Jul 2026 10:42:16 +0200 Subject: [PATCH 34/36] Support all --- .../egress/syslog/filtering_drain_writer.go | 12 +-- .../syslog/filtering_drain_writer_test.go | 91 +++++++++++++++++++ 2 files changed, 96 insertions(+), 7 deletions(-) diff --git a/src/pkg/egress/syslog/filtering_drain_writer.go b/src/pkg/egress/syslog/filtering_drain_writer.go index 57e7bef6b..a9a4d27b5 100644 --- a/src/pkg/egress/syslog/filtering_drain_writer.go +++ b/src/pkg/egress/syslog/filtering_drain_writer.go @@ -35,17 +35,13 @@ func NewFilteringDrainWriter(binding Binding, writer egress.Writer) (*FilteringD } func (w *FilteringDrainWriter) Write(env *loggregator_v2.Envelope) error { - if w.binding.DrainData == ALL { - return w.writer.Write(env) - } - if env.GetTimer() != nil { - if w.binding.DrainData == TRACES { + if w.binding.DrainData == TRACES || w.binding.DrainData == ALL { return w.writer.Write(env) } } if env.GetEvent() != nil { - if w.binding.DrainData == LOGS { + if w.binding.DrainData == LOGS || w.binding.DrainData == ALL { return w.writer.Write(env) } } @@ -65,7 +61,7 @@ func (w *FilteringDrainWriter) Write(env *loggregator_v2.Envelope) error { } func sendsLogs(drainData DrainData, logFilter *LogFilter, sourceTypeTag string) bool { - if drainData != LOGS && drainData != LOGS_AND_METRICS && drainData != LOGS_NO_EVENTS { + if drainData == TRACES || drainData == METRICS { return false } @@ -78,6 +74,8 @@ func sendsMetrics(drainData DrainData) bool { return true case METRICS: return true + case ALL: + return true default: return false } diff --git a/src/pkg/egress/syslog/filtering_drain_writer_test.go b/src/pkg/egress/syslog/filtering_drain_writer_test.go index e8e54b33b..d12f9d197 100644 --- a/src/pkg/egress/syslog/filtering_drain_writer_test.go +++ b/src/pkg/egress/syslog/filtering_drain_writer_test.go @@ -120,6 +120,97 @@ var _ = Describe("Filtering Drain Writer", func() { }) Context("when source_type tag is present", func() { + It("applies the include filter even when drain-data is ALL", func() { + binding := syslog.Binding{ + DrainData: syslog.ALL, + LogFilter: syslog.NewLogFilter(syslog.LogSourceTypeSet{syslog.LOG_SOURCE_APP: struct{}{}}, syslog.LogFilterModeInclude), + } + fakeWriter := &fakeWriter{} + drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) + Expect(err).NotTo(HaveOccurred()) + + envelopes := []*loggregator_v2.Envelope{ + { + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{Payload: []byte("app log")}, + }, + Tags: map[string]string{"source_type": "APP/PROC/WEB/0"}, + }, + { + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{Payload: []byte("rtr log")}, + }, + Tags: map[string]string{"source_type": "RTR/1"}, + }, + } + + for _, envelope := range envelopes { + err = drainWriter.Write(envelope) + Expect(err).NotTo(HaveOccurred()) + } + + // Only APP log should be sent; RTR must be dropped even with drain-data=all + Expect(fakeWriter.received).To(Equal(1)) + }) + + It("applies the exclude filter even when drain-data is ALL", func() { + binding := syslog.Binding{ + DrainData: syslog.ALL, + LogFilter: syslog.NewLogFilter(syslog.LogSourceTypeSet{syslog.LOG_SOURCE_RTR: struct{}{}}, syslog.LogFilterModeExclude), + } + fakeWriter := &fakeWriter{} + drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) + Expect(err).NotTo(HaveOccurred()) + + envelopes := []*loggregator_v2.Envelope{ + { + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{Payload: []byte("app log")}, + }, + Tags: map[string]string{"source_type": "APP/PROC/WEB/0"}, + }, + { + Message: &loggregator_v2.Envelope_Log{ + Log: &loggregator_v2.Log{Payload: []byte("rtr log")}, + }, + Tags: map[string]string{"source_type": "RTR/1"}, + }, + } + + for _, envelope := range envelopes { + err = drainWriter.Write(envelope) + Expect(err).NotTo(HaveOccurred()) + } + + // APP log should be sent, RTR should be filtered out even with drain-data=all + Expect(fakeWriter.received).To(Equal(1)) + }) + + It("still forwards non-log envelopes when drain-data is ALL with a log filter", func() { + binding := syslog.Binding{ + DrainData: syslog.ALL, + LogFilter: syslog.NewLogFilter(syslog.LogSourceTypeSet{syslog.LOG_SOURCE_APP: struct{}{}}, syslog.LogFilterModeInclude), + } + fakeWriter := &fakeWriter{} + drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) + Expect(err).NotTo(HaveOccurred()) + + envelopes := []*loggregator_v2.Envelope{ + {Message: &loggregator_v2.Envelope_Counter{Counter: &loggregator_v2.Counter{}}}, + {Message: &loggregator_v2.Envelope_Gauge{Gauge: &loggregator_v2.Gauge{}}}, + {Message: &loggregator_v2.Envelope_Event{Event: &loggregator_v2.Event{}}}, + {Message: &loggregator_v2.Envelope_Timer{Timer: &loggregator_v2.Timer{}}}, + } + + for _, envelope := range envelopes { + err = drainWriter.Write(envelope) + Expect(err).NotTo(HaveOccurred()) + } + + // All non-log categories still flow through under drain-data=all + Expect(fakeWriter.received).To(Equal(4)) + }) + It("filters logs based on include filter - includes only APP logs", func() { binding := syslog.Binding{ DrainData: syslog.LOGS, From 2143322110da98428a5ffc9499a0cd32b44442bc Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Fri, 17 Jul 2026 11:51:17 +0200 Subject: [PATCH 35/36] Filter envelopes by source type just like logs --- .../egress/syslog/filtering_drain_writer.go | 23 ++++++------ .../syslog/filtering_drain_writer_test.go | 37 +++++++++++++++++-- 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/src/pkg/egress/syslog/filtering_drain_writer.go b/src/pkg/egress/syslog/filtering_drain_writer.go index a9a4d27b5..236d3a3d7 100644 --- a/src/pkg/egress/syslog/filtering_drain_writer.go +++ b/src/pkg/egress/syslog/filtering_drain_writer.go @@ -41,13 +41,12 @@ func (w *FilteringDrainWriter) Write(env *loggregator_v2.Envelope) error { } } if env.GetEvent() != nil { - if w.binding.DrainData == LOGS || w.binding.DrainData == ALL { + if sendsEvents(w.binding.DrainData, w.binding.LogFilter, env.GetTags()["source_type"]) { return w.writer.Write(env) } } if env.GetLog() != nil { - sourceType := env.GetTags()["source_type"] - if sendsLogs(w.binding.DrainData, w.binding.LogFilter, sourceType) { + if sendsLogs(w.binding.DrainData, w.binding.LogFilter, env.GetTags()["source_type"]) { return w.writer.Write(env) } } @@ -60,6 +59,14 @@ func (w *FilteringDrainWriter) Write(env *loggregator_v2.Envelope) error { return nil } +func sendsEvents(drainData DrainData, logFilter *LogFilter, sourceTypeTag string) bool { + if drainData != LOGS && drainData != ALL { + return false + } + + return logFilter.ShouldInclude(sourceTypeTag) +} + func sendsLogs(drainData DrainData, logFilter *LogFilter, sourceTypeTag string) bool { if drainData == TRACES || drainData == METRICS { return false @@ -69,14 +76,8 @@ func sendsLogs(drainData DrainData, logFilter *LogFilter, sourceTypeTag string) } func sendsMetrics(drainData DrainData) bool { - switch drainData { - case LOGS_AND_METRICS: - return true - case METRICS: - return true - case ALL: - return true - default: + if drainData != LOGS_AND_METRICS && drainData != METRICS && drainData != ALL { return false } + return true } diff --git a/src/pkg/egress/syslog/filtering_drain_writer_test.go b/src/pkg/egress/syslog/filtering_drain_writer_test.go index d12f9d197..f308f5abb 100644 --- a/src/pkg/egress/syslog/filtering_drain_writer_test.go +++ b/src/pkg/egress/syslog/filtering_drain_writer_test.go @@ -186,7 +186,7 @@ var _ = Describe("Filtering Drain Writer", func() { Expect(fakeWriter.received).To(Equal(1)) }) - It("still forwards non-log envelopes when drain-data is ALL with a log filter", func() { + It("still forwards metrics and traces when drain-data is ALL with a log filter", func() { binding := syslog.Binding{ DrainData: syslog.ALL, LogFilter: syslog.NewLogFilter(syslog.LogSourceTypeSet{syslog.LOG_SOURCE_APP: struct{}{}}, syslog.LogFilterModeInclude), @@ -198,7 +198,6 @@ var _ = Describe("Filtering Drain Writer", func() { envelopes := []*loggregator_v2.Envelope{ {Message: &loggregator_v2.Envelope_Counter{Counter: &loggregator_v2.Counter{}}}, {Message: &loggregator_v2.Envelope_Gauge{Gauge: &loggregator_v2.Gauge{}}}, - {Message: &loggregator_v2.Envelope_Event{Event: &loggregator_v2.Event{}}}, {Message: &loggregator_v2.Envelope_Timer{Timer: &loggregator_v2.Timer{}}}, } @@ -208,7 +207,39 @@ var _ = Describe("Filtering Drain Writer", func() { } // All non-log categories still flow through under drain-data=all - Expect(fakeWriter.received).To(Equal(4)) + Expect(fakeWriter.received).To(Equal(3)) + }) + + It("filters event envelopes by source type just like logs", func() { + binding := syslog.Binding{ + DrainData: syslog.ALL, + LogFilter: syslog.NewLogFilter(syslog.LogSourceTypeSet{syslog.LOG_SOURCE_APP: struct{}{}}, syslog.LogFilterModeInclude), + } + fakeWriter := &fakeWriter{} + drainWriter, err := syslog.NewFilteringDrainWriter(binding, fakeWriter) + Expect(err).NotTo(HaveOccurred()) + + appEvent := &loggregator_v2.Envelope{ + Message: &loggregator_v2.Envelope_Event{ + Event: &loggregator_v2.Event{Title: "app crash", Body: "exited"}, + }, + Tags: map[string]string{"source_type": "APP/PROC/WEB/0"}, + } + rtrEvent := &loggregator_v2.Envelope{ + Message: &loggregator_v2.Envelope_Event{ + Event: &loggregator_v2.Event{Title: "route", Body: "request"}, + }, + Tags: map[string]string{"source_type": "RTR/1"}, + } + + for _, envelope := range []*loggregator_v2.Envelope{appEvent, rtrEvent} { + err = drainWriter.Write(envelope) + Expect(err).NotTo(HaveOccurred()) + } + + // The include filter applies to events too: only the APP event passes, + // the RTR event is dropped. + Expect(fakeWriter.received).To(Equal(1)) }) It("filters logs based on include filter - includes only APP logs", func() { From a0146723252e496fe4b70dbe0435c80d8065422d Mon Sep 17 00:00:00 2001 From: Joris Baum Date: Fri, 17 Jul 2026 13:39:54 +0200 Subject: [PATCH 36/36] Clarify event forwarding via drain-type param --- src/pkg/egress/syslog/filtering_drain_writer_test.go | 3 +-- src/pkg/ingress/bindings/binding_config.go | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pkg/egress/syslog/filtering_drain_writer_test.go b/src/pkg/egress/syslog/filtering_drain_writer_test.go index f308f5abb..fc9d89011 100644 --- a/src/pkg/egress/syslog/filtering_drain_writer_test.go +++ b/src/pkg/egress/syslog/filtering_drain_writer_test.go @@ -41,7 +41,7 @@ var _ = Describe("Filtering Drain Writer", func() { Entry("traces", syslog.TRACES, false, false, false, true), Entry("all", syslog.ALL, true, true, true, true), Entry("log without events", syslog.LOGS_NO_EVENTS, true, false, false, false), - Entry("metrics and logs", syslog.LOGS_AND_METRICS, true, true, false, false), + Entry("metrics and logs without events", syslog.LOGS_AND_METRICS, true, true, false, false), ) It("errors on invalid binding type", func() { @@ -206,7 +206,6 @@ var _ = Describe("Filtering Drain Writer", func() { Expect(err).NotTo(HaveOccurred()) } - // All non-log categories still flow through under drain-data=all Expect(fakeWriter.received).To(Equal(3)) }) diff --git a/src/pkg/ingress/bindings/binding_config.go b/src/pkg/ingress/bindings/binding_config.go index f9f01bc3e..5a5b32328 100644 --- a/src/pkg/ingress/bindings/binding_config.go +++ b/src/pkg/ingress/bindings/binding_config.go @@ -58,6 +58,7 @@ func getOmitMetadata(url *url.URL, defaultDrainMetadata bool) bool { } func getBindingType(u *url.URL) syslog.DrainData { + // Legacy drain-type query param does not forward events drainData := syslog.LOGS switch u.Query().Get("drain-type") { case "logs":