Skip to content

Commit 4848240

Browse files
authored
fix: Handle target errors gracefully and prevent cron from getting stuck (#777)
* fix(target): properly propagate errors from LoadImages and Remove methods Changes the Target interface to return errors from LoadImages() and Remove() methods, preventing panics when operations fail. Updates all implementations (dtrack, git, oci, configmap) and call sites to properly handle and log errors. Fixes panic in dtrack target when client initialization fails due to connection issues (internal/target/dtrack/dtrack_target.go:240). * fix(daemon): prevent cron service from getting stuck after panic Moves the running flag from global variable to CronService struct field protected by sync.Mutex. Uses defer to ensure the flag is always reset even when panics occur in runBackgroundService, preventing the cron routine from staying stuck in running state and never executing again.
1 parent d2a517f commit 4848240

File tree

7 files changed

+77
-29
lines changed

7 files changed

+77
-29
lines changed

internal/daemon/daemon.go

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package daemon
22

33
import (
4+
"sync"
45
"time"
56

67
"github.com/ckotzbauer/libstandard"
@@ -15,10 +16,10 @@ import (
1516
type CronService struct {
1617
cron string
1718
processor *processor.Processor
19+
mu sync.Mutex
20+
running bool
1821
}
1922

20-
var running = false
21-
2223
func Start(cronTime string, appVersion string) {
2324
cr := libstandard.Unescape(cronTime)
2425
logrus.Debugf("Cron set to: %v", cr)
@@ -50,11 +51,23 @@ func (c *CronService) printNextExecution() {
5051
}
5152

5253
func (c *CronService) runBackgroundService() {
53-
if running {
54+
// Atomically check and set running flag
55+
c.mu.Lock()
56+
if c.running {
57+
c.mu.Unlock()
58+
logrus.Info("Background-service already running, skipping execution")
5459
return
5560
}
61+
c.running = true
62+
c.mu.Unlock()
63+
64+
// Ensure running flag is reset even on panic
65+
defer func() {
66+
c.mu.Lock()
67+
c.running = false
68+
c.mu.Unlock()
69+
}()
5670

57-
running = true
5871
logrus.Info("Execute background-service")
5972

6073
if !processor.HasJobImage() {
@@ -64,15 +77,17 @@ func (c *CronService) runBackgroundService() {
6477
logrus.WithError(err).Fatal("Target could not be initialized,")
6578
}
6679

67-
t.LoadImages()
80+
_, err = t.LoadImages()
81+
if err != nil {
82+
logrus.WithError(err).Error("Failed to load images from target")
83+
}
6884
}
6985
}
7086

7187
namespaceSelector := internal.OperatorConfig.NamespaceLabelSelector
7288
namespaces, err := c.processor.K8s.Client.ListNamespaces(namespaceSelector)
7389
if err != nil {
7490
logrus.WithError(err).Errorf("failed to list namespaces with selector: %s, abort background-service", namespaceSelector)
75-
running = false
7691
return
7792
}
7893

@@ -81,5 +96,4 @@ func (c *CronService) runBackgroundService() {
8196
c.processor.ProcessAllPods(pods, allImages)
8297

8398
c.printNextExecution()
84-
running = false
8599
}

internal/processor/processor.go

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,12 @@ func (p *Processor) executeSyftScans(pods []libk8s.PodInfo, allImages []*liboci.
182182
}
183183

184184
for _, t := range p.Targets {
185-
targetImages := t.LoadImages()
185+
targetImages, err := t.LoadImages()
186+
if err != nil {
187+
logrus.WithError(err).Error("Failed to load images from target")
188+
continue
189+
}
190+
186191
removableImages := make([]*liboci.RegistryImage, 0)
187192
for _, t := range targetImages {
188193
if !containsImage(allImages, t.ImageID) {
@@ -193,7 +198,10 @@ func (p *Processor) executeSyftScans(pods []libk8s.PodInfo, allImages []*liboci.
193198
}
194199

195200
if len(removableImages) > 0 && internal.OperatorConfig.DeleteOrphanImages {
196-
t.Remove(removableImages)
201+
err := t.Remove(removableImages)
202+
if err != nil {
203+
logrus.WithError(err).Error("Failed to remove images from target")
204+
}
197205
}
198206
}
199207
}
@@ -295,7 +303,10 @@ func (p *Processor) cleanupImagesIfNeeded(removedContainers []*libk8s.ContainerI
295303
if len(images) > 0 {
296304
for _, t := range p.Targets {
297305
if internal.OperatorConfig.DeleteOrphanImages {
298-
t.Remove(images)
306+
err := t.Remove(images)
307+
if err != nil {
308+
logrus.WithError(err).Error("Failed to remove images from target")
309+
}
299310
}
300311
}
301312
}
@@ -347,7 +358,12 @@ func (p *Processor) runInformerAsync(informer cache.SharedIndexInformer) {
347358
allImages := make([]*liboci.RegistryImage, 0)
348359

349360
for _, t := range p.Targets {
350-
targetImages := t.LoadImages()
361+
targetImages, err := t.LoadImages()
362+
if err != nil {
363+
logrus.WithError(err).Error("Failed to load images from target")
364+
continue
365+
}
366+
351367
for _, po := range pods {
352368
pod := po.(*corev1.Pod)
353369
info := p.K8s.Client.ExtractPodInfos(*pod)

internal/target/configmap/configmap_target.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,11 @@ func (g *ConfigMapTarget) ProcessSbom(ctx *target.TargetContext) error {
4747
return err
4848
}
4949

50-
func (g *ConfigMapTarget) LoadImages() []*libk8s.RegistryImage {
50+
func (g *ConfigMapTarget) LoadImages() ([]*libk8s.RegistryImage, error) {
5151
configMaps, err := g.k8s.ListConfigMaps()
5252
if err != nil {
5353
logrus.WithError(err).Error("Could not load configmaps.")
54-
return []*libk8s.RegistryImage{}
54+
return nil, err
5555
}
5656

5757
images := make([]*libk8s.RegistryImage, 0)
@@ -63,13 +63,14 @@ func (g *ConfigMapTarget) LoadImages() []*libk8s.RegistryImage {
6363
}
6464
}
6565

66-
return images
66+
return images, nil
6767
}
6868

69-
func (g *ConfigMapTarget) Remove(allImages []*libk8s.RegistryImage) {
69+
func (g *ConfigMapTarget) Remove(allImages []*libk8s.RegistryImage) error {
7070
configMaps, err := g.k8s.ListConfigMaps()
7171
if err != nil {
7272
logrus.WithError(err).Error("failed to load configmaps")
73+
return err
7374
}
7475

7576
for _, i := range allImages {
@@ -82,4 +83,6 @@ func (g *ConfigMapTarget) Remove(allImages []*libk8s.RegistryImage) {
8283
}
8384
}
8485
}
86+
87+
return nil
8588
}

internal/target/dtrack/dtrack_target.go

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -228,17 +228,21 @@ func (g *DependencyTrackTarget) ProcessSbom(ctx *target.TargetContext) error {
228228

229229
if g.imageProjectMap == nil {
230230
// prepropulate imageProjectMap
231-
g.LoadImages()
231+
_, err := g.LoadImages()
232+
if err != nil {
233+
return err
234+
}
232235
}
233236

234237
g.imageProjectMap[ctx.Image.ImageID] = project.UUID
235238
return nil
236239
}
237240

238-
func (g *DependencyTrackTarget) LoadImages() []*libk8s.RegistryImage {
241+
func (g *DependencyTrackTarget) LoadImages() ([]*libk8s.RegistryImage, error) {
239242
client, err := dtrack.NewClient(g.baseUrl, g.clientOptions...)
240243
if err != nil {
241244
logrus.WithError(err).Errorf("failed to init dtrack client")
245+
return nil, err
242246
}
243247

244248
if g.imageProjectMap == nil {
@@ -258,6 +262,7 @@ func (g *DependencyTrackTarget) LoadImages() []*libk8s.RegistryImage {
258262
})
259263
if err != nil {
260264
logrus.Errorf("Could not load projects: %v", err)
265+
return nil, err
261266
}
262267

263268
var imageId string
@@ -295,18 +300,22 @@ func (g *DependencyTrackTarget) LoadImages() []*libk8s.RegistryImage {
295300
pageNumber++
296301
}
297302

298-
return images
303+
return images, nil
299304
}
300305

301-
func (g *DependencyTrackTarget) Remove(images []*libk8s.RegistryImage) {
306+
func (g *DependencyTrackTarget) Remove(images []*libk8s.RegistryImage) error {
302307
if g.imageProjectMap == nil {
303308
// prepropulate imageProjectMap
304-
g.LoadImages()
309+
_, err := g.LoadImages()
310+
if err != nil {
311+
return err
312+
}
305313
}
306314

307315
client, err := dtrack.NewClient(g.baseUrl, g.clientOptions...)
308316
if err != nil {
309317
logrus.WithError(err).Errorf("failed to init dtrack client")
318+
return err
310319
}
311320

312321
for _, img := range images {
@@ -356,6 +365,8 @@ func (g *DependencyTrackTarget) Remove(images []*libk8s.RegistryImage) {
356365
}
357366
}
358367
}
368+
369+
return nil
359370
}
360371

361372
func getNameAndVersionFromString(input string, delimiter string) (string, string) {

internal/target/git/git_target.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ func (g *GitTarget) ProcessSbom(ctx *target.TargetContext) error {
8686
return g.gitAccount.CommitAll(g.workingTree, fmt.Sprintf("Created new SBOM for image %s", imageID))
8787
}
8888

89-
func (g *GitTarget) LoadImages() []*libk8s.RegistryImage {
89+
func (g *GitTarget) LoadImages() ([]*libk8s.RegistryImage, error) {
9090
ignoreDirs := []string{".git"}
9191
fileName := syft.GetFileName(g.sbomFormat)
9292
basePath := filepath.Join(g.workingTree, g.workPath)
@@ -118,13 +118,13 @@ func (g *GitTarget) LoadImages() []*libk8s.RegistryImage {
118118

119119
if err != nil {
120120
logrus.WithError(err).Error("Could not list all SBOMs")
121-
return []*libk8s.RegistryImage{}
121+
return nil, err
122122
}
123123

124-
return images
124+
return images, nil
125125
}
126126

127-
func (g *GitTarget) Remove(images []*libk8s.RegistryImage) {
127+
func (g *GitTarget) Remove(images []*libk8s.RegistryImage) error {
128128
logrus.Debug("Start to remove old SBOMs")
129129
sbomFiles := g.mapToFiles(images)
130130

@@ -142,7 +142,10 @@ func (g *GitTarget) Remove(images []*libk8s.RegistryImage) {
142142
err := g.gitAccount.CommitAndPush(g.workingTree, "Deleted old SBOMs")
143143
if err != nil {
144144
logrus.WithError(err).Error("Could not commit SBOM removal to git")
145+
return err
145146
}
147+
148+
return nil
146149
}
147150

148151
func (g *GitTarget) mapToFiles(allImages []*libk8s.RegistryImage) []string {

internal/target/oci/oci_target.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,10 @@ func (g *OciTarget) ProcessSbom(ctx *target.TargetContext) error {
9292
return err
9393
}
9494

95-
func (g *OciTarget) LoadImages() []*libk8s.RegistryImage {
96-
return []*libk8s.RegistryImage{}
95+
func (g *OciTarget) LoadImages() ([]*libk8s.RegistryImage, error) {
96+
return []*libk8s.RegistryImage{}, nil
9797
}
9898

99-
func (g *OciTarget) Remove(allImages []*libk8s.RegistryImage) {
99+
func (g *OciTarget) Remove(allImages []*libk8s.RegistryImage) error {
100+
return nil
100101
}

internal/target/target.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ type Target interface {
1616
Initialize() error
1717
ValidateConfig() error
1818
ProcessSbom(ctx *TargetContext) error
19-
LoadImages() []*oci.RegistryImage
20-
Remove(images []*oci.RegistryImage)
19+
LoadImages() ([]*oci.RegistryImage, error)
20+
Remove(images []*oci.RegistryImage) error
2121
}
2222

2323
func NewContext(sbom string, image *oci.RegistryImage, container *libk8s.ContainerInfo, pod *libk8s.PodInfo) *TargetContext {

0 commit comments

Comments
 (0)