Skip to content
This repository was archived by the owner on Mar 16, 2024. It is now read-only.

Commit daf8b68

Browse files
author
Oscar Ward
authored
Merge pull request #1995 from keyallis/issue-1726
Nested acorn validation support for `acorn update --confirm-upgrade` (#1726)
2 parents c375329 + 83c7f09 commit daf8b68

File tree

5 files changed

+144
-23
lines changed

5 files changed

+144
-23
lines changed

pkg/server/registry/apigroups/acorn/apps/confirmupgrade.go

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
apiv1 "github.com/acorn-io/runtime/pkg/apis/api.acorn.io/v1"
99
v1 "github.com/acorn-io/runtime/pkg/apis/internal.acorn.io/v1"
1010
kclient "github.com/acorn-io/runtime/pkg/k8sclient"
11+
"github.com/acorn-io/runtime/pkg/labels"
12+
apierrors "k8s.io/apimachinery/pkg/api/errors"
1113
"k8s.io/apiserver/pkg/endpoints/request"
1214
"k8s.io/apiserver/pkg/registry/rest"
1315
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -17,7 +19,7 @@ func NewConfirmUpgrade(c client.WithWatch) rest.Storage {
1719
return stores.NewBuilder(c.Scheme(), &apiv1.ConfirmUpgrade{}).
1820
WithCreate(&ConfirmUpgradeStrategy{
1921
client: c,
20-
}).Build()
22+
}).WithValidateName(nestedValidator{}).Build()
2123
}
2224

2325
type ConfirmUpgradeStrategy struct {
@@ -35,7 +37,19 @@ func (s *ConfirmUpgradeStrategy) Create(ctx context.Context, obj types.Object) (
3537
// The app validation logic should not run there.
3638
app := &v1.AppInstance{}
3739
err := s.client.Get(ctx, kclient.ObjectKey{Namespace: ri.Namespace, Name: ri.Name}, app)
38-
if err != nil {
40+
if apierrors.IsNotFound(err) {
41+
// See if this is a public name
42+
appList := &v1.AppInstanceList{}
43+
listErr := s.client.List(ctx, appList, client.MatchingLabels{labels.AcornPublicName: ri.Name}, client.InNamespace(ri.Namespace))
44+
if listErr != nil {
45+
return nil, listErr
46+
}
47+
if len(appList.Items) != 1 {
48+
//return the NotFound error we got originally
49+
return nil, err
50+
}
51+
app = &appList.Items[0]
52+
} else if err != nil {
3953
return nil, err
4054
}
4155
app.Status.AvailableAppImage = app.Status.ConfirmUpgradeAppImage

pkg/server/registry/apigroups/acorn/apps/ignorecleanup.go

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package apps
33
import (
44
"context"
55
"fmt"
6-
"strings"
76

87
"github.com/acorn-io/mink/pkg/stores"
98
"github.com/acorn-io/mink/pkg/types"
@@ -13,9 +12,6 @@ import (
1312
kclient "github.com/acorn-io/runtime/pkg/k8sclient"
1413
"github.com/acorn-io/runtime/pkg/labels"
1514
apierrors "k8s.io/apimachinery/pkg/api/errors"
16-
"k8s.io/apimachinery/pkg/runtime"
17-
"k8s.io/apimachinery/pkg/util/validation"
18-
"k8s.io/apimachinery/pkg/util/validation/field"
1915
"k8s.io/apiserver/pkg/endpoints/request"
2016
"k8s.io/apiserver/pkg/registry/rest"
2117
"k8s.io/utils/strings/slices"
@@ -26,21 +22,7 @@ func NewIgnoreCleanup(c client.WithWatch) rest.Storage {
2622
return stores.NewBuilder(c.Scheme(), &apiv1.IgnoreCleanup{}).
2723
WithCreate(&ignoreCleanupStrategy{
2824
client: c,
29-
}).
30-
WithValidateName(ignoreCleanupValidator{}).
31-
Build()
32-
}
33-
34-
type ignoreCleanupValidator struct{}
35-
36-
func (s ignoreCleanupValidator) ValidateName(ctx context.Context, _ runtime.Object) (result field.ErrorList) {
37-
ri, _ := request.RequestInfoFrom(ctx)
38-
for _, piece := range strings.Split(ri.Name, ".") {
39-
if errs := validation.IsDNS1035Label(piece); len(errs) > 0 {
40-
result = append(result, field.Invalid(field.NewPath("metadata", "name"), ri.Name, strings.Join(errs, ",")))
41-
}
42-
}
43-
return
25+
}).WithValidateName(nestedValidator{}).Build()
4426
}
4527

4628
type ignoreCleanupStrategy struct {
@@ -58,7 +40,7 @@ func (s *ignoreCleanupStrategy) Create(ctx context.Context, obj types.Object) (t
5840
// The app validation logic should not run there.
5941
app := &v1.AppInstance{}
6042
err := s.client.Get(ctx, kclient.ObjectKey{Namespace: ri.Namespace, Name: ri.Name}, app)
61-
if err != nil && apierrors.IsNotFound(err) {
43+
if apierrors.IsNotFound(err) {
6244
// See if this is a public name
6345
appList := &v1.AppInstanceList{}
6446
listErr := s.client.List(ctx, appList, client.MatchingLabels{labels.AcornPublicName: ri.Name}, client.InNamespace(ri.Namespace))
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
package apps
2+
3+
import (
4+
"context"
5+
"strings"
6+
7+
"k8s.io/apimachinery/pkg/runtime"
8+
"k8s.io/apimachinery/pkg/util/validation"
9+
"k8s.io/apimachinery/pkg/util/validation/field"
10+
kclient "sigs.k8s.io/controller-runtime/pkg/client"
11+
)
12+
13+
type nestedValidator struct{}
14+
15+
func (s nestedValidator) ValidateName(_ context.Context, obj runtime.Object) (result field.ErrorList) {
16+
name := obj.(kclient.Object).GetName()
17+
for _, piece := range strings.Split(name, ".") {
18+
if errs := validation.IsDNS1035Label(piece); len(errs) > 0 {
19+
result = append(result, field.Invalid(field.NewPath("metadata", "name"), name, strings.Join(errs, ",")))
20+
}
21+
}
22+
return
23+
}
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
package apps
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
apiv1 "github.com/acorn-io/runtime/pkg/apis/api.acorn.io/v1"
8+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
9+
)
10+
11+
func TestNestedValidateAppName(t *testing.T) {
12+
validator := &nestedValidator{}
13+
14+
tests := []struct {
15+
name string
16+
appName string
17+
expectValid bool
18+
}{
19+
{
20+
name: "Invalid Name: Underscore",
21+
appName: "my_app",
22+
expectValid: false,
23+
},
24+
{
25+
name: "Invalid Name: Uppercase",
26+
appName: "MyApp",
27+
expectValid: false,
28+
},
29+
{
30+
name: "Invalid Name: Starts with number",
31+
appName: "1app",
32+
expectValid: false,
33+
},
34+
{
35+
name: "Invalid Name: Starts with dash",
36+
appName: "-app",
37+
expectValid: false,
38+
},
39+
{
40+
name: "Invalid Name: Ends with dash",
41+
appName: "app-",
42+
expectValid: false,
43+
},
44+
{
45+
name: "Valid Name: Lowercase",
46+
appName: "myapp",
47+
expectValid: true,
48+
},
49+
{
50+
name: "Valid Name: Nested",
51+
appName: "myapp.nested",
52+
expectValid: true,
53+
},
54+
{
55+
name: "Valid Name: Multi Nested",
56+
appName: "myapp.nested.nested",
57+
expectValid: true,
58+
},
59+
{
60+
name: "Invalid Name: Nested Underscore",
61+
appName: "myapp.my_app",
62+
expectValid: false,
63+
},
64+
{
65+
name: "Invalid Name: Uppercase",
66+
appName: "myapp.MyApp",
67+
expectValid: false,
68+
},
69+
{
70+
name: "Invalid Name: Starts with number",
71+
appName: "myapp.1app",
72+
expectValid: false,
73+
},
74+
{
75+
name: "Invalid Name: Starts with dash",
76+
appName: "myapp.-app",
77+
expectValid: false,
78+
},
79+
{
80+
name: "Invalid Name: Ends with dash",
81+
appName: "myapp.app-",
82+
expectValid: false,
83+
},
84+
}
85+
86+
for _, tt := range tests {
87+
t.Run(tt.name, func(t *testing.T) {
88+
app := &apiv1.App{
89+
ObjectMeta: metav1.ObjectMeta{
90+
Name: tt.appName,
91+
},
92+
}
93+
err := validator.ValidateName(context.Background(), app)
94+
if tt.expectValid && err != nil {
95+
t.Fatalf("Expected valid, got error: %v", err)
96+
}
97+
if !tt.expectValid && err == nil {
98+
t.Fatalf("Expected error, got nil")
99+
}
100+
})
101+
}
102+
}

pkg/server/registry/apigroups/acorn/apps/validator.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func NewValidator(client kclient.Client, clientFactory *client.Factory, deleter
5252
}
5353
}
5454

55-
func (s *Validator) ValidateName(ctx context.Context, obj runtime.Object) (result field.ErrorList) {
55+
func (s *Validator) ValidateName(_ context.Context, obj runtime.Object) (result field.ErrorList) {
5656
name := obj.(kclient.Object).GetName()
5757
if errs := validation.IsDNS1035Label(name); len(errs) > 0 {
5858
result = append(result, field.Invalid(field.NewPath("metadata", "name"), name, strings.Join(errs, ",")))

0 commit comments

Comments
 (0)