Skip to content

feat(site): add annotation to display values of type clibase.Duration correctly #10667

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 15, 2023
Merged

feat(site): add annotation to display values of type clibase.Duration correctly #10667

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8166f57
feat(site): correctly display values of type clibase.Duration
johnstcn Nov 14, 2023
d29e21f
fixup! feat(site): correctly display values of type clibase.Duration
johnstcn Nov 14, 2023
6900281
rename format_duration_ns -> format_duration
johnstcn Nov 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions codersdk/deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,8 +403,9 @@ type HealthcheckConfig struct {
}

const (
annotationEnterpriseKey = "enterprise"
annotationSecretKey = "secret"
annotationFormatDuration = "format_duration"
annotationEnterpriseKey = "enterprise"
annotationSecretKey = "secret"
// annotationExternalProxies is used to mark options that are used by workspace
// proxies. This is used to filter out options that are not relevant.
annotationExternalProxies = "external_workspace_proxies"
Expand Down Expand Up @@ -630,6 +631,7 @@ when required by your organization's security policy.`,
Default: time.Minute.String(),
Value: &c.AutobuildPollInterval,
YAML: "autobuildPollInterval",
Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"),
},
{
Name: "Job Hang Detector Interval",
Expand All @@ -640,6 +642,7 @@ when required by your organization's security policy.`,
Default: time.Minute.String(),
Value: &c.JobHangDetectorInterval,
YAML: "jobHangDetectorInterval",
Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"),
},
httpAddress,
tlsBindAddress,
Expand Down Expand Up @@ -1333,6 +1336,7 @@ when required by your organization's security policy.`,
Value: &c.Provisioner.DaemonPollInterval,
Group: &deploymentGroupProvisioning,
YAML: "daemonPollInterval",
Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"),
},
{
Name: "Poll Jitter",
Expand All @@ -1343,6 +1347,7 @@ when required by your organization's security policy.`,
Value: &c.Provisioner.DaemonPollJitter,
Group: &deploymentGroupProvisioning,
YAML: "daemonPollJitter",
Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"),
},
{
Name: "Force Cancel Interval",
Expand All @@ -1353,6 +1358,7 @@ when required by your organization's security policy.`,
Value: &c.Provisioner.ForceCancelInterval,
Group: &deploymentGroupProvisioning,
YAML: "forceCancelInterval",
Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"),
},
{
Name: "Provisioner Daemon Pre-shared Key (PSK)",
Expand Down Expand Up @@ -1502,10 +1508,11 @@ when required by your organization's security policy.`,
// The default value is essentially "forever", so just use 100 years.
// We have to add in the 25 leap days for the frontend to show the
// "100 years" correctly.
Default: ((100 * 365 * time.Hour * 24) + (25 * time.Hour * 24)).String(),
Value: &c.MaxTokenLifetime,
Group: &deploymentGroupNetworkingHTTP,
YAML: "maxTokenLifetime",
Default: ((100 * 365 * time.Hour * 24) + (25 * time.Hour * 24)).String(),
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: this value currently does not display correctly (screenshot from dev.coder.com):

image

We can modify this in a separate PR but I would consider it out of scope here.

Value: &c.MaxTokenLifetime,
Group: &deploymentGroupNetworkingHTTP,
YAML: "maxTokenLifetime",
Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"),
},
{
Name: "Enable swagger endpoint",
Expand Down Expand Up @@ -1613,6 +1620,7 @@ when required by your organization's security policy.`,
Hidden: true,
Default: time.Hour.String(),
Value: &c.MetricsCacheRefreshInterval,
Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"),
},
{
Name: "Agent Stat Refresh Interval",
Expand All @@ -1622,6 +1630,7 @@ when required by your organization's security policy.`,
Hidden: true,
Default: (30 * time.Second).String(),
Value: &c.AgentStatRefreshInterval,
Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"),
},
{
Name: "Agent Fallback Troubleshooting URL",
Expand Down Expand Up @@ -1688,6 +1697,7 @@ when required by your organization's security policy.`,
Value: &c.SessionDuration,
Group: &deploymentGroupNetworkingHTTP,
YAML: "sessionDuration",
Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"),
},
{
Name: "Disable Session Expiry Refresh",
Expand Down Expand Up @@ -1790,6 +1800,7 @@ Write out the current server config as YAML to stdout.`,
Value: &c.ProxyHealthStatusInterval,
Group: &deploymentGroupNetworkingHTTP,
YAML: "proxyHealthInterval",
Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"),
},
{
Name: "Default Quiet Hours Schedule",
Expand Down Expand Up @@ -1821,6 +1832,7 @@ Write out the current server config as YAML to stdout.`,
Value: &c.Healthcheck.Refresh,
Group: &deploymentGroupIntrospectionHealthcheck,
YAML: "refresh",
Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"),
},
{
Name: "Health Check Threshold: Database",
Expand All @@ -1831,6 +1843,7 @@ Write out the current server config as YAML to stdout.`,
Value: &c.Healthcheck.ThresholdDatabase,
Group: &deploymentGroupIntrospectionHealthcheck,
YAML: "thresholdDatabase",
Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"),
},
}

Expand Down
21 changes: 21 additions & 0 deletions codersdk/deployment_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,3 +256,24 @@ func must[T any](value T, err error) T {
}
return value
}

func TestDeploymentValues_DurationFormatNanoseconds(t *testing.T) {
t.Parallel()

set := (&codersdk.DeploymentValues{}).Options()
for _, s := range set {
if s.Value.Type() != "duration" {
continue
}
// Just make sure the annotation is set.
// If someone wants to not format a duration, they can
// explicitly set the annotation to false.
if s.Annotations.IsSet("format_duration") {
continue
}
t.Logf("Option %q is a duration but does not have the format_duration annotation.", s.Name)
t.Logf("To fix this, add the following to the option declaration:")
t.Logf(`Annotations: clibase.Annotations{}.Mark(annotationFormatDurationNS, "true"),`)
t.FailNow()
}
}
26 changes: 24 additions & 2 deletions site/src/components/DeploySettingsLayout/optionValue.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,16 @@ describe("optionValue", () => {
...defaultOption,
name: "Max Token Lifetime",
value: 3600 * 1e9,
annotations: { format_duration: "false" },
},
expected: "1 hour",
expected: 3600000000000,
},
{
option: {
...defaultOption,
name: "Max Token Lifetime",
value: 24 * 3600 * 1e9,
annotations: { format_duration: "true" },
},
expected: "1 day",
},
Expand All @@ -37,14 +39,16 @@ describe("optionValue", () => {
...defaultOption,
name: "Session Duration",
value: 3600 * 1e9,
annotations: { format_duration: "some non-truthful value" },
},
expected: "1 hour",
expected: 3600000000000,
},
{
option: {
...defaultOption,
name: "Session Duration",
value: 24 * 3600 * 1e9,
annotations: { format_duration: "true" },
},
expected: "1 day",
},
Expand Down Expand Up @@ -104,6 +108,24 @@ describe("optionValue", () => {
additionalValues: ["single_tailnet", "deployment_health_page"],
expected: { single_tailnet: true, deployment_health_page: true },
},
{
option: {
...defaultOption,
name: "Some Go Duration We Want To Show As A String",
value: 30 * 1e9,
annotations: { format_duration: "true" },
},
expected: "30 seconds",
},
{
option: {
...defaultOption,
name: "Some Other Go Duration We Want To Just Display As A Number",
value: 30 * 1e9,
annotations: { format_duration: "false" },
},
expected: 30000000000,
},
])(
`[$option.name]optionValue($option.value)`,
({ option, expected, additionalValues }) => {
Expand Down
27 changes: 21 additions & 6 deletions site/src/components/DeploySettingsLayout/optionValue.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,28 @@ export function optionValue(
option: ClibaseOption,
additionalValues?: string[],
) {
// If option annotations are present, use them to format the value.
if (option.annotations) {
for (const [k, v] of Object.entries(option.annotations)) {
if (v !== "true") {
continue; // skip if not explicitly true
}
switch (k) {
case "format_duration":
return formatDuration(
// intervalToDuration takes ms, so convert nanoseconds to ms
intervalToDuration({
start: 0,
end: (option.value as number) / 1e6,
}),
);
// Add additional cases here as needed.
}
}
}

// If no format annotations are present, use the option name to format the value.
switch (option.name) {
case "Max Token Lifetime":
case "Session Duration":
// intervalToDuration takes ms, so convert nanoseconds to ms
return formatDuration(
intervalToDuration({ start: 0, end: (option.value as number) / 1e6 }),
);
case "Strict-Transport-Security":
if (option.value === 0) {
return "Disabled";
Expand Down