v1 edits

api/server/handlers/porter_app/parse_yaml.go

@@ -37,6 +37,7 @@ func NewParsePorterYAMLToProtoHandler(
 // ParsePorterYAMLToProtoRequest is the request object for the /apps/parse endpoint
 type ParsePorterYAMLToProtoRequest struct {
 	B64Yaml string `json:"b64_yaml"`
+	AppName string `json:"app_name"`
 }
 
 // ParsePorterYAMLToProtoResponse is the response object for the /apps/parse endpoint
@@ -82,7 +83,7 @@ func (c *ParsePorterYAMLToProtoHandler) ServeHTTP(w http.ResponseWriter, r *http
 		return
 	}
 
-	appProto, err := porter_app.ParseYAML(ctx, yaml)
+	appProto, err := porter_app.ParseYAML(ctx, yaml, request.AppName)
 	if err != nil {
 		err := telemetry.Error(ctx, span, err, "error parsing yaml")
 		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(err, http.StatusBadRequest))

internal/porter_app/parse.go

@@ -23,7 +23,7 @@ const (
 )
 
 // ParseYAML converts a Porter YAML file into a PorterApp proto object
-func ParseYAML(ctx context.Context, porterYaml []byte) (*porterv1.PorterApp, error) {
+func ParseYAML(ctx context.Context, porterYaml []byte, appName string) (*porterv1.PorterApp, error) {
 	ctx, span := telemetry.NewSpan(ctx, "porter-app-parse-yaml")
 	defer span.End()
 
@@ -49,7 +49,10 @@ func ParseYAML(ctx context.Context, porterYaml []byte) (*porterv1.PorterApp, err
 	// track this span in telemetry and reach out to customers who are still using old porter.yaml if they exist.
 	// once no one is converting from old porter.yaml, we can remove this code
 	case PorterYamlVersion_V1, "":
-		appProto, err = v1.AppProtoFromYaml(ctx, porterYaml)
+		if appName == "" {
+			return nil, telemetry.Error(ctx, span, nil, "v1 porter yaml requires externally-provided app name")
+		}
+		appProto, err = v1.AppProtoFromYaml(ctx, porterYaml, appName)
 		if err != nil {
 			return nil, telemetry.Error(ctx, span, err, "error converting v1 yaml to proto")
 		}

internal/porter_app/parse_test.go

@@ -30,7 +30,7 @@ func TestParseYAML(t *testing.T) {
 			want, err := os.ReadFile(fmt.Sprintf("testdata/%s.yaml", tt.porterYamlFileName))
 			is.NoErr(err) // no error expected reading test file
 
-			got, err := ParseYAML(context.Background(), want)
+			got, err := ParseYAML(context.Background(), want, "test-app")
 			is.NoErr(err) // umbrella chart values should convert to map[string]any without issues
 
 			diffProtoWithFailTest(t, is, tt.want, got)
@@ -118,7 +118,7 @@ var result_nobuild = &porterv1.PorterApp{
 }
 
 var v1_result_nobuild_no_image = &porterv1.PorterApp{
-	Name: "",
+	Name: "test-app",
 	Services: map[string]*porterv1.Service{
 		"example-job": {
 			Run:          "echo 'hello world'",

internal/porter_app/testdata/v1_input_no_build_no_image.yaml

@@ -65,9 +65,8 @@ apps:
           - test2.example.com
         porter_hosts: []
         annotations:
-          kubernetes.io/test: nginx
       service:
-        port: '3000'
+        port: '8080'
       health:
         startupProbe:
           enabled: false
@@ -82,7 +81,7 @@ apps:
         livenessProbe:
           enabled: true
           failureThreshold: '3'
-          path: /livez
+          path: /healthz
           periodSeconds: '5'
       cloudsql:
         enabled: false

internal/porter_app/v1/types.go

@@ -1,68 +1,39 @@
 package v1
 
-type v1_ServiceConfig struct {
-	// Autoscaling contains all configuration for autoscaling.  If enabled, ReplicaCount is ignored.
-	Autoscaling *Autoscaling `yaml:"autoscaling,omitempty" validate:"excluded_if=Type job"`
-	// Container contains configuration for Kubernetes container spec
-	Container Container `yaml:"container"`
-	// Health contains configuration for Kubernetes health probes
-	Health *Health `yaml:"health,omitempty" validate:"excluded_unless=Type web"`
-	// Ingress contains all configuration for ingress
-	Ingress Ingress `yaml:"ingress"`
-	// ReplicaCount is the number of replicas to run. Ignored if Autoscaling is enabled.
-	ReplicaCount string `yaml:"replicaCount"`
-	// Resources contains all configuration for resources requests
-	Resources Resources `yaml:"resources"`
-	// Service contains all configuration for the Kubernetes Service associated with the chart
-	Service Service `yaml:"service"`
-	// Labels contains all the labels to be included in controller specs
-	Labels map[string]string `yaml:"labels"`
-	// PodLabels contains all the labels to be included in pod specs
-	PodLabels map[string]string `yaml:"podLabels"`
-	// AllowConcurrent allows multiple instances of the job to run at the same time
-	AllowConcurrency bool `yaml:"allowConcurrent" validate:"excluded_unless=Type job"`
-	// Schedule is the cron schedule for the job
-	Schedule Schedule `yaml:"schedule" validate:"excluded_unless=Type job"`
+// ServiceConfig contains the configuration exposed to users in v1stack porter.yaml
+type ServiceConfig struct {
+	Autoscaling      *Autoscaling      `yaml:"autoscaling,omitempty" validate:"excluded_if=Type job"`
+	Container        Container         `yaml:"container"`
+	Health           *Health           `yaml:"health,omitempty" validate:"excluded_unless=Type web"`
+	Ingress          Ingress           `yaml:"ingress"`
+	ReplicaCount     string            `yaml:"replicaCount"`
+	Resources        Resources         `yaml:"resources"`
+	Service          KubernetesService `yaml:"service"`
+	AllowConcurrency bool              `yaml:"allowConcurrent" validate:"excluded_unless=Type job"`
+	Schedule         Schedule          `yaml:"schedule" validate:"excluded_unless=Type job"`
 }
 
 // Schedule contains all configuration for job schedules
 type Schedule struct {
-	// Enabled specifies whether or not to use a schedule
 	Enabled bool `yaml:"enabled"`
 	// Value is the cron schedule
 	Value string `yaml:"value,omitempty"`
 }
 
-// Autoscaling contains all configuration for autoscaling in a web/worker chart.  If enabled, ReplicaCount is ignored.
+// Autoscaling contains all configuration for autoscaling in a web/worker chart
 type Autoscaling struct {
-	// Enabled specifies whether or not to use autoscaling
-	Enabled bool `yaml:"enabled"`
-	// MaxReplicas is the maximum number of replicas to scale to
-	MaxReplicas string `yaml:"maxReplicas"`
-	// MinReplicas is the minimum number of replicas to scale to
-	MinReplicas string `yaml:"minReplicas"`
-	// TargetCPUUtilizationPercentage is the target CPU utilization percentage to scale on
-	TargetCPUUtilizationPercentage string `yaml:"targetCPUUtilizationPercentage"`
-	// TargetMemoryUtilizationPercentage is the target memory utilization percentage to scale on
+	Enabled                           bool   `yaml:"enabled"`
+	MaxReplicas                       string `yaml:"maxReplicas"`
+	MinReplicas                       string `yaml:"minReplicas"`
+	TargetCPUUtilizationPercentage    string `yaml:"targetCPUUtilizationPercentage"`
 	TargetMemoryUtilizationPercentage string `yaml:"targetMemoryUtilizationPercentage"`
 }
 
 // Container contains all configuration for containers
 type Container struct {
-	// Command is the command to run in the container
-	Command string `yaml:"command"`
-	// Env contains the environment variables for the container
-	Env ContainerEnv `yaml:"env"`
-	// Port is the port that the container exposes
 	Port string `yaml:"port"`
 }
 
-// ContainerEnv represents the environment variables for a container
-type ContainerEnv struct {
-	// Normal represents the service-specific environment variables (as opposed to environment variables from synced env groups)
-	Normal map[string]string `yaml:"normal"`
-}
-
 // Health contains user-configurable health probes
 type Health struct {
 	// LivenessProbe checks whether a container should be considered healthy
@@ -73,7 +44,6 @@ type Health struct {
 
 // LivenessProbe contains user-configurable values for a liveness probe
 type LivenessProbe struct {
-	// Enabled specifies whether or not to use a liveness probe
 	Enabled bool `yaml:"enabled"`
 	// Path is the endpoint path to use for the probe
 	Path string `yaml:"path"`
@@ -81,26 +51,21 @@ type LivenessProbe struct {
 
 // ReadinessProbe contains user-configurable values for a readiness probe
 type ReadinessProbe struct {
-	// Enabled specifies whether or not to use a readiness probe
 	Enabled bool `yaml:"enabled"`
 	// Path is the endpoint path to use for the probe
 	Path string `yaml:"path"`
 }
 
-// Image contains configuration for images
-type Image struct {
-	// Repository is url of the image repository where the image should be pulled from
-	Repository string `yaml:"repository"`
-	// Tag is the tag of the image to pull
-	Tag string `yaml:"tag"`
-}
-
 // Ingress contains configuration for ingress used by web charts
 type Ingress struct {
 	// Enabled specifies whether or not to use an ingress
 	Enabled bool `yaml:"enabled"`
-	// Hosts specifies the domains to include in the routing rules. If Ingress is enabled, hosts must not be empty.
+	// Hosts specifies the domains to include in the routing rules
 	Hosts []string `yaml:"hosts"`
+	// PorterHosts specifies the porter domains to include in the routing rules
+	PorterHosts []string `yaml:"porter_hosts"`
+	// Annotations specifies annotations to add to the ingress
+	Annotations map[string]string `yaml:"annotations"`
 }
 
 // Resources is a wrapper over requests
@@ -117,20 +82,20 @@ type Requests struct {
 	Memory string `yaml:"memory"`
 }
 
-// Service contains configuration for exposing services
-type Service struct {
+// KubernetesService contains configuration for exposing services
+type KubernetesService struct {
 	// Port is the port to expose the service on. This port should match the port in the container.
 	Port string `yaml:"port"`
 }
 
-type v1_PorterYAML struct {
-	Version  *string               `yaml:"version"`
-	Build    *Build                `yaml:"build"`
-	Env      map[string]string     `yaml:"env"`
-	Apps     map[string]v1_Service `yaml:"apps" validate:"required_without=Applications Services"`
-	Services map[string]v1_Service `yaml:"services" validate:"required_without=Applications Apps"`
+type PorterYAML struct {
+	Version  *string            `yaml:"version"`
+	Build    *Build             `yaml:"build"`
+	Env      map[string]string  `yaml:"env"`
+	Apps     map[string]Service `yaml:"apps" validate:"required_without=Services"`
+	Services map[string]Service `yaml:"services" validate:"required_without=Apps"`
 
-	Release *v1_Service `yaml:"release"`
+	Release *Service `yaml:"release"`
 }
 
 // Build represents the build settings for a Porter app
@@ -143,8 +108,8 @@ type Build struct {
 	Image      string   `yaml:"image" validate:"required_if=Method registry"`
 }
 
-type v1_Service struct {
-	Run    string           `yaml:"run"`
-	Config v1_ServiceConfig `yaml:"config"`
-	Type   string           `yaml:"type" validate:"required, oneof=web worker job"`
+type Service struct {
+	Run    string        `yaml:"run"`
+	Config ServiceConfig `yaml:"config"`
+	Type   string        `yaml:"type" validate:"required, oneof=web worker job"`
 }

internal/porter_app/v1/yaml.go

@@ -15,23 +15,27 @@ import (
 )
 
 // AppProtoFromYaml converts an old version Porter YAML file into a PorterApp proto object
-func AppProtoFromYaml(ctx context.Context, porterYamlBytes []byte) (*porterv1.PorterApp, error) {
+func AppProtoFromYaml(ctx context.Context, porterYamlBytes []byte, appName string) (*porterv1.PorterApp, error) {
 	ctx, span := telemetry.NewSpan(ctx, "v1-app-proto-from-yaml")
 	defer span.End()
 
+	if appName == "" {
+		return nil, telemetry.Error(ctx, span, nil, "app name is empty")
+	}
+	telemetry.WithAttributes(span, telemetry.AttributeKV{Key: "app-name", Value: appName})
+
 	if porterYamlBytes == nil {
 		return nil, telemetry.Error(ctx, span, nil, "porter yaml is nil")
 	}
 
-	porterYaml := &v1_PorterYAML{}
+	porterYaml := &PorterYAML{}
 	err := yaml.Unmarshal(porterYamlBytes, porterYaml)
 	if err != nil {
 		return nil, telemetry.Error(ctx, span, err, "error unmarshaling porter yaml")
 	}
 
 	appProto := &porterv1.PorterApp{
-		// TODO: figure out what to do about no name spec in v1
-		Name: "",
+		Name: appName,
 		Env:  porterYaml.Env,
 	}
 
@@ -53,6 +57,7 @@ func AppProtoFromYaml(ctx context.Context, porterYamlBytes []byte) (*porterv1.Po
 				Tag:        imageSpl[1],
 			}
 		} else {
+			telemetry.WithAttributes(span, telemetry.AttributeKV{Key: "image", Value: porterYaml.Build.Image})
 			return nil, telemetry.Error(ctx, span, err, "error parsing image")
 		}
 	}
@@ -60,7 +65,7 @@ func AppProtoFromYaml(ctx context.Context, porterYamlBytes []byte) (*porterv1.Po
 	if porterYaml.Apps != nil && porterYaml.Services != nil {
 		return nil, telemetry.Error(ctx, span, nil, "'apps' and 'services' are synonymous but both were defined")
 	}
-	var services map[string]v1_Service
+	var services map[string]Service
 	if porterYaml.Apps != nil {
 		services = porterYaml.Apps
 	}
@@ -77,11 +82,13 @@ func AppProtoFromYaml(ctx context.Context, porterYamlBytes []byte) (*porterv1.Po
 	for name, service := range services {
 		serviceType, err := protoEnumFromType(name, service)
 		if err != nil {
+			telemetry.WithAttributes(span, telemetry.AttributeKV{Key: "failing-service-name", Value: name})
 			return nil, telemetry.Error(ctx, span, err, "error getting service type")
 		}
 
 		serviceProto, err := serviceProtoFromConfig(service, serviceType)
 		if err != nil {
+			telemetry.WithAttributes(span, telemetry.AttributeKV{Key: "failing-service-name", Value: name})
 			return nil, telemetry.Error(ctx, span, err, "error casting service config")
 		}
 
@@ -100,7 +107,7 @@ func AppProtoFromYaml(ctx context.Context, porterYamlBytes []byte) (*porterv1.Po
 	return appProto, nil
 }
 
-func protoEnumFromType(name string, service v1_Service) (porterv1.ServiceType, error) {
+func protoEnumFromType(name string, service Service) (porterv1.ServiceType, error) {
 	var serviceType porterv1.ServiceType
 
 	if service.Type != "" {
@@ -136,7 +143,7 @@ func protoEnumFromType(name string, service v1_Service) (porterv1.ServiceType, e
 	return serviceType, errors.New("no type provided and could not parse service type from name")
 }
 
-func serviceProtoFromConfig(service v1_Service, serviceType porterv1.ServiceType) (*porterv1.Service, error) {
+func serviceProtoFromConfig(service Service, serviceType porterv1.ServiceType) (*porterv1.Service, error) {
 	serviceProto := &porterv1.Service{
 		Run:  service.Run,
 		Type: serviceType,
@@ -145,7 +152,7 @@ func serviceProtoFromConfig(service v1_Service, serviceType porterv1.ServiceType
 	// if the revision number cannot be converted, it will default to 0
 	replicaCount, _ := strconv.Atoi(service.Config.ReplicaCount)
 	if replicaCount < math.MinInt32 || replicaCount > math.MaxInt32 {
-		return nil, fmt.Errorf("replica count is out of range")
+		return nil, fmt.Errorf("replica count is out of range of int32")
 	}
 	// nolint:gosec
 	serviceProto.Instances = int32(replicaCount)
@@ -178,13 +185,26 @@ func serviceProtoFromConfig(service v1_Service, serviceType porterv1.ServiceType
 		serviceProto.RamMegabytes = int32(memoryFloat64)
 	}
 
+	if service.Config.Container.Port != "" && service.Config.Service.Port != "" && service.Config.Container.Port != service.Config.Service.Port {
+		return nil, errors.New("container port and service port do not match")
+	}
 	if service.Config.Container.Port != "" {
 		port, err := strconv.Atoi(service.Config.Container.Port)
 		if err != nil {
-			return nil, fmt.Errorf("invalid port '%s'", service.Config.Container.Port)
+			return nil, fmt.Errorf("container port cannot be converted to int: %w", err)
+		}
+		if port < math.MinInt32 || port > math.MaxInt32 {
+			return nil, fmt.Errorf("port is out of range of int32")
+		}
+		serviceProto.Port = int32(port)
+	}
+	if service.Config.Service.Port != "" {
+		port, err := strconv.Atoi(service.Config.Service.Port)
+		if err != nil {
+			return nil, fmt.Errorf("service port cannot be converted to int: %w", err)
 		}
 		if port < math.MinInt32 || port > math.MaxInt32 {
-			return nil, fmt.Errorf("port is out of range")
+			return nil, fmt.Errorf("port is out of range of int32")
 		}
 		serviceProto.Port = int32(port)
 	}
@@ -193,7 +213,7 @@ func serviceProtoFromConfig(service v1_Service, serviceType porterv1.ServiceType
 	default:
 		return nil, fmt.Errorf("invalid service type '%s'", serviceType)
 	case porterv1.ServiceType_SERVICE_TYPE_UNSPECIFIED:
-		return nil, errors.New("Service type unspecified")
+		return nil, errors.New("KubernetesService type unspecified")
 	case porterv1.ServiceType_SERVICE_TYPE_WEB:
 		webConfig := &porterv1.WebServiceConfig{}
 
@@ -204,25 +224,25 @@ func serviceProtoFromConfig(service v1_Service, serviceType porterv1.ServiceType
 			}
 			minReplicas, _ := strconv.Atoi(service.Config.Autoscaling.MinReplicas)
 			if minReplicas < math.MinInt32 || minReplicas > math.MaxInt32 {
-				return nil, fmt.Errorf("minReplicas is out of range")
+				return nil, errors.New("minReplicas is out of range of int32")
 			}
 			// nolint:gosec
 			autoscaling.MinInstances = int32(minReplicas)
 			maxReplicas, _ := strconv.Atoi(service.Config.Autoscaling.MaxReplicas)
 			if maxReplicas < math.MinInt32 || maxReplicas > math.MaxInt32 {
-				return nil, fmt.Errorf("maxReplicas is out of range")
+				return nil, errors.New("maxReplicas is out of range of int32")
 			}
 			// nolint:gosec
 			autoscaling.MaxInstances = int32(maxReplicas)
 			cpuThresholdPercent, _ := strconv.Atoi(service.Config.Autoscaling.TargetCPUUtilizationPercentage)
 			if cpuThresholdPercent < math.MinInt32 || cpuThresholdPercent > math.MaxInt32 {
-				return nil, fmt.Errorf("cpuThresholdPercent is out of range")
+				return nil, fmt.Errorf("cpuThresholdPercent is out of range of int32")
 			}
 			// nolint:gosec
 			autoscaling.CpuThresholdPercent = int32(cpuThresholdPercent)
 			memoryThresholdPercent, _ := strconv.Atoi(service.Config.Autoscaling.TargetMemoryUtilizationPercentage)
 			if memoryThresholdPercent < math.MinInt32 || memoryThresholdPercent > math.MaxInt32 {
-				return nil, fmt.Errorf("memoryThresholdPercent is out of range")
+				return nil, fmt.Errorf("memoryThresholdPercent is out of range of int32")
 			}
 			// nolint:gosec
 			autoscaling.MemoryThresholdPercent = int32(memoryThresholdPercent)
@@ -232,11 +252,23 @@ func serviceProtoFromConfig(service v1_Service, serviceType porterv1.ServiceType
 		var healthCheck *porterv1.HealthCheck
 		// note that we are only reading from the readiness probe config, since readiness and liveness share the same config now
 		if service.Config.Health != nil {
-			healthCheck = &porterv1.HealthCheck{
-				Enabled:  service.Config.Health.ReadinessProbe.Enabled,
-				HttpPath: service.Config.Health.ReadinessProbe.Path,
+			health := service.Config.Health
+			if health.ReadinessProbe.Enabled && health.LivenessProbe.Enabled && health.ReadinessProbe.Path != health.LivenessProbe.Path {
+				return nil, errors.New("liveness and readiness probes must have the same path")
+			}
+			if health.ReadinessProbe.Enabled {
+				healthCheck = &porterv1.HealthCheck{
+					Enabled:  service.Config.Health.ReadinessProbe.Enabled,
+					HttpPath: service.Config.Health.ReadinessProbe.Path,
+				}
+			} else if health.LivenessProbe.Enabled {
+				healthCheck = &porterv1.HealthCheck{
+					Enabled:  service.Config.Health.LivenessProbe.Enabled,
+					HttpPath: service.Config.Health.LivenessProbe.Path,
+				}
 			}
 		}
+
 		webConfig.HealthCheck = healthCheck
 
 		domains := make([]*porterv1.Domain, 0)
@@ -246,6 +278,15 @@ func serviceProtoFromConfig(service v1_Service, serviceType porterv1.ServiceType
 				Name: hostName,
 			})
 		}
+		for _, domain := range service.Config.Ingress.PorterHosts {
+			hostName := domain
+			domains = append(domains, &porterv1.Domain{
+				Name: hostName,
+			})
+		}
+		if service.Config.Ingress.Annotations != nil && len(service.Config.Ingress.Annotations) > 0 {
+			return nil, errors.New("annotations are not supported")
+		}
 		webConfig.Domains = domains
 		webConfig.Private = !service.Config.Ingress.Enabled
 
@@ -262,25 +303,25 @@ func serviceProtoFromConfig(service v1_Service, serviceType porterv1.ServiceType
 			}
 			minReplicas, _ := strconv.Atoi(service.Config.Autoscaling.MinReplicas)
 			if minReplicas < math.MinInt32 || minReplicas > math.MaxInt32 {
-				return nil, fmt.Errorf("minReplicas is out of range")
+				return nil, fmt.Errorf("minReplicas is out of range of int32")
 			}
 			// nolint:gosec
 			autoscaling.MinInstances = int32(minReplicas)
 			maxReplicas, _ := strconv.Atoi(service.Config.Autoscaling.MaxReplicas)
 			if maxReplicas < math.MinInt32 || maxReplicas > math.MaxInt32 {
-				return nil, fmt.Errorf("maxReplicas is out of range")
+				return nil, fmt.Errorf("maxReplicas is out of range of int32")
 			}
 			// nolint:gosec
 			autoscaling.MaxInstances = int32(maxReplicas)
 			cpuThresholdPercent, _ := strconv.Atoi(service.Config.Autoscaling.TargetCPUUtilizationPercentage)
 			if cpuThresholdPercent < math.MinInt32 || cpuThresholdPercent > math.MaxInt32 {
-				return nil, fmt.Errorf("cpuThresholdPercent is out of range")
+				return nil, fmt.Errorf("cpuThresholdPercent is out of range of int32")
 			}
 			// nolint:gosec
 			autoscaling.CpuThresholdPercent = int32(cpuThresholdPercent)
 			memoryThresholdPercent, _ := strconv.Atoi(service.Config.Autoscaling.TargetMemoryUtilizationPercentage)
 			if memoryThresholdPercent < math.MinInt32 || memoryThresholdPercent > math.MaxInt32 {
-				return nil, fmt.Errorf("memoryThresholdPercent is out of range")
+				return nil, fmt.Errorf("memoryThresholdPercent is out of range of int32")
 			}
 			// nolint:gosec
 			autoscaling.MemoryThresholdPercent = int32(memoryThresholdPercent)