diff --git a/changelog/fragments/1784830745-journald-facility-old-journalctl.yaml b/changelog/fragments/1784830745-journald-facility-old-journalctl.yaml new file mode 100644 index 000000000000..f230a15c9ff6 --- /dev/null +++ b/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 diff --git a/filebeat/input/journald/config.go b/filebeat/input/journald/config.go index c909bde2fd64..421abcbf6f9a 100644 --- a/filebeat/input/journald/config.go +++ b/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 { diff --git a/filebeat/input/journald/config_test.go b/filebeat/input/journald/config_test.go index 9e30f92f9e35..59d9fe2ab6d8 100644 --- a/filebeat/input/journald/config_test.go +++ b/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) + } + }) + } +} diff --git a/filebeat/input/journald/input.go b/filebeat/input/journald/input.go index 31689b89b656..25b6f4edd519 100644 --- a/filebeat/input/journald/input.go +++ b/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, diff --git a/filebeat/input/journald/pkg/journalctl/reader.go b/filebeat/input/journald/pkg/journalctl/reader.go index 0277a61114f8..d7cf1ec4b72c 100644 --- a/filebeat/input/journald/pkg/journalctl/reader.go +++ b/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) diff --git a/filebeat/input/journald/pkg/journalctl/reader_test.go b/filebeat/input/journald/pkg/journalctl/reader_test.go index 8630865d0065..746867d7646b 100644 --- a/filebeat/input/journald/pkg/journalctl/reader_test.go +++ b/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