Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stages/users: fix user names schema validation #662

Merged
merged 2 commits into from Jun 9, 2021

Conversation

gicmo
Copy link
Contributor

@gicmo gicmo commented Jun 9, 2021

Use patternProperties instead of propertyNames and pattern, which is not in draft 4 and so did not work (but also did not throw an error).

Found by @thozza with help from @achilleas-k

achilleas-k
achilleas-k previously approved these changes Jun 9, 2021
Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

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

LGTM.
Thanks for the quick fix.

@achilleas-k achilleas-k self-requested a review June 9, 2021 11:06
ondrejbudai
ondrejbudai previously approved these changes Jun 9, 2021
Copy link
Member

@ondrejbudai ondrejbudai left a comment

Choose a reason for hiding this comment

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

Looks great!

Hmm, can we validate the schemas to prevent these issues?

@achilleas-k
Copy link
Member

Same change should also be made in org.osbuild.groups.

@achilleas-k
Copy link
Member

Hmm, can we validate the schemas to prevent these issues?

I would expect check_schema() to take care of this, but then I'd be wrong

Validator.check_schema(self.data)

gicmo added 2 commits June 9, 2021 13:53
Use `patternProperties` instead of `propertyNames` and `pattern`,
which is not in draft 4 and so did not work (but also did not
throw an error).

Co-Developed-by: Achilleas Koutsou <achilleas@koutsou.net>
Use `patternProperties` instead of `propertyNames` and `pattern`,
which is not in draft 4 and so did not work (but also did not
throw an error).
@gicmo gicmo dismissed stale reviews from ondrejbudai and achilleas-k via ee6d53a June 9, 2021 11:54
@gicmo gicmo force-pushed the users_properties_validation branch from b6a2ea7 to ee6d53a Compare June 9, 2021 11:54
@gicmo
Copy link
Contributor Author

gicmo commented Jun 9, 2021

Hmm, can we validate the schemas to prevent these issues?

I would expect check_schema() to take care of this, but then I'd be wrong

Seems to be not supported by python-jsonschema, see python-jsonschema/jsonschema#778

Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks!

@larskarlitski larskarlitski merged commit d62c829 into osbuild:main Jun 9, 2021
@gicmo gicmo deleted the users_properties_validation branch June 9, 2021 18:35
@gicmo gicmo added this to the osbuild 29 milestone Jun 14, 2021
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.

jsonschema property validation using propertyNames and pattern does not seem to work
4 participants