From b635ae48ff5742e6857e749f58d3a7659e473313 Mon Sep 17 00:00:00 2001 From: "blink-so[bot]" <211532188+blink-so[bot]@users.noreply.github.com> Date: Wed, 4 Jun 2025 11:02:13 +0000 Subject: [PATCH 1/5] docs: clarify cron attribute format for coder_script resource - Update cron description to specify 6-field format (seconds minutes hours day month day-of-week) - Fix misleading example that used 5-field Unix format - Add additional cron examples with proper 6-field format - Include inline comments explaining the schedule Fixes #353 --- docs/resources/script.md | 19 +++++++++++++++---- examples/resources/coder_script/resource.tf | 6 +++--- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/docs/resources/script.md b/docs/resources/script.md index 22ac1b50..452a6f3b 100644 --- a/docs/resources/script.md +++ b/docs/resources/script.md @@ -43,15 +43,26 @@ resource "coder_script" "code-server" { }) } -resource "coder_script" "nightly_sleep_reminder" { +resource "coder_script" "nightly_update" { agent_id = coder_agent.dev.agent_id display_name = "Nightly update" icon = "/icon/database.svg" - cron = "0 22 * * *" + cron = "0 0 22 * * *" # Run at 22:00 (10 PM) every day script = <"`. - `log_path` (String) The path of a file to write the logs to. If relative, it will be appended to tmp. - `run_on_start` (Boolean) This option defines whether or not the script should run when the agent starts. The script should exit when it is done to signal that the agent is ready. diff --git a/examples/resources/coder_script/resource.tf b/examples/resources/coder_script/resource.tf index b7fced38..14ee439b 100644 --- a/examples/resources/coder_script/resource.tf +++ b/examples/resources/coder_script/resource.tf @@ -28,15 +28,15 @@ resource "coder_script" "code-server" { }) } -resource "coder_script" "nightly_sleep_reminder" { +resource "coder_script" "nightly_update" { agent_id = coder_agent.dev.agent_id display_name = "Nightly update" icon = "/icon/database.svg" - cron = "0 22 * * *" + cron = "0 0 22 * * *" # Run at 22:00 (10 PM) every day script = < Date: Wed, 4 Jun 2025 11:08:18 +0000 Subject: [PATCH 2/5] feat: add validation warnings for 5-field cron expressions - Detect when users provide Unix 5-field cron format - Provide helpful warning with suggested 6-field conversion - Maintain backwards compatibility - expressions still work - Add comprehensive tests for validation function This helps users avoid confusion where '*/5 * * * *' (intended for every 5 minutes) actually runs every 5 seconds due to the missing seconds field. --- provider/script.go | 33 ++++++++++++++++--- provider/script_test.go | 72 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 5 deletions(-) diff --git a/provider/script.go b/provider/script.go index df436ead..1d75e6ca 100644 --- a/provider/script.go +++ b/provider/script.go @@ -3,6 +3,7 @@ package provider import ( "context" "fmt" + "strings" "github.com/google/uuid" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" @@ -13,6 +14,32 @@ import ( var ScriptCRONParser = cron.NewParser(cron.Second | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.DowOptional | cron.Descriptor) +// ValidateCronExpression validates a cron expression and provides helpful warnings for common mistakes +func ValidateCronExpression(cronExpr string) (warnings []string, errors []error) { + // Check if it looks like a 5-field Unix cron expression + fields := strings.Fields(cronExpr) + if len(fields) == 5 { + // Try to parse as standard Unix cron (without seconds) + unixParser := cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.DowOptional | cron.Descriptor) + if _, err := unixParser.Parse(cronExpr); err == nil { + // It's a valid 5-field expression, provide a helpful warning + warnings = append(warnings, fmt.Sprintf( + "The cron expression '%s' appears to be in Unix 5-field format. "+ + "Coder uses 6-field format (seconds minutes hours day month day-of-week). "+ + "Consider prefixing with '0 ' to run at the start of each minute: '0 %s'", + cronExpr, cronExpr)) + } + } + + // Validate with the actual 6-field parser + _, err := ScriptCRONParser.Parse(cronExpr) + if err != nil { + errors = append(errors, fmt.Errorf("%s is not a valid cron expression: %w", cronExpr, err)) + } + + return warnings, errors +} + func scriptResource() *schema.Resource { return &schema.Resource{ SchemaVersion: 1, @@ -78,11 +105,7 @@ func scriptResource() *schema.Resource { if !ok { return []string{}, []error{fmt.Errorf("got type %T instead of string", i)} } - _, err := ScriptCRONParser.Parse(v) - if err != nil { - return []string{}, []error{fmt.Errorf("%s is not a valid cron expression: %w", v, err)} - } - return nil, nil + return ValidateCronExpression(v) }, }, "start_blocks_login": { diff --git a/provider/script_test.go b/provider/script_test.go index 37f1a819..64808372 100644 --- a/provider/script_test.go +++ b/provider/script_test.go @@ -8,6 +8,8 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + + "github.com/coder/terraform-provider-coder/v2/provider" ) func TestScript(t *testing.T) { @@ -124,3 +126,73 @@ func TestScriptStartBlocksLoginRequiresRunOnStart(t *testing.T) { }}, }) } + +func TestValidateCronExpression(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + cronExpr string + expectWarnings bool + expectErrors bool + warningContains string + }{ + { + name: "valid 6-field expression", + cronExpr: "0 0 22 * * *", + expectWarnings: false, + expectErrors: false, + }, + { + name: "valid 6-field expression with seconds", + cronExpr: "30 0 9 * * 1-5", + expectWarnings: false, + expectErrors: false, + }, + { + name: "5-field Unix format - should warn", + cronExpr: "0 22 * * *", + expectWarnings: true, + expectErrors: false, + warningContains: "appears to be in Unix 5-field format", + }, + { + name: "5-field every 5 minutes - should warn", + cronExpr: "*/5 * * * *", + expectWarnings: true, + expectErrors: false, + warningContains: "Consider prefixing with '0 '", + }, + { + name: "invalid expression", + cronExpr: "invalid", + expectErrors: true, + }, + { + name: "too many fields", + cronExpr: "0 0 0 0 0 0 0", + expectErrors: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + warnings, errors := provider.ValidateCronExpression(tt.cronExpr) + + if tt.expectWarnings { + require.NotEmpty(t, warnings, "Expected warnings but got none") + if tt.warningContains != "" { + require.Contains(t, warnings[0], tt.warningContains) + } + } else { + require.Empty(t, warnings, "Expected no warnings but got: %v", warnings) + } + + if tt.expectErrors { + require.NotEmpty(t, errors, "Expected errors but got none") + } else { + require.Empty(t, errors, "Expected no errors but got: %v", errors) + } + }) + } +} From a219015225916c7c170fb3535f81e954626d53a1 Mon Sep 17 00:00:00 2001 From: "blink-so[bot]" <211532188+blink-so[bot]@users.noreply.github.com> Date: Wed, 4 Jun 2025 14:37:31 +0000 Subject: [PATCH 3/5] fix: terraform formatting for cron comment spacing --- examples/resources/coder_script/resource.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/resources/coder_script/resource.tf b/examples/resources/coder_script/resource.tf index 14ee439b..e4b3e803 100644 --- a/examples/resources/coder_script/resource.tf +++ b/examples/resources/coder_script/resource.tf @@ -32,7 +32,7 @@ resource "coder_script" "nightly_update" { agent_id = coder_agent.dev.agent_id display_name = "Nightly update" icon = "/icon/database.svg" - cron = "0 0 22 * * *" # Run at 22:00 (10 PM) every day + cron = "0 0 22 * * *" # Run at 22:00 (10 PM) every day script = < Date: Wed, 4 Jun 2025 14:48:23 +0000 Subject: [PATCH 4/5] feat: add every_5_minutes example to match documentation - Added the missing every_5_minutes cron script example to examples/resources/coder_script/resource.tf - This example demonstrates the 6-field cron format with a health check that runs every 5 minutes - Ensures examples directory matches the documentation examples --- examples/resources/coder_script/resource.tf | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/examples/resources/coder_script/resource.tf b/examples/resources/coder_script/resource.tf index e4b3e803..8b3fa661 100644 --- a/examples/resources/coder_script/resource.tf +++ b/examples/resources/coder_script/resource.tf @@ -40,6 +40,17 @@ resource "coder_script" "nightly_update" { EOF } +resource "coder_script" "every_5_minutes" { + agent_id = coder_agent.dev.agent_id + display_name = "Health check" + icon = "/icon/heart.svg" + cron = "0 */5 * * * *" # Run every 5 minutes + script = < Date: Wed, 4 Jun 2025 14:55:12 +0000 Subject: [PATCH 5/5] fix: update cron field schema description and fix formatting - Updated the cron field description in provider/script.go to include detailed 6-field format explanation - Fixed formatting issues in documentation (removed extra spaces before comments) - Regenerated documentation to ensure consistency between schema and docs - This ensures the documentation generation preserves the detailed cron description instead of reverting to generic text --- docs/resources/script.md | 4 ++-- provider/script.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/resources/script.md b/docs/resources/script.md index 452a6f3b..9058fce4 100644 --- a/docs/resources/script.md +++ b/docs/resources/script.md @@ -47,7 +47,7 @@ resource "coder_script" "nightly_update" { agent_id = coder_agent.dev.agent_id display_name = "Nightly update" icon = "/icon/database.svg" - cron = "0 0 22 * * *" # Run at 22:00 (10 PM) every day + cron = "0 0 22 * * *" # Run at 22:00 (10 PM) every day script = <