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(openapi-tooling): add checks for API breaking changes #23370

Merged

Conversation

aramissennyeydd
Copy link
Contributor

@aramissennyeydd aramissennyeydd commented Mar 2, 2024

Hey, I just made a Pull Request!

This is the initial work to integrate with Optic's breaking changes checks. You can check the output from the artifacts of the API Breaking Changes (Trigger) job as comment.md. This builds on #18689 with better security.

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)
  • All your commits have a Signed-off-by line in the message. (more info)

@github-actions github-actions bot added the area:catalog Related to the Catalog Project Area label Mar 2, 2024
@backstage-goalie
Copy link
Contributor

backstage-goalie bot commented Mar 2, 2024

Missing Changesets

The following package(s) are changed by this PR but do not have a changeset:

  • @backstage/plugin-catalog-backend

See CONTRIBUTING.md for more information about how to add changesets.

Changed Packages

Package Name Package Path Changeset Bump Current Version
@backstage/repo-tools packages/repo-tools minor v0.8.0
@backstage/plugin-catalog-backend plugins/catalog-backend none v1.21.1

Copy link
Contributor

github-actions bot commented Mar 2, 2024

Uffizzi Cluster pr-23370 was deleted.

@aramissennyeydd
Copy link
Contributor Author

aramissennyeydd commented Mar 2, 2024

Summary for commit (5f549db)

1 API had breaking changes.

1 API had warnings.

APIs with Changes

API Changes Rules
/plugins/catalog-backend/ 1 operation changed
  `POST /entities/by-refs` (changed)
