Be more strict with access resource titles / ACL syntax #324
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
smortex
changed the title
smortex
force-pushed
the
294
branch
3 times, most recently
from
0c68141 to
b2c1130
Compare
smortex
changed the title
bastelfreak
reviewed
Oct 8, 2021
bastelfreak
reviewed
Oct 8, 2021
ghoneycutt
requested changes
Oct 24, 2021
smortex
force-pushed
the
294
branch
3 times, most recently
from
3dc0735 to
7bd61bb
Compare
bastelfreak
reviewed
Dec 10, 2021
bastelfreak
reviewed
Dec 10, 2021
bastelfreak
approved these changes
Dec 10, 2021
bastelfreak
approved these changes
Dec 13, 2021
The openldap_access resource allows a lot of variations in the title for declaring a resource, making it possible to skip passing parameters such as `what` and `suffix`. This flexibility however can confuse Puppet when it is prefetching resources, leading to catalog compilation failures. Impose a more strict format for resource titles, and validate it with tighter custom types to raise a hopefully meaningful error instead of a Ruby error because something borked bad.
|
I rebased these changes on top of master, squashed the minor fixes that where added part of previous reviews, and added a new commit to address the final issue that worried me with this change. Can I have a review please? |
smortex
changed the title
The role of these rumbers was to allow multiple ACL with the same `to` to not overwrite each other when placed into the same hash. They have no further purpose and do not really avoid the issue since one can reuse the number. Rework the expected data structure to allow constructs that do not allow such misconfiguration.
bastelfreak
reviewed
Jan 25, 2022
| @@ -10,35 +10,64 @@ | |||
| # | |||
| # [*acl*] | |||
There was a problem hiding this comment.
we should also update this to puppet-strings soonish
There was a problem hiding this comment.
I saw this yesterday and bravely decided to go look away 😆
bastelfreak
approved these changes
Jan 25, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The openldap_access resource allows a lot of variations in the title for
declaring a resource, making it possible to skip passing parameters such
as
whatandsuffix. This flexibility however can confuse Puppetwhen it is prefetching resources, leading to catalog compilation
failures.
Impose a more strict format for resource titles, and validate it with
tighter custom types to raise a hopefully meaningful error instead of a
Ruby error because something borked bad.
Fixes #294