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

Fixes license check issue with invalid license UUID and check on 32 bits system. #11649

Merged
merged 8 commits into from
Apr 4, 2019
Merged

Fixes license check issue with invalid license UUID and check on 32 bits system. #11649

merged 8 commits into from
Apr 4, 2019

Conversation

ph
Copy link
Contributor

@ph ph commented Apr 4, 2019 •
edited
Loading

  • Relax validation of the X-Pack license UID value.
  • Fix a parsing error with the X-Pack license check on 32-bit system.

Fixes: #11640
Fixes: #11650

ph added 2 commits April 4, 2019 08:52
@ph
Uses a string for License.UUID instead of uuid.UUID
0fe3b39 
There are some case where the license UUID is a valid license but not a
valid UUID.

Fixes: elastic#11640
@ph
changelog
7788eff
@ph ph added review libbeat needs_backport PR is waiting to be backported to other branches. labels Apr 4, 2019
@ph ph requested a review from tsg April 4, 2019 12:55
@ph
Copy link
Contributor Author

ph commented Apr 4, 2019

Note: Need backport to 6.7 7.0 and 7.x

andrewkroh
andrewkroh reviewed Apr 4, 2019
View reviewed changes
x-pack/libbeat/licenser/license.go Show resolved Hide resolved
CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved
@ruflin
Copy link
Contributor

ruflin commented Apr 4, 2019

As a follow up we should add some (fake) license ID's which are not UUID to the testing.

@ph ph force-pushed the fix/relax-license-uuid branch from 078f97f to cdab676 Compare April 4, 2019 14:54
@ph ph changed the title Uses a string for License.UUID instead of uuid.UUID Fixes license check issue with invalid license UUID and check on 32 bits system. Apr 4, 2019
@ph ph requested a review from ruflin April 4, 2019 14:57
@ph
Copy link
Contributor Author

ph commented Apr 4, 2019

@ruflin it also include a test case for the relax uuid.

andrewkroh
andrewkroh reviewed Apr 4, 2019
View reviewed changes
CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved
Vagrantfile
@@ -141,7 +143,17 @@ Vagrant.configure(2) do |config|
c.vm.network :forwarded_port, guest: 22, host: 2226, id: "ssh", auto_correct: true

c.vm.provision "shell", inline: $unixProvision, privileged: false
c.vm.provision "shell", inline: $linuxGvmProvision, privileged: false
c.vm.provision "shell", inline: linuxGvmProvision, privileged: false
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be linuxGvmProvision()? (Not my area of expertise.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because I've added a new optional parameter to define the architecture, GVM doesn't current use i386 and I didn't want to add conditionals when we provision the machine on the bash scripts to correctly map the artifact to machine instead I just use the paremeters to correctly set the downloadable artifact.

Copy link
Contributor Author

@ph ph Apr 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Just to clarify the above, () are not mandatory in ruby, it frequent to not use them if the method does not accept an arguments or if in this case we just want to use the default.

Copy link
Member

Choose a reason for hiding this comment

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

Great. That's what I was wondering about. It looks like a function handle to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually the

c.vm.provision "shell", inline: linuxGvmProvision, privileged: false

Is also a function call.

c.vm.provision("shell", inline: linuxGvmProvision, privileged: false)

The following is also valid,

c.vm.provision("shell", { inline: linuxGvmProvision, privileged: false })

x-pack/libbeat/licenser/elastic_fetcher.go Outdated Show resolved Hide resolved
@ph ph merged commit 4b7823b into elastic:master Apr 4, 2019
@ph ph mentioned this pull request Apr 4, 2019
Merged
ph added a commit to ph/beats that referenced this pull request Apr 4, 2019
@ph
Fixes license check issue with invalid license UUID and check on 32 b…
ff72ae4 
…its system. (elastic#11649)

- Relax validation of the X-Pack license UID value.
- Fix a parsing error with the X-Pack license check on 32-bit system. 

Fixes: elastic#11640
Fixes: elastic#11650
(cherry picked from commit 4b7823b)
@ph ph added v7.0.0 and removed needs_backport PR is waiting to be backported to other branches. labels Apr 4, 2019
ph added a commit to ph/beats that referenced this pull request Apr 4, 2019
@ph
Fixes license check issue with invalid license UUID and check on 32 b…
0933419 
…its system. (elastic#11649)

- Relax validation of the X-Pack license UID value.
- Fix a parsing error with the X-Pack license check on 32-bit system. 

Fixes: elastic#11640
Fixes: elastic#11650
(cherry picked from commit 4b7823b)
@ph ph mentioned this pull request Apr 4, 2019
Merged
@ph ph added the v6.7.2 label Apr 4, 2019
ph added a commit that referenced this pull request Apr 4, 2019
@ph
Cherry-pick #11649 to 7.0: Fixes license check issue with invalid lic…
40b5d12 
…ense UUID and check on 32 bits system. (#11657)

* Fixes license check issue with invalid license UUID and check on 32 bits system. (#11649)

- Relax validation of the X-Pack license UID value.
- Fix a parsing error with the X-Pack license check on 32-bit system. 

Fixes: #11640
Fixes: #11650
(cherry picked from commit 4b7823b)
@graphaelli graphaelli mentioned this pull request Apr 5, 2019
Merged
graphaelli added a commit to elastic/apm-server that referenced this pull request Apr 5, 2019
@graphaelli
[7.0] Update beats framework to aa6953a (#2077)
fb4d677 
* Fix index template always being overwritten - elastic/beats#11671
* Perform Basic license check on Elasticsearch connect - elastic/beats#11296 + elastic/beats#11649
@graphaelli graphaelli mentioned this pull request Apr 5, 2019
Merged
ph added a commit that referenced this pull request Apr 8, 2019
@ph
Cherry-pick #11649 to 6.7: Fixes license check issue with invalid lic…
fcb8490 
…ense UUID and check on 32 bits system. (#11658)

Cherry-pick of PR #11649 to 6.7 branch. Original message: 

- Relax validation of the X-Pack license UID value.
- Fix a parsing error with the X-Pack license check on 32-bit system. 

Fixes: #11640
Fixes: #11650
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewkroh andrewkroh andrewkroh approved these changes

@tsg tsg tsg approved these changes

@ruflin ruflin Awaiting requested review from ruflin

Assignees
No one assigned
Labels
libbeat review v6.7.2 v7.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ph @ruflin @tsg @andrewkroh