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

Commit

Permalink
Nested acorn validation support on confirm-upgrade (#1726)
Browse files Browse the repository at this point in the history
Signed-off-by: Oscar Ward <oscar@acorn.io>
  • Loading branch information
keyallis authored and Oscar Ward committed Jul 31, 2023
1 parent ff1e20f commit 83c7f09
Show file tree
Hide file tree
Showing 5 changed files with 144 additions and 23 deletions.
18 changes: 16 additions & 2 deletions pkg/server/registry/apigroups/acorn/apps/confirmupgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
apiv1 "github.com/acorn-io/runtime/pkg/apis/api.acorn.io/v1"
v1 "github.com/acorn-io/runtime/pkg/apis/internal.acorn.io/v1"
kclient "github.com/acorn-io/runtime/pkg/k8sclient"
"github.com/acorn-io/runtime/pkg/labels"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/registry/rest"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -17,7 +19,7 @@ func NewConfirmUpgrade(c client.WithWatch) rest.Storage {
return stores.NewBuilder(c.Scheme(), &apiv1.ConfirmUpgrade{}).
WithCreate(&ConfirmUpgradeStrategy{
client: c,
}).Build()
}).WithValidateName(nestedValidator{}).Build()
}

type ConfirmUpgradeStrategy struct {
Expand All @@ -35,7 +37,19 @@ func (s *ConfirmUpgradeStrategy) Create(ctx context.Context, obj types.Object) (
// The app validation logic should not run there.
app := &v1.AppInstance{}
err := s.client.Get(ctx, kclient.ObjectKey{Namespace: ri.Namespace, Name: ri.Name}, app)
if err != nil {
if apierrors.IsNotFound(err) {
// See if this is a public name
appList := &v1.AppInstanceList{}
listErr := s.client.List(ctx, appList, client.MatchingLabels{labels.AcornPublicName: ri.Name}, client.InNamespace(ri.Namespace))
if listErr != nil {
return nil, listErr
}
if len(appList.Items) != 1 {
//return the NotFound error we got originally
return nil, err
}
app = &appList.Items[0]
} else if err != nil {
return nil, err
}
app.Status.AvailableAppImage = app.Status.ConfirmUpgradeAppImage
Expand Down
22 changes: 2 additions & 20 deletions pkg/server/registry/apigroups/acorn/apps/ignorecleanup.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package apps
import (
"context"
"fmt"
"strings"

"github.com/acorn-io/mink/pkg/stores"
"github.com/acorn-io/mink/pkg/types"
Expand All @@ -13,9 +12,6 @@ import (
kclient "github.com/acorn-io/runtime/pkg/k8sclient"
"github.com/acorn-io/runtime/pkg/labels"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/registry/rest"
"k8s.io/utils/strings/slices"
Expand All @@ -26,21 +22,7 @@ func NewIgnoreCleanup(c client.WithWatch) rest.Storage {
return stores.NewBuilder(c.Scheme(), &apiv1.IgnoreCleanup{}).
WithCreate(&ignoreCleanupStrategy{
client: c,
}).
WithValidateName(ignoreCleanupValidator{}).
Build()
}

type ignoreCleanupValidator struct{}

func (s ignoreCleanupValidator) ValidateName(ctx context.Context, _ runtime.Object) (result field.ErrorList) {
ri, _ := request.RequestInfoFrom(ctx)
for _, piece := range strings.Split(ri.Name, ".") {
if errs := validation.IsDNS1035Label(piece); len(errs) > 0 {
result = append(result, field.Invalid(field.NewPath("metadata", "name"), ri.Name, strings.Join(errs, ",")))
}
}
return
}).WithValidateName(nestedValidator{}).Build()
}

type ignoreCleanupStrategy struct {
Expand All @@ -58,7 +40,7 @@ func (s *ignoreCleanupStrategy) Create(ctx context.Context, obj types.Object) (t
// The app validation logic should not run there.
app := &v1.AppInstance{}
err := s.client.Get(ctx, kclient.ObjectKey{Namespace: ri.Namespace, Name: ri.Name}, app)
if err != nil && apierrors.IsNotFound(err) {
if apierrors.IsNotFound(err) {
// See if this is a public name
appList := &v1.AppInstanceList{}
listErr := s.client.List(ctx, appList, client.MatchingLabels{labels.AcornPublicName: ri.Name}, client.InNamespace(ri.Namespace))
Expand Down
23 changes: 23 additions & 0 deletions pkg/server/registry/apigroups/acorn/apps/nested_validator.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package apps

import (
"context"
"strings"

"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
kclient "sigs.k8s.io/controller-runtime/pkg/client"
)

type nestedValidator struct{}

func (s nestedValidator) ValidateName(_ context.Context, obj runtime.Object) (result field.ErrorList) {
name := obj.(kclient.Object).GetName()
for _, piece := range strings.Split(name, ".") {
if errs := validation.IsDNS1035Label(piece); len(errs) > 0 {
result = append(result, field.Invalid(field.NewPath("metadata", "name"), name, strings.Join(errs, ",")))
}
}
return
}
102 changes: 102 additions & 0 deletions pkg/server/registry/apigroups/acorn/apps/nested_validator_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
package apps

import (
"context"
"testing"

apiv1 "github.com/acorn-io/runtime/pkg/apis/api.acorn.io/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func TestNestedValidateAppName(t *testing.T) {
validator := &nestedValidator{}

tests := []struct {
name string
appName string
expectValid bool
}{
{
name: "Invalid Name: Underscore",
appName: "my_app",
expectValid: false,
},
{
name: "Invalid Name: Uppercase",
appName: "MyApp",
expectValid: false,
},
{
name: "Invalid Name: Starts with number",
appName: "1app",
expectValid: false,
},
{
name: "Invalid Name: Starts with dash",
appName: "-app",
expectValid: false,
},
{
name: "Invalid Name: Ends with dash",
appName: "app-",
expectValid: false,
},
{
name: "Valid Name: Lowercase",
appName: "myapp",
expectValid: true,
},
{
name: "Valid Name: Nested",
appName: "myapp.nested",
expectValid: true,
},
{
name: "Valid Name: Multi Nested",
appName: "myapp.nested.nested",
expectValid: true,
},
{
name: "Invalid Name: Nested Underscore",
appName: "myapp.my_app",
expectValid: false,
},
{
name: "Invalid Name: Uppercase",
appName: "myapp.MyApp",
expectValid: false,
},
{
name: "Invalid Name: Starts with number",
appName: "myapp.1app",
expectValid: false,
},
{
name: "Invalid Name: Starts with dash",
appName: "myapp.-app",
expectValid: false,
},
{
name: "Invalid Name: Ends with dash",
appName: "myapp.app-",
expectValid: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
app := &apiv1.App{
ObjectMeta: metav1.ObjectMeta{
Name: tt.appName,
},
}
err := validator.ValidateName(context.Background(), app)
if tt.expectValid && err != nil {
t.Fatalf("Expected valid, got error: %v", err)
}
if !tt.expectValid && err == nil {
t.Fatalf("Expected error, got nil")
}
})
}
}
2 changes: 1 addition & 1 deletion pkg/server/registry/apigroups/acorn/apps/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func NewValidator(client kclient.Client, clientFactory *client.Factory, deleter
}
}

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

0 comments on commit 83c7f09

Please sign in to comment.