Always set AutoUpgrade if it is implied (#1459)

integration/client/apps/apps_test.go

@@ -482,18 +482,50 @@ func TestAppRunImageVariations(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	for desc, imageName := range map[string]string{
-		"ref":         "foo/bar:baz",
-		"id":          imageID,
-		"sha256":      fmt.Sprintf("sha256:%s", imageID),
-		"short":       imageID[:8],
-		"autoupgrade": "foo/bar:**",
-	} {
-		imageName := imageName
-		t.Run(desc, func(t *testing.T) {
+	testCases := []struct {
+		name      string
+		image     string
+		options   *client.AppRunOptions
+		expectErr bool
+	}{
+		{
+			name:  "ref",
+			image: "foo/bar:baz",
+		},
+		{
+			name:  "id",
+			image: imageID,
+		},
+		{
+			name:  "sha256",
+			image: fmt.Sprintf("sha256:%s", imageID),
+		},
+		{
+			name:  "short",
+			image: imageID[:8],
+		},
+		{
+			name:  "autoupgrade",
+			image: "foo/bar:**",
+			options: &client.AppRunOptions{
+				AutoUpgrade: &[]bool{true}[0],
+			},
+		},
+		{
+			name:      "autoupgrade-fail",
+			image:     "foo/bar:**",
+			expectErr: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
-			_, err := c.AppRun(ctx, imageName, nil)
-			assert.NoError(t, err)
+			_, err := c.AppRun(ctx, tc.image, tc.options)
+			if err != nil && !tc.expectErr {
+				t.Fatal("unexpected error:", err)
+			}
 		})
 	}
 }

pkg/autoupgrade/daemon.go

@@ -319,6 +319,15 @@ func AutoUpgradePattern(image string) (string, bool) {
 	return tag, strings.ContainsAny(tag, "#*")
 }
 
+// ImpliedAutoUpgrade returns boolean indicating if auto-upgrade is implied either
+// by a pattern in the app.Spec.image or app.Spec.AutoUpgradeInterval being specified.
+func ImpliedAutoUpgrade(interval, image string) bool {
+	if _, isPattern := AutoUpgradePattern(image); isPattern || interval != "" {
+		return true
+	}
+	return false
+}
+
 func Mode(appSpec v1.AppInstanceSpec) (string, bool) {
 	_, isPat := AutoUpgradePattern(appSpec.Image)
 	on := appSpec.GetAutoUpgrade() || appSpec.GetNotifyUpgrade() || isPat

pkg/cli/run.go

@@ -10,6 +10,7 @@ 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"
+	"github.com/acorn-io/runtime/pkg/autoupgrade"
 	cli "github.com/acorn-io/runtime/pkg/cli/builder"
 	"github.com/acorn-io/runtime/pkg/client"
 	"github.com/acorn-io/runtime/pkg/dev"
@@ -239,6 +240,11 @@ func (s *Run) Run(cmd *cobra.Command, args []string) (err error) {
 		return err
 	}
 
+	// Ensure opts.AutoUpgrade is set if is implied by opts.AutoUpgradeInterval or imageSource.Image's pattern.
+	if opts.AutoUpgrade == nil || !*opts.AutoUpgrade {
+		opts.AutoUpgrade = &[]bool{autoupgrade.ImpliedAutoUpgrade(opts.AutoUpgradeInterval, imageSource.Image)}[0]
+	}
+
 	// Force install prompt if needed
 	_, err = c.Info(cmd.Context())
 	if err != nil {

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

@@ -96,6 +96,12 @@ func (s *Validator) Validate(ctx context.Context, obj runtime.Object) (result fi
 		return
 	}
 
+	au := app.Spec.AutoUpgrade
+	if (au == nil || !*au) && autoupgrade.ImpliedAutoUpgrade(app.Spec.AutoUpgradeInterval, app.Spec.Image) {
+		result = append(result, field.Invalid(field.NewPath("spec", "autoUpgrade"), app.Spec.AutoUpgrade, "must be true if autoupgrade options are set"))
+		return
+	}
+
 	var (
 		imageGrantedPerms []v1.Permissions
 		checkImage        = app.Spec.Image