diff --git a/pkg/server/registry/apigroups/acorn/apps/confirmupgrade.go b/pkg/server/registry/apigroups/acorn/apps/confirmupgrade.go index 07d0b7afd..d34a98e87 100644 --- a/pkg/server/registry/apigroups/acorn/apps/confirmupgrade.go +++ b/pkg/server/registry/apigroups/acorn/apps/confirmupgrade.go @@ -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" @@ -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 { @@ -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 diff --git a/pkg/server/registry/apigroups/acorn/apps/ignorecleanup.go b/pkg/server/registry/apigroups/acorn/apps/ignorecleanup.go index 6fea132f5..1fa86c3b5 100644 --- a/pkg/server/registry/apigroups/acorn/apps/ignorecleanup.go +++ b/pkg/server/registry/apigroups/acorn/apps/ignorecleanup.go @@ -3,7 +3,6 @@ package apps import ( "context" "fmt" - "strings" "github.com/acorn-io/mink/pkg/stores" "github.com/acorn-io/mink/pkg/types" @@ -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" @@ -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 { @@ -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)) diff --git a/pkg/server/registry/apigroups/acorn/apps/nested_validator.go b/pkg/server/registry/apigroups/acorn/apps/nested_validator.go new file mode 100644 index 000000000..13a4b846b --- /dev/null +++ b/pkg/server/registry/apigroups/acorn/apps/nested_validator.go @@ -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 +} diff --git a/pkg/server/registry/apigroups/acorn/apps/nested_validator_test.go b/pkg/server/registry/apigroups/acorn/apps/nested_validator_test.go new file mode 100644 index 000000000..17ab38ce6 --- /dev/null +++ b/pkg/server/registry/apigroups/acorn/apps/nested_validator_test.go @@ -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") + } + }) + } +} diff --git a/pkg/server/registry/apigroups/acorn/apps/validator.go b/pkg/server/registry/apigroups/acorn/apps/validator.go index ecb3fcf79..711e1aa6b 100644 --- a/pkg/server/registry/apigroups/acorn/apps/validator.go +++ b/pkg/server/registry/apigroups/acorn/apps/validator.go @@ -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, ",")))