-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 Replicated as a Gitpod license evaluator #8211
Conversation
Codecov Report
@@ Coverage Diff @@
## main #8211 +/- ##
==========================================
+ Coverage 12.31% 20.06% +7.75%
==========================================
Files 20 22 +2
Lines 1161 1171 +10
==========================================
+ Hits 143 235 +92
+ Misses 1014 919 -95
- Partials 4 17 +13
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
015ad0d
to
30f98c9
Compare
f86182b
to
f7f38a9
Compare
f7f38a9
to
1b3d816
Compare
Quick test with a valid license looks good. However, I'm pretty unhappy that an invalid license would prevent using Gitpod at all. In my opinion, with an invalid license, we should also fallback like there is no license (not only for the Replicated license but also for the Gitpod license but the latter is not so important at the moment), which means basically using the default built-in license when the license is invalid: gitpod/components/licensor/ee/pkg/licensor/licensor.go Lines 118 to 123 in 558d979
I think it's odd that an installation with an expired license or with a license for another domain is in a worse position than an installation with no license at all. That means, in my opinion, a test like this one:
added here should succeed and not fail (we don't have this test with a wrong domain but that should work as well). And there is also an actual need for such a behavior: When we want to publish a generic community license, we need to ignore the domain at least. Because this license needs to work with all domains. However, for this use case, we should probably think about making the domain value optional. |
So, from a Replicated point-of-view, we can't get through the license gate in their UI with an invalid/expired license. I am fully in agreement that this is a retrograde step and it's one of the things I've scheduled to discuss with them in our weekly meeting. With regards to the community license, I'm forming the opinion that we're not going to be able to have a single license for all community members. See the "future improvements" section of this PR and #8329 for details. Again, this is scheduled for discussion with Replicated. If we've gotten through the initial Replicated license gate in their UI, an expired/invalid license gives the default license. Ticket #8328 exists to make this default license match what we advertise (either free or community, dependent upon the number of seats in use) I've added that test in you put in One thing I'm not overly happy with is what happens if the |
99ad50b
to
82dd964
Compare
If field(s) not found, it returns a warning rather than an error
Defaults to "gitpod" and allows "replicated". This is defined by the secret
82dd964
to
a31a39b
Compare
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 tested this heavily and it works pretty well. I think that's a very good first iteration that we can ship. However, I would like to add a few issues for follow-up pull requests. Stay tuned … 😆
Thanks @corneliusludmann. I'm definitely seeing this as the first iteration of many in the licensing epic |
a31a39b
to
896fe39
Compare
/hold |
Have checked that this still works with the original license, which it does /unhold |
@mrsimonemms: You haven't touched the helm charts but changed the server deployment in the installer, right? Would that break SaaS deployment for team WebApp (as far as I know, they still use the helm charts)? |
@corneliusludmann I don't believe so. It defaults to the Gitpod evaluator and only uses the Replicated one if the secret tells it to |
Starting to review now... |
{"within default license seats", 0, 7, true, true, false}, | ||
{"within default license seats (edge)", 0, 10, true, true, false}, | ||
{"beyond default license seats", 0, 11, false, true, false}, | ||
{"Gitpod: unlimited seats", 0, 1000, true, false, false, LicenseTypeGitpod, false}, |
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.
❤️
@@ -53,7 +54,6 @@ func deployment(ctx *common.RenderContext) ([]runtime.Object, error) { | |||
Name: "gitpod-license-key", | |||
MountPath: licenseFilePath, | |||
SubPath: "license", | |||
ReadOnly: 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.
Why is this necessary? 🤔 Maybe it make sense to make that explicit here, and add a comment that states why that is required?
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.
Arguably not, but it feels prudent to make readonly values explicitly readonly to me
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.
Code LG - and nicely clean - TM, thx!
/hold
In case you might want to add a comment
Thanks @geropl - much appreciated you checking it out |
/unhold |
Description
Replicated is how we will now distribute Gitpod to enterprise teams. This adds Replicated as a licensor and is used if the secret has
type = replicated
(which sets the envvarGITPOD_LICENSE_TYPE=replicated
in theserver
).This PR also renames the
Evaluator
asGitpodEvaluator
and introduces anEvaluator
interface, which theGitpodEvaluator
andReplicatedEvaluator
implements. The GitpodEvaluator will remain in-place for existing customers (eventually, will migrate to Replicated) and also for SaaS.Like the Gitpod license, the Replicated license is only validated at runtime. If the user wishes to update their license, they would have to click "sync license" in their Kots portal which would tell them to redeploy the application - this would create a new
server
pod which would verify the new license at runtime.Unlike the Gitpod license, the Replicated license is retrieved from a local service and has no (obvious) way of verifying the data is from a legitimate source, rather than a mocked service. For that reason, the URL is hardcoded as per the docs and offers no way of configuring this.
The license has three custom entitlements -
domain
,levelId
andseats
. An entitlement is custom data that's attached to the license. These are used to provide the data that we use for validation.Future improvements
Talk with Replicated and establish a way of verifying that the data is valid and from an approved source. Replicated provides a signature which has an
innerSignature
which has a public key and signature but this verification is entirely undocumented.We may also want to verify community licenses slightly differently, eg without the domain so we can just have a single community license. NB it may be tricky for a user to go from community to paid if we have a single license - this will need raising with Replicated
Resources
Related Issue(s)
Fixes gitpod-io/replicated#1
How to test
Release Notes
Documentation