Skip to content

schema: make validation error private - #329

Open
thaJeztah wants to merge 1 commit into
cncf-tags:mainfrom
thaJeztah:unexport_error
Open

schema: make validation error private#329
thaJeztah wants to merge 1 commit into
cncf-tags:mainfrom
thaJeztah:unexport_error

Conversation

@thaJeztah

Copy link
Copy Markdown
Contributor

The exported Error type exposes gojsonschema.Result, leaking details of the JSON schema validator implementation, as well as the dependency itself, into the public API.

Error was introduced in 711b138 as part of PR 54. Neither the commit nor the related PR discussion indicate an intent for the validator result or the Error type itself to form part of the public API; it appears to have only been used internally to carry validation details until the error was formatted.

There are no known external consumers of the schema package at all, let alone consumers depending on the exported Error type or its Result field.

Make the validation error implementation private. This keeps the public API independent of the JSON schema validator and allows that implementation and dependency to be replaced without affecting callers.

The exported Error type exposes gojsonschema.Result, leaking details of the
JSON schema validator implementation, as well as the dependency itself, into
the public API.

Error was introduced in 711b138 as part
of PR 54. Neither the commit nor the related PR discussion indicate an intent
for the validator result or the Error type itself to form part of the public
API; it appears to have only been used internally to carry validation details
until the error was formatted.

There are no known external consumers of the schema package at all, let alone
consumers depending on the exported Error type or its Result field.

Make the validation error implementation private. This keeps the public API
independent of the JSON schema validator and allows that implementation and
dependency to be replaced without affecting callers.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah

Copy link
Copy Markdown
Contributor Author

@elezar @klihub PTAL; I was looking at possible replacements for the unmaintained json-schema dependencies, when I noticed that the dependency leaked into the public API of the schema module (yikes!).

From a search though, it looks like there's no external consumers of the schema module, and the Error type (and Result) field were only used internally within the package, so I think we should be fine to un-export this.

@klihub

klihub commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

@elezar @klihub PTAL; I was looking at possible replacements for the unmaintained json-schema dependencies...

Yes, it would be nice to find a properly maintained alternative.

@klihub klihub left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Well, if your summary is an accurate description of the current situation, then I don't see what this could break and how... so LGTM.

@thaJeztah

Copy link
Copy Markdown
Contributor Author

Thanks! Yes, I started working on a branch to switch to https://github.com/santhosh-tekuri/jsonschema, which looks to be the implementation that I see many projects converge on; it has very little dependencies, which is also nice;

require (
	github.com/dlclark/regexp2 v1.11.0 // used for testing
	golang.org/x/text v0.14.0
)

@thaJeztah

Copy link
Copy Markdown
Contributor Author

@elezar @klihub pushed my branch (only the last commit is the actual migration / rewrite);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants