From a92980a33bc961ffaef980d6c5e9d543f0cde221 Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Fri, 14 Aug 2026 13:06:47 +0100 Subject: [PATCH] Pin credentialed periodics to the dedicated node pool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Any periodic carrying a credential preset now gets a nodeSelector and toleration for the dedicated=credentialed-jobs node pool, injected in Periodics — the single point through which every generated periodic passes — so no generator can accidentally schedule a credentialed job onto the shared worker pool, where presubmits run unreviewed PR code in privileged (dind) pods and can poison the node-local hostPath build caches. The pool itself is added by cert-manager/infrastructure#89. Co-Authored-By: Claude Fable 5 Signed-off-by: Richard Wall --- .../master/cert-manager-master.yaml | 7 ++++ .../cert-manager-release-1.20.yaml | 7 ++++ .../cert-manager-release-1.21.yaml | 7 ++++ config/prowgen/pkg/context.go | 30 ++++++++++++-- config/prowgen/pkg/context_test.go | 40 +++++++++++++++++-- config/prowgen/pkg/types.go | 15 +++++-- config/prowgen/presubmit_policy_test.go | 2 +- 7 files changed, 98 insertions(+), 10 deletions(-) diff --git a/config/jobs/cert-manager/cert-manager/master/cert-manager-master.yaml b/config/jobs/cert-manager/cert-manager/master/cert-manager-master.yaml index 6f8fa40bd..b139db987 100644 --- a/config/jobs/cert-manager/cert-manager/master/cert-manager-master.yaml +++ b/config/jobs/cert-manager/cert-manager/master/cert-manager-master.yaml @@ -620,6 +620,13 @@ periodics: nameservers: - 8.8.8.8 - 8.8.4.4 + nodeSelector: + dedicated: credentialed-jobs + tolerations: + - key: dedicated + operator: Equal + value: credentialed-jobs + effect: NoSchedule extra_refs: - org: cert-manager repo: cert-manager diff --git a/config/jobs/cert-manager/cert-manager/release-1.20/cert-manager-release-1.20.yaml b/config/jobs/cert-manager/cert-manager/release-1.20/cert-manager-release-1.20.yaml index 50b3f5667..1333c5118 100644 --- a/config/jobs/cert-manager/cert-manager/release-1.20/cert-manager-release-1.20.yaml +++ b/config/jobs/cert-manager/cert-manager/release-1.20/cert-manager-release-1.20.yaml @@ -593,6 +593,13 @@ periodics: nameservers: - 8.8.8.8 - 8.8.4.4 + nodeSelector: + dedicated: credentialed-jobs + tolerations: + - key: dedicated + operator: Equal + value: credentialed-jobs + effect: NoSchedule extra_refs: - org: cert-manager repo: cert-manager diff --git a/config/jobs/cert-manager/cert-manager/release-1.21/cert-manager-release-1.21.yaml b/config/jobs/cert-manager/cert-manager/release-1.21/cert-manager-release-1.21.yaml index 7e1d08404..363a522aa 100644 --- a/config/jobs/cert-manager/cert-manager/release-1.21/cert-manager-release-1.21.yaml +++ b/config/jobs/cert-manager/cert-manager/release-1.21/cert-manager-release-1.21.yaml @@ -593,6 +593,13 @@ periodics: nameservers: - 8.8.8.8 - 8.8.4.4 + nodeSelector: + dedicated: credentialed-jobs + tolerations: + - key: dedicated + operator: Equal + value: credentialed-jobs + effect: NoSchedule extra_refs: - org: cert-manager repo: cert-manager diff --git a/config/prowgen/pkg/context.go b/config/prowgen/pkg/context.go index 1c941f09c..69b336138 100644 --- a/config/prowgen/pkg/context.go +++ b/config/prowgen/pkg/context.go @@ -85,14 +85,17 @@ func (pc *ProwContext) OptionalPresubmitIfChanged(job *Job, changedFileRegex str pc.addPresubmit(job, false, true, changedFileRegex) } -// presubmitForbiddenLabels are presets that mount live credentials. Presubmits run +// credentialPresets are presets that mount live credentials. Presubmits run // unreviewed PR code, so they must never carry these; only periodics (which run // merged, reviewed code) may. Panicking here, at the single point through which // every presubmit passes, means no generator can accidentally add them to a // presubmit: generation fails loudly and the developer must remove the preset or // make the job a periodic. TestNoPresubmitUsesSecretBearingPreset is the backstop // covering hand-written job files and presets not in this list. -var presubmitForbiddenLabels = []string{ +// +// Any periodic carrying one of these presets is additionally pinned, in +// Periodics, to the dedicated credentialed-jobs node pool. +var credentialPresets = []string{ "preset-venafi-tpp-credentials", "preset-venafi-cloud-credentials", "preset-venafi-ngts-credentials", @@ -101,7 +104,7 @@ var presubmitForbiddenLabels = []string{ func (pc *ProwContext) addPresubmit(job *Job, alwaysRun bool, optional bool, changedFileRegex string) { job.Name = pc.presubmitJobName(job.Name) - for _, label := range presubmitForbiddenLabels { + for _, label := range credentialPresets { if _, found := job.Labels[label]; found { // note: we panic for the same reason as configurers.go: this tool is // developer-facing and a generator adding live credentials to a @@ -135,6 +138,27 @@ func (pc *ProwContext) Periodics(job *Job, periodicityHours int) { addTestGridAnnotations(pc.periodicDashboardName())(job) } + // Jobs which mount live credentials must run on the dedicated + // credentialed-jobs node pool, so that they never share a node — or its + // hostPath build caches — with presubmits, which run unreviewed PR code + // in privileged (dind) pods. Injecting the placement here, at the single + // point through which every periodic passes, means no generator can + // accidentally schedule a credentialed job onto the shared worker pool. + // The pool, its label and its taint are defined in + // cert-manager/infrastructure gcp/modules/gcp-cluster/main.tf. + for _, label := range credentialPresets { + if _, found := job.Labels[label]; found { + job.Spec.NodeSelector = map[string]string{"dedicated": "credentialed-jobs"} + job.Spec.Tolerations = []Toleration{{ + Key: "dedicated", + Operator: "Equal", + Value: "credentialed-jobs", + Effect: "NoSchedule", + }} + break + } + } + pc.periodics = append(pc.periodics, &PeriodicJob{ Job: *job, ExtraRefs: []ExtraRef{ diff --git a/config/prowgen/pkg/context_test.go b/config/prowgen/pkg/context_test.go index 1ec7767d0..73b00b591 100644 --- a/config/prowgen/pkg/context_test.go +++ b/config/prowgen/pkg/context_test.go @@ -35,7 +35,7 @@ func testProwContext() *ProwContext { // presubmits must never mount, so we can assert they are rejected/retained. func jobWithForbiddenLabels(name string) *Job { job := jobTemplate(name, "some description") - for _, label := range presubmitForbiddenLabels { + for _, label := range credentialPresets { job.Labels[label] = "true" } return job @@ -44,7 +44,7 @@ func jobWithForbiddenLabels(name string) *Job { // Presubmits run unreviewed PR code, so addPresubmit must fail generation for // a job carrying a credential preset, regardless of which generator added it. func Test_addPresubmit_rejectsCredentialLabels(t *testing.T) { - for _, label := range presubmitForbiddenLabels { + for _, label := range credentialPresets { t.Run(label, func(t *testing.T) { defer func() { if recover() == nil { @@ -71,9 +71,43 @@ func Test_Periodics_retainsCredentialLabels(t *testing.T) { } labels := pc.periodics[0].Labels - for _, label := range presubmitForbiddenLabels { + for _, label := range credentialPresets { if _, ok := labels[label]; !ok { t.Errorf("periodic must retain credential preset %q, but it was stripped", label) } } } + +// Periodics carrying a credential preset must be pinned to the dedicated +// credentialed-jobs node pool, so they never share a node (or its hostPath +// build caches) with presubmits running unreviewed PR code. +func Test_Periodics_pinsCredentialedJobsToDedicatedPool(t *testing.T) { + for _, label := range credentialPresets { + t.Run(label, func(t *testing.T) { + pc := testProwContext() + job := jobTemplate("e2e", "some description") + job.Labels[label] = "true" + pc.Periodics(job, 2) + + spec := pc.periodics[0].Spec + if got := spec.NodeSelector["dedicated"]; got != "credentialed-jobs" { + t.Errorf("credentialed periodic must have nodeSelector dedicated=credentialed-jobs, got %q", got) + } + if len(spec.Tolerations) != 1 || spec.Tolerations[0].Value != "credentialed-jobs" { + t.Errorf("credentialed periodic must tolerate the credentialed-jobs taint, got %+v", spec.Tolerations) + } + }) + } +} + +// Periodics without credentials stay on the shared worker pool: the dedicated +// pool is reserved for jobs which must not co-tenant with presubmits. +func Test_Periodics_leavesUncredentialedJobsUnpinned(t *testing.T) { + pc := testProwContext() + pc.Periodics(jobTemplate("e2e", "some description"), 2) + + spec := pc.periodics[0].Spec + if spec.NodeSelector != nil || spec.Tolerations != nil { + t.Errorf("uncredentialed periodic must not be pinned to the credentialed pool, got nodeSelector=%v tolerations=%+v", spec.NodeSelector, spec.Tolerations) + } +} diff --git a/config/prowgen/pkg/types.go b/config/prowgen/pkg/types.go index 306fe7df3..b4fbf756f 100644 --- a/config/prowgen/pkg/types.go +++ b/config/prowgen/pkg/types.go @@ -43,9 +43,18 @@ type Job struct { } type JobSpec struct { - Containers []Container `yaml:"containers"` - DNSPolicy string `yaml:"dnsPolicy"` - DNSConfig DNSConfig `yaml:"dnsConfig"` + Containers []Container `yaml:"containers"` + DNSPolicy string `yaml:"dnsPolicy"` + DNSConfig DNSConfig `yaml:"dnsConfig"` + NodeSelector map[string]string `yaml:"nodeSelector,omitempty"` + Tolerations []Toleration `yaml:"tolerations,omitempty"` +} + +type Toleration struct { + Key string `yaml:"key"` + Operator string `yaml:"operator"` + Value string `yaml:"value"` + Effect string `yaml:"effect"` } type Container struct { diff --git a/config/prowgen/presubmit_policy_test.go b/config/prowgen/presubmit_policy_test.go index 74761914f..b779c7fad 100644 --- a/config/prowgen/presubmit_policy_test.go +++ b/config/prowgen/presubmit_policy_test.go @@ -58,7 +58,7 @@ type policyConfig struct { // check covers hand-written job files as well as prowgen-generated ones // (which are committed and kept in sync by verify-prowgen). This is the // repo-wide enforcement backing the generation-time strip of -// presubmitForbiddenLabels in pkg/context.go. +// credentialPresets in pkg/context.go. func TestNoPresubmitUsesSecretBearingPreset(t *testing.T) { files := []string{"../config.yaml"} err := filepath.WalkDir("../jobs", func(path string, d fs.DirEntry, err error) error {