Skip to content
Merged
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
62 changes: 62 additions & 0 deletions data/sync/configs/20261014-all-checks-job.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
---
# Add the 'All checks' aggregate job to the pupmod PR Tests workflow.
#
# GitHub only blocks a merge on status checks that branch protection lists
# by name, so a job can fail without stopping a merge. The new `all-checks`
# job needs every other job in pr_tests.yml and fails unless all of them
# succeeded, giving branch protection one check to require in every repo.
#
# pr_tests.yml (merge_github_workflows): the repo's own `jobs.acceptance`
# block is kept as-is. A module with no acceptance job keeps having none,
# and the merge drops `acceptance` from all-checks' `needs:` there.
#
# Run this after the 20261003-gate-ruby-style PRs merge. Before that, these
# PRs would also carry the Ruby Style gating change.
#
# After the PRs merge, require the 'All checks' status check in each repo's
# branch protection. Requiring it before the job exists leaves every PR
# waiting on a check that never reports.
puppetsync::plan_config:
puppetsync:
permitted_project_types:
- pupmod
- pupmod_skeleton
plans:
sync:
github_api_delay_seconds: 1
merge_github_workflows:
files:
- pr_tests.yml
preserve_blocks:
- jobs.acceptance
stages:
- install_gems
- checkout_git_feature_branch_in_each_repo
- merge_github_workflows
- git_commit_changes
- ensure_github_fork
- ensure_git_remote
- git_push_to_remote
- ensure_github_pr

approve_github_prs:
stages:
- approve_github_pr_for_each_repo

merge_github_prs:
stages:
- merge_github_pr_for_each_repo

git:
feature_branch: puppetsync/20261014-all-checks-job
commit_message: |
[puppetsync] Add an 'All checks' job to PR Tests

GitHub only blocks a merge on status checks that branch protection
lists by name. This patch adds an `all-checks` job that needs every
other PR Tests job and fails unless all of them succeeded, so branch
protection can require that single check.

github:
pr_user: silug # This should be the account that *submitted* the PRs
approval_message: ':+1: :ghost:'
2 changes: 1 addition & 1 deletion data/sync/configs/latest.yaml
52 changes: 49 additions & 3 deletions dist/puppetsync/tasks/merge_gha_workflows.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@
# block it is appended to the parent; and when the EXISTING file lacks it,
# the template's block is REMOVED — the template must not introduce a
# repo-owned section (an acceptance job would be dead weight on a module
# with no acceptance suites).
# with no acceptance suites). A removed job is also dropped from every
# other job's `needs:`, since GitHub rejects a workflow whose `needs:`
# names a job that doesn't exist (the template's aggregate check job
# needs every job, acceptance included).

require 'fileutils'
require 'json'
Expand Down Expand Up @@ -105,6 +108,7 @@ def node_at_path(root, path)
# key.start_line ... value.end_line, exclusive)
def preserve_existing_blocks(template, existing_text, paths)
preserved = []
removed = []
existing_lines = existing_text.split("\n", -1)
existing_root = root_mapping(existing_text)

Expand All @@ -120,6 +124,7 @@ def preserve_existing_blocks(template, existing_text, paths)
next if template_pair.nil?

template_lines[template_pair[0].start_line...template_pair[1].end_line] = []
removed << path
elsif template_pair
block = existing_lines[existing_pair[0].start_line...existing_pair[1].end_line]
next if template_lines[template_pair[0].start_line...template_pair[1].end_line] == block
Expand All @@ -137,11 +142,52 @@ def preserve_existing_blocks(template, existing_text, paths)
template = template_lines.join("\n")
end

[template, preserved]
[template, preserved, removed]
end

# Drop the named jobs from every job's `needs:` (a scalar, a flow sequence
# on one line, or a block sequence with one item per line), removing the
# `needs:` key entirely when nothing is left. One edit per pass, re-parsing
# after each so line/column positions stay valid.
def prune_needs(template, job_names)
return template if job_names.empty?

loop do
jobs = (root = root_mapping(template)) && node_at_path(root, 'jobs')
return template unless jobs && jobs[1].is_a?(Psych::Nodes::Mapping)

lines = template.split("\n", -1)
edit = jobs[1].children.each_slice(2).lazy.map { |_name, job|
needs = job.is_a?(Psych::Nodes::Mapping) && node_at_path(job, 'needs')
next unless needs

key, value = needs
names = value.is_a?(Psych::Nodes::Sequence) ? value.children.map(&:value) : [value.value]
kept = names - job_names
next if kept == names

if kept.empty?
# A one-line value ends on the key's own line
lines[key.start_line...[value.end_line, key.start_line + 1].max] = []
elsif value.is_a?(Psych::Nodes::Sequence) && value.style == Psych::Nodes::Sequence::BLOCK
drop = value.children.reject { |item| kept.include?(item.value) }.map(&:start_line)
lines = lines.reject.with_index { |_l, i| drop.include?(i) }
else
line = lines[value.start_line]
lines[value.start_line] = line[0...value.start_column] + "[#{kept.join(', ')}]" + line[value.end_column..]
end
lines.join("\n")
}.find(&:itself)
return template if edit.nil? || edit == template

