Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 27 additions & 3 deletions config/prowgen/pkg/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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
Expand Down Expand Up @@ -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{
Expand Down
40 changes: 37 additions & 3 deletions config/prowgen/pkg/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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)
}
}
15 changes: 12 additions & 3 deletions config/prowgen/pkg/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion config/prowgen/presubmit_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down