Skip to content

Acmesmith#238

Open
hanazuki wants to merge 4 commits intomasterfrom
acmesmith
Open

Acmesmith#238
hanazuki wants to merge 4 commits intomasterfrom
acmesmith

Conversation

@hanazuki
Copy link
Copy Markdown
Member

次回はcert-managerにします

Comment thread tf/acmesmith/acmesmith.tf
Comment on lines +158 to +160
name = "acmesmith-autorenew-${each.key}"
namespace = "default"
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: Critical one-time setup jobs are configured with wait_for_completion = false, which can lead to silent failures and leave the system in a non-functional state without any operator warning.
Severity: HIGH

Suggested Fix

Set wait_for_completion = true for the new-account and order-resolver-rubykaigi-net kubernetes_job_v1 resources. This will ensure that the Terraform apply process waits for these critical setup jobs to complete successfully and will fail if they do not, providing immediate feedback to the operator.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: tf/acmesmith/acmesmith.tf#L158-L160

Potential issue: The `kubernetes_job_v1` resources for `new-account` and
`order-resolver-rubykaigi-net` are configured with `wait_for_completion = false`. These
are critical one-time setup jobs that provision ACME account keys and order
certificates. If these jobs fail due to network issues, API errors from Let's Encrypt,
or configuration problems, Terraform will still report a successful `apply` and exit.
This silent failure leaves the infrastructure in a broken state, as dependent production
services will lack the necessary certificates to function, and the issue will only be
discoverable by manually inspecting Kubernetes job statuses.

Did we get this right? 👍 / 👎 to inform future reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants