Skip to content

Remove alias / duplicate spec.yml files#2719

Merged
aramprice merged 1 commit intomainfrom
spec-file-cleanup
Apr 28, 2026
Merged

Remove alias / duplicate spec.yml files#2719
aramprice merged 1 commit intomainfrom
spec-file-cleanup

Conversation

@aramprice
Copy link
Copy Markdown
Member

The bosh convention is ${JOB}/spec.

Copilot AI review requested due to automatic review settings April 28, 2026 00:55
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 28, 2026

Walkthrough

Several job spec YAML files were removed or emptied (jobs/blobstore/spec.yml, jobs/director/spec.yml, jobs/health_monitor/spec.yml, jobs/nats/spec.yml, jobs/postgres-13/spec.yml, jobs/postgres/spec.yml). Health-monitor and NATS job specs and templates were changed to split a single CA certificate into separate director and UAA CA cert templates/properties. Multiple libraries (bosh-monitor, bosh-nats-sync) and their tests were updated to accept separate director_ca_cert and uaa_ca_cert, add CA-file selection logic, and adjust some auth provider/UAAToken method signatures and logging.

Suggested reviewers

  • mkocher
  • ystros
  • neddp
  • KauzClay
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is severely incomplete. It provides only a single sentence explaining the convention, but omits all required template sections including contextual information, testing details, release notes guidance, breaking change assessment, and team tags. Complete the description by adding sections for contextual information, comprehensive test results, release notes guidance, breaking change assessment, and appropriate team/reviewer tags.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: removing duplicate spec.yml files to follow BOSH conventions that spec files should be located at ${JOB}/spec instead of ${JOB}/spec.yml.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch spec-file-cleanup

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aligns the release with the BOSH job convention of using ${JOB}/spec by removing duplicate spec.yml files and updating the integration helper to read from the extensionless spec file.

Changes:

  • Update Postgres version integration helper to load jobs/postgres/spec instead of jobs/postgres/spec.yml.
  • Remove duplicate job spec YAML files (jobs/postgres/spec.yml, jobs/postgres-13/spec.yml) now that jobs/*/spec exists.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

File Description
src/spec/integration_support/postgres_version_helper.rb Switches job spec path lookup from spec.yml to spec and updates the mismatch error message.
jobs/postgres/spec.yml Removes duplicate job spec file in favor of jobs/postgres/spec.
jobs/postgres-13/spec.yml Removes duplicate job spec file in favor of jobs/postgres-13/spec.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/spec/integration_support/postgres_version_helper.rb Outdated
coderabbitai[bot]
coderabbitai Bot previously requested changes Apr 28, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/spec/integration_support/postgres_version_helper.rb`:
- Line 11: The raised error message is misleading: it uses the wrong path string
and swaps the reported values. Update the raise string so the path reads
something like "jobs/postgres_spec" (replace the current "jobs/postgres?spec")
and map the variables correctly so the spec/release value uses release_version
and the local value uses local_version (i.e., "Postgres major version mismatch:
jobs/postgres_spec: #{release_version}; local: #{local_version}."). Ensure you
modify the raise call that references local_version and release_version in
postgres_version_helper.rb.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: d12a146a-8803-4132-a752-ddf65bd5ad3e

📥 Commits

Reviewing files that changed from the base of the PR and between 9b93b34 and b76db88.

📒 Files selected for processing (7)
  • jobs/blobstore/spec.yml
  • jobs/director/spec.yml
  • jobs/health_monitor/spec.yml
  • jobs/nats/spec.yml
  • jobs/postgres-13/spec.yml
  • jobs/postgres/spec.yml
  • src/spec/integration_support/postgres_version_helper.rb
💤 Files with no reviewable changes (6)
  • jobs/health_monitor/spec.yml
  • jobs/nats/spec.yml
  • jobs/director/spec.yml
  • jobs/blobstore/spec.yml
  • jobs/postgres-13/spec.yml
  • jobs/postgres/spec.yml

Comment thread src/spec/integration_support/postgres_version_helper.rb Outdated
The bosh convention is `${JOB}/spec`.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
src/spec/integration_support/postgres_version_helper.rb (1)

10-12: ⚠️ Potential issue | 🟡 Minor

Swap the reported versions in the mismatch message.

The path is now correct, but the values are still reversed: jobs/postgres/spec is printing local_version, and local is printing release_version. That makes the exception misleading when the versions diverge.

Proposed fix
-          raise "Postgres major version mismatch: jobs/postgres/spec: #{local_version}; local: #{release_version}."
+          raise "Postgres major version mismatch: jobs/postgres/spec: #{release_version}; local: #{local_version}."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/spec/integration_support/postgres_version_helper.rb` around lines 10 -
12, The error message raised when major versions mismatch is printing the
versions reversed; in the raise call inside the block that checks
local_major_version == release_major_version, swap the interpolated variables so
the "jobs/postgres/spec:" segment shows release_version and the "local:" segment
shows local_version; update the raise string in the method that uses
local_major_version, release_major_version, local_version and release_version
accordingly to reflect correct values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/spec/integration_support/postgres_version_helper.rb`:
- Around line 10-12: The error message raised when major versions mismatch is
printing the versions reversed; in the raise call inside the block that checks
local_major_version == release_major_version, swap the interpolated variables so
the "jobs/postgres/spec:" segment shows release_version and the "local:" segment
shows local_version; update the raise string in the method that uses
local_major_version, release_major_version, local_version and release_version
accordingly to reflect correct values.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1b6da530-aaa9-4fd3-bf90-77f08b40ea26

📥 Commits

Reviewing files that changed from the base of the PR and between b76db88 and 7d34d09.

📒 Files selected for processing (7)
  • jobs/blobstore/spec.yml
  • jobs/director/spec.yml
  • jobs/health_monitor/spec.yml
  • jobs/nats/spec.yml
  • jobs/postgres-13/spec.yml
  • jobs/postgres/spec.yml
  • src/spec/integration_support/postgres_version_helper.rb
💤 Files with no reviewable changes (6)
  • jobs/director/spec.yml
  • jobs/health_monitor/spec.yml
  • jobs/blobstore/spec.yml
  • jobs/nats/spec.yml
  • jobs/postgres/spec.yml
  • jobs/postgres-13/spec.yml

coderabbitai[bot]
coderabbitai Bot previously approved these changes Apr 28, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@spec/health_monitor_templates_spec.rb`:
- Around line 73-74: Replace the weak type assertions on
parsed_yaml['director']['director_ca_cert'] and
parsed_yaml['director']['uaa_ca_cert'] with exact path equality checks:
determine the expected absolute file paths used by the template (or derive them
via the same helper/fixture used in the spec setup) and assert
parsed_yaml['director']['director_ca_cert'].to eq(expected_director_ca_path) and
parsed_yaml['director']['uaa_ca_cert'].to eq(expected_uaa_ca_path) so the spec
verifies the rendered paths rather than just their types.

In `@src/bosh-nats-sync/lib/nats_sync/auth_provider.rb`:
- Line 24: Replace the use of Base64.encode64 when building the HTTP Basic
Authorization header so it doesn't insert newlines; specifically change the call
to Base64.strict_encode64 for the value constructed in auth_provider.rb (the
string building that currently uses
Base64.encode64("#{`@user`}:#{`@password`}").strip) so the Authorization header
generation in the auth provider (where "Basic #{...}" is composed) emits a
single-line RFC 4648-compliant base64 value.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3e849299-1d38-4582-8135-4344d33a02d8

📥 Commits

Reviewing files that changed from the base of the PR and between 7d34d09 and 389d883.

📒 Files selected for processing (23)
  • jobs/health_monitor/spec
  • jobs/health_monitor/templates/director_ca_cert.pem.erb
  • jobs/health_monitor/templates/health_monitor.yml.erb
  • jobs/health_monitor/templates/uaa.pem.erb
  • jobs/health_monitor/templates/uaa_ca_cert.pem.erb
  • jobs/nats/spec
  • jobs/nats/templates/bosh_nats_sync_config.yml.erb
  • jobs/nats/templates/director_ca_cert.pem.erb
  • jobs/nats/templates/uaa.pem.erb
  • jobs/nats/templates/uaa_ca_cert.pem.erb
  • spec/health_monitor_templates_spec.rb
  • spec/nats_templates_spec.rb
  • src/bosh-monitor/lib/bosh/monitor/auth_provider.rb
  • src/bosh-monitor/spec/unit/bosh/monitor/auth_provider_spec.rb
  • src/bosh-monitor/spec/unit/bosh/monitor/director_spec.rb
  • src/bosh-monitor/spec/unit/bosh/monitor/plugins/event_logger_spec.rb
  • src/bosh-monitor/spec/unit/bosh/monitor/plugins/resurrector_spec.rb
  • src/bosh-nats-sync/lib/nats_sync/auth_provider.rb
  • src/bosh-nats-sync/lib/nats_sync/users_sync.rb
  • src/bosh-nats-sync/spec/assets/sample_config.yml
  • src/bosh-nats-sync/spec/nats_sync/auth_provider_spec.rb
  • src/bosh-nats-sync/spec/nats_sync/runner_spec.rb
  • src/bosh-nats-sync/spec/nats_sync/users_sync_spec.rb
💤 Files with no reviewable changes (2)
  • jobs/health_monitor/templates/uaa.pem.erb
  • jobs/nats/templates/uaa.pem.erb

Comment thread spec/health_monitor_templates_spec.rb Outdated
Comment thread src/bosh-nats-sync/lib/nats_sync/auth_provider.rb Outdated
@github-project-automation github-project-automation Bot moved this from Pending Merge | Prioritized to Waiting for Changes | Open for Contribution in Foundational Infrastructure Working Group Apr 28, 2026
@github-project-automation github-project-automation Bot moved this from Waiting for Changes | Open for Contribution to Pending Merge | Prioritized in Foundational Infrastructure Working Group Apr 28, 2026
@aramprice aramprice merged commit 9b8764a into main Apr 28, 2026
36 checks passed
@aramprice aramprice deleted the spec-file-cleanup branch April 28, 2026 23:56
@github-project-automation github-project-automation Bot moved this from Pending Merge | Prioritized to Done in Foundational Infrastructure Working Group Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants