Skip to content

Conversation

@JakeSCahill
Copy link
Contributor

This pull request refactors the GitHub Actions workflow for API documentation deployment and diffing, focusing on modularity and maintainability. Overlay detection logic has been moved out of the workflow YAML into dedicated shell scripts, and admin API version detection is now handled by a script. The workflow steps for overlays and admin version handling are simplified and made more robust, with additional validation and testing scripts included. AWS credential usage is clarified and limited to appropriate jobs.

Workflow and Script Refactoring

  • Moved overlay detection logic from inline bash in .github/workflows/bump.yml to a new script .github/scripts/determine-overlays.sh, which dynamically determines overlays for both admin and non-admin docs. Workflow steps now call this script instead of duplicating logic. [1] [2] [3]
  • Added .github/scripts/detect-admin-versions.sh to automatically detect available admin API versions and output them as JSON. This is now called in the workflow to set up admin version branches and overlays. [1] [2] [3]
  • Updated workflow overlay references so that admin v1/v2 overlays use their respective step outputs (steps.admin-v1-overlays.outputs.overlay_paths and steps.admin-v2-overlays.outputs.overlay_paths), rather than the generic overlays step. [1] [2]

Testing and Validation

  • Added .github/scripts/test-overlays.sh to test overlay detection for various document types and edge cases, ensuring overlays are correctly found and errors are handled.
  • Added .github/scripts/validate-workflow.sh to validate the workflow file for YAML syntax, required script existence, correct step output references, conditional logic, and AWS credential usage.

AWS Credentials Usage

  • Removed AWS credentials setup from the determine-doc-ids job, restricting AWS usage to deploy and diff jobs only, in line with best practices and workflow validation. [1] [2]

These changes make the workflow easier to maintain and extend, reduce duplication, and improve error handling and validation.

@JakeSCahill JakeSCahill requested a review from a team as a code owner November 11, 2025 10:38
@coderabbitai
Copy link

coderabbitai bot commented Nov 11, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Walkthrough

This pull request introduces four new helper scripts to support admin API versioning and overlay management in the documentation build process. The detect-admin-versions.sh script scans the filesystem for admin API version files and outputs them as JSON. The determine-overlays.sh script computes which overlay YAML files apply to a given documentation target, supporting distinct overlay paths for admin versioned documentation versus standard documentation. A new test-overlays.sh script validates the overlay detection logic and admin version detection across multiple targets. The validate-workflow.sh script performs automated validation of the bump workflow YAML syntax, script availability, and configuration correctness. The bump workflow itself is refactored to move AWS credentials configuration into individual jobs, delegate overlay determination to the helper script, and introduce separate processing paths for admin v1 and admin v2 documentation.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • validate-workflow.sh: Contains 7 distinct test cases with YAML parsing logic and conditional checks; requires understanding of what the workflow should enforce
  • determine-overlays.sh: Logic branches on doc_id type (admin, cloud-, default) with multiple overlay collection paths; requires understanding of overlay naming conventions and directory structures
  • bump.yml refactor: Substantial restructuring with conditional blocks for admin vs. non-admin paths, AWS credential movement between jobs, and changes to how outputs are named and referenced throughout the workflow
  • Integration complexity: New scripts must be correctly invoked by the workflow with proper parameter passing and output handling; any misalignment breaks the CI pipeline

Possibly related PRs

  • PR #25: Overlapping changes to .github/workflows/bump.yml for separate admin v1/v2 deployment paths and Admin API version handling

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main objectives of the changeset: refactoring overlay detection logic and eliminating duplication in the workflow.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, detailing workflow refactoring, new scripts, and AWS credential reorganization.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

@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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 64b8ef3 and be5d8d9.

