POR-1850 Run cmd fixes (#3748)

Co-authored-by: d-g-town <66391417+d-g-town@users.noreply.github.com>

dashboard/src/lib/hooks/useAppValidation.ts

@@ -116,7 +116,6 @@ export const useAppValidation = ({
         },
         {}
       );
-
       const res = await api.validatePorterApp(
         "<token>",
         {
@@ -148,7 +147,8 @@ export const useAppValidation = ({
         .parseAsync(res.data);
 
       const validatedAppProto = PorterApp.fromJsonString(
-        atob(validAppData.validate_b64_app_proto), {
+        atob(validAppData.validate_b64_app_proto),
+        {
           ignoreUnknownFields: true,
         }
       );

dashboard/src/lib/porter-apps/index.ts

@@ -82,12 +82,23 @@ export const clientAppValidator = z.object({
 export type ClientPorterApp = z.infer<typeof clientAppValidator>;
 
 // porterAppFormValidator is used to validate inputs when creating + updating an app
-export const porterAppFormValidator = z.object({
-  app: clientAppValidator,
-  source: sourceValidator,
-  deletions: deletionValidator,
-  redeployOnSave: z.boolean().default(false),
-});
+export const porterAppFormValidator = z
+  .object({
+    app: clientAppValidator,
+    source: sourceValidator,
+    deletions: deletionValidator,
+    redeployOnSave: z.boolean().default(false),
+  })
+  .refine(
+    ({ app, source }) => {
+      if (source.type === "docker-registry" || app.build.method === "pack") {
+        return app.services.every((svc) => svc.run.value.length > 0);
+      }
+
+      return true;
+    },
+    { message: "All services must include a run command" }
+  );
 export type PorterAppFormData = z.infer<typeof porterAppFormValidator>;
 
 // serviceOverrides is used to generate the services overrides for an app from porter.yaml
@@ -114,7 +125,7 @@ export function serviceOverrides({
         });
       }
 
-      return deserializeService({ service: svc,  setDefaults: false});
+      return deserializeService({ service: svc, setDefaults: false });
     });
 
   const validatedBuild = buildValidator
@@ -189,9 +200,9 @@ export function clientAppToProto(data: PorterAppFormData): PorterApp {
   const { app, source } = data;
 
   const services = app.services.reduce((acc: Record<string, Service>, svc) => {
-    const serialized = serializeService(svc)
-    const proto = serviceProto(serialized)
-    acc[svc.name.value] = proto
+    const serialized = serializeService(svc);
+    const proto = serviceProto(serialized);
+    acc[svc.name.value] = proto;
     return acc;
   }, {});
 
@@ -366,15 +377,15 @@ export function clientAppFromProto({
   const predeployOverrides = serializeService(overrides.predeploy);
   const predeploy = proto.predeploy
     ? [
-      deserializeService({
-        service: serializedServiceFromProto({
-          name: "pre-deploy",
-          service: proto.predeploy,
-          isPredeploy: true,
+        deserializeService({
+          service: serializedServiceFromProto({
+            name: "pre-deploy",
+            service: proto.predeploy,
+            isPredeploy: true,
+          }),
+          override: predeployOverrides,
         }),
-        override: predeployOverrides,
-      }),
-    ]
+      ]
     : undefined;
 
   return {

dashboard/src/lib/porter-apps/services.ts

@@ -98,29 +98,29 @@ export type SerializedService = {
   ramMegabytes: number;
   smartOptimization?: boolean;
   config:
-  | {
-    type: "web";
-    domains: {
-      name: string;
-    }[];
-    autoscaling?: SerializedAutoscaling;
-    healthCheck?: SerializedHealthcheck;
-    private?: boolean;
-  }
-  | {
-    type: "worker";
-    autoscaling?: SerializedAutoscaling;
-  }
-  | {
-    type: "job";
-    allowConcurrent?: boolean;
-    cron: string;
-    suspendCron?: boolean;
-    timeoutSeconds: number;
-  }
-  | {
-    type: "predeploy";
-  };
+    | {
+        type: "web";
+        domains: {
+          name: string;
+        }[];
+        autoscaling?: SerializedAutoscaling;
+        healthCheck?: SerializedHealthcheck;
+        private?: boolean;
+      }
+    | {
+        type: "worker";
+        autoscaling?: SerializedAutoscaling;
+      }
+    | {
+        type: "job";
+        allowConcurrent?: boolean;
+        cron: string;
+        suspendCron?: boolean;
+        timeoutSeconds: number;
+      }
+    | {
+        type: "predeploy";
+      };
 };
 
 export function isPredeployService(service: SerializedService | ClientService) {
@@ -305,7 +305,10 @@ export function deserializeService({
       service.ramMegabytes,
       override?.ramMegabytes
     ),
-    smartOptimization: ServiceField.boolean(service.smartOptimization, override?.smartOptimization),
+    smartOptimization: ServiceField.boolean(
+      service.smartOptimization,
+      override?.smartOptimization
+    ),
     domainDeletions: [],
   };
 
@@ -328,12 +331,12 @@ export function deserializeService({
           autoscaling: deserializeAutoscaling({
             autoscaling: config.autoscaling,
             override: overrideWebConfig?.autoscaling,
-              setDefaults: setDefaults,
+            setDefaults: setDefaults,
           }),
           healthCheck: deserializeHealthCheck({
             health: config.healthCheck,
             override: overrideWebConfig?.healthCheck,
-              setDefaults: setDefaults,
+            setDefaults: setDefaults,
           }),
 
           domains: uniqueDomains.map((domain) => ({
@@ -346,9 +349,11 @@ export function deserializeService({
           })),
           private:
             typeof config.private === "boolean" ||
-              typeof overrideWebConfig?.private === "boolean"
+            typeof overrideWebConfig?.private === "boolean"
               ? ServiceField.boolean(config.private, overrideWebConfig?.private)
-              : (setDefaults ? ServiceField.boolean(false, undefined) : undefined),
+              : setDefaults
+              ? ServiceField.boolean(false, undefined)
+              : undefined,
         },
       };
     })
@@ -363,7 +368,7 @@ export function deserializeService({
           autoscaling: deserializeAutoscaling({
             autoscaling: config.autoscaling,
             override: overrideWorkerConfig?.autoscaling,
-              setDefaults: setDefaults,
+            setDefaults: setDefaults,
           }),
         },
       };
@@ -378,27 +383,34 @@ export function deserializeService({
           type: "job" as const,
           allowConcurrent:
             typeof config.allowConcurrent === "boolean" ||
-              typeof overrideJobConfig?.allowConcurrent === "boolean"
+            typeof overrideJobConfig?.allowConcurrent === "boolean"
               ? ServiceField.boolean(
-                config.allowConcurrent,
-                overrideJobConfig?.allowConcurrent
-              )
-              : (setDefaults ? ServiceField.boolean(false, undefined) : undefined),
+                  config.allowConcurrent,
+                  overrideJobConfig?.allowConcurrent
+                )
+              : setDefaults
+              ? ServiceField.boolean(false, undefined)
+              : undefined,
           cron: ServiceField.string(config.cron, overrideJobConfig?.cron),
           suspendCron:
             typeof config.suspendCron === "boolean" ||
-              typeof overrideJobConfig?.suspendCron === "boolean"
+            typeof overrideJobConfig?.suspendCron === "boolean"
               ? ServiceField.boolean(
-                config.suspendCron,
-                overrideJobConfig?.suspendCron
-              )
-              : (setDefaults ? ServiceField.boolean(false, undefined) : undefined),
+                  config.suspendCron,
+                  overrideJobConfig?.suspendCron
+                )
+              : setDefaults
+              ? ServiceField.boolean(false, undefined)
+              : undefined,
           timeoutSeconds:
             config.timeoutSeconds != 0
-             ? ServiceField.number(
-                config.timeoutSeconds,
-                overrideJobConfig?.timeoutSeconds
-              ) : (setDefaults ? ServiceField.number(3600, overrideJobConfig?.timeoutSeconds) : ServiceField.number(0, overrideJobConfig?.timeoutSeconds)),
+              ? ServiceField.number(
+                  config.timeoutSeconds,
+                  overrideJobConfig?.timeoutSeconds
+                )
+              : setDefaults
+              ? ServiceField.number(3600, overrideJobConfig?.timeoutSeconds)
+              : ServiceField.number(0, overrideJobConfig?.timeoutSeconds),
         },
       };
     })
@@ -430,6 +442,7 @@ export function serviceProto(service: SerializedService): Service {
       (config) =>
         new Service({
           ...service,
+          runOptional: service.run,
           type: serviceTypeEnumProto(config.type),
           config: {
             value: {
@@ -444,6 +457,7 @@ export function serviceProto(service: SerializedService): Service {
       (config) =>
         new Service({
           ...service,
+          runOptional: service.run,
           type: serviceTypeEnumProto(config.type),
           config: {
             value: {
@@ -458,6 +472,7 @@ export function serviceProto(service: SerializedService): Service {
       (config) =>
         new Service({
           ...service,
+          runOptional: service.run ? service.run : undefined,
           type: serviceTypeEnumProto(config.type),
           config: {
             value: {
@@ -474,6 +489,7 @@ export function serviceProto(service: SerializedService): Service {
       (config) =>
         new Service({
           ...service,
+          runOptional: service.run ? service.run : undefined,
           type: serviceTypeEnumProto(config.type),
           config: {
             value: {},
@@ -504,6 +520,7 @@ export function serializedServiceFromProto({
     .with({ case: "webConfig" }, ({ value }) => ({
       ...service,
       name,
+      run: service.runOptional ? service.runOptional : "",
       config: {
         type: "web" as const,
         autoscaling: value.autoscaling ? value.autoscaling : undefined,
@@ -514,6 +531,7 @@ export function serializedServiceFromProto({
     .with({ case: "workerConfig" }, ({ value }) => ({
       ...service,
       name,
+      run: service.runOptional ? service.runOptional : "",
       config: {
         type: "worker" as const,
         autoscaling: value.autoscaling ? value.autoscaling : undefined,
@@ -523,22 +541,24 @@ export function serializedServiceFromProto({
     .with({ case: "jobConfig" }, ({ value }) =>
       isPredeploy
         ? {
-          ...service,
-          name,
-          config: {
-            type: "predeploy" as const,
-          },
-        }
+            ...service,
+            name,
+            run: service.runOptional ? service.runOptional : "",
+            config: {
+              type: "predeploy" as const,
+            },
+          }
         : {
-          ...service,
-          name,
-          config: {
-            type: "job" as const,
-            ...value,
-            allowConcurrent: value.allowConcurrentOptional,
-            timeoutSeconds: Number(value.timeoutSeconds),
-          },
-        }
+            ...service,
+            name,
+            run: service.runOptional ? service.runOptional : "",
+            config: {
+              type: "job" as const,
+              ...value,
+              allowConcurrent: value.allowConcurrentOptional,
+              timeoutSeconds: Number(value.timeoutSeconds),
+            },
+          }
     )
     .exhaustive();
 }

dashboard/src/lib/porter-apps/values.ts

@@ -51,7 +51,7 @@ export const ServiceField = {
   string: (defaultValue: string, overrideValue?: string): ServiceString => {
     return {
       readOnly: !!overrideValue,
-      value: overrideValue ?? defaultValue,
+      value: overrideValue || defaultValue,
     };
   },
   number: (
@@ -70,7 +70,7 @@ export const ServiceField = {
   ): ServiceBoolean => {
     return {
       readOnly: typeof overrideValue === "boolean",
-      value: overrideValue ?? defaultValue ?? false,
+      value: overrideValue || defaultValue || false,
     };
   },
 };

dashboard/src/main/home/app-dashboard/app-view/AppDataContainer.tsx

@@ -1,5 +1,5 @@
 import React, { useCallback, useEffect, useMemo, useState } from "react";
-import { FormProvider, useForm } from "react-hook-form";
+import { FieldErrors, FormProvider, useForm } from "react-hook-form";
 import {
   PorterAppFormData,
   SourceOptions,
@@ -37,6 +37,8 @@ import ImageSettingsTab from "./tabs/ImageSettingsTab";
 import { useAppAnalytics } from "lib/hooks/useAppAnalytics";
 import { useClusterResourceLimits } from "lib/hooks/useClusterResourceLimits";
 import { Error as ErrorComponent } from "components/porter/Error";
+import _ from "lodash";
+import axios from "axios";
 
 // commented out tabs are not yet implemented
 // will be included as support is available based on data from app revisions rather than helm releases
@@ -305,9 +307,13 @@ const AppDataContainer: React.FC<AppDataContainerProps> = ({ tabParam }) => {
         errorStackTrace: stack,
       });
 
-      if ((err as any).response?.data?.message) {
+      if (axios.isAxiosError(err)) {
         setError("app", {
-          message: `App update failed: ${(err as any).response.data.message}`,
+          message: `App update failed: ${err.message}`,
+        });
+      } else {
+        setError("app", {
+          message: `App update failed: Please try again or contact support if the error persists.`,
         });
       }
     }
@@ -316,8 +322,8 @@ const AppDataContainer: React.FC<AppDataContainerProps> = ({ tabParam }) => {
   const cancelRedeploy = useCallback(() => {
     const resetProto = previewRevision
       ? PorterApp.fromJsonString(atob(previewRevision.b64_app_proto), {
-        ignoreUnknownFields: true,
-      })
+          ignoreUnknownFields: true,
+        })
       : latestProto;
 
     // we don't store versions of build settings because they are stored in the db, so we just have to use the latest version
@@ -325,12 +331,12 @@ const AppDataContainer: React.FC<AppDataContainerProps> = ({ tabParam }) => {
     const resetSource =
       porterAppRecord.image_repo_uri && resetProto.image
         ? {
-          type: "docker-registry" as const,
-          image: {
-            repository: resetProto.image.repository,
-            tag: resetProto.image.tag,
-          },
-        }
+            type: "docker-registry" as const,
+            image: {
+              repository: resetProto.image.repository,
+              tag: resetProto.image.tag,
+            },
+          }
         : latestSource;
 
     reset({
@@ -355,17 +361,20 @@ const AppDataContainer: React.FC<AppDataContainerProps> = ({ tabParam }) => {
     onSubmit();
   }, [onSubmit, setConfirmDeployModalOpen]);
 
+  const errorMessagesDeep = useMemo(() => {
+    return Object.values(_.mapValues(errors, (error) => error?.message));
+  }, [errors]);
+
   const buttonStatus = useMemo(() => {
     if (isSubmitting) {
       return "loading";
     }
 
-    if (Object.keys(errors).length > 0) {
-      console.log(errors);
-      return errors.app?.message ? (
-        <ErrorComponent message={errors.app.message} />
-      ) : (
-        <ErrorComponent message="App update failed. If the error persists, please contact support." />
+    if (errorMessagesDeep.length > 0) {
+      return (
+        <ErrorComponent
+          message={`App update failed. ${errorMessagesDeep[0]}`}
+        />
       );
     }
 
@@ -374,13 +383,13 @@ const AppDataContainer: React.FC<AppDataContainerProps> = ({ tabParam }) => {
     }
 
     return "";
-  }, [isSubmitting, errors]);
+  }, [isSubmitting, errorMessagesDeep]);
 
   useEffect(() => {
     const newProto = previewRevision
       ? PorterApp.fromJsonString(atob(previewRevision.b64_app_proto), {
-        ignoreUnknownFields: true,
-      })
+          ignoreUnknownFields: true,
+        })
       : latestProto;
 
     // we don't store versions of build settings because they are stored in the db, so we just have to use the latest version
@@ -388,12 +397,12 @@ const AppDataContainer: React.FC<AppDataContainerProps> = ({ tabParam }) => {
     const newSource =
       porterAppRecord.image_repo_uri && newProto.image
         ? {
-          type: "docker-registry" as const,
-          image: {
-            repository: newProto.image.repository,
-            tag: newProto.image.tag,
-          },
-        }
+            type: "docker-registry" as const,
+            image: {
+              repository: newProto.image.repository,
+              tag: newProto.image.tag,
+            },
+          }
         : latestSource;
 
     reset({
@@ -472,17 +481,17 @@ const AppDataContainer: React.FC<AppDataContainerProps> = ({ tabParam }) => {
             { label: "Environment", value: "environment" },
             ...(latestProto.build
               ? [
-                {
-                  label: "Build Settings",
-                  value: "build-settings",
-                },
-              ]
+                  {
+                    label: "Build Settings",
+                    value: "build-settings",
+                  },
+                ]
               : [
-                {
-                  label: "Image Settings",
-                  value: "image-settings",
-                },
-              ]),
+                  {
+                    label: "Image Settings",
+                    value: "image-settings",
+                  },
+                ]),
             { label: "Settings", value: "settings" },
           ]}
           currentTab={currentTab}

internal/porter_app/parse_test.go

@@ -48,7 +48,7 @@ var result_nobuild = &porterv1.PorterApp{
 	Name: "test-app",
 	Services: map[string]*porterv1.Service{
 		"example-job": {
-			Run:          "echo 'hello world'",
+			RunOptional:  pointer.String("echo 'hello world'"),
 			CpuCores:     0.1,
 			RamMegabytes: 256,
 			Config: &porterv1.Service_JobConfig{
@@ -60,7 +60,7 @@ var result_nobuild = &porterv1.PorterApp{
 			Type: 3,
 		},
 		"example-wkr": {
-			Run:          "echo 'work'",
+			RunOptional:  pointer.String("echo 'work'"),
 			Instances:    1,
 			Port:         80,
 			CpuCores:     0.1,
@@ -73,7 +73,7 @@ var result_nobuild = &porterv1.PorterApp{
 			Type: 2,
 		},
 		"example-web": {
-			Run:          "node index.js",
+			RunOptional:  pointer.String("node index.js"),
 			Instances:    0,
 			Port:         8080,
 			CpuCores:     0.1,
@@ -105,7 +105,7 @@ var result_nobuild = &porterv1.PorterApp{
 		},
 	},
 	Predeploy: &porterv1.Service{
-		Run:          "ls",
+		RunOptional:  pointer.String("ls"),
 		Instances:    0,
 		Port:         0,
 		CpuCores:     0,
@@ -123,7 +123,7 @@ var v1_result_nobuild_no_image = &porterv1.PorterApp{
 	Name: "test-app",
 	Services: map[string]*porterv1.Service{
 		"example-job": {
-			Run:          "echo 'hello world'",
+			RunOptional:  pointer.String("echo 'hello world'"),
 			CpuCores:     0.1,
 			RamMegabytes: 256,
 			Config: &porterv1.Service_JobConfig{
@@ -135,7 +135,7 @@ var v1_result_nobuild_no_image = &porterv1.PorterApp{
 			Type: 3,
 		},
 		"example-wkr": {
-			Run:          "echo 'work'",
+			RunOptional:  pointer.String("echo 'work'"),
 			Instances:    1,
 			Port:         80,
 			CpuCores:     0.1,
@@ -148,7 +148,7 @@ var v1_result_nobuild_no_image = &porterv1.PorterApp{
 			Type: 2,
 		},
 		"example-web": {
-			Run:          "node index.js",
+			RunOptional:  pointer.String("node index.js"),
 			Instances:    0,
 			Port:         8080,
 			CpuCores:     0.1,
@@ -181,7 +181,7 @@ var v1_result_nobuild_no_image = &porterv1.PorterApp{
 		},
 	},
 	Predeploy: &porterv1.Service{
-		Run:          "ls",
+		RunOptional:  pointer.String("ls"),
 		Instances:    0,
 		Port:         0,
 		CpuCores:     0,

internal/porter_app/v1/types.go

@@ -111,7 +111,7 @@ type Build struct {
 
 // Service represents a service in a Porter app
 type Service struct {
-	Run    string        `yaml:"run"`
+	Run    *string       `yaml:"run,omitempty"`
 	Config ServiceConfig `yaml:"config"`
 	Type   string        `yaml:"type" validate:"required, oneof=web worker job"`
 }

internal/porter_app/v1/yaml.go

@@ -122,8 +122,8 @@ func protoEnumFromType(name string, service Service) porterv1.ServiceType {
 
 func serviceProtoFromConfig(service Service, serviceType porterv1.ServiceType) (*porterv1.Service, error) {
 	serviceProto := &porterv1.Service{
-		Run:  service.Run,
-		Type: serviceType,
+		RunOptional: service.Run,
+		Type:        serviceType,
 	}
 
 	// if the revision number cannot be converted, it will default to 0

internal/porter_app/v2/yaml.go

@@ -90,7 +90,7 @@ type Build struct {
 
 // Service represents a single service in a porter app
 type Service struct {
-	Run               string       `yaml:"run"`
+	Run               *string      `yaml:"run,omitempty"`
 	Type              string       `yaml:"type" validate:"required, oneof=web worker job"`
 	Instances         int          `yaml:"instances"`
 	CpuCores          float32      `yaml:"cpuCores"`
@@ -221,7 +221,7 @@ func protoEnumFromType(name string, service Service) porterv1.ServiceType {
 
 func serviceProtoFromConfig(service Service, serviceType porterv1.ServiceType) (*porterv1.Service, error) {
 	serviceProto := &porterv1.Service{
-		Run:               service.Run,
+		RunOptional:       service.Run,
 		Type:              serviceType,
 		Instances:         int32(service.Instances),
 		CpuCores:          service.CpuCores,