feat(catalog-client): Support generating a front end client based on an OpenAPI spec by sennyeya · Pull Request #17470 · backstage/backstage

sennyeya · 2023-04-20T15:35:57Z

Hey, I just made a Pull Request!

Summary

Another step for #2566. There's a lot of code with this PR, the basic idea is to support using openapi-generator with some overrides to create a "closer to the spec" API client.

What's in this PR?

The primary work is in the packages/repo-tools/templates which is a set of overrides to the Mustache templates that openapi-generator's typescript generator uses under the hood. There are some unfortunate requirements there, namely the .model.ts and .client.ts. My goal here was to reuse the upstream template as much as possible.

I also adjusted the current CatalogClient to use the generated code under the hood instead of creating URLs and requests. The packages/catalog-client/src/generated folder is from the custom generator.

✔️ 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)

backstage-goalie · 2023-04-20T15:36:20Z

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
@backstage/catalog-client	packages/catalog-client	minor	`v1.4.6`
@backstage/cli	packages/cli	minor	`v0.24.1-next.0`
@backstage/repo-tools	packages/repo-tools	minor	`v0.4.0`
@backstage/plugin-catalog-backend	plugins/catalog-backend	patch	`v1.15.1-next.0`

sennyeya · 2023-04-20T15:44:48Z

@freben @taras I was able to get a custom generator set up from openapi-generator that resembles most of the CatalogClient today. It's in Java and a lot of the code is based off of TypescriptClientCodegen.java. I can take a look at openapi-generator-plus but openapi-generator is definitely more standard and has support for more cases like server stubs and clients down the line.

That being said, I have 2 asks,

