Skip to content

fix: Fix yaml validation for key named "type" #588

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

suejung-sentry
Copy link
Contributor

@suejung-sentry suejung-sentry commented Mar 27, 2025

The cerberus library that we use for yaml validation has a bug when a schema field is named "type," a reserved word (see similar unresolved bug report in their repo here).
This results in codecov yaml validator returns 500 internal server error instead of a more specific validation error if you accidentally pass an object instead of a list under flag_management and component_management(as was reported here).

Aside from waiting for the underlying library to fix the issue (or vendoring the library and fixing it in our own copy), it seems we can use a workaround with the anyof union type offered by the library.
When declaring the schema type is anyof with a single element describing the expected shape, we can get around the problem, as done in this PR.

So now it will return error must be of list type instead of original error none is not iterable which happens when you try to iterate over none at here

    def _normalize_rename(self, mapping, schema, field):
        """{'type': 'hashable'}"""
        print("schema[field]", schema[field])
        print("field", field)
        if 'rename' in schema[field]:
            mapping[schema[field]['rename']] = mapping[field]
            del mapping[field]

So that will give a richer error message for this edge case of schema fields named "type", and expecting a list instead of an object.

Note that the error message for extraneous subfields becomes slightly different with "unknown field" instead of "extra keys not allowed" (see updated test)

Closes codecov/feedback#146

Copy link

codspeed-hq bot commented Mar 27, 2025

CodSpeed Performance Report

Merging #588 will not alter performance

Comparing sshin/fix/yaml-validation (1c27a0b) with main (ae1d4cf)

Summary

✅ 9 untouched benchmarks

@suejung-sentry suejung-sentry force-pushed the sshin/fix/yaml-validation branch from 9158495 to 4203249 Compare April 8, 2025 17:29
Copy link

codecov bot commented Apr 8, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.59%. Comparing base (0478220) to head (1c27a0b).
Report is 9 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #588      +/-   ##
==========================================
- Coverage   88.62%   88.59%   -0.04%     
==========================================
  Files         462      463       +1     
  Lines       12875    12786      -89     
  Branches     1461     1458       -3     
==========================================
- Hits        11411    11328      -83     
+ Misses       1159     1153       -6     
  Partials      305      305              
Flag Coverage Δ
shared-docker-uploader 88.59% <ø> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@codecov-notifications
Copy link

codecov-notifications bot commented Apr 8, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@suejung-sentry suejung-sentry marked this pull request as ready for review April 8, 2025 21:22
Copy link
Contributor

@ajay-sentry ajay-sentry left a comment

Choose a reason for hiding this comment

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

very cool! I wish the wording of the message was of format must be of type list rather than must be of list type but thats outside of our control seems like :(

Thanks for documenting this thoroughly!

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.

Server 500 when validating YAML
2 participants