Skip to content

[backport] jsonschema: warn and ignore unresolved URN $ref to match v3.18.4#31247

Closed
benoittgt wants to merge 0 commit intohelm:dev-v3from
benoittgt:fix-31170-URN-backport
Closed

[backport] jsonschema: warn and ignore unresolved URN $ref to match v3.18.4#31247
benoittgt wants to merge 0 commit intohelm:dev-v3from
benoittgt:fix-31170-URN-backport

Conversation

@benoittgt
Copy link
Copy Markdown
Contributor

@benoittgt benoittgt commented Sep 5, 2025

What this PR does / why we need it:

  • v3.18.5 switched jsonschema and began resolving external $ref at compile-time, exposing missing urn handling (“no URLLoader registered for urn:…”).
  • Add urn scheme loader and pluggable URNResolver. If unresolved, log a warning and return a permissive true schema (back-compat).
  • Avoid having a duplicated warn logs when we load the schema twice on linting for example.

Close: #31170
Related: #31240

Special notes for your reviewer:

🤗

If applicable:

  • this PR contains user facing changes (the docs needed label should be applied if so)
  • this PR contains unit tests
  • this PR has been tested for backwards compatibility

@pull-request-size pull-request-size Bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 5, 2025
benoittgt
benoittgt commented Sep 5, 2025
View reviewed changes
Comment thread pkg/chartutil/jsonschema.go Outdated
"crypto/tls"
"errors"
"fmt"
"log"
Copy link
Copy Markdown
Contributor Author

@benoittgt benoittgt Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used log and not slog because this is what we use here. https://github.com/benoittgt/helm/blob/dev-v3/pkg/chartutil/dependencies.go#L67

@benoittgt benoittgt marked this pull request as ready for review September 5, 2025 08:43
@benoittgt benoittgt mentioned this pull request Sep 5, 2025
Merged
@scottrigby scottrigby added the bug Categorizes issue or PR as related to a bug. label Sep 5, 2025
@scottrigby scottrigby added this to the 3.19.1 milestone Sep 9, 2025
@ashanvb ashanvb mentioned this pull request Oct 3, 2025
Closed
1 task
@matheuscscp matheuscscp mentioned this pull request Oct 3, 2025
Closed
@benoittgt
Copy link
Copy Markdown
Contributor Author

benoittgt commented Oct 3, 2025

@scottrigby @mattfarina @gjenkins8 Could you review this? It affects Flux and other users. This approach was validated during the Helm dev meeting a couple of weeks ago.

@benoittgt benoittgt force-pushed the fix-31170-URN-backport branch from 6057440 to 70d09b8 Compare October 30, 2025 14:56
@pull-request-size pull-request-size Bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 30, 2025
@benoittgt benoittgt force-pushed the fix-31170-URN-backport branch from 70d09b8 to 987947a Compare October 30, 2025 14:57
@pull-request-size pull-request-size Bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 30, 2025
@benoittgt benoittgt closed this Oct 30, 2025
@benoittgt benoittgt force-pushed the fix-31170-URN-backport branch from 987947a to 73c269f Compare October 30, 2025 14:58
@pull-request-size pull-request-size Bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 30, 2025
@benoittgt benoittgt mentioned this pull request Oct 30, 2025
Merged
3 tasks
@scottrigby scottrigby removed this from the 3.19.1 milestone Nov 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bug Categorizes issue or PR as related to a bug. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@benoittgt @scottrigby