Skip to content

Commit

Permalink
Merge pull request #339 from viccuad/fix-summary
Browse files Browse the repository at this point in the history 
fix: Propagate skipped,errored numbers from audited policies
  • Loading branch information
@viccuad
viccuad authored Aug 2, 2024
2 parents 3c6ed1d + cdb2de9 commit e4a49fa
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 10 deletions.
10 changes: 10 additions & 0 deletions internal/report/report_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ func TestNewPolicyReport(t *testing.T) {
resource.SetResourceVersion("12345")

policyReport := NewPolicyReport("runUID", resource)
policyReport.Summary.Skip = 1
policyReport.Summary.Error = 1

assert.Equal(t, "uid", policyReport.ObjectMeta.Name)
assert.Equal(t, "namespace", policyReport.ObjectMeta.Namespace)
Expand All @@ -42,6 +44,9 @@ func TestNewPolicyReport(t *testing.T) {
assert.Equal(t, types.UID("uid"), policyReport.Scope.UID)
assert.Equal(t, "12345", policyReport.Scope.ResourceVersion)

assert.Equal(t, 1, policyReport.Summary.Skip)
assert.Equal(t, 1, policyReport.Summary.Error)

assert.Empty(t, policyReport.Results)
}

Expand Down Expand Up @@ -73,6 +78,8 @@ func TestNewClusterPolicyReport(t *testing.T) {
resource.SetResourceVersion("12345")

clusterPolicyReport := NewClusterPolicyReport("runUID", resource)
clusterPolicyReport.Summary.Skip = 1
clusterPolicyReport.Summary.Error = 1

assert.Equal(t, "uid", clusterPolicyReport.ObjectMeta.Name)
assert.Equal(t, "kubewarden", clusterPolicyReport.ObjectMeta.Labels[labelAppManagedBy])
Expand All @@ -90,6 +97,9 @@ func TestNewClusterPolicyReport(t *testing.T) {
assert.Equal(t, types.UID("uid"), clusterPolicyReport.Scope.UID)
assert.Equal(t, "12345", clusterPolicyReport.Scope.ResourceVersion)

assert.Equal(t, 1, clusterPolicyReport.Summary.Skip)
assert.Equal(t, 1, clusterPolicyReport.Summary.Error)

assert.Empty(t, clusterPolicyReport.Results)
}

Expand Down
20 changes: 12 additions & 8 deletions internal/scanner/scanner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func (s *Scanner) ScanNamespace(ctx context.Context, nsName, runUID string) erro
Int("policies-errored", policies.ErroredNum),
).Msg("policy count")

for gvr, policies := range policies.PoliciesByGVR {
for gvr, pols := range policies.PoliciesByGVR {
pager, err := s.k8sClient.GetResources(gvr, nsName)
if err != nil {
log.Error().Err(err).Str("gvr", gvr.String()).Str("ns", nsName).Msg("failed to get resources")
Expand All @@ -169,13 +169,13 @@ func (s *Scanner) ScanNamespace(ctx context.Context, nsName, runUID string) erro
return err
}
workers.Add(1)
policiesToAudit := policies
policiesToAudit := pols

go func() {
defer semaphore.Release(1)
defer workers.Done()

if err := s.auditResource(ctx, policiesToAudit, *resource, runUID); err != nil {
if err := s.auditResource(ctx, policiesToAudit, *resource, runUID, policies.SkippedNum, policies.ErroredNum); err != nil {
log.Error().Err(err).Str("RunUID", runUID).Msg("error auditing resource")
}
}()
Expand Down Expand Up @@ -257,7 +257,7 @@ func (s *Scanner) ScanClusterWideResources(ctx context.Context, runUID string) e
Int("parallel-resources-audits", s.parallelResourcesAudits),
).Msg("cluster admission policies count")

for gvr, policies := range policies.PoliciesByGVR {
for gvr, pols := range policies.PoliciesByGVR {
pager, err := s.k8sClient.GetResources(gvr, "")
if err != nil {
return err
Expand All @@ -274,13 +274,13 @@ func (s *Scanner) ScanClusterWideResources(ctx context.Context, runUID string) e
if err != nil {
return err
}
policiesToAudit := policies
policiesToAudit := pols

go func() {
defer semaphore.Release(1)
defer workers.Done()

s.auditClusterResource(ctx, policiesToAudit, *resource, runUID)
s.auditClusterResource(ctx, policiesToAudit, *resource, runUID, policies.SkippedNum, policies.ErroredNum)
}()

return nil
Expand All @@ -305,7 +305,7 @@ type policyAuditResult struct {
errored bool
}

func (s *Scanner) auditResource(ctx context.Context, policies []*policies.Policy, resource unstructured.Unstructured, runUID string) error {
func (s *Scanner) auditResource(ctx context.Context, policies []*policies.Policy, resource unstructured.Unstructured, runUID string, skippedPoliciesNum, erroredPoliciesNum int) error {
log.Info().
Str("resource", resource.GetName()).
Dict("dict", zerolog.Dict().
Expand Down Expand Up @@ -388,6 +388,8 @@ func (s *Scanner) auditResource(ctx context.Context, policies []*policies.Policy
close(auditResults)

policyReport := report.NewPolicyReport(runUID, resource)
policyReport.Summary.Skip = skippedPoliciesNum
policyReport.Summary.Error = erroredPoliciesNum
for res := range auditResults {
report.AddResultToPolicyReport(policyReport, res.policy, res.admissionReviewResponse, res.errored)
}
Expand All @@ -413,14 +415,16 @@ func (s *Scanner) auditResource(ctx context.Context, policies []*policies.Policy
return nil
}

func (s *Scanner) auditClusterResource(ctx context.Context, policies []*policies.Policy, resource unstructured.Unstructured, runUID string) {
func (s *Scanner) auditClusterResource(ctx context.Context, policies []*policies.Policy, resource unstructured.Unstructured, runUID string, skippedPoliciesNum, erroredPoliciesNum int) {
log.Info().
Str("resource", resource.GetName()).
Dict("dict", zerolog.Dict().
Int("policies-to-evaluate", len(policies)),
).Msg("audit clusterwide resource")

clusterPolicyReport := report.NewClusterPolicyReport(runUID, resource)
clusterPolicyReport.Summary.Skip = skippedPoliciesNum
clusterPolicyReport.Summary.Error = erroredPoliciesNum
for _, p := range policies {
url := p.PolicyServer
policy := p.Policy
Expand Down
37 changes: 35 additions & 2 deletions internal/scanner/scanner_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ func TestScanAllNamespaces(t *testing.T) {
Status(policiesv1.PolicyStatusActive).
Build()

// an AdmissionPolicy targeting an unknown GVR, should error and be skipped
// an AdmissionPolicy targeting an unknown GVR, should be counted as error
admissionPolicyUnknownGVR := testutils.
NewAdmissionPolicyFactory().
Name("policy6").
Expand All @@ -225,6 +225,19 @@ func TestScanAllNamespaces(t *testing.T) {
Status(policiesv1.PolicyStatusActive).
Build()

// an AdmissionPolicy targeting a GVR with *, should be skipped
admissionPolicyAsteriskGVR := testutils.
NewAdmissionPolicyFactory().
Name("policy7").
Namespace("namespace1").
Rule(admissionregistrationv1.Rule{
APIGroups: []string{"apps"},
APIVersions: []string{"v1"},
Resources: []string{"*"},
}).
Status(policiesv1.PolicyStatusActive).
Build()

// add a policy report that should be deleted by the scanner
oldPolicyReportRunUID := uuid.New().String()
oldPolicyReport := testutils.NewPolicyReportFactory().
Expand Down Expand Up @@ -260,6 +273,7 @@ func TestScanAllNamespaces(t *testing.T) {
admissionPolicy3,
admissionPolicy4,
admissionPolicyUnknownGVR,
admissionPolicyAsteriskGVR,
clusterAdmissionPolicy,
oldPolicyReport,
)
Expand Down Expand Up @@ -288,6 +302,8 @@ func TestScanAllNamespaces(t *testing.T) {
err = client.Get(context.TODO(), types.NamespacedName{Name: string(pod1.GetUID()), Namespace: "namespace1"}, &policyReport)
require.NoError(t, err)
assert.Equal(t, 2, policyReport.Summary.Pass)
assert.Equal(t, 1, policyReport.Summary.Error)
assert.Equal(t, 1, policyReport.Summary.Skip)
assert.Len(t, policyReport.Results, 2)
assert.Equal(t, runUID, policyReport.GetLabels()[auditConstants.AuditScannerRunUIDLabel])

Expand All @@ -300,6 +316,8 @@ func TestScanAllNamespaces(t *testing.T) {
err = client.Get(context.TODO(), types.NamespacedName{Name: string(deployment1.GetUID()), Namespace: "namespace1"}, &policyReport)
require.NoError(t, err)
assert.Equal(t, 2, policyReport.Summary.Pass)
assert.Equal(t, 1, policyReport.Summary.Error)
assert.Equal(t, 1, policyReport.Summary.Skip)
assert.Len(t, policyReport.Results, 2)
assert.Equal(t, runUID, policyReport.GetLabels()[auditConstants.AuditScannerRunUIDLabel])

Expand Down Expand Up @@ -397,7 +415,7 @@ func TestScanClusterWideResources(t *testing.T) {
Status(policiesv1.PolicyStatusActive).
Build()

// a ClusterAdmissionPolicy targeting an unknown GVR, should error and be skipped
// a ClusterAdmissionPolicy targeting an unknown GVR, should be counted as error
clusterAdmissionPolicyUnknownGVR := testutils.
NewClusterAdmissionPolicyFactory().
Name("policy4").
Expand All @@ -409,6 +427,18 @@ func TestScanClusterWideResources(t *testing.T) {
Status(policiesv1.PolicyStatusActive).
Build()

// a ClusterAdmissionPolicy targeting a GVR with *, should be counted as skipped
clusterAdmissionPolicyAsteriskGVR := testutils.
NewClusterAdmissionPolicyFactory().
Name("policy5").
Rule(admissionregistrationv1.Rule{
APIGroups: []string{""},
APIVersions: []string{"v1"},
Resources: []string{"*"},
}).
Status(policiesv1.PolicyStatusActive).
Build()

// add a policy report that should be deleted by the scanner
oldClusterPolicyReportRunUID := uuid.New().String()
oldClusterPolicyReport := testutils.NewClusterPolicyReportFactory().
Expand Down Expand Up @@ -436,6 +466,7 @@ func TestScanClusterWideResources(t *testing.T) {
clusterAdmissionPolicy2,
clusterAdmissionPolicy3,
clusterAdmissionPolicyUnknownGVR,
clusterAdmissionPolicyAsteriskGVR,
oldClusterPolicyReport,
)
require.NoError(t, err)
Expand Down Expand Up @@ -463,6 +494,8 @@ func TestScanClusterWideResources(t *testing.T) {
err = client.Get(context.TODO(), types.NamespacedName{Name: string(namespace1.GetUID())}, &clusterPolicyReport)
require.NoError(t, err)
assert.Equal(t, 1, clusterPolicyReport.Summary.Pass)
assert.Equal(t, 1, clusterPolicyReport.Summary.Error)
assert.Equal(t, 1, clusterPolicyReport.Summary.Skip)
assert.Len(t, clusterPolicyReport.Results, 1)
assert.Equal(t, runUID, clusterPolicyReport.GetLabels()[auditConstants.AuditScannerRunUIDLabel])

Expand Down

0 comments on commit e4a49fa

Please sign in to comment.