-
Notifications
You must be signed in to change notification settings - Fork 168
Pass --header
enrollment option to fleet-server
#8071
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?
Conversation
This pull request does not have a backport label. Could you fix it @blakerouse? 🙏
|
Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane) |
|
💚 Build Succeeded
cc @blakerouse |
kind: enhancement | ||
|
||
# Change summary; a 80ish characters long description of the change. | ||
summary: Add --header to enrollment communication with 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.
summary: Add --header to enrollment communication with Fleet Server | |
summary: Use --header from enrollment when communicating with Fleet Server |
The current version doesn't quite read correctly as a sentence to me.
Path string `config:"path" yaml:"path,omitempty"` | ||
Host string `config:"host" yaml:"host,omitempty"` | ||
Hosts []string `config:"hosts" yaml:"hosts,omitempty"` | ||
Headers map[string]string `config:"headers" yaml:"headers,omitempty"` |
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.
If the headers are secrets, is this going to leak them in diagnostics? Are they either already redacted or not included in diagnostics at all?
func(t *testing.T) *http.ServeMux { | ||
mux := http.NewServeMux() | ||
mux.HandleFunc("/api/fleet/agents/enroll", func(w http.ResponseWriter, r *http.Request) { | ||
if r.Header.Get("X-Test-Header") != "Test-Value" { |
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.
This proves we use a header for /enroll
, is this enough to also prove the same header would be included in a checkin?
I see in enroll.go we explicitly add the header to enrollCmdOption
so it isn't obvious just by reading it. Regardless, would there be a way to test that?
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.
Changes look good to me. Once the ci is green and Craig's comments are addressed I can approve
What does this PR do?
Currently the
--header
enrollment flag is used when bootstrapping Fleet Server. Those headers should also be used when communicating with Fleet Server that is not being bootstrapped.Why is it important?
When enrolling to a Fleet Server that is behind a proxy that requires specific headers for communication it is not possible to enroll the Elastic Agent into the Fleet Server.
Checklist
[ ] I have made corresponding change to the default configuration files./changelog/fragments
using the changelog tool[ ] I have added an integration test or an E2E test(covered well in unit tests)Disruptive User Impact
None
How to test this PR locally
Setup a Fleet Server behind a proxy that requires a specific header for traffic to flow. Then enroll the elastic-agent with the required header:
./elastic-agent enroll --url http://proxy-url --enrollment-token ${token} --header X-Custom-Header=TEST
Related issues