Skip to content

Conversation

@kevinconaway
Copy link
Contributor

@kevinconaway kevinconaway commented Jan 10, 2020

The default group should be dd-agent, not root

@kevinconaway kevinconaway requested review from a team January 10, 2020 16:03
mode: 0640
owner: "root"
owner: "{{ datadog_user }}"
group: "{{ datadog_group }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be the group that changes, not the owner. The default value for datadog_group in ansible is root.

In chef the owner is root, and the group is datadog-agent.

https://github.com/DataDog/chef-datadog/blob/a3adb45dee5d821a4af860c8d20ced13ad9cc62b/recipes/system-probe.rb#L44-L45

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we need to change the group globally in the playbook or just for the system probe?

@KSerrania do you know why datadog_group defaults to root?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not really, the user and group were set 4 years ago. I found this commit: 70b3d5d which explains that the group must be root, otherwise the debian package install will fail. However, since this was 4 years ago, we should check if that's still the case. If it is not, aligning the role with our other installation methods may be a good idea.

Copy link
Contributor

Choose a reason for hiding this comment

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

We changed the group on Puppet [1] from root to dd-agent and so far we have seen no complaints. I think it should be safe to do it everywhere.

[1] https://github.com/DataDog/puppet-datadog-agent/blame/master/manifests/params.pp#L47

Copy link
Contributor

@albertvaka albertvaka left a comment

Choose a reason for hiding this comment

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

I think we should change the group to not be root instead of the owner.

@olivielpeau
Copy link
Member

FWIW, these config mgmt tools used to set the root group because that's what the agent package itself did a long time ago, before v5.12: see DataDog/dd-agent-omnibus#101 if you like archeology. So it's perfectly fine to change the group to dd-agent now.

Note that it's important for the system-probe.yaml config file to not be writeable by the dd-agent user, since it's read by a root process. I think we should change the file's mode to 0440, similar to what the agent package itself does here: https://github.com/DataDog/datadog-agent/blob/7785d5b66e3f4122f4938b876bd66b548c2ab8c9/omnibus/package-scripts/agent/postinst#L65-L69

The default group should be dd-agent, not root
@kevinconaway kevinconaway force-pushed the kevinconaway/sysprobe-cfg-owner branch from 982106c to 351843f Compare January 10, 2020 18:20
@kevinconaway kevinconaway changed the title Fix system probe ownership. Change default datadog_agent group Jan 10, 2020
@kevinconaway
Copy link
Contributor Author

I think we should change the file's mode to 0440, similar to what the agent package itself does here

In both the chef recipe and here in ansible, we have it as 0640 which should suffice since it is owned by root.

@kevinconaway kevinconaway merged commit 5f255c4 into master Jan 16, 2020
@olivielpeau
Copy link
Member

In both the chef recipe and here in ansible, we have it as 0640 which should suffice since it is owned by root.

OK, just a note then: for consistency it'd be nice to change the postinst script of the package so that it applies the same ownership and permissions to the system-probe's config files as our config mgmt tools.

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.

7 participants