chore(backend): update OpenAPI specs #799

wilsonianb · 2022-11-29T17:38:06Z

Changes proposed in this pull request

Context

fixes Update backend OpenAPI specs #740

Checklist

Related issues linked using fixes #number
Tests added/updated
Documentation added
Make sure that all checks pass

sabineschaller

I'm confused, why are we fetching the specs now instead of updating URLs in env files?

wilsonianb · 2022-11-30T12:57:55Z

Here's the previous two related pr's:

The Rafiki specific specs use the shared Open Payments schemas file
https://github.com/interledger/rafiki/tree/main/packages/auth/src/openapi
https://github.com/interledger/open-payments/blob/main/openapi/schemas.yaml

So it is fetched instead of updating the $ref urls in the local files, and to support local dev using modified (pending changes to) specs/schemas:

fix(auth): bring in Open API schemas for IdP and RS #684 (comment)

I suggested also fetching the relevant Open Payments spec(s)

feat(auth): use split auth specs in app #692 (comment)

mkurapov · 2022-12-07T12:04:35Z

packages/backend/scripts/get-op-schema.sh

+curl -o "$OUTDIR/resource-server.yaml" https://raw.githubusercontent.com/interledger/open-payments/b363d33038fe789e5388f04f80ddd06a4fa97093/openapi/resource-server.yaml
+curl -o "$OUTDIR/schemas.yaml" https://raw.githubusercontent.com/interledger/open-payments/b363d33038fe789e5388f04f80ddd06a4fa97093/openapi/schemas.yaml


Can we also update the open-payments commit version in this PR

or would it make sense to export these from open-payments? so we can be sure both are using the same spec? This does seem vaguely familiar though... have we discussed this before?

We could even do it for both of backend and auth projects since open-payments contains both, which would eliminate the need for the packages/backend/src/openapi/auth-server.yaml dependency

would it make sense to export these from open-payments?

I'm favor of defining the (default) commit in a single place.
It seems like that could happen while still allowing subsequent local tinkering of the specs
#684 (comment)

which would eliminate the need for the packages/backend/src/openapi/auth-server.yaml dependency

Did you mean resource-server.yaml?

mkurapov · 2022-12-07T12:07:00Z

packages/backend/src/index.ts

+    const authServerSpec = await createOpenAPI(
+      path.resolve(__dirname, './openapi/auth-server.yaml')
+    )
+    const resourceServerSpec = await createOpenAPI(
+      path.resolve(__dirname, './openapi/resource-server.yaml')
+    )


Could we add a good error message here, for when the files can't be found?

https://interledgerfoundation.slack.com/archives/C0302LMKBT7/p1670413450794209?thread_ts=1670407497.730339&cid=C0302LMKBT7

As is, we get something like

ResolverError: Error opening file "/home/brandon/rafiki/packages/auth/src/openapi/auth-server.yaml" at ReadFileContext.callback (node_modules/.pnpm/@[email protected]/node_modules/@apidevtools/json-schema-ref-parser/lib/resolvers/file.js:52:20)

Are you thinking of having it mention the command to fetch the schemas?

Yes, that's what I was thinking, something along the lines of "Could not find Open API schema files. Did you run <command> ?"

As long we remember not to change the command 😅

mkurapov · 2022-12-07T12:13:53Z

README.md

 pnpm --filter auth fetch-schemas
+pnpm --filter backend fetch-schemas


can we add a root level command?
"localenv:fetch-op-schemas": "pnpm --filter auth --filter backend fetch-schemas" , and then include it as part of the local env setup readme?

https://interledgerfoundation.slack.com/archives/C0302LMKBT7/p1670414356297499?thread_ts=1670407497.730339&cid=C0302LMKBT7

I was thinking that devs would run through this environment setup before doing local env, but I guess that's not entirely necessary.
https://github.com/interledger/rafiki#environment-setup
Should we split out installing node & pnpm and fetching schemas into a separate prerequisite section?

I also noticed that as is it shouldn't work to have tests before fetch schemas.

Restructure local development / localenv instructions.

mkurapov

Once the specs stabilize it would be good to have them be defined in a singular place, even like with the exported commit hash as you suggested - I am a bit worried about specs diverging accidentally

mkurapov · 2022-12-09T15:19:46Z

packages/auth/src/index.ts

