Beats enrollment subcommand#7182
Conversation
e678420 to
be92b42
Compare
libbeat/management/config.go
Outdated
There was a problem hiding this comment.
I applaud your attempt to use the official YAML file extension, but we seem to use .yml for everything.
libbeat/cmd/enroll.go
Outdated
libbeat/setup/kibana/client.go
Outdated
There was a problem hiding this comment.
Not relate to this PR but it seems Kibana becomes more then just for setup. Perhaps we should move this into libbeat/kibana/client in the future.
There was a problem hiding this comment.
I was planning to extract the kibana code to a different PR anyway, will move it 👍
libbeat/management/config.go
Outdated
There was a problem hiding this comment.
I remember we even have some reusable code for this to make sure it's the correct user.
|
Moved kibana client change to #7211 |
0bc1a3f to
aa2c7f6
Compare
|
Thank you for the reviews! I've moved the code to the proper folders, this one should be ready now |
We don't have permissions to retrieve that endpoint, so it would fail with a 401 error
CHANGELOG.asciidoc
Outdated
There was a problem hiding this comment.
It seems this was a glitch from Github, I changed the last commit and it got fixed
x-pack/libbeat/cmd/enroll.go
Outdated
| b, err := instance.NewBeat(name, "", version) | ||
|
|
||
| if err != nil { | ||
| return nil, fmt.Errorf("error initializing beat: %s", err) |
There was a problem hiding this comment.
error creating beat. To make sure the two error messages are different.
| Run: cli.RunWith(func(cmd *cobra.Command, args []string) error { | ||
| beat, err := getBeat(name, version) | ||
| kibanaURL := args[0] | ||
| enrollmentToken := args[1] |
There was a problem hiding this comment.
What happens if we don't have enough arguments?
There was a problem hiding this comment.
cobra library handles that:
X1 :: beats/x-pack/metricbeat ‹enrollment*› » go run main.go -c ../../metricbeat/metricbeat.yml enroll 1 ↵
Error: accepts 2 arg(s), received 0
Usage:
metricbeat enroll <kibana_url> <enrollment_token> [flags]
| return errors.Wrap(err, "Error while enrolling") | ||
| } | ||
|
|
||
| fmt.Println("Enrolled and ready to retrieve settings from Kibana") |
There was a problem hiding this comment.
As we know the Kibana url, we could share it here directly (without passwords)
There was a problem hiding this comment.
I would need to parse it to remove credentials, having that the user has just inputted it for the command I'm not sure this is worth it, wdyt?
There was a problem hiding this comment.
Nice to have, not a requirement :-)
| err = extractError(result) | ||
| } else { | ||
| if err = json.Unmarshal(result, message); err != nil { | ||
| return statusCode, errors.Wrap(err, "error unmarshaling response") |
There was a problem hiding this comment.
error unmarshaling Kibana response
| Message string | ||
| } | ||
| if err := json.Unmarshal(result, &kibanaResult); err != nil { | ||
| return errors.Wrap(err, "parsing kibana response") |
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| func TestEnrollValid(t *testing.T) { |
| t.Fatal(err) | ||
| } | ||
|
|
||
| assert.Equal(t, "fooo", accessToken) |
There was a problem hiding this comment.
Perhaps make fooo a variable/const
| return err | ||
| } | ||
|
|
||
| // write temporary file first |
There was a problem hiding this comment.
Don't we have this code already in libbeat for Filebeat?
There was a problem hiding this comment.
I didn't find any common code to perform this
There was a problem hiding this comment.
I think this is what @ruflin is talking about, he wrotes it :)
There was a problem hiding this comment.
I was too quick to comment here.
There was a problem hiding this comment.
the code is os specific: https://github.com/elastic/beats/blob/870fffca8ad6590b8e794cc259cc58a18d0d9af0/libbeat/common/file/helper_windows.go The new / old part we only need on Windows.
There was a problem hiding this comment.
Yep, I'm using that one, good to know it has specific implementation for windows 👍
There was a problem hiding this comment.
I was actually hoping we have a generic function for just writeData that does also the open file magic, but it seems we just have the same code in multiple places :-(
|
|
||
| // Load settings from its source file | ||
| func (c *Config) Load() error { | ||
| path := paths.Resolve(paths.Data, "management.yml") |
There was a problem hiding this comment.
How is this file exactly used? The data received from Central Management will be written into this? I think I initially got confused by it that it's called config. Interesting further down you call it settings but probably that is similar to config.
Don't have a better proposal yet but would be nice to make it clear this is automtically generated data and not meant to be edited by the user (right?).
There was a problem hiding this comment.
As you said, this file stores internal data for central management, like kibana endpoint settings or cache of retrieved configs. It's not meant to be edited by the user
There was a problem hiding this comment.
What about naming the object Management? Perhaps it's just me but when I read Config and see unpack, I'm thinking of our beat.yml file :-)
There was a problem hiding this comment.
I plan to add a Management/Manager object, as this is management.Config I think the namespace clarifies it. What about renaming it to Settings to avoid the Config term?
There was a problem hiding this comment.
It could help but let's discuss it again when you add the management/manager object.
| } | ||
|
|
||
| // Enrolled, persist state | ||
| // TODO use beat.Keystore() for access_token |
There was a problem hiding this comment.
As soon as the keystore is used, will the management.yml file become obsolete or still used for the ohter data?
There was a problem hiding this comment.
I think we would keep management.yml, as it contains structured data and state, keystore doesn't seem the best place for this as it may be too obscure?
There was a problem hiding this comment.
I would consider this file similar to Filebeat registry, it's something internal, but good to have it around for debugging / understanding what's going on
There was a problem hiding this comment.
Yes, agree we should keep it around. I wonder if we should in the future use the new "registry" writer for it, but still a different file. So all our state documents have the same structure (@urso FYI).
| assert.Equal(t, "6.3.0", request.Version) | ||
|
|
||
| fmt.Fprintf(w, `{"access_token": "fooo"}`) | ||
| })) |
There was a problem hiding this comment.
Thanks for the test @exekias I used the same pattern for the License checker! 👍
| return err | ||
| } | ||
|
|
||
| // write temporary file first |
There was a problem hiding this comment.
I was actually hoping we have a generic function for just writeData that does also the open file magic, but it seems we just have the same code in multiple places :-(
|
Should I squash and use the PR message as commit message? |
This PR implements intial enrollment to Central Management in Kibana. After running the enrollment command, beats will have a valid access token to use when retrieving configurations. To test this: - Use the following branches: - Elasticsearch: https://github.com/ycombinator/elasticsearch/tree/x-pack/management/beats - Kibana: https://github.com/elastic/kibana/tree/feature/x-pack/management/beats - Retrieve a valid enrollment token: ``` curl \ -u elastic \ -H 'kbn-xsrf: foobar' \ -H 'Content-Type: application/json' \ -X POST \ http://localhost:5601/api/beats/enrollment_tokens ``` - Use it: ``` <beat> enroll http://localhost:5601 <enrollment_token> ``` - Check agent is enrolled: ``` curl http://localhost:5601/api/beats/agents | jq ``` This is part of #7028, closes #7032
* Beats enrollment subcommand (#7182) This PR implements intial enrollment to Central Management in Kibana. After running the enrollment command, beats will have a valid access token to use when retrieving configurations. To test this: - Use the following branches: - Elasticsearch: https://github.com/ycombinator/elasticsearch/tree/x-pack/management/beats - Kibana: https://github.com/elastic/kibana/tree/feature/x-pack/management/beats - Retrieve a valid enrollment token: ``` curl \ -u elastic \ -H 'kbn-xsrf: foobar' \ -H 'Content-Type: application/json' \ -X POST \ http://localhost:5601/api/beats/enrollment_tokens ``` - Use it: ``` <beat> enroll http://localhost:5601 <enrollment_token> ``` - Check agent is enrolled: ``` curl http://localhost:5601/api/beats/agents | jq ``` This is part of #7028, closes #7032 * Add API client to retrieve configurations from CM (#8155) * Add central management service (#8263) * Add config manager initial skeleton Config manager will poll configs from Kibana and apply them locally. It must be started with the beat. In order to check the user is not trying to override configurations provided by central management, the Config Manager can check the exisitng configuration and return errors if something is wrong. * Register output for reloading (#8378) * Also send beat name when enrolling (#8380) * Refactor how configs are stored (#8379) * Refactor configs storage to avoid YAML issues * Refactor manager loop to avoid repeated code * Use beat name var when registering confs (#8435) This should make Auditbeat or any other beat based on Metricbeat have their own namespace for confs * Allow user/passwd based enrollment (#8524) * Allow user/passwd based enrollment This allows to enroll using the following workflow: ``` $ <beat> enroll http://kibana:5601 --username elastic Enter password: Enrolled and ready to retrieve settings from Kibana ``` It also allows to pass the password as an env variable: ``` PASS=... $ <beat> enroll http://kibana:5601 --username elastic --password env:PASS Enrolled and ready to retrieve settings from Kibana ``` * Fix some strings after review comments * Add changelog
* Beats enrollment subcommand (elastic#7182) This PR implements intial enrollment to Central Management in Kibana. After running the enrollment command, beats will have a valid access token to use when retrieving configurations. To test this: - Use the following branches: - Elasticsearch: https://github.com/ycombinator/elasticsearch/tree/x-pack/management/beats - Kibana: https://github.com/elastic/kibana/tree/feature/x-pack/management/beats - Retrieve a valid enrollment token: ``` curl \ -u elastic \ -H 'kbn-xsrf: foobar' \ -H 'Content-Type: application/json' \ -X POST \ http://localhost:5601/api/beats/enrollment_tokens ``` - Use it: ``` <beat> enroll http://localhost:5601 <enrollment_token> ``` - Check agent is enrolled: ``` curl http://localhost:5601/api/beats/agents | jq ``` This is part of elastic#7028, closes elastic#7032 * Add API client to retrieve configurations from CM (elastic#8155) * Add central management service (elastic#8263) * Add config manager initial skeleton Config manager will poll configs from Kibana and apply them locally. It must be started with the beat. In order to check the user is not trying to override configurations provided by central management, the Config Manager can check the exisitng configuration and return errors if something is wrong. * Register output for reloading (elastic#8378) * Also send beat name when enrolling (elastic#8380) * Refactor how configs are stored (elastic#8379) * Refactor configs storage to avoid YAML issues * Refactor manager loop to avoid repeated code * Use beat name var when registering confs (elastic#8435) This should make Auditbeat or any other beat based on Metricbeat have their own namespace for confs * Allow user/passwd based enrollment (elastic#8524) * Allow user/passwd based enrollment This allows to enroll using the following workflow: ``` $ <beat> enroll http://kibana:5601 --username elastic Enter password: Enrolled and ready to retrieve settings from Kibana ``` It also allows to pass the password as an env variable: ``` PASS=... $ <beat> enroll http://kibana:5601 --username elastic --password env:PASS Enrolled and ready to retrieve settings from Kibana ``` * Fix some strings after review comments * Add changelog (cherry picked from commit 4247bc3)
* Add Central Management feature (#8559) * Beats enrollment subcommand (#7182) This PR implements intial enrollment to Central Management in Kibana. After running the enrollment command, beats will have a valid access token to use when retrieving configurations. To test this: - Use the following branches: - Elasticsearch: https://github.com/ycombinator/elasticsearch/tree/x-pack/management/beats - Kibana: https://github.com/elastic/kibana/tree/feature/x-pack/management/beats - Retrieve a valid enrollment token: ``` curl \ -u elastic \ -H 'kbn-xsrf: foobar' \ -H 'Content-Type: application/json' \ -X POST \ http://localhost:5601/api/beats/enrollment_tokens ``` - Use it: ``` <beat> enroll http://localhost:5601 <enrollment_token> ``` - Check agent is enrolled: ``` curl http://localhost:5601/api/beats/agents | jq ``` This is part of #7028, closes #7032 * Add API client to retrieve configurations from CM (#8155) * Add central management service (#8263) * Add config manager initial skeleton Config manager will poll configs from Kibana and apply them locally. It must be started with the beat. In order to check the user is not trying to override configurations provided by central management, the Config Manager can check the exisitng configuration and return errors if something is wrong. * Register output for reloading (#8378) * Also send beat name when enrolling (#8380) * Refactor how configs are stored (#8379) * Refactor configs storage to avoid YAML issues * Refactor manager loop to avoid repeated code * Use beat name var when registering confs (#8435) This should make Auditbeat or any other beat based on Metricbeat have their own namespace for confs * Allow user/passwd based enrollment (#8524) * Allow user/passwd based enrollment This allows to enroll using the following workflow: ``` $ <beat> enroll http://kibana:5601 --username elastic Enter password: Enrolled and ready to retrieve settings from Kibana ``` It also allows to pass the password as an env variable: ``` PASS=... $ <beat> enroll http://kibana:5601 --username elastic --password env:PASS Enrolled and ready to retrieve settings from Kibana ``` * Fix some strings after review comments * Add changelog (cherry picked from commit 4247bc3) * Fix monitoring registry usage
This PR implements intial enrollment to Central Management in Kibana. After running the enrollment command, beats will have a valid access token to use when retrieving configurations. To test this: - Use the following branches: - Elasticsearch: https://github.com/ycombinator/elasticsearch/tree/x-pack/management/beats - Kibana: https://github.com/elastic/kibana/tree/feature/x-pack/management/beats - Retrieve a valid enrollment token: ``` curl \ -u elastic \ -H 'kbn-xsrf: foobar' \ -H 'Content-Type: application/json' \ -X POST \ http://localhost:5601/api/beats/enrollment_tokens ``` - Use it: ``` <beat> enroll http://localhost:5601 <enrollment_token> ``` - Check agent is enrolled: ``` curl http://localhost:5601/api/beats/agents | jq ``` This is part of elastic#7028, closes elastic#7032
This PR implements intial enrollment to Central Management in Kibana. After running the enrollment command, beats will have a valid access token to use when retrieving configurations.
To test this:
This is part of #7028, closes #7032