mirrors/kyverno
mirrors/kyverno
1
0
Fork 0
mirror of https://github.com/kyverno/kyverno.git synced 2025-03-31 03:45:17 +00:00
Code Activity

fix: do not pass dynamicConfig to matchesResourceDescriptionMatchHelper (#6231) (#6242)

Browse source 
* fix: do not pass dynamicConfig to matchesResourceDescriptionMatchHelper (#6231)

(and only pass it to call for the exclude part of the rule)

Signed-off-by: Ray Burgemeestre <rayb@nvidia.com>

* chore: add unit tests to cover the fix, and to ensure the exclude behavior still works (#6231)

Signed-off-by: Ray Burgemeestre <rayb@nvidia.com>

---------

Signed-off-by: Ray Burgemeestre <rayb@nvidia.com>
Co-authored-by: Ray Burgemeestre <rayb@nvidia.com>
This commit is contained in:
Ray Burgemeestre 2023-02-08 09:08:09 +01:00 • committed by GitHub
parent f0341b0faf
commit 71edb10fcf
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 153 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
pkg/engine/utils.go
View file

@ -180,6 +180,7 @@ func MatchesResourceDescription(subresourceGVKToAPIResource map[string]*metav1.A
rule := ruleRef.DeepCopy()
resource := *resourceRef.DeepCopy()
admissionInfo := *admissionInfoRef.DeepCopy()
empty := []string{}
var reasonsForFailure []error
if policyNamespace != "" && policyNamespace != resourceRef.GetNamespace() {
@ -192,7 +193,7 @@ func MatchesResourceDescription(subresourceGVKToAPIResource map[string]*metav1.A
oneMatched := false
for _, rmr := range rule.MatchResources.Any {
// if there are no errors it means it was a match
if len(matchesResourceDescriptionMatchHelper(subresourceGVKToAPIResource, rmr, admissionInfo, resource, dynamicConfig, namespaceLabels, subresourceInAdmnReview)) == 0 {
if len(matchesResourceDescriptionMatchHelper(subresourceGVKToAPIResource, rmr, admissionInfo, resource, empty, namespaceLabels, subresourceInAdmnReview)) == 0 {
oneMatched = true
break
}
@ -203,11 +204,11 @@ func MatchesResourceDescription(subresourceGVKToAPIResource map[string]*metav1.A
} else if len(rule.MatchResources.All) > 0 {
// include object if ALL of the criteria match
for _, rmr := range rule.MatchResources.All {
reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionMatchHelper(subresourceGVKToAPIResource, rmr, admissionInfo, resource, dynamicConfig, namespaceLabels, subresourceInAdmnReview)...)
reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionMatchHelper(subresourceGVKToAPIResource, rmr, admissionInfo, resource, empty, namespaceLabels, subresourceInAdmnReview)...)
}
} else {
rmr := kyvernov1.ResourceFilter{UserInfo: rule.MatchResources.UserInfo, ResourceDescription: rule.MatchResources.ResourceDescription}
reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionMatchHelper(subresourceGVKToAPIResource, rmr, admissionInfo, resource, dynamicConfig, namespaceLabels, subresourceInAdmnReview)...)
reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionMatchHelper(subresourceGVKToAPIResource, rmr, admissionInfo, resource, empty, namespaceLabels, subresourceInAdmnReview)...)
}
if len(rule.ExcludeResources.Any) > 0 {

149
pkg/engine/utils_test.go
View file

@ -4,6 +4,9 @@ import (
"encoding/json"
"testing"
authenticationv1 "k8s.io/api/authentication/v1"
rbacv1 "k8s.io/api/rbac/v1"
v1 "github.com/kyverno/kyverno/api/kyverno/v1"
"github.com/kyverno/kyverno/api/kyverno/v1beta1"
"github.com/kyverno/kyverno/pkg/autogen"
@ -1880,6 +1883,152 @@ func TestResourceDescriptionMatch_MultipleKind(t *testing.T) {
}
}
func TestResourceDescriptionMatch_ExcludeDefaultGroups(t *testing.T) {
// slightly simplified ingress controller pod that lives in the ingress-nginx namespace
rawResource := []byte(`{
"apiVersion": "v1",
"kind": "Pod",
"metadata": {
"name": "ingress-nginx-controller-57bc6474bb-mcpt2",
"namespace": "ingress-nginx"
},
"spec": {
"containers": [
{
"args": ["/nginx-ingress-controller"],
"env": [],
"image": "registry.k8s.io/ingress-nginx/controller:v1.5.1",
"imagePullPolicy": "IfNotPresent",
"name": "controller",
"securityContext": {
"allowPrivilegeEscalation": true,
"capabilities": {
"add": [
"NET_BIND_SERVICE"
],
"drop": [
"ALL"
]
},
"runAsUser": 101
},
"volumeMounts": [
{
"mountPath": "/usr/local/certificates/",
"name": "webhook-cert",
"readOnly": true
},
{
"mountPath": "/var/run/secrets/kubernetes.io/serviceaccount",
"name": "kube-api-access-vc2qz",
"readOnly": true
}
]
}
],
"securityContext": {},
"serviceAccount": "ingress-nginx",
"serviceAccountName": "ingress-nginx"
}
}`)
resource, err := kubeutils.BytesToUnstructured(rawResource)
if err != nil {
t.Errorf("unable to convert raw resource to unstructured: %v", err)
}
// this rule should match only pods in the user1-restricted namespace, and also pods of User:user1.
rule := v1.Rule{
MatchResources: v1.MatchResources{Any: v1.ResourceFilters{
// pods in user1-restricted namespace
v1.ResourceFilter{
ResourceDescription: v1.ResourceDescription{
Kinds: []string{"Pod"},
Namespaces: []string{
"user1-restricted",
},
},
},
// pods for User user1 account
v1.ResourceFilter{
ResourceDescription: v1.ResourceDescription{
Kinds: []string{"Pod"},
},
UserInfo: v1.UserInfo{
Subjects: []rbacv1.Subject{
{
Kind: "User",
Name: "user1",
},
},
},
},
}},
ExcludeResources: v1.MatchResources{},
}
// the following groups are added by default to ensure that these groups or users are always excluded
// these should not affect the rule in our case, we expect our rule to NOT match with the nginx pod.
dynamicConfig := []string{
"system:serviceaccounts:kube-system",
"system:nodes",
"system:kube-scheduler",
}
// this is the request info that was also passed with the mocked pod
requestInfo := v1beta1.RequestInfo{
AdmissionUserInfo: authenticationv1.UserInfo{
Username: "system:serviceaccount:kube-system:replicaset-controller",
UID: "8f36cad4-eb68-4931-bea8-8a42dd1fee4c",
Groups: []string{
"system:serviceaccounts",
"system:serviceaccounts:kube-system",
"system:authenticated",
},
},
}
// First test: confirm that this above rule produces errors (and raise an error if err == nil)
if err := MatchesResourceDescription(make(map[string]*metav1.APIResource), *resource, rule, requestInfo, dynamicConfig, nil, "", ""); err == nil {
t.Error("Testcase was expected to fail, but err was nil")
}
// This next rule *should* match, because we explicitly match the ingress-nginx namespace this time.
rule2 := v1.Rule{
MatchResources: v1.MatchResources{Any: v1.ResourceFilters{
v1.ResourceFilter{
ResourceDescription: v1.ResourceDescription{
Kinds: []string{"Pod"},
Namespaces: []string{
"ingress-nginx",
},
},
},
}},
ExcludeResources: v1.MatchResources{Any: v1.ResourceFilters{}},
}
// Second test: confirm that matching this rule does not create any errors (and raise if err != nil)
if err := MatchesResourceDescription(make(map[string]*metav1.APIResource), *resource, rule2, requestInfo, dynamicConfig, nil, "", ""); err != nil {
t.Errorf("Testcase was expected to not fail, but err was %s", err)
}
// Now we extend the previous rule to have an Exclude part. Making it 'not-empty' should make the exclude-code run.
rule2.ExcludeResources = v1.MatchResources{Any: v1.ResourceFilters{
v1.ResourceFilter{
ResourceDescription: v1.ResourceDescription{
Kinds: []string{"Pod"},
},
},
}}
// Third test: confirm that now the custom exclude-snippet should run in CheckSubjects() and that should result in this rule failing (raise if err == nil for that reason)
if err := MatchesResourceDescription(make(map[string]*metav1.APIResource), *resource, rule2, requestInfo, dynamicConfig, nil, "", ""); err == nil {
t.Error("Testcase was expected to fail, but err was nil #1!")
}
}
// Match resource name
func TestResourceDescriptionMatch_Name(t *testing.T) {
rawResource := []byte(`{