Skip to content

feat(gateway): add shutdown-delay and grace-period caddy settings#471

Merged
sylr merged 1 commit into
mainfrom
feat/caddy-shutdown-delay
May 28, 2026
Merged

feat(gateway): add shutdown-delay and grace-period caddy settings#471
sylr merged 1 commit into
mainfrom
feat/caddy-shutdown-delay

Conversation

@sylr
Copy link
Copy Markdown
Contributor

@sylr sylr commented May 28, 2026

Expose Caddy's global shutdown_delay and grace_period directives via two new settings under gateway.caddyfile.*. Both are optional with no default — when unset, Caddy's own defaults apply.

Add settings.GetDuration / GetDurationOrDefault helpers and register them in the catalog generator so future duration settings are picked up automatically.

Confidence: high
Scope-risk: narrow
Not-tested: no unit/e2e test added for the new caddyfile directives

Expose Caddy's global `shutdown_delay` and `grace_period` directives via
two new settings under `gateway.caddyfile.*`. Both are optional with no
default — when unset, Caddy's own defaults apply.

Add `settings.GetDuration` / `GetDurationOrDefault` helpers and register
them in the catalog generator so future duration settings are picked up
automatically.

Confidence: high
Scope-risk: narrow
Not-tested: no unit/e2e test added for the new caddyfile directives
@sylr sylr requested a review from a team as a code owner May 28, 2026 11:03
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

Review Change Stack

Walkthrough

This PR adds type-safe duration configuration for Caddy gateway shutdown behavior. New GetDuration and GetDurationOrDefault settings helpers parse duration strings, the catalog scanner recognizes these functions, gateway configuration reads shutdown-delay and grace-period settings, and the Caddyfile template emits the corresponding Caddy directives with proper template integration.

Changes

Gateway Caddy shutdown and grace-period settings

Layer / File(s) Summary
Duration settings helper functions
internal/resources/settings/helpers.go
Exported GetDuration and GetDurationOrDefault functions parse settings as time.Duration via time.ParseDuration, returning typed values with nil/error handling for absent or invalid durations.
Settings catalog scanner recognition
cmd/settings-catalog/main.go
Extended knownFunction map recognizes GetDuration and GetDurationOrDefault calls during scanning with appropriate key argument indices.
Gateway configuration duration reading
internal/resources/gateways/configuration.go
Updated createConfigMap to read shutdown-delay, grace-period, and idle-timeout as durations from settings, conditionally appending corresponding Caddy option helpers.
Caddyfile template option helpers
internal/resources/gateways/caddyfile.go
Added time import; refactored withIdleTimeout to accept time.Duration; introduced withShutdownDelay and withGracePeriod option helpers converting durations to string form.
Caddy template emission
internal/resources/gateways/Caddyfile.gotpl
Added conditional template directives in global block to emit shutdown_delay and grace_period Caddy directives when template values are provided.
Settings documentation
docs/09-Configuration reference/01-Settings.md
Documented gateway.caddyfile.shutdown-delay and gateway.caddyfile.grace-period as Go duration string settings for graceful shutdown configuration.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🐰 Duration strings now flow with type-safe grace,
From helper to config, at Caddy's own pace.
Shutdown delays and grace periods align,
A temporal dance, oh how they shine! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: adding two new Caddy settings (shutdown-delay and grace-period) for the gateway component.
Description check ✅ Passed The description clearly explains the purpose of the changes: exposing Caddy directives via new settings, adding helper functions, and registering them in the catalog generator.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/caddy-shutdown-delay

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
internal/resources/settings/helpers.go (1)

437-440: ⚡ Quick win

Include setting key context in duration parse errors.

When parsing fails, the error should identify which setting key was invalid to make reconciliation failures actionable.

Proposed change
 func GetDuration(ctx core.Context, stack string, keys ...string) (*time.Duration, error) {
 	value, err := GetString(ctx, stack, keys...)
 	if err != nil {
 		return nil, err
 	}
@@
 	duration, err := time.ParseDuration(*value)
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrapf(err, "invalid duration for setting '%s'", strings.Join(keys, "."))
 	}
 	return &duration, nil
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/resources/settings/helpers.go` around lines 437 - 440, The
time.ParseDuration call that parses *value should return an error that includes
the setting key so callers know which setting failed; replace the bare return of
err after time.ParseDuration(*value) with a wrapped error that references the
setting key (e.g., using fmt.Errorf("invalid duration for setting %q: %w", key,
err)) so the function (surrounding symbols: time.ParseDuration, value, duration)
returns the original error wrapped with the setting key context.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/resources/settings/helpers.go`:
- Around line 437-440: The time.ParseDuration call that parses *value should
return an error that includes the setting key so callers know which setting
failed; replace the bare return of err after time.ParseDuration(*value) with a
wrapped error that references the setting key (e.g., using fmt.Errorf("invalid
duration for setting %q: %w", key, err)) so the function (surrounding symbols:
time.ParseDuration, value, duration) returns the original error wrapped with the
setting key context.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1e406d2e-31ab-40ea-af67-e2fb3da962be

📥 Commits

Reviewing files that changed from the base of the PR and between 7473953 and 13fd6f8.

⛔ Files ignored due to path filters (1)
  • docs/09-Configuration reference/settings.catalog.json is excluded by !**/*.json
📒 Files selected for processing (6)
  • cmd/settings-catalog/main.go
  • docs/09-Configuration reference/01-Settings.md
  • internal/resources/gateways/Caddyfile.gotpl
  • internal/resources/gateways/caddyfile.go
  • internal/resources/gateways/configuration.go
  • internal/resources/settings/helpers.go

@sylr sylr merged commit 495152a into main May 28, 2026
15 of 21 checks passed
@sylr sylr deleted the feat/caddy-shutdown-delay branch May 28, 2026 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants