Skip to content

Create a central image utility package #1103

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
liamfallon merged 10 commits into from
Jul 20, 2026
Merged

Create a central image utility package #1103

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f09b493
image parsing util
mozesl-nokia Jul 3, 2026
183bb3e
stringer interface
mozesl-nokia Jul 3, 2026
636f477
AI review
mozesl-nokia Jul 3, 2026
bda0baa
fix unit tests
mozesl-nokia Jul 3, 2026
d78b858
fix lint
mozesl-nokia Jul 6, 2026
0ba41be
improve coverage
mozesl-nokia Jul 6, 2026
846f92a
don't expect weird whitespaces
mozesl-nokia Jul 6, 2026
8df46b8
clean up GetBinaryFromCacheByConstraint
mozesl-nokia Jul 6, 2026
33bd121
add unit tests
mozesl-nokia Jul 6, 2026
3152f20
Merge main into image-utils
mozesl-nokia Jul 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 16 additions & 26 deletions controllers/functionconfigs/reconciler/functionconfigreconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
"github.com/kptdev/krm-functions-catalog/functions/go/starlark/starlark"
fnsdk "github.com/kptdev/krm-functions-sdk/go/fn"
configapi "github.com/kptdev/porch/api/porchconfig/v1alpha1"
"github.com/kptdev/porch/pkg/util"
imageutil "github.com/kptdev/porch/pkg/util/image"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"
"k8s.io/klog/v2"
Expand Down Expand Up @@ -94,16 +94,6 @@ func (s *FunctionConfigStore) generateRegexPattern(prefixes []string, imageName

}

func splitImage(image string) (name string, tag string) {
lastSlash := strings.LastIndex(image, "/")
lastColon := strings.LastIndex(image, ":")

if lastColon > lastSlash {
return image[:lastColon], image[lastColon+1:]
}
return image, ""
}

func (s *FunctionConfigStore) UpdateBinaryCache(_ string, obj *configapi.FunctionConfig) {
s.mu.Lock()
defer s.mu.Unlock()
Expand Down Expand Up @@ -181,13 +171,13 @@ func (s *FunctionConfigStore) GetBinaryFromCache(image string) (string, bool) {
s.mu.RLock()
defer s.mu.RUnlock()

image, tag := splitImage(image)
prefixToCheck := util.GetImageRepository(image)
binaryStore, exists := s.binaryExecutorCache[util.GetImageName(image)]
parsedImage := imageutil.Parse(image)
prefixToCheck := parsedImage.Prefix()
binaryStore, exists := s.binaryExecutorCache[parsedImage.BaseName]
if exists {
regex := regexp.MustCompile(binaryStore.PrefixRegex)
if regex.MatchString(prefixToCheck) {
binaryPath, tagExists := binaryStore.Tags[tag]
binaryPath, tagExists := binaryStore.Tags[parsedImage.Tag]
if tagExists {
return binaryPath, true
}
Expand All @@ -200,24 +190,25 @@ func (s *FunctionConfigStore) GetBinaryFromCacheByConstraint(image, tag string)
s.mu.RLock()
defer s.mu.RUnlock()

baseName := util.GetImageName(image)
cacheEntry := s.binaryExecutorCache[baseName]
parsedImage := imageutil.Parse(image)
cacheEntry := s.binaryExecutorCache[parsedImage.BaseName]

cacheKeys := make([]string, 0, len(s.binaryExecutorCache))
for k := range cacheEntry.Tags {
cacheKeys = append(cacheKeys, k)
}

selectedKey, err := util.FindBestSemverMatch(tag, image, cacheKeys)
selectedKey, err := imageutil.FindBestSemverMatch(tag, cacheKeys)
if err != nil {
return "", false
}
selectedBinary := cacheEntry.Tags[selectedKey]

prefixToCheck, tag := splitImage(image)
// TODO: something is not right here

regex := regexp.MustCompile(cacheEntry.PrefixRegex)
if regex.MatchString(prefixToCheck) {
binaryPath, tagExists := cacheEntry.Tags[tag]
if regex.MatchString(parsedImage.Prefix()) {
binaryPath, tagExists := cacheEntry.Tags[parsedImage.Tag]
if tagExists {
return binaryPath, true
Comment thread
mozesl-nokia marked this conversation as resolved.
Outdated
}
Expand All @@ -236,14 +227,13 @@ func (s *FunctionConfigStore) GetExecCache() map[string]BuiltInCacheEntry {
func (s *FunctionConfigStore) GetProcessorFromCache(image string) (fnsdk.ResourceListProcessor, bool) {
s.mu.RLock()
defer s.mu.RUnlock()
baseName := util.GetImageName(image)
tag := util.GetImageTag(image)
entry, found := s.builtInExecutorCache[baseName]
prefixToCheck := util.GetImageRepository(image)
parsedImage := imageutil.Parse(image)
entry, found := s.builtInExecutorCache[parsedImage.BaseName]
prefixToCheck := parsedImage.Prefix()
if prefixToCheck == "" {
prefixToCheck = s.defaultImagePrefix
}
if slices.Contains(entry.Tags, tag) {
if slices.Contains(entry.Tags, parsedImage.Tag) {
regex := regexp.MustCompile(entry.PrefixRegex)
if regex.MatchString(prefixToCheck) {
return entry.Process, found
Expand Down
2 changes: 1 addition & 1 deletion func/internal/executableevaluator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func (e *executableEvaluator) EvaluateFunction(ctx context.Context, req *pb.Eval
}
selectedBinary = binary
} else {
klog.Infof("Image tag is empty, using the image with explicit tag: %q", req.Image)
klog.V(2).Infof("Image tag is empty, using the image with explicit tag: %q", req.Image)
binary, exists := e.FunctionConfigStore.GetBinaryFromCache(req.Image)
if !exists {
return nil, &fn.NotFoundError{
Expand Down
36 changes: 19 additions & 17 deletions func/internal/executableevaluator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,18 @@ package internal

import (
"bytes"
"flag"
"fmt"
"os"
"path/filepath"
"testing"

kptfilev1 "github.com/kptdev/kpt/api/kptfile/v1"
"github.com/kptdev/kpt/pkg/fn"
configapi "github.com/kptdev/porch/api/porchconfig/v1alpha1"
"github.com/kptdev/porch/controllers/functionconfigs/reconciler"
pb "github.com/kptdev/porch/func/evaluator"
"github.com/kptdev/porch/pkg/util"
imageutil "github.com/kptdev/porch/pkg/util/image"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/klog/v2"
Expand Down Expand Up @@ -87,6 +89,10 @@ func TestNewExecutableEvaluator(t *testing.T) {
}

func TestEvaluateExecutableFunction(t *testing.T) {
flagSet := flag.NewFlagSet("log-level", flag.ContinueOnError)
klog.InitFlags(flagSet)
_ = flagSet.Parse([]string{"--v", "3"})

const tempCacheDir = "/tmp/func_cache"
t.Run("invalid semver constraint will cause function not found error", func(t *testing.T) {
ctx := t.Context()
Expand All @@ -96,7 +102,7 @@ func TestEvaluateExecutableFunction(t *testing.T) {

req := &pb.EvaluateFunctionRequest{
ResourceList: []byte("req-rl"),
Image: util.ImageJoin(defaultKRMImagePrefix, testImageName),
Image: imageutil.Join(defaultKRMImagePrefix, testImageName),
Tag: ">> 0.1.3 < 0.2.0", // Invalid semver constraint, '>>' is not a valid operator
}

Expand All @@ -118,7 +124,7 @@ func TestEvaluateExecutableFunction(t *testing.T) {
req := &pb.EvaluateFunctionRequest{
ResourceList: []byte("req-rl"),
// This image is not included in the config.yaml -> function not found
Image: util.ImageJoin(defaultKRMImagePrefix, testImageName),
Image: imageutil.Join(defaultKRMImagePrefix, testImageName),
Tag: "> 0.1.3 < 0.2.0", // This is a valid semver constraint syntax
}

Expand All @@ -135,7 +141,7 @@ func TestEvaluateExecutableFunction(t *testing.T) {

req := &pb.EvaluateFunctionRequest{
ResourceList: []byte("req-rl"),
Image: util.ImageJoin(defaultKRMImagePrefix, setImageFunction),
Image: imageutil.Join(defaultKRMImagePrefix, setImageFunction),
Tag: "> 0.1.3 < 0.2.0",
}

Expand All @@ -152,7 +158,7 @@ func TestEvaluateExecutableFunction(t *testing.T) {

req := &pb.EvaluateFunctionRequest{
ResourceList: []byte("req-rl"),
Image: util.ImageJoin(defaultKRMImagePrefix, setImageFunction),
Image: imageutil.Join(defaultKRMImagePrefix, setImageFunction),
Tag: ">= 0.1.2 < 0.2.0",
}

Expand All @@ -169,7 +175,7 @@ func TestEvaluateExecutableFunction(t *testing.T) {
tmpDir := t.TempDir()

// Create a simple test executable that echoes input as a valid KRM function
testBinary := util.ImageJoin(tmpDir, setImageFunction)
testBinary := filepath.Join(tmpDir, setImageFunction)
const testScript = `#!/bin/sh
# Emulating the KRM function execution by running this shell script
cat
Expand All @@ -192,7 +198,7 @@ items: []
// We expect v0.1.3 to be selected as it's the greatest version
req := &pb.EvaluateFunctionRequest{
ResourceList: []byte(resourceList),
Image: util.ImageJoin(defaultKRMImagePrefix, setImageFunction),
Image: imageutil.Join(defaultKRMImagePrefix, setImageFunction),
Tag: ">= 0.1.2 < 0.2.0",
}

Expand Down Expand Up @@ -221,9 +227,7 @@ items: []
assert.NotNil(t, resp)

// Verify the klog message contains the expected version selection
assert.Contains(t, logOutput, `Selected image "ghcr.io/kptdev/krm-functions-catalog/set-image:v0.1.3"`)
assert.Contains(t, logOutput, `(version "0.1.3")`)
assert.Contains(t, logOutput, `for request "ghcr.io/kptdev/krm-functions-catalog/set-image"`)
assert.Contains(t, logOutput, `Selected tag "v0.1.3"`)
})
t.Run("successful function execution with explicit tagging", func(t *testing.T) {
ctx := t.Context()
Expand All @@ -232,7 +236,7 @@ items: []
tmpDir := t.TempDir()

// Create a simple test executable that echoes input as a valid KRM function
testBinary := util.ImageJoin(tmpDir, setImageFunction)
testBinary := filepath.Join(tmpDir, setImageFunction)
const testScript = `#!/bin/sh
# Emulating the KRM function execution by running this shell script
cat
Expand All @@ -254,7 +258,7 @@ items: []
// Explicit tagging
req := &pb.EvaluateFunctionRequest{
ResourceList: []byte(resourceList),
Image: util.ImageJoin(defaultKRMImagePrefix, setImageFunction) + ":v0.1.3",
Image: imageutil.Join(defaultKRMImagePrefix, setImageFunction) + ":v0.1.3",
}

// Capture klog output by redirecting stderr
Expand Down Expand Up @@ -291,7 +295,7 @@ items: []
tmpDir := t.TempDir()

// Create a simple test executable that echoes input as a valid KRM function
testBinary := util.ImageJoin(tmpDir, setImageFunction)
testBinary := filepath.Join(tmpDir, setImageFunction)
const testScript = `#!/bin/sh
# Emulating the KRM function execution by running this shell script
cat
Expand All @@ -312,7 +316,7 @@ items: []

req := &pb.EvaluateFunctionRequest{
ResourceList: []byte(resourceList),
Image: util.ImageJoin(defaultKRMImagePrefix, setImageFunction) + ":v0.0.1",
Image: imageutil.Join(defaultKRMImagePrefix, setImageFunction) + ":v0.0.1",
Tag: ">= 0.1.2 < 0.2.0",
}

Expand Down Expand Up @@ -341,8 +345,6 @@ items: []
assert.NotNil(t, resp)

// Verify the klog message contains the expected version selection
assert.Contains(t, logOutput, `Selected image "ghcr.io/kptdev/krm-functions-catalog/set-image:v0.1.3"`)
assert.Contains(t, logOutput, `(version "0.1.3")`)
assert.Contains(t, logOutput, `for request "ghcr.io/kptdev/krm-functions-catalog/set-image"`)
assert.Contains(t, logOutput, `Selected tag "v0.1.3"`)
})
}
15 changes: 5 additions & 10 deletions func/internal/podcachemanager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,12 @@ import (
"fmt"
"net"
"slices"
"strings"
"sync/atomic"
"time"

configapi "github.com/kptdev/porch/api/porchconfig/v1alpha1"
fnconf "github.com/kptdev/porch/controllers/functionconfigs/reconciler"
"github.com/kptdev/porch/pkg/util"
imageutil "github.com/kptdev/porch/pkg/util/image"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/util/wait"
Expand Down Expand Up @@ -141,7 +140,7 @@ func (pcm *podCacheManager) podCacheManager(ctx context.Context) {

fn.pods = append(fn.pods, NewPodInfo(req.responseCh))

functionConfig, exists := pcm.functionConfigMap.GetFunctionConfig(util.GetImageName(req.image))
functionConfig, exists := pcm.functionConfigMap.GetFunctionConfig(imageutil.Parse(req.image).BaseName)
if !exists {
functionConfig = &configapi.FunctionConfig{}
}
Expand Down Expand Up @@ -259,7 +258,7 @@ func (pcm *podCacheManager) podCacheManager(ctx context.Context) {
// If the image is present in the configMap, it returns the specific parameters for that image.
// Otherwise, it falls back to the global defaults (pcm.podTTL, pcm.maxWaitlistLength, pcm.maxParallelPodsPerFunction).
func (pcm *podCacheManager) getParamsForImage(image string) (ttl time.Duration, maxWaitlist, maxPods int) {
if entry, ok := pcm.functionConfigMap.GetFunctionConfig(util.GetImageName(image)); ok && entry.Spec.PodExecutor != nil {
if entry, ok := pcm.functionConfigMap.GetFunctionConfig(imageutil.Parse(image).BaseName); ok && entry.Spec.PodExecutor != nil {
podExecutorConfig := entry.Spec.PodExecutor
parsedTTL := podExecutorConfig.TimeToLive.Duration
if parsedTTL <= 0 {
Expand Down Expand Up @@ -374,9 +373,9 @@ func (pcm *podCacheManager) warmupCache(defaultImagePrefix string) error {
image = fmt.Sprintf("%s:%s", entry.Spec.Image, entry.Spec.PodExecutor.Tags[0])
}
if len(entry.Spec.Prefixes) > 0 && entry.Spec.Prefixes[0] != "" {
image = ImageJoin(entry.Spec.Prefixes[0], image)
image = imageutil.Join(entry.Spec.Prefixes[0], image)
} else {
image = ImageJoin(defaultImagePrefix, image)
image = imageutil.Join(defaultImagePrefix, image)
}
image = pcm.podManager.imageResolver(image)
fn := pcm.FunctionInfo(image)
Expand All @@ -397,10 +396,6 @@ func (pcm *podCacheManager) warmupCache(defaultImagePrefix string) error {
return nil
}

func ImageJoin(prefix, image string) string {
return strings.TrimRight(prefix, "/") + "/" + strings.TrimLeft(image, "/")
}

// findBestPod returns with the index of the best pod for the given function.
// It uses round-robin among pods with equal load to ensure even distribution.
// If there are no suitable pods, it returns with -1.
Expand Down
6 changes: 3 additions & 3 deletions pkg/engine/builtinruntime.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
"github.com/kptdev/kpt/pkg/lib/kptops"
fnsdk "github.com/kptdev/krm-functions-sdk/go/fn"
"github.com/kptdev/porch/controllers/functionconfigs/reconciler"
"github.com/kptdev/porch/pkg/util"
imageutil "github.com/kptdev/porch/pkg/util/image"
regclientref "github.com/regclient/regclient/types/ref"
"k8s.io/klog/v2"
)
Expand Down Expand Up @@ -64,12 +64,12 @@ func (br *builtinRuntime) GetRunner(ctx context.Context, funct *kptfilev1.Functi
funct.Image = stripped
}
}
baseName := util.GetImageName(funct.Image)
baseName := imageutil.Parse(funct.Image).BaseName

builtinEntry := cache[baseName]
cacheKeys := make([]string, 0, len(builtinEntry.Tags))
cacheKeys = append(cacheKeys, builtinEntry.Tags...)
_, err = util.FindBestSemverMatch(funct.Tag, funct.Image, cacheKeys)
_, err = imageutil.FindBestSemverMatch(funct.Tag, cacheKeys)
if err != nil {
return nil, &fn.NotFoundError{
Function: kptfilev1.Function{Image: funct.Image},
Expand Down
17 changes: 9 additions & 8 deletions pkg/engine/builtinruntime_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package engine

import (
"bytes"
"flag"
"os"
"path/filepath"
"testing"
Expand All @@ -25,7 +26,7 @@ import (
fnsdk "github.com/kptdev/krm-functions-sdk/go/fn"
configapi "github.com/kptdev/porch/api/porchconfig/v1alpha1"
"github.com/kptdev/porch/controllers/functionconfigs/reconciler"
"github.com/kptdev/porch/pkg/util"
imageutil "github.com/kptdev/porch/pkg/util/image"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -115,6 +116,10 @@ func TestNewBuiltinRuntime(t *testing.T) {
}

func TestBuiltinRuntime(t *testing.T) {
flagSet := flag.NewFlagSet("log-level", flag.ContinueOnError)
klog.InitFlags(flagSet)
_ = flagSet.Parse([]string{"--v", "3"})

t.Run("invalid semver constraint syntax", func(t *testing.T) {
ctx := t.Context()
functionConfig := configapi.FunctionConfig{
Expand Down Expand Up @@ -210,7 +215,7 @@ func TestBuiltinRuntime(t *testing.T) {
functionConfigStore.UpdateExecCache(setNamespaceFunction, &functionConfig)
br := newBuiltinRuntime(functionConfigStore)
funct := &kptfilev1.Function{
Image: util.ImageJoin(defaultKRMImagePrefix, setNamespaceFunction) + ":v0.4.2",
Image: imageutil.Join(defaultKRMImagePrefix, setNamespaceFunction) + ":v0.4.2",
// Image is explicitly tagged with v0.4.2, however,
// there is no function with this explicit tag in the cache
}
Expand Down Expand Up @@ -303,9 +308,7 @@ functionConfig:
logOutput := logBuffer.String()

// Verify the klog message contains the expected version selection
assert.Contains(t, logOutput, `Selected image "ghcr.io/kptdev/krm-functions-catalog/set-namespace:v0.4.1"`)
assert.Contains(t, logOutput, `version "0.4.1"`)
assert.Contains(t, logOutput, `for request "ghcr.io/kptdev/krm-functions-catalog/set-namespace"`)
assert.Contains(t, logOutput, `Selected tag "v0.4.1"`)

reader := bytes.NewReader([]byte(`apiVersion: config.kubernetes.io/v1alpha1
kind: ResourceList
Expand Down Expand Up @@ -441,9 +444,7 @@ functionConfig:
logOutput := logBuffer.String()

// Verify the klog message contains the expected version selection
assert.Contains(t, logOutput, `Selected image "ghcr.io/kptdev/krm-functions-catalog/set-namespace:v0.4.1"`)
assert.Contains(t, logOutput, `(version "0.4.1")`)
assert.Contains(t, logOutput, `for request "ghcr.io/kptdev/krm-functions-catalog/set-namespace"`)
assert.Contains(t, logOutput, `Selected tag "v0.4.1"`)

reader := bytes.NewReader([]byte(`apiVersion: config.kubernetes.io/v1alpha1
kind: ResourceList
Expand Down
Loading
Loading