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

Add a permissions option to logging.files for all beats #4428

Merged
merged 1 commit into from
Jun 6, 2017
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
1 change: 1 addition & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ https://github.com/elastic/beats/compare/v6.0.0-alpha1...master[Check the HEAD d
- Add the option to write the generated Elasticsearch mapping template into a file. {pull}4323[4323]
- Add instance_name in GCE add_cloud_metadata processor. {pull}4414[4414]
- Add `add_docker_metadata` processor. {pull}4352[4352]
- Add `logging.files` `permissions` option. {pull}4295[4295]

*Filebeat*
- Add experimental Redis slow log prospector type. {pull}4180[4180]
Expand Down
13 changes: 13 additions & 0 deletions libbeat/docs/loggingconfig.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ logging.files:
path: /var/log/mybeat
name: mybeat.log
keepfiles: 7
permissions: 0644
Copy link
Contributor

Choose a reason for hiding this comment

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

I worry slightly about how YAML interprets this number, is it sure to be octal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From the point of view of go, it's supposed to be a uint32. So it should be ok?
https://golang.org/pkg/os/#FileMode
Note I added validation to make sure the number is less than 7777, but now I realize we could do better.

Copy link
Member

Choose a reason for hiding this comment

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

The users will be required to start the value with a leading 0 in order for it to be interpreted as octal in YAML.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andrewkroh I have added some information in the doc to that effect.

------------------------------------------------------------------------------

TIP: In addition to setting logging options in the config file, you can modify
Expand Down Expand Up @@ -124,6 +125,18 @@ The number of most recent rotated log files to keep on disk. Older files are
deleted during log rotation. The default value is 7. The `keepfiles` options has to be
in the range of 2 to 1024 files.

===== files.permissions

The permission mask to apply when rotating log files. The default value is 0600. The
`permissions` options must be a valid Unix-style file permissions mask expressed
in octal notation. In Go, numbers in octal notation must start with '0'.

Examples:
* 0644: give read and write access to the file owner, and read access to all others.
* 0600: give read and write access to the file owner, and no access to all others.
* 0664: give read and write access to the file owner and members of the group
associated with the file, as well as read access to all other users.

==== Logging Format

The logging format is different for each logging type:
Expand Down
20 changes: 16 additions & 4 deletions libbeat/logp/file_rotator.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ type FileRotator struct {
Name string
RotateEveryBytes *uint64
KeepFiles *int
Permissions *uint32

current *os.File
currentSize uint64
Expand All @@ -42,7 +43,7 @@ func (rotator *FileRotator) CreateDirectory() error {

func (rotator *FileRotator) CheckIfConfigSane() error {
if len(rotator.Name) == 0 {
return fmt.Errorf("File logging requires a name for the file names")
return fmt.Errorf("file logging requires a name for the file names")
}
if rotator.KeepFiles == nil {
rotator.KeepFiles = new(int)
Expand All @@ -54,7 +55,11 @@ func (rotator *FileRotator) CheckIfConfigSane() error {
}

if *rotator.KeepFiles < 2 || *rotator.KeepFiles >= RotatorMaxFiles {
return fmt.Errorf("The number of files to keep should be between 2 and %d", RotatorMaxFiles-1)
return fmt.Errorf("the number of files to keep should be between 2 and %d", RotatorMaxFiles-1)
}

if rotator.Permissions != nil && (*rotator.Permissions > uint32(os.ModePerm)) {
return fmt.Errorf("the permissions mask %d is invalid", *rotator.Permissions)
}
return nil
}
Expand Down Expand Up @@ -134,7 +139,7 @@ func (rotator *FileRotator) Rotate() error {

if rotator.FileExists(fileNo + 1) {
// next file exists, something is strange
return fmt.Errorf("File %s exists, when rotating would overwrite it", rotator.FilePath(fileNo+1))
return fmt.Errorf("file %s exists, when rotating would overwrite it", rotator.FilePath(fileNo+1))
}

err := os.Rename(path, rotator.FilePath(fileNo+1))
Expand All @@ -145,7 +150,7 @@ func (rotator *FileRotator) Rotate() error {

// create the new file
path := rotator.FilePath(0)
current, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600)
current, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, os.FileMode(rotator.getPermissions()))
if err != nil {
return err
}
Expand All @@ -158,3 +163,10 @@ func (rotator *FileRotator) Rotate() error {

return nil
}

func (rotator *FileRotator) getPermissions() uint32 {
if rotator.Permissions == nil {
return 0600
}
return *rotator.Permissions
}
13 changes: 13 additions & 0 deletions libbeat/logp/file_rotator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,4 +160,17 @@ func TestConfigSane(t *testing.T) {
}
assert.NotNil(t, rotator.CheckIfConfigSane())

perms := uint32(0544)
rotator = FileRotator{
Name: "test2",
Permissions: &perms,
}
assert.Nil(t, rotator.CheckIfConfigSane())

perms = uint32(077777)
rotator = FileRotator{
Name: "test2",
Permissions: &perms,
}
assert.NotNil(t, rotator.CheckIfConfigSane())
}