Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: automated Open API syntax validation on submitting PR to master #443

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

homosapien14
Copy link

@homosapien14 homosapien14 commented Aug 1, 2024

Description (fixes #366 )

Problem:
Sometimes when pull requests are merged, syntactical errors in the OpenAPI document get committed to the master branch, which can lead to integration issues and broken functionality.

Feature Request:
Implement an automated script that validates OpenAPI specifications when a pull request is created. The script should specifically validate transaction.yaml, meta.yaml, and registry.yaml against the OpenAPI 3.0 specification.

Goals:

  • Create an automated workflow that triggers on pull requests and pushes to the master branch.
  • Validate transaction.yaml, meta.yaml, and registry.yaml to ensure they conform to the OpenAPI 3.0 specification.

Solution

How I Solved It:

Workflow Configuration:

  • Configured a GitHub Actions workflow to run on both push events to the master branch and pull requests targeting the master branch.
  • Added steps to:
    • Checkout the code.
    • Set up Node.js.
    • Install the openapi-cli tool globally.
    • Ensure the validation script (validate-openapi.sh) is executable.
    • Execute the validation script to check OpenAPI files for syntax errors.

Validation Script:

  • Created validate-openapi.sh to iterate over the OpenAPI files (transaction.yaml, meta.yaml, and registry.yaml).
  • The script uses openapi lint to check for errors and warnings. If any issues are found, it reports them and exits with a non-zero status.

Expected Outcome

  • Specs on Master Branch:
    • The OpenAPI specifications on the master branch should not have any syntax errors after a successful PR merge.

Acceptance Criteria

  • Error Handling:
    • The workflow should fail if syntax errors are present in the OpenAPI spec during a PR check.
  • Success Criteria:
    • The workflow should pass if no syntax errors are found in the OpenAPI spec during a PR check.

How to Test

  1. Create a Pull Request:

    • Make changes to the OpenAPI files (transaction.yaml, meta.yaml, registry.yaml) in a new branch and open a pull request targeting the master branch.
  2. Check Workflow Execution:

    • Ensure the GitHub Actions workflow runs automatically.
    • Verify that the validation steps execute correctly.
  3. Test with Errors:

    • Introduce syntactical errors in the OpenAPI files and observe if the workflow fails and reports the errors.
  4. Test without Errors:

    • Ensure that a PR with valid OpenAPI files passes the checks successfully without any errors.

@homosapien14
Copy link
Author

@ravi-prakash-v Hii , could you please review it once.

@homosapien14
Copy link
Author

@rajaneeshk90 can you please look at this?

@VedantKhairnar
Copy link

Hello @harshcrop
Will you pls review the contribution?
Thanks.

@homosapien14
Copy link
Author

hey @harshcrop , can you even give me a feedback?

@rajaneeshk90
Copy link
Collaborator

rajaneeshk90 commented Sep 3, 2024

Hi @homosapien14,

Thank you for submitting the PR! I appreciate your effort.

I have a few suggestions that could help improve the submission:

  • Could you please provide more detailed steps to verify that the changes are working as intended?
  • The changes to the api/meta/build/meta.yaml file aren't necessary, so they can be removed.
  • It would be better to avoid using a shell script for the validation process.

Thanks again for your contribution!

Best regards,
Rajaneesh

@VedantKhairnar
Copy link

Hello @homosapien14
Any update on this?
Also, Augtoberfest ends on 15th Sept, so it would be great if we finish this before that.
Thanks.

@homosapien14
Copy link
Author

homosapien14 commented Sep 15, 2024

Hi @rajaneeshk90, Thank you for your response. I am sorry for the delay (I was busy with the office project deadline).

I have made the required changes as you suggested in this 82fa17b commit.

How to Test the Changes

  1. Create a New Branch:

    • First, create a new branch from swapnil/openapispecs.
    • Update the push branches in the validate-openapi.yml file to your new branch name (xyz).
    • Push the changes to the remote repository.
  2. Test the Workflow:

    • Create a new branch from master and make invalid changes (e.g., introduce syntax errors or remove required lines) in the meta.yaml, registry.yaml, or transaction.yaml files.
  3. Push and Create a Draft PR:

    • Push the invalid changes to the remote repository.
    • Create a draft pull request with the target branch set to xyz.
  4. Check the CI Workflow:

    • The GitHub Actions workflow will run automatically.
    • It should throw an error in the CI step if there are any issues with the OpenAPI files, such as syntax problems or missing fields.

Here are the sample errors while I was testing the implementation:

image

testing1

@rajaneeshk90 rajaneeshk90 self-requested a review January 15, 2025 10:51
@rajaneeshk90
Copy link
Collaborator

rajaneeshk90 commented Jan 15, 2025

Hi @homosapien14,

Thank you for the changes in the PR!

Here are a few points that need attention:

  1. The statement exists('api/meta/build/meta.yaml') is not valid and will cause an error.
  2. Sections like servers and paths.<method>.summary are technically optional in OpenAPI, but Redocly is strict and treats them as required, reporting them as errors. While these are not hard requirements in OpenAPI, Redocly might not be the best fit for validation here. You might want to consider using swagger-api/swagger-validator-action as an alternative.
  3. It would be beneficial to change the workflow name to something more descriptive and relevant.
  4. The workflow should also be triggered on the draft branch.
  5. We can include the "paths" subsection inside the "push" section as well.
  6. It’s a good idea to check if the latest version (v3) of these tools is available and use that version for compatibility.
  7. The specification files are now using OpenAPI 3.1. We should validate the files against OpenAPI version 3.1 in the run: openapi lint api/meta/build/meta.yaml step.

Thanks again for your contribution!

Best,
Rajaneesh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need automated Open API syntax validation on submitting PR to master
4 participants