Skip to content

Commit 657b390

Browse files
deansheatherdeansheather
committed
PR comments and additional changes
1 parent 777bcf2 commit 657b390

File tree

3 files changed

+122
-96
lines changed

3 files changed

+122
-96
lines changed

‎codersdk/deployment.go

Lines changed: 28 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ var (
119119

120120
// FeatureNamesMap is a map of all feature names for quick lookups.
121121
FeatureNamesMap = func() map[FeatureName]struct{} {
122-
featureNamesMap := make(map[FeatureName]struct{})
122+
featureNamesMap := make(map[FeatureName]struct{}, len(FeatureNames))
123123
for _, featureName := range FeatureNames {
124124
featureNamesMap[featureName] = struct{}{}
125125
}
@@ -222,9 +222,6 @@ func (set FeatureSet) Features() []FeatureName {
222222
})
223223
// FeatureSetPremium is just all features.
224224
return premiumFeatures
225-
case FeatureSetNone:
226-
default:
227-
panic("unexpected codersdk.FeatureSet")
228225
}
229226
// By default, return an empty set.
230227
return []FeatureName{}
@@ -242,7 +239,7 @@ type Feature struct {
242239
// included limits in the dashboard. No license validation or warnings are
243240
// generated from this value.
244241
SoftLimit *int64 `json:"soft_limit,omitempty"`
245-
// Usage period denotes that the usage is a counter that accumulates over
242+
// UsagePeriod denotes that the usage is a counter that accumulates over
246243
// this period (and most likely resets with the issuance of the next
247244
// license).
248245
//
@@ -251,9 +248,13 @@ type Feature struct {
251248
//
252249
// Only certain features set these fields:
253250
// - FeatureManagedAgentLimit
254-
UsagePeriodIssuedAt *time.Time `json:"usage_period_issued_at,omitempty" format:"date-time"`
255-
UsagePeriodStart *time.Time `json:"usage_period_start,omitempty" format:"date-time"`
256-
UsagePeriodEnd *time.Time `json:"usage_period_end,omitempty" format:"date-time"`
251+
UsagePeriod *UsagePeriod `json:"usage_period,omitempty"`
252+
}
253+
254+
type UsagePeriod struct {
255+
IssuedAt time.Time `json:"issued_at" format:"date-time"`
256+
Start time.Time `json:"start" format:"date-time"`
257+
End time.Time `json:"end" format:"date-time"`
257258
}
258259

259260
// Compare compares two features and returns an integer representing
@@ -262,14 +263,28 @@ type Feature struct {
262263
// than the second feature. It is assumed the features are for the same FeatureName.
263264
//
264265
// A feature is considered greater than another feature if:
265-
// 1. Graceful & capable > Entitled & not capable
266+
// 1. The usage period has a greater issued at date (note: only certain features use usage periods)
266267
// 2. The usage period has a greater end date (note: only certain features use usage periods)
267-
// 3. The usage period has a greater issued at date (note: only certain features use usage periods)
268+
// 3. Graceful & capable > Entitled & not capable (only if both have "Actual" values)
268269
// 4. The entitlement is greater
269270
// 5. The limit is greater
270271
// 6. Enabled is greater than disabled
271272
// 7. The actual is greater
272273
func (f Feature) Compare(b Feature) int {
274+
// For features with usage period constraints only, check the issued at and
275+
// end dates.
276+
bothHaveUsagePeriod := f.UsagePeriod != nil && b.UsagePeriod != nil
277+
if bothHaveUsagePeriod {
278+
issuedAtCmp := f.UsagePeriod.IssuedAt.Compare(b.UsagePeriod.IssuedAt)
279+
if issuedAtCmp != 0 {
280+
return issuedAtCmp
281+
}
282+
endCmp := f.UsagePeriod.End.Compare(b.UsagePeriod.End)
283+
if endCmp != 0 {
284+
return endCmp
285+
}
286+
}
287+
273288
// Only perform capability comparisons if both features have actual values.
274289
if f.Actual != nil && b.Actual != nil && (!f.Capable() || !b.Capable()) {
275290
// If either is incapable, then it is possible a grace period
@@ -289,26 +304,11 @@ func (f Feature) Compare(b Feature) int {
289304
// Strict entitlement check. Higher is better. We don't apply this check for
290305
// usage period features as we always want the issued at date to be the main
291306
// decision maker.
292-
bothHaveIssuedAt := f.UsagePeriodIssuedAt != nil && b.UsagePeriodIssuedAt != nil
293307
entitlementDifference := f.Entitlement.Weight() - b.Entitlement.Weight()
294-
if !bothHaveIssuedAt && entitlementDifference != 0 {
308+
if entitlementDifference != 0 {
295309
return entitlementDifference
296310
}
297311

298-
// For features with usage period constraints only:
299-
if bothHaveIssuedAt {
300-
cmp := f.UsagePeriodIssuedAt.Compare(*b.UsagePeriodIssuedAt)
301-
if cmp != 0 {
302-
return cmp
303-
}
304-
}
305-
if f.UsagePeriodEnd != nil && b.UsagePeriodEnd != nil {
306-
cmp := f.UsagePeriodEnd.Compare(*b.UsagePeriodEnd)
307-
if cmp != 0 {
308-
return cmp
309-
}
310-
}
311-
312312
// If the entitlement is the same, then we can compare the limits.
313313
if f.Limit == nil && b.Limit != nil {
314314
return -1
@@ -394,14 +394,11 @@ func (e *Entitlements) AddFeature(name FeatureName, add Feature) {
394394
// If we're trying to add a feature that uses a usage period and it's not
395395
// set, then we should not add it.
396396
if name.UsesUsagePeriod() {
397-
if add.UsagePeriodIssuedAt == nil || add.UsagePeriodStart == nil || add.UsagePeriodEnd == nil {
397+
if add.UsagePeriod == nil || add.UsagePeriod.IssuedAt.IsZero() || add.UsagePeriod.Start.IsZero() || add.UsagePeriod.End.IsZero() {
398398
return
399399
}
400400
} else {
401-
// Ensure the usage period values are not set.
402-
add.UsagePeriodIssuedAt = nil
403-
add.UsagePeriodStart = nil
404-
add.UsagePeriodEnd = nil
401+
add.UsagePeriod = nil
405402
}
406403

407404
// Compare the features, keep the one that is "better"

‎enterprise/coderd/license/license.go

Lines changed: 60 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,17 @@ import (
1717
)
1818

1919
const (
20+
// These features are only included in the license and are not actually
21+
// entitlements after the licenses are processed. These values will be
22+
// merged into the codersdk.FeatureManagedAgentLimit feature.
23+
//
24+
// The reason we need two separate features is because the License v3 format
25+
// uses map[string]int64 for features, so we're unable to use a single value
26+
// with a struct like `{"soft": 100, "hard": 200}`. This is unfortunate and
27+
// we should fix this with a new license format v4 in the future.
28+
//
29+
// These are intentionally not exported as they should not be used outside
30+
// of this package (except tests).
2031
featureManagedAgentLimitHard codersdk.FeatureName = "managed_agent_limit_hard"
2132
featureManagedAgentLimitSoft codersdk.FeatureName = "managed_agent_limit_soft"
2233
)
@@ -88,8 +99,9 @@ func Entitlements(
8899
ActiveUserCount: activeUserCount,
89100
ReplicaCount: replicaCount,
90101
ExternalAuthCount: externalAuthCount,
91-
ManagedAgentCountFn: func(ctx context.Context, from time.Time, to time.Time) (int64, error) {
92-
// TODO: this
102+
ManagedAgentCountFn: func(_ context.Context, _ time.Time, _ time.Time) (int64, error) {
103+
// TODO(@deansheather): replace this with a real implementation in a
104+
// follow up PR.
93105
return 0, nil
94106
},
95107
})
@@ -107,9 +119,11 @@ type FeatureArguments struct {
107119
// Unfortunately, managed agent count is not a simple count of the current
108120
// state of the world, but a count between two points in time determined by
109121
// the licenses.
110-
ManagedAgentCountFn func(ctx context.Context, from time.Time, to time.Time) (int64, error)
122+
ManagedAgentCountFn ManagedAgentCountFn
111123
}
112124

125+
type ManagedAgentCountFn func(ctx context.Context, from time.Time, to time.Time) (int64, error)
126+
113127
// LicensesEntitlements returns the entitlements for licenses. Entitlements are
114128
// merged from all licenses and the highest entitlement is used for each feature.
115129
// Arguments:
@@ -297,13 +311,15 @@ func LicensesEntitlements(
297311
defaultHardAgentLimit = 1000 * featureValue
298312
)
299313
entitlements.AddFeature(codersdk.FeatureManagedAgentLimit, codersdk.Feature{
300-
Enabled: true,
301-
Entitlement: entitlement,
302-
SoftLimit: &defaultSoftAgentLimit,
303-
Limit: &defaultHardAgentLimit,
304-
UsagePeriodIssuedAt: &issueTime,
305-
UsagePeriodStart: &usagePeriodStart,
306-
UsagePeriodEnd: &usagePeriodEnd,
314+
Enabled: true,
315+
Entitlement: entitlement,
316+
SoftLimit: &defaultSoftAgentLimit,
317+
Limit: &defaultHardAgentLimit,
318+
UsagePeriod: &codersdk.UsagePeriod{
319+
IssuedAt: issueTime,
320+
Start: usagePeriodStart,
321+
End: usagePeriodEnd,
322+
},
307323
})
308324
}
309325
default:
@@ -342,11 +358,12 @@ func LicensesEntitlements(
342358
Entitlement: entitlement,
343359
SoftLimit: ul.Soft,
344360
Limit: ul.Hard,
345-
// Actual value will be populated below when warnings are
346-
// generated.
347-
UsagePeriodIssuedAt: &claims.IssuedAt.Time,
348-
UsagePeriodStart: &usagePeriodStart,
349-
UsagePeriodEnd: &usagePeriodEnd,
361+
// `Actual` will be populated below when warnings are generated.
362+
UsagePeriod: &codersdk.UsagePeriod{
363+
IssuedAt: claims.IssuedAt.Time,
364+
Start: usagePeriodStart,
365+
End: usagePeriodEnd,
366+
},
350367
}
351368
// If the hard limit is 0, the feature is disabled.
352369
if *ul.Hard <= 0 {
@@ -404,15 +421,15 @@ func LicensesEntitlements(
404421
// generate a warning if the license actually has managed agents.
405422
// Note that agents are free when unlicensed.
406423
agentLimit := entitlements.Features[codersdk.FeatureManagedAgentLimit]
407-
if entitlements.HasLicense && agentLimit.UsagePeriodStart != nil && agentLimit.UsagePeriodEnd != nil {
424+
if entitlements.HasLicense && agentLimit.UsagePeriod != nil {
408425
// Calculate the amount of agents between the usage period start and
409426
// end.
410427
var (
411428
managedAgentCount int64
412429
err = xerrors.New("dev error: managed agent count function is not set")
413430
)
414431
if featureArguments.ManagedAgentCountFn != nil {
415-
managedAgentCount, err = featureArguments.ManagedAgentCountFn(ctx, *agentLimit.UsagePeriodStart, *agentLimit.UsagePeriodEnd)
432+
managedAgentCount, err = featureArguments.ManagedAgentCountFn(ctx, agentLimit.UsagePeriod.Start, agentLimit.UsagePeriod.End)
416433
}
417434
if err != nil {
418435
entitlements.Errors = append(entitlements.Errors,
@@ -421,30 +438,33 @@ func LicensesEntitlements(
421438
agentLimit.Actual = &managedAgentCount
422439
entitlements.AddFeature(codersdk.FeatureManagedAgentLimit, agentLimit)
423440

424-
var softLimit int64
425-
if agentLimit.SoftLimit != nil {
426-
softLimit = *agentLimit.SoftLimit
427-
}
428-
var hardLimit int64
429-
if agentLimit.Limit != nil {
430-
hardLimit = *agentLimit.Limit
431-
}
441+
// Only issue warnings if the feature is enabled.
442+
if agentLimit.Enabled {
443+
var softLimit int64
444+
if agentLimit.SoftLimit != nil {
445+
softLimit = *agentLimit.SoftLimit
446+
}
447+
var hardLimit int64
448+
if agentLimit.Limit != nil {
449+
hardLimit = *agentLimit.Limit
450+
}
432451

433-
// Issue a warning early:
434-
// 1. If the soft limit and hard limit are equal, at 75% of the hard
435-
// limit.
436-
// 2. If the limit is greater than the soft limit, at 75% of the
437-
// difference between the hard limit and the soft limit.
438-
softWarningThreshold := int64(float64(hardLimit) * 0.75)
439-
if hardLimit > softLimit && softLimit > 0 {
440-
softWarningThreshold = softLimit + int64(float64(hardLimit-softLimit)*0.75)
441-
}
442-
if managedAgentCount >= *agentLimit.Limit {
443-
entitlements.Warnings = append(entitlements.Warnings,
444-
"You have built more workspaces with managed agents than your license allows. Further managed agent builds will be blocked.")
445-
} else if managedAgentCount >= softWarningThreshold {
446-
entitlements.Warnings = append(entitlements.Warnings,
447-
"You are approaching the managed agent limit in your license. Please refer to the Deployment Licenses page for more information.")
452+
// Issue a warning early:
453+
// 1. If the soft limit and hard limit are equal, at 75% of the hard
454+
// limit.
455+
// 2. If the limit is greater than the soft limit, at 75% of the
456+
// difference between the hard limit and the soft limit.
457+
softWarningThreshold := int64(float64(hardLimit) * 0.75)
458+
if hardLimit > softLimit && softLimit > 0 {
459+
softWarningThreshold = softLimit + int64(float64(hardLimit-softLimit)*0.75)
460+
}
461+
if managedAgentCount >= *agentLimit.Limit {
462+
entitlements.Warnings = append(entitlements.Warnings,
463+
"You have built more workspaces with managed agents than your license allows. Further managed agent builds will be blocked.")
464+
} else if managedAgentCount >= softWarningThreshold {
465+
entitlements.Warnings = append(entitlements.Warnings,
466+
"You are approaching the managed agent limit in your license. Please refer to the Deployment Licenses page for more information.")
467+
}
448468
}
449469
}
450470
}

0 commit comments

Comments
 (0)