[FR] Support forked rules with 100 version buffer space #1946
Conversation
Mikaayenson
added
enhancement
|
Tested the outcome of the versions when locked and it appears new version in version lock will be 100+. Not sure if this is the expected outcome but I have detailed my findings and repeatable steps below to reach route B. Steps to re-create:
outcome |
| if exclude_version_update: | ||
| buffer_int -= 1 |
There was a problem hiding this comment.
This is to make sure we dont also include the rule version autobump
There was a problem hiding this comment.
if excluding version update, shouldn't the buffer be skipped too?
Mikaayenson
changed the title
Update Sep 19 2022Not only was this out of date with the latest code, but also highlights new concerns based on after all of our recent updates (forking all rules, etc.). In order to actually use this workflow, rule developers would need to:
This seems like a rare edge case that we would not want rule developers to do. I removed the route C option which calculates version space between a forked version and a version not adjacent to the forked version. E.g. (manually edited for example purposes) "000047bb-b27a-47ec-8b62-ef1a5d2c9e19": {
"min_stack_version": "8.4",
"previous": {
"7.16": {
"max_allowable_version": 99,
"rule_name": "Attempt to Modify an Okta Policy Rule",
"sha256": "fc9d05639917fdd13a3a474200a618648fe3dbd6fbc059714179e692544d1354",
"type": "query",
"version": 9
},
"8.0": {. # The plan is not to support this as we would have to add a new old fork in between versions
"max_allowable_version": 7.16["max_allowable_version"],
"rule_name": "Attempt to Modify an Okta Policy Rule",
"sha256": some new hash,
"type": "query",
"version": (7.16["max_allowable_version"] - 7.16["version"]) / 2
}
"8.3": {
"max_allowable_version": 199,
"rule_name": "Attempt to Modify an Okta Policy Rule",
"sha256": "dedf2a77f86a3ecebeba40e8a1f54e713510e09384f2ca228c8adb9cc6322490",
"type": "query",
"version": 100
}
},
"rule_name": "Attempt to Modify an Okta Policy Rule",
"sha256": "533da2247bc44a3b45e2bcd6a798f9afae92fde1d5a6e8e99c1f0ebfcb9b0e2d",
"type": "query",
"version": 200. # automatic 100 bump supported
}, |
brokensound77
left a comment
brokensound77
left a comment
There was a problem hiding this comment.
I think this looks good, but lets hold off merging until more discussions
punt for #2333 Co-authored-by: Justin Ibarra <brokensound77@users.noreply.github.com>
terrancedejesus
left a comment
terrancedejesus
left a comment
There was a problem hiding this comment.
This looks good to me.
|
Spent a good amount of time catching up with the thread of conversations and the solution implemented. This LGTM |
|
Tested locally with a rule and made min-stack changes along with changes to the file before updating version lock. Results |
brokensound77
left a comment
brokensound77
left a comment
There was a problem hiding this comment.
Testing looked good, but to verify, run the lock workflow and we can review that after merge
(cherry picked from commit 7b596c7)
(cherry picked from commit 7b596c7)
(cherry picked from commit 7b596c7)
(cherry picked from commit 7b596c7)
(cherry picked from commit 7b596c7)
(cherry picked from commit 7b596c7)
(cherry picked from commit 7b596c7)
Issues
Resolves #1935
Summary
Route C: If forking on an old stack, attempts to calculate half the version space remaining between the latest stack and the max previous version number, and errors if there is no space leftTesting
Ran the
etc/lock-multiple.shscript after min_stacking a rule.