OpenApiRefProcessor: add basepath support for $ref

[mfrinnstrom]

Hey, I just made a Pull Request!

There was an error reported in https://github.com/backstage/backstage/issues/12687. I'm not sure if this fixes that issue since I don't have all the details yet but this seems like a case that should be supported anyway.

This adds a openapi/ref-basepath annotation that allows you to specify from what base path all $ref instances should be resolved instead of them always being resolved from the catalog-info.yaml file location.

:heavy_check_mark: Checklist

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

[github-actions[bot]]

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
@backstage/plugin-catalog-backend-module-openapi	plugins/catalog-backend-module-openapi	patch	v0.1.1-next.0

[benjdlambert]

I'm not sure that an annotation is the best place for this to be honest, feeling like it could be more beneficial to supply a function to the Processor that can resolve this for you using whatever method you want. Need to think about this a little bit more. Will get back to you. 🙏

[mfrinnstrom]

@benjdlambert I'm more than happy to do this in another way if possible.

The problem as I see it is that the information that I would need is the path in definition field but by the time the OpenApiRefProcessor gets called that has already been replaces by the $text resolver.

I tried to see if I could write a wrapper for that that somehow stores the path temporarily but there wasn't enough information available to the resolver to determine if we are working on the definition key or not.

Then I tried to see if I could handle it by wrapping the PlaceholderProcessor but that is not possible, for good reasons, I believe.

That left me with the annotation to at least be able to support this use case right now. The OpenApiRefProcessor is also opt-in so not everybody will have to handle this for their OpenAPI spec to work. If they have everything in one file then they don't need to touch this at all.

I hope that makes sense and, as I said, if I have missed something that would allow me to this without the annotation I'm more than happy to do that.

apiVersion: backstage.io/v1alpha1
kind: API
metadata:
  name: vehicle-search
  description: Vehicle Search
  annotations:
    openapi/ref-basepath: ./spec
spec:
  type: openapi
  lifecycle: production
  owner: vehicle-search-team
  definition:
    $text: ./spec/openapi.yaml # I would need access to this value to do this without the annotation

[benjdlambert]

Sorry for the delay! Yep, @mfrinnstrom this ability has come up before actually, I wonder if it's possible for us to maybe forward some of this metadata from the unfurled stuff from the Placeholder processor. Going to want some more eyes from @backstage/maintainers for this I think after they're back from vacation to see if we can't come up with something a little better than an annotation.

[niallmccullagh]

Feels somewhat similar to the backstage.io/techdocs-ref annotation.

[benjdlambert]

@niallmccullagh I agree for the most part, but the difference between them is we actually do have the basepath already declared in the file once in this case as it's used for $text replacer. Would be a shame to have to duplicate it was all I was thinking.

[mfrinnstrom]

@benjdlambert I realized that we do have the URL for the catalog-info.yaml file. Would it be a more acceptable solution to just reread that and have some logic check if $text is the only key in the spec.definition field and in that case process that?

[benjdlambert]

Hmm - seems a little unfortunate to have to re-read grab the contents again, but maybe that's getting warmer :pray: The other folks are back monday, so we will have a chat.

[Rugvip]

Really feels unfortunate to have to add annotations for this tbh, the information is already there. It's also risky to try to fetch the source entity again, as that might not be reachable or have the shape that one would expect.

I'm thinking the best way forward here might actually be to deprecate the OpenApiRefProcessor and instead package it up as a custom resolver for the PlaceholderProcessor? Probably either as a decoration of the built-in $text resolver, or making it an additional $openapi resolver. That was we're hooking into the fetching logic and we know the URLs and paths that are involved.

[mfrinnstrom]

@Rugvip that is fine. The processor was just a quick initial take on this that supported the use-case that I have had before.

Do you have any preference on the $openapi resolver vs $text decorator? I'm thinking it might be better with a separate $openapi resolver to make sure that it will never interfere with any other $text fields. You can also be more specific when you want to use it as well then.

[Rugvip]

Yep thinking $openapi first and see if that makes sense.

[mfrinnstrom]

@Rugvip I switched it to a custom resolver now. I did copy some helpers from the PlaceholderProcessor since the weren't exported. Not sure how you want to handle that. Should they be exported instead?

I have tested it against my example repo here and those work as they should. The basepath one doesn't work with the current processor so that is an improvement at least 🙂

[mfrinnstrom]

Not sure where that change to yarn.lock came from. I rebased and now it is gone.

[asotog]

Is this change on npm ? I upgraded our backstage today but still not working, this part of our api catalog yaml:

 annotations:
    openapi/ref-basepath: https://bitbucket.org/...

...

 definition:
    $openapi: https://bitbucket.org/....openapi.yaml

[asotog]

ignore my previous comment, had to follow the plugin documentation https://github.com/backstage/backstage/tree/master/plugins/catalog-backend-module-openapi , works like a charm