Can you take a look at the output types (in catalog-client/src/generated/models/*) and the output API (in catalog-client/src/generated/api/DefaultApi.ts) and see if they're helpful?
I pulled in the existing serialization code from their Typescript generator, which makes a lot of the methods much more verbose, does this make sense to add? It feels like it adds a fair amount of safety to our API calls that we didn't have before.

taras · 2023-04-20T16:10:18Z

@sennyeya, awesome work on this. Thank you for pushing it forward.

Can you take a look at the output types (in catalog-client/src/generated/models/*) and the output API (in catalog-client/src/generated/api/DefaultApi.ts) and see if they're helpful?

generated/api/DefaultAPI.ts has Promise<Response> as return for all methods. This was a problem that I saw previously with openapi-generator. We need the return to be typed, but it's not (at least from what I can see).

catalog-client/src/generated/models/* is very java-ish. It generates a serializer class for something that could be a type. It doesn't use that class to type the response.

I pulled in the existing serialization code from their Typescript generator, which makes a lot of the methods much more verbose, does this make sense to add?

I'm not sure, maybe @freben has a better idea.

I can take a look at openapi-generator-plus but openapi-generator is definitely more standard and has support for more cases like server stubs and clients down the line.

Just a heads up, I did look at them previously. One of the challenge with those libraries is that their maintainers don't have much time. I opened a few issues a year ago karlvr/openapi-generator-plus-generators#32 and karlvr/openapi-generator-plus-generators#33. We could add eslint ignore, but just a heads up.

sennyeya · 2023-04-20T16:29:05Z

@taras Thanks!

generated/api/DefaultAPI.ts has Promise as return for all methods. This was a problem that I saw previously with openapi-generator. We need the return to be typed, but it's not (at least from what I can see).

This was something that I was trying to hit an in between state with, the CatalogClient currently handles different endpoints separately (see the requestRequired, requestOptional and requestIgnored internal methods). We know the return types in the template but have to decide how to parse them and if we want to handle errors (ideally the errors are pushed to the caller and they handle them).

catalog-client/src/generated/models/* is very java-ish. It generates a serializer class for something that could be a type. It doesn't use that class to type the response.

Similar to above, I kept the serializer class from their implementation as an easier bridge from their typescript client, we can definitely remove (see template-openapi-client-plugin/generator/src/main/resources/typescript-backstage/model.mustache, but figured it would be useful for correctly serializing FormData/bodies.

Just a heads up, I did look at them previously. One of the challenge with those libraries is that their maintainers don't have much time. I opened a few issues a year ago karlvr/openapi-generator-plus-generators#32 and karlvr/openapi-generator-plus-generators#33. We could add eslint ignore, but just a heads up.

Yeah, that was one of my concerns, that library seems to have just 1 or 2 maintainers vs openapi-generator which has quite a few and a wider range of templates. My main issue with it is we have to write Java to customize the template if we want to maintain a custom generator (which I think we should). Not sure where this project sits on adding Java code, which is probably a question for @freben.

github-actions · 2023-04-24T19:02:19Z

Uffizzi Ephemeral Environment `deployment-39136`

☁️ https://app.uffizzi.com/github.com/backstage/backstage/pull/17470

📄 View Application Logs etc.

What is Uffizzi? Learn more!

github-actions · 2023-05-01T19:33:46Z

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 · 2023-05-09T10:17:18Z

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 · 2023-05-16T13:21:55Z

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!

sennyeya · 2023-05-22T14:54:45Z

Did a little more investigation on the generated types and support, the openapi-generator typescript generator doesn't have support for oneOf or allOf, or additionalProperties: true. That's causing an issue from the lint step where the Entity from @backstage/catalog-models and the autogenerated Entity from the spec don't match. (There's work in the future to combine the two of those). Basically, because openapi-generator doesn't create a [key:string]: any; in addition to known properties on the interface, Typscript can't merge the types and we fail validation.

I looked into autorest to see if it handles those cases better and it handles them as expected. I'm not at all versed in ts-morph, but will start taking a look this week as openapi-generator probably isn't the direction anymore.

freben · 2023-10-26T16:40:42Z

Sorry I haven't reviewed yet - but the changesets are missing!

sennyeya · 2023-10-26T16:46:17Z

@freben Added!

Rugvip

Nice 😎

Rugvip · 2023-11-09T19:35:28Z

Would it be possible to have this injected or runtime or smth like that instead? Be nice to not have any new files in the repo root

I was able to remove this one, however there will probably need to be one in packages/repo-tools. It could be injected at runtime, but it serves as a version lock file that can hold additional config (we're not using that yet, but could in future).

github-actions · 2023-11-10T17:26:02Z

Uffizzi Ephemeral Environment - Virtual Cluster

Your cluster pr-17470 was successfully created. Learn more about Uffizzi virtual clusters
To connect to this cluster, follow these steps:

Download and install the Uffizzi CLI from https://docs.uffizzi.com/install
Login to Uffizzi, then select the backstage account and project:

uffizzi login

Select an account: 
  ‣ backstage
    jdoe

Select a project or create a new project: 
  ‣ backstage-6783521

Update your kubeconfig: uffizzi cluster update-kubeconfig pr-17470 --kubeconfig=[PATH_TO_KUBECONFIG]
After updating your kubeconfig, you can manage your cluster with kubectl, kustomize, helm, and other tools that use kubeconfig files: kubectl get namespace --kubeconfig [PATH_TO_KUBECONFIG]

Access the backstage endpoint at https://backstage-default-pr-17470-c1838.uclusters.app.uffizzi.com

Signed-off-by: Aramis Sennyey <sennyeya@amazon.com> Signed-off-by: Aramis Sennyey <sennyeyaramis@gmail.com> Signed-off-by: Aramis Sennyey <aramiss@spotify.com>

Signed-off-by: Aramis Sennyey <aramiss@spotify.com>

github-actions · 2023-11-22T19:25:31Z

Uffizzi Cluster pr-17470 was deleted.

Signed-off-by: Aramis Sennyey <aramiss@spotify.com>

Rugvip

NIce! 😁

I think we're good to ship this and keep iterating. In particular figuring out the error responses, I agree defaulting to @backstage/errors would be nice, as well as cleaning up the lint rules and avoid the extra dep.

Rugvip · 2023-11-28T12:22:52Z

-      : {};
-    const response = await this.fetchApi.fetch(url, { method, headers });
-
+  private async requestIgnored(response: Response): Promise<void> {


I think we could tweak the naming of these now that they're not executing the request and just handling the response. Feeling we can leave that for a followup though

Rugvip · 2023-11-28T12:31:52Z

+          'unused-imports/no-unused-imports': 'error',
+          'unused-imports/no-unused-vars': [


Hold up, why are these added here? @typescript-eslint is what we use for unused vars. In general I feel there's really no point in enabling any additional rules for generated code?

Ah, yeah let me take another look at this. This is the library I've used in the past. Without this, we're getting a tsc error for unused variables if we keep this as is. The @typescript-eslint rule adds the check, but the unused-imports plugin adds an autofix rule, unless something changed in @typescript-eslint recently?

Ah so it trims away things to make tsc work? Makes sense then, let's

github-actions · 2023-11-28T13:18:57Z

Thank you for contributing to Backstage! The changes in this pull request will be part of the 1.21.0 release, scheduled for Tue, 19 Dec 2023.

taras · 2023-11-28T13:31:45Z

Amazing work @sennyeya thank you 🙏

sennyeya requested review from a team and backstage-service as code owners April 20, 2023 15:35

sennyeya requested a review from freben April 20, 2023 15:35

github-actions Bot added the area:catalog Related to the Catalog Project Area label Apr 20, 2023

sennyeya marked this pull request as draft April 20, 2023 16:34

sennyeya mentioned this pull request Apr 21, 2023

[Meta Issue]: OpenAPI Support #17482

Open

20 tasks

github-advanced-security AI found potential problems Apr 24, 2023

View reviewed changes

Comment thread packages/repo-tools/src/commands/openapi/client/generate.ts Fixed

Comment thread packages/repo-tools/src/commands/openapi/client/generate.ts Fixed

github-advanced-security AI found potential problems Apr 24, 2023

View reviewed changes

Comment thread packages/repo-tools/src/commands/openapi/client/generate.ts Fixed

Comment thread packages/repo-tools/src/commands/openapi/client/generate.ts Fixed

Comment thread packages/repo-tools/src/commands/openapi/client/generate.ts Fixed

sennyeya mentioned this pull request Apr 25, 2023

Org Member: sennyeya backstage/community#92

Closed

1 task

github-actions Bot added the stale label May 1, 2023

freben removed the stale label May 2, 2023

github-actions Bot added the stale label May 9, 2023

freben removed the stale label May 9, 2023

sennyeya marked this pull request as ready for review May 9, 2023 13:12

sennyeya requested a review from a team as a code owner May 9, 2023 13:12

sennyeya requested a review from nuritizra May 9, 2023 13:12

github-actions Bot added the stale label May 16, 2023

sennyeya added the no stale These issues will not be closed due to going stale label May 16, 2023

freben removed the stale label May 16, 2023

sennyeya force-pushed the generator branch from 82c9cfe to 86266fe Compare May 17, 2023 23:39

github-advanced-security AI found potential problems May 18, 2023

View reviewed changes

Comment thread packages/repo-tools/src/commands/openapi/client/generate.ts Fixed

Comment thread packages/repo-tools/src/commands/openapi/client/generate.ts Fixed

Comment thread packages/repo-tools/src/commands/openapi/client/generate.ts Fixed

github-actions Bot added the stale label Nov 2, 2023

freben removed the stale label Nov 2, 2023

backstage deleted a comment from github-actions Bot Nov 7, 2023

Rugvip requested changes Nov 10, 2023

View reviewed changes

github-advanced-security AI found potential problems Nov 10, 2023

View reviewed changes

Comment thread packages/repo-tools/src/commands/openapi/client/generate.ts Fixed

sennyeya commented Nov 10, 2023

View reviewed changes

sennyeya force-pushed the generator branch from 805c829 to 9b54d21 Compare November 13, 2023 19:35

nuritizra approved these changes Nov 15, 2023

View reviewed changes

feat(openapi): Create Backstage plugins through generator tooling.

3834067

Signed-off-by: Aramis Sennyey <sennyeya@amazon.com> Signed-off-by: Aramis Sennyey <sennyeyaramis@gmail.com> Signed-off-by: Aramis Sennyey <aramiss@spotify.com>

sennyeya force-pushed the generator branch from e94a25d to 3834067 Compare November 17, 2023 18:59

sennyeya added 3 commits November 17, 2023 14:34

fix types

63dfae0

Signed-off-by: Aramis Sennyey <aramiss@spotify.com>

fix api report

ea2e5f8

Signed-off-by: Aramis Sennyey <aramiss@spotify.com>

fix warning in api report

ffddac5

Signed-off-by: Aramis Sennyey <aramiss@spotify.com>

sennyeya requested a review from Rugvip November 20, 2023 15:03

Rugvip requested changes Nov 22, 2023

View reviewed changes

Comment thread packages/catalog-client/src/CatalogClient.ts Outdated

sennyeya added 2 commits November 22, 2023 14:16

remove response error from template and keep parsing at the client level

364765c

Signed-off-by: Aramis Sennyey <aramiss@spotify.com>

remove packages/errors changes

004f74e

Signed-off-by: Aramis Sennyey <aramiss@spotify.com>

sennyeya added 3 commits November 22, 2023 14:48

add return type for queryEntities

77e7ccd

Signed-off-by: Aramis Sennyey <aramiss@spotify.com>

fix encoding issue

a10b5bb

Signed-off-by: Aramis Sennyey <aramiss@spotify.com>

update to just be request

d80662d

Signed-off-by: Aramis Sennyey <aramiss@spotify.com>

Rugvip approved these changes Nov 28, 2023

View reviewed changes

sennyeya merged commit ce3f7d4 into backstage:master Nov 28, 2023

awanlin mentioned this pull request Dec 2, 2023

🐛 Bug Report: [Regression] Members list empty when there are confirmed members #21717

Closed

2 tasks

sennyeya mentioned this pull request Jan 19, 2024

🐛 Bug Report: non RFC compliant URI for retrieving members and relations #19357

Closed

2 tasks

		'unused-imports/no-unused-imports': 'error',
		'unused-imports/no-unused-vars': [

Conversation

sennyeya commented Apr 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Hey, I just made a Pull Request!

Summary

What's in this PR?

✔️ Checklist

Uh oh!

backstage-goalie Bot commented Apr 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changed Packages

Uh oh!

sennyeya commented Apr 20, 2023

Uh oh!

taras commented Apr 20, 2023

Uh oh!

sennyeya commented Apr 20, 2023

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Apr 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uffizzi Ephemeral Environment deployment-39136

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 1, 2023

Uh oh!

github-actions Bot commented May 9, 2023

Uh oh!

github-actions Bot commented May 16, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sennyeya commented May 22, 2023

Uh oh!

freben commented Oct 26, 2023

Uh oh!

sennyeya commented Oct 26, 2023

Uh oh!

Rugvip left a comment

Choose a reason for hiding this comment

Uh oh!

Rugvip Nov 9, 2023

Choose a reason for hiding this comment

Uh oh!

sennyeya Nov 13, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Nov 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uffizzi Ephemeral Environment - Virtual Cluster

Uh oh!

Uh oh!

github-actions Bot commented Nov 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Rugvip left a comment

Choose a reason for hiding this comment

Uh oh!

Rugvip Nov 28, 2023

sennyeya commented Apr 20, 2023 •

edited

Loading

backstage-goalie Bot commented Apr 20, 2023 •

edited

Loading

github-actions Bot commented Apr 24, 2023 •

edited

Loading

Uffizzi Ephemeral Environment `deployment-39136`

github-actions Bot commented Nov 10, 2023 •

edited

Loading

github-actions Bot commented Nov 22, 2023 •

edited

Loading

sennyeya Nov 28, 2023 •

edited

Loading