-
Notifications
You must be signed in to change notification settings - Fork 204
Allow agent to override fleet settings #11524
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
base: main
Are you sure you want to change the base?
Allow agent to override fleet settings #11524
Conversation
|
Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane) |
swiatekm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic looks fine to me but I don't like where it's located in the codebase. Ideally, this change should try to make reasoning about config loading easier, not harder.
swiatekm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Capabilities is a nice way to control this, good idea 👍 The capabilities.yml file has to be read from the config path, do we do permissions checks on ownership in this directory? This new capability is quite powerful so I think we want to do some extra work to make sure that the file was not created by an arbitrary user. For example if agent is root, the config path and capabilities.yml file need to be owned by root too. I don't see any on the capabilities.yml file itself: elastic-agent/internal/pkg/capabilities/capabilities.go Lines 43 to 58 in 7d6cf59
|
|
@swiatekm i'm reverting to previous implementation. whole patchers is a bit cumbersome, and need more thorough refactoring that i don't want to do in this PR. |
…ow-fleet-config-override
|
@cmacknz i added check for capabilities permissions |
|
Maybe I have code blindness at the end of the day, but I don't see permissions checks, maybe forgotten push? |
…ristas/elastic-agent into feat/allow-fleet-config-override
|
no code blindness, i lost the work |
cmacknz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Main concern is limiting security implications because this new capability is pretty powerful for a Fleet managed agent.
If we can restrict this to only working for containers that would help a lot IMO, so it's scoped to only where we need it, and we don't to worry about arbitrary users on a machine disabling the policy output and similar things via the new override capability.
blakerouse
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. With the feedback from @cmacknz and the added check for being in a container, I think this is a good change and will help a lot with agentless.
cmacknz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, we may want to log that we are ignoring the fleet override capability though.
e670978
💛 Build succeeded, but was flaky
Failed CI Steps
History
|
This pull request introduces support for a new "Fleet override" capability, allowing persisted configuration from a file to override configuration received from Fleet, but only if explicitly enabled by capabilities. The changes include the implementation of the new capability type, its integration into the configuration processing flow, and comprehensive unit tests to ensure correct behavior.
Opted for capabilities rather than FF or agent config option as it is clear this is set on purpose. It is not coming from fleet and cannot override itself.
Fleet override capability implementation and integration:
fleet_override, to the capabilities system, including parsing from YAML, internal representation, and logic to determine if Fleet override is allowed (AllowFleetOverride).capabilitiesManagerto support the newfleetOverrideCapsand wire it into the capability loading process.Configuration override logic:
applyPersistedConfigfunction, which applies persisted configuration from a file on disk if the Fleet override capability is enabled. This function is now called during configuration processing in the coordinator.osto support file operations in the coordinator.Testing:
applyPersistedConfigto verify correct merging of persisted configuration based on the Fleet override capability, including both enabled and disabled scenarios.Manual testing:
For manual testing build and enroll to fleet to a policy with monitoring disabled.
Verify http server is not running with
curl http://localhost:6774/liveness -IUpdate
elastic-agent.ymlwith following contentcurl http://localhost:6774/liveness -I