[9.4](backport #52231) Fix journald input on journalctl older than 245 with facility filters

changelog/fragments/1784830745-journald-facility-old-journalctl.yaml

@@ -0,0 +1,9 @@
+kind: bug-fix
+summary: Fix journald input facility filters on journalctl older than 245
+description: >-
+  The journald input passed `--facility` to journalctl unconditionally, but
+  the flag only exists on journalctl >= 245, so on older systems (e.g. RHEL 8)
+  configurations with `facilities` set collected no events. Facilities are now
+  passed as SYSLOG_FACILITY matches, which are supported on every version and
+  are what `--facility` translates to internally.
+component: filebeat

filebeat/input/journald/config.go

@@ -22,16 +22,15 @@
 package journald
 
 import (
+	"fmt"
 	"sync"
 	"time"
 
-	"github.com/elastic/elastic-agent-libs/logp"
 	"github.com/elastic/go-ucfg"
 
 	"github.com/elastic/beats/v7/filebeat/input/journald/pkg/journalctl"
 	"github.com/elastic/beats/v7/filebeat/input/journald/pkg/journalfield"
 
-	"github.com/elastic/beats/v7/libbeat/common/cfgwarn"
 	"github.com/elastic/beats/v7/libbeat/reader/parser"
 )
 
@@ -102,9 +101,32 @@ type config struct {
 	JournalctlPath string `config:"journalctl_path"`
 }
 
+// Validate checks the configuration. It is called by go-ucfg when
+// the configuration is unpacked.
+func (c config) Validate() error {
+	// Facilities are passed to journalctl as SYSLOG_FACILITY=N matches,
+	// which journalctl does not range-check (unlike its `--facility` flag),
+	// so an out-of-range value would silently match nothing. The accepted
+	// range mirrors `--facility`, which allows up to LOG_FACMASK >> 3 (127)
+	// even though only 0-23 are standardized.
+	for _, facility := range c.Facilities {
+		if facility < 0 || facility > 127 {
+			return fmt.Errorf("facility %d is invalid, it must be in the range 0-127", facility)
+		}
+	}
+
+	return nil
+}
+
 // bwcIncludeMatches is a wrapper that accepts include_matches configuration
 // from 7.x to allow old config to remain compatible.
-type bwcIncludeMatches journalfield.IncludeMatches
+type bwcIncludeMatches struct {
+	journalfield.IncludeMatches
+
+	// legacyFormat records that the deprecated 7.x array format was used,
+	// so a deprecation warning can be logged once a logger is available.
+	legacyFormat bool
+}
 
 func (im *bwcIncludeMatches) Unpack(c *ucfg.Config) error {
 	// Handle 7.x config format in a backwards compatible manner. Old format:
@@ -115,16 +137,11 @@ func (im *bwcIncludeMatches) Unpack(c *ucfg.Config) error {
 			return err
 		}
 		im.Matches = append(im.Matches, matches...)
-
-		includeMatchesWarnOnce.Do(func() {
-			// TODO: use a local logger here
-			logp.NewLogger("journald").Warn(cfgwarn.Deprecate("", "Please migrate your journald input's "+
-				"include_matches config to the new more expressive format."))
-		})
+		im.legacyFormat = true
 		return nil
 	}
 
-	return c.Unpack((*journalfield.IncludeMatches)(im))
+	return c.Unpack(&im.IncludeMatches)
 }
 
 func defaultConfig() config {

filebeat/input/journald/config_test.go

@@ -61,3 +61,41 @@ include_matches:
 		verify(t, yaml)
 	})
 }
+
+func TestConfigValidateFacilities(t *testing.T) {
+	tests := []struct {
+		name    string
+		yaml    string
+		wantErr string
+	}{
+		{
+			name: "valid facilities",
+			yaml: "facilities: [0, 4, 23, 127]",
+		},
+		{
+			name:    "negative facility",
+			yaml:    "facilities: [-1]",
+			wantErr: "facility -1 is invalid",
+		},
+		{
+			name:    "facility above 127",
+			yaml:    "facilities: [128]",
+			wantErr: "facility 128 is invalid",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			c, err := conf.NewConfigWithYAML([]byte(tc.yaml), "source")
+			require.NoError(t, err)
+
+			config := defaultConfig()
+			err = c.Unpack(&config)
+			if tc.wantErr == "" {
+				assert.NoError(t, err)
+			} else {
+				assert.ErrorContains(t, err, tc.wantErr)
+			}
+		})
+	}
+}

filebeat/input/journald/input.go

@@ -31,6 +31,7 @@ import (
 	"github.com/elastic/beats/v7/filebeat/input/journald/pkg/journalfield"
 	input "github.com/elastic/beats/v7/filebeat/input/v2"
 	cursor "github.com/elastic/beats/v7/filebeat/input/v2/input-cursor"
+	"github.com/elastic/beats/v7/libbeat/common/cfgwarn"
 	"github.com/elastic/beats/v7/libbeat/feature"
 	"github.com/elastic/beats/v7/libbeat/management/status"
 	"github.com/elastic/beats/v7/libbeat/reader"
@@ -99,12 +100,19 @@ var cursorVersion = 1
 
 func (p pathSource) Name() string { return string(p) }
 
-func Configure(cfg *conf.C, _ *logp.Logger) ([]cursor.Source, cursor.Input, error) {
+func Configure(cfg *conf.C, logger *logp.Logger) ([]cursor.Source, cursor.Input, error) {
 	config := defaultConfig()
 	if err := cfg.Unpack(&config); err != nil {
 		return nil, nil, err
 	}
 
+	if config.Matches.legacyFormat {
+		includeMatchesWarnOnce.Do(func() {
+			logger.Warn(cfgwarn.Deprecate("", "Please migrate your journald input's "+
+				"include_matches config to the new more expressive format."))
+		})
+	}
+
 	paths := config.Paths
 	if len(paths) == 0 {
 		paths = []string{localSystemJournalID}
@@ -145,7 +153,7 @@ func Configure(cfg *conf.C, _ *logp.Logger) ([]cursor.Source, cursor.Input, erro
 		ID:                 config.ID,
 		Since:              config.Since,
 		Seek:               config.Seek,
-		Matches:            journalfield.IncludeMatches(config.Matches),
+		Matches:            config.Matches.IncludeMatches,
 		Units:              config.Units,
 		Transports:         config.Transports,
 		Identifiers:        config.Identifiers,

filebeat/input/journald/pkg/journalctl/reader.go

@@ -269,8 +269,12 @@ func New(
 		args = append(args, fmt.Sprintf("_TRANSPORT=%s", m))
 	}
 
+	// SYSLOG_FACILITY matches are used instead of `--facility` because the
+	// flag only exists on journalctl >= 245 and is implemented as exactly
+	// these matches. Matches on the same field are ORed by journalctl, which
+	// is the same semantics as repeated `--facility` flags.
 	for _, facility := range facilities {
-		args = append(args, "--facility", fmt.Sprintf("%d", facility))
+		args = append(args, fmt.Sprintf("SYSLOG_FACILITY=%d", facility))
 	}
 
 	supportsBootAll := journalctlSupportsBootAll(logger, newJctl)

filebeat/input/journald/pkg/journalctl/reader_test.go

@@ -28,6 +28,7 @@ import (
 	"os"
 	"path/filepath"
 	"slices"
+	"strings"
 	"sync/atomic"
 	"testing"
 	"time"
@@ -61,7 +62,7 @@ func TestEventWithNonStringData(t *testing.T) {
 				KillFunc: func() error { return nil },
 			}
 			r := Reader{
-				logger: logp.L(),
+				logger: logptest.NewTestingLogger(t, ""),
 				jctl:   &mock,
 			}
 
@@ -281,6 +282,53 @@ func TestJournalctlSupportsBootAll(t *testing.T) {
 	}
 }
 
+func TestFacilityArgs(t *testing.T) {
+	// Facilities are always passed as SYSLOG_FACILITY matches regardless of
+	// the journalctl version: `--facility` only exists on journalctl >= 245
+	// and is implemented as exactly these matches.
+	for _, version := range []int{239, 250} {
+		t.Run(fmt.Sprintf("version %d", version), func(t *testing.T) {
+			f := func(_ input.Canceler, _ *logp.Logger, s ...string) (Jctl, error) {
+				return &JctlMock{
+					NextFunc: func(canceler input.Canceler) ([]byte, error) {
+						ret := fmt.Sprintf("systemd %d (%d-test)\n+PAM +AUDIT", version, version)
+						return []byte(ret), nil
+					},
+					KillFunc: func() error { return nil },
+				}, nil
+			}
+
+			r, err := New(
+				logptest.NewTestingLogger(t, ""),
+				t.Context(),
+				nil,
+				nil,
+				nil,
+				journalfield.IncludeMatches{},
+				[]int{4, 10},
+				SeekHead,
+				"",
+				0,
+				"",
+				false,
+				f)
+			if err != nil {
+				t.Fatalf("did not expect an error when calling New: %s", err)
+			}
+
+			argsStr := strings.Join(r.args, " ")
+			for _, want := range []string{"SYSLOG_FACILITY=4", "SYSLOG_FACILITY=10"} {
+				if !strings.Contains(argsStr, want) {
+					t.Errorf("expected %q in args %q", want, argsStr)
+				}
+			}
+			if strings.Contains(argsStr, "--facility") {
+				t.Errorf("did not expect \"--facility\" in args %q", argsStr)
+			}
+		})
+	}
+}
+
 func TestHandleSeekAndCursor(t *testing.T) {
 	tests := []struct {
 		name        string