-
Notifications
You must be signed in to change notification settings - Fork 326
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
Extend list of permitted classes for YAML safe load and allow aliases #1758
Extend list of permitted classes for YAML safe load and allow aliases #1758
Conversation
Codecov ReportBase: 74.93% // Head: 74.93% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## master #1758 +/- ##
=======================================
Coverage 74.93% 74.93%
=======================================
Files 82 82
Lines 4876 4876
=======================================
Hits 3654 3654
Misses 1222 1222
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
d9f2fda
to
0e108d6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good thing to get released. If someone can eventually look in to paring down what we persist for known hosts in the first place that would also be good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there some test that exercises the code that requires the serialization? As you can see, we already had tests for 3.1 but since they passed, clearly there's a gap in coverage.
0e108d6
to
ab71ae5
Compare
Good call. I added a hosts_preserved.yml to try to load in the spec. It's not testing the specific permitted classes or anything, just verifying it can load the file. We can get fancier with it if you think it's important. |
2c4653b
to
56565ae
Compare
When trying to load hosts_preserved.yml, all of these classes need to be able to be loaded. We should probably serialize fewer things to this file, but in the meantime, this allows things to work. This also uses strings for the list, since not all the classes are loaded and available at the time we are declaring the array. This also allows aliases, since we are using those in the hosts_preserved.yml file. This also adds a test to the spec to verify we can load a representative hosts_preserved.yml.
56565ae
to
4335485
Compare
When trying to load hosts_preserved.yml, all of these classes need to be able to be loaded. We should probably serialize fewer things to this file, but in the meantime, this allows things to work. This also uses strings for the list, since not all the classes are loaded and available at the time we are declaring this array. This also allows aliases, since we are using those in the hosts_preserved.yml file.