diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 05cb081..b8dfa8d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -62,6 +62,15 @@ jobs: - name: managed_file bootstrap/enforce contract test run: ./scripts/ci-managed-file-contract.sh + - name: BoltSpec plan specs (under Bolt's Ruby) + # -O /dev/null skips .rspec, whose exclude-pattern hides spec/plans + # from the plain-Ruby spec job (bolt_spec needs Bolt's Ruby). + # The .gems binstub is invoked explicitly: `ruby -S rspec` only works + # when some rspec is already on PATH + run: | + GEM_HOME=.gems /opt/puppetlabs/bolt/bin/gem install rspec --no-document + GEM_HOME=.gems /opt/puppetlabs/bolt/bin/ruby .gems/bin/rspec -O /dev/null spec/plans + - name: List pipeline stages (plan dry run) env: # The plan requires these to be set, but nothing calls the APIs in diff --git a/.rspec b/.rspec new file mode 100644 index 0000000..d9dc4bc --- /dev/null +++ b/.rspec @@ -0,0 +1,6 @@ +# The plan specs under spec/plans/ require Bolt's Ruby (bolt_spec ships with +# the openbolt package) and are run separately — see spec/plans/plan_spec_helper.rb: +# +# GEM_HOME=.gems /opt/puppetlabs/bolt/bin/ruby .gems/bin/rspec -O /dev/null spec/plans +# +--exclude-pattern plans/**/*_spec.rb diff --git a/AGENTS.md b/AGENTS.md index 572a483..6b98450 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,7 +30,7 @@ bolt plan run puppetsync options='{"list_pipeline_stages": true}' config=... rep - `config=` / `repolist=` name YAML files in `data/sync/configs/` and `data/sync/repolists/`; both default to the `latest.yaml` symlink in each directory. - Use `--log-level info` to watch progress (the `apply()` step can look hung otherwise). - Required environment variables and their purposes are documented in README.md ("Environment variables"). -- Unit tests: `rspec` from the repo root (plain rspec; no bundle needed). The specs in `spec/` run the Bolt tasks as standalone scripts and sanity-check the Hiera data. CI (`.github/workflows/ci.yml`) also validates Puppet syntax and smoke-tests plan loading with `bolt plan show` / `list_pipeline_stages`. End-to-end validation is still running a plan against a test repolist (e.g. the `*_test.yaml` repolists). +- Unit tests: `rspec` from the repo root (plain rspec; no bundle needed). The specs in `spec/` run the Bolt tasks as standalone scripts and sanity-check the Hiera data. Plan logic is unit-tested with BoltSpec under Bolt's Ruby: `GEM_HOME=.gems /opt/puppetlabs/bolt/bin/ruby .gems/bin/rspec -O /dev/null spec/plans` (excluded from the plain `rspec` run via `.rspec`; conventions in `spec/plans/plan_spec_helper.rb`). CI (`.github/workflows/ci.yml`) also validates Puppet syntax, smoke-tests plan loading, and runs e2e/contract scripts from `scripts/`. End-to-end validation is still running a plan against a test repolist (e.g. the `*_test.yaml` repolists). ## Architecture diff --git a/spec/fixtures/modules/puppetsync_test/plans/stage_bad_result.pp b/spec/fixtures/modules/puppetsync_test/plans/stage_bad_result.pp new file mode 100644 index 0000000..5f5538b --- /dev/null +++ b/spec/fixtures/modules/puppetsync_test/plans/stage_bad_result.pp @@ -0,0 +1,13 @@ +# Test-support plan: a stage block returning something other than Bolt +# results must fail the plan (see #52/#64) +plan puppetsync_test::stage_bad_result() { + $t = Target.new('name' => 'some-repo') + $t.set_var('puppetsync_stage_results', {}) + + [$t].puppetsync::pipeline_stage( + 'bad_stage', + { 'stages' => ['bad_stage'] } + ) |$ok_repos, $stage_name| { + 'this is not a Bolt::Result' + } +} diff --git a/spec/fixtures/modules/puppetsync_test/plans/stage_holdback.pp b/spec/fixtures/modules/puppetsync_test/plans/stage_holdback.pp new file mode 100644 index 0000000..c4238d3 --- /dev/null +++ b/spec/fixtures/modules/puppetsync_test/plans/stage_holdback.pp @@ -0,0 +1,23 @@ +# Test-support plan: a target that fails a stage must be held back from +# later stages while other targets proceed +plan puppetsync_test::stage_holdback() { + $good = Target.new('name' => 'good-repo') + $bad = Target.new('name' => 'bad-repo') + [$good, $bad].each |$t| { $t.set_var('puppetsync_stage_results', {}) } + + [$good, $bad].puppetsync::pipeline_stage( + 'first_stage', + { 'stages' => ['first_stage', 'second_stage'] } + ) |$ok_repos, $stage_name| { + run_command('stage-one', $ok_repos, { '_catch_errors' => true }) + } + + $survivors = [$good, $bad].puppetsync::pipeline_stage( + 'second_stage', + { 'stages' => ['first_stage', 'second_stage'] } + ) |$ok_repos, $stage_name| { + run_command('stage-two', $ok_repos, { '_catch_errors' => true }) + } + + return($survivors.map |$t| { $t.name }) +} diff --git a/spec/fixtures/modules/puppetsync_test/plans/stage_smoke.pp b/spec/fixtures/modules/puppetsync_test/plans/stage_smoke.pp new file mode 100644 index 0000000..22644b3 --- /dev/null +++ b/spec/fixtures/modules/puppetsync_test/plans/stage_smoke.pp @@ -0,0 +1,23 @@ +# Test-support plan: exercises puppetsync::pipeline_stage gating +plan puppetsync_test::stage_smoke() { + $changed = Target.new('name' => 'changed-repo') + $unchanged = Target.new('name' => 'unchanged-repo') + [$changed, $unchanged].each |$t| { $t.set_var('puppetsync_stage_results', {}) } + $unchanged.set_var('puppetsync_unchanged', true) + + $ran = [$changed, $unchanged].puppetsync::pipeline_stage( + 'gated_stage', + { 'stages' => ['gated_stage'], 'skip_unchanged_targets' => true } + ) |$ok_repos, $stage_name| { + run_command('true', $ok_repos) + } + + $skipped = [$changed, $unchanged].puppetsync::pipeline_stage( + 'not_in_stage_list', + { 'stages' => ['gated_stage'] } + ) |$ok_repos, $stage_name| { + run_command('true', $ok_repos) + } + + return({ 'ran' => $ran.map |$t| { $t.name }, 'skipped_stage_result' => $skipped }) +} diff --git a/spec/plans/approve_github_prs_spec.rb b/spec/plans/approve_github_prs_spec.rb new file mode 100644 index 0000000..c3d2ac4 --- /dev/null +++ b/spec/plans/approve_github_prs_spec.rb @@ -0,0 +1,96 @@ +require_relative 'plan_spec_helper' + +describe 'plan: puppetsync::approve_github_prs' do + include_context 'puppetsync plan specs' + + let(:repos_config) do + { + 'https://github.com/simp/repo-a' => { 'branch' => 'master' }, + 'https://github.com/simp/repo-b' => { 'branch' => 'main' }, + } + end + + let(:puppetsync_config) do + { + 'puppetsync' => { + 'plans' => { + 'approve_github_prs' => { + 'github_api_delay_seconds' => 0, + 'stages' => ['approve_github_pr_for_each_repo'], + }, + }, + }, + 'git' => { 'feature_branch' => 'SIMP-TEST' }, + } + end + + def plan_params + { + 'project_dir' => PROJECT_ROOT, + 'puppetsync_config' => puppetsync_config, + 'repos_config' => repos_config, + 'pr_user' => 'bot-user', + 'approval_message' => ':+1: specs', + } + end + + # Task metadata marks some params sensitive, so Bolt wraps them before the + # stub sees them + def unwrap(value) + value.respond_to?(:unwrap) ? value.unwrap : value + end + + def task_result(target, task, value) + Bolt::Result.new(target, value: value, action: 'task', object: task) + end + + it 'approves the PR for every repo with per-repo params from the repolist' do + allow_out_message + calls = [] + expect_task('puppetsync::approve_github_pr').be_called_times(2).return do |targets:, task:, params:| + calls << { 'target' => targets.first.name }.merge(params.transform_values { |v| unwrap(v) }) + Bolt::ResultSet.new(targets.map { |t| task_result(t, task, 'approved' => true) }) + end + + result = run_plan('puppetsync::approve_github_prs', plan_params) + + expect(result.ok?).to be(true), result.value.to_s + expect(calls.map { |c| c['target'] }).to contain_exactly('repo-a', 'repo-b') + repo_a = calls.find { |c| c['target'] == 'repo-a' } + expect(repo_a).to include( + 'target_repo' => 'simp/repo-a', + 'target_branch' => 'master', + 'fork_user' => 'bot-user', + 'fork_branch' => 'SIMP-TEST', + 'approval_message' => ':+1: specs', + 'github_authtoken' => ENV.fetch('GITHUB_API_TOKEN'), + ) + repo_b = calls.find { |c| c['target'] == 'repo-b' } + expect(repo_b).to include('target_repo' => 'simp/repo-b', 'target_branch' => 'main') + end + + it 'summarizes and fails the plan when approval fails for a repo' do + allow_out_message + allow_task('puppetsync::approve_github_pr').with_targets(['repo-a']).return do |targets:, task:, params:| + Bolt::ResultSet.new(targets.map { |t| task_result(t, task, 'approved' => true) }) + end + allow_task('puppetsync::approve_github_pr').with_targets(['repo-b']) + .error_with('kind' => 'spec/approval-denied', 'msg' => 'PR not found') + + result = run_plan('puppetsync::approve_github_prs', plan_params) + + expect(result.ok?).to be(false) + expect(result.value.msg).to match(/failures occured/) + end + + it 'skips the approval stage entirely when the config stage list omits it' do + allow_out_message + config = puppetsync_config.dup + config['puppetsync'] = { 'plans' => { 'approve_github_prs' => { 'stages' => [] } } } + + result = run_plan('puppetsync::approve_github_prs', plan_params.merge('puppetsync_config' => config)) + + # No task stubs declared: any approve/install task call would raise + expect(result.ok?).to be(true), result.value.to_s + end +end diff --git a/spec/plans/batch_spec.rb b/spec/plans/batch_spec.rb new file mode 100644 index 0000000..1df98d7 --- /dev/null +++ b/spec/plans/batch_spec.rb @@ -0,0 +1,22 @@ +require_relative 'plan_spec_helper' + +describe 'plan: puppetsync::batch' do + include_context 'puppetsync plan specs' + + it 'runs the sync plan once per repolist in the batch, in order' do + allow_out_message + repolists_run = [] + expect_plan('puppetsync').be_called_times(2).return do |plan:, params:| + repolists_run << params['repolist'] + Bolt::PlanResult.new("ran #{params['repolist']}", 'success') + end + + result = run_plan('puppetsync::batch', { + 'project_dir' => PROJECT_ROOT, + 'batches_config' => { 'repolists' => ['batch-one', 'batch-two'], 'delay' => 0 }, + }) + + expect(result.ok?).to be(true), result.value.to_s + expect(repolists_run).to eq(['batch-one', 'batch-two']) + end +end diff --git a/spec/plans/merge_github_prs_spec.rb b/spec/plans/merge_github_prs_spec.rb new file mode 100644 index 0000000..e88e3e6 --- /dev/null +++ b/spec/plans/merge_github_prs_spec.rb @@ -0,0 +1,78 @@ +require_relative 'plan_spec_helper' + +describe 'plan: puppetsync::merge_github_prs' do + include_context 'puppetsync plan specs' + + let(:repos_config) do + { + 'https://github.com/simp/repo-a' => { 'branch' => 'master' }, + 'https://github.com/simp/repo-b' => { 'branch' => 'main' }, + } + end + + let(:puppetsync_config) do + { + 'puppetsync' => { + 'plans' => { + 'merge_github_prs' => { + 'github_api_delay_seconds' => 0, + 'stages' => ['merge_github_pr_for_each_repo'], + }, + }, + }, + 'git' => { 'feature_branch' => 'SIMP-TEST' }, + } + end + + def plan_params + { + 'project_dir' => PROJECT_ROOT, + 'puppetsync_config' => puppetsync_config, + 'repos_config' => repos_config, + 'pr_user' => 'bot-user', + } + end + + def unwrap(value) + value.respond_to?(:unwrap) ? value.unwrap : value + end + + def task_result(target, task, value) + Bolt::Result.new(target, value: value, action: 'task', object: task) + end + + it 'merges the PR for every repo with per-repo params from the repolist' do + allow_out_message + calls = [] + expect_task('puppetsync::merge_github_pr').be_called_times(2).return do |targets:, task:, params:| + calls << { 'target' => targets.first.name }.merge(params.transform_values { |v| unwrap(v) }) + Bolt::ResultSet.new(targets.map { |t| task_result(t, task, 'merged' => true) }) + end + + result = run_plan('puppetsync::merge_github_prs', plan_params) + + expect(result.ok?).to be(true), result.value.to_s + expect(calls.map { |c| c['target'] }).to contain_exactly('repo-a', 'repo-b') + expect(calls.find { |c| c['target'] == 'repo-a' }).to include( + 'target_repo' => 'simp/repo-a', + 'target_branch' => 'master', + 'fork_user' => 'bot-user', + 'fork_branch' => 'SIMP-TEST', + ) + expect(calls.find { |c| c['target'] == 'repo-b' }).to include('target_branch' => 'main') + end + + it 'fails the plan at the summary when a merge fails' do + allow_out_message + allow_task('puppetsync::merge_github_pr').with_targets(['repo-a']).return do |targets:, task:, params:| + Bolt::ResultSet.new(targets.map { |t| task_result(t, task, 'merged' => true) }) + end + allow_task('puppetsync::merge_github_pr').with_targets(['repo-b']) + .error_with('kind' => 'spec/merge-conflict', 'msg' => 'cannot merge') + + result = run_plan('puppetsync::merge_github_prs', plan_params) + + expect(result.ok?).to be(false) + expect(result.value.msg).to match(/failures occured/) + end +end diff --git a/spec/plans/pipeline_stage_spec.rb b/spec/plans/pipeline_stage_spec.rb new file mode 100644 index 0000000..f6b58fc --- /dev/null +++ b/spec/plans/pipeline_stage_spec.rb @@ -0,0 +1,49 @@ +require_relative 'plan_spec_helper' + +describe 'puppetsync::pipeline_stage' do + include_context 'puppetsync plan specs' + + context 'stage gating and unchanged-target skipping (stage_smoke)' do + it 'runs gated stages only on changed targets and honors the stage list' do + expect_command('true').with_targets(['changed-repo']) + allow_out_message + expect_out_message.with_params('===== SKIPPING 1 UNCHANGED TARGET(S) FOR STAGE: gated_stage') + + result = run_plan('puppetsync_test::stage_smoke', {}) + + expect(result.ok?).to be(true), result.value.to_s + expect(result.value['ran']).to eq(['changed-repo']) + expect(result.value['skipped_stage_result']).to eq([]) + end + end + + context 'failure holdback (stage_holdback)' do + it 'holds a failed target back from later stages while others proceed' do + allow_out_message + allow_command('stage-one').return do |targets:, command:, params:| + Bolt::ResultSet.new(targets.map do |target| + exit_code = target.name == 'bad-repo' ? 1 : 0 + value = { 'stdout' => '', 'stderr' => '', 'merged_output' => '', 'exit_code' => exit_code } + Bolt::Result.for_command(target, value, 'command', command, []) + end) + end + expect_command('stage-two').with_targets(['good-repo']) + + result = run_plan('puppetsync_test::stage_holdback', {}) + + expect(result.ok?).to be(true), result.value.to_s + expect(result.value).to eq(['good-repo']) + end + end + + context 'unrecordable results (stage_bad_result)' do + it 'fails the plan when a stage block returns a non-Bolt-result' do + allow_out_message + + result = run_plan('puppetsync_test::stage_bad_result', {}) + + expect(result.ok?).to be(false) + expect(result.value.msg).to match(/the stage block returned String/) + end + end +end diff --git a/spec/plans/plan_spec_helper.rb b/spec/plans/plan_spec_helper.rb new file mode 100644 index 0000000..5fc7bac --- /dev/null +++ b/spec/plans/plan_spec_helper.rb @@ -0,0 +1,47 @@ +# Helper for BoltSpec plan specs (simp/puppetsync#76). +# +# These specs run the project's plans with stubbed tasks/commands — no +# clones, no network. They require Bolt's Ruby (bolt_spec ships with the +# openbolt package) and are excluded from the default plain-Ruby `rspec` +# run via `.rspec`. Run them with: +# +# GEM_HOME=.gems /opt/puppetlabs/bolt/bin/gem install rspec --no-document +# GEM_HOME=.gems /opt/puppetlabs/bolt/bin/ruby .gems/bin/rspec -O /dev/null spec/plans +# +# Why not `bundle exec`: Bundler's load-path isolation hides the packaged +# openbolt gem, and declaring `gem 'openbolt'` makes bundler install a +# SECOND bolt from rubygems (vendored, full native-extension dependency +# closure) and test THAT copy instead of the OS package that runs real +# syncs — reintroducing the dual-install the README warns against, with +# silent version drift between Gemfile.lock and the package. The explicit +# binstub invocation above is deterministic without any of that. +# +# Conventions learned the hard way: +# - Stub matching is last-defined-wins: declare catch-all +# `allow_out_message` BEFORE specific `expect_out_message` stubs +# - Plans that default parameters from `lookup()` must be given those +# parameters explicitly, or the spec depends on the project's Hiera data +require 'bolt_spec/plans' + +PROJECT_ROOT = File.expand_path(File.join(__dir__, '..', '..')) unless defined?(PROJECT_ROOT) + +RSpec.shared_context 'puppetsync plan specs' do + include BoltSpec::Plans + + def modulepath + [ + File.join(PROJECT_ROOT, 'spec', 'fixtures', 'modules'), + File.join(PROJECT_ROOT, 'dist'), + File.join(PROJECT_ROOT, 'modules'), + File.join(PROJECT_ROOT, '.modules'), + ] + end + + before(:each) do + BoltSpec::Plans.init + # Sensitive[String[1]] / String[1] params default from these + ENV['GITHUB_API_TOKEN'] ||= 'spec-dummy-token' + ENV['JIRA_USER'] ||= 'spec-dummy-user' + ENV['JIRA_API_TOKEN'] ||= 'spec-dummy-token' + end +end diff --git a/spec/plans/puppetsync_plan_spec.rb b/spec/plans/puppetsync_plan_spec.rb new file mode 100644 index 0000000..e410a73 --- /dev/null +++ b/spec/plans/puppetsync_plan_spec.rb @@ -0,0 +1,125 @@ +require_relative 'plan_spec_helper' +require 'tmpdir' +require 'yaml' + +describe 'plan: puppetsync (dynamic inventory, #55)' do + include_context 'puppetsync plan specs' + + let(:puppetsync_config) do + { + 'puppetsync' => { + 'plans' => { + 'sync' => { + 'clone_git_repos' => false, + 'filter_permitted_repos' => false, + 'stages' => [], + }, + }, + }, + 'git' => { 'feature_branch' => 'SIMP-TEST' }, + } + end + + let(:repos_source) do + { 'org' => 'simp', 'include' => ['repo-*'] } + end + + let(:generated_config) do + { + 'https://github.com/simp/repo-x' => { 'branch' => 'main' }, + 'https://github.com/simp/repo-y' => { 'branch' => 'master' }, + } + end + + def unwrap(value) + value.respond_to?(:unwrap) ? value.unwrap : value + end + + around(:each) do |example| + Dir.mktmpdir do |dir| + @project_dir = dir + FileUtils.mkdir_p(File.join(dir, 'data', 'sync', 'repolists')) + example.run + end + end + + def run_sync(extra = {}) + run_plan('puppetsync', { + 'project_dir' => @project_dir, + 'config' => 'spec-run', + 'puppetsync_config' => puppetsync_config, + 'repos_config' => {}, + 'repos_source' => repos_source, + }.merge(extra)) + end + + it 'builds the inventory from the listing task and snapshots it as a normal repolist' do + allow_out_message + sources_seen = [] + expect_task('puppetsync::list_github_repos').with_targets(['localhost']).return do |targets:, task:, params:| + sources_seen << params.transform_values { |v| unwrap(v) } + Bolt::ResultSet.new(targets.map do |t| + Bolt::Result.new(t, value: { 'repos_config' => generated_config, 'count' => 2 }, action: 'task', object: task) + end) + end + + result = run_sync + + expect(result.ok?).to be(true), result.value.to_s + expect(sources_seen.first['source']).to eq(repos_source) + + snapshot = File.join(@project_dir, 'data', 'sync', 'repolists', 'generated-spec-run.yaml') + expect(File).to exist(snapshot) + expect(YAML.load_file(snapshot)).to eq('puppetsync::repos_config' => generated_config) + end + + it 'merges static repos_config entries on top of the generated list (static wins)' do + allow_out_message + allow_task('puppetsync::list_github_repos').return do |targets:, task:, params:| + Bolt::ResultSet.new(targets.map do |t| + Bolt::Result.new(t, value: { 'repos_config' => generated_config, 'count' => 2 }, action: 'task', object: task) + end) + end + static = { + 'https://github.com/simp/repo-x' => { 'branch' => 'static-override' }, + 'https://github.com/simp/repo-static' => { 'branch' => 'master' }, + } + + result = run_sync('repos_config' => static) + + expect(result.ok?).to be(true), result.value.to_s + snapshot = YAML.load_file(File.join(@project_dir, 'data', 'sync', 'repolists', 'generated-spec-run.yaml')) + merged = snapshot['puppetsync::repos_config'] + expect(merged['https://github.com/simp/repo-x']).to eq('branch' => 'static-override') + expect(merged.keys).to contain_exactly( + 'https://github.com/simp/repo-x', + 'https://github.com/simp/repo-y', + 'https://github.com/simp/repo-static', + ) + end + + it 'does not call the listing task in list_pipeline_stages mode' do + allow_out_message + # No task stubs declared: a listing call would raise + + result = run_sync('options' => { 'list_pipeline_stages' => true }) + + expect(result.ok?).to be(true), result.value.to_s + expect(Dir.glob(File.join(@project_dir, 'data', 'sync', 'repolists', 'generated-*'))).to be_empty + end + + it 'uses the static repos_config unchanged when no repos_source is given' do + allow_out_message + static = { 'https://github.com/simp/repo-static' => { 'branch' => 'master' } } + + result = run_plan('puppetsync', { + 'project_dir' => @project_dir, + 'config' => 'spec-run', + 'puppetsync_config' => puppetsync_config, + 'repos_config' => static, + }) + + expect(result.ok?).to be(true), result.value.to_s + expect(Dir.glob(File.join(@project_dir, 'data', 'sync', 'repolists', 'generated-*'))).to be_empty + end +end