From 4527037cf91316619cf756991c2d03127c4337f8 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 15 Jul 2024 13:19:53 -0500 Subject: [PATCH 01/15] chore: implement feature sets to licenses Implement different sets of licensed features. Refactor the license logic to fix up some edge cases --- codersdk/deployment.go | 112 ++++++++++++ enterprise/coderd/license/license.go | 262 ++++++++++++++++----------- site/src/api/typesGenerated.ts | 4 + 3 files changed, 274 insertions(+), 104 deletions(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index c1a1e19e810b0..1657a526c665f 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -34,6 +34,23 @@ const ( EntitlementNotEntitled Entitlement = "not_entitled" ) +func CompareEntitlements(a, b Entitlement) int { + return entitlementWeight(a) - entitlementWeight(b) +} + +func entitlementWeight(e Entitlement) int { + switch e { + case EntitlementEntitled: + return 2 + case EntitlementGracePeriod: + return 1 + case EntitlementNotEntitled: + return 0 + default: + return -1 + } +} + // FeatureName represents the internal name of a feature. // To add a new feature, add it to this set of enums as well as the FeatureNames // array below. @@ -108,6 +125,51 @@ func (n FeatureName) AlwaysEnable() bool { }[n] } +// FeatureSet represents a grouping of features. This is easier +// than manually assigning features al-la-carte when making a license. +// These sets are dynamic in the sense a feature can be added to an existing +// set to grant an additional feature to an existing license. +// If features were granted al-la-carte, we would need to reissue the license +// to include the new feature. +type FeatureSet string + +const ( + FeatureSetNone FeatureSet = "" + FeatureSetEnterprise FeatureSet = "enterprise" + FeatureSetPremium FeatureSet = "premium" +) + +func (set FeatureSet) Features() []FeatureName { + switch FeatureSet(strings.ToLower(string(set))) { + case FeatureSetEnterprise: + // List all features that should be included in the Enterprise feature set. + return []FeatureName{ + FeatureUserLimit, + FeatureAuditLog, + FeatureBrowserOnly, + FeatureSCIM, + FeatureTemplateRBAC, + FeatureHighAvailability, + FeatureMultipleExternalAuth, + FeatureExternalProvisionerDaemons, + FeatureAppearance, + FeatureAdvancedTemplateScheduling, + FeatureWorkspaceProxy, + FeatureUserRoleManagement, + FeatureExternalTokenEncryption, + FeatureWorkspaceBatchActions, + FeatureAccessControl, + FeatureControlSharedPorts, + FeatureCustomRoles, + } + case FeatureSetPremium: + // FeatureSetPremium is a superset of Enterprise + return append(FeatureSetEnterprise.Features()) + } + // By default, return an empty set. + return []FeatureName{} +} + type Feature struct { Entitlement Entitlement `json:"entitlement"` Enabled bool `json:"enabled"` @@ -125,6 +187,56 @@ type Entitlements struct { RefreshedAt time.Time `json:"refreshed_at" format:"date-time"` } +// AddFeature will add the feature to the entitlements iff it expands +// the set of features granted by the entitlements. If it does not, it will +// be ignored and the existing feature with the same name will remain. +// +// All features should be added as atomic items, and not merged in any way. +// Merging entitlements could lead to unexpected behavior, like a larger user +// limit in grace period merging with a smaller one in a grace period. This could +// lead to the larger limit being extended as "entitled", which is not correct. +func (e *Entitlements) AddFeature(name FeatureName, add Feature) { + existing, ok := e.Features[name] + if !ok { + e.Features[name] = add + return + } + + comparison := CompareEntitlements(add.Entitlement, existing.Entitlement) + // If the new entitlement is greater than the existing entitlement, replace it. + // The edge case is if the previous entitlement is in a grace period with a + // higher value. + // TODO: Address the edge case. + if comparison > 0 { + e.Features[name] = add + return + } + + // If they have the same entitlement, then we can compare the limits. + if comparison == 0 { + if add.Limit != nil { + if existing.Limit == nil || *add.Limit > *existing.Limit { + e.Features[name] = add + return + } + } + + // Enabled is better than disabled. + if add.Enabled && !existing.Enabled { + e.Features[name] = add + return + } + + // If the actual value is greater than the existing actual value, replace it. + if add.Actual != nil { + if existing.Actual == nil || *add.Actual > *existing.Actual { + e.Features[name] = add + return + } + } + } +} + func (c *Client) Entitlements(ctx context.Context) (Entitlements, error) { res, err := c.Request(ctx, http.MethodGet, "/api/v2/entitlements", nil) if err != nil { diff --git a/enterprise/coderd/license/license.go b/enterprise/coderd/license/license.go index e5ce3d203b100..499629b6896b3 100644 --- a/enterprise/coderd/license/license.go +++ b/enterprise/coderd/license/license.go @@ -11,7 +11,6 @@ import ( "golang.org/x/xerrors" "cdr.dev/slog" - "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/codersdk" @@ -28,51 +27,95 @@ func Entitlements( enablements map[codersdk.FeatureName]bool, ) (codersdk.Entitlements, error) { now := time.Now() - // Default all entitlements to be disabled. - entitlements := codersdk.Entitlements{ - Features: map[codersdk.FeatureName]codersdk.Feature{}, - Warnings: []string{}, - Errors: []string{}, - } - for _, featureName := range codersdk.FeatureNames { - entitlements.Features[featureName] = codersdk.Feature{ - Entitlement: codersdk.EntitlementNotEntitled, - Enabled: enablements[featureName], - } - } // nolint:gocritic // Getting unexpired licenses is a system function. licenses, err := db.GetUnexpiredLicenses(dbauthz.AsSystemRestricted(ctx)) if err != nil { - return entitlements, err + return codersdk.Entitlements{}, err } // nolint:gocritic // Getting active user count is a system function. activeUserCount, err := db.GetActiveUserCount(dbauthz.AsSystemRestricted(ctx)) if err != nil { - return entitlements, xerrors.Errorf("query active user count: %w", err) + return codersdk.Entitlements{}, xerrors.Errorf("query active user count: %w", err) } // always shows active user count regardless of license - entitlements.Features[codersdk.FeatureUserLimit] = codersdk.Feature{ - Entitlement: codersdk.EntitlementNotEntitled, - Enabled: enablements[codersdk.FeatureUserLimit], - Actual: &activeUserCount, + entitlements, err := LicensesEntitlements(now, licenses, enablements, keys, FeatureArguments{ + ActiveUserCount: activeUserCount, + ReplicaCount: replicaCount, + ExternalAuthCount: externalAuthCount, + }) + + return entitlements, nil +} + +type FeatureArguments struct { + ActiveUserCount int64 + ReplicaCount int + ExternalAuthCount int +} + +// LicensesEntitlements returns the entitlements for licenses. Entitlements are +// merged from all licenses and the highest entitlement is used for each feature. +// Arguments: +// +// now: The time to use for checking license expiration. +// license: The license to check. +// enablements: Features can be explicitly disabled by the deployment even if +// the license has the feature entitled. Features can also have +// the 'feat.AlwaysEnable()' return true to disallow disabling. +// featureArguments: Additional arguments required by specific features. +func LicensesEntitlements( + now time.Time, + licenses []database.License, + enablements map[codersdk.FeatureName]bool, + keys map[string]ed25519.PublicKey, + featureArguments FeatureArguments, +) (codersdk.Entitlements, error) { + + // Default all entitlements to be disabled. + entitlements := codersdk.Entitlements{ + Features: map[codersdk.FeatureName]codersdk.Feature{ + // always shows active user count regardless of license. + codersdk.FeatureUserLimit: { + Entitlement: codersdk.EntitlementNotEntitled, + Enabled: enablements[codersdk.FeatureUserLimit], + Actual: &featureArguments.ActiveUserCount, + }, + }, + Warnings: []string{}, + Errors: []string{}, } - allFeatures := false - allFeaturesEntitlement := codersdk.EntitlementNotEntitled + // By default, enumerate all features and set them to not entitled. + for _, featureName := range codersdk.FeatureNames { + entitlements.AddFeature(featureName, codersdk.Feature{ + Entitlement: codersdk.EntitlementNotEntitled, + Enabled: enablements[featureName], + }) + } - // Here we loop through licenses to detect enabled features. - for _, l := range licenses { - claims, err := ParseClaims(l.JWT, keys) + // TODO: License specific warnings and errors should be tied to the license, not the + // 'Entitlements' group as a whole. + for _, license := range licenses { + claims, err := ParseClaims(license.JWT, keys) if err != nil { - logger.Debug(ctx, "skipping invalid license", - slog.F("id", l.ID), slog.Error(err)) + entitlements.Errors = append(entitlements.Errors, + fmt.Sprintf("Invalid license (%s) parsing claims: %s", license.UUID.String(), err.Error())) continue } + + // Any valid license should toggle this boolean entitlements.HasLicense = true + + // If any license requires telemetry, the deployment should require telemetry. + entitlements.RequireTelemetry = entitlements.RequireTelemetry || claims.RequireTelemetry + + // entitlement is the highest entitlement for any features in this license. entitlement := codersdk.EntitlementEntitled + // If any license is a trial license, this should be set to true. + // The user should delete the trial license to remove this. entitlements.Trial = claims.Trial if now.After(claims.LicenseExpires.Time) { // if the grace period were over, the validation fails, so if we are after @@ -80,22 +123,27 @@ func Entitlements( entitlement = codersdk.EntitlementGracePeriod } - // Add warning if license is expiring soon - daysToExpire := int(math.Ceil(claims.LicenseExpires.Sub(now).Hours() / 24)) - isTrial := entitlements.Trial - showWarningDays := 30 - if isTrial { - showWarningDays = 7 + // Will add a warning if the license is expiring soon. + // This warning can be raised multiple times if there is more than 1 license. + licenseExpirationWarning(&entitlements, now, claims) + + // 'claims.AllFeature' is the legacy way to set 'claims.FeatureSet = codersdk.FeatureSetEnterprise' + // If both are set, ignore the legacy 'claims.AllFeature' + if claims.AllFeatures && claims.FeatureSet == "" { + claims.FeatureSet = codersdk.FeatureSetEnterprise } - isExpiringSoon := daysToExpire > 0 && daysToExpire < showWarningDays - if isExpiringSoon { - day := "day" - if daysToExpire > 1 { - day = "days" - } - entitlements.Warnings = append(entitlements.Warnings, fmt.Sprintf("Your license expires in %d %s.", daysToExpire, day)) + + // Add all features from the feature set defined. + for _, featureName := range claims.FeatureSet.Features() { + entitlements.AddFeature(featureName, codersdk.Feature{ + Entitlement: entitlement, + Enabled: enablements[featureName] || featureName.AlwaysEnable(), + Limit: nil, + Actual: nil, + }) } + // Features al-la-carte for featureName, featureValue := range claims.Features { // Can this be negative? if featureValue <= 0 { @@ -103,55 +151,76 @@ func Entitlements( } switch featureName { - // User limit has special treatment as our only non-boolean feature. case codersdk.FeatureUserLimit: + // User limit has special treatment as our only non-boolean feature. limit := featureValue - priorLimit := entitlements.Features[codersdk.FeatureUserLimit] - if priorLimit.Limit != nil && *priorLimit.Limit > limit { - limit = *priorLimit.Limit - } - entitlements.Features[codersdk.FeatureUserLimit] = codersdk.Feature{ + entitlements.AddFeature(codersdk.FeatureUserLimit, codersdk.Feature{ Enabled: true, Entitlement: entitlement, Limit: &limit, - Actual: &activeUserCount, - } + Actual: &featureArguments.ActiveUserCount, + }) default: entitlements.Features[featureName] = codersdk.Feature{ - Entitlement: maxEntitlement(entitlements.Features[featureName].Entitlement, entitlement), + Entitlement: entitlement, Enabled: enablements[featureName] || featureName.AlwaysEnable(), } } } + } - if claims.AllFeatures { - allFeatures = true - allFeaturesEntitlement = maxEntitlement(allFeaturesEntitlement, entitlement) + // Now the license specific warnings and errors are added to the entitlements. + + // If HA is enabled, ensure the feature is entitled. + if featureArguments.ReplicaCount > 1 { + feature := entitlements.Features[codersdk.FeatureHighAvailability] + + switch feature.Entitlement { + case codersdk.EntitlementNotEntitled: + if entitlements.HasLicense { + entitlements.Errors = append(entitlements.Errors, + "You have multiple replicas but your license is not entitled to high availability. You will be unable to connect to workspaces.") + } else { + entitlements.Errors = append(entitlements.Errors, + "You have multiple replicas but high availability is an Enterprise feature. You will be unable to connect to workspaces.") + } + case codersdk.EntitlementGracePeriod: + entitlements.Warnings = append(entitlements.Warnings, + "You have multiple replicas but your license for high availability is expired. Reduce to one replica or workspace connections will stop working.") } - entitlements.RequireTelemetry = entitlements.RequireTelemetry || claims.RequireTelemetry } - if allFeatures { - for _, featureName := range codersdk.FeatureNames { - // No user limit! - if featureName == codersdk.FeatureUserLimit { - continue + if featureArguments.ExternalAuthCount > 1 { + feature := entitlements.Features[codersdk.FeatureMultipleExternalAuth] + + switch feature.Entitlement { + case codersdk.EntitlementNotEntitled: + if entitlements.HasLicense { + entitlements.Errors = append(entitlements.Errors, + "You have multiple External Auth Providers configured but your license is limited at one.", + ) + } else { + entitlements.Errors = append(entitlements.Errors, + "You have multiple External Auth Providers configured but this is an Enterprise feature. Reduce to one.", + ) } - feature := entitlements.Features[featureName] - feature.Entitlement = maxEntitlement(feature.Entitlement, allFeaturesEntitlement) - feature.Enabled = enablements[featureName] || featureName.AlwaysEnable() - entitlements.Features[featureName] = feature + case codersdk.EntitlementGracePeriod: + entitlements.Warnings = append(entitlements.Warnings, + "You have multiple External Auth Providers configured but your license is expired. Reduce to one.", + ) } } if entitlements.HasLicense { userLimit := entitlements.Features[codersdk.FeatureUserLimit].Limit - if userLimit != nil && activeUserCount > *userLimit { + if userLimit != nil && featureArguments.ActiveUserCount > *userLimit { entitlements.Warnings = append(entitlements.Warnings, fmt.Sprintf( "Your deployment has %d active users but is only licensed for %d.", - activeUserCount, *userLimit)) + featureArguments.ActiveUserCount, *userLimit)) } + // Add a warning for every feature that is enabled but not entitled or + // is in a grace period. for _, featureName := range codersdk.FeatureNames { // The user limit has it's own warnings! if featureName == codersdk.FeatureUserLimit { @@ -165,6 +234,7 @@ func Entitlements( if featureName == codersdk.FeatureMultipleExternalAuth { continue } + feature := entitlements.Features[featureName] if !feature.Enabled { continue @@ -182,45 +252,7 @@ func Entitlements( } } - if replicaCount > 1 { - feature := entitlements.Features[codersdk.FeatureHighAvailability] - - switch feature.Entitlement { - case codersdk.EntitlementNotEntitled: - if entitlements.HasLicense { - entitlements.Errors = append(entitlements.Errors, - "You have multiple replicas but your license is not entitled to high availability. You will be unable to connect to workspaces.") - } else { - entitlements.Errors = append(entitlements.Errors, - "You have multiple replicas but high availability is an Enterprise feature. You will be unable to connect to workspaces.") - } - case codersdk.EntitlementGracePeriod: - entitlements.Warnings = append(entitlements.Warnings, - "You have multiple replicas but your license for high availability is expired. Reduce to one replica or workspace connections will stop working.") - } - } - - if externalAuthCount > 1 { - feature := entitlements.Features[codersdk.FeatureMultipleExternalAuth] - - switch feature.Entitlement { - case codersdk.EntitlementNotEntitled: - if entitlements.HasLicense { - entitlements.Errors = append(entitlements.Errors, - "You have multiple External Auth Providers configured but your license is limited at one.", - ) - } else { - entitlements.Errors = append(entitlements.Errors, - "You have multiple External Auth Providers configured but this is an Enterprise feature. Reduce to one.", - ) - } - case codersdk.EntitlementGracePeriod: - entitlements.Warnings = append(entitlements.Warnings, - "You have multiple External Auth Providers configured but your license is expired. Reduce to one.", - ) - } - } - + // Wrap up by disabling all features that are not entitled. for _, featureName := range codersdk.FeatureNames { feature := entitlements.Features[featureName] if feature.Entitlement == codersdk.EntitlementNotEntitled { @@ -261,8 +293,11 @@ type Claims struct { AccountType string `json:"account_type,omitempty"` AccountID string `json:"account_id,omitempty"` // DeploymentIDs enforces the license can only be used on a set of deployments. - DeploymentIDs []string `json:"deployment_ids,omitempty"` - Trial bool `json:"trial"` + DeploymentIDs []string `json:"deployment_ids,omitempty"` + Trial bool `json:"trial"` + FeatureSet codersdk.FeatureSet `json:"feature_set"` + // AllFeatures represents 'FeatureSet = FeatureSetEnterprise' + // Deprecated: AllFeatures is deprecated in favor of FeatureSet. AllFeatures bool `json:"all_features"` Version uint64 `json:"version"` Features Features `json:"features"` @@ -340,3 +375,22 @@ func maxEntitlement(e1, e2 codersdk.Entitlement) codersdk.Entitlement { } return codersdk.EntitlementNotEntitled } + +// licenseExpirationWarning adds a warning message if the license is expiring soon. +func licenseExpirationWarning(entitlements *codersdk.Entitlements, now time.Time, claims *Claims) { + // Add warning if license is expiring soon + daysToExpire := int(math.Ceil(claims.LicenseExpires.Sub(now).Hours() / 24)) + showWarningDays := 30 + isTrial := entitlements.Trial + if isTrial { + showWarningDays = 7 + } + isExpiringSoon := daysToExpire > 0 && daysToExpire < showWarningDays + if isExpiringSoon { + day := "day" + if daysToExpire > 1 { + day = "days" + } + entitlements.Warnings = append(entitlements.Warnings, fmt.Sprintf("Your license expires in %d %s.", daysToExpire, day)) + } +} diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index bc4ff5a038ccb..a649970733ae6 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -2104,6 +2104,10 @@ export const FeatureNames: FeatureName[] = [ "workspace_proxy", ]; +// From codersdk/deployment.go +export type FeatureSet = "" | "enterprise" | "premium"; +export const FeatureSets: FeatureSet[] = ["", "enterprise", "premium"]; + // From codersdk/groups.go export type GroupSource = "oidc" | "user"; export const GroupSources: GroupSource[] = ["oidc", "user"]; From 743c442fa0b50c5f0f6389dd886a8f0964266260 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 16 Jul 2024 13:59:03 -0500 Subject: [PATCH 02/15] feat: implement premium vs enterprise licenses Refactor license logic to keep license features atomic (no merging). Refactor logic to be more unit testable. --- codersdk/deployment.go | 124 +++++--- .../coderd/coderdenttest/coderdenttest.go | 47 ++- enterprise/coderd/license/license.go | 17 +- enterprise/coderd/license/license_test.go | 280 ++++++++++++++++++ enterprise/coderd/license/testdata/README.md | 13 + 5 files changed, 441 insertions(+), 40 deletions(-) create mode 100644 enterprise/coderd/license/testdata/README.md diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 1657a526c665f..2b23e680236b6 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -45,9 +45,9 @@ func entitlementWeight(e Entitlement) int { case EntitlementGracePeriod: return 1 case EntitlementNotEntitled: - return 0 - default: return -1 + default: + return -2 } } @@ -177,6 +177,91 @@ type Feature struct { Actual *int64 `json:"actual,omitempty"` } +// CompareFeatures compares two features and returns an integer representing +// if the first feature is greater than, equal to, or less than the second feature. +// "Greater than" means the first feature has more functionality than the +// second feature. It is assumed the features are for the same FeatureName. +// +// A feature is considered greater than another feature if: +// - Graceful & capable > Entitled & not capable +// - The entitlement is greater +// - The limit is greater +// - Enabled is greater than disabled +// - The actual is greater +func CompareFeatures(a, b Feature) int { + if !a.Capable() || !b.Capable() { + // If either is incapable, then it is possible a grace period + // feature can be "greater" than an entitled. + // If either is "NotEntitled" then we can defer to a strict entitlement + // check. + if entitlementWeight(a.Entitlement) >= 0 && entitlementWeight(b.Entitlement) >= 0 { + if a.Capable() && !b.Capable() { + return 1 + } + if b.Capable() && !a.Capable() { + return -1 + } + } + } + + entitlement := CompareEntitlements(a.Entitlement, b.Entitlement) + if entitlement > 0 { + return 1 + } + if entitlement < 0 { + return -1 + } + + // If the entitlement is the same, then we can compare the limits. + if a.Limit == nil && b.Limit != nil { + return -1 + } + if a.Limit != nil && b.Limit == nil { + return 1 + } + if a.Limit != nil && b.Limit != nil { + difference := *a.Limit - *b.Limit + if *a.Limit-*b.Limit != 0 { + return int(difference) + } + } + + // Enabled is better than disabled. + if a.Enabled && !b.Enabled { + return 1 + } + if !a.Enabled && b.Enabled { + return -1 + } + + // Higher actual is better + if a.Actual == nil && b.Actual != nil { + return -1 + } + if a.Actual != nil && b.Actual == nil { + return 1 + } + if a.Actual != nil && b.Actual != nil { + difference := *a.Actual - *b.Actual + if *a.Actual-*b.Actual != 0 { + return int(difference) + } + } + + return 0 +} + +// Capable is a helper function that returns if a given feature has a limit +// that is greater than or equal to the actual. +// If this condition is not true, then the feature is not capable of being used +// since the limit is not high enough. +func (f Feature) Capable() bool { + if f.Limit != nil && f.Actual != nil { + return *f.Limit >= *f.Actual + } + return true +} + type Entitlements struct { Features map[FeatureName]Feature `json:"features"` Warnings []string `json:"warnings"` @@ -193,8 +278,8 @@ type Entitlements struct { // // All features should be added as atomic items, and not merged in any way. // Merging entitlements could lead to unexpected behavior, like a larger user -// limit in grace period merging with a smaller one in a grace period. This could -// lead to the larger limit being extended as "entitled", which is not correct. +// limit in grace period merging with a smaller one in an "entitled" state. This +// could lead to the larger limit being extended as "entitled", which is not correct. func (e *Entitlements) AddFeature(name FeatureName, add Feature) { existing, ok := e.Features[name] if !ok { @@ -202,39 +287,12 @@ func (e *Entitlements) AddFeature(name FeatureName, add Feature) { return } - comparison := CompareEntitlements(add.Entitlement, existing.Entitlement) - // If the new entitlement is greater than the existing entitlement, replace it. - // The edge case is if the previous entitlement is in a grace period with a - // higher value. - // TODO: Address the edge case. + // Compare the features, keep the one that is "better" + comparison := CompareFeatures(add, existing) if comparison > 0 { e.Features[name] = add return } - - // If they have the same entitlement, then we can compare the limits. - if comparison == 0 { - if add.Limit != nil { - if existing.Limit == nil || *add.Limit > *existing.Limit { - e.Features[name] = add - return - } - } - - // Enabled is better than disabled. - if add.Enabled && !existing.Enabled { - e.Features[name] = add - return - } - - // If the actual value is greater than the existing actual value, replace it. - if add.Actual != nil { - if existing.Actual == nil || *add.Actual > *existing.Actual { - e.Features[name] = add - return - } - } - } } func (c *Client) Entitlements(ctx context.Context) (Entitlements, error) { diff --git a/enterprise/coderd/coderdenttest/coderdenttest.go b/enterprise/coderd/coderdenttest/coderdenttest.go index 882c675213546..d55b7f8d445b1 100644 --- a/enterprise/coderd/coderdenttest/coderdenttest.go +++ b/enterprise/coderd/coderdenttest/coderdenttest.go @@ -146,15 +146,55 @@ func NewWithAPI(t *testing.T, options *Options) ( return client, provisionerCloser, coderAPI, user } +// LicenseOptions is used to generate a license for testing. +// It supports the builder pattern for easy customization. type LicenseOptions struct { AccountType string AccountID string DeploymentIDs []string Trial bool + FeatureSet codersdk.FeatureSet AllFeatures bool - GraceAt time.Time - ExpiresAt time.Time - Features license.Features + // GraceAt is the time at which the license will enter the grace period. + GraceAt time.Time + // ExpiresAt is the time at which the license will hard expire. + // ExpiresAt should always be greater then GraceAt. + ExpiresAt time.Time + Features license.Features +} + +func (opts *LicenseOptions) Expired(now time.Time) *LicenseOptions { + opts.ExpiresAt = now.Add(time.Hour * 24 * -2) + opts.GraceAt = now.Add(time.Hour * 24 * -3) + return opts +} + +func (opts *LicenseOptions) GracePeriod(now time.Time) *LicenseOptions { + opts.ExpiresAt = now.Add(time.Hour * 24) + opts.GraceAt = now.Add(time.Hour * 24 * -1) + return opts +} + +func (opts *LicenseOptions) Valid(now time.Time) *LicenseOptions { + opts.ExpiresAt = now.Add(time.Hour * 24 * 60) + opts.GraceAt = now.Add(time.Hour * 24 * 53) + return opts +} + +func (opts *LicenseOptions) UserLimit(limit int64) *LicenseOptions { + return opts.Feature(codersdk.FeatureUserLimit, limit) +} + +func (opts *LicenseOptions) Feature(name codersdk.FeatureName, value int64) *LicenseOptions { + if opts.Features == nil { + opts.Features = license.Features{} + } + opts.Features[name] = value + return opts +} + +func (opts *LicenseOptions) Generate(t *testing.T) string { + return GenerateLicense(t, *opts) } // AddFullLicense generates a license with all features enabled. @@ -195,6 +235,7 @@ func GenerateLicense(t *testing.T, options LicenseOptions) string { Trial: options.Trial, Version: license.CurrentVersion, AllFeatures: options.AllFeatures, + FeatureSet: options.FeatureSet, Features: options.Features, } tok := jwt.NewWithClaims(jwt.SigningMethodEdDSA, c) diff --git a/enterprise/coderd/license/license.go b/enterprise/coderd/license/license.go index 499629b6896b3..7dcadaeb0ca7d 100644 --- a/enterprise/coderd/license/license.go +++ b/enterprise/coderd/license/license.go @@ -135,6 +135,11 @@ func LicensesEntitlements( // Add all features from the feature set defined. for _, featureName := range claims.FeatureSet.Features() { + if featureName == codersdk.FeatureUserLimit { + // FeatureUserLimit is unique in that it must be specifically defined + // in the license. There is no default meaning if no "limit" is set. + continue + } entitlements.AddFeature(featureName, codersdk.Feature{ Entitlement: entitlement, Enabled: enablements[featureName] || featureName.AlwaysEnable(), @@ -212,11 +217,15 @@ func LicensesEntitlements( } if entitlements.HasLicense { - userLimit := entitlements.Features[codersdk.FeatureUserLimit].Limit - if userLimit != nil && featureArguments.ActiveUserCount > *userLimit { + userLimit := entitlements.Features[codersdk.FeatureUserLimit] + if userLimit.Limit != nil && featureArguments.ActiveUserCount > *userLimit.Limit { entitlements.Warnings = append(entitlements.Warnings, fmt.Sprintf( "Your deployment has %d active users but is only licensed for %d.", - featureArguments.ActiveUserCount, *userLimit)) + featureArguments.ActiveUserCount, *userLimit.Limit)) + } else if userLimit.Limit != nil && userLimit.Entitlement == codersdk.EntitlementGracePeriod { + entitlements.Warnings = append(entitlements.Warnings, fmt.Sprintf( + "Your deployment has %d active users but the license with the limit %d is expired.", + featureArguments.ActiveUserCount, *userLimit.Limit)) } // Add a warning for every feature that is enabled but not entitled or @@ -298,7 +307,7 @@ type Claims struct { FeatureSet codersdk.FeatureSet `json:"feature_set"` // AllFeatures represents 'FeatureSet = FeatureSetEnterprise' // Deprecated: AllFeatures is deprecated in favor of FeatureSet. - AllFeatures bool `json:"all_features"` + AllFeatures bool `json:"all_features,omitempty"` Version uint64 `json:"version"` Features Features `json:"features"` RequireTelemetry bool `json:"require_telemetry,omitempty"` diff --git a/enterprise/coderd/license/license_test.go b/enterprise/coderd/license/license_test.go index f57dd0292d5c2..13184973a9e68 100644 --- a/enterprise/coderd/license/license_test.go +++ b/enterprise/coderd/license/license_test.go @@ -7,12 +7,14 @@ import ( "time" "github.com/google/uuid" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "cdr.dev/slog" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbmem" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" "github.com/coder/coder/v2/enterprise/coderd/license" @@ -529,3 +531,281 @@ func TestEntitlements(t *testing.T) { require.Equal(t, "You have multiple External Auth Providers configured but your license is expired. Reduce to one.", entitlements.Warnings[0]) }) } + +func TestLicenseEntitlements(t *testing.T) { + t.Parallel() + + // We must use actual 'time.Now()' in tests because the jwt library does + // not accept a custom time function. The only way to change it is as a + // package global, which does not work in t.Parallel(). + + // This list comes from coderd.go on launch. This list is a bit arbitrary, + // maybe some should be moved to "AlwaysEnabled" instead. + defaultEnablements := map[codersdk.FeatureName]bool{ + codersdk.FeatureAuditLog: true, + codersdk.FeatureBrowserOnly: true, + codersdk.FeatureSCIM: true, + codersdk.FeatureMultipleExternalAuth: true, + codersdk.FeatureTemplateRBAC: true, + codersdk.FeatureExternalTokenEncryption: true, + codersdk.FeatureExternalProvisionerDaemons: true, + codersdk.FeatureAdvancedTemplateScheduling: true, + codersdk.FeatureWorkspaceProxy: true, + codersdk.FeatureUserRoleManagement: true, + codersdk.FeatureAccessControl: true, + codersdk.FeatureControlSharedPorts: true, + } + + legacyLicense := func() *coderdenttest.LicenseOptions { + return (&coderdenttest.LicenseOptions{ + AccountType: "salesforce", + AccountID: "Alice", + Trial: false, + // Use the legacy boolean + AllFeatures: true, + }).Valid(time.Now()) + } + + enterpriseLicense := func() *coderdenttest.LicenseOptions { + return (&coderdenttest.LicenseOptions{ + AccountType: "salesforce", + AccountID: "Bob", + DeploymentIDs: nil, + Trial: false, + FeatureSet: codersdk.FeatureSetEnterprise, + AllFeatures: false, + }).Valid(time.Now()) + } + + testCases := []struct { + Name string + Licenses []*coderdenttest.LicenseOptions + Enablements map[codersdk.FeatureName]bool + Arguments license.FeatureArguments + + ExpectedErrorContains string + AssertEntitlements func(t *testing.T, entitlements codersdk.Entitlements) + }{ + { + Name: "NoLicenses", + AssertEntitlements: func(t *testing.T, entitlements codersdk.Entitlements) { + assertNoErrors(t, entitlements) + assertNoWarnings(t, entitlements) + assert.False(t, entitlements.HasLicense) + assert.False(t, entitlements.Trial) + }, + }, + { + Name: "MixedUsedCounts", + Licenses: []*coderdenttest.LicenseOptions{ + legacyLicense().UserLimit(100), + enterpriseLicense().UserLimit(500), + }, + Enablements: defaultEnablements, + Arguments: license.FeatureArguments{ + ActiveUserCount: 50, + ReplicaCount: 0, + ExternalAuthCount: 0, + }, + AssertEntitlements: func(t *testing.T, entitlements codersdk.Entitlements) { + assertEnterpriseFeatures(t, entitlements) + assertNoErrors(t, entitlements) + assertNoWarnings(t, entitlements) + userFeature := entitlements.Features[codersdk.FeatureUserLimit] + assert.Equalf(t, int64(500), *userFeature.Limit, "user limit") + assert.Equalf(t, int64(50), *userFeature.Actual, "user count") + }, + }, + { + Name: "MixedUsedCountsWithExpired", + Licenses: []*coderdenttest.LicenseOptions{ + // This license is ignored + enterpriseLicense().UserLimit(500).Expired(time.Now()), + enterpriseLicense().UserLimit(100), + }, + Enablements: defaultEnablements, + Arguments: license.FeatureArguments{ + ActiveUserCount: 200, + ReplicaCount: 0, + ExternalAuthCount: 0, + }, + AssertEntitlements: func(t *testing.T, entitlements codersdk.Entitlements) { + assertEnterpriseFeatures(t, entitlements) + userFeature := entitlements.Features[codersdk.FeatureUserLimit] + assert.Equalf(t, int64(100), *userFeature.Limit, "user limit") + assert.Equalf(t, int64(200), *userFeature.Actual, "user count") + }, + }, + { + // The new license does not have enough seats to cover the active user count. + // The old license is in it's grace period. + Name: "MixedUsedCountsWithGrace", + Licenses: []*coderdenttest.LicenseOptions{ + enterpriseLicense().UserLimit(500).GracePeriod(time.Now()), + enterpriseLicense().UserLimit(100), + }, + Enablements: defaultEnablements, + Arguments: license.FeatureArguments{ + ActiveUserCount: 200, + ReplicaCount: 0, + ExternalAuthCount: 0, + }, + AssertEntitlements: func(t *testing.T, entitlements codersdk.Entitlements) { + userFeature := entitlements.Features[codersdk.FeatureUserLimit] + assert.Equalf(t, int64(500), *userFeature.Limit, "user limit") + assert.Equalf(t, int64(200), *userFeature.Actual, "user count") + assert.Equal(t, userFeature.Entitlement, codersdk.EntitlementGracePeriod) + }, + }, + { + // Legacy license uses the "AllFeatures" boolean + Name: "LegacyLicense", + Licenses: []*coderdenttest.LicenseOptions{ + legacyLicense().UserLimit(100), + }, + Enablements: defaultEnablements, + Arguments: license.FeatureArguments{ + ActiveUserCount: 50, + ReplicaCount: 0, + ExternalAuthCount: 0, + }, + AssertEntitlements: func(t *testing.T, entitlements codersdk.Entitlements) { + assertEnterpriseFeatures(t, entitlements) + assertNoErrors(t, entitlements) + assertNoWarnings(t, entitlements) + userFeature := entitlements.Features[codersdk.FeatureUserLimit] + assert.Equalf(t, int64(100), *userFeature.Limit, "user limit") + assert.Equalf(t, int64(50), *userFeature.Actual, "user count") + }, + }, + } + + for _, tc := range testCases { + tc := tc + + t.Run(tc.Name, func(t *testing.T) { + t.Parallel() + + generatedLicenses := make([]database.License, 0, len(tc.Licenses)) + for i, lo := range tc.Licenses { + generatedLicenses = append(generatedLicenses, database.License{ + ID: int32(i), + UploadedAt: time.Now().Add(time.Hour * -1), + JWT: lo.Generate(t), + Exp: lo.GraceAt, + UUID: uuid.New(), + }) + } + + entitlements, err := license.LicensesEntitlements(time.Now(), generatedLicenses, tc.Enablements, coderdenttest.Keys, tc.Arguments) + if tc.ExpectedErrorContains != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.ExpectedErrorContains) + } else { + require.NoError(t, err) + tc.AssertEntitlements(t, entitlements) + } + }) + } +} + +func TestFeatureComparison(t *testing.T) { + t.Parallel() + + testCases := []struct { + Name string + A codersdk.Feature + B codersdk.Feature + Expected int + }{ + { + Name: "Empty", + Expected: 0, + }, + { + // Tests an exceeded limit that is entitled vs a graceful limit that + // is not exceeded. This is the edge case that we should use the graceful period + // instead of the entitled. + Name: "UserLimitExceeded", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(200))}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementGracePeriod, Limit: ptr.Ref(int64(300)), Actual: ptr.Ref(int64(200))}, + Expected: -1, + }, + { + Name: "UserLimitExceededNoEntitled", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(200))}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementNotEntitled, Limit: ptr.Ref(int64(300)), Actual: ptr.Ref(int64(200))}, + Expected: 1, + }, + { + Name: "HigherLimit", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(110)), Actual: ptr.Ref(int64(200))}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(200))}, + Expected: 10, // Diff in the limit # + }, + { + Name: "HigherActual", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(300))}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(200))}, + Expected: 100, // Diff in the actual # + }, + { + Name: "LimitExists", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(300))}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: nil, Actual: ptr.Ref(int64(200))}, + Expected: 1, + }, + { + Name: "ActualExists", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(300))}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: nil}, + Expected: 1, + }, + { + Name: "NotNils", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(300))}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: nil, Actual: nil}, + Expected: 1, + }, + { + // This is super strange, but it is possible to have a limit but no actual. + // Just adding this test case to solidify the behavior. + // Feel free to change this if you think it should be different. + Name: "LimitVsActual", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: nil}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: nil, Actual: ptr.Ref(int64(200))}, + Expected: 1, + }, + } + + for _, tc := range testCases { + tc := tc + + t.Run(tc.Name, func(t *testing.T) { + t.Parallel() + + r := codersdk.CompareFeatures(tc.A, tc.B) + assert.Equal(t, tc.Expected, r) + + // Comparisons should be like addition. A - B = -1 * (B - A) + r = codersdk.CompareFeatures(tc.B, tc.A) + assert.Equal(t, tc.Expected*-1, r, "the inverse comparison should also be true") + }) + } +} + +func assertNoErrors(t *testing.T, entitlements codersdk.Entitlements) { + assert.Empty(t, entitlements.Errors, "no errors") +} + +func assertNoWarnings(t *testing.T, entitlements codersdk.Entitlements) { + assert.Empty(t, entitlements.Warnings, "no warnings") +} + +func assertEnterpriseFeatures(t *testing.T, entitlements codersdk.Entitlements) { + for _, expected := range codersdk.FeatureSetEnterprise.Features() { + f := entitlements.Features[expected] + assert.Equalf(t, codersdk.EntitlementEntitled, f.Entitlement, "%s entitled", expected) + assert.Equalf(t, true, f.Enabled, "%s enabled", expected) + } +} diff --git a/enterprise/coderd/license/testdata/README.md b/enterprise/coderd/license/testdata/README.md new file mode 100644 index 0000000000000..f4558f3ce3969 --- /dev/null +++ b/enterprise/coderd/license/testdata/README.md @@ -0,0 +1,13 @@ +# Licensing + +Licensing in Coderd defines what features are allowed to be used by a given deployment. Without a license, or with a license that grants 0 features, Coderd will refuse to execute code paths for some features. These features are typically gated with a middleware that checks the license before allowing the request to proceed. + + +## Terms + +- **Feature**: A specific functionality that Coderd provides, such as external provisioners. Features are defined in the `Feature` enum in [`codersdk/deployment.go`](https://github.com/coder/coder/blob/main/codersdk/deployment.go#L36-L60). A feature can be "entitled", "grace period", or "not entitled". Additionally, a feature can be "disabled" that prevents usage of the feature even if the deployment is entitled to it. Disabled features are a configuration option by the deployment operator. + - **Entitled**: The feature is allowed to be used by the deployment. + - **Grace Period**: The feature is allowed to be used by the deployment, but the license is expired. There is a grace period before the feature is disabled. + - **Not Entitled**: The feature is not allowed to be used by the deployment. Either by expiration, or by not being included in the license. +- **License**: A signed JWT that lists the features that are allowed to be used by a given deployment. A license can have extra properties like, `IsTrial`, `DeploymentIDs`, etc that can be used to further define usage of the license. +- **Entitlements**: A parsed set of licenses, yes you can have more than 1 license on a deployment! Entitlements will enumerate all features that are allowed to be used. From 44078dc6e4240b662fa299554ba6919be7bcf26f Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 16 Jul 2024 14:35:08 -0500 Subject: [PATCH 03/15] add unit tests with variants --- codersdk/deployment.go | 38 ++-- codersdk/deployment_test.go | 236 ++++++++++++++++++++++ enterprise/coderd/license/license_test.go | 86 -------- 3 files changed, 252 insertions(+), 108 deletions(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 2b23e680236b6..a824604cf25a8 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -34,10 +34,6 @@ const ( EntitlementNotEntitled Entitlement = "not_entitled" ) -func CompareEntitlements(a, b Entitlement) int { - return entitlementWeight(a) - entitlementWeight(b) -} - func entitlementWeight(e Entitlement) int { switch e { case EntitlementEntitled: @@ -125,12 +121,12 @@ func (n FeatureName) AlwaysEnable() bool { }[n] } -// FeatureSet represents a grouping of features. This is easier -// than manually assigning features al-la-carte when making a license. -// These sets are dynamic in the sense a feature can be added to an existing -// set to grant an additional feature to an existing license. -// If features were granted al-la-carte, we would need to reissue the license -// to include the new feature. +// FeatureSet represents a grouping of features. Rather than manually +// assigning features al-la-carte when making a license, a set can be specified. +// Sets are dynamic in the sense a feature can be added to an existing +// set, granting the feature to existing licenses. +// If features were granted al-la-carte, we would need to reissue the existing +// licenses to include the new feature. type FeatureSet string const ( @@ -164,7 +160,7 @@ func (set FeatureSet) Features() []FeatureName { } case FeatureSetPremium: // FeatureSetPremium is a superset of Enterprise - return append(FeatureSetEnterprise.Features()) + return append(FeatureSetEnterprise.Features(), FeatureMultipleOrganizations) } // By default, return an empty set. return []FeatureName{} @@ -183,11 +179,11 @@ type Feature struct { // second feature. It is assumed the features are for the same FeatureName. // // A feature is considered greater than another feature if: -// - Graceful & capable > Entitled & not capable -// - The entitlement is greater -// - The limit is greater -// - Enabled is greater than disabled -// - The actual is greater +// 1. Graceful & capable > Entitled & not capable +// 2. The entitlement is greater +// 3. The limit is greater +// 4. Enabled is greater than disabled +// 5. The actual is greater func CompareFeatures(a, b Feature) int { if !a.Capable() || !b.Capable() { // If either is incapable, then it is possible a grace period @@ -204,12 +200,10 @@ func CompareFeatures(a, b Feature) int { } } - entitlement := CompareEntitlements(a.Entitlement, b.Entitlement) - if entitlement > 0 { - return 1 - } - if entitlement < 0 { - return -1 + // Strict entitlement check. Higher is better + entitlementDifference := entitlementWeight(a.Entitlement) - entitlementWeight(b.Entitlement) + if entitlementDifference != 0 { + return entitlementDifference } // If the entitlement is the same, then we can compare the limits. diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index 810dc2539343e..f781aca49588c 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -3,15 +3,18 @@ package codersdk_test import ( "bytes" "embed" + "encoding/json" "fmt" "runtime" "strings" "testing" "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gopkg.in/yaml.v3" + "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" "github.com/coder/serpent" ) @@ -379,3 +382,236 @@ func TestExternalAuthYAMLConfig(t *testing.T) { output := strings.Replace(out.String(), "value:", "externalAuthProviders:", 1) require.Equal(t, inputYAML, output, "re-marshaled is the same as input") } + +type featureVariants struct { + original codersdk.Feature + + variants []codersdk.Feature +} + +func variants(f codersdk.Feature) *featureVariants { + return &featureVariants{original: f} +} + +func (f *featureVariants) Limits() *featureVariants { + f.variant(func(v *codersdk.Feature) { + if v.Limit == nil { + v.Limit = ptr.Ref[int64](100) + return + } + v.Limit = nil + }) + return f +} + +func (f *featureVariants) Actual() *featureVariants { + f.variant(func(v *codersdk.Feature) { + if v.Actual == nil { + v.Actual = ptr.Ref[int64](100) + return + } + v.Actual = nil + }) + return f +} + +func (f *featureVariants) Enabled() *featureVariants { + f.variant(func(v *codersdk.Feature) { + v.Enabled = !v.Enabled + }) + return f +} + +func (f *featureVariants) variant(new func(f *codersdk.Feature)) { + newVariants := make([]codersdk.Feature, 0, len(f.variants)*2) + for _, v := range f.variants { + cpy := v + new(&cpy) + newVariants = append(newVariants, v, cpy) + } +} + +func (f *featureVariants) Features() []codersdk.Feature { + return append([]codersdk.Feature{f.original}, f.variants...) +} + +func TestFeatureComparison(t *testing.T) { + t.Parallel() + + strictEntitlement := func(v codersdk.Feature) []codersdk.Feature { + // Entitlement checks should ignore limits, actuals, and enables + return variants(v).Limits().Actual().Enabled().Features() + } + + testCases := []struct { + Name string + A codersdk.Feature + B codersdk.Feature + Expected int + // To assert variants do not affect the end result, a function can be + // used to generate additional variants of each feature to check. + Variants func(v codersdk.Feature) []codersdk.Feature + }{ + { + Name: "Empty", + Expected: 0, + }, + // Entitlement check + // Entitled + { + Name: "EntitledVsGracePeriod", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementGracePeriod}, + Expected: 1, + Variants: strictEntitlement, + }, + { + Name: "EntitledVsNotEntitled", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementNotEntitled}, + Expected: 3, + Variants: strictEntitlement, + }, + { + Name: "EntitledVsUnknown", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled}, + B: codersdk.Feature{Entitlement: ""}, + Expected: 4, + Variants: strictEntitlement, + }, + // GracePeriod + { + Name: "GracefulVsNotEntitled", + A: codersdk.Feature{Entitlement: codersdk.EntitlementGracePeriod}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementNotEntitled}, + Expected: 2, + Variants: strictEntitlement, + }, + { + Name: "GracefulVsUnknown", + A: codersdk.Feature{Entitlement: codersdk.EntitlementGracePeriod}, + B: codersdk.Feature{Entitlement: ""}, + Expected: 3, + Variants: strictEntitlement, + }, + // NotEntitled + { + Name: "NotEntitledVsUnknown", + A: codersdk.Feature{Entitlement: codersdk.EntitlementNotEntitled}, + B: codersdk.Feature{Entitlement: ""}, + Expected: 1, + Variants: strictEntitlement, + }, + // -- + { + Name: "EntitledVsGracePeriodCapable", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref[int64](100), Actual: ptr.Ref[int64](200)}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementGracePeriod, Limit: ptr.Ref[int64](300), Actual: ptr.Ref[int64](200)}, + Expected: -1, + }, + // UserLimits + { + // Tests an exceeded limit that is entitled vs a graceful limit that + // is not exceeded. This is the edge case that we should use the graceful period + // instead of the entitled. + Name: "UserLimitExceeded", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(200))}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementGracePeriod, Limit: ptr.Ref(int64(300)), Actual: ptr.Ref(int64(200))}, + Expected: -1, + }, + { + Name: "UserLimitExceededNoEntitled", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(200))}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementNotEntitled, Limit: ptr.Ref(int64(300)), Actual: ptr.Ref(int64(200))}, + Expected: 3, + }, + { + Name: "HigherLimit", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(110)), Actual: ptr.Ref(int64(200))}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(200))}, + Expected: 10, // Diff in the limit # + }, + { + Name: "HigherActual", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(300))}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(200))}, + Expected: 100, // Diff in the actual # + }, + { + Name: "LimitExists", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(50))}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: nil, Actual: ptr.Ref(int64(200))}, + Expected: 1, + }, + { + Name: "LimitExistsGrace", + A: codersdk.Feature{Entitlement: codersdk.EntitlementGracePeriod, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(50))}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementGracePeriod, Limit: nil, Actual: ptr.Ref(int64(200))}, + Expected: 1, + }, + { + Name: "ActualExists", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(50))}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: nil}, + Expected: 1, + }, + { + Name: "NotNils", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(50))}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: nil, Actual: nil}, + Expected: 1, + }, + { + // This is super strange, but it is possible to have a limit but no actual. + // Just adding this test case to solidify the behavior. + // Feel free to change this if you think it should be different. + Name: "LimitVsActual", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: nil}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: nil, Actual: ptr.Ref(int64(200))}, + Expected: 1, + }, + } + + for _, tc := range testCases { + tc := tc + + t.Run(tc.Name, func(t *testing.T) { + t.Parallel() + + if tc.Variants == nil { + tc.Variants = func(v codersdk.Feature) []codersdk.Feature { + return []codersdk.Feature{v} + } + } + + VariantLoop: + for i, a := range tc.Variants(tc.A) { + for j, b := range tc.Variants(tc.B) { + r := codersdk.CompareFeatures(a, b) + logIt := !assert.Equalf(t, tc.Expected, r, "variant %d vs %d", i, j) + + // Comparisons should be like addition. A - B = -1 * (B - A) + r = codersdk.CompareFeatures(tc.B, tc.A) + logIt = logIt || !assert.Equalf(t, tc.Expected*-1, r, "the inverse comparison should also be true, variant %d vs %d", j, i) + if logIt { + ad, _ := json.Marshal(a) + bd, _ := json.Marshal(b) + t.Logf("variant %d vs %d\ni = %s\nj = %s", i, j, ad, bd) + // Do not iterate into more variants if the test fails. + break VariantLoop + } + } + } + }) + } +} + +// TestPremiumSuperSet tests that the "premium" feature set is a superset of the +// "enterprise" feature set. +func TestPremiumSuperSet(t *testing.T) { + t.Parallel() + + enterprise := codersdk.FeatureSetEnterprise + premium := codersdk.FeatureSetPremium + require.Subset(t, premium.Features(), enterprise.Features(), "premium should be a superset of enterprise. If this fails, update the premium feature set to include all enterprise features.") +} diff --git a/enterprise/coderd/license/license_test.go b/enterprise/coderd/license/license_test.go index 13184973a9e68..c95166894059f 100644 --- a/enterprise/coderd/license/license_test.go +++ b/enterprise/coderd/license/license_test.go @@ -14,7 +14,6 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbmem" "github.com/coder/coder/v2/coderd/database/dbtime" - "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" "github.com/coder/coder/v2/enterprise/coderd/license" @@ -709,91 +708,6 @@ func TestLicenseEntitlements(t *testing.T) { } } -func TestFeatureComparison(t *testing.T) { - t.Parallel() - - testCases := []struct { - Name string - A codersdk.Feature - B codersdk.Feature - Expected int - }{ - { - Name: "Empty", - Expected: 0, - }, - { - // Tests an exceeded limit that is entitled vs a graceful limit that - // is not exceeded. This is the edge case that we should use the graceful period - // instead of the entitled. - Name: "UserLimitExceeded", - A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(200))}, - B: codersdk.Feature{Entitlement: codersdk.EntitlementGracePeriod, Limit: ptr.Ref(int64(300)), Actual: ptr.Ref(int64(200))}, - Expected: -1, - }, - { - Name: "UserLimitExceededNoEntitled", - A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(200))}, - B: codersdk.Feature{Entitlement: codersdk.EntitlementNotEntitled, Limit: ptr.Ref(int64(300)), Actual: ptr.Ref(int64(200))}, - Expected: 1, - }, - { - Name: "HigherLimit", - A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(110)), Actual: ptr.Ref(int64(200))}, - B: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(200))}, - Expected: 10, // Diff in the limit # - }, - { - Name: "HigherActual", - A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(300))}, - B: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(200))}, - Expected: 100, // Diff in the actual # - }, - { - Name: "LimitExists", - A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(300))}, - B: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: nil, Actual: ptr.Ref(int64(200))}, - Expected: 1, - }, - { - Name: "ActualExists", - A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(300))}, - B: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: nil}, - Expected: 1, - }, - { - Name: "NotNils", - A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(300))}, - B: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: nil, Actual: nil}, - Expected: 1, - }, - { - // This is super strange, but it is possible to have a limit but no actual. - // Just adding this test case to solidify the behavior. - // Feel free to change this if you think it should be different. - Name: "LimitVsActual", - A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: nil}, - B: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: nil, Actual: ptr.Ref(int64(200))}, - Expected: 1, - }, - } - - for _, tc := range testCases { - tc := tc - - t.Run(tc.Name, func(t *testing.T) { - t.Parallel() - - r := codersdk.CompareFeatures(tc.A, tc.B) - assert.Equal(t, tc.Expected, r) - - // Comparisons should be like addition. A - B = -1 * (B - A) - r = codersdk.CompareFeatures(tc.B, tc.A) - assert.Equal(t, tc.Expected*-1, r, "the inverse comparison should also be true") - }) - } -} - func assertNoErrors(t *testing.T, entitlements codersdk.Entitlements) { assert.Empty(t, entitlements.Errors, "no errors") } From 3d234e9ad7a6c94c97a5bfe9166f4db77ef803f7 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 16 Jul 2024 14:52:31 -0500 Subject: [PATCH 04/15] add more unit tests --- enterprise/coderd/license/license_test.go | 120 ++++++++++++++++++++-- 1 file changed, 114 insertions(+), 6 deletions(-) diff --git a/enterprise/coderd/license/license_test.go b/enterprise/coderd/license/license_test.go index c95166894059f..6e03e3f4cf7ee 100644 --- a/enterprise/coderd/license/license_test.go +++ b/enterprise/coderd/license/license_test.go @@ -9,6 +9,7 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/exp/slices" "cdr.dev/slog" "github.com/coder/coder/v2/coderd/database" @@ -357,6 +358,90 @@ func TestEntitlements(t *testing.T) { require.False(t, entitlements.Trial) }) + t.Run("Enterprise", func(t *testing.T) { + t.Parallel() + db := dbmem.New() + _, err := db.InsertLicense(context.Background(), database.InsertLicenseParams{ + Exp: time.Now().Add(time.Hour), + JWT: coderdenttest.GenerateLicense(t, coderdenttest.LicenseOptions{ + FeatureSet: codersdk.FeatureSetEnterprise, + }), + }) + require.NoError(t, err) + entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, all) + require.NoError(t, err) + require.True(t, entitlements.HasLicense) + require.False(t, entitlements.Trial) + + // All enterprise features should be entitled + enterpriseFeatures := codersdk.FeatureSetEnterprise.Features() + for _, featureName := range codersdk.FeatureNames { + if featureName == codersdk.FeatureUserLimit { + continue + } + if slices.Contains(enterpriseFeatures, featureName) { + require.True(t, entitlements.Features[featureName].Enabled, featureName) + require.Equal(t, codersdk.EntitlementEntitled, entitlements.Features[featureName].Entitlement) + } else { + require.False(t, entitlements.Features[featureName].Enabled, featureName) + require.Equal(t, codersdk.EntitlementNotEntitled, entitlements.Features[featureName].Entitlement) + } + } + }) + + t.Run("Premium", func(t *testing.T) { + t.Parallel() + db := dbmem.New() + _, err := db.InsertLicense(context.Background(), database.InsertLicenseParams{ + Exp: time.Now().Add(time.Hour), + JWT: coderdenttest.GenerateLicense(t, coderdenttest.LicenseOptions{ + FeatureSet: codersdk.FeatureSetPremium, + }), + }) + require.NoError(t, err) + entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, all) + require.NoError(t, err) + require.True(t, entitlements.HasLicense) + require.False(t, entitlements.Trial) + + // All premium features should be entitled + enterpriseFeatures := codersdk.FeatureSetPremium.Features() + for _, featureName := range codersdk.FeatureNames { + if featureName == codersdk.FeatureUserLimit { + continue + } + if slices.Contains(enterpriseFeatures, featureName) { + require.True(t, entitlements.Features[featureName].Enabled, featureName) + require.Equal(t, codersdk.EntitlementEntitled, entitlements.Features[featureName].Entitlement) + } else { + require.False(t, entitlements.Features[featureName].Enabled, featureName) + require.Equal(t, codersdk.EntitlementNotEntitled, entitlements.Features[featureName].Entitlement) + } + } + }) + + t.Run("SetNone", func(t *testing.T) { + t.Parallel() + db := dbmem.New() + _, err := db.InsertLicense(context.Background(), database.InsertLicenseParams{ + Exp: time.Now().Add(time.Hour), + JWT: coderdenttest.GenerateLicense(t, coderdenttest.LicenseOptions{ + FeatureSet: "", + }), + }) + require.NoError(t, err) + entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, all) + require.NoError(t, err) + require.True(t, entitlements.HasLicense) + require.False(t, entitlements.Trial) + + for _, featureName := range codersdk.FeatureNames { + require.False(t, entitlements.Features[featureName].Enabled, featureName) + require.Equal(t, codersdk.EntitlementNotEntitled, entitlements.Features[featureName].Entitlement) + } + }) + + // AllFeatures uses the deprecated 'AllFeatures' boolean. t.Run("AllFeatures", func(t *testing.T) { t.Parallel() db := dbmem.New() @@ -370,12 +455,20 @@ func TestEntitlements(t *testing.T) { require.NoError(t, err) require.True(t, entitlements.HasLicense) require.False(t, entitlements.Trial) + + // All enterprise features should be entitled + enterpriseFeatures := codersdk.FeatureSetEnterprise.Features() for _, featureName := range codersdk.FeatureNames { if featureName == codersdk.FeatureUserLimit { continue } - require.True(t, entitlements.Features[featureName].Enabled) - require.Equal(t, codersdk.EntitlementEntitled, entitlements.Features[featureName].Entitlement) + if slices.Contains(enterpriseFeatures, featureName) { + require.True(t, entitlements.Features[featureName].Enabled, featureName) + require.Equal(t, codersdk.EntitlementEntitled, entitlements.Features[featureName].Entitlement) + } else { + require.False(t, entitlements.Features[featureName].Enabled, featureName) + require.Equal(t, codersdk.EntitlementNotEntitled, entitlements.Features[featureName].Entitlement) + } } }) @@ -392,13 +485,21 @@ func TestEntitlements(t *testing.T) { require.NoError(t, err) require.True(t, entitlements.HasLicense) require.False(t, entitlements.Trial) + // All enterprise features should be entitled + enterpriseFeatures := codersdk.FeatureSetEnterprise.Features() for _, featureName := range codersdk.FeatureNames { if featureName == codersdk.FeatureUserLimit { continue } + feature := entitlements.Features[featureName] - require.Equal(t, featureName.AlwaysEnable(), feature.Enabled) - require.Equal(t, codersdk.EntitlementEntitled, feature.Entitlement) + if slices.Contains(enterpriseFeatures, featureName) { + require.Equal(t, featureName.AlwaysEnable(), feature.Enabled) + require.Equal(t, codersdk.EntitlementEntitled, feature.Entitlement) + } else { + require.False(t, entitlements.Features[featureName].Enabled, featureName) + require.Equal(t, codersdk.EntitlementNotEntitled, entitlements.Features[featureName].Entitlement) + } } }) @@ -417,12 +518,19 @@ func TestEntitlements(t *testing.T) { require.NoError(t, err) require.True(t, entitlements.HasLicense) require.False(t, entitlements.Trial) + // All enterprise features should be entitled + enterpriseFeatures := codersdk.FeatureSetEnterprise.Features() for _, featureName := range codersdk.FeatureNames { if featureName == codersdk.FeatureUserLimit { continue } - require.True(t, entitlements.Features[featureName].Enabled) - require.Equal(t, codersdk.EntitlementGracePeriod, entitlements.Features[featureName].Entitlement) + if slices.Contains(enterpriseFeatures, featureName) { + require.True(t, entitlements.Features[featureName].Enabled, featureName) + require.Equal(t, codersdk.EntitlementGracePeriod, entitlements.Features[featureName].Entitlement) + } else { + require.False(t, entitlements.Features[featureName].Enabled, featureName) + require.Equal(t, codersdk.EntitlementNotEntitled, entitlements.Features[featureName].Entitlement) + } } }) From 649d30fa823e062e2f6a0ba3fb1f5d5d8355e99e Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 16 Jul 2024 14:54:47 -0500 Subject: [PATCH 05/15] make fmt --- codersdk/deployment_test.go | 1 + enterprise/coderd/license/license.go | 15 +++------------ enterprise/coderd/license/testdata/README.md | 1 - 3 files changed, 4 insertions(+), 13 deletions(-) diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index f781aca49588c..cba98dc940032 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -429,6 +429,7 @@ func (f *featureVariants) variant(new func(f *codersdk.Feature)) { new(&cpy) newVariants = append(newVariants, v, cpy) } + f.variants = newVariants } func (f *featureVariants) Features() []codersdk.Feature { diff --git a/enterprise/coderd/license/license.go b/enterprise/coderd/license/license.go index 7dcadaeb0ca7d..30c5356427790 100644 --- a/enterprise/coderd/license/license.go +++ b/enterprise/coderd/license/license.go @@ -46,6 +46,9 @@ func Entitlements( ReplicaCount: replicaCount, ExternalAuthCount: externalAuthCount, }) + if err != nil { + return entitlements, err + } return entitlements, nil } @@ -73,7 +76,6 @@ func LicensesEntitlements( keys map[string]ed25519.PublicKey, featureArguments FeatureArguments, ) (codersdk.Entitlements, error) { - // Default all entitlements to be disabled. entitlements := codersdk.Entitlements{ Features: map[codersdk.FeatureName]codersdk.Feature{ @@ -374,17 +376,6 @@ func keyFunc(keys map[string]ed25519.PublicKey) func(*jwt.Token) (interface{}, e } } -// maxEntitlement is the "greater" entitlement between the given values -func maxEntitlement(e1, e2 codersdk.Entitlement) codersdk.Entitlement { - if e1 == codersdk.EntitlementEntitled || e2 == codersdk.EntitlementEntitled { - return codersdk.EntitlementEntitled - } - if e1 == codersdk.EntitlementGracePeriod || e2 == codersdk.EntitlementGracePeriod { - return codersdk.EntitlementGracePeriod - } - return codersdk.EntitlementNotEntitled -} - // licenseExpirationWarning adds a warning message if the license is expiring soon. func licenseExpirationWarning(entitlements *codersdk.Entitlements, now time.Time, claims *Claims) { // Add warning if license is expiring soon diff --git a/enterprise/coderd/license/testdata/README.md b/enterprise/coderd/license/testdata/README.md index f4558f3ce3969..63fa82c5794a8 100644 --- a/enterprise/coderd/license/testdata/README.md +++ b/enterprise/coderd/license/testdata/README.md @@ -2,7 +2,6 @@ Licensing in Coderd defines what features are allowed to be used by a given deployment. Without a license, or with a license that grants 0 features, Coderd will refuse to execute code paths for some features. These features are typically gated with a middleware that checks the license before allowing the request to proceed. - ## Terms - **Feature**: A specific functionality that Coderd provides, such as external provisioners. Features are defined in the `Feature` enum in [`codersdk/deployment.go`](https://github.com/coder/coder/blob/main/codersdk/deployment.go#L36-L60). A feature can be "entitled", "grace period", or "not entitled". Additionally, a feature can be "disabled" that prevents usage of the feature even if the deployment is entitled to it. Disabled features are a configuration option by the deployment operator. From 1ebb3a59a18b8e455c841990824f66aeb0b99469 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 16 Jul 2024 15:24:50 -0500 Subject: [PATCH 06/15] linting --- enterprise/coderd/coderd.go | 2 +- enterprise/coderd/license/license.go | 2 - enterprise/coderd/license/license_test.go | 51 +++++++++++------------ 3 files changed, 26 insertions(+), 29 deletions(-) diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 784695a7ac2e3..080b947ccd598 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -569,7 +569,7 @@ func (api *API) updateEntitlements(ctx context.Context) error { entitlements, err := license.Entitlements( ctx, api.Database, - api.Logger, len(agedReplicas), len(api.ExternalAuthConfigs), api.LicenseKeys, map[codersdk.FeatureName]bool{ + len(agedReplicas), len(api.ExternalAuthConfigs), api.LicenseKeys, map[codersdk.FeatureName]bool{ codersdk.FeatureAuditLog: api.AuditLogging, codersdk.FeatureBrowserOnly: api.BrowserOnly, codersdk.FeatureSCIM: len(api.SCIMAPIKey) != 0, diff --git a/enterprise/coderd/license/license.go b/enterprise/coderd/license/license.go index 30c5356427790..831b3c41cd0e4 100644 --- a/enterprise/coderd/license/license.go +++ b/enterprise/coderd/license/license.go @@ -10,7 +10,6 @@ import ( "github.com/golang-jwt/jwt/v4" "golang.org/x/xerrors" - "cdr.dev/slog" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/codersdk" @@ -20,7 +19,6 @@ import ( func Entitlements( ctx context.Context, db database.Store, - logger slog.Logger, replicaCount int, externalAuthCount int, keys map[string]ed25519.PublicKey, diff --git a/enterprise/coderd/license/license_test.go b/enterprise/coderd/license/license_test.go index 6e03e3f4cf7ee..e9f8a5396aa53 100644 --- a/enterprise/coderd/license/license_test.go +++ b/enterprise/coderd/license/license_test.go @@ -11,7 +11,6 @@ import ( "github.com/stretchr/testify/require" "golang.org/x/exp/slices" - "cdr.dev/slog" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbmem" "github.com/coder/coder/v2/coderd/database/dbtime" @@ -32,7 +31,7 @@ func TestEntitlements(t *testing.T) { t.Run("Defaults", func(t *testing.T) { t.Parallel() db := dbmem.New() - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, all) + entitlements, err := license.Entitlements(context.Background(), db, 1, 1, coderdenttest.Keys, all) require.NoError(t, err) require.False(t, entitlements.HasLicense) require.False(t, entitlements.Trial) @@ -44,7 +43,7 @@ func TestEntitlements(t *testing.T) { t.Run("Always return the current user count", func(t *testing.T) { t.Parallel() db := dbmem.New() - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, all) + entitlements, err := license.Entitlements(context.Background(), db, 1, 1, coderdenttest.Keys, all) require.NoError(t, err) require.False(t, entitlements.HasLicense) require.False(t, entitlements.Trial) @@ -57,7 +56,7 @@ func TestEntitlements(t *testing.T) { JWT: coderdenttest.GenerateLicense(t, coderdenttest.LicenseOptions{}), Exp: time.Now().Add(time.Hour), }) - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, empty) + entitlements, err := license.Entitlements(context.Background(), db, 1, 1, coderdenttest.Keys, empty) require.NoError(t, err) require.True(t, entitlements.HasLicense) require.False(t, entitlements.Trial) @@ -81,7 +80,7 @@ func TestEntitlements(t *testing.T) { }), Exp: time.Now().Add(time.Hour), }) - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, empty) + entitlements, err := license.Entitlements(context.Background(), db, 1, 1, coderdenttest.Keys, empty) require.NoError(t, err) require.True(t, entitlements.HasLicense) require.False(t, entitlements.Trial) @@ -104,7 +103,7 @@ func TestEntitlements(t *testing.T) { }), Exp: time.Now().Add(time.Hour), }) - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, all) + entitlements, err := license.Entitlements(context.Background(), db, 1, 1, coderdenttest.Keys, all) require.NoError(t, err) require.True(t, entitlements.HasLicense) require.False(t, entitlements.Trial) @@ -131,7 +130,7 @@ func TestEntitlements(t *testing.T) { Exp: time.Now().AddDate(0, 0, 5), }) - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, all) + entitlements, err := license.Entitlements(context.Background(), db, 1, 1, coderdenttest.Keys, all) require.NoError(t, err) require.True(t, entitlements.HasLicense) @@ -160,7 +159,7 @@ func TestEntitlements(t *testing.T) { Exp: time.Now().AddDate(0, 0, 5), }) - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, all) + entitlements, err := license.Entitlements(context.Background(), db, 1, 1, coderdenttest.Keys, all) require.NoError(t, err) require.True(t, entitlements.HasLicense) @@ -190,7 +189,7 @@ func TestEntitlements(t *testing.T) { Exp: time.Now().AddDate(0, 0, 5), }) - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, all) + entitlements, err := license.Entitlements(context.Background(), db, 1, 1, coderdenttest.Keys, all) require.NoError(t, err) require.True(t, entitlements.HasLicense) @@ -219,7 +218,7 @@ func TestEntitlements(t *testing.T) { Exp: time.Now().AddDate(0, 0, 5), }) - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, all) + entitlements, err := license.Entitlements(context.Background(), db, 1, 1, coderdenttest.Keys, all) require.NoError(t, err) require.True(t, entitlements.HasLicense) @@ -239,7 +238,7 @@ func TestEntitlements(t *testing.T) { JWT: coderdenttest.GenerateLicense(t, coderdenttest.LicenseOptions{}), Exp: time.Now().Add(time.Hour), }) - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, all) + entitlements, err := license.Entitlements(context.Background(), db, 1, 1, coderdenttest.Keys, all) require.NoError(t, err) require.True(t, entitlements.HasLicense) require.False(t, entitlements.Trial) @@ -301,7 +300,7 @@ func TestEntitlements(t *testing.T) { }), Exp: time.Now().Add(time.Hour), }) - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, empty) + entitlements, err := license.Entitlements(context.Background(), db, 1, 1, coderdenttest.Keys, empty) require.NoError(t, err) require.True(t, entitlements.HasLicense) require.Contains(t, entitlements.Warnings, "Your deployment has 2 active users but is only licensed for 1.") @@ -329,7 +328,7 @@ func TestEntitlements(t *testing.T) { }), Exp: time.Now().Add(60 * 24 * time.Hour), }) - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, empty) + entitlements, err := license.Entitlements(context.Background(), db, 1, 1, coderdenttest.Keys, empty) require.NoError(t, err) require.True(t, entitlements.HasLicense) require.Empty(t, entitlements.Warnings) @@ -352,7 +351,7 @@ func TestEntitlements(t *testing.T) { }), }) - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, empty) + entitlements, err := license.Entitlements(context.Background(), db, 1, 1, coderdenttest.Keys, empty) require.NoError(t, err) require.True(t, entitlements.HasLicense) require.False(t, entitlements.Trial) @@ -368,7 +367,7 @@ func TestEntitlements(t *testing.T) { }), }) require.NoError(t, err) - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, all) + entitlements, err := license.Entitlements(context.Background(), db, 1, 1, coderdenttest.Keys, all) require.NoError(t, err) require.True(t, entitlements.HasLicense) require.False(t, entitlements.Trial) @@ -399,7 +398,7 @@ func TestEntitlements(t *testing.T) { }), }) require.NoError(t, err) - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, all) + entitlements, err := license.Entitlements(context.Background(), db, 1, 1, coderdenttest.Keys, all) require.NoError(t, err) require.True(t, entitlements.HasLicense) require.False(t, entitlements.Trial) @@ -430,7 +429,7 @@ func TestEntitlements(t *testing.T) { }), }) require.NoError(t, err) - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, all) + entitlements, err := license.Entitlements(context.Background(), db, 1, 1, coderdenttest.Keys, all) require.NoError(t, err) require.True(t, entitlements.HasLicense) require.False(t, entitlements.Trial) @@ -451,7 +450,7 @@ func TestEntitlements(t *testing.T) { AllFeatures: true, }), }) - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, all) + entitlements, err := license.Entitlements(context.Background(), db, 1, 1, coderdenttest.Keys, all) require.NoError(t, err) require.True(t, entitlements.HasLicense) require.False(t, entitlements.Trial) @@ -481,7 +480,7 @@ func TestEntitlements(t *testing.T) { AllFeatures: true, }), }) - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, empty) + entitlements, err := license.Entitlements(context.Background(), db, 1, 1, coderdenttest.Keys, empty) require.NoError(t, err) require.True(t, entitlements.HasLicense) require.False(t, entitlements.Trial) @@ -514,7 +513,7 @@ func TestEntitlements(t *testing.T) { ExpiresAt: dbtime.Now().Add(time.Hour), }), }) - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 1, coderdenttest.Keys, all) + entitlements, err := license.Entitlements(context.Background(), db, 1, 1, coderdenttest.Keys, all) require.NoError(t, err) require.True(t, entitlements.HasLicense) require.False(t, entitlements.Trial) @@ -537,7 +536,7 @@ func TestEntitlements(t *testing.T) { t.Run("MultipleReplicasNoLicense", func(t *testing.T) { t.Parallel() db := dbmem.New() - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 2, 1, coderdenttest.Keys, all) + entitlements, err := license.Entitlements(context.Background(), db, 2, 1, coderdenttest.Keys, all) require.NoError(t, err) require.False(t, entitlements.HasLicense) require.Len(t, entitlements.Errors, 1) @@ -555,7 +554,7 @@ func TestEntitlements(t *testing.T) { }, }), }) - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 2, 1, coderdenttest.Keys, map[codersdk.FeatureName]bool{ + entitlements, err := license.Entitlements(context.Background(), db, 2, 1, coderdenttest.Keys, map[codersdk.FeatureName]bool{ codersdk.FeatureHighAvailability: true, }) require.NoError(t, err) @@ -577,7 +576,7 @@ func TestEntitlements(t *testing.T) { }), Exp: time.Now().Add(time.Hour), }) - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 2, 1, coderdenttest.Keys, map[codersdk.FeatureName]bool{ + entitlements, err := license.Entitlements(context.Background(), db, 2, 1, coderdenttest.Keys, map[codersdk.FeatureName]bool{ codersdk.FeatureHighAvailability: true, }) require.NoError(t, err) @@ -589,7 +588,7 @@ func TestEntitlements(t *testing.T) { t.Run("MultipleGitAuthNoLicense", func(t *testing.T) { t.Parallel() db := dbmem.New() - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 2, coderdenttest.Keys, all) + entitlements, err := license.Entitlements(context.Background(), db, 1, 2, coderdenttest.Keys, all) require.NoError(t, err) require.False(t, entitlements.HasLicense) require.Len(t, entitlements.Errors, 1) @@ -607,7 +606,7 @@ func TestEntitlements(t *testing.T) { }, }), }) - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 2, coderdenttest.Keys, map[codersdk.FeatureName]bool{ + entitlements, err := license.Entitlements(context.Background(), db, 1, 2, coderdenttest.Keys, map[codersdk.FeatureName]bool{ codersdk.FeatureMultipleExternalAuth: true, }) require.NoError(t, err) @@ -629,7 +628,7 @@ func TestEntitlements(t *testing.T) { }), Exp: time.Now().Add(time.Hour), }) - entitlements, err := license.Entitlements(context.Background(), db, slog.Logger{}, 1, 2, coderdenttest.Keys, map[codersdk.FeatureName]bool{ + entitlements, err := license.Entitlements(context.Background(), db, 1, 2, coderdenttest.Keys, map[codersdk.FeatureName]bool{ codersdk.FeatureMultipleExternalAuth: true, }) require.NoError(t, err) From 021fb9bcb126371704eca0b52a1bf639f5da7d79 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 16 Jul 2024 15:38:14 -0500 Subject: [PATCH 07/15] remove variants --- codersdk/deployment_test.go | 98 +++-------------------- enterprise/coderd/license/license_test.go | 5 ++ 2 files changed, 14 insertions(+), 89 deletions(-) diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index cba98dc940032..80497ce7db6ba 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -383,75 +383,14 @@ func TestExternalAuthYAMLConfig(t *testing.T) { require.Equal(t, inputYAML, output, "re-marshaled is the same as input") } -type featureVariants struct { - original codersdk.Feature - - variants []codersdk.Feature -} - -func variants(f codersdk.Feature) *featureVariants { - return &featureVariants{original: f} -} - -func (f *featureVariants) Limits() *featureVariants { - f.variant(func(v *codersdk.Feature) { - if v.Limit == nil { - v.Limit = ptr.Ref[int64](100) - return - } - v.Limit = nil - }) - return f -} - -func (f *featureVariants) Actual() *featureVariants { - f.variant(func(v *codersdk.Feature) { - if v.Actual == nil { - v.Actual = ptr.Ref[int64](100) - return - } - v.Actual = nil - }) - return f -} - -func (f *featureVariants) Enabled() *featureVariants { - f.variant(func(v *codersdk.Feature) { - v.Enabled = !v.Enabled - }) - return f -} - -func (f *featureVariants) variant(new func(f *codersdk.Feature)) { - newVariants := make([]codersdk.Feature, 0, len(f.variants)*2) - for _, v := range f.variants { - cpy := v - new(&cpy) - newVariants = append(newVariants, v, cpy) - } - f.variants = newVariants -} - -func (f *featureVariants) Features() []codersdk.Feature { - return append([]codersdk.Feature{f.original}, f.variants...) -} - func TestFeatureComparison(t *testing.T) { t.Parallel() - strictEntitlement := func(v codersdk.Feature) []codersdk.Feature { - // Entitlement checks should ignore limits, actuals, and enables - return variants(v).Limits().Actual().Enabled().Features() - } - testCases := []struct { Name string A codersdk.Feature B codersdk.Feature Expected int - // To assert variants do not affect the end result, a function can be - // used to generate additional variants of each feature to check. - Variants func(v codersdk.Feature) []codersdk.Feature }{ { Name: "Empty", @@ -464,21 +403,18 @@ func TestFeatureComparison(t *testing.T) { A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled}, B: codersdk.Feature{Entitlement: codersdk.EntitlementGracePeriod}, Expected: 1, - Variants: strictEntitlement, }, { Name: "EntitledVsNotEntitled", A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled}, B: codersdk.Feature{Entitlement: codersdk.EntitlementNotEntitled}, Expected: 3, - Variants: strictEntitlement, }, { Name: "EntitledVsUnknown", A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled}, B: codersdk.Feature{Entitlement: ""}, Expected: 4, - Variants: strictEntitlement, }, // GracePeriod { @@ -486,14 +422,12 @@ func TestFeatureComparison(t *testing.T) { A: codersdk.Feature{Entitlement: codersdk.EntitlementGracePeriod}, B: codersdk.Feature{Entitlement: codersdk.EntitlementNotEntitled}, Expected: 2, - Variants: strictEntitlement, }, { Name: "GracefulVsUnknown", A: codersdk.Feature{Entitlement: codersdk.EntitlementGracePeriod}, B: codersdk.Feature{Entitlement: ""}, Expected: 3, - Variants: strictEntitlement, }, // NotEntitled { @@ -501,7 +435,6 @@ func TestFeatureComparison(t *testing.T) { A: codersdk.Feature{Entitlement: codersdk.EntitlementNotEntitled}, B: codersdk.Feature{Entitlement: ""}, Expected: 1, - Variants: strictEntitlement, }, // -- { @@ -579,29 +512,16 @@ func TestFeatureComparison(t *testing.T) { t.Run(tc.Name, func(t *testing.T) { t.Parallel() - if tc.Variants == nil { - tc.Variants = func(v codersdk.Feature) []codersdk.Feature { - return []codersdk.Feature{v} - } - } + r := codersdk.CompareFeatures(tc.A, tc.B) + logIt := !assert.Equal(t, tc.Expected, r) - VariantLoop: - for i, a := range tc.Variants(tc.A) { - for j, b := range tc.Variants(tc.B) { - r := codersdk.CompareFeatures(a, b) - logIt := !assert.Equalf(t, tc.Expected, r, "variant %d vs %d", i, j) - - // Comparisons should be like addition. A - B = -1 * (B - A) - r = codersdk.CompareFeatures(tc.B, tc.A) - logIt = logIt || !assert.Equalf(t, tc.Expected*-1, r, "the inverse comparison should also be true, variant %d vs %d", j, i) - if logIt { - ad, _ := json.Marshal(a) - bd, _ := json.Marshal(b) - t.Logf("variant %d vs %d\ni = %s\nj = %s", i, j, ad, bd) - // Do not iterate into more variants if the test fails. - break VariantLoop - } - } + // Comparisons should be like addition. A - B = -1 * (B - A) + r = codersdk.CompareFeatures(tc.B, tc.A) + logIt = logIt || !assert.Equalf(t, tc.Expected*-1, r, "the inverse comparison should also be true") + if logIt { + ad, _ := json.Marshal(tc.A) + bd, _ := json.Marshal(tc.B) + t.Logf("a = %s\nb = %s", ad, bd) } }) } diff --git a/enterprise/coderd/license/license_test.go b/enterprise/coderd/license/license_test.go index e9f8a5396aa53..bb875eedfc2ad 100644 --- a/enterprise/coderd/license/license_test.go +++ b/enterprise/coderd/license/license_test.go @@ -740,6 +740,11 @@ func TestLicenseEntitlements(t *testing.T) { userFeature := entitlements.Features[codersdk.FeatureUserLimit] assert.Equalf(t, int64(100), *userFeature.Limit, "user limit") assert.Equalf(t, int64(200), *userFeature.Actual, "user count") + + require.Len(t, entitlements.Errors, 1, "invalid license error") + require.Len(t, entitlements.Warnings, 1, "user count exceeds warning") + require.Contains(t, entitlements.Errors[0], "Invalid license") + require.Contains(t, entitlements.Warnings[0], "active users but is only licensed for") }, }, { From 6fc3959d1e6b54fc7f2f77f9404855ffbee0c059 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 16 Jul 2024 15:39:07 -0500 Subject: [PATCH 08/15] Update unit tests --- codersdk/deployment_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index 80497ce7db6ba..a040381b47e04 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -404,6 +404,13 @@ func TestFeatureComparison(t *testing.T) { B: codersdk.Feature{Entitlement: codersdk.EntitlementGracePeriod}, Expected: 1, }, + { + Name: "EntitledVsGracePeriodLimits", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled}, + // Entitled should still win here + B: codersdk.Feature{Entitlement: codersdk.EntitlementGracePeriod, Limit: ptr.Ref[int64](100), Actual: ptr.Ref[int64](50)}, + Expected: 1, + }, { Name: "EntitledVsNotEntitled", A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled}, From 1bc8288ea9413b47e1af885edcc70159d3f0176e Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 17 Jul 2024 10:11:52 -0500 Subject: [PATCH 09/15] move readme to doc.go --- codersdk/deployment.go | 12 +++++--- codersdk/deployment_test.go | 15 +++++---- enterprise/coderd/license/doc.go | 32 ++++++++++++++++++++ enterprise/coderd/license/license.go | 2 +- enterprise/coderd/license/testdata/README.md | 12 -------- 5 files changed, 49 insertions(+), 24 deletions(-) create mode 100644 enterprise/coderd/license/doc.go delete mode 100644 enterprise/coderd/license/testdata/README.md diff --git a/codersdk/deployment.go b/codersdk/deployment.go index a824604cf25a8..c07080d81e2fb 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -34,6 +34,8 @@ const ( EntitlementNotEntitled Entitlement = "not_entitled" ) +// entitlementWeight converts the enum types to a numerical value for easier +// comparisons. Easier than sets of if statements. func entitlementWeight(e Entitlement) int { switch e { case EntitlementEntitled: @@ -123,10 +125,10 @@ func (n FeatureName) AlwaysEnable() bool { // FeatureSet represents a grouping of features. Rather than manually // assigning features al-la-carte when making a license, a set can be specified. -// Sets are dynamic in the sense a feature can be added to an existing -// set, granting the feature to existing licenses. +// Sets are dynamic in the sense a feature can be added to a set, granting the +// feature to existing licenses out in the wild. // If features were granted al-la-carte, we would need to reissue the existing -// licenses to include the new feature. +// old licenses to include the new feature. type FeatureSet string const ( @@ -215,7 +217,7 @@ func CompareFeatures(a, b Feature) int { } if a.Limit != nil && b.Limit != nil { difference := *a.Limit - *b.Limit - if *a.Limit-*b.Limit != 0 { + if difference != 0 { return int(difference) } } @@ -237,7 +239,7 @@ func CompareFeatures(a, b Feature) int { } if a.Actual != nil && b.Actual != nil { difference := *a.Actual - *b.Actual - if *a.Actual-*b.Actual != 0 { + if difference != 0 { return int(difference) } } diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index a040381b47e04..1db96cb03e82d 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -503,12 +503,15 @@ func TestFeatureComparison(t *testing.T) { Expected: 1, }, { - // This is super strange, but it is possible to have a limit but no actual. - // Just adding this test case to solidify the behavior. - // Feel free to change this if you think it should be different. - Name: "LimitVsActual", - A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: nil}, - B: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: nil, Actual: ptr.Ref(int64(200))}, + Name: "EnabledVsDisabled", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Enabled: true, Limit: ptr.Ref(int64(300)), Actual: ptr.Ref(int64(200))}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(300)), Actual: ptr.Ref(int64(200))}, + Expected: 1, + }, + { + Name: "NotNils", + A: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: ptr.Ref(int64(100)), Actual: ptr.Ref(int64(50))}, + B: codersdk.Feature{Entitlement: codersdk.EntitlementEntitled, Limit: nil, Actual: nil}, Expected: 1, }, } diff --git a/enterprise/coderd/license/doc.go b/enterprise/coderd/license/doc.go new file mode 100644 index 0000000000000..d806c02107089 --- /dev/null +++ b/enterprise/coderd/license/doc.go @@ -0,0 +1,32 @@ +// Package license provides the license parsing and validation logic for Coderd. +// Licensing in Coderd defines what features are allowed to be used in a +// given deployment. Without a license, or with a license that grants 0 features, +// Coderd will refuse to execute some feature code paths. These features are +// typically gated with a middleware that checks the license before allowing +// the http request to proceed. +// +// Terms: +// - FeatureName: A specific functionality that Coderd provides, such as +// external provisioners. +// +// - Feature: Entitlement definition for a FeatureName. A feature can be: +// - "entitled": The feature is allowed to be used by the deployment. +// - "grace period": The feature is allowed to be used by the deployment, +// but the license is expired. There is a grace period +// before the feature is disabled. +// - "not entitled": The deployment is not allowed to use the feature. +// Either by expiration, or by not being included +// in the license. +// A feature can also be "disabled" that prevents usage of the feature +// even if entitled. This is usually a deployment configuration option. +// +// - License: A signed JWT that lists the features that are allowed to be used by +// a given deployment. A license can have extra properties like, +// `IsTrial`, `DeploymentIDs`, etc that can be used to further define +// usage of the license. +/**/ +// - Entitlements: A parsed set of licenses. Yes you can have more than 1 license +// on a deployment! Entitlements will enumerate all features that +// are allowed to be used. +// +package license diff --git a/enterprise/coderd/license/license.go b/enterprise/coderd/license/license.go index 831b3c41cd0e4..fdb177d753eae 100644 --- a/enterprise/coderd/license/license.go +++ b/enterprise/coderd/license/license.go @@ -64,7 +64,7 @@ type FeatureArguments struct { // now: The time to use for checking license expiration. // license: The license to check. // enablements: Features can be explicitly disabled by the deployment even if -// the license has the feature entitled. Features can also have +// the license has the feature entitled. Features can also have // the 'feat.AlwaysEnable()' return true to disallow disabling. // featureArguments: Additional arguments required by specific features. func LicensesEntitlements( diff --git a/enterprise/coderd/license/testdata/README.md b/enterprise/coderd/license/testdata/README.md deleted file mode 100644 index 63fa82c5794a8..0000000000000 --- a/enterprise/coderd/license/testdata/README.md +++ /dev/null @@ -1,12 +0,0 @@ -# Licensing - -Licensing in Coderd defines what features are allowed to be used by a given deployment. Without a license, or with a license that grants 0 features, Coderd will refuse to execute code paths for some features. These features are typically gated with a middleware that checks the license before allowing the request to proceed. - -## Terms - -- **Feature**: A specific functionality that Coderd provides, such as external provisioners. Features are defined in the `Feature` enum in [`codersdk/deployment.go`](https://github.com/coder/coder/blob/main/codersdk/deployment.go#L36-L60). A feature can be "entitled", "grace period", or "not entitled". Additionally, a feature can be "disabled" that prevents usage of the feature even if the deployment is entitled to it. Disabled features are a configuration option by the deployment operator. - - **Entitled**: The feature is allowed to be used by the deployment. - - **Grace Period**: The feature is allowed to be used by the deployment, but the license is expired. There is a grace period before the feature is disabled. - - **Not Entitled**: The feature is not allowed to be used by the deployment. Either by expiration, or by not being included in the license. -- **License**: A signed JWT that lists the features that are allowed to be used by a given deployment. A license can have extra properties like, `IsTrial`, `DeploymentIDs`, etc that can be used to further define usage of the license. -- **Entitlements**: A parsed set of licenses, yes you can have more than 1 license on a deployment! Entitlements will enumerate all features that are allowed to be used. From 96f0b2d1bb02717f7a571e0f23bb19bfb83b33ce Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 18 Jul 2024 11:43:18 -0500 Subject: [PATCH 10/15] premium license has multi-org enabled --- codersdk/deployment.go | 8 +++-- enterprise/coderd/coderd.go | 1 - enterprise/coderd/license/license_test.go | 37 ++++++++++++++++++++++- 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index c07080d81e2fb..7e84f4737f26a 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -110,8 +110,11 @@ func (n FeatureName) Humanize() string { } // AlwaysEnable returns if the feature is always enabled if entitled. -// Warning: We don't know if we need this functionality. -// This method may disappear at any time. +// This is required because some features are only enabled if they are entitled +// and not required. +// E.g: "multiple-organizations" is disabled by default in AGPL and enterprise +// deployments. This feature should only be enabled for premium deployments +// when it is entitled. func (n FeatureName) AlwaysEnable() bool { return map[FeatureName]bool{ FeatureMultipleExternalAuth: true, @@ -120,6 +123,7 @@ func (n FeatureName) AlwaysEnable() bool { FeatureWorkspaceBatchActions: true, FeatureHighAvailability: true, FeatureCustomRoles: true, + FeatureMultipleOrganizations: true, }[n] } diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 080b947ccd598..6b0ba0b70f4a8 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -582,7 +582,6 @@ func (api *API) updateEntitlements(ctx context.Context) error { codersdk.FeatureUserRoleManagement: true, codersdk.FeatureAccessControl: true, codersdk.FeatureControlSharedPorts: true, - codersdk.FeatureMultipleOrganizations: true, }) if err != nil { return err diff --git a/enterprise/coderd/license/license_test.go b/enterprise/coderd/license/license_test.go index bb875eedfc2ad..5089b33c022fa 100644 --- a/enterprise/coderd/license/license_test.go +++ b/enterprise/coderd/license/license_test.go @@ -679,7 +679,18 @@ func TestLicenseEntitlements(t *testing.T) { DeploymentIDs: nil, Trial: false, FeatureSet: codersdk.FeatureSetEnterprise, - AllFeatures: false, + AllFeatures: true, + }).Valid(time.Now()) + } + + premiumLicense := func() *coderdenttest.LicenseOptions { + return (&coderdenttest.LicenseOptions{ + AccountType: "salesforce", + AccountID: "Charlie", + DeploymentIDs: nil, + Trial: false, + FeatureSet: codersdk.FeatureSetPremium, + AllFeatures: true, }).Valid(time.Now()) } @@ -789,6 +800,30 @@ func TestLicenseEntitlements(t *testing.T) { assert.Equalf(t, int64(50), *userFeature.Actual, "user count") }, }, + { + Name: "EnterpriseDisabledMultiOrg", + Licenses: []*coderdenttest.LicenseOptions{ + enterpriseLicense().UserLimit(100), + }, + Enablements: defaultEnablements, + Arguments: license.FeatureArguments{}, + ExpectedErrorContains: "", + AssertEntitlements: func(t *testing.T, entitlements codersdk.Entitlements) { + assert.False(t, entitlements.Features[codersdk.FeatureMultipleOrganizations].Enabled, "multi-org only enabled for premium") + }, + }, + { + Name: "PremiumEnabledMultiOrg", + Licenses: []*coderdenttest.LicenseOptions{ + premiumLicense().UserLimit(100), + }, + Enablements: defaultEnablements, + Arguments: license.FeatureArguments{}, + ExpectedErrorContains: "", + AssertEntitlements: func(t *testing.T, entitlements codersdk.Entitlements) { + assert.True(t, entitlements.Features[codersdk.FeatureMultipleOrganizations].Enabled, "multi-org enabled for premium") + }, + }, } for _, tc := range testCases { From 41c272615819fb5c35ddfd74b0c9fbcbc7251b16 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 24 Jul 2024 10:54:07 -0500 Subject: [PATCH 11/15] make enterprise = All - [multi-org] --- codersdk/deployment.go | 38 ++++++++++++++++++------------------- codersdk/deployment_test.go | 15 +++++++++++++++ 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 7e84f4737f26a..4a4c81a9909eb 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -9,6 +9,7 @@ import ( "os" "path/filepath" "reflect" + "slices" "strconv" "strings" "time" @@ -144,26 +145,23 @@ const ( func (set FeatureSet) Features() []FeatureName { switch FeatureSet(strings.ToLower(string(set))) { case FeatureSetEnterprise: - // List all features that should be included in the Enterprise feature set. - return []FeatureName{ - FeatureUserLimit, - FeatureAuditLog, - FeatureBrowserOnly, - FeatureSCIM, - FeatureTemplateRBAC, - FeatureHighAvailability, - FeatureMultipleExternalAuth, - FeatureExternalProvisionerDaemons, - FeatureAppearance, - FeatureAdvancedTemplateScheduling, - FeatureWorkspaceProxy, - FeatureUserRoleManagement, - FeatureExternalTokenEncryption, - FeatureWorkspaceBatchActions, - FeatureAccessControl, - FeatureControlSharedPorts, - FeatureCustomRoles, - } + // Enterprise is the set 'AllFeatures' minus some select features. + + // Copy the list of all features + enterpriseFeatures := make([]FeatureName, len(FeatureNames)) + copy(enterpriseFeatures, FeatureNames) + // Remove the selection + enterpriseFeatures = slices.DeleteFunc(enterpriseFeatures, func(f FeatureName) bool { + switch f { + // Add all features that should be excluded in the Enterprise feature set. + case FeatureMultipleOrganizations: + return true + default: + return false + } + }) + + return enterpriseFeatures case FeatureSetPremium: // FeatureSetPremium is a superset of Enterprise return append(FeatureSetEnterprise.Features(), FeatureMultipleOrganizations) diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index 1db96cb03e82d..12f522aba90e9 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -544,5 +544,20 @@ func TestPremiumSuperSet(t *testing.T) { enterprise := codersdk.FeatureSetEnterprise premium := codersdk.FeatureSetPremium + + // Premium > Enterprise + require.Greater(t, len(premium.Features()), len(enterprise.Features()), "premium should have more features than enterprise") + + // Premium ⊃ Enterprise require.Subset(t, premium.Features(), enterprise.Features(), "premium should be a superset of enterprise. If this fails, update the premium feature set to include all enterprise features.") + + // Premium = All Features + // This is currently true. If this assertion changes, update this test + // to reflect the change in feature sets. + require.ElementsMatch(t, premium.Features(), codersdk.FeatureNames, "premium should contain all features") + + // This check exists because if you misuse the slices.Delete, you can end up + // with zero'd values. + require.NotContains(t, enterprise.Features(), "", "enterprise should not contain empty string") + require.NotContains(t, premium.Features(), "", "premium should not contain empty string") } From e9f1aa9a94b5f148db685653003b3ff465c3cabb Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 24 Jul 2024 10:56:34 -0500 Subject: [PATCH 12/15] make entitlement weight a method on the enum --- codersdk/deployment.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 4a4c81a9909eb..4c272fb8c3734 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -35,9 +35,9 @@ const ( EntitlementNotEntitled Entitlement = "not_entitled" ) -// entitlementWeight converts the enum types to a numerical value for easier +// Weight converts the enum types to a numerical value for easier // comparisons. Easier than sets of if statements. -func entitlementWeight(e Entitlement) int { +func (e Entitlement) Weight() int { switch e { case EntitlementEntitled: return 2 @@ -194,7 +194,7 @@ func CompareFeatures(a, b Feature) int { // feature can be "greater" than an entitled. // If either is "NotEntitled" then we can defer to a strict entitlement // check. - if entitlementWeight(a.Entitlement) >= 0 && entitlementWeight(b.Entitlement) >= 0 { + if a.Entitlement.Weight() >= 0 && b.Entitlement.Weight() >= 0 { if a.Capable() && !b.Capable() { return 1 } @@ -205,7 +205,7 @@ func CompareFeatures(a, b Feature) int { } // Strict entitlement check. Higher is better - entitlementDifference := entitlementWeight(a.Entitlement) - entitlementWeight(b.Entitlement) + entitlementDifference := a.Entitlement.Weight() - b.Entitlement.Weight() if entitlementDifference != 0 { return entitlementDifference } From 1c4a305a9aad472074d2dce8fb02d76931c3b3b1 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 24 Jul 2024 10:58:03 -0500 Subject: [PATCH 13/15] premium = All --- codersdk/deployment.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 4c272fb8c3734..dca5dea5cfe66 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -163,8 +163,10 @@ func (set FeatureSet) Features() []FeatureName { return enterpriseFeatures case FeatureSetPremium: - // FeatureSetPremium is a superset of Enterprise - return append(FeatureSetEnterprise.Features(), FeatureMultipleOrganizations) + premiumFeatures := make([]FeatureName, len(FeatureNames)) + copy(premiumFeatures, FeatureNames) + // FeatureSetPremium is just all features. + return premiumFeatures } // By default, return an empty set. return []FeatureName{} From 20aa6b4b38a2864274c1a8001bc7da36cdf1db6d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 24 Jul 2024 11:44:33 -0500 Subject: [PATCH 14/15] move CompareFeatures to a method on the struct --- codersdk/deployment.go | 12 ++++++------ codersdk/deployment_test.go | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index dca5dea5cfe66..6af2201e8d49a 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -179,10 +179,10 @@ type Feature struct { Actual *int64 `json:"actual,omitempty"` } -// CompareFeatures compares two features and returns an integer representing -// if the first feature is greater than, equal to, or less than the second feature. -// "Greater than" means the first feature has more functionality than the -// second feature. It is assumed the features are for the same FeatureName. +// Compare compares two features and returns an integer representing +// if the first feature (a) is greater than, equal to, or less than the second +// feature (b). "Greater than" means the first feature has more functionality +// than the second feature. It is assumed the features are for the same FeatureName. // // A feature is considered greater than another feature if: // 1. Graceful & capable > Entitled & not capable @@ -190,7 +190,7 @@ type Feature struct { // 3. The limit is greater // 4. Enabled is greater than disabled // 5. The actual is greater -func CompareFeatures(a, b Feature) int { +func (a Feature) Compare(b Feature) int { if !a.Capable() || !b.Capable() { // If either is incapable, then it is possible a grace period // feature can be "greater" than an entitled. @@ -288,7 +288,7 @@ func (e *Entitlements) AddFeature(name FeatureName, add Feature) { } // Compare the features, keep the one that is "better" - comparison := CompareFeatures(add, existing) + comparison := add.Compare(existing) if comparison > 0 { e.Features[name] = add return diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index 12f522aba90e9..b84eda1f7250b 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -522,11 +522,11 @@ func TestFeatureComparison(t *testing.T) { t.Run(tc.Name, func(t *testing.T) { t.Parallel() - r := codersdk.CompareFeatures(tc.A, tc.B) + r := tc.A.Compare(tc.B) logIt := !assert.Equal(t, tc.Expected, r) // Comparisons should be like addition. A - B = -1 * (B - A) - r = codersdk.CompareFeatures(tc.B, tc.A) + r = tc.B.Compare(tc.A) logIt = logIt || !assert.Equalf(t, tc.Expected*-1, r, "the inverse comparison should also be true") if logIt { ad, _ := json.Marshal(tc.A) From ef707cfc20755a135111280d15ca654a6b3c5d91 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 24 Jul 2024 11:58:01 -0500 Subject: [PATCH 15/15] linting --- codersdk/deployment.go | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 6af2201e8d49a..9099c26b5ab03 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -180,7 +180,7 @@ type Feature struct { } // Compare compares two features and returns an integer representing -// if the first feature (a) is greater than, equal to, or less than the second +// if the first feature (f) is greater than, equal to, or less than the second // feature (b). "Greater than" means the first feature has more functionality // than the second feature. It is assumed the features are for the same FeatureName. // @@ -190,59 +190,59 @@ type Feature struct { // 3. The limit is greater // 4. Enabled is greater than disabled // 5. The actual is greater -func (a Feature) Compare(b Feature) int { - if !a.Capable() || !b.Capable() { +func (f Feature) Compare(b Feature) int { + if !f.Capable() || !b.Capable() { // If either is incapable, then it is possible a grace period // feature can be "greater" than an entitled. // If either is "NotEntitled" then we can defer to a strict entitlement // check. - if a.Entitlement.Weight() >= 0 && b.Entitlement.Weight() >= 0 { - if a.Capable() && !b.Capable() { + if f.Entitlement.Weight() >= 0 && b.Entitlement.Weight() >= 0 { + if f.Capable() && !b.Capable() { return 1 } - if b.Capable() && !a.Capable() { + if b.Capable() && !f.Capable() { return -1 } } } // Strict entitlement check. Higher is better - entitlementDifference := a.Entitlement.Weight() - b.Entitlement.Weight() + entitlementDifference := f.Entitlement.Weight() - b.Entitlement.Weight() if entitlementDifference != 0 { return entitlementDifference } // If the entitlement is the same, then we can compare the limits. - if a.Limit == nil && b.Limit != nil { + if f.Limit == nil && b.Limit != nil { return -1 } - if a.Limit != nil && b.Limit == nil { + if f.Limit != nil && b.Limit == nil { return 1 } - if a.Limit != nil && b.Limit != nil { - difference := *a.Limit - *b.Limit + if f.Limit != nil && b.Limit != nil { + difference := *f.Limit - *b.Limit if difference != 0 { return int(difference) } } // Enabled is better than disabled. - if a.Enabled && !b.Enabled { + if f.Enabled && !b.Enabled { return 1 } - if !a.Enabled && b.Enabled { + if !f.Enabled && b.Enabled { return -1 } // Higher actual is better - if a.Actual == nil && b.Actual != nil { + if f.Actual == nil && b.Actual != nil { return -1 } - if a.Actual != nil && b.Actual == nil { + if f.Actual != nil && b.Actual == nil { return 1 } - if a.Actual != nil && b.Actual != nil { - difference := *a.Actual - *b.Actual + if f.Actual != nil && b.Actual != nil { + difference := *f.Actual - *b.Actual if difference != 0 { return int(difference) }