From 8166f573f48672af7d0acd77a123adf2a471f62e Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 14 Nov 2023 11:59:57 +0000 Subject: [PATCH 1/3] feat(site): correctly display values of type clibase.Duration --- codersdk/deployment.go | 25 ++++++++++++++----- codersdk/deployment_test.go | 21 ++++++++++++++++ .../DeploySettingsLayout/optionValue.test.ts | 18 +++++++++++++ .../DeploySettingsLayout/optionValue.ts | 24 +++++++++++++----- 4 files changed, 76 insertions(+), 12 deletions(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 613e3b17045fa..5b12eb691a4a8 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -403,8 +403,9 @@ type HealthcheckConfig struct { } const ( - annotationEnterpriseKey = "enterprise" - annotationSecretKey = "secret" + annotationFormatDurationNS = "format_duration_ns" + 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" @@ -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(annotationFormatDurationNS, "true"), }, { Name: "Job Hang Detector Interval", @@ -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(annotationFormatDurationNS, "true"), }, httpAddress, tlsBindAddress, @@ -1333,6 +1336,7 @@ when required by your organization's security policy.`, Value: &c.Provisioner.DaemonPollInterval, Group: &deploymentGroupProvisioning, YAML: "daemonPollInterval", + Annotations: clibase.Annotations{}.Mark(annotationFormatDurationNS, "true"), }, { Name: "Poll Jitter", @@ -1343,6 +1347,7 @@ when required by your organization's security policy.`, Value: &c.Provisioner.DaemonPollJitter, Group: &deploymentGroupProvisioning, YAML: "daemonPollJitter", + Annotations: clibase.Annotations{}.Mark(annotationFormatDurationNS, "true"), }, { Name: "Force Cancel Interval", @@ -1353,6 +1358,7 @@ when required by your organization's security policy.`, Value: &c.Provisioner.ForceCancelInterval, Group: &deploymentGroupProvisioning, YAML: "forceCancelInterval", + Annotations: clibase.Annotations{}.Mark(annotationFormatDurationNS, "true"), }, { Name: "Provisioner Daemon Pre-shared Key (PSK)", @@ -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(), + Value: &c.MaxTokenLifetime, + Group: &deploymentGroupNetworkingHTTP, + YAML: "maxTokenLifetime", + Annotations: clibase.Annotations{}.Mark(annotationFormatDurationNS, "true"), }, { Name: "Enable swagger endpoint", @@ -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(annotationFormatDurationNS, "true"), }, { Name: "Agent Stat Refresh Interval", @@ -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(annotationFormatDurationNS, "true"), }, { Name: "Agent Fallback Troubleshooting URL", @@ -1688,6 +1697,7 @@ when required by your organization's security policy.`, Value: &c.SessionDuration, Group: &deploymentGroupNetworkingHTTP, YAML: "sessionDuration", + Annotations: clibase.Annotations{}.Mark(annotationFormatDurationNS, "true"), }, { Name: "Disable Session Expiry Refresh", @@ -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(annotationFormatDurationNS, "true"), }, { Name: "Default Quiet Hours Schedule", @@ -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(annotationFormatDurationNS, "true"), }, { Name: "Health Check Threshold: Database", @@ -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(annotationFormatDurationNS, "true"), }, } diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index 5bf9c98543849..af38a7b1c6003 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -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_ns") { + continue + } + t.Logf("Option %q is a duration but does not have the format_duration_ns 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() + } +} diff --git a/site/src/components/DeploySettingsLayout/optionValue.test.ts b/site/src/components/DeploySettingsLayout/optionValue.test.ts index f9797b98f60d2..ba4c93f673206 100644 --- a/site/src/components/DeploySettingsLayout/optionValue.test.ts +++ b/site/src/components/DeploySettingsLayout/optionValue.test.ts @@ -104,6 +104,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_ns": "true" }, + }, + expected: "30s", + }, + { + option: { + ...defaultOption, + name: "Some Other Go Duration We Want To Just Display As A Number", + value: 30 * 1e9, + annotations: { "format_duration_ns": "false" }, + }, + expected: "30000000000" + }, ])( `[$option.name]optionValue($option.value)`, ({ option, expected, additionalValues }) => { diff --git a/site/src/components/DeploySettingsLayout/optionValue.ts b/site/src/components/DeploySettingsLayout/optionValue.ts index 6221356075fbb..1bd2b0c8b8a5c 100644 --- a/site/src/components/DeploySettingsLayout/optionValue.ts +++ b/site/src/components/DeploySettingsLayout/optionValue.ts @@ -6,13 +6,25 @@ 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_ns": + 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"; From d29e21f2ae0f89c1a815e4fc93425027d85b50ff Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 14 Nov 2023 12:30:26 +0000 Subject: [PATCH 2/3] fixup! feat(site): correctly display values of type clibase.Duration --- .../DeploySettingsLayout/optionValue.test.ts | 16 ++++++++++------ .../DeploySettingsLayout/optionValue.ts | 9 ++++++--- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/site/src/components/DeploySettingsLayout/optionValue.test.ts b/site/src/components/DeploySettingsLayout/optionValue.test.ts index ba4c93f673206..15496e418e8b1 100644 --- a/site/src/components/DeploySettingsLayout/optionValue.test.ts +++ b/site/src/components/DeploySettingsLayout/optionValue.test.ts @@ -21,14 +21,16 @@ describe("optionValue", () => { ...defaultOption, name: "Max Token Lifetime", value: 3600 * 1e9, + annotations: { format_duration_ns: "false" }, }, - expected: "1 hour", + expected: 3600000000000, }, { option: { ...defaultOption, name: "Max Token Lifetime", value: 24 * 3600 * 1e9, + annotations: { format_duration_ns: "true" }, }, expected: "1 day", }, @@ -37,14 +39,16 @@ describe("optionValue", () => { ...defaultOption, name: "Session Duration", value: 3600 * 1e9, + annotations: { format_duration_ns: "falsae" }, }, - expected: "1 hour", + expected: 3600000000000, }, { option: { ...defaultOption, name: "Session Duration", value: 24 * 3600 * 1e9, + annotations: { format_duration_ns: "true" }, }, expected: "1 day", }, @@ -109,18 +113,18 @@ describe("optionValue", () => { ...defaultOption, name: "Some Go Duration We Want To Show As A String", value: 30 * 1e9, - annotations: { "format_duration_ns": "true" }, + annotations: { format_duration_ns: "true" }, }, - expected: "30s", + expected: "30 seconds", }, { option: { ...defaultOption, name: "Some Other Go Duration We Want To Just Display As A Number", value: 30 * 1e9, - annotations: { "format_duration_ns": "false" }, + annotations: { format_duration_ns: "false" }, }, - expected: "30000000000" + expected: 30000000000, }, ])( `[$option.name]optionValue($option.value)`, diff --git a/site/src/components/DeploySettingsLayout/optionValue.ts b/site/src/components/DeploySettingsLayout/optionValue.ts index 1bd2b0c8b8a5c..fc164c5a528cd 100644 --- a/site/src/components/DeploySettingsLayout/optionValue.ts +++ b/site/src/components/DeploySettingsLayout/optionValue.ts @@ -16,11 +16,14 @@ export function optionValue( case "format_duration_ns": return formatDuration( // intervalToDuration takes ms, so convert nanoseconds to ms - intervalToDuration({ start: 0, end: option.value as number / 1e6 }), - ) + 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. From 6900281a03b40a8fce20962d3b191ca6dfea7007 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 15 Nov 2023 11:58:44 +0000 Subject: [PATCH 3/3] rename format_duration_ns -> format_duration --- codersdk/deployment.go | 30 +++++++++---------- codersdk/deployment_test.go | 4 +-- .../DeploySettingsLayout/optionValue.test.ts | 12 ++++---- .../DeploySettingsLayout/optionValue.ts | 2 +- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 5b12eb691a4a8..68fcac1a91a76 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -403,9 +403,9 @@ type HealthcheckConfig struct { } const ( - annotationFormatDurationNS = "format_duration_ns" - 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" @@ -631,7 +631,7 @@ when required by your organization's security policy.`, Default: time.Minute.String(), Value: &c.AutobuildPollInterval, YAML: "autobuildPollInterval", - Annotations: clibase.Annotations{}.Mark(annotationFormatDurationNS, "true"), + Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"), }, { Name: "Job Hang Detector Interval", @@ -642,7 +642,7 @@ when required by your organization's security policy.`, Default: time.Minute.String(), Value: &c.JobHangDetectorInterval, YAML: "jobHangDetectorInterval", - Annotations: clibase.Annotations{}.Mark(annotationFormatDurationNS, "true"), + Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"), }, httpAddress, tlsBindAddress, @@ -1336,7 +1336,7 @@ when required by your organization's security policy.`, Value: &c.Provisioner.DaemonPollInterval, Group: &deploymentGroupProvisioning, YAML: "daemonPollInterval", - Annotations: clibase.Annotations{}.Mark(annotationFormatDurationNS, "true"), + Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"), }, { Name: "Poll Jitter", @@ -1347,7 +1347,7 @@ when required by your organization's security policy.`, Value: &c.Provisioner.DaemonPollJitter, Group: &deploymentGroupProvisioning, YAML: "daemonPollJitter", - Annotations: clibase.Annotations{}.Mark(annotationFormatDurationNS, "true"), + Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"), }, { Name: "Force Cancel Interval", @@ -1358,7 +1358,7 @@ when required by your organization's security policy.`, Value: &c.Provisioner.ForceCancelInterval, Group: &deploymentGroupProvisioning, YAML: "forceCancelInterval", - Annotations: clibase.Annotations{}.Mark(annotationFormatDurationNS, "true"), + Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"), }, { Name: "Provisioner Daemon Pre-shared Key (PSK)", @@ -1512,7 +1512,7 @@ when required by your organization's security policy.`, Value: &c.MaxTokenLifetime, Group: &deploymentGroupNetworkingHTTP, YAML: "maxTokenLifetime", - Annotations: clibase.Annotations{}.Mark(annotationFormatDurationNS, "true"), + Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"), }, { Name: "Enable swagger endpoint", @@ -1620,7 +1620,7 @@ when required by your organization's security policy.`, Hidden: true, Default: time.Hour.String(), Value: &c.MetricsCacheRefreshInterval, - Annotations: clibase.Annotations{}.Mark(annotationFormatDurationNS, "true"), + Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"), }, { Name: "Agent Stat Refresh Interval", @@ -1630,7 +1630,7 @@ when required by your organization's security policy.`, Hidden: true, Default: (30 * time.Second).String(), Value: &c.AgentStatRefreshInterval, - Annotations: clibase.Annotations{}.Mark(annotationFormatDurationNS, "true"), + Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"), }, { Name: "Agent Fallback Troubleshooting URL", @@ -1697,7 +1697,7 @@ when required by your organization's security policy.`, Value: &c.SessionDuration, Group: &deploymentGroupNetworkingHTTP, YAML: "sessionDuration", - Annotations: clibase.Annotations{}.Mark(annotationFormatDurationNS, "true"), + Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"), }, { Name: "Disable Session Expiry Refresh", @@ -1800,7 +1800,7 @@ Write out the current server config as YAML to stdout.`, Value: &c.ProxyHealthStatusInterval, Group: &deploymentGroupNetworkingHTTP, YAML: "proxyHealthInterval", - Annotations: clibase.Annotations{}.Mark(annotationFormatDurationNS, "true"), + Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"), }, { Name: "Default Quiet Hours Schedule", @@ -1832,7 +1832,7 @@ Write out the current server config as YAML to stdout.`, Value: &c.Healthcheck.Refresh, Group: &deploymentGroupIntrospectionHealthcheck, YAML: "refresh", - Annotations: clibase.Annotations{}.Mark(annotationFormatDurationNS, "true"), + Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"), }, { Name: "Health Check Threshold: Database", @@ -1843,7 +1843,7 @@ Write out the current server config as YAML to stdout.`, Value: &c.Healthcheck.ThresholdDatabase, Group: &deploymentGroupIntrospectionHealthcheck, YAML: "thresholdDatabase", - Annotations: clibase.Annotations{}.Mark(annotationFormatDurationNS, "true"), + Annotations: clibase.Annotations{}.Mark(annotationFormatDuration, "true"), }, } diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index af38a7b1c6003..3a7d32d7ada57 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -268,10 +268,10 @@ func TestDeploymentValues_DurationFormatNanoseconds(t *testing.T) { // 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_ns") { + if s.Annotations.IsSet("format_duration") { continue } - t.Logf("Option %q is a duration but does not have the format_duration_ns annotation.", s.Name) + 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() diff --git a/site/src/components/DeploySettingsLayout/optionValue.test.ts b/site/src/components/DeploySettingsLayout/optionValue.test.ts index 15496e418e8b1..387917723e00f 100644 --- a/site/src/components/DeploySettingsLayout/optionValue.test.ts +++ b/site/src/components/DeploySettingsLayout/optionValue.test.ts @@ -21,7 +21,7 @@ describe("optionValue", () => { ...defaultOption, name: "Max Token Lifetime", value: 3600 * 1e9, - annotations: { format_duration_ns: "false" }, + annotations: { format_duration: "false" }, }, expected: 3600000000000, }, @@ -30,7 +30,7 @@ describe("optionValue", () => { ...defaultOption, name: "Max Token Lifetime", value: 24 * 3600 * 1e9, - annotations: { format_duration_ns: "true" }, + annotations: { format_duration: "true" }, }, expected: "1 day", }, @@ -39,7 +39,7 @@ describe("optionValue", () => { ...defaultOption, name: "Session Duration", value: 3600 * 1e9, - annotations: { format_duration_ns: "falsae" }, + annotations: { format_duration: "some non-truthful value" }, }, expected: 3600000000000, }, @@ -48,7 +48,7 @@ describe("optionValue", () => { ...defaultOption, name: "Session Duration", value: 24 * 3600 * 1e9, - annotations: { format_duration_ns: "true" }, + annotations: { format_duration: "true" }, }, expected: "1 day", }, @@ -113,7 +113,7 @@ describe("optionValue", () => { ...defaultOption, name: "Some Go Duration We Want To Show As A String", value: 30 * 1e9, - annotations: { format_duration_ns: "true" }, + annotations: { format_duration: "true" }, }, expected: "30 seconds", }, @@ -122,7 +122,7 @@ describe("optionValue", () => { ...defaultOption, name: "Some Other Go Duration We Want To Just Display As A Number", value: 30 * 1e9, - annotations: { format_duration_ns: "false" }, + annotations: { format_duration: "false" }, }, expected: 30000000000, }, diff --git a/site/src/components/DeploySettingsLayout/optionValue.ts b/site/src/components/DeploySettingsLayout/optionValue.ts index fc164c5a528cd..6df38820f75a1 100644 --- a/site/src/components/DeploySettingsLayout/optionValue.ts +++ b/site/src/components/DeploySettingsLayout/optionValue.ts @@ -13,7 +13,7 @@ export function optionValue( continue; // skip if not explicitly true } switch (k) { - case "format_duration_ns": + case "format_duration": return formatDuration( // intervalToDuration takes ms, so convert nanoseconds to ms intervalToDuration({