From f09b493bcae9863d683c44f2974e7c6da3986c66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=B3zes=20L=C3=A1szl=C3=B3=20M=C3=A1t=C3=A9?= Date: Fri, 3 Jul 2026 12:12:09 +0200 Subject: [PATCH 1/9] image parsing util MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Mózes László Máté --- .../reconciler/functionconfigreconciler.go | 25 +- func/internal/executableevaluator_test.go | 22 +- func/internal/podcachemanager.go | 6 +- pkg/engine/builtinruntime.go | 6 +- pkg/engine/builtinruntime_test.go | 4 +- pkg/util/image/image.go | 119 ++++ pkg/util/image/image_test.go | 225 +++++++ pkg/util/image/regex.go | 40 ++ pkg/util/image/types.go | 83 +++ pkg/util/util.go | 85 --- pkg/util/util_test.go | 603 +----------------- 11 files changed, 499 insertions(+), 719 deletions(-) create mode 100644 pkg/util/image/image.go create mode 100644 pkg/util/image/image_test.go create mode 100644 pkg/util/image/regex.go create mode 100644 pkg/util/image/types.go diff --git a/controllers/functionconfigs/reconciler/functionconfigreconciler.go b/controllers/functionconfigs/reconciler/functionconfigreconciler.go index b7a225002..69008c4ed 100644 --- a/controllers/functionconfigs/reconciler/functionconfigreconciler.go +++ b/controllers/functionconfigs/reconciler/functionconfigreconciler.go @@ -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" @@ -181,13 +181,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 } @@ -200,15 +200,15 @@ 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 } @@ -236,14 +236,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 diff --git a/func/internal/executableevaluator_test.go b/func/internal/executableevaluator_test.go index c5e36e906..91032c278 100644 --- a/func/internal/executableevaluator_test.go +++ b/func/internal/executableevaluator_test.go @@ -25,7 +25,7 @@ import ( 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" @@ -96,7 +96,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 } @@ -118,7 +118,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 } @@ -135,7 +135,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", } @@ -152,7 +152,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", } @@ -169,7 +169,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 := imageutil.Join(tmpDir, setImageFunction) const testScript = `#!/bin/sh # Emulating the KRM function execution by running this shell script cat @@ -192,7 +192,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", } @@ -232,7 +232,7 @@ items: [] tmpDir := t.TempDir() // Create a simple test executable that echoes input as a valid KRM function - testBinary := util.ImageJoin(tmpDir, setImageFunction) + testBinary := imageutil.Join(tmpDir, setImageFunction) const testScript = `#!/bin/sh # Emulating the KRM function execution by running this shell script cat @@ -254,7 +254,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 @@ -291,7 +291,7 @@ items: [] tmpDir := t.TempDir() // Create a simple test executable that echoes input as a valid KRM function - testBinary := util.ImageJoin(tmpDir, setImageFunction) + testBinary := imageutil.Join(tmpDir, setImageFunction) const testScript = `#!/bin/sh # Emulating the KRM function execution by running this shell script cat @@ -312,7 +312,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", } diff --git a/func/internal/podcachemanager.go b/func/internal/podcachemanager.go index 2d5382fa7..3dda4c164 100644 --- a/func/internal/podcachemanager.go +++ b/func/internal/podcachemanager.go @@ -25,7 +25,7 @@ import ( 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" @@ -141,7 +141,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{} } @@ -259,7 +259,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 { diff --git a/pkg/engine/builtinruntime.go b/pkg/engine/builtinruntime.go index d9bc39694..da3b3e731 100644 --- a/pkg/engine/builtinruntime.go +++ b/pkg/engine/builtinruntime.go @@ -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" ) @@ -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}, diff --git a/pkg/engine/builtinruntime_test.go b/pkg/engine/builtinruntime_test.go index 613a41603..601adf448 100644 --- a/pkg/engine/builtinruntime_test.go +++ b/pkg/engine/builtinruntime_test.go @@ -25,7 +25,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" @@ -210,7 +210,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 } diff --git a/pkg/util/image/image.go b/pkg/util/image/image.go new file mode 100644 index 000000000..c0e40030d --- /dev/null +++ b/pkg/util/image/image.go @@ -0,0 +1,119 @@ +// Copyright 2026 The kpt Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package image + +import ( + "fmt" + "slices" + "strings" + + "github.com/Masterminds/semver/v3" + "k8s.io/klog/v2" +) + +// FindBestSemverMatch selects the tag whose semver value best satisfies the constraint. +// It returns the highest matching tag from cachedTags (e.g. "v1.2.3"). +func FindBestSemverMatch(constraint string, cachedTags []string) (string, error) { + c, err := semver.NewConstraint(constraint) + if err != nil { + return "", fmt.Errorf("invalid semver constraint %q: %w", constraint, err) + } + + type candidate struct { + key string + version *semver.Version + } + + var matches []candidate + for _, tag := range cachedTags { + v, err := semver.NewVersion(tag) + if err != nil { + klog.V(2).Infof("Failed to parse version %q: %v", tag, err) + continue + } + + if c.Check(v) { + matches = append(matches, candidate{key: tag, version: v}) + } + } + + if len(matches) == 0 { + return "", fmt.Errorf("no tag matching constraint %q found among %+v", constraint, cachedTags) + } + + slices.SortFunc(matches, func(a, b candidate) int { + return a.version.Compare(b.version) + }) + + selected := matches[len(matches)-1] + klog.V(3).Infof("Selected tag %q", selected.key) + + return selected.key, nil +} + +func Join(parts ...string) string { + if len(parts) == 0 { + return "" + } + + sb := strings.Builder{} + sb.WriteString(strings.Trim(parts[0], "/")) + + for _, part := range parts[1:] { + part = strings.Trim(part, "/") + sb.WriteString("/") + sb.WriteString(part) + } + return sb.String() +} + +// Parse creates a ParsedImage object from the full image name. +// Does not guarantee that the input string is a valid reference, unlike regclientref.New(). +func Parse(fullImageName string) ParsedImage { + output := ParsedImage{Original: fullImageName} + + firstSlash := strings.Index(fullImageName, "/") + lastSlash := strings.LastIndex(fullImageName, "/") + + if firstSlash != -1 { + str := fullImageName[:firstSlash] + if registryRE.MatchString(str) { + output.Registry = strings.TrimRight(str, "/") + fullImageName = fullImageName[firstSlash+1:] + + lastSlash = strings.LastIndex(fullImageName, "/") + if lastSlash != -1 { + output.SubPath = strings.Trim(fullImageName[:lastSlash], "/") + fullImageName = fullImageName[lastSlash+1:] + } + } else { + output.SubPath = strings.Trim(fullImageName[:lastSlash], "/") + fullImageName = fullImageName[lastSlash+1:] + } + } + + if lastAt := strings.LastIndex(fullImageName, "@"); lastAt != -1 { + output.Digest = fullImageName[lastAt+1:] + fullImageName = fullImageName[:lastAt] + } + + if lastColon := strings.LastIndex(fullImageName, ":"); lastColon != -1 { + output.Tag = fullImageName[lastColon+1:] + fullImageName = fullImageName[:lastColon] + } + + output.BaseName = strings.TrimLeft(fullImageName, "/") + return output +} diff --git a/pkg/util/image/image_test.go b/pkg/util/image/image_test.go new file mode 100644 index 000000000..079e46873 --- /dev/null +++ b/pkg/util/image/image_test.go @@ -0,0 +1,225 @@ +// Copyright 2026 The kpt Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package image + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFindBestSemverMatch(t *testing.T) { + testCases := map[string]struct { + constraint string + tags []string + expected string + expectedErr string + }{ + "selects highest matching version": { + constraint: ">= 0.4.0 < 0.5.0", + tags: []string{ + "v0.4.1", + "v0.4", + "@sha256:abcdef123456", + }, + expected: "v0.4.1", + }, + "exact version match": { + constraint: "0.1.1", + tags: []string{ + "v0.1.1", + "v0.1", + }, + expected: "v0.1.1", + }, + "no matching version for valid constraint": { + constraint: "> 1.0.0", + tags: []string{ + "v0.1.1", + "v0.1", + }, + expectedErr: "no tag matching", + }, + "invalid semver constraint": { + constraint: ">> 1.0.0", + tags: []string{ + "v1.1.0", + }, + expectedErr: "invalid semver constraint", + }, + "skips sha256-tagged entries": { + constraint: ">= 0.4.0", + tags: []string{ + "v0.4.1", + "v0.4", + "@sha256:abcdef123456", + }, + expected: "v0.4.1", + }, + "matches without registry prefix": { + constraint: ">= 0.4.0", + tags: []string{ + "v0.4.1", + "v0.4", + "@sha256:abcdef123456", + }, + expected: "v0.4.1", + }, + "empty cache keys": { + constraint: ">= 0.1.0", + tags: []string{}, + expectedErr: "no tag matching", + }, + "selects greatest from multiple matches": { + constraint: ">= 1.0.0 < 2.0.0", + tags: []string{ + "v1.0.0", + "v1.1.0", + "v1.2.0", + "v2.0.0", + }, + expected: "v1.2.0", + }, + "skips entries with unparseable versions": { + constraint: ">= 1.0.0", + tags: []string{ + "v1.0.0", + "vnotaversion", + }, + expected: "v1.0.0", + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + best, err := FindBestSemverMatch(tc.constraint, tc.tags) + if tc.expectedErr != "" { + assert.ErrorContains(t, err, tc.expectedErr) + } else { + require.NoError(t, err) + assert.Equal(t, tc.expected, best) + } + }) + } +} + +func TestImageParse(t *testing.T) { + const ( + registry = "ghcr.io" + registryWithPort = "my-registry.com:5000" + subpath = "kptdev/krm-functions-catalog" + image = "apply-setters" + tag = "v0.2.3" + digest = "sha256:7d89a74f106241391f687fc2985c8e6de597bb21f0d0014def5edc730618d9cc" + ) + + testCases := map[string]struct { + input string + want ParsedImage + }{ + "empty": { + input: "", + want: ParsedImage{}, + }, + "base name only": { + input: image, + want: ParsedImage{ + Original: image, + BaseName: image, + }, + }, + "tag only": { + input: fmt.Sprintf("%s:%s", image, tag), + want: ParsedImage{ + Original: fmt.Sprintf("%s:%s", image, tag), + BaseName: image, + Tag: tag, + }, + }, + "registry no path": { + input: fmt.Sprintf("%s/%s", registry, image), + want: ParsedImage{ + Original: fmt.Sprintf("%s/%s", registry, image), + Registry: registry, + BaseName: image, + }, + }, + "registry with path": { + input: fmt.Sprintf("%s/%s:%s", subpath, image, tag), + want: ParsedImage{ + Original: fmt.Sprintf("%s/%s:%s", subpath, image, tag), + SubPath: subpath, + BaseName: image, + Tag: tag, + }, + }, + "fully qualified, no digest": { + input: fmt.Sprintf("%s/%s/%s:%s", registry, subpath, image, tag), + want: ParsedImage{ + Original: fmt.Sprintf("%s/%s/%s:%s", registry, subpath, image, tag), + Registry: registry, + SubPath: subpath, + BaseName: image, + Tag: tag, + }, + }, + "digest without tag": { + input: fmt.Sprintf("%s@%s", image, digest), + want: ParsedImage{ + Original: fmt.Sprintf("%s@%s", image, digest), + BaseName: image, + Digest: digest, + }, + }, + "tag and digest": { + input: fmt.Sprintf("%s:%s@%s", image, tag, digest), + want: ParsedImage{ + Original: fmt.Sprintf("%s:%s@%s", image, tag, digest), + BaseName: image, + Tag: tag, + Digest: digest, + }, + }, + "registry with port": { + input: fmt.Sprintf("%s/%s/%s:%s", registryWithPort, subpath, image, tag), + want: ParsedImage{ + Original: fmt.Sprintf("%s/%s/%s:%s", registryWithPort, subpath, image, tag), + Registry: registryWithPort, + SubPath: subpath, + BaseName: image, + Tag: tag, + }, + }, + "digest with nested repository": { + input: fmt.Sprintf("%s/%s/%s@%s", registry, subpath, image, digest), + want: ParsedImage{ + Original: fmt.Sprintf("%s/%s/%s@%s", registry, subpath, image, digest), + Registry: registry, + SubPath: subpath, + BaseName: image, + Digest: digest, + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + got := Parse(tc.input) + assert.Equal(t, tc.want, got) + }) + } +} diff --git a/pkg/util/image/regex.go b/pkg/util/image/regex.go new file mode 100644 index 000000000..68ef87d48 --- /dev/null +++ b/pkg/util/image/regex.go @@ -0,0 +1,40 @@ +// Copyright 2020 The regclient Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// taken from https://github.com/regclient/regclient/blob/v0.11.5/types/ref/ref.go + +package image + +import "regexp" + +var ( + hostPartS = `(?:[a-zA-Z0-9](?:[a-zA-Z0-9-]*[a-zA-Z0-9])?)` + portS = `(?:` + regexp.QuoteMeta(`:`) + `[0-9]+)` + ipv6PartS = `(?:[0-9a-fA-F]{1,4}:){0,7}[0-9a-fA-F]{1,4}` + ipv6S = `(?:` + regexp.QuoteMeta(`[`) + `(?:` + + ipv6PartS + `|` + // uncompressed + regexp.QuoteMeta(`::`) + ipv6PartS + `|` + // prefix compressed + ipv6PartS + regexp.QuoteMeta(`::`) + ipv6PartS + `|` + // middle compressed + ipv6PartS + regexp.QuoteMeta(`::`) + // suffix compressed + `)` + regexp.QuoteMeta(`]`) + `)` + localhostS = `localhost` + hostDomainS = `(?:` + hostPartS + `(?:(?:` + regexp.QuoteMeta(`.`) + hostPartS + `)+` + regexp.QuoteMeta(`.`) + `?|` + regexp.QuoteMeta(`.`) + `))` + hostUpperS = `(?:[a-zA-Z0-9]*[A-Z][a-zA-Z0-9-]*[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[A-Z][a-zA-Z0-9]*)` + registryS = `(?:` + + `(?:` + hostDomainS + `|` + hostUpperS + `|` + ipv6S + `|` + localhostS + `)` + portS + `?|` + // name with dotted domain, upper case, or IPv6 with optional port + hostPartS + portS + // a short name with required port + `)` + + registryRE = regexp.MustCompile(`^(` + registryS + `)$`) +) diff --git a/pkg/util/image/types.go b/pkg/util/image/types.go new file mode 100644 index 000000000..b447ad214 --- /dev/null +++ b/pkg/util/image/types.go @@ -0,0 +1,83 @@ +// Copyright 2026 The kpt Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package image + +import "strings" + +// ParsedImage is a structured representation of a container image reference, +// broken into registry, sub-path, base name, tag, and digest components. +type ParsedImage struct { + // The registry part of the image name without trailing slash. + // Example: ghcr.io + Registry string + // The part of the image name between Registry and BaseName without leading or trailing slashes. + // Example: kptdev/krm-functions-catalog + SubPath string + // The last part of the image name, after all slashes, without the leading slash. + // Example: apply-setters + BaseName string + // The tag of the image without the leading colon. + // Example: v0.2.3 + Tag string + // The sha256 digest of the image, without the leading @, but containing the `sha256` prefix. + // Example: sha256:7d89a74f106241391f687fc2985c8e6de597bb21f0d0014def5edc730618d9cc + Digest string + // Original contains the unparsed image name. Intended for testing. + // Should be the same as the output of Full(). + Original string +} + +// Full reconstructs the full image name from the parsed parts. +func (p *ParsedImage) Full() string { + sb := strings.Builder{} + + if p.Registry != "" { + sb.WriteString(p.Registry) + sb.WriteString("/") + } + + if p.SubPath != "" { + sb.WriteString(p.SubPath) + sb.WriteString("/") + } + + sb.WriteString(p.BaseName) + if p.Tag != "" { + sb.WriteString(":") + sb.WriteString(p.Tag) + } + + if p.Digest != "" { + sb.WriteString("@") + sb.WriteString(p.Digest) + } + + return sb.String() +} + +// Prefix returns the part before BaseName with *no* trailing slash. +func (p *ParsedImage) Prefix() string { + sb := strings.Builder{} + + if p.Registry != "" { + sb.WriteString(p.Registry) + sb.WriteString("/") + } + if p.SubPath != "" { + sb.WriteString(p.SubPath) + } + + return sb.String() +} diff --git a/pkg/util/util.go b/pkg/util/util.go index fa9833022..2f0361f16 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -27,7 +27,6 @@ import ( "slices" "strings" - semver "github.com/Masterminds/semver/v3" "github.com/google/uuid" "github.com/kptdev/krm-functions-sdk/go/fn/kptfileapi" porchapi "github.com/kptdev/porch/api/porch" @@ -332,90 +331,6 @@ func RetryOnErrorConditional(retries int, shouldRetryFunc func(error) bool, f fu return err } -// FindBestSemverMatch selects the highest semver tag from cachedTags that satisfies constraint. -// It returns the selected tag (e.g. "v1.2.3") for the given imageName (used for logging only). -func FindBestSemverMatch(constraint string, imageName string, cachedTags []string) (string, error) { - c, err := semver.NewConstraint(constraint) - if err != nil { - return "", fmt.Errorf("invalid semver constraint %q: %w", constraint, err) - } - - type candidate struct { - key string - version *semver.Version - } - - var matches []candidate - for _, tag := range cachedTags { - v, err := semver.NewVersion(tag) - if err != nil { - klog.Infof("Failed to parse version %q from cached image %q: %v", tag, imageName, err) - continue - } - - if c.Check(v) { - matches = append(matches, candidate{key: tag, version: v}) - } - } - - if len(matches) == 0 { - klog.Infof("Image %q with constraint %q is not found in the cache", imageName, constraint) - return "", fmt.Errorf("no image matching %q with constraint %q found in the cache", imageName, constraint) - } - - slices.SortFunc(matches, func(a, b candidate) int { - return a.version.Compare(b.version) - }) - - selected := matches[len(matches)-1] - klog.Infof("Selected image %q (version %q) for request %q", - imageName+":"+selected.key, selected.version, imageName) - - return selected.key, nil -} - -func GetImageName(image string) string { - if i := strings.Index(image, "@"); i != -1 { - image = image[:i] - } - - if i := strings.LastIndex(image, ":"); i != -1 && !strings.Contains(image[i+1:], "/") { - image = image[:i] - } - - if i := strings.LastIndex(image, "/"); i != -1 { - image = image[i+1:] - } - return image -} - -func GetImageRepository(image string) string { - lastSlash := strings.LastIndex(image, "/") - if lastSlash == -1 { - return "" - } - return image[:lastSlash] -} - -func GetImageTag(image string) string { - if strings.Contains(image, "@sha256:") { - return "" - } - - lastSlash := strings.LastIndex(image, "/") - lastColon := strings.LastIndex(image, ":") - - if lastColon == -1 || lastColon < lastSlash { - return "latest" - } - - return image[lastColon+1:] -} - -func ImageJoin(prefix, image string) string { - return strings.TrimRight(prefix, "/") + "/" + strings.TrimLeft(image, "/") -} - func GetRepoPackageRefFromUpstream(upstream *kptfileapi.Upstream) (upstreamRepoSpec *configapi.RepositorySpec, upstreamPackage, upstreamRef string, isManagedReference bool, err error) { isManagedReference = false diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index 0af0fa04f..93726dc19 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -1,4 +1,4 @@ -// Copyright 2022,2025 The kpt Authors +// Copyright 2022,2025-2026 The kpt Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -22,10 +22,7 @@ import ( "strings" "testing" - "github.com/kptdev/krm-functions-sdk/go/fn/kptfileapi" - configapi "github.com/kptdev/porch/api/porchconfig/v1alpha1" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "gopkg.in/yaml.v2" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -476,601 +473,3 @@ func getPartErrMsg(errorSlice []string, start string) string { return "" } - -func TestFindBestSemverMatch(t *testing.T) { - cacheKeys := []string{ - "ghcr.io/kptdev/krm-functions-catalog/set-namespace:v0.4.1", - "ghcr.io/kptdev/krm-functions-catalog/set-namespace:v0.4", - "ghcr.io/kptdev/krm-functions-catalog/set-namespace@sha256:abcdef123456", - "ghcr.io/kptdev/krm-functions-catalog/apply-replacements:v0.1.1", - "ghcr.io/kptdev/krm-functions-catalog/apply-replacements:v0.1", - "ghcr.io/kptdev/krm-functions-catalog/starlark:v0.4.3", - "set-namespace:v0.4.1", - } - - t.Run("selects highest matching version", func(t *testing.T) { - cacheKeys := []string{ - "v0.4.1", - "v0.4", - "@sha256:abcdef123456", - } - key, err := FindBestSemverMatch( - ">= 0.4.0 < 0.5.0", - "ghcr.io/kptdev/krm-functions-catalog/set-namespace", - cacheKeys, - ) - assert.NoError(t, err) - assert.Equal(t, "v0.4.1", key) - }) - - t.Run("exact version match", func(t *testing.T) { - cacheKeys := []string{ - "v0.1.1", - "v0.1", - } - key, err := FindBestSemverMatch( - "0.1.1", - "ghcr.io/kptdev/krm-functions-catalog/apply-replacements", - cacheKeys, - ) - assert.NoError(t, err) - assert.Equal(t, "v0.1.1", key) - }) - - t.Run("no matching version for valid constraint", func(t *testing.T) { - _, err := FindBestSemverMatch( - "> 1.0.0", - "ghcr.io/kptdev/krm-functions-catalog/set-namespace", - cacheKeys, - ) - assert.Error(t, err) - assert.Contains(t, err.Error(), "no image matching") - }) - - t.Run("image not in cache", func(t *testing.T) { - _, err := FindBestSemverMatch( - ">= 0.1.0", - "ghcr.io/kptdev/krm-functions-catalog/nonexistent", - cacheKeys, - ) - assert.Error(t, err) - assert.Contains(t, err.Error(), "no image matching") - }) - - t.Run("invalid semver constraint", func(t *testing.T) { - _, err := FindBestSemverMatch( - ">> 1.0.0", - "ghcr.io/kptdev/krm-functions-catalog/set-namespace", - cacheKeys, - ) - assert.Error(t, err) - assert.Contains(t, err.Error(), "invalid semver constraint") - }) - - t.Run("skips sha256-tagged entries", func(t *testing.T) { - cacheKeys := []string{ - "v0.4.1", - "v0.4", - "@sha256:abcdef123456", - } - key, err := FindBestSemverMatch( - ">= 0.4.0", - "ghcr.io/kptdev/krm-functions-catalog/set-namespace", - cacheKeys, - ) - assert.NoError(t, err) - assert.NotContains(t, key, "@sha256:") - }) - - t.Run("matches without registry prefix", func(t *testing.T) { - cacheKeys := []string{ - "v0.4.1", - "v0.4", - "@sha256:abcdef123456", - } - key, err := FindBestSemverMatch( - ">= 0.4.0", - "set-namespace", - cacheKeys, - ) - assert.NoError(t, err) - assert.Equal(t, "v0.4.1", key) - }) - - t.Run("empty cache keys", func(t *testing.T) { - _, err := FindBestSemverMatch( - ">= 0.1.0", - "ghcr.io/kptdev/krm-functions-catalog/set-namespace", - []string{}, - ) - assert.Error(t, err) - assert.Contains(t, err.Error(), "no image matching") - }) - - t.Run("selects greatest from multiple matches", func(t *testing.T) { - keys := []string{ - "v1.0.0", - "v1.1.0", - "v1.2.0", - "v2.0.0", - } - key, err := FindBestSemverMatch(">= 1.0.0 < 2.0.0", "myimage", keys) - assert.NoError(t, err) - assert.Equal(t, "v1.2.0", key) - }) - - t.Run("skips entries with unparseable versions", func(t *testing.T) { - keys := []string{ - "v1.0.0", - "vnotaversion", - } - key, err := FindBestSemverMatch(">= 1.0.0", "myimage", keys) - assert.NoError(t, err) - assert.Equal(t, "v1.0.0", key) - }) -} - -func TestGetRepoPackageRefFromUpstream(t *testing.T) { - tests := []struct { - name string - upstream *kptfileapi.Upstream - wantRepoSpec *configapi.RepositorySpec - wantPkg string - wantRef string - wantManagedRef bool - wantErr string - }{ - { - name: "nil upstream returns error", - upstream: nil, - wantErr: "upstream does not contain a valid git repository", - }, - { - name: "nil git returns error", - upstream: &kptfileapi.Upstream{Git: nil}, - wantErr: "upstream does not contain a valid git repository", - }, - { - name: "empty repo returns error", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "", - Ref: "mypkg/v1", - Directory: "mypkg", - }, - }, - wantErr: "upstream does not contain a valid git repository", - }, - { - name: "empty directory returns error", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "https://github.com/example/repo.git", - Ref: "mypkg/v1", - Directory: "", - }, - }, - wantErr: "git directory reference", - }, - { - name: "directory with leading slash returns error", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "https://github.com/example/repo.git", - Ref: "mypkg/v1", - Directory: "/mypkg", - }, - }, - wantErr: "git directory reference \"/mypkg\" in upstream is invalid", - }, - { - name: "directory with trailing slash returns error", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "https://github.com/example/repo.git", - Ref: "mypkg/v1", - Directory: "mypkg/", - }, - }, - wantErr: "git directory reference \"mypkg/\" in upstream is invalid", - }, - { - name: "empty ref returns error", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "https://github.com/example/repo.git", - Ref: "", - Directory: "mypkg", - }, - }, - wantErr: "git ref reference", - }, - { - name: "unmanaged ref - ref does not contain directory pattern", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "https://github.com/example/repo.git", - Ref: "main", - Directory: "mypkg", - }, - }, - wantRepoSpec: &configapi.RepositorySpec{ - Type: configapi.RepositoryTypeGit, - Git: &configapi.GitRepository{ - Repo: "https://github.com/example/repo", - Directory: "mypkg", - }, - }, - wantPkg: "mypkg", - wantRef: "main", - wantManagedRef: false, - }, - { - name: "unmanaged ref - arbitrary tag", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "https://github.com/example/repo.git", - Ref: "release-2.0", - Directory: "packages/foo", - }, - }, - wantRepoSpec: &configapi.RepositorySpec{ - Type: configapi.RepositoryTypeGit, - Git: &configapi.GitRepository{ - Repo: "https://github.com/example/repo", - Directory: "packages/foo", - }, - }, - wantPkg: "packages.foo", - wantRef: "release-2.0", - wantManagedRef: false, - }, - { - name: "managed ref - simple package", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "https://github.com/example/repo.git", - Ref: "mypkg/v1", - Directory: "mypkg", - }, - }, - wantRepoSpec: &configapi.RepositorySpec{ - Type: configapi.RepositoryTypeGit, - Git: &configapi.GitRepository{ - Repo: "https://github.com/example/repo", - Directory: "", - }, - }, - wantPkg: "mypkg", - wantRef: "v1", - wantManagedRef: true, - }, - { - name: "managed ref - nested package", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "https://github.com/example/repo.git", - Ref: "packages/mypkg/v2", - Directory: "packages/mypkg", - }, - }, - wantRepoSpec: &configapi.RepositorySpec{ - Type: configapi.RepositoryTypeGit, - Git: &configapi.GitRepository{ - Repo: "https://github.com/example/repo", - Directory: "", - }, - }, - wantPkg: "packages.mypkg", - wantRef: "v2", - wantManagedRef: true, - }, - { - name: "managed ref - with repo directory prefix", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "https://github.com/example/repo.git", - Ref: "blueprints/mypkg/v3", - Directory: "mypkg", - }, - }, - wantRepoSpec: &configapi.RepositorySpec{ - Type: configapi.RepositoryTypeGit, - Git: &configapi.GitRepository{ - Repo: "https://github.com/example/repo", - Directory: "blueprints", - }, - }, - wantPkg: "mypkg", - wantRef: "v3", - wantManagedRef: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - repoSpec, pkg, ref, managedRef, err := GetRepoPackageRefFromUpstream(tt.upstream) - - if tt.wantErr != "" { - assert.ErrorContains(t, err, tt.wantErr) - assert.Nil(t, repoSpec) - } else { - require.NoError(t, err) - assert.Equal(t, tt.wantRepoSpec, repoSpec) - assert.Equal(t, tt.wantPkg, pkg) - assert.Equal(t, tt.wantRef, ref) - assert.Equal(t, tt.wantManagedRef, managedRef) - } - }) - } -} - -func TestGetRepoPackageRefFromUpstreamRealData(t *testing.T) { - tests := []struct { - name string - upstream *kptfileapi.Upstream - wantRepoSpec *configapi.RepositorySpec - wantPkg string - wantRef string - wantManagedRef bool - wantErr string - }{ - { - name: "real test case 1", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "http://172.18.255.204:3000/porch/porch-test.git", - Directory: "upstream-function1", - Ref: "upstream-function1/v1", - }, - }, - wantRepoSpec: &configapi.RepositorySpec{ - Type: configapi.RepositoryTypeGit, - Git: &configapi.GitRepository{ - Repo: "http://172.18.255.204:3000/porch/porch-test", - Directory: "", - }, - }, - wantPkg: "upstream-function1", - wantRef: "v1", - wantManagedRef: true, - }, - { - name: "real test case 2", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "http://172.18.255.204:3000/porch/porch-test.git", - Directory: "r2/r2/r2/r2", - Ref: "r2/r2/r2/r2/r2/r2/v1", - }, - }, - wantRepoSpec: &configapi.RepositorySpec{ - Type: configapi.RepositoryTypeGit, - Git: &configapi.GitRepository{ - Repo: "http://172.18.255.204:3000/porch/porch-test", - Directory: "r2/r2", - }, - }, - wantPkg: "r2.r2.r2.r2", - wantRef: "v1", - wantManagedRef: true, - }, - { - name: "real test case 3", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "http://172.18.255.204:3000/porch/porch-test.git", - Directory: "upstream-function", - Ref: "l1/upstream-function/v1", - }, - }, - wantRepoSpec: &configapi.RepositorySpec{ - Type: configapi.RepositoryTypeGit, - Git: &configapi.GitRepository{ - Repo: "http://172.18.255.204:3000/porch/porch-test", - Directory: "l1", - }, - }, - wantPkg: "upstream-function", - wantRef: "v1", - wantManagedRef: true, - }, - { - name: "real test case 4", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "http://172.18.255.204:3000/porch/porch-test.git", - Directory: "upstream-function", - Ref: "r1/upstream-function/v1", - }, - }, - wantRepoSpec: &configapi.RepositorySpec{ - Type: configapi.RepositoryTypeGit, - Git: &configapi.GitRepository{ - Repo: "http://172.18.255.204:3000/porch/porch-test", - Directory: "r1", - }, - }, - wantPkg: "upstream-function", - wantRef: "v1", - wantManagedRef: true, - }, - { - name: "real test case 5", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "http://172.18.255.204:3000/porch/porch-test.git", - Directory: "upstream-function", - Ref: "l3/l3/l3/upstream-function/v1", - }, - }, - wantRepoSpec: &configapi.RepositorySpec{ - Type: configapi.RepositoryTypeGit, - Git: &configapi.GitRepository{ - Repo: "http://172.18.255.204:3000/porch/porch-test", - Directory: "l3/l3/l3", - }, - }, - wantPkg: "upstream-function", - wantRef: "v1", - wantManagedRef: true, - }, - { - name: "real test case 6", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "http://172.18.255.204:3000/porch/porch-test.git", - Directory: "upstream-dir/upstream-function", - Ref: "l3/l3/l3/upstream-dir/upstream-function/v1", - }, - }, - wantRepoSpec: &configapi.RepositorySpec{ - Type: configapi.RepositoryTypeGit, - Git: &configapi.GitRepository{ - Repo: "http://172.18.255.204:3000/porch/porch-test", - Directory: "l3/l3/l3", - }, - }, - wantPkg: "upstream-dir.upstream-function", - wantRef: "v1", - wantManagedRef: true, - }, - { - name: "real test case 7", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "http://172.18.255.204:3000/porch/porch-test.git", - Directory: "upstream-function", - Ref: "r3/r3/r3/upstream-function/v1", - }, - }, - wantRepoSpec: &configapi.RepositorySpec{ - Type: configapi.RepositoryTypeGit, - Git: &configapi.GitRepository{ - Repo: "http://172.18.255.204:3000/porch/porch-test", - Directory: "r3/r3/r3", - }, - }, - wantPkg: "upstream-function", - wantRef: "v1", - wantManagedRef: true, - }, - { - name: "real test case 8", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "https://github.com/kptdev/kpt.git", - Directory: "package-examples/wordpress", - Ref: "v1.0.0-beta.64", - }, - }, - wantRepoSpec: &configapi.RepositorySpec{ - Type: configapi.RepositoryTypeGit, - Git: &configapi.GitRepository{ - Repo: "https://github.com/kptdev/kpt", - Directory: "package-examples/wordpress", - }, - }, - wantPkg: "package-examples.wordpress", - wantRef: "v1.0.0-beta.64", - wantManagedRef: false, - }, - { - name: "real test case 9", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "https://github.com/kptdev/kpt-samples.git", - Directory: "echo", - Ref: "main", - }, - }, - wantRepoSpec: &configapi.RepositorySpec{ - Type: configapi.RepositoryTypeGit, - Git: &configapi.GitRepository{ - Repo: "https://github.com/kptdev/kpt-samples", - Directory: "echo", - }, - }, - wantPkg: "echo", - wantRef: "main", - wantManagedRef: false, - }, - { - name: "real test case 10", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "https://github.com/kptdev/kpt-samples.git", - Directory: "echo", - Ref: "v0", - }, - }, - wantRepoSpec: &configapi.RepositorySpec{ - Type: configapi.RepositoryTypeGit, - Git: &configapi.GitRepository{ - Repo: "https://github.com/kptdev/kpt-samples", - Directory: "echo", - }, - }, - wantPkg: "echo", - wantRef: "v0", - wantManagedRef: false, - }, - { - name: "real test case 11", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "http://172.18.255.204:3000/porch/porch-test.git", - Directory: "r2/r2/r2/r2/r2/r2", - Ref: "r2/r2/r2/r2/r2/r2/v1", - }, - }, - wantRepoSpec: &configapi.RepositorySpec{ - Type: configapi.RepositoryTypeGit, - Git: &configapi.GitRepository{ - Repo: "http://172.18.255.204:3000/porch/porch-test", - Directory: "", - }, - }, - wantPkg: "r2.r2.r2.r2.r2.r2", - wantRef: "v1", - wantManagedRef: true, - }, - { - name: "real test case 12", - upstream: &kptfileapi.Upstream{ - Git: &kptfileapi.Git{ - Repo: "https://github.com/nephio-project/catalog.git", - Directory: "nephio/core/workload-crds", - Ref: "nephio/core/workload-crds/v3.0.0", - }, - }, - wantRepoSpec: &configapi.RepositorySpec{ - Type: configapi.RepositoryTypeGit, - Git: &configapi.GitRepository{ - Repo: "https://github.com/nephio-project/catalog", - Directory: "", - }, - }, - wantPkg: "nephio.core.workload-crds", - wantRef: "v3.0.0", - wantManagedRef: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - repoSpec, pkg, ref, managedRef, err := GetRepoPackageRefFromUpstream(tt.upstream) - - if tt.wantErr != "" { - assert.ErrorContains(t, err, tt.wantErr) - assert.Nil(t, repoSpec) - } else { - require.NoError(t, err) - assert.Equal(t, tt.wantRepoSpec, repoSpec) - assert.Equal(t, tt.wantPkg, pkg) - assert.Equal(t, tt.wantRef, ref) - assert.Equal(t, tt.wantManagedRef, managedRef) - } - }) - } -} From 183bb3ef0e2d8c1c6761a4a5748f198fa77e25a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=B3zes=20L=C3=A1szl=C3=B3=20M=C3=A1t=C3=A9?= Date: Fri, 3 Jul 2026 13:00:56 +0200 Subject: [PATCH 2/9] stringer interface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Mózes László Máté --- pkg/util/image/types.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/util/image/types.go b/pkg/util/image/types.go index b447ad214..e9b3080ae 100644 --- a/pkg/util/image/types.go +++ b/pkg/util/image/types.go @@ -14,7 +14,9 @@ package image -import "strings" +import ( + "strings" +) // ParsedImage is a structured representation of a container image reference, // broken into registry, sub-path, base name, tag, and digest components. @@ -81,3 +83,7 @@ func (p *ParsedImage) Prefix() string { return sb.String() } + +func (p *ParsedImage) String() string { + return p.Full() +} From 636f47770abde17c0878f20e9582c9320bcf6e53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=B3zes=20L=C3=A1szl=C3=B3=20M=C3=A1t=C3=A9?= Date: Fri, 3 Jul 2026 15:21:46 +0200 Subject: [PATCH 3/9] AI review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Assisted-by: Cursor:composer-2.5 Signed-off-by: Mózes László Máté --- .../reconciler/functionconfigreconciler.go | 17 +- func/internal/podcachemanager.go | 9 +- pkg/util/image/image.go | 3 + pkg/util/image/image_test.go | 1 + pkg/util/util_test.go | 468 ++++++++++++++++++ 5 files changed, 478 insertions(+), 20 deletions(-) diff --git a/controllers/functionconfigs/reconciler/functionconfigreconciler.go b/controllers/functionconfigs/reconciler/functionconfigreconciler.go index 69008c4ed..712991050 100644 --- a/controllers/functionconfigs/reconciler/functionconfigreconciler.go +++ b/controllers/functionconfigs/reconciler/functionconfigreconciler.go @@ -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() @@ -214,10 +204,11 @@ func (s *FunctionConfigStore) GetBinaryFromCacheByConstraint(image, tag string) } 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 } diff --git a/func/internal/podcachemanager.go b/func/internal/podcachemanager.go index 3dda4c164..a38322680 100644 --- a/func/internal/podcachemanager.go +++ b/func/internal/podcachemanager.go @@ -19,7 +19,6 @@ import ( "fmt" "net" "slices" - "strings" "sync/atomic" "time" @@ -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) @@ -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. diff --git a/pkg/util/image/image.go b/pkg/util/image/image.go index c0e40030d..54069f261 100644 --- a/pkg/util/image/image.go +++ b/pkg/util/image/image.go @@ -73,6 +73,9 @@ func Join(parts ...string) string { for _, part := range parts[1:] { part = strings.Trim(part, "/") + if part == "" { + continue + } sb.WriteString("/") sb.WriteString(part) } diff --git a/pkg/util/image/image_test.go b/pkg/util/image/image_test.go index 079e46873..8f3d66ab7 100644 --- a/pkg/util/image/image_test.go +++ b/pkg/util/image/image_test.go @@ -220,6 +220,7 @@ func TestImageParse(t *testing.T) { t.Run(name, func(t *testing.T) { got := Parse(tc.input) assert.Equal(t, tc.want, got) + assert.Equal(t, got.Original, got.Full()) }) } } diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index 93726dc19..f0935927d 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -22,7 +22,10 @@ import ( "strings" "testing" + "github.com/kptdev/krm-functions-sdk/go/fn/kptfileapi" + configapi "github.com/kptdev/porch/api/porchconfig/v1alpha1" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "gopkg.in/yaml.v2" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -473,3 +476,468 @@ func getPartErrMsg(errorSlice []string, start string) string { return "" } + +func TestGetRepoPackageRefFromUpstream(t *testing.T) { + tests := []struct { + name string + upstream *kptfileapi.Upstream + wantRepoSpec *configapi.RepositorySpec + wantPkg string + wantRef string + wantManagedRef bool + wantErr string + }{ + { + name: "nil upstream returns error", + upstream: nil, + wantErr: "upstream does not contain a valid git repository", + }, + { + name: "nil git returns error", + upstream: &kptfileapi.Upstream{Git: nil}, + wantErr: "upstream does not contain a valid git repository", + }, + { + name: "empty repo returns error", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "", + Ref: "mypkg/v1", + Directory: "mypkg", + }, + }, + wantErr: "upstream does not contain a valid git repository", + }, + { + name: "empty directory returns error", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "https://github.com/example/repo.git", + Ref: "mypkg/v1", + Directory: "", + }, + }, + wantErr: "git directory reference", + }, + { + name: "directory with leading slash returns error", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "https://github.com/example/repo.git", + Ref: "mypkg/v1", + Directory: "/mypkg", + }, + }, + wantErr: "git directory reference \"/mypkg\" in upstream is invalid", + }, + { + name: "directory with trailing slash returns error", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "https://github.com/example/repo.git", + Ref: "mypkg/v1", + Directory: "mypkg/", + }, + }, + wantErr: "git directory reference \"mypkg/\" in upstream is invalid", + }, + { + name: "empty ref returns error", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "https://github.com/example/repo.git", + Ref: "", + Directory: "mypkg", + }, + }, + wantErr: "git ref reference", + }, + { + name: "unmanaged ref - ref does not contain directory pattern", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "https://github.com/example/repo.git", + Ref: "main", + Directory: "mypkg", + }, + }, + wantRepoSpec: &configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: "https://github.com/example/repo", + Directory: "mypkg", + }, + }, + wantPkg: "mypkg", + wantRef: "main", + wantManagedRef: false, + }, + { + name: "unmanaged ref - arbitrary tag", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "https://github.com/example/repo.git", + Ref: "release-2.0", + Directory: "packages/foo", + }, + }, + wantRepoSpec: &configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: "https://github.com/example/repo", + Directory: "packages/foo", + }, + }, + wantPkg: "packages.foo", + wantRef: "release-2.0", + wantManagedRef: false, + }, + { + name: "managed ref - simple package", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "https://github.com/example/repo.git", + Ref: "mypkg/v1", + Directory: "mypkg", + }, + }, + wantRepoSpec: &configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: "https://github.com/example/repo", + Directory: "", + }, + }, + wantPkg: "mypkg", + wantRef: "v1", + wantManagedRef: true, + }, + { + name: "managed ref - nested package", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "https://github.com/example/repo.git", + Ref: "packages/mypkg/v2", + Directory: "packages/mypkg", + }, + }, + wantRepoSpec: &configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: "https://github.com/example/repo", + Directory: "", + }, + }, + wantPkg: "packages.mypkg", + wantRef: "v2", + wantManagedRef: true, + }, + { + name: "managed ref - with repo directory prefix", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "https://github.com/example/repo.git", + Ref: "blueprints/mypkg/v3", + Directory: "mypkg", + }, + }, + wantRepoSpec: &configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: "https://github.com/example/repo", + Directory: "blueprints", + }, + }, + wantPkg: "mypkg", + wantRef: "v3", + wantManagedRef: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + repoSpec, pkg, ref, managedRef, err := GetRepoPackageRefFromUpstream(tt.upstream) + + if tt.wantErr != "" { + assert.ErrorContains(t, err, tt.wantErr) + assert.Nil(t, repoSpec) + } else { + require.NoError(t, err) + assert.Equal(t, tt.wantRepoSpec, repoSpec) + assert.Equal(t, tt.wantPkg, pkg) + assert.Equal(t, tt.wantRef, ref) + assert.Equal(t, tt.wantManagedRef, managedRef) + } + }) + } +} + +func TestGetRepoPackageRefFromUpstreamRealData(t *testing.T) { + tests := []struct { + name string + upstream *kptfileapi.Upstream + wantRepoSpec *configapi.RepositorySpec + wantPkg string + wantRef string + wantManagedRef bool + wantErr string + }{ + { + name: "real test case 1", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "http://172.18.255.204:3000/porch/porch-test.git", + Directory: "upstream-function1", + Ref: "upstream-function1/v1", + }, + }, + wantRepoSpec: &configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: "http://172.18.255.204:3000/porch/porch-test", + Directory: "", + }, + }, + wantPkg: "upstream-function1", + wantRef: "v1", + wantManagedRef: true, + }, + { + name: "real test case 2", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "http://172.18.255.204:3000/porch/porch-test.git", + Directory: "r2/r2/r2/r2", + Ref: "r2/r2/r2/r2/r2/r2/v1", + }, + }, + wantRepoSpec: &configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: "http://172.18.255.204:3000/porch/porch-test", + Directory: "r2/r2", + }, + }, + wantPkg: "r2.r2.r2.r2", + wantRef: "v1", + wantManagedRef: true, + }, + { + name: "real test case 3", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "http://172.18.255.204:3000/porch/porch-test.git", + Directory: "upstream-function", + Ref: "l1/upstream-function/v1", + }, + }, + wantRepoSpec: &configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: "http://172.18.255.204:3000/porch/porch-test", + Directory: "l1", + }, + }, + wantPkg: "upstream-function", + wantRef: "v1", + wantManagedRef: true, + }, + { + name: "real test case 4", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "http://172.18.255.204:3000/porch/porch-test.git", + Directory: "upstream-function", + Ref: "r1/upstream-function/v1", + }, + }, + wantRepoSpec: &configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: "http://172.18.255.204:3000/porch/porch-test", + Directory: "r1", + }, + }, + wantPkg: "upstream-function", + wantRef: "v1", + wantManagedRef: true, + }, + { + name: "real test case 5", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "http://172.18.255.204:3000/porch/porch-test.git", + Directory: "upstream-function", + Ref: "l3/l3/l3/upstream-function/v1", + }, + }, + wantRepoSpec: &configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: "http://172.18.255.204:3000/porch/porch-test", + Directory: "l3/l3/l3", + }, + }, + wantPkg: "upstream-function", + wantRef: "v1", + wantManagedRef: true, + }, + { + name: "real test case 6", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "http://172.18.255.204:3000/porch/porch-test.git", + Directory: "upstream-dir/upstream-function", + Ref: "l3/l3/l3/upstream-dir/upstream-function/v1", + }, + }, + wantRepoSpec: &configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: "http://172.18.255.204:3000/porch/porch-test", + Directory: "l3/l3/l3", + }, + }, + wantPkg: "upstream-dir.upstream-function", + wantRef: "v1", + wantManagedRef: true, + }, + { + name: "real test case 7", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "http://172.18.255.204:3000/porch/porch-test.git", + Directory: "upstream-function", + Ref: "r3/r3/r3/upstream-function/v1", + }, + }, + wantRepoSpec: &configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: "http://172.18.255.204:3000/porch/porch-test", + Directory: "r3/r3/r3", + }, + }, + wantPkg: "upstream-function", + wantRef: "v1", + wantManagedRef: true, + }, + { + name: "real test case 8", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "https://github.com/kptdev/kpt.git", + Directory: "package-examples/wordpress", + Ref: "v1.0.0-beta.64", + }, + }, + wantRepoSpec: &configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: "https://github.com/kptdev/kpt", + Directory: "package-examples/wordpress", + }, + }, + wantPkg: "package-examples.wordpress", + wantRef: "v1.0.0-beta.64", + wantManagedRef: false, + }, + { + name: "real test case 9", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "https://github.com/kptdev/kpt-samples.git", + Directory: "echo", + Ref: "main", + }, + }, + wantRepoSpec: &configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: "https://github.com/kptdev/kpt-samples", + Directory: "echo", + }, + }, + wantPkg: "echo", + wantRef: "main", + wantManagedRef: false, + }, + { + name: "real test case 10", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "https://github.com/kptdev/kpt-samples.git", + Directory: "echo", + Ref: "v0", + }, + }, + wantRepoSpec: &configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: "https://github.com/kptdev/kpt-samples", + Directory: "echo", + }, + }, + wantPkg: "echo", + wantRef: "v0", + wantManagedRef: false, + }, + { + name: "real test case 11", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "http://172.18.255.204:3000/porch/porch-test.git", + Directory: "r2/r2/r2/r2/r2/r2", + Ref: "r2/r2/r2/r2/r2/r2/v1", + }, + }, + wantRepoSpec: &configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: "http://172.18.255.204:3000/porch/porch-test", + Directory: "", + }, + }, + wantPkg: "r2.r2.r2.r2.r2.r2", + wantRef: "v1", + wantManagedRef: true, + }, + { + name: "real test case 12", + upstream: &kptfileapi.Upstream{ + Git: &kptfileapi.Git{ + Repo: "https://github.com/nephio-project/catalog.git", + Directory: "nephio/core/workload-crds", + Ref: "nephio/core/workload-crds/v3.0.0", + }, + }, + wantRepoSpec: &configapi.RepositorySpec{ + Type: configapi.RepositoryTypeGit, + Git: &configapi.GitRepository{ + Repo: "https://github.com/nephio-project/catalog", + Directory: "", + }, + }, + wantPkg: "nephio.core.workload-crds", + wantRef: "v3.0.0", + wantManagedRef: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + repoSpec, pkg, ref, managedRef, err := GetRepoPackageRefFromUpstream(tt.upstream) + + if tt.wantErr != "" { + assert.ErrorContains(t, err, tt.wantErr) + assert.Nil(t, repoSpec) + } else { + require.NoError(t, err) + assert.Equal(t, tt.wantRepoSpec, repoSpec) + assert.Equal(t, tt.wantPkg, pkg) + assert.Equal(t, tt.wantRef, ref) + assert.Equal(t, tt.wantManagedRef, managedRef) + } + }) + } +} From bda0baa1a3c81ac9aa5b6cb2f8385432b4b77e14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=B3zes=20L=C3=A1szl=C3=B3=20M=C3=A1t=C3=A9?= Date: Fri, 3 Jul 2026 16:29:19 +0200 Subject: [PATCH 4/9] fix unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Mózes László Máté --- func/internal/executableevaluator.go | 2 +- func/internal/executableevaluator_test.go | 20 +++++++++++--------- pkg/engine/builtinruntime_test.go | 13 +++++++------ 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/func/internal/executableevaluator.go b/func/internal/executableevaluator.go index 8faf5456f..4e10a4f42 100644 --- a/func/internal/executableevaluator.go +++ b/func/internal/executableevaluator.go @@ -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{ diff --git a/func/internal/executableevaluator_test.go b/func/internal/executableevaluator_test.go index 91032c278..426b27a34 100644 --- a/func/internal/executableevaluator_test.go +++ b/func/internal/executableevaluator_test.go @@ -16,8 +16,10 @@ package internal import ( "bytes" + "flag" "fmt" "os" + "path/filepath" "testing" kptfilev1 "github.com/kptdev/kpt/api/kptfile/v1" @@ -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() @@ -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 := imageutil.Join(tmpDir, setImageFunction) + testBinary := filepath.Join(tmpDir, setImageFunction) const testScript = `#!/bin/sh # Emulating the KRM function execution by running this shell script cat @@ -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() @@ -232,7 +236,7 @@ items: [] tmpDir := t.TempDir() // Create a simple test executable that echoes input as a valid KRM function - testBinary := imageutil.Join(tmpDir, setImageFunction) + testBinary := filepath.Join(tmpDir, setImageFunction) const testScript = `#!/bin/sh # Emulating the KRM function execution by running this shell script cat @@ -291,7 +295,7 @@ items: [] tmpDir := t.TempDir() // Create a simple test executable that echoes input as a valid KRM function - testBinary := imageutil.Join(tmpDir, setImageFunction) + testBinary := filepath.Join(tmpDir, setImageFunction) const testScript = `#!/bin/sh # Emulating the KRM function execution by running this shell script cat @@ -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"`) }) } diff --git a/pkg/engine/builtinruntime_test.go b/pkg/engine/builtinruntime_test.go index 601adf448..15b14fc37 100644 --- a/pkg/engine/builtinruntime_test.go +++ b/pkg/engine/builtinruntime_test.go @@ -16,6 +16,7 @@ package engine import ( "bytes" + "flag" "os" "path/filepath" "testing" @@ -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{ @@ -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 @@ -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 From d78b858149df1edadbe3e3b1f04e07ee46bef0fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=B3zes=20L=C3=A1szl=C3=B3=20M=C3=A1t=C3=A9?= Date: Mon, 6 Jul 2026 11:32:15 +0200 Subject: [PATCH 5/9] fix lint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Mózes László Máté --- pkg/util/util.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/util/util.go b/pkg/util/util.go index 2f0361f16..bc7a667a9 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -38,7 +38,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation" - "k8s.io/klog/v2" registrationapi "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/config" From 0ba41be8cbd18f71086d401b4dfd7dc848568501 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=B3zes=20L=C3=A1szl=C3=B3=20M=C3=A1t=C3=A9?= Date: Mon, 6 Jul 2026 13:54:48 +0200 Subject: [PATCH 6/9] improve coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Mózes László Máté --- pkg/util/image/image.go | 21 +++---- pkg/util/image/image_test.go | 106 ++++++++++++++++++++++++++++++++--- pkg/util/image/types.go | 7 ++- 3 files changed, 110 insertions(+), 24 deletions(-) diff --git a/pkg/util/image/image.go b/pkg/util/image/image.go index 54069f261..3a1ce2517 100644 --- a/pkg/util/image/image.go +++ b/pkg/util/image/image.go @@ -64,22 +64,15 @@ func FindBestSemverMatch(constraint string, cachedTags []string) (string, error) } func Join(parts ...string) string { - if len(parts) == 0 { - return "" - } - - sb := strings.Builder{} - sb.WriteString(strings.Trim(parts[0], "/")) - - for _, part := range parts[1:] { - part = strings.Trim(part, "/") - if part == "" { - continue + var outparts []string + for _, part := range parts { + trimmed := strings.Trim(part, "/ \t\n\v\f\r\x85\xA0") // whitespaces taken from unicode.IsSpace + if trimmed != "" { + outparts = append(outparts, trimmed) } - sb.WriteString("/") - sb.WriteString(part) } - return sb.String() + + return strings.Join(outparts, "/") } // Parse creates a ParsedImage object from the full image name. diff --git a/pkg/util/image/image_test.go b/pkg/util/image/image_test.go index 8f3d66ab7..77d8f5b6e 100644 --- a/pkg/util/image/image_test.go +++ b/pkg/util/image/image_test.go @@ -22,6 +22,15 @@ import ( "github.com/stretchr/testify/require" ) +const ( + registry = "ghcr.io" + registryWithPort = "my-registry.com:5000" + subpath = "kptdev/krm-functions-catalog" + image = "apply-setters" + tag = "v0.2.3" + digest = "sha256:7d89a74f106241391f687fc2985c8e6de597bb21f0d0014def5edc730618d9cc" +) + func TestFindBestSemverMatch(t *testing.T) { testCases := map[string]struct { constraint string @@ -118,15 +127,6 @@ func TestFindBestSemverMatch(t *testing.T) { } func TestImageParse(t *testing.T) { - const ( - registry = "ghcr.io" - registryWithPort = "my-registry.com:5000" - subpath = "kptdev/krm-functions-catalog" - image = "apply-setters" - tag = "v0.2.3" - digest = "sha256:7d89a74f106241391f687fc2985c8e6de597bb21f0d0014def5edc730618d9cc" - ) - testCases := map[string]struct { input string want ParsedImage @@ -224,3 +224,91 @@ func TestImageParse(t *testing.T) { }) } } + +func TestPrefix(t *testing.T) { + testCases := map[string]struct { + input string + want string + }{ + "empty": { + input: fmt.Sprintf("%s:%s", image, tag), + want: "", + }, + "registry only": { + input: fmt.Sprintf("%s/%s:%s", registry, image, tag), + want: registry, + }, + "registry with path": { + input: fmt.Sprintf("%s/%s/%s:%s", registry, subpath, image, tag), + want: fmt.Sprintf("%s/%s", registry, subpath), + }, + "localhost registry only": { + input: fmt.Sprintf("%s/%s:%s", "localhost:8080", image, tag), + want: "localhost:8080", + }, + "localhost registry with path": { + input: fmt.Sprintf("%s/%s/%s:%s", "localhost:8080", subpath, image, tag), + want: fmt.Sprintf("%s/%s", "localhost:8080", subpath), + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + got := Parse(tc.input) + assert.Equal(t, tc.want, got.Prefix()) + }) + } +} + +func TestStringer(t *testing.T) { + orig := fmt.Sprintf("%s/%s/%s:%s@%s", registry, subpath, image, tag, digest) + parsed := Parse(orig) + assert.Equal(t, orig, parsed.String()) +} + +func TestJoin(t *testing.T) { + testCases := map[string]struct { + input []string + want string + }{ + "empty": { + input: []string{}, + want: "", + }, + "no prefix": { + input: []string{image}, + want: image, + }, + "empty prefix": { + input: []string{"", image}, + want: image, + }, + "registry only": { + input: []string{registry, image}, + want: fmt.Sprintf("%s/%s", registry, image), + }, + "registry with path": { + input: []string{registry, subpath, image}, + want: fmt.Sprintf("%s/%s/%s", registry, subpath, image), + }, + "empty registry with path": { + input: []string{"", subpath, image}, + want: fmt.Sprintf("%s/%s", subpath, image), + }, + "extra slashes": { + input: []string{"/" + registry, subpath + "/", image}, + want: fmt.Sprintf("%s/%s/%s", registry, subpath, image), + }, + "dangling slash": { + input: []string{"/", subpath, image}, + want: fmt.Sprintf("%s/%s", subpath, image), + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + got := Join(tc.input...) + assert.Equal(t, tc.want, got) + }) + } +} diff --git a/pkg/util/image/types.go b/pkg/util/image/types.go index e9b3080ae..b2af0ae50 100644 --- a/pkg/util/image/types.go +++ b/pkg/util/image/types.go @@ -15,6 +15,7 @@ package image import ( + "fmt" "strings" ) @@ -75,7 +76,9 @@ func (p *ParsedImage) Prefix() string { if p.Registry != "" { sb.WriteString(p.Registry) - sb.WriteString("/") + if p.SubPath != "" { + sb.WriteString("/") + } } if p.SubPath != "" { sb.WriteString(p.SubPath) @@ -84,6 +87,8 @@ func (p *ParsedImage) Prefix() string { return sb.String() } +var _ fmt.Stringer = &ParsedImage{} + func (p *ParsedImage) String() string { return p.Full() } From 846f92ad206145ea836231e098aa6cc44c7549dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=B3zes=20L=C3=A1szl=C3=B3=20M=C3=A1t=C3=A9?= Date: Mon, 6 Jul 2026 14:28:01 +0200 Subject: [PATCH 7/9] don't expect weird whitespaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Mózes László Máté --- pkg/util/image/image.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/util/image/image.go b/pkg/util/image/image.go index 3a1ce2517..59d9a2392 100644 --- a/pkg/util/image/image.go +++ b/pkg/util/image/image.go @@ -66,7 +66,7 @@ func FindBestSemverMatch(constraint string, cachedTags []string) (string, error) func Join(parts ...string) string { var outparts []string for _, part := range parts { - trimmed := strings.Trim(part, "/ \t\n\v\f\r\x85\xA0") // whitespaces taken from unicode.IsSpace + trimmed := strings.Trim(part, "/ \t\n\r") if trimmed != "" { outparts = append(outparts, trimmed) } From 8df46b837822527367c9ed6914b7f81209957fbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=B3zes=20L=C3=A1szl=C3=B3=20M=C3=A1t=C3=A9?= Date: Mon, 6 Jul 2026 16:11:00 +0200 Subject: [PATCH 8/9] clean up GetBinaryFromCacheByConstraint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Mózes László Máté --- .../reconciler/functionconfigreconciler.go | 39 +++++++------------ 1 file changed, 14 insertions(+), 25 deletions(-) diff --git a/controllers/functionconfigs/reconciler/functionconfigreconciler.go b/controllers/functionconfigs/reconciler/functionconfigreconciler.go index 712991050..f17a298ae 100644 --- a/controllers/functionconfigs/reconciler/functionconfigreconciler.go +++ b/controllers/functionconfigs/reconciler/functionconfigreconciler.go @@ -43,12 +43,12 @@ const FunctionRunnerFinalizer = BaseFinalizer + "-function-runner" const ControllerFinalizer = BaseFinalizer + "-controller" type BinaryCacheEntry struct { - PrefixRegex string + PrefixRegex *regexp.Regexp Tags map[string]string } type BuiltInCacheEntry struct { - PrefixRegex string + PrefixRegex *regexp.Regexp Process fnsdk.ResourceListProcessor Tags []string } @@ -80,7 +80,7 @@ func (s *FunctionConfigStore) UpsertFunctionConfig(name string, obj *configapi.F s.functionConfigurations[name] = obj } -func (s *FunctionConfigStore) generateRegexPattern(prefixes []string, imageName string) string { +func (s *FunctionConfigStore) generateRegexPattern(prefixes []string) *regexp.Regexp { var preparedPrefixes []string for _, prefix := range prefixes { if prefix == "" { @@ -90,7 +90,7 @@ func (s *FunctionConfigStore) generateRegexPattern(prefixes []string, imageName } } - return "^(?:" + strings.Join(preparedPrefixes, "|") + ")$" + return regexp.MustCompile("^(?:" + strings.Join(preparedPrefixes, "|") + ")$") } @@ -101,7 +101,7 @@ func (s *FunctionConfigStore) UpdateBinaryCache(_ string, obj *configapi.Functio var binaryCacheEntry BinaryCacheEntry binaryCacheEntry.Tags = make(map[string]string) // Create a prefix Regex - binaryCacheEntry.PrefixRegex = s.generateRegexPattern(obj.Spec.Prefixes, obj.Spec.Image) + binaryCacheEntry.PrefixRegex = s.generateRegexPattern(obj.Spec.Prefixes) abs := obj.Spec.BinaryExecutor.Path if abs[0] != '/' { @@ -139,7 +139,7 @@ func (s *FunctionConfigStore) UpdateExecCache(name string, functionConfig *confi s.builtInExecutorCache[id] = BuiltInCacheEntry{ Process: fn, Tags: functionConfig.Spec.GoExecutor.Tags, - PrefixRegex: s.generateRegexPattern(functionConfig.Spec.Prefixes, functionConfig.Spec.Image), + PrefixRegex: s.generateRegexPattern(functionConfig.Spec.Prefixes), } } @@ -175,8 +175,7 @@ func (s *FunctionConfigStore) GetBinaryFromCache(image string) (string, bool) { prefixToCheck := parsedImage.Prefix() binaryStore, exists := s.binaryExecutorCache[parsedImage.BaseName] if exists { - regex := regexp.MustCompile(binaryStore.PrefixRegex) - if regex.MatchString(prefixToCheck) { + if binaryStore.PrefixRegex.MatchString(prefixToCheck) { binaryPath, tagExists := binaryStore.Tags[parsedImage.Tag] if tagExists { return binaryPath, true @@ -193,28 +192,19 @@ func (s *FunctionConfigStore) GetBinaryFromCacheByConstraint(image, tag string) 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) + if !cacheEntry.PrefixRegex.MatchString(parsedImage.Prefix()) { + return "", false } + cacheKeys := slices.Collect(maps.Keys(cacheEntry.Tags)) + selectedKey, err := imageutil.FindBestSemverMatch(tag, cacheKeys) if err != nil { return "", false } - selectedBinary := cacheEntry.Tags[selectedKey] - - // TODO: something is not right here - - regex := regexp.MustCompile(cacheEntry.PrefixRegex) - if regex.MatchString(parsedImage.Prefix()) { - binaryPath, tagExists := cacheEntry.Tags[parsedImage.Tag] - if tagExists { - return binaryPath, true - } - } + selectedBinary, ok := cacheEntry.Tags[selectedKey] - return selectedBinary, true + return selectedBinary, ok } func (s *FunctionConfigStore) GetExecCache() map[string]BuiltInCacheEntry { @@ -234,8 +224,7 @@ func (s *FunctionConfigStore) GetProcessorFromCache(image string) (fnsdk.Resourc prefixToCheck = s.defaultImagePrefix } if slices.Contains(entry.Tags, parsedImage.Tag) { - regex := regexp.MustCompile(entry.PrefixRegex) - if regex.MatchString(prefixToCheck) { + if entry.PrefixRegex.MatchString(prefixToCheck) { return entry.Process, found } } From 33bd121115412bca7627f650fb448c85763398eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=B3zes=20L=C3=A1szl=C3=B3=20M=C3=A1t=C3=A9?= Date: Mon, 6 Jul 2026 16:18:59 +0200 Subject: [PATCH 9/9] add unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Assisted-by: Cursor:composer-2.5 Signed-off-by: Mózes László Máté --- .../reconciler/functionconfigreconciler.go | 5 +- .../functionconfigreconciler_test.go | 71 +++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/controllers/functionconfigs/reconciler/functionconfigreconciler.go b/controllers/functionconfigs/reconciler/functionconfigreconciler.go index f17a298ae..67e2e6ce1 100644 --- a/controllers/functionconfigs/reconciler/functionconfigreconciler.go +++ b/controllers/functionconfigs/reconciler/functionconfigreconciler.go @@ -190,7 +190,10 @@ func (s *FunctionConfigStore) GetBinaryFromCacheByConstraint(image, tag string) defer s.mu.RUnlock() parsedImage := imageutil.Parse(image) - cacheEntry := s.binaryExecutorCache[parsedImage.BaseName] + cacheEntry, ok := s.binaryExecutorCache[parsedImage.BaseName] + if !ok { + return "", false + } if !cacheEntry.PrefixRegex.MatchString(parsedImage.Prefix()) { return "", false diff --git a/controllers/functionconfigs/reconciler/functionconfigreconciler_test.go b/controllers/functionconfigs/reconciler/functionconfigreconciler_test.go index 1a5086aa6..d0daa10ac 100644 --- a/controllers/functionconfigs/reconciler/functionconfigreconciler_test.go +++ b/controllers/functionconfigs/reconciler/functionconfigreconciler_test.go @@ -279,6 +279,77 @@ func TestFinalizersAdded(t *testing.T) { } } +func TestGetBinaryFromCacheByConstraint(t *testing.T) { + store := NewFunctionConfigStore(defaultImagePrefix, functionCacheDir) + + obj := &configapi.FunctionConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "set-image", Namespace: testNamespace}, + Spec: configapi.FunctionConfigSpec{ + Image: "set-image", + Prefixes: []string{""}, + BinaryExecutor: &configapi.BinaryExecutorConfig{ + Tags: []string{"v0.1.2", "v0.1.3"}, + Path: "set-image", + }, + }, + } + store.UpdateBinaryCache(obj.Name, obj) + + const expectedPath = "/functions/set-image" + const qualifiedImage = "ghcr.io/kptdev/krm-functions-catalog/set-image" + + tests := map[string]struct { + image string + constraint string + wantPath string + wantFound bool + }{ + "selects highest matching version": { + image: qualifiedImage, + constraint: ">= 0.1.2 < 0.2.0", + wantPath: expectedPath, + wantFound: true, + }, + "prefix mismatch": { + image: "evil.registry/set-image", + constraint: ">= 0.1.2 < 0.2.0", + wantFound: false, + }, + "unknown image basename": { + image: "ghcr.io/kptdev/krm-functions-catalog/nonexistent", + constraint: ">= 0.1.0", + wantFound: false, + }, + "invalid semver constraint": { + image: qualifiedImage, + constraint: ">> 1.0.0", + wantFound: false, + }, + "no matching version for valid constraint": { + image: qualifiedImage, + constraint: "> 1.0.0", + wantFound: false, + }, + "short form without registry prefix": { + image: "set-image", + constraint: ">= 0.1.2", + wantFound: false, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + path, found := store.GetBinaryFromCacheByConstraint(tc.image, tc.constraint) + assert.Equal(t, tc.wantFound, found) + if tc.wantFound { + assert.Equal(t, tc.wantPath, path) + } else { + assert.Empty(t, path) + } + }) + } +} + func TestGetProcessorFromCache(t *testing.T) { store := NewFunctionConfigStore(defaultImagePrefix, functionCacheDir)