-
Notifications
You must be signed in to change notification settings - Fork 784
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
fix: added details regarding match.resources #1654
Conversation
Signed-off-by: Arsh Sharma <arshsharma461@gmail.com>
pkg/api/kyverno/v1/policy_types.go
Outdated
@@ -209,6 +211,8 @@ type ExcludeResources struct { | |||
UserInfo `json:",omitempty" yaml:",omitempty"` | |||
|
|||
// ResourceDescription contains information about the resource being created or modified. | |||
// Requires at least one resource to be specified when under MatchResources, if not, | |||
// then ExcludeResources should be present instead. |
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.
"Requires at least one resource ... " this does not apply to exclude.resources
.
pkg/api/kyverno/v1/policy_types.go
Outdated
@@ -198,6 +198,8 @@ type MatchResources struct { | |||
UserInfo `json:",omitempty" yaml:",omitempty"` | |||
|
|||
// ResourceDescription contains information about the resource being created or modified. | |||
// Requires at least one resource to be specified when under MatchResources, if not, | |||
// then ExcludeResources should be present instead. |
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.
Can we change to "Requires at least one tag to be ..."
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.
@RinkiyaKeDad From this validation, it seems that match
is required in the rule definition:
kyverno/pkg/policy/validate.go
Lines 477 to 480 in 10c714d
// matched resources | |
if path, err := validateMatchedResourceDescription(rule.MatchResources.ResourceDescription); err != nil { | |
return fmt.Sprintf("match.resources.%s", path), err | |
} |
So we need to remove "if not, then ExcludeResources should be present instead.".
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.
@realshuting - Made the edit, please review.
Signed-off-by: Arsh Sharma <arshsharma461@gmail.com>
Signed-off-by: Arsh Sharma <arshsharma461@gmail.com>
Signed-off-by: Arsh Sharma arshsharma461@gmail.com
Related issue
Fixes #1394
What type of PR is this
Proposed changes
Checklist
Further comments