-
Notifications
You must be signed in to change notification settings - Fork 469
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 and AgendaJob that will refresh the license once a day #2404
Conversation
Your preview environment pr-2404-bttf has been deployed. Preview environment endpoints are available at: |
@@ -9,7 +8,6 @@ export async function init() { | |||
initPromise = (async () => { | |||
await mongoInit(); | |||
await queueInit(); | |||
await initializeLicenseForOrg(); |
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 required for the server to function properly, right? In a horizontally scaled prod environment, are we 100% sure the agenda job will always run immediately at startup?
I don't want this getting blocked by some other jobs for 10 minutes and the instance basically being downgraded to free during that time.
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.
The only time this does anything is if the license is stored in env var and it is their very first time loading the server so that there is nothing in the mongo cache. However there will be no other jobs the first time the server starts up so the job will run right away. Even if it doesn't they won't be downgraded to free, they would just wait for a single license server call to return.
// no orgs upon first initialization - in which case we get the key from the env var | ||
// exactly one org normally - in which case we get the key from the org | ||
// or multiple orgs if it is a MULTI_ORG site - in which case the org is not allowed to have a license key and we shall also fall back to the env var | ||
org = (await getOrganization()) || undefined; |
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.
I didn't know this getOrganization()
method existed. We should delete this ASAP as it is extremely dangerous to have in the codebase. It doesn't check for self-hosted, so if this were to be called on cloud, it could potentially expose a random organization to other users.
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.
I see it's still being used in another file now. We can keep the function, but we should rename it to be foolproof and super obvious about what it's used for and add additional protections for Cloud/MultiOrg within the function.
I don't want a dev to think "I need the org in this API route I'm building, getOrganization()
looks like it does what I want". Our dev machines are usually single tenant and the function name seems harmless, so it's the type of bug that might go unnoticed until it hits production.
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.
Good point
|
||
const org = await getOrganization(); | ||
if (org) { | ||
initializeLicenseForOrg(org, true); |
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 the true
here? I think the behavior without forcing a refresh is closer to what we want - it will basically only refresh when the cached data is more than 1 day old. So with this job, the license should never get more than 1-2 days old as long as the server is running, which is perfect. And if the server is restarted 20 times in a day, it will only result in 1 API call to the license server
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.
Another good point.
Features and Changes
For self-hosted installations this job will update their license data once a day.
This will allow us to make sure that any installation that hasn't contacted us within a day may have had their connection to us blocked, and their license would then be void within a week if something is not done to unblock the connection.
Testing
Testing a new installation
rename organizations collection in mongo.
Start server
see no error
Testing an org without a license key
Create an organization
Restart server
See no error
Testing a new style env var LICENSE_KEY
Set a LICENSE_KEY in back-end/.env.var to a valid
license_
new style license.Restart the server
See in the logs that it loads the license
Got to localhost:3000 and see that the license info is displayed.
Testing a old style env var LICENSE_KEY
Set LICENSE_KEY to be an old style license key
Restart the server
See in the logs that it loads the license
Got to localhost:3000 and see that the license info is displayed.
Testing a license key on the org
Set licenseKey property in mongo to a new style license
Restart the server
See in the logs that it loads the license
Got to localhost:3000 and see that the license info is displayed.