From ee11c57ea31afdc4d5db9c94fd4d13c6f8fa2bc3 Mon Sep 17 00:00:00 2001 From: Frank Jogeleit Date: Mon, 10 Mar 2025 09:28:07 +0100 Subject: [PATCH] simplify client informer handling (#811) * simplify client informer handling Signed-off-by: Frank Jogeleit * add new test cases Signed-off-by: Frank Jogeleit * update startup logic to handle runtime target changes Signed-off-by: Frank Jogeleit --------- Signed-off-by: Frank Jogeleit --- Makefile | 2 +- cmd/run.go | 21 +-- pkg/cache/cache.go | 1 - pkg/cache/memory.go | 20 --- pkg/cache/redis.go | 5 - pkg/config/resolver.go | 44 ++--- pkg/config/resolver_test.go | 4 +- pkg/fixtures/policy_results.go | 55 ++++++ pkg/helper/chunk_slice_test.go | 18 ++ pkg/helper/title_test.go | 14 ++ pkg/kubernetes/policy_report_client.go | 2 +- pkg/listener/cleanup_test.go | 11 ++ pkg/listener/fixture_test.go | 19 +++ pkg/listener/new_result.go | 8 +- pkg/listener/new_result_test.go | 66 ++++++++ pkg/listener/scope_results.go | 3 + pkg/listener/send_result.go | 3 + pkg/report/source_filter_test.go | 225 +++++++++++++++++++++++++ pkg/targetconfig/client.go | 77 +++++++++ pkg/targetconfig/client_test.go | 197 ++++++++++++++++++++++ pkg/targetconfig/tc.go | 120 ------------- pkg/validate/model_test.go | 29 ++++ 22 files changed, 738 insertions(+), 206 deletions(-) create mode 100644 pkg/helper/chunk_slice_test.go create mode 100644 pkg/helper/title_test.go create mode 100644 pkg/report/source_filter_test.go create mode 100644 pkg/targetconfig/client.go create mode 100644 pkg/targetconfig/client_test.go delete mode 100644 pkg/targetconfig/tc.go create mode 100644 pkg/validate/model_test.go diff --git a/Makefile b/Makefile index e82f76f5e..98345fc9f 100644 --- a/Makefile +++ b/Makefile @@ -241,7 +241,7 @@ test: .PHONY: coverage coverage: go test -v ./... -covermode=count -coverprofile=coverage.out.tmp -timeout=30s - cat coverage.out.tmp | grep -v "github.com/kyverno/policy-reporter/cmd/" | grep -v "github.com/kyverno/policy-reporter/main.go" | grep -v "github.com/kyverno/policy-reporter/pkg/crd/" > coverage.out + cat coverage.out.tmp | grep -v "github.com/kyverno/policy-reporter/cmd/" | grep -v "github.com/kyverno/policy-reporter/main.go" | grep -v "github.com/kyverno/policy-reporter/pkg/crd/" | grep -v "github.com/kyverno/policy-reporter/hack/main.go" | grep -v "github.com/kyverno/policy-reporter/hack/controller-gen/" > coverage.out rm coverage.out.tmp .PHONY: build diff --git a/cmd/run.go b/cmd/run.go index f3105d657..eebf77dd3 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -17,7 +17,6 @@ import ( "github.com/kyverno/policy-reporter/pkg/config" "github.com/kyverno/policy-reporter/pkg/database" "github.com/kyverno/policy-reporter/pkg/listener" - "github.com/kyverno/policy-reporter/pkg/targetconfig" ) func newRunCMD(version string) *cobra.Command { @@ -63,7 +62,6 @@ func newRunCMD(version string) *cobra.Command { return err } - targetChan := make(chan targetconfig.TcEvent) g := &errgroup.Group{} var store *database.Store @@ -142,7 +140,7 @@ func newRunCMD(version string) *cobra.Command { } } - resolver.RegisterSendResultListener(targetChan) + resolver.RegisterSendResultListener() readinessProbe.Ready() }).RegisterOnNew(func(currentID, lockID string) { @@ -167,7 +165,7 @@ func newRunCMD(version string) *cobra.Command { return elector.Run(cmd.Context()) }) } else { - resolver.RegisterSendResultListener(targetChan) + resolver.RegisterSendResultListener() readinessProbe.Ready() } @@ -179,11 +177,8 @@ func newRunCMD(version string) *cobra.Command { g.Go(server.Start) g.Go(func() error { - // call TargetClients to ensure targets passed from the config file are initialized - resolver.TargetClients() stop := make(chan struct{}) - - _, err = resolver.TargetConfigClient(targetChan) + _, err = resolver.TargetConfigClient() if err != nil { return err } @@ -196,7 +191,7 @@ func newRunCMD(version string) *cobra.Command { }) g.Go(func() error { - logger.Info("wait policy informer") + logger.Info("wait for policy informer") readinessProbe.Wait() logger.Info("start client", zap.Int("worker", c.WorkerCount)) @@ -212,15 +207,11 @@ func newRunCMD(version string) *cobra.Command { }) g.Go(func() error { - collection := resolver.TargetClients() - if !collection.UsesSecrets() { - return nil - } - + logger.Info("wait for secret informer") readinessProbe.Wait() stop := make(chan struct{}) - if err := secretInformer.Sync(collection, stop); err != nil { + if err := secretInformer.Sync(resolver.TargetClients(), stop); err != nil { zap.L().Error("secret informer error", zap.Error(err)) return err diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index 884183d09..2ba095c7d 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -8,5 +8,4 @@ type Cache interface { GetResults(id string) []string Shared() bool Clear() - Clone() Cache } diff --git a/pkg/cache/memory.go b/pkg/cache/memory.go index f65495daf..597b6c486 100644 --- a/pkg/cache/memory.go +++ b/pkg/cache/memory.go @@ -82,26 +82,6 @@ func (c *inMemoryCache) Shared() bool { return false } -func (c *inMemoryCache) Clone() Cache { - oldItems := c.caches.Items() - // this is the upper cache - newCache := gocache.New(gocache.NoExpiration, 5*time.Minute) - - for key, item := range oldItems { - c2 := item.Object.(*gocache.Cache).Items() - innerCache := gocache.New(gocache.NoExpiration, 5*time.Minute) - - for innerKey := range c2 { - innerCache.Set(innerKey, struct{}{}, c.keepDuration) - } - - newCache.Set(key, innerCache, c.keepDuration) - } - return &inMemoryCache{ - caches: newCache, - } -} - func NewInMermoryCache(keepDuration, keepReport time.Duration) Cache { cache := gocache.New(gocache.NoExpiration, 5*time.Minute) cache.OnEvicted(func(s string, i interface{}) { diff --git a/pkg/cache/redis.go b/pkg/cache/redis.go index 75b72cb14..db0748ef4 100644 --- a/pkg/cache/redis.go +++ b/pkg/cache/redis.go @@ -40,11 +40,6 @@ func (r *redisCache) AddReport(report v1alpha2.ReportInterface) { } } -// doesn't make sense in redis -func (r *redisCache) Clone() Cache { - return r -} - func (r *redisCache) RemoveReport(id string) { keys, err := r.rdb.Keys(context.Background(), r.generateKeyPattern(id)).Result() if err != nil { diff --git a/pkg/config/resolver.go b/pkg/config/resolver.go index 936ef2aa3..069428880 100644 --- a/pkg/config/resolver.go +++ b/pkg/config/resolver.go @@ -4,7 +4,6 @@ import ( "context" "crypto/tls" "crypto/x509" - "fmt" "os" "strings" "time" @@ -63,7 +62,7 @@ type Resolver struct { targetClients *target.Collection targetsCreated bool targetFactory target.Factory - targetConfigClient *targetconfig.TargetConfigClient + targetConfigClient *targetconfig.Client logger *zap.Logger resultListener *listener.ResultListener polrRestartCh chan struct{} @@ -261,42 +260,22 @@ func (r *Resolver) Queue() (*kubernetes.Queue, error) { func (r *Resolver) RegisterNewResultsListener() { targets := r.TargetClients() - newResultListener := listener.NewResultListener(r.SkipExistingOnStartup(), r.ResultCache(), time.Now()) - r.resultListener = newResultListener - r.EventPublisher().RegisterListener(listener.NewResults, newResultListener.Listen) + r.resultListener = listener.NewResultListener(r.SkipExistingOnStartup(), r.ResultCache(), time.Now()) + r.EventPublisher().RegisterListener(listener.NewResults, r.resultListener.Listen) r.EventPublisher().RegisterPostListener(listener.CleanUpListener, listener.NewCleanupListener(context.Background(), targets)) } // RegisterSendResultListener resolver method -func (r *Resolver) RegisterSendResultListener(targetChan chan targetconfig.TcEvent) { - registerFunc := func(targets *target.Collection) { - r.resultListener.RegisterListener(listener.NewSendResultListener(targets)) - r.resultListener.RegisterScopeListener(listener.NewSendScopeResultsListener(targets)) - r.resultListener.RegisterSyncListener(listener.NewSendSyncResultsListener(targets)) - } - - go func() { - r.logger.Info("starting listener loop") - for { - select { - case event := <-targetChan: - // clear existing listeners and create new ones - r.logger.Info(fmt.Sprintf("received targetconfig event of type %s", event.Type)) - r.resultListener.ResetListeners() - registerFunc(event.Targets) - - case <-time.After(5 * time.Second): - } - } - }() - +func (r *Resolver) RegisterSendResultListener() { targets := r.TargetClients() if r.resultListener == nil { r.RegisterNewResultsListener() } - registerFunc(targets) + r.resultListener.RegisterListener(listener.NewSendResultListener(targets)) + r.resultListener.RegisterScopeListener(listener.NewSendScopeResultsListener(targets)) + r.resultListener.RegisterSyncListener(listener.NewSendSyncResultsListener(targets)) } // UnregisterSendResultListener resolver method @@ -569,7 +548,7 @@ func (r *Resolver) EmailClient() *email.Client { return email.NewClient(r.config.EmailReports.SMTP.From, r.SMTPServer()) } -func (r *Resolver) TargetConfigClient(targetChan chan targetconfig.TcEvent) (*targetconfig.TargetConfigClient, error) { +func (r *Resolver) TargetConfigClient() (*targetconfig.Client, error) { if r.targetConfigClient != nil { return r.targetConfigClient, nil } @@ -579,11 +558,8 @@ func (r *Resolver) TargetConfigClient(targetChan chan targetconfig.TcEvent) (*ta return nil, err } - tcc := targetconfig.NewTargetConfigClient(tcClient, r.TargetFactory(), r.TargetClients(), r.logger) - err = tcc.CreateInformer(targetChan) - if err != nil { - return nil, err - } + tcc := targetconfig.NewClient(tcClient, r.TargetFactory(), r.TargetClients(), r.logger) + tcc.ConfigureInformer() r.targetConfigClient = tcc return tcc, nil diff --git a/pkg/config/resolver_test.go b/pkg/config/resolver_test.go index bc089f756..dc6d736c8 100644 --- a/pkg/config/resolver_test.go +++ b/pkg/config/resolver_test.go @@ -12,7 +12,6 @@ import ( "github.com/kyverno/policy-reporter/pkg/database" "github.com/kyverno/policy-reporter/pkg/report" "github.com/kyverno/policy-reporter/pkg/target" - "github.com/kyverno/policy-reporter/pkg/targetconfig" ) var targets = target.Targets{ @@ -464,8 +463,7 @@ func Test_RegisterSendResultListener(t *testing.T) { t.Run("Register SendResultListener with Targets", func(t *testing.T) { resolver := config.NewResolver(testConfig, &rest.Config{}) resolver.Logger() - targetChan := make(chan targetconfig.TcEvent) - resolver.RegisterSendResultListener(targetChan) + resolver.RegisterSendResultListener() assert.Len(t, resolver.EventPublisher().GetListener(), 1, "Expected one Listener to be registered") }) diff --git a/pkg/fixtures/policy_results.go b/pkg/fixtures/policy_results.go index b0b1f68df..a76ded99a 100644 --- a/pkg/fixtures/policy_results.go +++ b/pkg/fixtures/policy_results.go @@ -45,6 +45,44 @@ var PassPodResult = v1alpha2.PolicyReportResult{ Properties: map[string]string{}, } +var WarnPodResult = v1alpha2.PolicyReportResult{ + ID: "124", + Message: "validation error: requests and limits required. Rule autogen-check-for-requests-and-limits failed at path /spec/template/spec/containers/0/resources/requests/", + Policy: "require-requests-and-limits-required", + Rule: "autogen-check-for-requests-and-limits", + Result: v1alpha2.StatusWarn, + Category: "Best Practices", + Scored: true, + Source: "Kyverno", + Resources: []corev1.ObjectReference{{ + APIVersion: "v1", + Kind: "Pod", + Name: "nginx", + Namespace: "test", + UID: "536ab69f-1b3c-4bd9-9ba4-274a56188419", + }}, + Properties: map[string]string{}, +} + +var ErrorPodResult = v1alpha2.PolicyReportResult{ + ID: "124", + Message: "validation error: requests and limits required. Rule autogen-check-for-requests-and-limits failed at path /spec/template/spec/containers/0/resources/requests/", + Policy: "require-requests-and-limits-required", + Rule: "autogen-check-for-requests-and-limits", + Result: v1alpha2.StatusError, + Category: "Best Practices", + Scored: true, + Source: "Kyverno", + Resources: []corev1.ObjectReference{{ + APIVersion: "v1", + Kind: "Pod", + Name: "nginx", + Namespace: "test", + UID: "536ab69f-1b3c-4bd9-9ba4-274a56188419", + }}, + Properties: map[string]string{}, +} + var TrivyResult = v1alpha2.PolicyReportResult{ ID: "124", Message: "validation error", @@ -112,6 +150,23 @@ var FailPodResult = v1alpha2.PolicyReportResult{ }}, } +var SkipPodResult = v1alpha2.PolicyReportResult{ + ID: "124", + Policy: "require-requests-and-limits-required", + Rule: "autogen-check-for-requests-and-limits", + Result: v1alpha2.StatusSkip, + Category: "Best Practices", + Scored: true, + Source: "Kyverno", + Resources: []corev1.ObjectReference{{ + APIVersion: "v1", + Kind: "Pod", + Name: "nginx", + Namespace: "test", + UID: "536ab69f-1b3c-4bd9-9ba4-274a56188419", + }}, +} + var FailResultWithoutResource = v1alpha2.PolicyReportResult{ Message: "validation error: requests and limits required. Rule autogen-check-for-requests-and-limits failed at path /spec/template/spec/containers/0/resources/requests/", Policy: "require-requests-and-limits-required", diff --git a/pkg/helper/chunk_slice_test.go b/pkg/helper/chunk_slice_test.go new file mode 100644 index 000000000..b9ce93220 --- /dev/null +++ b/pkg/helper/chunk_slice_test.go @@ -0,0 +1,18 @@ +package helper_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/kyverno/policy-reporter/pkg/helper" +) + +func TestChunkSize(t *testing.T) { + chunks := helper.ChunkSlice([]int{1, 2, 3, 4, 5, 6, 7}, 3) + + assert.Len(t, chunks, 3) + assert.Equal(t, []int{1, 2, 3}, chunks[0]) + assert.Equal(t, []int{4, 5, 6}, chunks[1]) + assert.Equal(t, []int{7}, chunks[2]) +} diff --git a/pkg/helper/title_test.go b/pkg/helper/title_test.go new file mode 100644 index 000000000..d712c69b1 --- /dev/null +++ b/pkg/helper/title_test.go @@ -0,0 +1,14 @@ +package helper_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/kyverno/policy-reporter/pkg/helper" +) + +func TestTitle(t *testing.T) { + assert.Equal(t, "Kyverno", helper.Title("kyverno")) + assert.Equal(t, "Trivy Vulnerability", helper.Title("trivy vulnerability")) +} diff --git a/pkg/kubernetes/policy_report_client.go b/pkg/kubernetes/policy_report_client.go index 72a834e75..ae7877dc9 100644 --- a/pkg/kubernetes/policy_report_client.go +++ b/pkg/kubernetes/policy_report_client.go @@ -60,7 +60,7 @@ func (k *k8sPolicyReportClient) Sync(stopper chan struct{}) error { k.synced = true - zap.L().Info("informer sync completed") + zap.L().Info("policy report informer sync completed") return nil } diff --git a/pkg/listener/cleanup_test.go b/pkg/listener/cleanup_test.go index d2b7656c7..fcea13d22 100644 --- a/pkg/listener/cleanup_test.go +++ b/pkg/listener/cleanup_test.go @@ -20,3 +20,14 @@ func Test_CleanupListener(t *testing.T) { assert.True(t, c.cleanupCalled, "expected cleanup method was called") }) } + +func Test_Cleanup_Listener_Skip_Added(t *testing.T) { + t.Run("Execute Cleanup Handler", func(t *testing.T) { + c := &client{cleanup: true} + + slistener := listener.NewCleanupListener(ctx, target.NewCollection(&target.Target{Client: c})) + slistener(report.LifecycleEvent{Type: report.Added, PolicyReport: preport1}) + + assert.False(t, c.cleanupCalled, "expected cleanup execution was skipped") + }) +} diff --git a/pkg/listener/fixture_test.go b/pkg/listener/fixture_test.go index de27a7af0..3095925bf 100644 --- a/pkg/listener/fixture_test.go +++ b/pkg/listener/fixture_test.go @@ -1,12 +1,31 @@ package listener_test import ( + "time" + + corev1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/kyverno/policy-reporter/pkg/crd/api/policyreport/v1alpha2" "github.com/kyverno/policy-reporter/pkg/fixtures" ) +var scopereport1 = &v1alpha2.PolicyReport{ + ObjectMeta: v1.ObjectMeta{ + Name: "polr-test", + Namespace: "test", + CreationTimestamp: v1.NewTime(time.Now().Add(time.Hour)), + }, + Scope: &corev1.ObjectReference{ + APIVersion: "v1", + Kind: "Pod", + Name: "test", + Namespace: "test", + }, + Results: []v1alpha2.PolicyReportResult{fixtures.FailResult}, + Summary: v1alpha2.PolicyReportSummary{Fail: 1}, +} + var preport1 = &v1alpha2.PolicyReport{ ObjectMeta: v1.ObjectMeta{ Name: "polr-test", diff --git a/pkg/listener/new_result.go b/pkg/listener/new_result.go index 5bf685225..f0d58d684 100644 --- a/pkg/listener/new_result.go +++ b/pkg/listener/new_result.go @@ -12,12 +12,6 @@ import ( const NewResults = "new_results_listener" -func (l *ResultListener) ResetListeners() { - l.listener = make([]report.PolicyReportResultListener, 0) - l.scopeListener = make([]report.ScopeResultsListener, 0) - l.syncListener = make([]report.SyncResultsListener, 0) -} - type ResultListener struct { skipExisting bool listener []report.PolicyReportResultListener @@ -139,6 +133,8 @@ func (l *ResultListener) Listen(event report.LifecycleEvent) { callback(event.PolicyReport, results, preExisted) }(cb, newResults) } + + wg.Wait() } if len(l.listener) == 0 { diff --git a/pkg/listener/new_result_test.go b/pkg/listener/new_result_test.go index 73f8dee95..532e16f0a 100644 --- a/pkg/listener/new_result_test.go +++ b/pkg/listener/new_result_test.go @@ -125,4 +125,70 @@ func Test_ResultListener(t *testing.T) { assert.False(t, called, "Expected Listener not called because it was unregistered") }) + + t.Run("Publish Scoped Report", func(t *testing.T) { + var called []v1alpha2.PolicyReportResult + + slistener := listener.NewResultListener(true, cache.NewInMermoryCache(time.Minute, time.Minute), time.Now()) + slistener.RegisterScopeListener(func(_ v1alpha2.ReportInterface, r []v1alpha2.PolicyReportResult, b bool) { + called = r + }) + + slistener.Listen(report.LifecycleEvent{Type: report.Added, PolicyReport: scopereport1}) + + assert.Equal(t, called[0].GetID(), fixtures.FailResult.GetID(), "Expected Listener to be called") + }) + + t.Run("Unregister Scope Listener", func(t *testing.T) { + var called []v1alpha2.PolicyReportResult + + slistener := listener.NewResultListener(true, cache.NewInMermoryCache(time.Minute, time.Minute), time.Now()) + slistener.RegisterScopeListener(func(_ v1alpha2.ReportInterface, r []v1alpha2.PolicyReportResult, b bool) { + called = r + }) + + slistener.UnregisterScopeListener() + + slistener.Listen(report.LifecycleEvent{Type: report.Added, PolicyReport: scopereport1}) + + assert.Len(t, called, 0, "Expected listener was unregistered") + }) + + t.Run("Publish Scoped Report to Sync Target", func(t *testing.T) { + var called v1alpha2.ReportInterface + + slistener := listener.NewResultListener(true, cache.NewInMermoryCache(time.Minute, time.Minute), time.Now()) + slistener.RegisterSyncListener(func(r v1alpha2.ReportInterface) { + called = r + }) + + slistener.Listen(report.LifecycleEvent{Type: report.Added, PolicyReport: scopereport1}) + + assert.Equal(t, called.GetName(), scopereport1.Name, "Expected Listener to be called") + }) + + t.Run("Publish Scoped Report to Sync Target", func(t *testing.T) { + var called v1alpha2.ReportInterface + + slistener := listener.NewResultListener(true, cache.NewInMermoryCache(time.Minute, time.Minute), time.Now()) + slistener.RegisterSyncListener(func(r v1alpha2.ReportInterface) { + called = r + }) + + slistener.UnregisterSyncListener() + + slistener.Listen(report.LifecycleEvent{Type: report.Added, PolicyReport: scopereport1}) + + assert.Nil(t, called, "Expected Listener was unregistered") + }) + + t.Run("Check Validation Logic", func(t *testing.T) { + slistener := listener.NewResultListener(true, cache.NewInMermoryCache(time.Minute, time.Minute), time.Now()) + + assert.True(t, slistener.Validate(fixtures.FailPodResult)) + assert.True(t, slistener.Validate(fixtures.WarnPodResult)) + assert.True(t, slistener.Validate(fixtures.ErrorPodResult)) + assert.False(t, slistener.Validate(fixtures.PassPodResult)) + assert.False(t, slistener.Validate(fixtures.SkipPodResult)) + }) } diff --git a/pkg/listener/scope_results.go b/pkg/listener/scope_results.go index 5bf92a9f2..91c3712ea 100644 --- a/pkg/listener/scope_results.go +++ b/pkg/listener/scope_results.go @@ -14,6 +14,9 @@ const SendScopeResults = "send_scope_results_listener" func NewSendScopeResultsListener(targets *target.Collection) report.ScopeResultsListener { return func(rep v1alpha2.ReportInterface, r []v1alpha2.PolicyReportResult, e bool) { clients := targets.BatchSendClients() + if len(clients) == 0 { + return + } wg := &sync.WaitGroup{} wg.Add(len(clients)) diff --git a/pkg/listener/send_result.go b/pkg/listener/send_result.go index 367e91588..b7ef3dba4 100644 --- a/pkg/listener/send_result.go +++ b/pkg/listener/send_result.go @@ -15,6 +15,9 @@ const SendResults = "send_results_listener" func NewSendResultListener(targets *target.Collection) report.PolicyReportResultListener { return func(rep v1alpha2.ReportInterface, r v1alpha2.PolicyReportResult, e bool) { clients := targets.SingleSendClients() + if len(clients) == 0 { + return + } wg := &sync.WaitGroup{} wg.Add(len(clients)) diff --git a/pkg/report/source_filter_test.go b/pkg/report/source_filter_test.go new file mode 100644 index 000000000..05c005e4b --- /dev/null +++ b/pkg/report/source_filter_test.go @@ -0,0 +1,225 @@ +package report_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/kyverno/policy-reporter/pkg/crd/api/policyreport/v1alpha2" + "github.com/kyverno/policy-reporter/pkg/fixtures" + "github.com/kyverno/policy-reporter/pkg/report" + "github.com/kyverno/policy-reporter/pkg/validate" +) + +var controlled = true + +type podClient struct { + pod *corev1.Pod + err error +} + +func (c *podClient) Get(res *corev1.ObjectReference) (*corev1.Pod, error) { + return c.pod, c.err +} + +type jobClient struct { + job *batchv1.Job + err error +} + +func (c jobClient) Get(res *corev1.ObjectReference) (*batchv1.Job, error) { + return c.job, c.err +} + +func TestSourceFilter(t *testing.T) { + t.Run("include by namespace succeed", func(t *testing.T) { + filter := report.NewSourceFilter(nil, nil, []report.SourceValidation{ + { + Selector: report.ReportSelector{ + Source: "kyverno", + }, + Namespaces: validate.RuleSets{ + Include: []string{"test"}, + }, + }, + }) + + result := filter.Validate(&v1alpha2.PolicyReport{ + ObjectMeta: v1.ObjectMeta{Name: "polr", Namespace: "test"}, + Scope: &corev1.ObjectReference{APIVersion: "v1", Kind: "Pod", Name: "nginx", Namespace: "test"}, + Results: []v1alpha2.PolicyReportResult{fixtures.FailPodResult}, + }) + + assert.True(t, result) + }) + + t.Run("include by namespace fails", func(t *testing.T) { + filter := report.NewSourceFilter(nil, nil, []report.SourceValidation{ + { + Selector: report.ReportSelector{ + Source: "kyverno", + }, + Namespaces: validate.RuleSets{ + Include: []string{"default"}, + }, + }, + }) + + result := filter.Validate(&v1alpha2.PolicyReport{ + ObjectMeta: v1.ObjectMeta{Name: "polr", Namespace: "test"}, + Scope: &corev1.ObjectReference{APIVersion: "v1", Kind: "Pod", Name: "nginx", Namespace: "test"}, + Results: []v1alpha2.PolicyReportResult{fixtures.FailPodResult}, + }) + + assert.False(t, result) + }) + + t.Run("include by kind succeed", func(t *testing.T) { + filter := report.NewSourceFilter(nil, nil, []report.SourceValidation{ + { + Selector: report.ReportSelector{ + Source: "kyverno", + }, + Kinds: validate.RuleSets{ + Include: []string{"Pod"}, + }, + }, + }) + + result := filter.Validate(&v1alpha2.PolicyReport{ + ObjectMeta: v1.ObjectMeta{Name: "polr", Namespace: "test"}, + Scope: &corev1.ObjectReference{APIVersion: "v1", Kind: "Pod", Name: "nginx", Namespace: "test"}, + Results: []v1alpha2.PolicyReportResult{fixtures.FailPodResult}, + }) + + assert.True(t, result) + }) + + t.Run("include by kind fails", func(t *testing.T) { + filter := report.NewSourceFilter(nil, nil, []report.SourceValidation{ + { + Selector: report.ReportSelector{ + Source: "kyverno", + }, + Kinds: validate.RuleSets{ + Include: []string{"Job"}, + }, + }, + }) + + result := filter.Validate(&v1alpha2.PolicyReport{ + ObjectMeta: v1.ObjectMeta{Name: "polr", Namespace: "test"}, + Scope: &corev1.ObjectReference{APIVersion: "v1", Kind: "Pod", Name: "nginx", Namespace: "test"}, + Results: []v1alpha2.PolicyReportResult{fixtures.FailPodResult}, + }) + + assert.False(t, result) + }) + + t.Run("disable cluster reports", func(t *testing.T) { + filter := report.NewSourceFilter(nil, nil, []report.SourceValidation{ + { + Selector: report.ReportSelector{ + Source: "kyverno", + }, + DisableClusterReports: true, + }, + }) + + assert.False(t, filter.Validate(&v1alpha2.ClusterPolicyReport{ + ObjectMeta: v1.ObjectMeta{Name: "polr", Namespace: ""}, + Results: []v1alpha2.PolicyReportResult{fixtures.FailNamespaceResult}, + })) + + assert.True(t, filter.Validate(&v1alpha2.PolicyReport{ + ObjectMeta: v1.ObjectMeta{Name: "polr", Namespace: "test"}, + Results: []v1alpha2.PolicyReportResult{fixtures.FailNamespaceResult}, + })) + }) + + t.Run("include by kind succeed", func(t *testing.T) { + filter := report.NewSourceFilter(nil, nil, []report.SourceValidation{ + { + Selector: report.ReportSelector{ + Source: "kyverno", + }, + Kinds: validate.RuleSets{ + Include: []string{"Pod"}, + }, + }, + }) + + result := filter.Validate(&v1alpha2.PolicyReport{ + ObjectMeta: v1.ObjectMeta{Name: "polr", Namespace: "test"}, + Scope: &corev1.ObjectReference{APIVersion: "v1", Kind: "Pod", Name: "nginx", Namespace: "test"}, + Results: []v1alpha2.PolicyReportResult{fixtures.FailPodResult}, + }) + + assert.True(t, result) + }) + + t.Run("filter controlled pod", func(t *testing.T) { + c := podClient{ + pod: &corev1.Pod{ObjectMeta: v1.ObjectMeta{Name: "nginx", Namespace: "test", OwnerReferences: []v1.OwnerReference{ + {APIVersion: "apps/v1", Kind: "ReplicaSet", Name: "nginx-rs", Controller: &controlled}, + }}}, + } + + filter := report.NewSourceFilter(&c, nil, []report.SourceValidation{ + { + Selector: report.ReportSelector{ + Source: "kyverno", + }, + UncontrolledOnly: true, + }, + }) + + assert.False(t, filter.Validate(&v1alpha2.PolicyReport{ + ObjectMeta: v1.ObjectMeta{Name: "polr", Namespace: "test"}, + Scope: &corev1.ObjectReference{APIVersion: "v1", Kind: "Pod", Name: "nginx", Namespace: "test"}, + Results: []v1alpha2.PolicyReportResult{fixtures.FailPodResult}, + })) + + c.pod = &corev1.Pod{ObjectMeta: v1.ObjectMeta{Name: "nginx", Namespace: "test"}} + + assert.True(t, filter.Validate(&v1alpha2.PolicyReport{ + ObjectMeta: v1.ObjectMeta{Name: "polr", Namespace: "test"}, + Scope: &corev1.ObjectReference{APIVersion: "v1", Kind: "Pod", Name: "nginx", Namespace: "test"}, + Results: []v1alpha2.PolicyReportResult{fixtures.FailPodResult}, + })) + }) + + t.Run("filter controlled job", func(t *testing.T) { + c := jobClient{ + job: &batchv1.Job{ObjectMeta: v1.ObjectMeta{Name: "nginx", Namespace: "test", OwnerReferences: []v1.OwnerReference{ + {APIVersion: "batch/v1", Kind: "CronJob", Name: "nginx-rs", Controller: &controlled}, + }}}, + } + + filter := report.NewSourceFilter(nil, &c, []report.SourceValidation{ + { + Selector: report.ReportSelector{ + Source: "kyverno", + }, + UncontrolledOnly: true, + }, + }) + + assert.False(t, filter.Validate(&v1alpha2.PolicyReport{ + ObjectMeta: v1.ObjectMeta{Name: "polr", Namespace: "test"}, + Scope: &corev1.ObjectReference{APIVersion: "v1", Kind: "Job", Name: "nginx", Namespace: "test"}, + Results: []v1alpha2.PolicyReportResult{fixtures.FailPodResult}, + })) + + c.job = &batchv1.Job{ObjectMeta: v1.ObjectMeta{Name: "nginx", Namespace: "test"}} + + assert.True(t, filter.Validate(&v1alpha2.PolicyReport{ + ObjectMeta: v1.ObjectMeta{Name: "polr", Namespace: "test"}, + Scope: &corev1.ObjectReference{APIVersion: "v1", Kind: "Job", Name: "nginx", Namespace: "test"}, + Results: []v1alpha2.PolicyReportResult{fixtures.FailPodResult}, + })) + }) +} diff --git a/pkg/targetconfig/client.go b/pkg/targetconfig/client.go new file mode 100644 index 000000000..b5ebaa01f --- /dev/null +++ b/pkg/targetconfig/client.go @@ -0,0 +1,77 @@ +package targetconfig + +import ( + "fmt" + + "go.uber.org/zap" + "k8s.io/client-go/tools/cache" + + "github.com/kyverno/policy-reporter/pkg/crd/api/targetconfig/v1alpha1" + tcv1alpha1 "github.com/kyverno/policy-reporter/pkg/crd/client/targetconfig/clientset/versioned" + tcinformer "github.com/kyverno/policy-reporter/pkg/crd/client/targetconfig/informers/externalversions" + "github.com/kyverno/policy-reporter/pkg/target" +) + +type Client struct { + targetFactory target.Factory + collection *target.Collection + logger *zap.Logger + informer cache.SharedIndexInformer +} + +func (c *Client) ConfigureInformer() { + c.informer.AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { + tc := obj.(*v1alpha1.TargetConfig) + c.logger.Info(fmt.Sprintf("new target: %s", tc.Name)) + + t, err := c.targetFactory.CreateSingleClient(tc) + if err != nil { + c.logger.Error("unable to create target from TargetConfig: " + err.Error()) + return + } + + c.collection.AddTarget(tc.Name, t) + }, + UpdateFunc: func(oldObj, newObj interface{}) { + tc := newObj.(*v1alpha1.TargetConfig) + c.logger.Info(fmt.Sprintf("update target: %s", tc.Name)) + + t, err := c.targetFactory.CreateSingleClient(tc) + if err != nil { + c.logger.Error("unable to create target from TargetConfig: " + err.Error()) + return + } + + c.collection.AddTarget(tc.Name, t) + }, + DeleteFunc: func(obj interface{}) { + tc := obj.(*v1alpha1.TargetConfig) + c.logger.Info(fmt.Sprintf("deleting target: %s", tc.Name)) + + c.collection.RemoveTarget(tc.Name) + }, + }) +} + +func (c *Client) Run(stopChan chan struct{}) { + go c.informer.Run(stopChan) + + if !cache.WaitForCacheSync(stopChan, c.informer.HasSynced) { + c.logger.Error("Failed to sync target config cache") + return + } + + c.logger.Info("target config cache synced") +} + +func NewClient(tcClient tcv1alpha1.Interface, f target.Factory, targets *target.Collection, logger *zap.Logger) *Client { + tcInformer := tcinformer.NewSharedInformerFactory(tcClient, 0) + + return &Client{ + informer: tcInformer.Policyreporter().V1alpha1().TargetConfigs().Informer(), + targetFactory: f, + collection: targets, + logger: logger, + } +} diff --git a/pkg/targetconfig/client_test.go b/pkg/targetconfig/client_test.go new file mode 100644 index 000000000..e1d08fa9b --- /dev/null +++ b/pkg/targetconfig/client_test.go @@ -0,0 +1,197 @@ +package targetconfig_test + +import ( + "context" + "reflect" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "go.uber.org/zap" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + corefake "k8s.io/client-go/kubernetes/fake" + v1 "k8s.io/client-go/kubernetes/typed/core/v1" + + "github.com/kyverno/policy-reporter/pkg/crd/api/targetconfig/v1alpha1" + "github.com/kyverno/policy-reporter/pkg/crd/client/targetconfig/clientset/versioned/fake" + tcv1alpha1 "github.com/kyverno/policy-reporter/pkg/crd/client/targetconfig/clientset/versioned/typed/targetconfig/v1alpha1" + "github.com/kyverno/policy-reporter/pkg/kubernetes/secrets" + "github.com/kyverno/policy-reporter/pkg/target" + "github.com/kyverno/policy-reporter/pkg/target/factory" + "github.com/kyverno/policy-reporter/pkg/targetconfig" +) + +const secretName = "secret-values" + +func newSecretClient() v1.SecretInterface { + return corefake.NewSimpleClientset(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: "default", + }, + Data: map[string][]byte{ + "host": []byte("http://localhost:9200"), + "username": []byte("username"), + "password": []byte("password"), + "apiKey": []byte("apiKey"), + "webhook": []byte("http://localhost:9200/webhook"), + "accessKeyId": []byte("accessKeyId"), + "secretAccessKey": []byte("secretAccessKey"), + "kmsKeyId": []byte("kmsKeyId"), + "token": []byte("token"), + "accountId": []byte("accountId"), + "database": []byte("database"), + "dsn": []byte("dsn"), + "typelessApi": []byte("false"), + }, + }).CoreV1().Secrets("default") +} + +func NewFakeClient() (*fake.Clientset, tcv1alpha1.TargetConfigInterface) { + client := fake.NewSimpleClientset() + + return client, client.PolicyreporterV1alpha1().TargetConfigs("") +} + +func Test_TargetConfig_TargetCreation(t *testing.T) { + ctx := context.Background() + stop := make(chan struct{}) + + defer close(stop) + + kclient, tclient := NewFakeClient() + collection := target.NewCollection() + factory := factory.NewFactory(secrets.NewClient(newSecretClient()), target.NewResultFilterFactory(nil)) + + client := targetconfig.NewClient(kclient, factory, collection, zap.L()) + client.ConfigureInformer() + + go func() { + client.Run(stop) + }() + + tclient.Create(ctx, &v1alpha1.TargetConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: v1alpha1.TargetConfigSpec{ + Slack: &v1alpha1.SlackOptions{ + WebhookOptions: v1alpha1.WebhookOptions{ + Webhook: "http://localhost:8080", + }, + }, + }, + }, metav1.CreateOptions{}) + + time.Sleep(10 * time.Millisecond) + + assert.Len(t, collection.Targets(), 1) + + target := reflect.ValueOf(collection.Client("test")).Elem() + + assert.NotNil(t, target) + + webhook := target.FieldByName("webhook").String() + assert.Equal(t, "http://localhost:8080", webhook) +} + +func Test_TargetConfig_TargetUpdates(t *testing.T) { + ctx := context.Background() + stop := make(chan struct{}) + + defer close(stop) + + kclient, tclient := NewFakeClient() + collection := target.NewCollection() + factory := factory.NewFactory(secrets.NewClient(newSecretClient()), target.NewResultFilterFactory(nil)) + + client := targetconfig.NewClient(kclient, factory, collection, zap.L()) + client.ConfigureInformer() + + go func() { + client.Run(stop) + }() + + tclient.Create(ctx, &v1alpha1.TargetConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: v1alpha1.TargetConfigSpec{ + Slack: &v1alpha1.SlackOptions{ + WebhookOptions: v1alpha1.WebhookOptions{ + Webhook: "http://localhost:8080", + }, + }, + }, + }, metav1.CreateOptions{}) + + time.Sleep(10 * time.Millisecond) + + tclient.Update(ctx, &v1alpha1.TargetConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: v1alpha1.TargetConfigSpec{ + Slack: &v1alpha1.SlackOptions{ + WebhookOptions: v1alpha1.WebhookOptions{ + Webhook: "http://localhost:9090", + }, + }, + }, + }, metav1.UpdateOptions{}) + + time.Sleep(10 * time.Millisecond) + + assert.Len(t, collection.Targets(), 1) + + target := reflect.ValueOf(collection.Client("test")).Elem() + + assert.NotNil(t, target) + + webhook := target.FieldByName("webhook").String() + assert.Equal(t, "http://localhost:9090", webhook) +} + +func Test_TargetConfig_TargetDeletion(t *testing.T) { + ctx := context.Background() + stop := make(chan struct{}) + + defer close(stop) + + kclient, tclient := NewFakeClient() + collection := target.NewCollection() + factory := factory.NewFactory(secrets.NewClient(newSecretClient()), target.NewResultFilterFactory(nil)) + + client := targetconfig.NewClient(kclient, factory, collection, zap.L()) + client.ConfigureInformer() + + go func() { + client.Run(stop) + }() + + tclient.Create(ctx, &v1alpha1.TargetConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: v1alpha1.TargetConfigSpec{ + Slack: &v1alpha1.SlackOptions{ + WebhookOptions: v1alpha1.WebhookOptions{ + Webhook: "http://localhost:8080", + }, + }, + }, + }, metav1.CreateOptions{}) + + time.Sleep(10 * time.Millisecond) + + tclient.Delete(ctx, "test", metav1.DeleteOptions{}) + + time.Sleep(10 * time.Millisecond) + + assert.Len(t, collection.Targets(), 0) + + target := collection.Client("test") + + assert.Nil(t, target) +} diff --git a/pkg/targetconfig/tc.go b/pkg/targetconfig/tc.go deleted file mode 100644 index 44c727bf4..000000000 --- a/pkg/targetconfig/tc.go +++ /dev/null @@ -1,120 +0,0 @@ -package targetconfig - -import ( - "context" - "fmt" - - "go.uber.org/zap" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/tools/cache" - - "github.com/kyverno/policy-reporter/pkg/crd/api/targetconfig/v1alpha1" - tcv1alpha1 "github.com/kyverno/policy-reporter/pkg/crd/client/targetconfig/clientset/versioned" - tcinformer "github.com/kyverno/policy-reporter/pkg/crd/client/targetconfig/informers/externalversions" - "github.com/kyverno/policy-reporter/pkg/target" -) - -type TargetConfigClient struct { - tcClient tcv1alpha1.Interface - targetFactory target.Factory - targetClients *target.Collection - logger *zap.Logger - informer cache.SharedIndexInformer - tcCount int - hasSynced bool -} - -type EventType string - -const ( - DeleteTcEvent = "delete" - CreateTcEvent = "create" -) - -type TcEvent struct { - Type EventType - Targets *target.Collection - RestartPolrInformer bool -} - -func (c *TargetConfigClient) TargetConfigCount() int { - return c.tcCount -} - -func (c *TargetConfigClient) configureInformer(targetChan chan TcEvent) { - c.informer.AddEventHandler(cache.ResourceEventHandlerFuncs{ - AddFunc: func(obj interface{}) { - tc := obj.(*v1alpha1.TargetConfig) - c.logger.Info(fmt.Sprintf("new target: %s", tc.Name)) - - t, err := c.targetFactory.CreateSingleClient(tc) - if err != nil { - c.logger.Error("unable to create target from TargetConfig: " + err.Error()) - return - } - - c.targetClients.AddTarget(tc.Name, t) - targetChan <- TcEvent{Type: CreateTcEvent, Targets: c.targetClients} - }, - UpdateFunc: func(oldObj, newObj interface{}) { - tc := newObj.(*v1alpha1.TargetConfig) - c.logger.Info(fmt.Sprintf("update target: %s", tc.Name)) - - t, err := c.targetFactory.CreateSingleClient(tc) - if err != nil { - c.logger.Error("unable to create target from TargetConfig: " + err.Error()) - return - } - - c.targetClients.AddTarget(tc.Name, t) - targetChan <- TcEvent{Type: CreateTcEvent, Targets: c.targetClients} - }, - DeleteFunc: func(obj interface{}) { - tc := obj.(*v1alpha1.TargetConfig) - c.logger.Info(fmt.Sprintf("deleting target: %s", tc.Name)) - - c.targetClients.RemoveTarget(tc.Name) - targetChan <- TcEvent{Type: DeleteTcEvent, Targets: c.targetClients} - }, - }) -} - -func (c *TargetConfigClient) CreateInformer(targetChan chan TcEvent) error { - tcInformer := tcinformer.NewSharedInformerFactory(c.tcClient, 0) - inf := tcInformer.Policyreporter().V1alpha1().TargetConfigs().Informer() - c.informer = inf - - tcs, err := c.tcClient.PolicyreporterV1alpha1().TargetConfigs("").List(context.TODO(), metav1.ListOptions{}) - if err != nil { - return err - } - - c.tcCount = len(tcs.Items) - c.configureInformer(targetChan) - return nil -} - -func (c *TargetConfigClient) HasSynced() bool { - return c.hasSynced -} - -func (c *TargetConfigClient) Run(stopChan chan struct{}) { - go c.informer.Run(stopChan) - - if !cache.WaitForCacheSync(stopChan, c.informer.HasSynced) { - c.logger.Error("Failed to sync target config cache") - return - } - - c.hasSynced = true - c.logger.Info("target config cache synced") -} - -func NewTargetConfigClient(tcClient tcv1alpha1.Interface, f target.Factory, targets *target.Collection, logger *zap.Logger) *TargetConfigClient { - return &TargetConfigClient{ - tcClient: tcClient, - targetFactory: f, - targetClients: targets, - logger: logger, - } -} diff --git a/pkg/validate/model_test.go b/pkg/validate/model_test.go new file mode 100644 index 000000000..eec4ad723 --- /dev/null +++ b/pkg/validate/model_test.go @@ -0,0 +1,29 @@ +package validate_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/kyverno/policy-reporter/pkg/validate" +) + +func TestCount(t *testing.T) { + t.Run("count include rules", func(t *testing.T) { + assert.Equal(t, 0, validate.RuleSets{}.Count()) + assert.Equal(t, 2, validate.RuleSets{Include: []string{"kyverno", "falco"}}.Count()) + }) + t.Run("count exclude rules", func(t *testing.T) { + assert.Equal(t, 2, validate.RuleSets{Exclude: []string{"kyverno", "falco"}}.Count()) + }) +} + +func TestEnabled(t *testing.T) { + t.Run("enabled when include rule exist", func(t *testing.T) { + assert.False(t, validate.RuleSets{}.Enabled()) + assert.True(t, validate.RuleSets{Include: []string{"kyverno"}}.Enabled()) + }) + t.Run("enabled when exclude rule exist", func(t *testing.T) { + assert.True(t, validate.RuleSets{Exclude: []string{"kyverno", "falco"}}.Enabled()) + }) +}