Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 audit/unenroll calls when agent runs fleet-server #6085
Fix audit/unenroll calls when agent runs fleet-server #6085
Changes from 18 commits
4fbee98
8b036d7
1bcf7f7
9950ed5
1b0f34c
d8ef38e
c349511
9da083e
3710448
7d68d68
1de9ae4
e5cff30
63110ce
79b3645
8804ab8
62bb765
933d6c8
8596b93
5913866
71c9cf5
5b3fdbf
8cc461b
60ba11c
6615db5
f01bc6a
0ad9069
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I'm not sure what's going on here; but
Client.Hosts
had the remote (cloud) fleet-server address, andClient.Host
was the internal port.I'm going to take some time tomorrow to verify that our normal agent operations still use the internal port when the agent is running fleet-server.
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.
agent does connect to
localhost:8221
; but strangely enough fleet-server does not populate theserver.address
attribute in HTTP logs; i'll open a fleet-server PR to fix thispr here: elastic/fleet-server#4142
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.
cfg.Fleet.Client.Hosts = []string{cfg.Fleet.Client.Host}
is a bit weird. This is perhaps something else that could be encapsulated into the output of an RPC call to the running agent service since agent itself must know that these represent two different things.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.
We should avoid using an RPC call because we don't want to introduce the requirement that an agent is running for the uninstall to succeed.
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.
What I think is happening here:
Host
from the enroll commandHosts
field coming from Fleet and tries to reset the other fields.I guess that the merged result of that is not quite right in this scenario and we end up with the discrepancy between
Host
andHosts
From discussion with @nchaulet at the time, it was determined that Fleet will send back only
Hosts
field while the separateHost
,Protocol
,Path
is only handled by agent when initializing config to store command line args. This refactor has been done in context of mTLS from policy support PR as an attempt to clean up the policy change handler.I filed an issue to have a look and possible remote Host, Protocol and Path fields from the agent config at the time but maybe this needs further evaluation/discussion taking into account fleet bootstrap scenarios
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.
Can we link to that issue and add this explanation to the code? This is going to be really not obvious to someone reading this code without the context of this PR.