From ab5545482eca39a210151cee9251642b2e846608 Mon Sep 17 00:00:00 2001 From: Joseph Lombrozo Date: Thu, 25 Jan 2024 14:55:00 -0500 Subject: [PATCH] skip apps that are being deleted (#130) we don't know if they've already been parsed, so we mark them as deleted, then skip them when building the results. --- pkg/diff/diff.go | 5 ++++- pkg/events/check.go | 10 +++++++--- pkg/message.go | 47 +++++++++++++++++++++++++++++++++++++++------ pkg/message_test.go | 3 ++- 4 files changed, 54 insertions(+), 11 deletions(-) diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index 57fc87c5..50592afb 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -48,7 +48,7 @@ changedFilePath should be the root of the changed folder from https://github.com/argoproj/argo-cd/blob/d3ff9757c460ae1a6a11e1231251b5d27aadcdd1/cmd/argocd/commands/app.go#L879 */ -func GetDiff(ctx context.Context, manifests []string, app argoappv1.Application, addApp func(argoappv1.Application)) (pkg.CheckResult, string, error) { +func GetDiff(ctx context.Context, manifests []string, app argoappv1.Application, addApp, removeApp func(application2 argoappv1.Application)) (pkg.CheckResult, string, error) { ctx, span := otel.Tracer("Kubechecks").Start(ctx, "GetDiff") defer span.End() @@ -164,6 +164,9 @@ func GetDiff(ctx context.Context, manifests []string, app argoappv1.Application, switch { case item.target == nil: removed++ + if app, ok := isApp(item, diffRes.NormalizedLive); ok { + removeApp(app) + } case item.live == nil: added++ if app, ok := isApp(item, diffRes.PredictedLive); ok { diff --git a/pkg/events/check.go b/pkg/events/check.go index 18ed1ec0..d829e7e3 100644 --- a/pkg/events/check.go +++ b/pkg/events/check.go @@ -245,6 +245,10 @@ func (ce *CheckEvent) ProcessApps(ctx context.Context) { ce.CommitStatus(ctx, worstStatus) } +func (ce *CheckEvent) removeApp(app v1alpha1.Application) { + ce.vcsNote.RemoveApp(app.Name) +} + func (ce *CheckEvent) queueApp(app v1alpha1.Application) { name := app.Name dir := app.Spec.GetSource().Path @@ -345,7 +349,7 @@ func (ce *CheckEvent) processApp(ctx context.Context, app v1alpha1.Application) run := ce.createRunner(span, ctx, appName, &wg) run("validating app against schema", ce.validateSchemas(ctx, appName, k8sVersion, ce.TempWorkingDir, formattedManifests)) - run("generating diff for app", ce.generateDiff(ctx, app, manifests, ce.queueApp)) + run("generating diff for app", ce.generateDiff(ctx, app, manifests, ce.queueApp, ce.removeApp)) if viper.GetBool("enable-conftest") { run("validation policy", ce.validatePolicy(ctx, appName)) @@ -438,9 +442,9 @@ func (ce *CheckEvent) validatePolicy(ctx context.Context, app string) func() (pk } } -func (ce *CheckEvent) generateDiff(ctx context.Context, app v1alpha1.Application, manifests []string, addApp func(app v1alpha1.Application)) func() (pkg.CheckResult, error) { +func (ce *CheckEvent) generateDiff(ctx context.Context, app v1alpha1.Application, manifests []string, addApp, removeApp func(app v1alpha1.Application)) func() (pkg.CheckResult, error) { return func() (pkg.CheckResult, error) { - cr, rawDiff, err := diff.GetDiff(ctx, manifests, app, addApp) + cr, rawDiff, err := diff.GetDiff(ctx, manifests, app, addApp, removeApp) if err != nil { return pkg.CheckResult{}, err } diff --git a/pkg/message.go b/pkg/message.go index 6d1cc676..07d4743b 100644 --- a/pkg/message.go +++ b/pkg/message.go @@ -32,8 +32,10 @@ func NewMessage(name string, prId, commentId int, vcs toEmoji) *Message { Name: name, CheckID: prId, NoteID: commentId, - apps: make(map[string]*AppResults), vcs: vcs, + + apps: make(map[string]*AppResults), + deletedAppsSet: make(map[string]struct{}), } } @@ -55,12 +57,18 @@ type Message struct { footer string lock sync.Mutex vcs toEmoji + + deletedAppsSet map[string]struct{} } func (m *Message) WorstState() CommitState { state := StateNone - for _, r := range m.apps { + for app, r := range m.apps { + if m.isDeleted(app) { + continue + } + for _, result := range r.results { state = WorstState(state, result.State) } @@ -69,7 +77,26 @@ func (m *Message) WorstState() CommitState { return state } +func (m *Message) RemoveApp(app string) { + m.lock.Lock() + defer m.lock.Unlock() + + m.deletedAppsSet[app] = struct{}{} +} + +func (m *Message) isDeleted(app string) bool { + if _, ok := m.deletedAppsSet[app]; ok { + return true + } + + return false +} + func (m *Message) AddNewApp(ctx context.Context, app string) { + if m.isDeleted(app) { + return + } + _, span := otel.Tracer("Kubechecks").Start(ctx, "AddNewApp") defer span.End() m.lock.Lock() @@ -79,6 +106,10 @@ func (m *Message) AddNewApp(ctx context.Context, app string) { } func (m *Message) AddToAppMessage(ctx context.Context, app string, result CheckResult) { + if m.isDeleted(app) { + return + } + _, span := otel.Tracer("Kubechecks").Start(ctx, "AddToAppMessage") defer span.End() m.lock.Lock() @@ -98,7 +129,7 @@ func (m *Message) SetFooter(start time.Time, commitSha string) { } func (m *Message) BuildComment(ctx context.Context) string { - return m.buildComment(ctx, m.apps) + return m.buildComment(ctx) } func buildFooter(start time.Time, commitSHA string) string { @@ -118,18 +149,22 @@ func buildFooter(start time.Time, commitSHA string) string { } // Iterate the map of all apps in this message, building a final comment from their current state -func (m *Message) buildComment(ctx context.Context, apps map[string]*AppResults) string { +func (m *Message) buildComment(ctx context.Context) string { _, span := otel.Tracer("Kubechecks").Start(ctx, "buildComment") defer span.End() - names := getSortedKeys(apps) + names := getSortedKeys(m.apps) var sb strings.Builder sb.WriteString("# Kubechecks Report\n") for _, appName := range names { + if m.isDeleted(appName) { + continue + } + var checkStrings []string - results := apps[appName] + results := m.apps[appName] appState := StateSuccess for _, check := range results.results { diff --git a/pkg/message_test.go b/pkg/message_test.go index 13c638cc..f86f6ea3 100644 --- a/pkg/message_test.go +++ b/pkg/message_test.go @@ -27,7 +27,8 @@ func TestBuildComment(t *testing.T) { }, } m := NewMessage("message", 1, 2, fakeEmojiable{":test:"}) - comment := m.buildComment(context.TODO(), appResults) + m.apps = appResults + comment := m.buildComment(context.TODO()) assert.Equal(t, `# Kubechecks Report