Share offline validation between job validate and job submit commands

cli/azd/extensions/azure.ai.customtraining/internal/cmd/job_submit.go

@@ -35,12 +35,23 @@ func newJobSubmitCommand() *cobra.Command {
 				return fmt.Errorf("--file (-f) is required: provide a path to a YAML job definition file")
 			}
 
-			// Parse and validate the YAML job definition
+			// Parse the YAML job definition
 			jobDef, err := utils.ParseJobFile(filePath)
 			if err != nil {
 				return err
 			}
 
+			// Run shared offline validation up-front (same checks as `job validate`).
+			// On any error finding, abort before doing any network or upload work.
+			yamlDir := filepath.Dir(filePath)
+			validation := utils.ValidateJobOffline(jobDef, yamlDir)
+			if len(validation.Findings) > 0 {
+				if err := utils.ReportValidationResult(filePath, validation, false); err != nil {
+					return err
+				}
+				fmt.Println()
+			}
+
 			azdClient, err := azdext.NewAzdClient()
 			if err != nil {
 				return fmt.Errorf("failed to create azd client: %w", err)
@@ -206,7 +217,7 @@ func buildJobResource(def *utils.JobDefinition) *models.JobResource {
 		}
 	}
 
-	// Services (e.g., SSH). Validation in ValidateJobDefinition restricts type to "ssh"
+	// Services (e.g., SSH). ValidateJobOffline restricts type to "ssh"
 	// and ensures ssh_public_keys is non-empty.
 	if len(def.Services) > 0 {
 		job.Services = make(map[string]interface{}, len(def.Services))
@@ -222,7 +233,7 @@ func buildJobResource(def *utils.JobDefinition) *models.JobResource {
 }
 
 // buildServiceRequest translates an AML YAML ServiceDefinition into the API request shape.
-// Currently only SSH is supported (enforced by ValidateJobDefinition).
+// Currently only SSH is supported (enforced by ValidateJobOffline).
 func buildServiceRequest(svc utils.ServiceDefinition) *models.JobServiceRequest {
 	req := &models.JobServiceRequest{
 		JobServiceType: mapServiceType(svc.Type),

cli/azd/extensions/azure.ai.customtraining/internal/cmd/job_validate.go

@@ -10,7 +10,6 @@ import (
 
 	"azure.ai.customtraining/internal/utils"
 
-	"github.com/fatih/color"
 	"github.com/spf13/cobra"
 	"gopkg.in/yaml.v3"
 )
@@ -24,7 +23,7 @@ func newJobValidateCommand() *cobra.Command {
 		Long:  "Validate a job YAML definition file offline without submitting.\n\nExample:\n  azd ai training job validate --file job.yaml",
 		// Override parent's PersistentPreRunE — validate is offline and needs no Azure setup.
 		PersistentPreRunE: func(_ *cobra.Command, _ []string) error { return nil },
-		Args: cobra.NoArgs,
+		Args:              cobra.NoArgs,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			if filePath == "" {
 				return fmt.Errorf("--file is required: provide a path to a YAML job definition file")
@@ -45,31 +44,7 @@ func newJobValidateCommand() *cobra.Command {
 			yamlDir := filepath.Dir(filePath)
 			result := utils.ValidateJobOffline(&jobDef, yamlDir)
 
-			// Print findings
-			if len(result.Findings) == 0 {
-				color.Green("✓ Validation passed: %s\n", filePath)
-				return nil
-			}
-
-			fmt.Printf("Validation results for: %s\n\n", filePath)
-
-			for _, f := range result.Findings {
-				prefix := "⚠"
-				if f.Severity == utils.SeverityError {
-					prefix = "✗"
-				}
-				fmt.Printf("  %s [%s] %s: %s\n", prefix, f.Severity, f.Field, f.Message)
-			}
-
-			fmt.Println()
-			fmt.Printf("  Errors: %d, Warnings: %d\n", result.ErrorCount(), result.WarningCount())
-
-			if result.HasErrors() {
-				return fmt.Errorf("validation failed with %d error(s)", result.ErrorCount())
-			}
-
-			color.Green("\n✓ Validation passed with warnings.\n")
-			return nil
+			return utils.ReportValidationResult(filePath, result, true)
 		},
 	}
 

cli/azd/extensions/azure.ai.customtraining/internal/utils/job_validator.go

@@ -10,6 +10,8 @@ import (
 	"reflect"
 	"regexp"
 	"strings"
+
+	"github.com/fatih/color"
 )
 
 // FindingSeverity indicates whether a finding is an error or a warning.
@@ -100,11 +102,21 @@ func ValidateJobOffline(job *JobDefinition, yamlDir string) *ValidationResult {
 		}
 	}
 
-	// 5. Local path existence checks
+	// 5. Local path existence checks + input type required.
+	// Inputs with `value:` are literals (type defaults to "literal" at submit
+	// time); inputs without `value:` carry a path/URI and must declare a type
+	// — otherwise the backend rejects with "Unexpected JobInputType in request body: []".
 	validateLocalPath(result, "code", job.Code, yamlDir)
 	for name, input := range job.Inputs {
 		if input.Value == "" {
 			validateLocalPath(result, fmt.Sprintf("inputs.%s.path", name), input.Path, yamlDir)
+			if strings.TrimSpace(input.Type) == "" {
+				result.Findings = append(result.Findings, ValidationFinding{
+					Field:    fmt.Sprintf("inputs.%s.type", name),
+					Severity: SeverityError,
+					Message:  "type is required (e.g. uri_folder, uri_file etc)",
+				})
+			}
 		}
 	}
 
@@ -116,9 +128,92 @@ func ValidateJobOffline(job *JobDefinition, yamlDir string) *ValidationResult {
 		validateInputOutputDefinitions(result, job, optionalInputs)
 	}
 
+	// 9. Outputs:
+	//    a) "default" is reserved by the backend and rejected at submit time
+	//       with a 400 ("Name of the output \"default\" is reserved by the system").
+	//    b) Each declared output must have a non-empty type — the backend rejects
+	//       missing/empty type with "Unexpected JobOutputType in request body: []".
+	//    Catch both offline so users don't have to wait for the backend round-trip.
+	for name, output := range job.Outputs {
+		if strings.EqualFold(name, "default") {
+			result.Findings = append(result.Findings, ValidationFinding{
+				Field:    fmt.Sprintf("outputs.%s", name),
+				Severity: SeverityError,
+				Message:  "output name 'default' is reserved by the system; use a different name",
+			})
+		}
+		if strings.TrimSpace(output.Type) == "" {
+			result.Findings = append(result.Findings, ValidationFinding{
+				Field:    fmt.Sprintf("outputs.%s.type", name),
+				Severity: SeverityError,
+				Message:  "type is required (e.g. uri_folder, uri_file etc)",
+			})
+		}
+	}
+
+	// 10. Services: only "ssh" is supported, and ssh_public_keys is required.
+	// The backend currently does not enforce key presence — without keys the SSH
+	// service is provisioned but unusable, and users hit the failure later.
+	for name, svc := range job.Services {
+		if !strings.EqualFold(svc.Type, "ssh") {
+			result.Findings = append(result.Findings, ValidationFinding{
+				Field:    fmt.Sprintf("services.%s.type", name),
+				Severity: SeverityError,
+				Message:  fmt.Sprintf("type %q is not supported; only 'ssh' is allowed", svc.Type),
+			})
+			continue
+		}
+		if strings.TrimSpace(svc.SshPublicKeys) == "" {
+			result.Findings = append(result.Findings, ValidationFinding{
+				Field:    fmt.Sprintf("services.%s.ssh_public_keys", name),
+				Severity: SeverityError,
+				Message:  "ssh_public_keys is required when type is 'ssh'",
+			})
+		}
+	}
+
 	return result
 }
 
+// ReportValidationResult prints findings to stdout and returns an error if any
+// finding has Error severity. Shared by the `job validate` command (which calls
+// it as the entire command body) and the `job submit` command (which calls it
+// as a pre-flight check before any network or upload work).
+//
+// When printSuccess is true, a green success line is printed for clean and
+// warnings-only results. Submit passes false so the success message doesn't
+// clutter its own "Submitting command job…" output flow.
+func ReportValidationResult(filePath string, result *ValidationResult, printSuccess bool) error {
+	if len(result.Findings) == 0 {
+		if printSuccess {
+			color.Green("✓ Validation passed: %s\n", filePath)
+		}
+		return nil
+	}
+
+	fmt.Printf("Validation results for: %s\n\n", filePath)
+
+	for _, f := range result.Findings {
+		prefix := "⚠"
+		if f.Severity == SeverityError {
+			prefix = "✗"
+		}
+		fmt.Printf("  %s [%s] %s: %s\n", prefix, f.Severity, f.Field, f.Message)
+	}
+
+	fmt.Println()
+	fmt.Printf("  Errors: %d, Warnings: %d\n", result.ErrorCount(), result.WarningCount())
+
+	if result.HasErrors() {
+		return fmt.Errorf("validation failed with %d error(s)", result.ErrorCount())
+	}
+
+	if printSuccess {
+		color.Green("\n✓ Validation passed with warnings.\n")
+	}
+	return nil
+}
+
 // validateLocalPath checks that a local path exists on disk.
 // Remote URIs (azureml://, https://, http://) and empty paths are skipped.
 func validateLocalPath(result *ValidationResult, field string, path string, yamlDir string) {
@@ -252,9 +347,10 @@ func validateSingleBracePlaceholders(result *ValidationResult, command string) {
 }
 
 // validateInputOutputDefinitions checks that inputs/outputs referenced in command
+// validateInputOutputDefinitions verifies that inputs referenced in the command
 // are not empty/nil definitions (all fields zero-valued).
-// Empty inputs are errors; empty outputs are warnings (backend uses defaults).
 // Inputs inside [...] optional blocks are skipped — empty definitions are valid for optional inputs.
+// Outputs are validated separately in ValidateJobOffline (rule 9).
 func validateInputOutputDefinitions(result *ValidationResult, job *JobDefinition, optionalInputs map[string]bool) {
 	command := job.Command
 	seen := make(map[string]bool)
@@ -263,34 +359,26 @@ func validateInputOutputDefinitions(result *ValidationResult, job *JobDefinition
 		kind := match[1]
 		key := match[2]
 
+		if kind != "inputs" || job.Inputs == nil {
+			continue
+		}
+
 		dedupeKey := kind + "." + key
 		if seen[dedupeKey] {
 			continue
 		}
 		seen[dedupeKey] = true
 
-		if kind == "inputs" && job.Inputs != nil {
-			if optionalInputs[key] {
-				continue
-			}
-			if input, exists := job.Inputs[key]; exists {
-				if (input == InputDefinition{}) {
-					result.Findings = append(result.Findings, ValidationFinding{
-						Field:    fmt.Sprintf("inputs.%s", key),
-						Severity: SeverityError,
-						Message:  fmt.Sprintf("input '%s' is referenced in command but has an empty definition", key),
-					})
-				}
-			}
-		} else if kind == "outputs" && job.Outputs != nil {
-			if output, exists := job.Outputs[key]; exists {
-				if (output == OutputDefinition{}) {
-					result.Findings = append(result.Findings, ValidationFinding{
-						Field:    fmt.Sprintf("outputs.%s", key),
-						Severity: SeverityWarning,
-						Message:  fmt.Sprintf("output '%s' has an empty definition — default values will be used", key),
-					})
-				}
+		if optionalInputs[key] {
+			continue
+		}
+		if input, exists := job.Inputs[key]; exists {
+			if (input == InputDefinition{}) {
+				result.Findings = append(result.Findings, ValidationFinding{
+					Field:    fmt.Sprintf("inputs.%s", key),
+					Severity: SeverityError,
+					Message:  fmt.Sprintf("input '%s' is referenced in command but has an empty definition", key),
+				})
 			}
 		}
 	}