Skip to content

Commit 67fb159

Browse files
authoredJan 9, 2024
Merge pull request #169 from viccuad/fix-resource-fetcher
fix: Correctly skip unexistent resources
2 parents 1e01745 + ea9999a commit 67fb159

File tree

5 files changed

+90
-17
lines changed

5 files changed

+90
-17
lines changed
 

‎go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ require (
88
github.com/kubewarden/kubewarden-controller v1.9.0
99
github.com/rs/zerolog v1.31.0
1010
github.com/spf13/cobra v1.8.0
11-
golang.org/x/exp v0.0.0-20240103183307-be819d1f06fc
1211
k8s.io/api v0.29.0
1312
k8s.io/apimachinery v0.29.0
1413
k8s.io/client-go v0.29.0
@@ -52,6 +51,7 @@ require (
5251
github.com/prometheus/common v0.44.0 // indirect
5352
github.com/prometheus/procfs v0.10.1 // indirect
5453
github.com/spf13/pflag v1.0.5 // indirect
54+
golang.org/x/exp v0.0.0-20240103183307-be819d1f06fc // indirect
5555
golang.org/x/net v0.17.0 // indirect
5656
golang.org/x/oauth2 v0.10.0 // indirect
5757
golang.org/x/sys v0.13.0 // indirect

‎go.sum

-8
Original file line numberDiff line numberDiff line change
@@ -476,14 +476,6 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0
476476
golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4=
477477
golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM=
478478
golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU=
479-
golang.org/x/exp v0.0.0-20231206192017-f3f8817b8deb h1:c0vyKkb6yr3KR7jEfJaOSv4lG7xPkbN6r52aJz1d8a8=
480-
golang.org/x/exp v0.0.0-20231206192017-f3f8817b8deb/go.mod h1:iRJReGqOEeBhDZGkGbynYwcHlctCvnjTYIamk7uXpHI=
481-
golang.org/x/exp v0.0.0-20231214170342-aacd6d4b4611 h1:qCEDpW1G+vcj3Y7Fy52pEM1AWm3abj8WimGYejI3SC4=
482-
golang.org/x/exp v0.0.0-20231214170342-aacd6d4b4611/go.mod h1:iRJReGqOEeBhDZGkGbynYwcHlctCvnjTYIamk7uXpHI=
483-
golang.org/x/exp v0.0.0-20231219180239-dc181d75b848 h1:+iq7lrkxmFNBM7xx+Rae2W6uyPfhPeDWD+n+JgppptE=
484-
golang.org/x/exp v0.0.0-20231219180239-dc181d75b848/go.mod h1:iRJReGqOEeBhDZGkGbynYwcHlctCvnjTYIamk7uXpHI=
485-
golang.org/x/exp v0.0.0-20231226003508-02704c960a9b h1:kLiC65FbiHWFAOu+lxwNPujcsl8VYyTYYEZnsOO1WK4=
486-
golang.org/x/exp v0.0.0-20231226003508-02704c960a9b/go.mod h1:iRJReGqOEeBhDZGkGbynYwcHlctCvnjTYIamk7uXpHI=
487479
golang.org/x/exp v0.0.0-20240103183307-be819d1f06fc h1:ao2WRsKSzW6KuUY9IWPwWahcHCgR0s52IfwutMfEbdM=
488480
golang.org/x/exp v0.0.0-20240103183307-be819d1f06fc/go.mod h1:iRJReGqOEeBhDZGkGbynYwcHlctCvnjTYIamk7uXpHI=
489481
golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=

‎internal/policies/filter_test.go

+18
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,23 @@ func TestFilterAuditablePolicies(t *testing.T) {
4545
},
4646
Status: policiesv1.PolicyStatus{PolicyStatus: policiesv1.PolicyStatusActive},
4747
}
48+
hasAllResourcesOperations := policiesv1.AdmissionPolicy{
49+
Spec: policiesv1.AdmissionPolicySpec{
50+
PolicySpec: policiesv1.PolicySpec{
51+
BackgroundAudit: true,
52+
Rules: []admissionregistrationv1.RuleWithOperations{{
53+
Operations: []admissionregistrationv1.OperationType{"*"},
54+
Rule: admissionregistrationv1.Rule{
55+
APIGroups: []string{"*"},
56+
APIVersions: []string{"*"},
57+
Resources: []string{"*"},
58+
Scope: nil,
59+
},
60+
}},
61+
},
62+
},
63+
Status: policiesv1.PolicyStatus{PolicyStatus: policiesv1.PolicyStatusActive},
64+
}
4865
auditable := policiesv1.AdmissionPolicy{
4966
Spec: policiesv1.AdmissionPolicySpec{
5067
PolicySpec: policiesv1.PolicySpec{
@@ -72,6 +89,7 @@ func TestFilterAuditablePolicies(t *testing.T) {
7289
{"policy not active", []policiesv1.Policy{&pending}, []policiesv1.Policy{}},
7390
{"policy without create operation", []policiesv1.Policy{&noCreate}, []policiesv1.Policy{}},
7491
{"policy with all resources (*)", []policiesv1.Policy{&hasAllResources}, []policiesv1.Policy{}},
92+
{"policy with all resources and operations (*)", []policiesv1.Policy{&hasAllResourcesOperations}, []policiesv1.Policy{}},
7593
{"auditable policy", []policiesv1.Policy{&auditable}, []policiesv1.Policy{&auditable}},
7694
{"auditable policy with all the others policies", []policiesv1.Policy{&auditable, &noBackgroundAudit, &noCreate, &pending, &hasAllResources}, []policiesv1.Policy{&auditable}},
7795
{"empty array", []policiesv1.Policy{}, []policiesv1.Policy{}},

‎internal/resources/fetcher.go

+3-5
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,9 @@ package resources
22

33
import (
44
"context"
5-
"errors"
65
"fmt"
76
"net/url"
87

9-
"github.com/kubewarden/audit-scanner/internal/constants"
108
policiesv1 "github.com/kubewarden/kubewarden-controller/pkg/apis/policies/v1"
119
"github.com/rs/zerolog"
1210
"github.com/rs/zerolog/log"
@@ -73,7 +71,7 @@ func (f *Fetcher) GetResourcesForPolicies(ctx context.Context, policies []polici
7371
for resourceFilter, policies := range gvrMap {
7472
isNamespaced, err := f.isNamespacedResource(resourceFilter.groupVersionResource)
7573
if err != nil {
76-
if errors.Is(err, constants.ErrResourceNotFound) {
74+
if apimachineryerrors.IsNotFound(err) {
7775
log.Warn().
7876
Str("resource GVK", resourceFilter.groupVersionResource.String()).
7977
Msg("API resource not found")
@@ -133,7 +131,7 @@ func (f *Fetcher) isNamespacedResource(gvr schema.GroupVersionResource) (bool, e
133131
return apiResource.Namespaced, nil
134132
}
135133
}
136-
return false, constants.ErrResourceNotFound
134+
return false, apimachineryerrors.NewNotFound(gvr.GroupResource(), gvr.Resource)
137135
}
138136

139137
// GetClusterWideResourcesForPolicies fetches all cluster wide resources that must be
@@ -150,7 +148,7 @@ func (f *Fetcher) GetClusterWideResourcesForPolicies(ctx context.Context, polici
150148
for resourceFilter, policies := range gvrMap {
151149
isNamespaced, err := f.isNamespacedResource(resourceFilter.groupVersionResource)
152150
if err != nil {
153-
if errors.Is(err, constants.ErrResourceNotFound) {
151+
if apimachineryerrors.IsNotFound(err) {
154152
log.Warn().
155153
Str("resource GVK", resourceFilter.groupVersionResource.String()).
156154
Msg("API resource not found")

‎internal/resources/fetcher_test.go

+68-3
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"testing"
77

88
"github.com/google/go-cmp/cmp"
9-
"github.com/kubewarden/audit-scanner/internal/constants"
109
policiesv1 "github.com/kubewarden/kubewarden-controller/pkg/apis/policies/v1"
1110
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
1211
appsv1 "k8s.io/api/apps/v1"
@@ -120,6 +119,22 @@ var policyPodsNamespaces = policiesv1.ClusterAdmissionPolicy{
120119
}},
121120
}
122121

122+
// used to test incorrect resources, when using *
123+
var policyAsteriskRules = policiesv1.ClusterAdmissionPolicy{
124+
Spec: policiesv1.ClusterAdmissionPolicySpec{PolicySpec: policiesv1.PolicySpec{
125+
Rules: []admissionregistrationv1.RuleWithOperations{
126+
{
127+
Operations: nil,
128+
Rule: admissionregistrationv1.Rule{
129+
APIGroups: []string{"*"},
130+
APIVersions: []string{"v1"},
131+
Resources: []string{"pods"},
132+
},
133+
},
134+
},
135+
}},
136+
}
137+
123138
func TestGetResourcesForPolicies(t *testing.T) {
124139
pod1 := v1.Pod{
125140
ObjectMeta: metav1.ObjectMeta{
@@ -247,6 +262,7 @@ func TestGetResourcesForPolicies(t *testing.T) {
247262
{"policy with label filter", []policiesv1.Policy{&policy4}, expectedP4, "kubewarden"},
248263
{"we skip incorrect GVKs", []policiesv1.Policy{&policyIncorrectRules}, expectedPIncorrectRules, "default"},
249264
{"we skip clusterwide resources", []policiesv1.Policy{&policyPodsNamespaces}, expectedPPodsNamespaces, "default"}, // namespaces get filtered
265+
{"we skip asterisk rules, they get expanded by clientgo", []policiesv1.Policy{&policyAsteriskRules}, []AuditableResources{}, "default"},
250266
}
251267

252268
for _, test := range tests {
@@ -537,6 +553,46 @@ func TestGetClusterWideResourcesForPolicies(t *testing.T) {
537553
PolicyStatus: policiesv1.PolicyStatusActive,
538554
},
539555
}
556+
policyWithAsteriskRules := policiesv1.ClusterAdmissionPolicy{
557+
ObjectMeta: metav1.ObjectMeta{
558+
Name: "cap",
559+
// It's necessary to define ResourceVersion and Generation
560+
// because the fake client can set values for these fields.
561+
// See more at docs:
562+
// ObjectMeta's `Generation` and `ResourceVersion` don't
563+
// behave properly, Patch or Update operations that rely
564+
// on these fields will fail, or give false positives.
565+
// https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/client/fake
566+
ResourceVersion: "123",
567+
Generation: 1,
568+
},
569+
Spec: policiesv1.ClusterAdmissionPolicySpec{
570+
PolicySpec: policiesv1.PolicySpec{
571+
BackgroundAudit: true,
572+
Rules: []admissionregistrationv1.RuleWithOperations{
573+
{
574+
Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.Create},
575+
Rule: admissionregistrationv1.Rule{
576+
APIGroups: []string{"*"},
577+
APIVersions: []string{"v1"},
578+
Resources: []string{"pods", "namespaces"},
579+
},
580+
},
581+
{
582+
Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.Create},
583+
Rule: admissionregistrationv1.Rule{
584+
APIGroups: []string{""},
585+
APIVersions: []string{"v1"},
586+
Resources: []string{"pods", "namespaces"},
587+
},
588+
},
589+
},
590+
},
591+
},
592+
Status: policiesv1.PolicyStatus{
593+
PolicyStatus: policiesv1.PolicyStatusActive,
594+
},
595+
}
540596

541597
customScheme := scheme.Scheme
542598
customScheme.AddKnownTypes(policiesv1.GroupVersion, &policiesv1.ClusterAdmissionPolicy{}, &policiesv1.AdmissionPolicy{}, &policiesv1.ClusterAdmissionPolicyList{}, &policiesv1.AdmissionPolicyList{})
@@ -605,6 +661,10 @@ func TestGetClusterWideResourcesForPolicies(t *testing.T) {
605661
Policies: []policiesv1.Policy{&policyWithLabelFilter},
606662
Resources: []unstructured.Unstructured{{Object: unstructuredNamespace2}},
607663
}}},
664+
{"Filter cluster wide resource with policies with asterisk rules", []policiesv1.Policy{&policyWithAsteriskRules}, []AuditableResources{{
665+
Policies: []policiesv1.Policy{&policyWithAsteriskRules},
666+
Resources: []unstructured.Unstructured{{Object: unstructuredNamespace}, {Object: unstructuredNamespace2}},
667+
}}},
608668
}
609669

610670
for _, test := range tests {
@@ -706,7 +766,12 @@ func TestIsNamespacedResource(t *testing.T) {
706766
Version: "v1",
707767
Resource: "foos",
708768
},
709-
false, constants.ErrResourceNotFound,
769+
false,
770+
apimachineryerrors.NewNotFound(
771+
schema.GroupResource{
772+
Group: "",
773+
Resource: "foos",
774+
}, "foos"),
710775
},
711776
}
712777

@@ -721,7 +786,7 @@ func TestIsNamespacedResource(t *testing.T) {
721786
fetcher := Fetcher{dynamicClient, "", "", fakeClientSet}
722787

723788
isNamespaced, err := fetcher.isNamespacedResource(ttest.gvr)
724-
if (err != nil && ttest.expectedErr != nil && !errors.Is(err, ttest.expectedErr)) || (err != nil && ttest.expectedErr == nil) {
789+
if (err != nil && ttest.expectedErr != nil && err.Error() != ttest.expectedErr.Error()) || (err != nil && ttest.expectedErr == nil) {
725790
t.Errorf("unexpected error: " + err.Error())
726791
}
727792
if isNamespaced != ttest.expectedIsNamespaced {

0 commit comments

Comments
 (0)
Please sign in to comment.