-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Beats enrollment subcommand #7182
Conversation
e678420
to
be92b42
Compare
libbeat/management/config.go
Outdated
|
||
// Load settings from its source file | ||
func (c *Config) Load() error { | ||
path := paths.Resolve(paths.Data, "management.yaml") |
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 applaud your attempt to use the official YAML file extension, but we seem to use .yml
for everything.
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.
🙈
libbeat/cmd/enroll.go
Outdated
func genEnrollCmd(name, version string) *cobra.Command { | ||
keystoreCmd := cobra.Command{ | ||
Use: "enroll <kibana_url> <enrollment_token>", | ||
Short: "Enroll Kibana for Central Management", |
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.
Entroll in Kibana?
libbeat/setup/kibana/client.go
Outdated
} | ||
|
||
// NewClientWithConfig creates and returns a kibana client using the given config | ||
func NewClientWithConfig(config *Config) (*Client, error) { |
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was planning to extract the kibana code to a different PR anyway, will move it 👍
libbeat/management/config.go
Outdated
} | ||
|
||
// write temporary file first | ||
// TODO this should be owned by the beats user, ensure 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.
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
@@ -45,6 +45,8 @@ https://github.com/elastic/beats/compare/v6.4.0...master[Check the HEAD diff] | |||
*Auditbeat* | |||
|
|||
- Fixed a crash in the file_integrity module under Linux. {issue}7753[7753] | |||
- Fixed a data race in the file_integrity module. {issue}8009[8009] |
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.
Probably from rebasing?
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if we don't have enough arguments?
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we know the Kibana url, we could share it here directly (without passwords)
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 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
error unmarshaling Kibana response
Message string | ||
} | ||
if err := json.Unmarshal(result, &kibanaResult); err != nil { | ||
return errors.Wrap(err, "parsing kibana response") |
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.
nit Kibana
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
func TestEnrollValid(t *testing.T) { |
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.
Nice test 👍
t.Fatal(err) | ||
} | ||
|
||
assert.Equal(t, "fooo", accessToken) |
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.
Perhaps make fooo
a variable/const
return err | ||
} | ||
|
||
// write temporary file first |
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.
Don't we have this code already in libbeat for Filebeat?
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 didn't find any common code to perform this
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 think this is what @ruflin is talking about, he wrotes it :)
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 was too quick to comment here.
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 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I'm using that one, good to know it has specific implementation for windows 👍
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 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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