-
Notifications
You must be signed in to change notification settings - Fork 580
Add EscapingExamples #2571
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
base: master
Are you sure you want to change the base?
Add EscapingExamples #2571
Conversation
|
Pipeline controller notification For optional jobs, comment |
|
Hello @mdbooth! Some important instructions when contributing to openshift/api: |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Adds a set of example which demonstrate the effects of escaping when generating validation rules. Specifically: * `` vs "" for rule quoting * "" vs r'' in CEL expressions These tests justify the following general recommendations to avoid complex escaping: * Always prefer `` for validation rules * Always prefer r'' for strings in CEL expressions
e21b55c to
73fa333
Compare
|
@mdbooth: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
@JoelSpeed The linter is rejecting because apparently Pattern is banned in this repo. That obviously make this example impossible to write. However, I also don't think it's a good idea. Certainly CEL has a lot of validation functions which we should require folks to use in preference to custom patterns. And as discussed, format markers in preference to those with the security related carve-outs documents in #2574. But if you're validating something that doesn't follow a standard kubernetes pattern then using A quick grep for Pattern in this repo shows that most, but critically far from all, of them should not exist. Not sure what to do with this. My recommendation would be to remove the prohibition on Pattern and replace it with a softer review requirement that it needs a good use case. If you really want to stick with a ban then I'd also have to update #2574 to document that, as well as removing it here. |
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Adds a set of example which demonstrate the effects of escaping when generating validation rules. Specifically:
These tests justify the following general recommendations to avoid complex escaping: