Bump the registry dependency to v1.0.0 and fix the conflicts #117
Conversation
rdimitrov
force-pushed
the
properties-bump
branch
from
5f062d7 to
32869b9
Compare
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
rdimitrov
force-pushed
the
properties-bump
branch
from
32869b9 to
7560909
Compare
rdimitrov
changed the title
rdimitrov
changed the title
| // Validate the complete registry against schema (warnings only for now) | ||
| if err := or.validateRegistry(registry); err != nil { | ||
| return fmt.Errorf("registry validation failed: %w", err) | ||
| fmt.Printf("⚠️ Schema validation warnings: %v\n", err) |
There was a problem hiding this comment.
It's temporary as the upstream schema enforces some requirements on values that we are not complying with, i.e. an sse/streamable-http server type don't have URLs on our side but is expected on the upstream side. It's actually an issue on our side because we made the assumption the MCP endpoints are on the root level of the server endpoint which may not always be the case
| // convertNameToReverseDNS converts simple server names to reverse-DNS format required by v1.0.0 schema | ||
| func (*OfficialRegistry) convertNameToReverseDNS(name string) string { | ||
| // If already in reverse-DNS format (contains '/'), return as-is | ||
| if strings.Contains(name, "/") { |
There was a problem hiding this comment.
Is this enough to enforce that it's a reverse DNS? Looking at the current thv registry all the names are simple strings with just a-z just wondering if you meant to include the full io.stacklok.toolhive/ string in the check
There was a problem hiding this comment.
Yeah, I wasn't particularly sure about this but for the time being it's not a problem. To be honest I don't think any FE client will like seeing those names so I'll be doing a follow up change where we have a display name as part of the publisher-provider extensions (that is what we have now as name on our side) and leave this to be the DNS one just so we are compliant.
jhrozek
left a comment
jhrozek
left a comment
There was a problem hiding this comment.
I just added some questions for my own education since it's the first PR I am looking at in this project. But looks good and matches the upstream format.
The following PR:
sse/streamable-httpserver type don't have URLs on our side but is expected on the upstream side)