Query a remote Elasticsearch for License and features #7946

ph · 2018-08-10T20:58:51Z

Implements a License manager inside beats, as we development more
features that depends on the licensing and the capabilities of a remote
cluster we need a unique way to access that information. This commit
implements the following:

Add a License manager that can be started at the beginning of the beats
instance initialization. The manager takes a fetcher, currently we only
support Elasticsearch as the license backend but we could add support
for an Logstash endpoint that could proxy the license.

Notes:

By default when the manager is started, no license is available,
calling Get() on the manager will return a license not found.
The manager will periodically retrieve the license from the fetcher.
When an error occurs on the periodic check, the license wont be
invalidated right away but will enter a grace period, after this period
the license will be invalidated and will replaced by the OSS license.
License and capabilities and be retrieved by calling Get() or
registering a type implementing the Watcher interface.

ruflin · 2018-08-13T12:12:26Z

libbeat/licenser/elastic_fetcher.go

+// Fetch connect to the cluster and call the xpack api parses the data and return an Info object.
+func (f *ElasticFetcher) Fetch() (*License, error) {
+	status, body, err := f.client.Request("GET", xPackURL, "", params, nil)
+	if status != statusOK {


If the license is Apache, it will return a 405 here as the endpoint does not exist.

Thanks, I was not sure of the behavior in the OSS world :)

ph · 2018-08-13T12:33:00Z

A few questions ported from #7953

Open questions:
~~Should we check for expiration?~~
~~I only tested with trial and basic, is the license type for gold and platinum~~
~~Is trial always platinum license?~~
~~What does the API endpoint look like if the trial is expired?~~

ph · 2018-08-23T17:45:14Z

This code is ready for review.

ph · 2018-08-23T18:39:04Z

🤦‍♂️ forgot to run the test with the data race on.

ph · 2018-08-23T19:00:13Z

libbeat/licenser/manager.go

+}
+
+// NewWithFetcher takes a fetcher and return a license manager.
+func NewWithFetcher(fetcher Fetcher, duration time.Duration, gracePeriod time.Duration) *Manager {


I think I should pass a Setting Object here so we can define the duration, grace period and anything related to the backoff.

Are you thinking of config options that are also exposed to the user?

I don't think we need to expose any options to the end users, they could put a really large refresh delay.. which is probably not what we want.

ruflin

Could we add some integration tests with Elasticsearch search. Even if we only test if basic is enabled it will tell us very quickly in case something in the API endpoint for format changed on the ES side.
How does this handle trial licenses? I wonder if that is something we should fetch and at least report in the logs. This could make debugging easier when we see in the logs it's a trial and the user tells us it was working and then stopped ...

I didn't check all the code in detail more functionality wise so would be good if someone else has a more detailed look.

ruflin · 2018-08-24T09:26:19Z

libbeat/licenser/elastic_fetcher.go

+	}
+	s := string(b[1 : len(b)-1])
+	license, ok := licenseLookup[s]
+	if ok {


Nit: Normally we check for the error an return the error inside the if clause and the flow to the end of the method is the non error flow.

ruflin · 2018-08-24T09:26:47Z

libbeat/licenser/elastic_fetcher.go

+
+	s := string(b[1 : len(b)-1])
+	state, ok := stateLookup[s]
+	if ok {


See comment above

ruflin · 2018-08-24T09:30:33Z

libbeat/licenser/elastic_fetcher_test.go

+
+	t.Run("200 response", func(t *testing.T) {
+		h := func(w http.ResponseWriter, r *http.Request) {
+			json, err := ioutil.ReadFile("data/xpack.json")


Could we make this data/xpack-6.4.1.json (or whatever version it is) and then run a glob here? This makes it easy to add more test files for different versions in the future.

It allows us to also add files from older ES versions.

++ love it.

ruflin · 2018-08-24T09:31:26Z

libbeat/licenser/license.go

+	Monitoring monitoring `json:"monitoring"`
+	Rollup     rollup     `json:"rollup"`
+	Security   security   `json:"security"`
+	Watcher    watcher    `json:"watcher"`


Should we track in here all the features or only the ones that Beats actually uses / requires?

I've exposed the current features, only Graph isn't used ~~is the less relevant for us~~, but we can revisit that when a new feature is exposed via elasticsearch.

ruflin · 2018-08-24T09:34:44Z

libbeat/licenser/manager.go

+}
+
+// NewWithFetcher takes a fetcher and return a license manager.
+func NewWithFetcher(fetcher Fetcher, duration time.Duration, gracePeriod time.Duration) *Manager {


Are you thinking of config options that are also exposed to the user?

ruflin · 2018-08-24T09:36:58Z

libbeat/licenser/manager.go

+// Stop terminates the license manager, the go routine will be stopped and the cached license will
+// be removed and no more checks can be done on the manager.
+func (m *Manager) Stop() {
+	defer m.log.Info("license manager stopped")


Should we have a lock on stop to make sure it's not called in the middle of a worker starting for example?

This is a valid case, let me make a change.

I refactored Stop so the shutdown is more explicit and sync with the worker, this makes the behavior of stopping the manager predictable and the callbacks are calling order is more sane.

ruflin · 2018-08-24T09:37:54Z

libbeat/licenser/manager.go

+}
+
+func (m *Manager) worker() {
+	m.log.Debug("starting periodic updater")


could you mention in the log line that this is the period license check

ruflin · 2018-08-24T09:40:07Z

libbeat/licenser/manager.go

+func NewWithFetcher(fetcher Fetcher, duration time.Duration, gracePeriod time.Duration) *Manager {
+	m := &Manager{
+		fetcher:     fetcher,
+		duration:    duration,


What do you expect as a default duration?

This is a good question, not sure what would be the best timing for that.
I've looked at Logstash and they refresh every 30sec, https://github.com/elastic/logstash/blob/master/x-pack/lib/license_checker/licensed.rb#L34-L42

Maybe every minute would be a good default, WDYT?

30s sounds like a bit too much TBH. If we have 100'000 Beats that would mean 100'000 additional requests every 30s. In most cases I would not expect the license to change so I was more thinking of something like once a day.

If someone is having a trial license, we should probably check more often.

Perhaps we could do something like "check once per day" and but also check every time on reconnect. Alternative we could back off over time or make it more random?

One thing that would be nice if in the future with central management it could be triggered there that a license check should happen on all Beats.

I believe the current monitoring elasticsearch reporter check every 30 seconds, I understand having a 100k beats could increase the load on the server.

A check every few hours should be fine because I find once a day a bit too infrequent, I will think about saner defaults

monitoring only checks every 30s on init. After license is found it won't update license information. but then, for monitoring it's not a license check, but test if features+API endpoint is available in ES.

Some randomized backoff on initial check would be nice. Once license data are available, we don't need to check now. We could also cache license check results, so to reduce checks even between restarts. Maybe we can add a check to the ES output triggering a license checks once it finds the version is updated?

I will add some initial jitter at startup.

Maybe we can add a check to the ES output triggering a license checks once it finds the version is updated?

I will create a followup issue for that.

andrewkroh

Is there a need to inspect licenses from OSS Beats? If not, then perhaps this belongs be in x-pack/?

ph · 2018-08-24T15:34:06Z

Is there a need to inspect licenses from OSS Beats? If not, then perhaps this belongs be in x-pack/?

@andrewkroh It also check for features, so we can refactor the monitoring code to use the same license checker instead of using the onConnect handler from the elasticsearch client so we only have one way to check it.

But this is will be mostly used by Beatless and config management at first.

ph · 2018-08-24T15:36:01Z

Could we add some integration tests with Elasticsearch search. Even if we only test if basic is enabled it will tell us very quickly in case something in the API endpoint for format changed on the ES side.

I have added a simple integration test for the '_xpack' endpoint, this is only a sanity check.

How does this handle trial licenses? I wonder if that is something we should fetch and at least report in the logs. This could make debugging easier when we see in the logs it's a trial and the user tells us it was working and then stopped ...

Do we still have a trial license type returned? I am not sure I will take a deeper look.

ruflin · 2018-08-27T08:54:26Z

I will need this feature to be in OSS for ILM.

ph · 2018-08-27T16:55:41Z

Adding Trial response for the LicenseType

urso

Change LGTM. Would be nice to have some go based integration tests (e.g. fetcher_integration_test.go + build tag integration).

ph · 2018-08-27T18:37:18Z

@urso we have a sanity check there https://github.com/elastic/beats/pull/7946/files#diff-52042abe2d3906a09d82626f97c7ab29R1 :)

I was surprised how easy it was, it also catched one bug.

urso · 2018-08-27T18:45:14Z

libbeat/licenser/manager.go

+// Manager keeps tracks of license management, it uses a fetcher usually the ElasticFetcher to
+// retrieve a licence from a specific cluster.
+//
+// Starting the manager will starts a go routine to periodically query the license fetcher.


s/will starts/will start/

urso · 2018-08-27T18:45:34Z

libbeat/licenser/manager.go

+// retrieve a licence from a specific cluster.
+//
+// Starting the manager will starts a go routine to periodically query the license fetcher.
+// if an error occur on the fetcher we will retries until we successfully


s/will retries/will retry/

Implements a License manager inside beats, as we development more features that depends on the licensing and the capabilities of a remote cluster we need a unique way to access that information. This commit implements the following: Add a License manager that can be started at the beginning of the beats instance initialization. The manager takes a fetcher, currently we only support Elasticsearch as the license backend but we could add support for an Logstash endpoint that could proxy the license. Notes: - By default when the manager is started, no license is available, calling `Get()` on the manager will return a license not found. - The manager will periodically retrieve the license from the fetcher. - When an error occurs on the periodic check, the license wont be invalidated right away but will enter a grace period, after this period the license will be invalidated and will replaced by the OSS license. - License and capabilities and be retrieved by calling `Get()` or registering a type implementing the `Watcher` interface.

* License manager Implements a License manager inside beats, as we development more features that depends on the licensing and the capabilities of a remote cluster we need a unique way to access that information. This commit implements the following: Add a License manager that can be started at the beginning of the beats instance initialization. The manager takes a fetcher, currently we only support Elasticsearch as the license backend but we could add support for an Logstash endpoint that could proxy the license. Notes: - By default when the manager is started, no license is available, calling `Get()` on the manager will return a license not found. - The manager will periodically retrieve the license from the fetcher. - When an error occurs on the periodic check, the license wont be invalidated right away but will enter a grace period, after this period the license will be invalidated and will replaced by the OSS license. - License and capabilities and be retrieved by calling `Get()` or registering a type implementing the `Watcher` interface.

This adds support to Beats for index lifecycle management. The PR is currently work in progress. It works against a snapshot build of Elasticsearch from the ILM branch and the predefined configs in the metricbeat.yml file. *Blocked* This PR is currently blocked as the Elasticsearch ilm branch is not yet merged into master. Also the license checker in elastic#7946 needs to be merged first. *TODO* * [x] Add License Check elastic#7946 * [x] Add ES version check * [x] Finish implementation of setup command for policy and test it * [ ] Add tests * [x] Add docs * [x] Check all todos For more details see elastic#7935 To test this feature, Metricbeat can be run as following: `./metricbeat -e -E output.elasticsearch.ilm.enabled=true`. It currently also requires a version of Elasticsearch from the branch `index-lifecycle` to be running. The policy loaded looks as following: ``` { "policy": { "type": "timeseries", "phases": { "hot": { "actions": { "rollover": { "max_size": "25gb", "max_age": "30d" } } } } } } ```

* License manager Implements a License manager inside beats, as we development more features that depends on the licensing and the capabilities of a remote cluster we need a unique way to access that information. This commit implements the following: Add a License manager that can be started at the beginning of the beats instance initialization. The manager takes a fetcher, currently we only support Elasticsearch as the license backend but we could add support for an Logstash endpoint that could proxy the license. Notes: - By default when the manager is started, no license is available, calling `Get()` on the manager will return a license not found. - The manager will periodically retrieve the license from the fetcher. - When an error occurs on the periodic check, the license wont be invalidated right away but will enter a grace period, after this period the license will be invalidated and will replaced by the OSS license. - License and capabilities and be retrieved by calling `Get()` or registering a type implementing the `Watcher` interface.

ph added in progress Pull request is currently in progress. libbeat labels Aug 10, 2018

ruflin reviewed Aug 13, 2018

View reviewed changes

ruflin mentioned this pull request Aug 13, 2018

Add GetLicense to Elasticsearch client #7953

Closed

ph changed the title ~~[WIP] License checker to use with x-pack.~~ [WIP] Query a remote Elasticsearch for License and capabilities Aug 13, 2018

ph mentioned this pull request Aug 13, 2018

[Meta] Add FunctionBeat to run beat on serverless infrastructure #7880

Closed

42 tasks

webmat mentioned this pull request Aug 13, 2018

Need to load modules from x-pack directory #7524

Closed

ruflin mentioned this pull request Aug 14, 2018

Add ILM support to Beats #7963

Merged

ph force-pushed the feature/license-check branch 2 times, most recently from a867016 to e26b92c Compare August 23, 2018 17:44

ph changed the title ~~[WIP] Query a remote Elasticsearch for License and capabilities~~ Query a remote Elasticsearch for License and features Aug 23, 2018

ph added review and removed in progress Pull request is currently in progress. labels Aug 23, 2018

ph commented Aug 23, 2018

View reviewed changes

ruflin suggested changes Aug 24, 2018

View reviewed changes

andrewkroh reviewed Aug 24, 2018

View reviewed changes

ph requested a review from urso August 24, 2018 15:58

urso reviewed Aug 27, 2018

View reviewed changes

ph added 2 commits August 29, 2018 12:49

Moving file to beatless

b0d4ca4

ph force-pushed the feature/license-check branch from 1384ec8 to b0d4ca4 Compare August 29, 2018 16:50

ph changed the base branch from master to feature/beatless August 29, 2018 16:56

ph force-pushed the feature/beatless branch from 0d58e1d to c9ab9c3 Compare August 29, 2018 16:58

ph merged commit ff6073a into elastic:feature/beatless Aug 29, 2018

ph deleted the feature/license-check branch August 29, 2018 18:03

ph added the Functionbeat label Jan 8, 2019

Query a remote Elasticsearch for License and features #7946

Query a remote Elasticsearch for License and features #7946

Conversation

ph commented Aug 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ph commented Aug 13, 2018 • edited Loading

ph commented Aug 23, 2018

ph commented Aug 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ph Aug 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

urso Aug 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewkroh left a comment

Choose a reason for hiding this comment

ph commented Aug 24, 2018

ph commented Aug 24, 2018

ruflin commented Aug 27, 2018

ph commented Aug 27, 2018

urso left a comment

Choose a reason for hiding this comment

ph commented Aug 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ph commented Aug 10, 2018 •

edited

Loading

ph commented Aug 13, 2018 •

edited

Loading

ph Aug 24, 2018 •

edited

Loading

urso Aug 27, 2018 •

edited

Loading