template = edit
end
end

def merge_workflow(template, existing_text, keys, blocks)
template, preserved_blocks = preserve_existing_blocks(template, existing_text, blocks)
template, preserved_blocks, removed_blocks = preserve_existing_blocks(template, existing_text, blocks)
removed_jobs = removed_blocks.filter_map { |path| path.delete_prefix('jobs.') if path.match?(/\Ajobs\.[^.]+\z/) }
template = prune_needs(template, removed_jobs)
existing = existing_values(existing_text, keys)
lines = template.split("\n", -1)
updated = []
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,32 @@ jobs:
- run: ${{matrix.test_suite.run}}
continue-on-error: ${{matrix.puppet.experimental}}

# The one check for branch protection to require: GitHub only blocks a
# merge on checks listed by name, and this job fails unless every job it
# needs succeeded, so the required list never has to track job or matrix
# names. Add new jobs to `needs:`; puppetsync drops a job a repo doesn't
# have (e.g. acceptance) from the list.
all-checks:
name: 'All checks'
if: always()
needs:
- puppet-syntax
- puppet-style
- ruby-style
- file-checks
- reference
- releng-checks
- spec-tests
- acceptance
runs-on: ubuntu-latest
steps:
- name: 'Fail unless every needed job succeeded'
env:
NEEDS: ${{ toJSON(needs) }}
run: |
jq -r 'to_entries[] | "\(.key): \(.value.result)"' <<< "$NEEDS"
jq -e 'all(.[]; .result == "success")' <<< "$NEEDS" > /dev/null

acceptance:
runs-on:
- ubuntu-latest
Expand Down
26 changes: 26 additions & 0 deletions modules/profile/files/pupmod/_github/workflows/pr_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,32 @@ jobs:
- run: 'bundle exec rake parallel_spec'
continue-on-error: ${{matrix.puppet.experimental}}

# The one check for branch protection to require: GitHub only blocks a
# merge on checks listed by name, and this job fails unless every job it
# needs succeeded, so the required list never has to track job or matrix
# names. Add new jobs to `needs:`; puppetsync drops a job a repo doesn't
# have (e.g. acceptance) from the list.
all-checks:
name: 'All checks'
if: always()
Comment thread
hcaballero2 marked this conversation as resolved.
Resolved
needs:
- puppet-syntax
- puppet-style
- ruby-style
- file-checks
- reference
- releng-checks
- spec-tests
- acceptance
runs-on: ubuntu-latest
steps:
- name: 'Fail unless every needed job succeeded'
env:
NEEDS: ${{ toJSON(needs) }}
run: |
jq -r 'to_entries[] | "\(.key): \(.value.result)"' <<< "$NEEDS"
jq -e 'all(.[]; .result == "success")' <<< "$NEEDS" > /dev/null

acceptance:
runs-on:
- ubuntu-latest
Expand Down
50 changes: 50 additions & 0 deletions spec/tasks/merge_gha_workflows_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,56 @@ def run_merge(workflows, preserve_blocks: nil)
expect(JSON.parse(stdout)['changed']).to be false
end

it "drops a removed job from other jobs' needs" do
with_needs = template.sub(" acceptance:\n", <<~YAML.gsub(/^/, ' ') + " acceptance:\n")
flow:
needs: [spec, acceptance]
block:
needs:
- spec
- acceptance
only:
needs: acceptance
only_list:
needs: [acceptance]
YAML
File.write(@wf, "name: PR Tests\njobs:\n spec:\n steps:\n - uses: actions/checkout@v5\n")

stdout, stderr, status = run_merge([{ 'path' => @wf, 'template' => with_needs }],
preserve_blocks: ['jobs.acceptance'])

expect(status).to be_success, stderr
jobs = Psych.safe_load(File.read(@wf))['jobs']
expect(jobs.keys).to eq(%w[spec flow block only only_list])
expect(jobs['flow']).to eq('needs' => ['spec'])
expect(jobs['block']).to eq('needs' => ['spec'])
expect(jobs['only']).to be_nil # `needs:` removed; the job is now empty
expect(jobs['only_list']).to be_nil
end

it "keeps a needs: entry for a job the repo has" do
File.write(@wf, real_template)

stdout, stderr, status = run_merge([{ 'path' => @wf, 'template' => real_template }],
preserve_blocks: ['jobs.acceptance'])

expect(status).to be_success, stderr
expect(JSON.parse(stdout)['changed']).to be false
expect(Psych.safe_load(File.read(@wf))['jobs']['all-checks']['needs']).to include('acceptance')
end

it 'gives a module without acceptance suites a valid aggregate job' do
File.write(@wf, real_template.sub(/^ acceptance:.*\z/m, ''))

stdout, stderr, status = run_merge([{ 'path' => @wf, 'template' => real_template }],
preserve_blocks: ['jobs.acceptance'])

expect(status).to be_success, stderr
jobs = Psych.safe_load(File.read(@wf))['jobs']
expect(jobs.keys).not_to include('acceptance')
expect(jobs['all-checks']['needs']).to eq(jobs.keys - ['all-checks'])
end

it 'is idempotent' do
existing = template
.sub('name: PR Tests', 'name: Old Name')
Expand Down