Skip to content

Commit

Permalink
Move Pinned check + removal to the collector and remover pods
Browse files Browse the repository at this point in the history
Signed-off-by: Fabian Gonzalez <fabiangonz98@gmail.com>
  • Loading branch information
inFocus7 committed Dec 2, 2023
1 parent c30da36 commit 7f95f58
Show file tree
Hide file tree
Showing 12 changed files with 42 additions and 38 deletions.
1 change: 1 addition & 0 deletions api/unversioned/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ const (
oneDay = unversioned.Duration(time.Hour * 24)
)

// TODO - add defaults for gathering/scanning/removing Pinned images
func Default() *unversioned.EraserConfig {
return &unversioned.EraserConfig{
Manager: unversioned.ManagerConfig{
Expand Down
5 changes: 0 additions & 5 deletions api/v1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions api/v1alpha1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions controllers/imagecollector/imagecollector_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,10 +308,12 @@ func (r *Reconciler) createImageJob(ctx context.Context) (ctrl.Result, error) {
fmt.Sprintf("--pprof-port=%d", profileConfig.Port),
}

collArgs := []string{"--scan-disabled=" + strconv.FormatBool(scanDisabled)}
// todo implement the config for this
collArgs := []string{"--scan-disabled=" + strconv.FormatBool(scanDisabled), "--scan-pinned=" + strconv.FormatBool(scanCfg.ScanPinned)}
collArgs = append(collArgs, profileArgs...)

removerArgs := []string{"--log-level=" + logger.GetLevel()}
// todo implement the config for this
removerArgs := []string{"--log-level=" + logger.GetLevel(), "--remove-pinned=" + strconv.FormatBool(eraserCfg.RemovePinned)}
removerArgs = append(removerArgs, profileArgs...)

pullSecrets := []corev1.LocalObjectReference{}
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ replace (
k8s.io/component-helpers => k8s.io/component-helpers v0.26.11
k8s.io/controller-manager => k8s.io/controller-manager v0.26.11
k8s.io/csi-translation-lib => k8s.io/csi-translation-lib v0.26.11
k8s.io/dynamic-resource-allocation => k8s.io/dynamic-resource-allocation v0.26.11
k8s.io/kube-aggregator => k8s.io/kube-aggregator v0.26.11
k8s.io/kube-controller-manager => k8s.io/kube-controller-manager v0.26.11
k8s.io/kube-proxy => k8s.io/kube-proxy v0.26.11
Expand Down
6 changes: 6 additions & 0 deletions pkg/collector/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ var (
enableProfile = flag.Bool("enable-pprof", false, "enable pprof profiling")
profilePort = flag.Int("pprof-port", 6060, "port for pprof profiling. defaulted to 6060 if unspecified")
scanDisabled = flag.Bool("scan-disabled", false, "boolean for if scanner container is disabled")
scanPinned = flag.Bool("scan-pinned", false, "boolean for if scanner container should scan pinned images")

// Timeout of connecting to server (default: 5m).
timeout = 5 * time.Minute
Expand Down Expand Up @@ -80,6 +81,11 @@ func main() {
}
log.Info("images collected", "finalImages:", finalImages)

if !(*scanPinned) {
log.Info("skipping scanning pinned images")
finalImages = util.RemovePinnedImages(finalImages)
}

data, err := json.Marshal(finalImages)
if err != nil {
log.Error(err, "failed to encode finalImages")
Expand Down
13 changes: 12 additions & 1 deletion pkg/remover/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
util "github.com/eraser-dev/eraser/pkg/utils"
)

func removeImages(c cri.Remover, targetImages []string) (int, error) {
func removeImages(c cri.Remover, removePinned bool, targetImages []string) (int, error) {
removed := 0

backgroundContext, cancel := context.WithTimeout(context.Background(), timeout)
Expand Down Expand Up @@ -76,6 +76,12 @@ func removeImages(c cri.Remover, targetImages []string) (int, error) {
continue
}

// TODO - figure out why is imgDigestOrTag used instead of imageID when it's called "idToImageMap" (copied usage from isExcluded).
if !removePinned && util.IsPinned(imageID, idToImageMap) {
log.Info("image is kept due to being pinned", "given", imgDigestOrTag, "imageID", imageID, "name", idToImageMap[imageID])
continue
}

err = c.DeleteImage(backgroundContext, imageID)
if err != nil {
log.Error(err, "error removing image", "given", imgDigestOrTag, "imageID", imageID, "name", idToImageMap[imageID])
Expand Down Expand Up @@ -109,6 +115,11 @@ func removeImages(c cri.Remover, targetImages []string) (int, error) {
continue
}

if !removePinned && util.IsPinned(imageID, idToImageMap) {
log.Info("image is kept due to being pinned", "imageID", imageID, "name", idToImageMap[imageID])
continue
}

if err := c.DeleteImage(backgroundContext, imageID); err != nil {
success = false
log.Error(err, "error removing image", "imageID", imageID, "name", idToImageMap[imageID])
Expand Down
4 changes: 3 additions & 1 deletion pkg/remover/remover.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ var (
imageListPtr = flag.String("imagelist", "", "name of ImageList")
enableProfile = flag.Bool("enable-pprof", false, "enable pprof profiling")
profilePort = flag.Int("pprof-port", 6060, "port for pprof profiling. defaulted to 6060 if unspecified")
removePinned = flag.Bool("remove-pinned", false, "skip over pinned images when removing")

// Timeout of connecting to server (default: 5m).
timeout = 5 * time.Minute
Expand Down Expand Up @@ -130,7 +131,8 @@ func main() {
log.Info("no images to exclude")
}

removed, err := removeImages(client, imagelist)
// we pass in the removePinned flag to removeImages, because as of now we just have a list of imageIDs, and we don't know if they are pinned or not
removed, err := removeImages(client, *removePinned, imagelist)
if err != nil {
log.Error(err, "failed to remove images")
os.Exit(generalErr)
Expand Down
10 changes: 0 additions & 10 deletions pkg/scanners/template/scanner_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ func NewImageProvider(funcs ...ConfigFunc) ImageProvider {
ctx: context.Background(),
log: logf.Log.WithName("scanner"),
deleteScanFailedImages: true,
deletePinnedImages: false,
reportMetrics: false,
}

Expand All @@ -58,7 +57,6 @@ func NewImageProvider(funcs ...ConfigFunc) ImageProvider {
return cfg
}

// TODO - 1. We could filter here, so the returned images are all images that are not pinned.
func (cfg *config) ReceiveImages() ([]unversioned.Image, error) {
var err error

Expand All @@ -83,7 +81,6 @@ func (cfg *config) ReceiveImages() ([]unversioned.Image, error) {
}

func (cfg *config) SendImages(nonCompliantImages, failedImages []unversioned.Image) error {
// TODO - 4. we could filter out pinned images here, so they are not deleted.
if cfg.deleteScanFailedImages {
nonCompliantImages = append(nonCompliantImages, failedImages...)
}
Expand Down Expand Up @@ -155,13 +152,6 @@ func WithDeleteEOLImages(deleteEOLImages bool) ConfigFunc {
}
}

// sets deletePinnedImages flag.
func WithDeletePinnedImages(deletePinnedImages bool) ConfigFunc {
return func(cfg *config) {
cfg.deletePinnedImages = deletePinnedImages
}
}

// provide custom logger.
func WithLogger(log logr.Logger) ConfigFunc {
return func(cfg *config) {
Expand Down
13 changes: 1 addition & 12 deletions pkg/scanners/trivy/trivy.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,30 +100,19 @@ func main() {
template.WithMetrics(recordMetrics),
template.WithDeleteScanFailedImages(userConfig.DeleteFailedImages),
template.WithDeleteEOLImages(userConfig.DeleteEOLImages),
template.WithDeletePinnedImages(userConfig.DeletePinnedImages),
)

allImages, err := provider.ReceiveImages()
if err != nil {
log.Error(err, "unable to read images from provider")
os.Exit(generalErr)
}
// TODO - 2. We could filter the pinned images out here, to not affect the template.

s, err := initScanner(&userConfig)
if err != nil {
log.Error(err, "error initializing scanner")
}

// TODO: 4 options to decide on how we'd want to {filter out/handle} `pinned` images.
// 1. Filter inside the `ReceiveImages` function, part of the scanner template, so `allImages` is all non-pinned.
// 2. Filter `allImages` AFTER `ReceiveImages`, so we don't affect the scanner template function.
// 3. During the `scan` (or `Scan`) function, check if the image is pinned and continue.
// - This could be the most performant, where we don't add extra filtering, and just `continue` during image scans.
// - We could decide here if we still want to scan the pinned image, even when we don't want to delete it.
// 4. Filter inside the `SendImages` function, part of the scanner template, so the images sent to the eraser are non-pinned.
// - Not sure how our template works, and if we'd want to filter there so other implementations don't have to.
// Adding filtering (aside from step 3 where we `continue`) would add an extra O(n) time complexity to go through all images and filter.
vulnerableImages, failedImages, err := scan(s, allImages)
if err != nil {
log.Error(err, "total image scan timed out")
Expand All @@ -135,6 +124,7 @@ func main() {
log.Info("Failed", "Images", failedImages)
}

// send to eraser?
err = provider.SendImages(vulnerableImages, failedImages)
if err != nil {
log.Error(err, "unable to write images")
Expand Down Expand Up @@ -193,7 +183,6 @@ func scan(s Scanner, allImages []unversioned.Image) ([]unversioned.Image, []unve
// track total scan job time

for idx, img := range allImages {
// TODO - 3. we could filter out Pinned images here by `continue`-ing. we'll need to be sure nothing wonky happens with pinned images on timeout.
select {
case <-s.Timer().C:
failedImages = append(failedImages, allImages[idx:]...)
Expand Down
2 changes: 0 additions & 2 deletions pkg/scanners/trivy/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ type (
DBRepo string `json:"dbRepo,omitempty"`
DeleteFailedImages bool `json:"deleteFailedImages,omitempty"`
DeleteEOLImages bool `json:"deleteEOLImages,omitempty"`
DeletePinnedImages bool `json:"deletePinnedImages,omitempty"`
Vulnerabilities VulnConfig `json:"vulnerabilities,omitempty"`
Timeout TimeoutConfig `json:"timeout,omitempty"`
}
Expand Down Expand Up @@ -72,7 +71,6 @@ func DefaultConfig() *Config {
DBRepo: "ghcr.io/aquasecurity/trivy-db",
DeleteFailedImages: true,
DeleteEOLImages: true,
DeletePinnedImages: false,
Vulnerabilities: VulnConfig{
IgnoreUnfixed: true,
Types: []string{
Expand Down
14 changes: 14 additions & 0 deletions pkg/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,10 @@ func GetNonRunningImages(runningImages map[string]string, allImages []unversione
return nonRunningImages
}

func IsPinned(img string, idToImageMap map[string]unversioned.Image) bool {
return idToImageMap[img].Pinned
}

func IsExcluded(excluded map[string]struct{}, img string, idToImageMap map[string]unversioned.Image) bool {
if len(excluded) == 0 {
return false
Expand Down Expand Up @@ -417,3 +421,13 @@ func ProcessRepoDigests(repoDigests []string) ([]string, []error) {

return digests, errs
}

func RemovePinnedImages(images []unversioned.Image) []unversioned.Image {
filteredImages := []unversioned.Image{}
for _, image := range images {
if !image.Pinned {
filteredImages = append(filteredImages, image)
}
}
return filteredImages
}

0 comments on commit 7f95f58

Please sign in to comment.