Improve scheduled function module: replace versioning with hash-based naming and add retry configuration - #4
Merged
Merged
Conversation
…of function buckets
jwbron
force-pushed
the
scheduled-fun-improvements
branch
from
August 20, 2025 18:03
9560792 to
90664ed
Compare
csilvers
reviewed
Aug 20, 2025
| default = 1 | ||
| } | ||
|
|
||
| variable "retries_enabled" { |
Member
There was a problem hiding this comment.
This will be very useful to disable for many cronjobs! Where if it fails we just wait for the next run again 5 minutes later.
Contributor
Author
There was a problem hiding this comment.
Yeah, that's why I decided to default this to false. It should probably be disabled for most jobs.
jwbron
added a commit
that referenced
this pull request
Jul 21, 2026
…l + write-only (#37) ## Summary The scheduled-job module reads `Slack__API_token_for_alertlib` with a `data "google_secret_manager_secret_version"` to build each job's Slack notification channel. Terraform state stores the full data-source response, including `secret_data`, so the token value sits in every consumer's state and every saved plan file; this is how the token was exposed by the committed-tfplan incident (see Khan/actions#274 and the consumer migrations). ## Change The channel stays fully Terraform-managed; the leak is fixed with the ephemeral/write-only mechanism (https://developer.hashicorp.com/terraform/language/manage-sensitive-data/ephemeral): - the token is read with `ephemeral "google_secret_manager_secret_version"` (never persisted to state or plans), and - written to the channel via the write-only `sensitive_labels.auth_token_wo` + `auth_token_wo_version` arguments, added to `google_monitoring_notification_channel` in provider 7.19.0. Rotation becomes explicit: after adding a new secret version, bump the new `slack_token_rotation` variable; the next apply re-reads the latest version and rewrites the channel token. (The old data-source design also only picked up new versions on the next plan, so nothing is lost; the bump makes the cutover reviewable.) An earlier revision of this PR added a `notification_channel_ids` input for pre-created channels; that is dropped in favor of keeping channel management in Terraform. ## Requirements and rollout - Terraform >= 1.11 (write-only arguments) and hashicorp/google >= 7.19.0; the module's `required_providers` now enforces both. - Suggest releasing as `scheduled-job-v0.4.0`. - Consumer follow-ups (culture-cron, beep-boop, internal-webserver; five jobs total, all currently embedding the token in state): bump the module pin, and bump the google provider pin from `7.8.0` to `>= 7.19.0` in the two stacks that pin exactly (no committed lock files, so that is the whole change). The adoption plan should show the channel's sensitive labels updating in place; after apply plus a state refresh, the token value is gone from live state. - Rotating the alertlib token (rotation runbook item #4) is still required regardless, since historical plan files already leaked it; after consumers adopt this version, the runbook's #4 mechanics change from "drift plan" to "bump slack_token_rotation in each repo". Author: jwbron Reviewers: nathanjd Required Reviewers: Approved By: nathanjd Checks: ⏭️ 1 check has been skipped Pull Request URL: #37
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
This PR improves the scheduled function Terraform module with several key enhancements:
Storage Bucket Improvements:
force_destroy = trueto allow bucket deletion during terraform destroyFunction Archive Management:
${function_name}-function-${hash}.zipSource Directory Handling:
abspath()wrapper aroundvar.source_dirfor more reliable path resolutionRetry Policy Configuration:
retries_enabledvariable (default: false) to control retry behaviorRETRY_POLICY_RETRYvsRETRY_POLICY_DO_NOT_RETRYThese changes improve resource management, reduce storage costs, and provide better configurability for the scheduled function module.
Test plan: