Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
behavior.
- The debug stream on the log server now honours `include_traces`, so
`include_traces=False` keeps tracebacks off the log server as well.
- `ConfigLogging` now rejects a stream named more than once in
`console_streams` or `syslog_streams`. The duplicate was silently dropped
before, which hid a typo or a merge mistake in the configuration.

### Added

Expand Down
12 changes: 11 additions & 1 deletion gfmodules/logging/config.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
from pydantic import BaseModel, Field
from collections import Counter

from pydantic import BaseModel, Field, ValidationInfo, field_validator


class ConfigLogging(BaseModel):
Expand All @@ -15,3 +17,11 @@ class ConfigLogging(BaseModel):
# Empty leaves stdout silent.
console_streams: list[str] = Field(default=["app", "siem"])
syslog_streams: list[str] = Field(default=["app", "siem", "public_inspect"])

@field_validator("console_streams", "syslog_streams")
@classmethod
def _reject_duplicate_streams(cls, names: list[str], info: ValidationInfo) -> list[str]:
duplicates = [name for name, count in Counter(names).items() if count > 1]
if duplicates:
raise ValueError(f"duplicate {info.field_name} {duplicates}")
return names
2 changes: 1 addition & 1 deletion gfmodules/logging/config_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def _selected(setting: str, names: list[str], choices: tuple[str, ...]) -> list[
if unknown:
raise ValueError(f"unknown {setting} {unknown}, choose from {list(choices)}")

return list(dict.fromkeys(names))
return names


def _at_least(loglevel: str, floor: int) -> str:
Expand Down
13 changes: 13 additions & 0 deletions tests/test_config.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import pytest
from pydantic import ValidationError

from gfmodules.logging.config import ConfigLogging


Expand All @@ -19,6 +22,16 @@ def test_an_empty_console_selection_is_kept_rather_than_read_as_the_default() ->
assert ConfigLogging(console_streams=[]).console_streams == []


def test_a_console_stream_named_twice_is_rejected() -> None:
with pytest.raises(ValidationError, match=r"duplicate console_streams \['debug'\]"):
ConfigLogging(console_streams=["app", "debug", "debug"])


def test_a_syslog_stream_named_twice_is_rejected() -> None:
with pytest.raises(ValidationError, match=r"duplicate syslog_streams \['siem'\]"):
ConfigLogging(syslog_streams=["app", "siem", "siem"])


def test_accepts_an_application_supplied_configuration() -> None:
config = ConfigLogging(
syslog_path="syslog:5514",
Expand Down
11 changes: 0 additions & 11 deletions tests/test_config_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,12 +263,6 @@ def test_an_empty_selection_still_reaches_the_log_server(self) -> None:
assert console_handlers(conf) == {}
assert conf["loggers"]["app"]["handlers"] == ["syslog_app", "syslog_siem", "syslog_public_inspect"]

def test_a_stream_named_twice_still_gets_one_handler(self) -> None:
conf = build(ConfigLogging(console_streams=["debug", "debug"]))

assert list(console_handlers(conf)) == ["console_debug"]
assert conf["loggers"]["app"]["handlers"] == ["console_debug"]


class TestIncludeTraces:
def test_the_console_formatters_keep_traces_by_default(self) -> None:
Expand Down Expand Up @@ -381,11 +375,6 @@ def test_an_empty_selection_sends_nothing_to_the_log_server(self) -> None:

assert syslog_handlers(conf) == []

def test_a_stream_named_twice_still_gets_one_handler(self) -> None:
conf = build(ConfigLogging(syslog_path=SYSLOG, syslog_streams=["debug", "debug"]))

assert conf["root"]["handlers"] == ["syslog_debug"]


class TestLoggers:
def test_uvicorn_access_is_silenced_because_the_middleware_logs_access(self) -> None:
Expand Down
Loading