Skip to content

fix(operator/cnpg): raise admission webhook timeouts for hosted control planes #19

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mariomeyer wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(operator/cnpg): raise admission webhook timeouts for hosted control planes #19

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions src/operator/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,59 @@ export function createOperator(
{ provider, dependsOn: [ns, ...operatorDeps] }
);

// CNPG's admission webhooks ship with timeouts tuned for a control plane
// co-located with the cluster: 10s mutating, 15s validating, both with
// failurePolicy: Fail. On a hosted control plane — Rackspace Spot, for
// instance, where the API server runs outside the cluster and every
// admission call has to tunnel back in through konnectivity — that round
// trip costs seconds rather than milliseconds.
//
// Measured on iad-1, ten consecutive mcluster.cnpg.io probes: 2.0s, 2.2s,
// 3.0s, 3.1s, 3.4s, 4.7s, 6.5s, 7.2s, 7.8s, 10.2s. One in ten already
// exceeded the 10s ceiling while the operator sat idle at 8 millicores, so
// it is the network path that is slow, not the operator. Concurrent
// admission — a Pulumi preview dry-running several resources at once —
// pushes more of them over, and with failurePolicy: Fail that aborts the
// run partway through an update.
//
// The chart exposes only webhook.{mutating,validating}.{create,failurePolicy},
// not timeoutSeconds, so this cannot be expressed through Helm values. Patch
// the field instead: `webhooks` is a listType=map keyed by `name`, so each
// entry merges by key and no other field of the configuration is touched.
// In particular failurePolicy stays Fail — nothing bypasses admission.
if (type === "cloudnative-pg" && !skipOperatorInstall) {

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Skip patches for webhook configurations disabled in Helm

When callers set config.values.webhook.mutating.create or config.values.webhook.validating.create to false, the chart deliberately omits the corresponding configuration, but this condition still registers both partial patch resources. With no Helm-created object to merge into, the update fails because the patch either cannot find its target or attempts to create an invalid configuration containing incomplete webhook entries. Gate each patch on the corresponding chart setting.

Useful? React with 👍 / 👎.

const webhookTimeoutSeconds = 30;

new k8s.admissionregistration.v1.MutatingWebhookConfigurationPatch(
`${type}-mutating-webhook-timeout`,
{
metadata: { name: "cnpg-mutating-webhook-configuration" },
webhooks: [
"mbackup.cnpg.io",
"mcluster.cnpg.io",
"mdatabase.cnpg.io",

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid patching webhook names absent from pinned charts

For a caller pinning config.version to a CNPG chart from before the Database webhook was introduced, mdatabase.cnpg.io is not an existing list-map entry. Server-side apply then treats this partial {name, timeoutSeconds} value as a new webhook rather than a field update, and Kubernetes rejects it because required fields such as clientConfig, sideEffects, and admissionReviewVersions are missing. The validating list has the analogous vdatabase.cnpg.io problem, so the implementation should constrain supported chart versions or patch only entries known to exist in the selected version.

Useful? React with 👍 / 👎.

"mscheduledbackup.cnpg.io",
].map((name) => ({ name, timeoutSeconds: webhookTimeoutSeconds })),
},
{ provider, dependsOn: [helmRelease], retainOnDelete: true }

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Make CNPG resources wait for the timeout patches

On an initial deployment, these patches and the CNPG Cluster resources are siblings that depend only on helmRelease: createSingleCnpgCluster() still builds its dependency list from the release and namespace, while neither patch resource is retained or passed into that path. Pulumi may therefore submit a Cluster admission request concurrently with, or before, the timeout patch, leaving the first update exposed to the same 10-second failure this change is intended to prevent. Collect both patch resources and include them in the dependencies used by CNPG custom resources.

Useful? React with 👍 / 👎.

);

new k8s.admissionregistration.v1.ValidatingWebhookConfigurationPatch(
`${type}-validating-webhook-timeout`,
{
metadata: { name: "cnpg-validating-webhook-configuration" },
webhooks: [
"vbackup.cnpg.io",
"vcluster.cnpg.io",
"vdatabase.cnpg.io",
"vpooler.cnpg.io",
"vscheduledbackup.cnpg.io",
].map((name) => ({ name, timeoutSeconds: webhookTimeoutSeconds })),
},
{ provider, dependsOn: [helmRelease], retainOnDelete: true }
);
}

// MinIO returns a different operator shape (createBucket instead of createCluster)
if (type === "minio") {
return createMinioOperator(config, helmRelease);
Expand Down
Loading