Skip to content
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

375 handle json numbers resubmit #427

Merged
merged 2 commits into from
Nov 5, 2019

Conversation

shivdudhani
Copy link
Contributor

@shivdudhani shivdudhani commented Oct 31, 2019

we parse the resource and configuration in policy using a custom parser. When using numbers if quotes '' are not specified we lose the type information, and we did not handle scenarios of variables with value numbers(int, float) but of type string.

  • Added support to compare string vs numbers. Based on the expected type in the pattern, we check if the string contains the value of the same type.
  • Generate used a helper function to identify if a configuration is a subset of resource but after the rewrite of validation pattern in Proposal to change validation condition & existence anchor behavior #356. We can use the validate with the default resource handler to check for subsets.
  • Update existing policies that had this limitation.

fixes #375

@@ -91,6 +92,14 @@ func validateValueWithIntPattern(value interface{}, pattern int64) bool {

glog.Warningf("Expected int, found float: %f\n", typedValue)
return false
case string:
// extract int64 from string
int64Num, err := strconv.ParseInt(typedValue, 10, 64)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I set memory limit in policy to "1Gi", and the resource has the limit set to "1024Mi", would this work? Same for the cpu "200m" vs "0.2" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@realshuting

  • first, value 1Gi will not be compared in validateValueWithIntPattern as its an alphanumeric and will be a string comparison. This check if for numbers types only.
  • second, here we parsing through each node and comparing the values based on types( custom parser, which is independent of the value string contains). The logic to compare Gi and Mi is kubernetes specific and will be handheld by the server. So if we cannot to Gi and Mi comparison here. If it is required, u'll need to create specifically handle these scenarios.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, if this pr is to fix numbers only, we should be good.

The logic to compare Gi and Mi is kubernetes specific and will be handheld by the server.

As Kyverno is doing the validation checks in webhook, it seems like we have to handle this before k8s, created an issue to tack #428.

@shivdudhani shivdudhani merged commit cfbd212 into master Nov 5, 2019
@shivdudhani shivdudhani deleted the 375_handle_json_numbers_resubmit branch November 6, 2019 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Numerical comparison in pattern
2 participants