feat: avoid getting image metadata from container registry when it is…

… not needed (#5727)

pkg/expressions/generic.go

@@ -9,6 +9,8 @@ import (
 	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
+var ErrWalkStop = errors.New("end walking")
+
 type tagData struct {
 	key   string
 	value string
@@ -259,6 +261,18 @@ func Simplify(t interface{}, m ...Machine) error {
 	return simplify(t, tagData{value: "include"}, m...)
 }
 
+func WalkVariables(t interface{}, variableFn func(name string) error) error {
+	m := NewMachine().RegisterAccessorExt(func(name string) (interface{}, bool, error) {
+		err := variableFn(name)
+		return nil, err != nil, err
+	})
+	err := Simplify(t, m)
+	if errors.Is(err, ErrWalkStop) {
+		return nil
+	}
+	return err
+}
+
 func SimplifyForce(t interface{}, m ...Machine) error {
 	return simplify(t, tagData{value: "force"}, m...)
 }

pkg/testworkflows/testworkflowprocessor/processor.go

@@ -193,17 +193,21 @@ func (p *processor) Bundle(ctx context.Context, workflow *testworkflowsv1.TestWo
 		pullSecretNames[i] = v.Name
 	}
 
-	// Load the image details
-	imageNames := root.GetImages()
+	// Load the image details when necessary
+	hasPodSecurityContextGroup := podConfig.SecurityContext != nil && podConfig.SecurityContext.RunAsGroup != nil
+	imageNames := root.GetImages(hasPodSecurityContextGroup)
 	images := make(map[string]*imageinspector.Info)
 	imageNameResolutions := map[string]string{}
-	for image := range imageNames {
-		info, err := p.inspector.Inspect(ctx, "", image, corev1.PullIfNotPresent, pullSecretNames)
+	for image, needsMetadata := range imageNames {
+		var info *imageinspector.Info
+		if needsMetadata {
+			info, err = p.inspector.Inspect(ctx, "", image, corev1.PullIfNotPresent, pullSecretNames)
+			images[image] = info
+		}
 		imageNameResolutions[image] = p.inspector.ResolveName("", image)
 		if err != nil {
 			return nil, fmt.Errorf("resolving image error: %s: %s", image, err.Error())
 		}
-		images[image] = info
 	}
 	err = root.ApplyImages(images, imageNameResolutions)
 	if err != nil {
@@ -221,21 +225,19 @@ func (p *processor) Bundle(ctx context.Context, workflow *testworkflowsv1.TestWo
 	}
 	if len(otherContainers) == 1 {
 		image := otherContainers[0].Container().Image()
-		if _, ok := images[image]; ok {
-			sc := otherContainers[0].Container().SecurityContext()
-			if sc == nil {
-				sc = &corev1.SecurityContext{}
-			}
-			if podConfig.SecurityContext == nil {
-				podConfig.SecurityContext = &corev1.PodSecurityContext{}
-			}
-			if sc.RunAsGroup == nil && podConfig.SecurityContext.RunAsGroup == nil {
-				sc.RunAsGroup = common.Ptr(images[image].Group)
-				otherContainers[0].Container().SetSecurityContext(sc)
-			}
-			if podConfig.SecurityContext.FSGroup == nil {
-				podConfig.SecurityContext.FSGroup = sc.RunAsGroup
-			}
+		sc := otherContainers[0].Container().SecurityContext()
+		if sc == nil {
+			sc = &corev1.SecurityContext{}
+		}
+		if podConfig.SecurityContext == nil {
+			podConfig.SecurityContext = &corev1.PodSecurityContext{}
+		}
+		if sc.RunAsGroup == nil && podConfig.SecurityContext.RunAsGroup == nil && images[image] != nil {
+			sc.RunAsGroup = common.Ptr(images[image].Group)
+			otherContainers[0].Container().SetSecurityContext(sc)
+		}
+		if podConfig.SecurityContext.FSGroup == nil {
+			podConfig.SecurityContext.FSGroup = sc.RunAsGroup
 		}
 	}
 	containerStages = nil

pkg/testworkflows/testworkflowprocessor/stage/container.go

@@ -49,6 +49,7 @@ type ContainerAccessors interface {
 	HasVolumeAt(path string) bool
 	ToContainerConfig() testworkflowsv1.ContainerConfig
 	IsToolkit() bool
+	NeedsImageData(isGroupNeeded bool) bool
 }
 
 type ContainerMutations[T any] interface {
@@ -384,6 +385,9 @@ func (c *container) ToKubernetesTemplate() (corev1.Container, error) {
 }
 
 func (c *container) ApplyImageData(image *imageinspector.Info, resolvedImageName string) error {
+	if resolvedImageName != "" && c.Image() != resolvedImageName {
+		c.SetImage(resolvedImageName)
+	}
 	if image == nil {
 		return nil
 	}
@@ -396,9 +400,6 @@ func (c *container) ApplyImageData(image *imageinspector.Info, resolvedImageName
 	}
 	command := c.Command()
 	args := c.Args()
-	if resolvedImageName != "" && c.Image() != resolvedImageName {
-		c.SetImage(resolvedImageName)
-	}
 	if len(command) == 0 {
 		c.SetCommand(image.Entrypoint...)
 		if len(args) == 0 {
@@ -413,6 +414,26 @@ func (c *container) ApplyImageData(image *imageinspector.Info, resolvedImageName
 	return nil
 }
 
+// NeedsImageData checks if we need to fetch metadata of the destination image from Container Registry
+func (c *container) NeedsImageData(isGroupNeeded bool) bool {
+	if len(c.Command()) == 0 || c.WorkingDir() == "" {
+		return true
+	}
+	securityContext := c.SecurityContext()
+	if isGroupNeeded && (securityContext == nil || securityContext.RunAsGroup == nil) {
+		return true
+	}
+	usesVariables := false
+	expressions.WalkVariables(c, func(name string) error {
+		if name == "image.command" || name == "image.args" || name == "image.workingDir" {
+			usesVariables = true
+			return expressions.ErrWalkStop
+		}
+		return nil
+	})
+	return usesVariables
+}
+
 func (c *container) IsToolkit() bool {
 	return c.toolkit || (c.parent != nil && c.parent.IsToolkit()) || slices.Contains(c.Cr.Env, BypassToolkitCheck)
 }
@@ -473,8 +494,8 @@ func (c *container) EnableToolkit(ref string) Container {
 		})
 }
 
-func (c *container) Resolve(m ...expressions.Machine) error {
-	base := expressions.NewMachine().
+func (c *container) envMachine() expressions.Machine {
+	return expressions.NewMachine().
 		RegisterAccessor(func(name string) (interface{}, bool) {
 			if !strings.HasPrefix(name, "env.") {
 				return nil, false
@@ -492,5 +513,8 @@ func (c *container) Resolve(m ...expressions.Machine) error {
 			}
 			return nil, false
 		})
-	return expressions.Simplify(c, append([]expressions.Machine{base}, m...)...)
+}
+
+func (c *container) Resolve(m ...expressions.Machine) error {
+	return expressions.Simplify(c, append([]expressions.Machine{c.envMachine()}, m...)...)
 }

pkg/testworkflows/testworkflowprocessor/stage/containerstage.go

@@ -52,8 +52,8 @@ func (s *containerStage) ContainerStages() []ContainerStage {
 	return []ContainerStage{s}
 }
 
-func (s *containerStage) GetImages() map[string]struct{} {
-	return map[string]struct{}{s.container.Image(): {}}
+func (s *containerStage) GetImages(isGroupNeeded bool) map[string]bool {
+	return map[string]bool{s.container.Image(): s.container.NeedsImageData(isGroupNeeded)}
 }
 
 func (s *containerStage) Flatten() []Stage {

pkg/testworkflows/testworkflowprocessor/stage/groupstage.go

@@ -1,8 +1,6 @@
 package stage
 
 import (
-	"maps"
-
 	"github.com/pkg/errors"
 
 	"github.com/kubeshop/testkube/pkg/expressions"
@@ -100,10 +98,12 @@ func (s *groupStage) RecursiveChildren() []Stage {
 	return res
 }
 
-func (s *groupStage) GetImages() map[string]struct{} {
-	v := make(map[string]struct{})
+func (s *groupStage) GetImages(isGroupNeeded bool) map[string]bool {
+	v := make(map[string]bool)
 	for _, ch := range s.children {
-		maps.Copy(v, ch.GetImages())
+		for name, needsMetadata := range ch.GetImages(isGroupNeeded) {
+			v[name] = v[name] || needsMetadata
+		}
 	}
 	return v
 }

pkg/testworkflows/testworkflowprocessor/stage/stage.go

@@ -14,7 +14,7 @@ type Stage interface {
 	Signature() Signature
 	Resolve(m ...expressions.Machine) error
 	ContainerStages() []ContainerStage
-	GetImages() map[string]struct{}
+	GetImages(isGroupNeeded bool) map[string]bool
 	ApplyImages(images map[string]*imageinspector.Info, imageNameResolutions map[string]string) error
 	Flatten() []Stage
 }