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.

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

Make the licensing match what's advertised #8593

Merged
merged 6 commits into from
Mar 14, 2022
Merged

Make the licensing match what's advertised #8593

merged 6 commits into from
Mar 14, 2022

Conversation

mrsimonemms
Copy link
Contributor

@mrsimonemms mrsimonemms commented Mar 4, 2022

Description

The self-hosted licensing now matches what we advertise, namely:

  • Default to free/professional license which allows full access for a specific number of users. Free is for 10 users, pro is for however many their license allows. Internally, this is the "Enterprise" license.
  • Fallback to community license, which allows for none of the EE features (except admin dashboard) for an unlimited number of users. Internally, this is the "Team" license.

This introduces the concept of a fallback license, in addition to the default license. It also adds the number of seats to the Enabled call in the licensor. As a workflow, if the license exceeds the number of seats requested in the call, it will try again with the fallback license, which is the community license.

Fallback rules

License type Fallback allowed
No license (ie, installed via Installer) Y
Gitpod paid license N
Replicated community license Y
Replicated paid license N

This table shows the fallback rules. It is done so that a user who is paying for Gitpod will have additional users rejected rather than having their service downgraded. It would be a pretty poor user experience if someone was paying for Gitpod, had an additional user sign up and then everyone lost all their premium services.

Gitpod licenses are always considered "paid". Now we are moving to Replicated for most licensing, Gitpod licenses will only be issued to paying customers who cannot use Replicated (eg, SaaS). Replicated has the concept of a community license - if their license is marked as "community", it will allow for fallback.

Related Issue(s)

Fixes #8328
Fixes #6932

How to test

Deploy to a self-hosted instance with a Replicated license. Tests already done:

  • Test prebuilds work with <=10 users
  • Test new user rejected when paid license has run out of seats
  • Test new user accepted when paid license changed to a community license
  • Test prebuilds rejected when on community license and > 10 users
  • Test prebuilds accepted when community license changed to a paid license with 20 users

Release Notes

 Make the licensing match what's advertised

Documentation

@codecov
Copy link

codecov bot commented Mar 4, 2022

Codecov Report

Merging #8593 (8fdfc65) into main (4580ba9) will not change coverage.
The diff coverage is n/a.

❗ Current head 8fdfc65 differs from pull request most recent head a9fb138. Consider uploading reports for the commit a9fb138 to get more accurate results

@@           Coverage Diff           @@
##             main    #8593   +/-   ##
=======================================
  Coverage   11.17%   11.17%           
=======================================
  Files          18       18           
  Lines         993      993           
=======================================
  Hits          111      111           
  Misses        880      880           
  Partials        2        2           
Flag Coverage Δ
components-gitpod-cli-app 11.17% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@mrsimonemms mrsimonemms force-pushed the sje/licensing branch 2 times, most recently from 4f94924 to 73d4261 Compare March 4, 2022 15:47
@roboquat roboquat added size/XL and removed size/L labels Mar 4, 2022
@roboquat roboquat added size/L and removed size/XL labels Mar 4, 2022
@roboquat roboquat added size/XL and removed size/L labels Mar 4, 2022
@mrsimonemms mrsimonemms force-pushed the sje/licensing branch 3 times, most recently from 15da68d to 96104e4 Compare March 7, 2022 11:05
@mrsimonemms
Copy link
Contributor Author

mrsimonemms commented Mar 7, 2022

/werft run

👍 started the job as gitpod-build-sje-licensing.12

@mrsimonemms mrsimonemms force-pushed the sje/licensing branch 10 times, most recently from 8fdfc65 to 4e958d8 Compare March 8, 2022 16:35
@mrsimonemms
Copy link
Contributor Author

@geropl I kept the caching of the getUsersCount call as a separate commit, to make it easier for you to check I'm on the right track with what you suggested. Happy to keep as-is or to merge into the other [server] commit - your call

