-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Fix moment.js license expiration parsing #19014
Conversation
@@ -116,11 +116,13 @@ export class XPackInfo { | |||
path: '/_xpack' | |||
}); | |||
|
|||
const millis = parseInt(get(response, 'license.expiry_date_in_millis') / 1000000); |
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.
This is an ES bug if the value for expiry_date_in_millis
is in microseconds.
💚 Build Succeeded |
@epixa ready for review. I checked with es team and got the sense that it's meant to be "never". but they didn't get back to me on whether it's enough to just say "never" if the license type is 'basic'. Is there a scenario where it could be basic but we want to flag a problem with an invalid date? Either way, this is written so that we show "never" when both the given expiry_date_in_millis to be unparseable by moment and the license is basic. pinging @tvernum here in case he can get back to me now with more context. |
_getExpiryDate(xpackInfo) { | ||
let expiryDate = moment(get(xpackInfo, 'license.expiry_date_in_millis')).format('YYYY MMM DD HH:mm'); | ||
|
||
if (expiryDate === 'Invalid date' && get(xpackInfo, 'license.mode') === 'basic') { |
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.
this.license.getType()
doesn't work here. is this the right way to check for basic license??
💚 Build Succeeded |
You'll need @tbrooks8 to confirm, I was just the messenger. |
It's still not clear to me what is actually being returned by Elasticsearch. We need to confirm that it is indeed nanoseconds. If it's anything but milliseconds, then there's a bug in Elasticsearch. If this is a bug in Elasticsearch and we went forward with this PR as-is, then sometime down the line when that bug is fixed in Elasticsearch, the logic we're relying on in Kibana would disappear, and we'd start showing a ridiculously far into the future date. Even if this was acceptable, it's not what we intended, and that's not good. Ideally Elasticsearch wouldn't be returning any timestamp at all. It may have been convenient from an engineering perspective to just throw in some super far into the future date, but we're telling everyone that basic licenses never expire, which doesn't seem to be accurate. |
It is milliseconds.
The binary signature of licenses have a expiration date in milliseconds. There is not really a way around that decision that was made a long time ago. Generated basic licenses have a hardcoded expiration milliseconds of 9223372005318775807 (1 year before 2^63). The current logic is remove that from the rest response if that is the expiration milliseconds to prevent a confusing rest response. If elasticsearch is returning 9223372005318776000 for something that seems to be a bug. We could use the steps from kibana that can reproduce this issue. |
I have identified an issue on the elasticsearch side and opened a PR (elastic/elasticsearch#30848). The |
@tbrooks8 Thanks a lot for clarifying. That all makes sense. You can reproduce with a GET request to
|
Ah yes, thanks. That is indeed the issue we had, so thanks a lot for identifying it and fixing it. Can you backport it to 6.3? For what it's worth, I get |
Yes. It will backported to 6.4 and 6.3 |
Closing this in favor of #19565 |
Attempting to fix #18388
I think this is actually in nanoseconds (see comments).