-
Notifications
You must be signed in to change notification settings - Fork 64
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
Add mqttClients to license claims #4732
Conversation
Pushed fixes for tests, will review the rest when the tests pass |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4732 +/- ##
=======================================
Coverage 78.72% 78.73%
=======================================
Files 311 311
Lines 14672 14697 +25
Branches 3355 3363 +8
=======================================
+ Hits 11551 11572 +21
- Misses 3121 3125 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Looks good, just a couple of question
hooks: function (M, app) { | ||
return { | ||
beforeCreate: async (project, opts) => { | ||
// if the product is licensed, we permit overage | ||
const isLicensed = app.license.active() | ||
if (isLicensed !== true) { | ||
const { mqttClients } = await app.license.usage('mqttClients') | ||
if (mqttClients.count >= mqttClients.limit) { | ||
throw new Error('license limit reached') | ||
} | ||
} | ||
}, | ||
afterCreate: async (project, opts) => { | ||
const { mqttClients } = await app.license.usage('mqttClients') | ||
if (mqttClients.count > mqttClients.limit) { | ||
await app.auditLog.Platform.platform.license.overage('system', null, mqttClients) | ||
} | ||
} | ||
} | ||
}, |
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.
Do we need to generate a new license for prod (and staging) before this gets merged?
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.
Technically no, as we are licensed so allowed to go over the limits at the platform level - but we'll get audit log overage reports. So in practice, we'll want to deploy a new license around the same time as this merging - just doesn't have to be strictly in order.
Need to add reporting on the license usage to the admin overview page - hold off merge until then. |
Have added mqttClients to Whilst there, also fixed reporting the number of Admin users on the platform. Noticed FFC was reporting 0 admin users via this API. This is because postgres returns booleans as true/false whereas sqlite returns as 1/0 - and the code was looking for |
Quick question before merging. Should the license version have been bumped from |
Part of #4655
Description
As part of enabling billing for MQTT Client usage, this first step adds them as a licensable claim. This will allow us to provide self-hosted customers with a license for more than the default allocation (Teams=5, Enterprise=20).
This has required a little bit of moving code around to enable. The commits in this PR do the following:
mqttClients
as a claim in the licensemqttClient
usage to the overage checksThe test updates verify the right limits are applied based on license state. Whilst there, I also updated the existing tests to run using a Team Owner who isn't an Admin (
bob
, notalice
). This is ensures the permissions checks aren't being bypassed by the admin flag.