-
Notifications
You must be signed in to change notification settings - Fork 610
Add test that newly introduced build-time fields for a min_stack for … #2262
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
Add test that newly introduced build-time fields for a min_stack for … #2262
Conversation
…applicable rules.
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.
Small suggestion
- Account for new rules that don't add min_stack_version in toml
| invalids = [] | ||
|
|
||
| for rule in self.production_rules: | ||
| min_stack = rule.contents.metadata.min_stack_version |
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.
Will we never use soft-forking then, only hard-forking from the metadata field??
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.
based on our last discussions, I don't think it is supportable (or advisable)
|
I realized this would fail when backported to <8.3 branches and in general for branches <= build enforcement, so I added a constraint to check the current version for each build field in b2bec63 |
Issues
related to #2251
Summary
Build-time fields create changes to rule hashes without being accounted for explicitly within the version lock (essentially creating a "soft" lock). This is problematic because it creates a buggy situation where non-latest rules version bump every time due to the delta hash resulting from the differences from the build time fields. To address this, build time field introduction requires that all fields.
This adds a test to enforce that on any defined build time rule