-      idpSpec
+  container.singleton('openApi', async (deps: IocContract<AppServices>) => {
+    try {
+      const authServerSpec = await createOpenAPI(


what do you think of openPaymentsAuthServerSpec? Likewise with openPaymentsResourceServerSpec in the backend?

Trying to see if there is a better way of distinguishing the several different spec we have

This one here is actually authServerSpec (from https://github.com/interledger/rafiki/blob/main/packages/auth/src/openapi/resource-server.yaml)
I think openPaymentsAuthServerSpec would be https://github.com/interledger/open-payments/blob/main/openapi/schemas.yaml

What if we just called the first one introspection/tokenIntrospection for both the file name (make it consistent in both auth and backend) and the variable name?

I'm gonna change https://github.com/interledger/rafiki/blob/main/packages/auth/src/openapi/resource-server.yaml to token-introspection.yaml with

Suggested change

const authServerSpec = await createOpenAPI(

const tokenIntrospectionSpec = await createOpenAPI(

and not

Suggested change

const authServerSpec = await createOpenAPI(

const tokenIntroSPECtion = await createOpenAPI(

and keep the other file and variable names as is.

The tokenIntrospectionSpec change makes sense, but it should be replacingresourceServerSpec variable (the one that contains the introspection route), right?

Ha, yeah. I was thinking these comments were for packages/backend/src/index.ts 🙃

chore(backend): update OpenAPI specs

695817d

github-actions bot added pkg: backend Changes in the backend package. type: ci Changes to the CI type: documentation type: source Changes business logic type: tests Testing related labels Nov 29, 2022

wilsonianb marked this pull request as ready for review November 29, 2022 18:29

wilsonianb requested review from sabineschaller, mkurapov and njlie November 29, 2022 18:29

njlie previously approved these changes Nov 29, 2022

View reviewed changes

sabineschaller reviewed Nov 30, 2022

View reviewed changes

wilsonianb requested a review from sabineschaller November 30, 2022 13:46

Merge branch 'main' into bw-backend-openapi

e568995

wilsonianb dismissed njlie’s stale review via e568995 December 1, 2022 20:40

wilsonianb requested a review from njlie December 6, 2022 21:49

mkurapov reviewed Dec 7, 2022

View reviewed changes

chore(localenv): add root level fetch-schemas command

927c322

Restructure local development / localenv instructions.

github-actions bot added type: localenv Local playground pkg: auth Changes in the GNAP auth package. pkg: open-payments labels Dec 7, 2022

chore(open-payments): update OpenAPI specs commit

bb4be91

wilsonianb force-pushed the bw-backend-openapi branch from ef3903c to bb4be91 Compare December 7, 2022 15:47

wilsonianb requested a review from mkurapov December 7, 2022 16:07

chore(backend): improve OpenAPI loading error message

faee3cb

mkurapov previously approved these changes Dec 9, 2022

View reviewed changes

wilsonianb added 2 commits December 9, 2022 10:56

chore(auth): rename resource-server.yaml as token-introspection.yaml

30a33e6

chore(open-payments): update OpenAPI specs commit

0c3c90a

wilsonianb dismissed mkurapov’s stale review via 0c3c90a December 9, 2022 17:20

wilsonianb requested a review from mkurapov December 9, 2022 17:37

mkurapov approved these changes Dec 11, 2022

View reviewed changes

wilsonianb merged commit 107e48f into main Dec 12, 2022

wilsonianb deleted the bw-backend-openapi branch December 12, 2022 21:07

wilsonianb mentioned this pull request Dec 13, 2022

chore(open-payments): fetch OpenAPI specs locally #846

Merged

4 tasks

huijing added the type: documentation (archived) Improvements or additions to documentation label Oct 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(backend): update OpenAPI specs #799

chore(backend): update OpenAPI specs #799

wilsonianb commented Nov 29, 2022 •

edited

Loading

sabineschaller left a comment

wilsonianb commented Nov 30, 2022

mkurapov Dec 7, 2022

mkurapov Dec 7, 2022

wilsonianb Dec 7, 2022 •

edited

Loading

mkurapov Dec 7, 2022

wilsonianb Dec 7, 2022 •

edited

Loading

mkurapov Dec 8, 2022 •

edited

Loading

wilsonianb Dec 8, 2022

mkurapov Dec 7, 2022

wilsonianb Dec 7, 2022

mkurapov left a comment

mkurapov Dec 9, 2022

wilsonianb Dec 9, 2022 •

edited

Loading

wilsonianb Dec 9, 2022

mkurapov Dec 9, 2022

wilsonianb Dec 9, 2022

		curl -o "$OUTDIR/resource-server.yaml" https://raw.githubusercontent.com/interledger/open-payments/b363d33038fe789e5388f04f80ddd06a4fa97093/openapi/resource-server.yaml
		curl -o "$OUTDIR/schemas.yaml" https://raw.githubusercontent.com/interledger/open-payments/b363d33038fe789e5388f04f80ddd06a4fa97093/openapi/schemas.yaml

		pnpm --filter auth fetch-schemas
		pnpm --filter backend fetch-schemas

	const authServerSpec = await createOpenAPI(
	const tokenIntrospectionSpec = await createOpenAPI(

	const authServerSpec = await createOpenAPI(
	const tokenIntroSPECtion = await createOpenAPI(

chore(backend): update OpenAPI specs #799

chore(backend): update OpenAPI specs #799

Conversation

wilsonianb commented Nov 29, 2022 • edited Loading

Changes proposed in this pull request

Context

Checklist

sabineschaller left a comment

Choose a reason for hiding this comment

wilsonianb commented Nov 30, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wilsonianb Dec 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wilsonianb Dec 7, 2022 • edited Loading

Choose a reason for hiding this comment

mkurapov Dec 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkurapov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wilsonianb Dec 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wilsonianb commented Nov 29, 2022 •

edited

Loading

wilsonianb Dec 7, 2022 •

edited

Loading

wilsonianb Dec 7, 2022 •

edited

Loading

mkurapov Dec 8, 2022 •

edited

Loading

wilsonianb Dec 9, 2022 •

edited

Loading