@mrsimonemms mrsimonemms marked this pull request as ready for review March 9, 2022 09:12
@mrsimonemms mrsimonemms requested review from a team March 9, 2022 09:12
@github-actions github-actions bot added team: delivery Issue belongs to the self-hosted team team: webapp Issue belongs to the WebApp team labels Mar 9, 2022
@corneliusludmann
Copy link
Contributor

Great progress, @mrsimonemms! 🙏

Suggestion

I'm totally fine with moving this suggestion to a follow-up PR (since this PR is already a minimum viable change). However, in order to fix #8412 I would suggest the following change:

  1. Allow fallback here:
    resp, err := client.Get(replicatedLicenseApiEndpoint)
    if err != nil {
    return &Evaluator{invalid: fmt.Sprintf("cannot query kots admin, %q", err)}
    }
    defer resp.Body.Close()
    var replicatedPayload replicatedLicensePayload
    err = json.NewDecoder(resp.Body).Decode(&replicatedPayload)
    if err != nil {
    return &Evaluator{invalid: fmt.Sprintf("cannot decode json data, %q", err)}
    }
  2. Return an invalid license that allows fallback here instead of the default replicated license:
    if !matchesDomain(lic.Domain, domain) {
    return defaultReplicatedLicense()
    }
    if replicatedPayload.ExpirationTime != nil {
    lic.ValidUntil = *replicatedPayload.ExpirationTime
    if lic.ValidUntil.Before(time.Now()) {
    return defaultReplicatedLicense()
    }
    }
  3. Allow fallback in case of an invalid license here instead of returning false:
    func (e *Evaluator) Enabled(feature Feature, seats int) bool {
    if e.invalid != "" {
    return false
    }

    func (e *Evaluator) hasEnoughSeats(seats int) bool {
    if e.invalid != "" {
    return false
    }

With this, we can display the result of

// Validate returns false if the license isn't valid and a message explaining why that is.
func (e *Evaluator) Validate() (msg string, valid bool) {
if e.invalid == "" {
return "", true
}
return e.invalid, false
}

in the admin dashboard and users know why the license is invalid and they still have basic functionality (as you would have with no license).

Copy link
Contributor

@corneliusludmann corneliusludmann left a comment

Choose a reason for hiding this comment

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

Licensor part looks good to me.

ℹ️ I would like to leave the final review of the server part to Team WebApp.

@mrsimonemms
Copy link
Contributor Author

@corneliusludmann I'll remove the "Fixes" for #8412 and we can continue the discussion there

Simon Emms added 6 commits March 11, 2022 10:04
The previous Gitpod and Replicated evaluators were functionally
identical anyway as all the logic happens in the constructors
The admin dashboard is now an essential part of administering Gitpod
so it makes no sense to block it for users.
…ures

The Enabled function now has knowledge of the number of seats in use. If
this is still within range, the features are checked against the loaded
license. If not, they will be checked against the fallback license.

The fallback is optional, based upon the license type - Gitpod licenses
always disable fallback. Replicated licenses disable fallback if it's a
paid license. This is so paying customers aren't inconvenienced by
losing features - instead, they will be unable to add additional users,
as is the current behaviour.

protected readonly timeout: number = 60 * 1000; // Cache data for 1 minute

get count(): number | null {
Copy link
Member

Choose a reason for hiding this comment

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

nit: we usually do not use null but undefined instead. But ignore this for how, we'll have a lint for it shortly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@geropl Would you like me to change this? I don't mind - normally, I use undefined as well so I can't understand why I used null

Copy link
Member

@geropl geropl left a comment

Choose a reason for hiding this comment

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

Thx @mrsimonemms , looks good to me!

Did not test the self-hosted setup, though. 😉

@roboquat roboquat merged commit 99e0371 into main Mar 14, 2022
@roboquat roboquat deleted the sje/licensing branch March 14, 2022 14:44
@roboquat roboquat added the deployed: webapp Meta team change is running in production label Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production release-note size/XL team: delivery Issue belongs to the self-hosted team team: webapp Issue belongs to the WebApp team
Projects
None yet
4 participants