quarkusio / registry.quarkus.io

Quarkus Extension Registry application
https://registry.quarkus.io
Apache License 2.0
10 stars 11 forks source link

Expose an example config.yaml over the registry's REST api #201

Closed gastaldi closed 1 year ago

gastaldi commented 1 year ago

@jmartisk I have played with it and I don't think we need to introduce a new property to expose this configuration. IMHO it doesn't do any harm if it is always exposed.

I have refactored the tests a bit and used the model classes instead of relying on the String output. Let me know what you think about it

jmartisk commented 1 year ago

AFAIR @aloubyansky asked for this not to be exposed in production deployments, so let's hear his opinion.

aloubyansky commented 1 year ago

One of the concerns that @maxandersen raised in Jan's PR was that it reveals details of the internal infrastructure, which isn't desired in prod.

gastaldi commented 1 year ago

AFAIK the URL returned in the YAML is the same used to access it and it's configured via QUARKUS_REGISTRY_MAVEN_REPO_URL environment. Can you elaborate?

gastaldi commented 1 year ago

The same URL is also returned when the registry descriptor artifact is requested: https://github.com/quarkusio/registry.quarkus.io/blob/main/src/main/java/io/quarkus/registry/app/maven/RegistryDescriptorContentProvider.java#L93

aloubyansky commented 1 year ago

In that case, it's fine, thanks for checking. A possible concern could be we expose an endpoint w/o a requirement in production. Somebody may discover it and start relying it, while we may for some reason change or remove it.

gastaldi commented 1 year ago

A possible concern could be we expose an endpoint w/o a requirement in production. Somebody may discover it and start relying it, while we may for some reason change or remove it.

I don't think that's the case here, we're just presenting a valid client configuration for the requested registry :)

Also, if that's the case, we don't necessarily need to expose it in the OpenAPI descriptor