Skip to content

Commit 499cc06

Browse files
authored
Move common API utility functions to common util go file (#1081)
* Move common API utility functions to common util go file Signed-off-by: liamfallon <liam.fallon@est.tech> * Address copilot comments Signed-off-by: liamfallon <liam.fallon@est.tech> * Fix uuid fails in unit tests Signed-off-by: liamfallon <liam.fallon@est.tech> * Fix uuid fails in unit tests Signed-off-by: liamfallon <liam.fallon@est.tech> * Use v1alpha1 for apiu service start Signed-off-by: liamfallon <liam.fallon@est.tech> * Fix GenerateUid() function to generate UIDs off internal interface Signed-off-by: liamfallon <liam.fallon@est.tech> * Use GVK to generate UIDs Signed-off-by: liamfallon <liam.fallon@est.tech> --------- Signed-off-by: liamfallon <liam.fallon@est.tech>
1 parent 7e0bdad commit 499cc06

11 files changed

Lines changed: 309 additions & 245 deletions

File tree

api/porch/util.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
// Copyright 2022-2024, 2026 The kpt Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License
14+
15+
package porch
16+
17+
import (
18+
"regexp"
19+
"strings"
20+
21+
pkgerrors "github.com/pkg/errors"
22+
"k8s.io/apimachinery/pkg/util/validation"
23+
)
24+
25+
// validRelativePathRegex validates the basic shape of a relative path (slash-separated segments made of allowed characters).
26+
// Additional constraints (e.g. no leading/trailing '/', no '.', and DNS1123-compliant name composition) are enforced in IsValidSubpackageDir.
27+
var validRelativePathRegex = regexp.MustCompile(`^(?:[a-zA-Z0-9._-]+(?:/[a-zA-Z0-9._-]+)*)?$`)
28+
29+
// IsValidSubpackageDir returns an error if subpackageDir is invalid.
30+
func IsValidSubpackageDir(subpackageDir string) error {
31+
// Empty string is invalid, a subpackage directory must be a relative path.
32+
if subpackageDir == "" {
33+
return pkgerrors.Errorf("subpackage directory %q is invalid", subpackageDir)
34+
}
35+
36+
// Check basic format and ensure it doesn't start with '/', doesn't end with '/', and doesn't contain '.'
37+
if subpackageDir[0] == '/' || strings.HasSuffix(subpackageDir, "/") || strings.Contains(subpackageDir, ".") {
38+
return pkgerrors.Errorf("subpackage directory %q is invalid, it cannot contain '.' or start with '/' or end with '/'", subpackageDir)
39+
}
40+
41+
if !validRelativePathRegex.MatchString(subpackageDir) {
42+
return pkgerrors.Errorf("subpackage directory %q is invalid, it must match regular expression %q", subpackageDir, validRelativePathRegex.String())
43+
}
44+
45+
if _, err := ComposeSubpkgObjName(subpackageDir); err != nil {
46+
return err
47+
}
48+
49+
return nil
50+
}
51+
52+
func ComposeSubpkgObjName(subpackageDir string) (string, error) {
53+
if subpackageDir == "" {
54+
return "", pkgerrors.Errorf("subpackage directory %q is invalid", subpackageDir)
55+
}
56+
57+
subpackageName := strings.ReplaceAll(subpackageDir, "/", ".")
58+
59+
objNameErrs := validation.IsDNS1123Subdomain(subpackageName)
60+
61+
if len(objNameErrs) == 0 {
62+
return subpackageName, nil
63+
} else {
64+
return "", pkgerrors.Errorf("subpackage resource name %q invalid: %s", subpackageName, strings.Join(objNameErrs, ","))
65+
}
66+
}

api/porch/util_test.go

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
// Copyright 2026 The kpt Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package porch
16+
17+
import (
18+
"testing"
19+
20+
"github.com/stretchr/testify/assert"
21+
)
22+
23+
func TestIsValidSubpackageDir(t *testing.T) {
24+
tests := []struct {
25+
name string
26+
dir string
27+
expectValid bool
28+
}{
29+
// Invalid cases
30+
{name: "empty string", dir: "", expectValid: false},
31+
{name: "leading slash", dir: "/subpkg", expectValid: false},
32+
{name: "trailing slash", dir: "subpkg/", expectValid: false},
33+
{name: "double dots at start", dir: "../subpkg", expectValid: false},
34+
{name: "double dots in middle", dir: "sub/../pkg", expectValid: false},
35+
{name: "double dots at end", dir: "subpkg/..", expectValid: false},
36+
{name: "only double dots", dir: "..", expectValid: false},
37+
{name: "dot segment at start", dir: "./subpkg", expectValid: false},
38+
{name: "dot segment in middle", dir: "sub/./pkg", expectValid: false},
39+
{name: "only dot", dir: ".", expectValid: false},
40+
{name: "leading and trailing slash", dir: "/subpkg/", expectValid: false},
41+
{name: "spaces in path", dir: "sub pkg", expectValid: false},
42+
{name: "special characters", dir: "sub@pkg", expectValid: false},
43+
{name: "backslash", dir: "sub\\pkg", expectValid: false},
44+
{name: "colon in path", dir: "sub:pkg", expectValid: false},
45+
{name: "empty segment (double slash)", dir: "sub//pkg", expectValid: false},
46+
{name: "with underscores (invalid DNS)", dir: "my_subpkg", expectValid: false},
47+
{name: "mixed with underscores (invalid DNS)", dir: "my-sub_pkg.v1/nested-dir", expectValid: false},
48+
{name: "with dots in name", dir: "my.subpkg", expectValid: false},
49+
50+
// Valid cases
51+
{name: "simple directory", dir: "subpkg", expectValid: true},
52+
{name: "nested directory", dir: "path/to/subpkg", expectValid: true},
53+
{name: "two levels", dir: "sub/pkg", expectValid: true},
54+
{name: "with hyphens", dir: "my-subpkg", expectValid: true},
55+
{name: "numeric name", dir: "123", expectValid: true},
56+
{name: "deeply nested", dir: "a/b/c/d/e", expectValid: true},
57+
{name: "single char segments", dir: "a/b/c", expectValid: true},
58+
{name: "starts with digit", dir: "1subpackage", expectValid: true},
59+
{name: "ends with digit", dir: "subpackage1", expectValid: true},
60+
{name: "contains digits", dir: "1subpckage2/3subpackage4/5subpackage6", expectValid: true},
61+
}
62+
63+
for _, tt := range tests {
64+
t.Run(tt.name, func(t *testing.T) {
65+
err := IsValidSubpackageDir(tt.dir)
66+
if tt.expectValid {
67+
assert.NoError(t, err)
68+
} else {
69+
assert.Error(t, err)
70+
}
71+
})
72+
}
73+
}
74+
75+
func TestComposeSubpkgObjName(t *testing.T) {
76+
subpackageName, err := ComposeSubpkgObjName("")
77+
assert.NotNil(t, err)
78+
assert.Equal(t, "", subpackageName)
79+
80+
subpackageName, err = ComposeSubpkgObjName("my-subpackage")
81+
assert.Nil(t, err)
82+
assert.Equal(t, "my-subpackage", subpackageName)
83+
84+
subpackageName, err = ComposeSubpkgObjName("level1/level2/my-subpackage")
85+
assert.Nil(t, err)
86+
assert.Equal(t, "level1.level2.my-subpackage", subpackageName)
87+
88+
subpackageName, err = ComposeSubpkgObjName("/level1/level2/")
89+
assert.NotNil(t, err)
90+
assert.Equal(t, "", subpackageName)
91+
}

api/porch/v1alpha1/util.go

Lines changed: 2 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,11 @@ package v1alpha1
1616

1717
import (
1818
"fmt"
19-
"regexp"
2019
"slices"
21-
"strings"
2220

23-
pkgerrors "github.com/pkg/errors"
24-
"k8s.io/apimachinery/pkg/util/validation"
21+
porchapi "github.com/kptdev/porch/api/porch"
2522
)
2623

27-
// validRelativePathRegex validates the basic shape of a relative path (slash-separated segments made of allowed characters).
28-
// Additional constraints (e.g. no leading/trailing '/', no '.', and DNS1123-compliant name composition) are enforced in IsValidSubpackageDir.
29-
var validRelativePathRegex = regexp.MustCompile(`^(?:[a-zA-Z0-9._-]+(?:/[a-zA-Z0-9._-]+)*)?$`)
30-
3124
func (pr *PackageRevision) IsPublished() bool {
3225
return LifecycleIsPublished(pr.Spec.Lifecycle)
3326
}
@@ -105,52 +98,13 @@ func GetSubpackageDir(pkgRev *PackageRevision) (string, error) {
10598
}
10699

107100
subpackageDir := getSubpackageDir(pkgRev.Spec.Tasks[1])
108-
if err := IsValidSubpackageDir(subpackageDir); err == nil {
101+
if err := porchapi.IsValidSubpackageDir(subpackageDir); err == nil {
109102
return subpackageDir, nil
110103
} else {
111104
return "", err
112105
}
113106
}
114107

115-
// IsValidSubpackageDir returns an error if subpackageDir is invalid.
116-
func IsValidSubpackageDir(subpackageDir string) error {
117-
// Empty string is invalid, a subpackage directory must be a relative path.
118-
if subpackageDir == "" {
119-
return pkgerrors.Errorf("subpackage directory %q is invalid", subpackageDir)
120-
}
121-
122-
// Check basic format and ensure it doesn't start with '/', doesn't end with '/', and doesn't contain '.'
123-
if subpackageDir[0] == '/' || strings.HasSuffix(subpackageDir, "/") || strings.Contains(subpackageDir, ".") {
124-
return pkgerrors.Errorf("subpackage directory %q is invalid, it cannot contain '.' or start with '/' or end with '/'", subpackageDir)
125-
}
126-
127-
if !validRelativePathRegex.MatchString(subpackageDir) {
128-
return pkgerrors.Errorf("subpackage directory %q is invalid, it must match regular expression %q", subpackageDir, validRelativePathRegex.String())
129-
}
130-
131-
if _, err := ComposeSubpkgObjName(subpackageDir); err != nil {
132-
return err
133-
}
134-
135-
return nil
136-
}
137-
138-
func ComposeSubpkgObjName(subpackageDir string) (string, error) {
139-
if subpackageDir == "" {
140-
return "", pkgerrors.Errorf("subpackage directory %q is invalid", subpackageDir)
141-
}
142-
143-
subpackageName := strings.ReplaceAll(subpackageDir, "/", ".")
144-
145-
objNameErrs := validation.IsDNS1123Subdomain(subpackageName)
146-
147-
if len(objNameErrs) == 0 {
148-
return subpackageName, nil
149-
} else {
150-
return "", pkgerrors.Errorf("subpackage resource name %q invalid: %s", subpackageName, strings.Join(objNameErrs, ","))
151-
}
152-
}
153-
154108
// getSubpackageDir gets the SubpackageDir from a task or returns "" if it does not exist
155109
func getSubpackageDir(task Task) string {
156110
switch task.Type {

api/porch/v1alpha1/util_test.go

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -21,58 +21,6 @@ import (
2121
"github.com/stretchr/testify/require"
2222
)
2323

24-
func TestIsValidSubpackageDir(t *testing.T) {
25-
tests := []struct {
26-
name string
27-
dir string
28-
expectValid bool
29-
}{
30-
// Invalid cases
31-
{name: "empty string", dir: "", expectValid: false},
32-
{name: "leading slash", dir: "/subpkg", expectValid: false},
33-
{name: "trailing slash", dir: "subpkg/", expectValid: false},
34-
{name: "double dots at start", dir: "../subpkg", expectValid: false},
35-
{name: "double dots in middle", dir: "sub/../pkg", expectValid: false},
36-
{name: "double dots at end", dir: "subpkg/..", expectValid: false},
37-
{name: "only double dots", dir: "..", expectValid: false},
38-
{name: "dot segment at start", dir: "./subpkg", expectValid: false},
39-
{name: "dot segment in middle", dir: "sub/./pkg", expectValid: false},
40-
{name: "only dot", dir: ".", expectValid: false},
41-
{name: "leading and trailing slash", dir: "/subpkg/", expectValid: false},
42-
{name: "spaces in path", dir: "sub pkg", expectValid: false},
43-
{name: "special characters", dir: "sub@pkg", expectValid: false},
44-
{name: "backslash", dir: "sub\\pkg", expectValid: false},
45-
{name: "colon in path", dir: "sub:pkg", expectValid: false},
46-
{name: "empty segment (double slash)", dir: "sub//pkg", expectValid: false},
47-
{name: "with underscores (invalid DNS)", dir: "my_subpkg", expectValid: false},
48-
{name: "mixed with underscores (invalid DNS)", dir: "my-sub_pkg.v1/nested-dir", expectValid: false},
49-
{name: "with dots in name", dir: "my.subpkg", expectValid: false},
50-
51-
// Valid cases
52-
{name: "simple directory", dir: "subpkg", expectValid: true},
53-
{name: "nested directory", dir: "path/to/subpkg", expectValid: true},
54-
{name: "two levels", dir: "sub/pkg", expectValid: true},
55-
{name: "with hyphens", dir: "my-subpkg", expectValid: true},
56-
{name: "numeric name", dir: "123", expectValid: true},
57-
{name: "deeply nested", dir: "a/b/c/d/e", expectValid: true},
58-
{name: "single char segments", dir: "a/b/c", expectValid: true},
59-
{name: "starts with digit", dir: "1subpackage", expectValid: true},
60-
{name: "ends with digit", dir: "subpackage1", expectValid: true},
61-
{name: "contains digits", dir: "1subpckage2/3subpackage4/5subpackage6", expectValid: true},
62-
}
63-
64-
for _, tt := range tests {
65-
t.Run(tt.name, func(t *testing.T) {
66-
err := IsValidSubpackageDir(tt.dir)
67-
if tt.expectValid {
68-
assert.NoError(t, err)
69-
} else {
70-
assert.Error(t, err)
71-
}
72-
})
73-
}
74-
}
75-
7624
func Test_getSubpackageDir(t *testing.T) {
7725
tests := []struct {
7826
name string

go.mod

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ go 1.26.3
44

55
replace k8s.io/apiserver v0.36.1 => ./third_party/k8s.io/apiserver-v0.36.1
66

7-
// replace github.com/kptdev/porch/api => ./api
7+
// TODO: Comment the line below out when the next version of the API is released.
8+
replace github.com/kptdev/porch/api => ./api
89

910
require (
1011
cloud.google.com/go/iam v1.11.0

go.sum

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,8 +278,6 @@ github.com/kptdev/krm-functions-catalog/functions/go/starlark v0.5.5 h1:2fVPRn0k
278278
github.com/kptdev/krm-functions-catalog/functions/go/starlark v0.5.5/go.mod h1:PE/l25mFdKm9MibK2sh/vO1YdFvrMIc3MXwyJW/scB0=
279279
github.com/kptdev/krm-functions-sdk/go/fn v1.0.4 h1:2Cl68JgaNva8eZ/YzqiZNOXkjIhavqLsoGOnxYN13Oc=
280280
github.com/kptdev/krm-functions-sdk/go/fn v1.0.4/go.mod h1:NqMHvghKasESpZImCDIOp5r10g3vmOeCcHzvjyL+4vk=
281-
github.com/kptdev/porch/api v1.0.0 h1:L/nNwnKTE2AqURKz/cl71B4nIODRuMcCszIavBj+vnA=
282-
github.com/kptdev/porch/api v1.0.0/go.mod h1:GPEGmRkk84PJx1ybrAuQSavZKD0e7OPnE8ETa0N8zag=
283281
github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
284282
github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
285283
github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=

0 commit comments

Comments
 (0)