fix(TKC-2188): use global.defaultRegistry for Test Workflows (#5644)

* fix(TKC-2188): use global registry for parallel/services syntax if available * fix(TKC-2188): use global.defaultRegistry for the TestWorkflow images
kubeshop · Jul 8, 2024 · 483b907 · 483b907
1 parent 58ee3e8
commit 483b907
Show file tree

Hide file tree

Showing 14 changed files with 147 additions and 26 deletions.
diff --git a/cmd/tcl/testworkflow-toolkit/spawn/utils.go b/cmd/tcl/testworkflow-toolkit/spawn/utils.go
@@ -291,6 +291,7 @@ func CreateBaseMachine() expressions.Machine {
 			"namespace":       env.Namespace(),
 			"defaultRegistry": env.Config().System.DefaultRegistry,
 
+			"images.defaultRegistry":     env.Config().System.DefaultRegistry,
 			"images.init":                env.Config().Images.Init,
 			"images.toolkit":             env.Config().Images.Toolkit,
 			"images.persistence.enabled": strconv.FormatBool(env.Config().Images.InspectorPersistenceEnabled),

diff --git a/pkg/imageinspector/inspector.go b/pkg/imageinspector/inspector.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"path/filepath"
+	"regexp"
 
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
@@ -18,6 +19,10 @@ type inspector struct {
 	storage         []Storage
 }
 
+var (
+	ImageWithRegistryRe = regexp.MustCompile(`^[^/]+\.[^/]+/`)
+)
+
 func NewInspector(defaultRegistry string, infoFetcher InfoFetcher, secretFetcher SecretFetcher, storage ...Storage) Inspector {
 	return &inspector{
 		defaultRegistry: defaultRegistry,
@@ -27,7 +32,7 @@ func NewInspector(defaultRegistry string, infoFetcher InfoFetcher, secretFetcher
 	}
 }
 
-func (i *inspector) get(ctx context.Context, registry, image string) *Info {
+func (i *inspector) rawGet(ctx context.Context, registry, image string) *Info {
 	for _, s := range i.storage {
 		v, err := s.Get(ctx, RequestBase{Registry: registry, Image: image})
 		if err != nil && !errors.Is(err, context.Canceled) {
@@ -40,6 +45,16 @@ func (i *inspector) get(ctx context.Context, registry, image string) *Info {
 	return nil
 }
 
+func (i *inspector) get(ctx context.Context, registry, image string) *Info {
+	if resolvedName := i.ResolveName(registry, image); resolvedName != image {
+		v := i.rawGet(ctx, "", resolvedName)
+		if v != nil {
+			return v
+		}
+	}
+	return i.rawGet(ctx, registry, image)
+}
+
 func (i *inspector) fetch(ctx context.Context, registry, image string, pullSecretNames []string) (*Info, error) {
 	// Fetch the secrets
 	secrets := make([]corev1.Secret, len(pullSecretNames))
@@ -68,13 +83,30 @@ func (i *inspector) save(ctx context.Context, registry, image string, info *Info
 	if info == nil {
 		return
 	}
+	if resolvedName := i.ResolveName(registry, image); resolvedName != image {
+		registry = ""
+		image = resolvedName
+	}
 	for _, s := range i.storage {
 		if err := s.Store(ctx, RequestBase{Registry: registry, Image: image}, *info); err != nil {
 			log.DefaultLogger.Warnw("error while saving image details in the cache", "registry", registry, "image", image, "error", err)
 		}
 	}
 }
 
+func (i *inspector) ResolveName(registry, image string) string {
+	if ImageWithRegistryRe.MatchString(image) {
+		return image
+	}
+	if registry == "" {
+		registry = i.defaultRegistry
+	}
+	if registry == "" {
+		return image
+	}
+	return fmt.Sprintf("%s/%s", registry, image)
+}
+
 func (i *inspector) Inspect(ctx context.Context, registry, image string, pullPolicy corev1.PullPolicy, pullSecretNames []string) (*Info, error) {
 	// Load from cache
 	if pullPolicy != corev1.PullAlways {

diff --git a/pkg/imageinspector/inspector_test.go b/pkg/imageinspector/inspector_test.go
@@ -16,17 +16,20 @@ func TestInspectorInspect(t *testing.T) {
 	secrets := NewMockSecretFetcher(ctrl)
 	storage1 := NewMockStorageWithTransfer(ctrl)
 	storage2 := NewMockStorageWithTransfer(ctrl)
-	inspector := NewInspector("default", infos, secrets, storage1, storage2)
+	inspector := NewInspector("default.io", infos, secrets, storage1, storage2)
 
 	sec := corev1.Secret{StringData: map[string]string{"foo": "bar"}}
-	req := RequestBase{Registry: "regname", Image: "imgname"}
+	req := RequestBase{Registry: "regname.io", Image: "imgname"}
+	resolvedReq := RequestBase{Image: "regname.io/imgname"}
+	storage1.EXPECT().Get(gomock.Any(), resolvedReq).Return(nil, nil)
+	storage2.EXPECT().Get(gomock.Any(), resolvedReq).Return(nil, nil)
 	storage1.EXPECT().Get(gomock.Any(), req).Return(nil, nil)
 	storage2.EXPECT().Get(gomock.Any(), req).Return(nil, nil)
 	secrets.EXPECT().Get(gomock.Any(), "secname").Return(&sec, nil)
 	infos.EXPECT().Fetch(gomock.Any(), req.Registry, req.Image, []corev1.Secret{sec}).Return(&info1, nil)
 
-	storage1.EXPECT().Store(gomock.Any(), req, info1).Return(nil)
-	storage2.EXPECT().Store(gomock.Any(), req, info1).Return(nil)
+	storage1.EXPECT().Store(gomock.Any(), resolvedReq, info1).Return(nil)
+	storage2.EXPECT().Store(gomock.Any(), resolvedReq, info1).Return(nil)
 
 	v, err := inspector.Inspect(context.Background(), req.Registry, req.Image, corev1.PullIfNotPresent, []string{"secname"})
 	assert.NoError(t, err)
@@ -42,10 +45,11 @@ func TestInspectorInspectWithCache(t *testing.T) {
 	secrets := NewMockSecretFetcher(ctrl)
 	storage1 := NewMockStorageWithTransfer(ctrl)
 	storage2 := NewMockStorageWithTransfer(ctrl)
-	inspector := NewInspector("default", infos, secrets, storage1, storage2)
+	inspector := NewInspector("default.io", infos, secrets, storage1, storage2)
 
-	req := RequestBase{Registry: "regname", Image: "imgname"}
-	storage1.EXPECT().Get(gomock.Any(), req).Return(&info1, nil)
+	req := RequestBase{Registry: "regname.io", Image: "imgname"}
+	resolvedReq := RequestBase{Image: "regname.io/imgname"}
+	storage1.EXPECT().Get(gomock.Any(), resolvedReq).Return(&info1, nil)
 
 	v, err := inspector.Inspect(context.Background(), req.Registry, req.Image, corev1.PullIfNotPresent, []string{"secname"})
 	assert.NoError(t, err)
@@ -54,3 +58,59 @@ func TestInspectorInspectWithCache(t *testing.T) {
 	// Wait until asynchronous storage will be done
 	<-time.After(10 * time.Millisecond)
 }
+
+func TestInspector_ResolveName_NoDefault_NoOverride(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	infos := NewMockInfoFetcher(ctrl)
+	secrets := NewMockSecretFetcher(ctrl)
+	inspector := NewInspector("", infos, secrets)
+
+	assert.Equal(t, "image:1.2.3", inspector.ResolveName("", "image:1.2.3"))
+	assert.Equal(t, "repo/image:1.2.3", inspector.ResolveName("", "repo/image:1.2.3"))
+	assert.Equal(t, "docker.io/image:1.2.3", inspector.ResolveName("", "docker.io/image:1.2.3"))
+	assert.Equal(t, "ghcr.io/image:1.2.3", inspector.ResolveName("", "ghcr.io/image:1.2.3"))
+	assert.Equal(t, "docker.io/repo/image:1.2.3", inspector.ResolveName("", "docker.io/repo/image:1.2.3"))
+	assert.Equal(t, "ghcr.io/repo/image:1.2.3", inspector.ResolveName("", "ghcr.io/repo/image:1.2.3"))
+}
+
+func TestInspector_ResolveName_Default_NoOverride(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	infos := NewMockInfoFetcher(ctrl)
+	secrets := NewMockSecretFetcher(ctrl)
+	inspector := NewInspector("default.io", infos, secrets)
+
+	assert.Equal(t, "default.io/image:1.2.3", inspector.ResolveName("", "image:1.2.3"))
+	assert.Equal(t, "default.io/repo/image:1.2.3", inspector.ResolveName("", "repo/image:1.2.3"))
+	assert.Equal(t, "docker.io/image:1.2.3", inspector.ResolveName("", "docker.io/image:1.2.3"))
+	assert.Equal(t, "ghcr.io/image:1.2.3", inspector.ResolveName("", "ghcr.io/image:1.2.3"))
+	assert.Equal(t, "docker.io/repo/image:1.2.3", inspector.ResolveName("", "docker.io/repo/image:1.2.3"))
+	assert.Equal(t, "ghcr.io/repo/image:1.2.3", inspector.ResolveName("", "ghcr.io/repo/image:1.2.3"))
+}
+
+func TestInspector_ResolveName_NoDefault_Override(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	infos := NewMockInfoFetcher(ctrl)
+	secrets := NewMockSecretFetcher(ctrl)
+	inspector := NewInspector("", infos, secrets)
+
+	assert.Equal(t, "default.io/image:1.2.3", inspector.ResolveName("default.io", "image:1.2.3"))
+	assert.Equal(t, "default.io/repo/image:1.2.3", inspector.ResolveName("default.io", "repo/image:1.2.3"))
+	assert.Equal(t, "docker.io/image:1.2.3", inspector.ResolveName("default.io", "docker.io/image:1.2.3"))
+	assert.Equal(t, "ghcr.io/image:1.2.3", inspector.ResolveName("default.io", "ghcr.io/image:1.2.3"))
+	assert.Equal(t, "docker.io/repo/image:1.2.3", inspector.ResolveName("default.io", "docker.io/repo/image:1.2.3"))
+	assert.Equal(t, "ghcr.io/repo/image:1.2.3", inspector.ResolveName("default.io", "ghcr.io/repo/image:1.2.3"))
+}
+
+func TestInspector_ResolveName_Default_Override(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	infos := NewMockInfoFetcher(ctrl)
+	secrets := NewMockSecretFetcher(ctrl)
+	inspector := NewInspector("default.io", infos, secrets)
+
+	assert.Equal(t, "default.io/image:1.2.3", inspector.ResolveName("default.io", "image:1.2.3"))
+	assert.Equal(t, "default.io/repo/image:1.2.3", inspector.ResolveName("default.io", "repo/image:1.2.3"))
+	assert.Equal(t, "docker.io/image:1.2.3", inspector.ResolveName("default.io", "docker.io/image:1.2.3"))
+	assert.Equal(t, "ghcr.io/image:1.2.3", inspector.ResolveName("default.io", "ghcr.io/image:1.2.3"))
+	assert.Equal(t, "docker.io/repo/image:1.2.3", inspector.ResolveName("default.io", "docker.io/repo/image:1.2.3"))
+	assert.Equal(t, "ghcr.io/repo/image:1.2.3", inspector.ResolveName("default.io", "ghcr.io/repo/image:1.2.3"))
+}
diff --git a/pkg/imageinspector/mock_inspector.go b/pkg/imageinspector/mock_inspector.go
diff --git a/pkg/imageinspector/types.go b/pkg/imageinspector/types.go
@@ -10,6 +10,7 @@ import (
 //go:generate mockgen -destination=./mock_inspector.go -package=imageinspector "github.com/kubeshop/testkube/pkg/imageinspector" Inspector
 type Inspector interface {
 	Inspect(ctx context.Context, registry, image string, pullPolicy corev1.PullPolicy, pullSecretNames []string) (*Info, error)
+	ResolveName(registry, image string) string
 }
 
 type StorageTransfer interface {

diff --git a/pkg/testworkflows/testworkflowexecutor/executor.go b/pkg/testworkflows/testworkflowexecutor/executor.go
@@ -430,6 +430,7 @@ func (e *executor) Execute(ctx context.Context, workflow testworkflowsv1.TestWor
 			"clusterId":       e.clusterID,
 			"cdeventsTarget":  os.Getenv("CDEVENTS_TARGET"),
 
+			"images.defaultRegistry":     e.defaultRegistry,
 			"images.init":                constants.DefaultInitImage,
 			"images.toolkit":             constants.DefaultToolkitImage,
 			"images.persistence.enabled": strconv.FormatBool(e.enableImageDataPersistentCache),

diff --git a/pkg/testworkflows/testworkflowprocessor/container.go b/pkg/testworkflows/testworkflowprocessor/container.go
@@ -65,7 +65,7 @@ type ContainerMutations[T any] interface {
 	SetSecurityContext(sc *corev1.SecurityContext) T
 
 	ApplyCR(cr *testworkflowsv1.ContainerConfig) T
-	ApplyImageData(image *imageinspector.Info) error
+	ApplyImageData(image *imageinspector.Info, resolvedImageName string) error
 	EnableToolkit(ref string) T
 }
 
@@ -385,7 +385,7 @@ func (c *container) ToKubernetesTemplate() (corev1.Container, error) {
 	}, nil
 }
 
-func (c *container) ApplyImageData(image *imageinspector.Info) error {
+func (c *container) ApplyImageData(image *imageinspector.Info, resolvedImageName string) error {
 	if image == nil {
 		return nil
 	}
@@ -396,8 +396,12 @@ func (c *container) ApplyImageData(image *imageinspector.Info) error {
 	if err != nil {
 		return err
 	}
-	if len(c.Command()) == 0 {
-		args := c.Args()
+	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 {
 			c.SetArgs(image.Cmd...)
@@ -429,6 +433,7 @@ func (c *container) EnableToolkit(ref string) Container {
 			"TK_EXR":                    "{{resource.root}}",
 			"TK_FS":                     "{{resource.fsPrefix}}",
 			"TK_SA":                     "{{internal.serviceaccount.default}}",
+			"TK_R":                      "{{internal.images.defaultRegistry}}",
 			"TK_DASH":                   "{{internal.dashboard.url}}",
 			"TK_API":                    "{{internal.api.url}}",
 			"TK_CLU":                    "{{internal.clusterId}}",

diff --git a/pkg/testworkflows/testworkflowprocessor/containerstage.go b/pkg/testworkflows/testworkflowprocessor/containerstage.go
@@ -52,8 +52,9 @@ func (s *containerStage) Flatten() []Stage {
 	return []Stage{s}
 }
 
-func (s *containerStage) ApplyImages(images map[string]*imageinspector.Info) error {
-	return s.container.ApplyImageData(images[s.container.Image()])
+func (s *containerStage) ApplyImages(images map[string]*imageinspector.Info, imageNameResolutions map[string]string) error {
+	originalImageName := s.container.Image()
+	return s.container.ApplyImageData(images[originalImageName], imageNameResolutions[originalImageName])
 }
 
 func (s *containerStage) Resolve(m ...expressions.Machine) error {

diff --git a/pkg/testworkflows/testworkflowprocessor/groupstage.go b/pkg/testworkflows/testworkflowprocessor/groupstage.go
@@ -149,9 +149,9 @@ func (s *groupStage) Add(stages ...Stage) GroupStage {
 	return s
 }
 
-func (s *groupStage) ApplyImages(images map[string]*imageinspector.Info) error {
+func (s *groupStage) ApplyImages(images map[string]*imageinspector.Info, imageNameResolutions map[string]string) error {
 	for i := range s.children {
-		err := s.children[i].ApplyImages(images)
+		err := s.children[i].ApplyImages(images, imageNameResolutions)
 		if err != nil {
 			return errors.Wrap(err, "applying image data")
 		}

diff --git a/pkg/testworkflows/testworkflowprocessor/mock_container.go b/pkg/testworkflows/testworkflowprocessor/mock_container.go
diff --git a/pkg/testworkflows/testworkflowprocessor/mock_stage.go b/pkg/testworkflows/testworkflowprocessor/mock_stage.go
diff --git a/pkg/testworkflows/testworkflowprocessor/presets/processor_test.go b/pkg/testworkflows/testworkflowprocessor/presets/processor_test.go
@@ -24,6 +24,10 @@ func (*dummyInspector) Inspect(ctx context.Context, registry, image string, pull
 	return &imageinspector.Info{}, nil
 }
 
+func (*dummyInspector) ResolveName(registry, image string) string {
+	return image
+}
+
 var (
 	ins         = &dummyInspector{}
 	proc        = NewPro(ins)

diff --git a/pkg/testworkflows/testworkflowprocessor/processor.go b/pkg/testworkflows/testworkflowprocessor/processor.go
@@ -193,14 +193,16 @@ func (p *processor) Bundle(ctx context.Context, workflow *testworkflowsv1.TestWo
 	// Load the image details
 	imageNames := root.GetImages()
 	images := make(map[string]*imageinspector.Info)
+	imageNameResolutions := map[string]string{}
 	for image := range imageNames {
 		info, err := p.inspector.Inspect(ctx, "", image, corev1.PullIfNotPresent, pullSecretNames)
+		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)
+	err = root.ApplyImages(images, imageNameResolutions)
 	if err != nil {
 		return nil, errors.Wrap(err, "applying image data")
 	}

diff --git a/pkg/testworkflows/testworkflowprocessor/stage.go b/pkg/testworkflows/testworkflowprocessor/stage.go
@@ -15,6 +15,6 @@ type Stage interface {
 	Resolve(m ...expressions.Machine) error
 	ContainerStages() []ContainerStage
 	GetImages() map[string]struct{}
-	ApplyImages(images map[string]*imageinspector.Info) error
+	ApplyImages(images map[string]*imageinspector.Info, imageNameResolutions map[string]string) error
 	Flatten() []Stage
 }