backstage / backstage

Backstage is an open framework for building developer portals
https://backstage.io/
Apache License 2.0
27.16k stars 5.68k forks source link

$text substitutions in catalog descriptor files do not support local files #14372

Open OrkoHunter opened 1 year ago

OrkoHunter commented 1 year ago

Expected Behavior

According to the documentation for $text, $json and $yaml, we should be able to use $text: <local file path> to substitute catalog-info.yaml. This is especially useful for spec.definition in API entities.

Example

kind: API
...
spec:
  type: openapi
  definition:
    $text: ./api-definition.yaml

Actual Behavior

Catalog throws an error when processing API entities where $test is either a relative local path or an absolute local path. It works only with URLs.

2022-10-28T12:16:01.069Z catalog warn Processor PlaceholderProcessor threw an error while preprocessing; caused by Error:
Placeholder $text could not form a URL out of /Users/himanshumishra/workspace/backstage/packages/catalog-
model/examples/apis/petstore-api.yaml and ./my-api-definition.yaml, Error: URL parsing failed. type=plugin entity=api:default/petstore

Steps to Reproduce

Update an API definition where $text points to a local file path

Context

Looking into the processor code, there is a comment saying the support for relative file is not there.

Your Environment

master branch of this repository
awanlin commented 1 year ago

Hi @OrkoHunter, should it be $yaml because you are pointing to a YAML file? I recently helped someone on Discord with this and they reported it worked: https://discord.com/channels/687207715902193673/1035213554065883146

OrkoHunter commented 1 year ago

Oh thank you @awanlin ! I did indeed try $yaml as well. Although looking at the conversation you point at, I am missing a - for the definitions, didn't know they are supposed to be a list. I'll try that out!

OrkoHunter commented 1 year ago

Unfortunately all of the alternatives give the same error to me

kind: API
...
spec:
  type: openapi
  definition:
    $text: ./api-definition.yaml
kind: API
...
spec:
  type: openapi
  definition:
    $yaml: ./api-definition.yaml
kind: API
...
spec:
  type: openapi
  definition:
    - $text: ./api-definition.yaml
kind: API
...
spec:
  type: openapi
  definition:
    - $text: ./api-definition.yaml
kind: API
...
spec:
  type: openapi
  definition: ./api-definition.yaml

Edit: Note that my catalog-info.yaml is also stored locally on the filesystem.

Rugvip commented 1 year ago

I think the code you linked to explains it well, we don't support this, but we could, as long as we implement it in a secure way.

Seeing as the source entity file is already stored on disk I don't see any huge issues tbh, since you'd need access to modifying the file to begin with. There might be some processors that could mess with things though, and open up for remote attacks. Limiting with resolveSafeChildPath is nice, but it's not enough as the only protection. That would give you access to this entire repo and config files, seeing as we have the catalog info file in the root.

I'm open to suggestions :grin:

OrkoHunter commented 1 year ago

Thanks for the response @Rugvip, I was going crazy :P

awanlin commented 1 year ago

Sorry for sending you down the wrong path @OrkoHunter. I wonder why the person on Discord said it worked for them 🤔

doemEINS commented 1 year ago

@OrkoHunter interestingly, I just had success with registering an API/Service in Backstage with the following in catalog-info.yaml:

kind: API
[...]
spec:
  type: openapi
  definition:
    $text: ./src/main/resources/oas/swagger.yaml
OrkoHunter commented 1 year ago

@doemEINS Oh awesome! I wonder if that still works if the catalog-info.yaml is registered locally instead of from a URL? In my case, I have a local instance where my catalog-info.yaml is registered locally using catalog.locations in app-config.yaml.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

JefferoNW commented 1 year ago

I am running into this same issue and have not been able to resolve it.

I have a local app-config.yaml file that references a local catalog-info.yaml file. And I am trying to reference a local MYswagger.yaml file using the following:

spec:
  type: openapi
  ...
  definition:
    $yaml: ./MYswagger.yaml

But I recieve an error:

warn Processor PlaceholderProcessor threw an error while preprocessing; caused by Error: Placeholder $yaml could not form a URL out of /Users/xxx/backstage/my-backstage/my_entities/catalog-info.yaml and ./MYswagger.yaml, TypeError [ERR_INVALID_URL]: Invalid URL type=plugin entity=api:default/MYswagger location=file:/Users/xxx/backstage/my-backstage/my_entities/catalog-info.yaml

I tried most every permutation that I have seen ($text, $yaml, -, $file, $include, ...) but can't get arround this error.

Did anyone find a functional solution? @OrkoHunter, @doemEINS ?

Thanks - Jeff

doemEINS commented 1 year ago

@JefferoNW I can only tell that our Backstage instance hosted on Azure Kubernetes Service (AKS), scrawls for catalog-info.yamls in our project repositories on our self-hosted GitLab (via URL ofc) and is able to register API definitions that come from a "local" file, that is also part of the git version control (thus has also a URL).

OrkoHunter commented 1 year ago

Hey @JefferoNW - as @Rugvip said above, if the source of your catalog-info.yaml is a local file path (instead of a URL), then relative paths like ../api.yaml is not supported by the processor currently. So maybe try ingesting that catalog-info.yaml from a URL - I guess it will work. The latter is anyway what you would end up doing in a stable scenario.

JefferoNW commented 1 year ago

Thanks to both of you! You are correct - this is only a temporary "testing to better understand the features and capabilities" phase and will not be a limitation once we move to URLs. THanks for your assistance!

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

GuerreiroLeonardo commented 1 year ago

I was able to fix it by doing this:

kind: API
[...]
spec:
  type: openapi
  definition:
    $text: './swagger.yaml'
mrkwtz commented 1 year ago

I've ran into the same problem and wonder about the documentation. It states

Files can be referenced from any configured integration similar to locations by passing an absolute URL. It's also possible to reference relative files like ./referenced.yaml from the same location. Relative references are handled relative to the folder of the catalog-info.yaml that contains the placeholder.

Am I misunderstanding something here?

jessesanford commented 1 year ago

I think the code you linked to explains it well, we don't support this, but we could, as long as we implement it in a secure way.

Seeing as the source entity file is already stored on disk I don't see any huge issues tbh, since you'd need access to modifying the file to begin with. There might be some processors that could mess with things though, and open up for remote attacks. Limiting with resolveSafeChildPath is nice, but it's not enough as the only protection. That would give you access to this entire repo and config files, seeing as we have the catalog info file in the root.

I'm open to suggestions grin

@Rugvip I would be interested in expanding upon the usecases driving our need for local file support. With your direction I can also muster the resources to create a proposal on how to make this possible within the context of the current implementation. How should I get started on this? Would this be something we would need an RFC for? Or can we just create a PR against this issue?

Rugvip commented 1 month ago

Reopening since this came up again in #25067