Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Change the license to use the new endpoint and format #15091

Merged
merged 3 commits into from
Dec 17, 2019
Merged

Change the license to use the new endpoint and format #15091

merged 3 commits into from
Dec 17, 2019

Conversation

ph
Copy link
Contributor

@ph ph commented Dec 12, 2019 •
edited by zube bot
Loading

  1. Use the _license endpoint instead of using the /_xpack
  2. Remove the features handling.
  3. Add more golden files for the different returned format.
  4. Remove unused code.

Fixes: #15090

@ph
Change the license to use the new endpoint and format
173b934 
1. Use the `_license` endpoint instead of using the `/_xpack`
2. Remove the `features` handling.
3. Add more golden files for the different returned format.
@ph
remove unused files and code
0ba98d8
@ph ph self-assigned this Dec 12, 2019
@ph ph added the in progress Pull request is currently in progress. label Dec 12, 2019
@ph ph requested a review from urso December 13, 2019 18:45
@ph ph added [zube]: In Review review and removed [zube]: In Progress in progress Pull request is currently in progress. labels Dec 13, 2019
@ph ph marked this pull request as ready for review December 13, 2019 18:45
urso
urso reviewed Dec 16, 2019
View reviewed changes
x-pack/libbeat/licenser/elastic_fetcher.go
"github.com/elastic/beats/libbeat/logp"
"github.com/elastic/beats/libbeat/outputs/elasticsearch"
)

const xPackURL = "/_xpack"
const xPackURL = "/_license"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm... would it make sense to be BC compatible and set the URL based on the ES version reported by the ES Client?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both endpoint answer on 7.x series for Elastic Licensed ES so I don't think it's necessary to have it backward compatible. Also we only added this checin in 7.x.

@urso
Copy link

urso commented Dec 16, 2019

This PR removes quite some code. What is the motivation and actual impact of removing the license manager and watcher?

@ph
Copy link
Contributor Author

ph commented Dec 17, 2019

This PR removes quite some code. What is the motivation and actual impact of removing the license manager and watcher?

Only functionbeat was actually using the Watcher/License manager and when we added Logstash back to the supported output we normalized the check to use the same between all the x-pack code which is just using a client calback. It requires one call but simplifies the logic on all the Elastic licensed beats. This move effectively made the Watcher and the Manager class unused/dead code. So I think it's better if we just remove it. If needed we can always ressurect it.

@urso urso self-assigned this Dec 17, 2019
@ph ph merged commit 556e8ff into elastic:master Dec 17, 2019
@ph ph added the needs_backport PR is waiting to be backported to other branches. label Dec 17, 2019
ph added a commit to ph/beats that referenced this pull request Dec 17, 2019
@ph
Change the license to use the new endpoint and format (elastic#15091)
dd3f974 
* Change the license to use the new endpoint and format

1. Use the `_license` endpoint instead of using the `/_xpack`
2. Remove the `features` handling.
3. Add more golden files for the different returned format.
4. remove unused files and code

(cherry picked from commit 556e8ff)
@ph ph mentioned this pull request Dec 17, 2019
Merged
@ph ph added v7.6.0 and removed needs_backport PR is waiting to be backported to other branches. labels Dec 17, 2019
ph added a commit that referenced this pull request Dec 19, 2019
@ph
Change the license to use the new endpoint and format (#15091) (#15147)
f440241 
* Change the license to use the new endpoint and format

1. Use the `_license` endpoint instead of using the `/_xpack`
2. Remove the `features` handling.
3. Add more golden files for the different returned format.
4. remove unused files and code

(cherry picked from commit 556e8ff)
@urso urso added the test-plan Add this PR to be manual test plan label Jan 15, 2020
@andresrc andresrc unassigned ph Jan 20, 2020
@andresrc andresrc assigned fearful-symmetry and unassigned urso Jan 20, 2020
@ph
Copy link
Contributor Author

ph commented Jan 24, 2020

testing license-related check is not a simple task when its not "trial" or "basic", in the linked private issues there are test servers that @bytebilly has created with credentials to be able to tested with a real server. The other way is to use the generated license in the private repository with dev artifact to run a local server. cc @fearful-symmetry

@fearful-symmetry fearful-symmetry added test-plan-ok This PR passed manual testing and removed test-plan Add this PR to be manual test plan labels Feb 6, 2020
@andresrc andresrc added the Team:Integrations Label for the Integrations team label Mar 6, 2020
@ycombinator ycombinator mentioned this pull request Apr 27, 2020
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@urso urso urso approved these changes

Assignees

@fearful-symmetry fearful-symmetry

Labels
breaking change needs testing notes review Team:Integrations Label for the Integrations team test-plan-ok This PR passed manual testing v7.6.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Beats licenser should query the _license endpoint instead of the xpack/_license.
4 participants
@ph @urso @andresrc @fearful-symmetry