ci: validation of generated OpenAPI schema (#604) #609
Conversation
WalkthroughThe changes in this pull request introduce a new job named Changes
Poem
Warning Tool Failures:Tool Failure Count:Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
holtgrewe
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (4)
openapi.schema.yaml (3)
1-10: Consider enhancing API metadata documentation.While the basic metadata is present, consider adding:
termsOfServiceURL- Extended API description including usage examples
- External documentation links
🧰 Tools
🪛 checkov
[HIGH] 1-93: Ensure that the global security field has rules defined
(CKV_OPENAPI_4)
[HIGH] 1-93: Ensure that security operations is not empty.
(CKV_OPENAPI_5)
35-38: Remove implementation detail from schema description.The description mentions it's "Copy from annonars with extension to derive
utoipa::ToSchema", which is an implementation detail that shouldn't be in the API documentation.description: |- Assembly to be passed on the command line. - - Copy from annonars with extension to derive `utoipa::ToSchema`.
1-91: Consider API versioning strategy.The current endpoint path includes
/v1/which indicates version 1, but the OpenAPI spec version 0.29.6 suggests this is still pre-1.0. Consider:
- Aligning the API version with the software version
- Documenting the versioning strategy in the API description
- Adding a deprecation policy for future version changes
🧰 Tools
🪛 checkov
[HIGH] 1-93: Ensure that the global security field has rules defined
(CKV_OPENAPI_4)
[HIGH] 1-93: Ensure that security operations is not empty.
(CKV_OPENAPI_5)
.github/workflows/rust.yml (1)
129-131: Consider parallel execution with other jobs.The
Schemajob currently only depends onFormatting. Since schema validation is independent of testing and linting, consider making it a parallel job to potentially reduce overall workflow execution time.Schema: - needs: Formatting + needs: [] runs-on: ubuntu-24.04
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
.github/workflows/rust.yml(1 hunks)openapi.schema.yaml(1 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/rust.yml
143-143: the runner of "actions-rs/toolchain@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 checkov
openapi.schema.yaml
[HIGH] 1-93: Ensure that the global security field has rules defined
(CKV_OPENAPI_4)
[HIGH] 1-93: Ensure that security operations is not empty.
(CKV_OPENAPI_5)
| paths: | ||
| /api/v1/versionsInfo: | ||
| get: | ||
| tags: | ||
| - versions | ||
| summary: Query for consequence of a variant. | ||
| operationId: versionsInfo | ||
| responses: | ||
| '200': | ||
| description: Version information. | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/VersionsInfoResponse' | ||
| '500': | ||
| description: Internal server error. | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/CustomError' |
There was a problem hiding this comment.
Add security definitions to the API specification.
The API specification lacks security definitions. Consider adding appropriate security schemes (e.g., API key, OAuth2, or Bearer token) to protect the endpoint.
Add the following security definitions:
components:
securitySchemes:
bearerAuth:
type: http
scheme: bearer
bearerFormat: JWT
security:
- bearerAuth: []Then, apply security to the endpoint:
/api/v1/versionsInfo:
get:
security:
- bearerAuth: []
# ... rest of the endpoint definition| get: | ||
| tags: | ||
| - versions | ||
| summary: Query for consequence of a variant. |
There was a problem hiding this comment.
Fix incorrect endpoint summary.
The summary "Query for consequence of a variant" doesn't match the endpoint's purpose of providing version information.
- summary: Query for consequence of a variant.
+ summary: Retrieve version information for the service.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| summary: Query for consequence of a variant. | |
| summary: Retrieve version information for the service. |
| description: Version of the `hgvs` crate. | ||
| VersionsInfoResponse: | ||
| type: object | ||
| description: Response of the `/v1/version` endpoint. |
There was a problem hiding this comment.
Fix incorrect endpoint path in description.
The description references /v1/version but the actual endpoint is /api/v1/versionsInfo.
- description: Response of the `/v1/version` endpoint.
+ description: Response of the `/api/v1/versionsInfo` endpoint.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| description: Response of the `/v1/version` endpoint. | |
| description: Response of the `/api/v1/versionsInfo` endpoint. |
.github/workflows/rust.yml
Outdated
| - name: Compare YAML in git to the one just generated | ||
| run: diff openapi.schema.yml /tmp/openapi.schema.yml |
There was a problem hiding this comment.
Fix inconsistent file extensions.
There's an inconsistency in file extensions between steps:
- Line 154: Uses
.yaml - Line 157: Uses
.yml - Line 162: Uses
.yml
This could cause the diff and validation steps to fail.
- name: Compare YAML in git to the one just generated
- run: diff openapi.schema.yml /tmp/openapi.schema.yml
+ run: diff openapi.schema.yaml /tmp/openapi.schema.yaml
- name: Validate OpenAPI schema
uses: thiyagu06/openapi-validator-action@v1
with:
- filepath: /tmp/openapi.schema.yml
+ filepath: /tmp/openapi.schema.yamlAlso applies to: 159-162
| - name: Install stable toolchain | ||
| uses: actions-rs/toolchain@v1 | ||
| with: | ||
| toolchain: stable | ||
| override: true | ||
|
|
There was a problem hiding this comment.
Update outdated GitHub Action.
The actions-rs/toolchain@v1 action is outdated. Consider updating to a newer version or switching to dtolnay/rust-toolchain@stable which is more actively maintained.
- - name: Install stable toolchain
- uses: actions-rs/toolchain@v1
- with:
- toolchain: stable
- override: true
+ - name: Install stable toolchain
+ uses: dtolnay/rust-toolchain@stable📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Install stable toolchain | |
| uses: actions-rs/toolchain@v1 | |
| with: | |
| toolchain: stable | |
| override: true | |
| - name: Install stable toolchain | |
| uses: dtolnay/rust-toolchain@stable | |
🧰 Tools
🪛 actionlint
143-143: the runner of "actions-rs/toolchain@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
| - name: Write schema | ||
| run: cargo run -- server schema --output-file /tmp/openapi.schema.yaml |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for schema generation.
The schema generation step should include error handling to ensure the command's success is properly checked.
- - name: Write schema
- run: cargo run -- server schema --output-file /tmp/openapi.schema.yaml
+ - name: Write schema
+ run: |
+ set -eo pipefail
+ cargo run -- server schema --output-file /tmp/openapi.schema.yaml📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Write schema | |
| run: cargo run -- server schema --output-file /tmp/openapi.schema.yaml | |
| - name: Write schema | |
| run: | | |
| set -eo pipefail | |
| cargo run -- server schema --output-file /tmp/openapi.schema.yaml |
.github/workflows/rust.yml
Outdated
| - name: Validate OpenAPI schema | ||
| uses: thiyagu06/openapi-validator-action@v1 | ||
| with: | ||
| filepath: /tmp/openapi.schema.yml |
There was a problem hiding this comment.
💡 Codebase verification
Pin OpenAPI validator action to specific commit hash
The schema validation setup needs improvement for security:
- The OpenAPI schema file is properly configured with OpenAPI 3.0.3
- The validator action is only pinned to major version
v1which is not secure - No explicit validation rules are configured, using defaults
To enhance security:
- Replace
thiyagu06/openapi-validator-action@v1with a specific commit hash - Consider adding validation rules via the action's
rulesinput parameter
🔗 Analysis chain
Verify OpenAPI validator version and configuration.
The OpenAPI validator is using a third-party action. Consider:
- Pinning to a specific version hash for security
- Adding validator configuration options for schema version and validation rules
Let me check the content of the OpenAPI schema file and look for any version or validation configurations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any OpenAPI schema files and validation configurations
echo "Searching for OpenAPI schema files..."
fd -e yaml -e yml . | grep -i 'openapi\|swagger'
echo "Checking for validator configurations..."
fd -g '*openapi*' -g '*swagger*' .
Length of output: 361
Script:
#!/bin/bash
# Check OpenAPI schema file content and validator usage
echo "OpenAPI schema content:"
cat openapi.schema.yaml | head -n 10
echo -e "\nChecking validator usage in workflows:"
rg "openapi-validator" .github/workflows/
Length of output: 523
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #609 +/- ##
==========================================
- Coverage 73.40% 73.38% -0.03%
==========================================
Files 28 28
Lines 9948 9948
==========================================
- Hits 7302 7300 -2
- Misses 2646 2648 +2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
.github/workflows/rust.yml (2)
129-131: Consider adding dependency on Testing job.The
Schemajob currently only depends onFormatting, but it requires a successful build to generate the schema. Consider addingTestingas a dependency to ensure the code builds successfully before attempting schema generation.Schema: - needs: Formatting + needs: [Formatting, Testing] runs-on: ubuntu-24.04
152-154: Add cargo caching for faster builds.The schema generation step requires building the project, but cargo caching is not configured. Add the Swatinem/rust-cache action to speed up subsequent runs.
- name: Setup protoc uses: arduino/[email protected] with: repo-token: ${{ secrets.GITHUB_TOKEN }} + - uses: Swatinem/rust-cache@v2 + with: + cache-directories: | + ~/.cargo/registry/src/**/librocksdb-sys-* - name: Write schema run: cargo run -- server schema --output-file /tmp/openapi.schema.yaml
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/rust.yml(1 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/rust.yml
143-143: the runner of "actions-rs/toolchain@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
Summary by CodeRabbit
New Features
GET /api/v1/versionsInfofor variant effect predictions.Documentation