📒 Files selected for processing (5)
  • .github/scripts/detect-admin-versions.sh (1 hunks)
  • .github/scripts/determine-overlays.sh (1 hunks)
  • .github/scripts/test-overlays.sh (1 hunks)
  • .github/scripts/validate-workflow.sh (1 hunks)
  • .github/workflows/bump.yml (6 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/bump.yml

94-94: shellcheck reported issue in this script: SC2086:info:2:35: Double quote to prevent globbing and word splitting

(shellcheck)


102-102: shellcheck reported issue in this script: SC2086:info:2:35: Double quote to prevent globbing and word splitting

(shellcheck)


110-110: shellcheck reported issue in this script: SC2086:info:2:35: Double quote to prevent globbing and word splitting

(shellcheck)


199-199: shellcheck reported issue in this script: SC2086:info:2:35: Double quote to prevent globbing and word splitting

(shellcheck)


207-207: shellcheck reported issue in this script: SC2086:info:2:35: Double quote to prevent globbing and word splitting

(shellcheck)


215-215: shellcheck reported issue in this script: SC2086:info:2:35: Double quote to prevent globbing and word splitting

(shellcheck)

🔇 Additional comments (7)
.github/scripts/test-overlays.sh (1)

1-94: Test script is well-structured and comprehensive. The overlay detection tests correctly verify file existence, and edge case handling (empty doc_id should error, admin without version should return empty) aligns with determine-overlays.sh behavior. The admin version detection test properly parses JSON output via jq.

.github/scripts/detect-admin-versions.sh (1)

1-31: Version detection and JSON output are correct. The script properly handles the v1 base case and extracts versions from versioned files using filename parsing. The nullglob safety and jq array composition are well-executed.

.github/scripts/determine-overlays.sh (1)

1-67: Overlay determination logic is sound. The script correctly branches on admin vs non-admin, handles versioning, and uses the comma-separated list format consistently. The prefix-based routing for cloud vs self-managed docs is a clean approach.

.github/scripts/validate-workflow.sh (1)

1-133: Workflow validation script is comprehensive for basic checks. The tests cover YAML syntax, script availability, step IDs, output references, conditionals, and AWS credential placement. The grep-based validation is somewhat brittle (e.g., grep patterns for conditionals at lines 79, 87 may miss variations in quoting or formatting), but appropriate for a CI validation script.

.github/workflows/bump.yml (3)

60-69: AWS credentials correctly moved into individual jobs. The placement in deploy-doc and api-diff jobs (rather than determine-doc-ids) aligns with the principle of least privilege and matches the PR objective to limit credentials to only the jobs that need them. This also removes unnecessary credential overhead from the matrix-determining job.

Also applies to: 165-174


77-77: Admin vs non-admin branching via conditionals is clear and correct. The use of if: ${{ matrix.doc_id != 'admin' }} and if: ${{ matrix.doc_id == 'admin' }} cleanly separates overlay determination and deployment for admin v1, admin v2, and non-admin documentation. The pattern is consistently applied across both deploy-doc and api-diff jobs.

Also applies to: 93-93, 101-101, 109-109, 145-145, 182-182, 198-198, 206-206, 252-252


99-114: Overlay determination steps correctly delegate to helper scripts. Each branch invokes determine-overlays.sh with the appropriate doc_id and optional admin-version parameter, stores the result in outputs, and feeds it to the bump-sh action. The separation of admin v1, admin v2, and non-admin overlays improves clarity and maintainability.

Please verify that the following are in place before merging:

  1. Script executability: Confirm .github/scripts/determine-overlays.sh, detect-admin-versions.sh, test-overlays.sh, and validate-workflow.sh are marked as executable (mode 755) in the repository, otherwise the workflow will fail with "Permission denied" errors.

  2. Overlay output format compatibility: The scripts output comma-separated file paths (e.g., admin/v1-overlays/foo.yaml,shared/overlays/sm-bar.yaml). Verify that the bump-sh GitHub Action (bump-sh/github-action@v1) correctly interprets this format for the overlay: parameter. Check the action's documentation to confirm it expects comma-separated paths or if a different delimiter is required.

  3. Script availability at runtime: Ensure that .github/scripts/determine-overlays.sh is present and executable when the workflow runs (the scripts must be committed to the repository and the git checkout in the workflow must fetch them).

Also applies to: 204-219

@JakeSCahill JakeSCahill merged commit 68a3db0 into main Nov 11, 2025
8 checks passed
@JakeSCahill JakeSCahill deleted the fix-overlays branch November 11, 2025 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants