Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix file audit log rotation, fixes #2388 #2549

Merged
merged 1 commit into from
Feb 10, 2019
Merged

Fix file audit log rotation, fixes #2388 #2549

merged 1 commit into from
Feb 10, 2019

Conversation

klizhentas
Copy link
Contributor

This commit improves on-disk events log rotation:

  • Previously, log rotation was not deterministic,
    as logs could be rotated mid-day UTC, and events
    from the same day could end up in different files.

This commit makes sure that logs are rotated
on the UTC boundary of the day, and log entries
from this day are located in the appropriate file,
e.g. 2019-02-09.00:00:00.log will contain all
event logs from 2019-02-09 day.

  • To improve UX, additional symlink event.log
    is located (by default in /var/lib/teleport/events.log)
    and is pointing to the latest events log file.

Copy link
Contributor

@kontsevoy kontsevoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

if cfg.RotationPeriod == 0 {
cfg.RotationPeriod = defaults.LogRotationPeriod
}
// make sure that rotation period is a multiple of one day
rotationDays := cfg.RotationPeriod / (24 * time.Hour)
if cfg.RotationPeriod != (rotationDays * 24 * time.Hour) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to make sure that cfg.RotationPeriod is a multiple of "24 hours", you could just do:

if cfg.RotationPeriod % (24 * time.Hour) == 0 { ... }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great point!

// make sure that rotation period is a multiple of one day
rotationDays := cfg.RotationPeriod / (24 * time.Hour)
if cfg.RotationPeriod != (rotationDays * 24 * time.Hour) {
return trace.BadParameter("rotation period should be expressed in multiples of a single day, got %v instead", cfg.RotationPeriod)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this message is gonna be shown to a user, I would add an example (e.g. 24h, 48h), otherwise it might not be clear what "multiples of a single day" means.

Also, is that okay that this new restriction might cause teleport not to start after upgrade for users who have a "wrong" value in their config?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this value is not exposed in the config right now

@@ -363,6 +395,12 @@ func (l *FileLog) matchingFiles(fromUTC, toUTC time.Time) ([]eventFile, error) {
return filtered, nil
}

// parseFileTimte parses file's timestamp encoded into name
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Timte -> Time

This commit improves on-disk events log rotation:

* Previously, log rotation was not deterministic,
as logs could be rotated mid-day UTC, and events
from the same day could end up in different files.

This commit makes sure that logs are rotated
on the UTC boundary of the day, and log entries
from this day are located in the appropriate file,
e.g. 2019-02-09.00:00:00.log will contain all
event logs from 2019-02-09 day.

* To improve UX, additional symlink event.log
is located (by default in /var/lib/teleport/events.log)
and is pointing to the latest events log file.
@klizhentas klizhentas merged commit 353842e into branch/3.1 Feb 10, 2019
@klizhentas klizhentas deleted the sasha/logs branch February 10, 2019 01:55
klizhentas added a commit that referenced this pull request Feb 10, 2019
This commit improves on-disk events log rotation:

* Previously, log rotation was not deterministic,
as logs could be rotated mid-day UTC, and events
from the same day could end up in different files.

This commit makes sure that logs are rotated
on the UTC boundary of the day, and log entries
from this day are located in the appropriate file,
e.g. 2019-02-09.00:00:00.log will contain all
event logs from 2019-02-09 day.

* To improve UX, additional symlink event.log
is located (by default in /var/lib/teleport/events.log)
and is pointing to the latest events log file.
klizhentas added a commit that referenced this pull request Feb 10, 2019
This commit improves on-disk events log rotation:

* Previously, log rotation was not deterministic,
as logs could be rotated mid-day UTC, and events
from the same day could end up in different files.

This commit makes sure that logs are rotated
on the UTC boundary of the day, and log entries
from this day are located in the appropriate file,
e.g. 2019-02-09.00:00:00.log will contain all
event logs from 2019-02-09 day.

* To improve UX, additional symlink event.log
is located (by default in /var/lib/teleport/events.log)
and is pointing to the latest events log file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants