Nested acorn validation support for acorn update --confirm-upgrade (#1726)

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

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))

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
+}

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")
+			}
+		})
+	}
+}

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, ",")))