⚠️ **1**/**2** failed

APIs with Warnings

API Warning
/plugins/todo-backend/No check:api script found in package.json

Routes with Breakages

  • /plugins/catalog-backend/

    • POST /entities/by-refs request body: application/json property: fields
      cannot make a request property required. This is a breaking change.
      

@aramissennyeydd aramissennyeydd marked this pull request as ready for review March 2, 2024 23:35
Copy link
Contributor

github-actions bot commented Mar 9, 2024

This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution!

Copy link
Contributor

This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution!

@github-actions github-actions bot added the stale label Mar 30, 2024
@freben freben removed the stale label Mar 30, 2024
Copy link
Contributor

github-actions bot commented Apr 8, 2024

This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution!

@github-actions github-actions bot added the stale label Apr 8, 2024
Signed-off-by: aramissennyeydd <aramis.sennyey@doordash.com>
Signed-off-by: aramissennyeydd <aramis.sennyey@doordash.com>
Signed-off-by: web-next-automation <web-platform@doordash.com>
Signed-off-by: aramissennyeydd <aramis.sennyey@doordash.com>
Signed-off-by: aramissennyeydd <aramis.sennyey@doordash.com>
Signed-off-by: web-next-automation <web-platform@doordash.com>
Signed-off-by: aramissennyeydd <aramis.sennyey@doordash.com>
Signed-off-by: web-next-automation <web-platform@doordash.com>
aramissennyeydd and others added 13 commits April 8, 2024 20:58
Signed-off-by: aramissennyeydd <aramis.sennyey@doordash.com>
Signed-off-by: web-next-automation <web-platform@doordash.com>
Signed-off-by: aramissennyeydd <aramis.sennyey@doordash.com>
Signed-off-by: web-next-automation <web-platform@doordash.com>
Signed-off-by: aramissennyeydd <aramis.sennyey@doordash.com>
Signed-off-by: web-next-automation <web-platform@doordash.com>
Signed-off-by: aramissennyeydd <aramis.sennyey@doordash.com>
Signed-off-by: web-next-automation <web-platform@doordash.com>
Signed-off-by: aramissennyeydd <aramis.sennyey@doordash.com>
Signed-off-by: web-next-automation <web-platform@doordash.com>
Signed-off-by: aramissennyeydd <aramis.sennyey@doordash.com>
Signed-off-by: web-next-automation <web-platform@doordash.com>
Signed-off-by: aramissennyeydd <aramis.sennyey@doordash.com>
Signed-off-by: web-next-automation <web-platform@doordash.com>
Signed-off-by: aramissennyeydd <aramis.sennyey@doordash.com>
Signed-off-by: web-next-automation <web-platform@doordash.com>
Signed-off-by: aramissennyeydd <aramis.sennyey@doordash.com>
Signed-off-by: web-next-automation <web-platform@doordash.com>
Signed-off-by: aramissennyeydd <aramis.sennyey@doordash.com>
Signed-off-by: web-next-automation <web-platform@doordash.com>
Signed-off-by: aramissennyeydd <aramis.sennyey@doordash.com>
Signed-off-by: web-next-automation <web-platform@doordash.com>
Signed-off-by: Aramis Sennyey <159921952+aramissennyeydd@users.noreply.github.com>
Signed-off-by: Aramis Sennyey <159921952+aramissennyeydd@users.noreply.github.com>
Signed-off-by: web-next-automation <web-platform@doordash.com>
@aramissennyeydd aramissennyeydd force-pushed the openapi-tooling/breaking-changes branch from 0f78254 to 5d091a6 Compare April 9, 2024 00:59
web-next-automation added 3 commits April 8, 2024 21:01
Signed-off-by: web-next-automation <web-platform@doordash.com>
Signed-off-by: web-next-automation <web-platform@doordash.com>
Signed-off-by: web-next-automation <web-platform@doordash.com>
@github-actions github-actions bot removed the stale label Apr 9, 2024
Copy link
Contributor

@camilaibs camilaibs left a comment

Choose a reason for hiding this comment

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

This is super helpful, thanks for opening this pull request!
Just left non-blocking comments 🙌🏻

# Fetch output (zip archive) from the workflow run that triggered this workflow.
uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1
with:
script: |
Copy link
Contributor

@camilaibs camilaibs Apr 11, 2024

Choose a reason for hiding this comment

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

This script looks similar to what we are doing on the uffizzi-preview action here, would it be a good idea to put it in /scripts and reuse it somehow? BTW, I'm not sure if what I suggested is recommended, let me check with @backstage/maintainers 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we could? The concern is users being able to edit the script and grab credentials, but I think a script in the secondary action would be fine as it would run from master.

results.completed.length > 0
? `### APIs with Changes

<table>
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed that the text is a bit hard to read because there's no indentation or syntax highlighting.... Is it possible to use a template? It would be nice to have this if it's possible and doesn't require too much effort. It might not be changed frequently, so I understand if it's not worth it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree it's not super readable, but I pulled this pretty much intact from the Optic codebase and I'd prefer to keep it as is for now to handle any updates to keep it in line with the output from Optic CLI.

@@ -42,6 +42,7 @@
],
"scripts": {
"build": "backstage-cli package build",
"check:api": "backstage-repo-tools package schema openapi check",
Copy link
Contributor

@camilaibs camilaibs Apr 12, 2024

Choose a reason for hiding this comment

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

Nit: This is something that may be not noticed by plugin developers who don't see the changelog. Is there a way to show what's new in this cli? For example, if you use the verify command log that there is a new check command as well, or perhaps other ideas to make this more visible to other plugin developers since it's optional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't put much thought into it, new commands in CLI seems like a hard problem to tackle, any examples of what you're suggesting that you could point me to?

@@ -49,6 +49,7 @@
"@stoplight/spectral-rulesets": "^1.18.0",
"@stoplight/spectral-runtime": "^1.1.2",
"@stoplight/types": "^14.0.0",
"@useoptic/openapi-utilities": "^0.54.8",
Copy link
Contributor

Choose a reason for hiding this comment

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

Curiosity: The community version of optic appears to be sufficient for us. Do you have any concerns?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure I'm following, wdym?

packages/repo-tools/src/commands/index.ts Outdated Show resolved Hide resolved
Signed-off-by: aramissennyeydd <aramis.sennyey@doordash.com>
Signed-off-by: aramissennyeydd <aramis.sennyey@doordash.com>
Copy link
Member

@freben freben left a comment

Choose a reason for hiding this comment

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

Alright, I'd say let's try this out and then tweak if needed. Maybe consolidate the scripts or so if we feel it gets unwieldy. But for now ... happy to merge and iterate

@freben freben merged commit b379061 into backstage:master May 2, 2024
36 checks passed
Copy link
Contributor

github-actions bot commented May 2, 2024

Thank you for contributing to Backstage! The changes in this pull request will be part of the 1.27.0 release, scheduled for Tue, 14 May 2024.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:catalog Related to the Catalog Project Area
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants