fix: harden schema migration guidance #368

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: Copilot

cmd/waza/cmd_migrate.go

@@ -1,13 +1,15 @@
 package main
 
 import (
+	"encoding/json"
 	"fmt"
 	"io"
 	"os"
 	"path/filepath"
 
 	"github.com/microsoft/waza/internal/models"
 	"github.com/spf13/cobra"
+	"gopkg.in/yaml.v3"
 )
 
 func newMigrateCommand() *cobra.Command {
@@ -28,20 +30,52 @@ migration steps here.`,
 }
 
 func runMigrate(out io.Writer, path string) error {
-	if _, err := os.Stat(path); err != nil {
+	data, err := os.ReadFile(path)
+	if err != nil {
 		return fmt.Errorf("reading %s: %w", path, err)
 	}
 
-	artifact := "artifact"
+	artifact, version, err := readArtifactSchemaVersion(path, data)
+	if err != nil {
+		return err
+	}
+
+	version, err = models.ValidateSchemaVersion(artifact, path, version)
+	if err != nil {
+		return err
+	}
+
+	if version != models.CurrentSchemaVersion {
+		_, err = fmt.Fprintf(out, "%s uses schemaVersion %s, which is compatible with waza schemaVersion %s; no migration needed.\n", artifact, version, models.CurrentSchemaVersion)
+		return err
+	}
+
+	_, err = fmt.Fprintf(out, "%s is already compatible with schemaVersion %s; no migration needed.\n", artifact, models.CurrentSchemaVersion)
+	return err
+}
+
+func readArtifactSchemaVersion(path string, data []byte) (artifact string, version string, err error) {
 	switch filepath.Base(path) {
 	case "eval.yaml", "eval.yml":
 		artifact = "eval.yaml"
+		var header struct {
+			SchemaVersion string `yaml:"schemaVersion"`
+		}
+		if err := yaml.Unmarshal(data, &header); err != nil {
+			return "", "", fmt.Errorf("parsing %s: %w", path, err)
+		}
+		return artifact, header.SchemaVersion, nil
 	default:
 		if filepath.Ext(path) == ".json" {
 			artifact = "results.json"
+			var header struct {
+				SchemaVersion string `json:"schemaVersion"`
+			}
+			if err := json.Unmarshal(data, &header); err != nil {
+				return "", "", fmt.Errorf("parsing %s: %w", path, err)
+			}
+			return artifact, header.SchemaVersion, nil
 		}
 	}
-
-	_, err := fmt.Fprintf(out, "%s is already compatible with schemaVersion %s; no migration needed.\n", artifact, models.CurrentSchemaVersion)
-	return err
+	return "", "", fmt.Errorf("unsupported schema artifact %s: expected eval.yaml, eval.yml, or a JSON results artifact", path)
 }

cmd/waza/cmd_migrate_test.go

@@ -21,6 +21,31 @@ func TestMigrateCommandNoOpForCurrentMajor(t *testing.T) {
 	require.Contains(t, out.String(), "already compatible with schemaVersion 1.0")
 }
 
+func TestMigrateCommandRejectsIncompatibleMajor(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "results.json")
+	require.NoError(t, os.WriteFile(path, []byte(`{"schemaVersion":"2.0"}`), 0o644))
+
+	var out bytes.Buffer
+	err := runMigrate(&out, path)
+
+	require.Error(t, err)
+	require.Contains(t, err.Error(), `waza migrate <file>`)
+	require.Empty(t, out.String())
+}
+
+func TestMigrateCommandDefaultsMissingSchemaVersion(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "results.json")
+	require.NoError(t, os.WriteFile(path, []byte(`{"eval_id":"run-1"}`), 0o644))
+
+	var out bytes.Buffer
+	err := runMigrate(&out, path)
+
+	require.NoError(t, err)
+	require.Contains(t, out.String(), "already compatible with schemaVersion 1.0")
+}
+
 func TestMigrateCommandMissingFile(t *testing.T) {
 	var out bytes.Buffer
 	err := runMigrate(&out, filepath.Join(t.TempDir(), "missing.json"))

internal/models/schema_version.go

@@ -25,7 +25,7 @@ func defaultSchemaVersion(version string) string {
 	return version
 }
 
-func validateSchemaVersion(artifact, source, version string) (string, error) {
+func ValidateSchemaVersion(artifact, source, version string) (string, error) {
 	version = defaultSchemaVersion(version)
 
 	major, _, err := parseSchemaVersion(version)
@@ -37,7 +37,7 @@ func validateSchemaVersion(artifact, source, version string) (string, error) {
 		return "", err
 	}
 	if major != currentMajor {
-		return "", fmt.Errorf("%s %s uses schemaVersion %q, but this waza supports schemaVersion %s; run \"waza migrate %s\" to migrate the file", artifact, source, version, CurrentSchemaVersion, source)
+		return "", fmt.Errorf("%s %s uses schemaVersion %q, but this waza supports schemaVersion %s; run \"waza migrate <file>\" to migrate the artifact file", artifact, source, version, CurrentSchemaVersion)
 	}
 	return version, nil
 }
@@ -121,7 +121,7 @@ func ParseEvaluationOutcome(data []byte, source string) (*EvaluationOutcome, err
 	if err := json.Unmarshal(data, &header); err != nil {
 		return nil, err
 	}
-	version, err := validateSchemaVersion("results.json", source, header.SchemaVersion)
+	version, err := ValidateSchemaVersion("results.json", source, header.SchemaVersion)
 	if err != nil {
 		return nil, err
 	}

internal/models/spec.go

@@ -309,7 +309,7 @@ func LoadEvalSpec(path string) (*EvalSpec, error) {
 	if err := yaml.Unmarshal(data, &header); err != nil {
 		return nil, fmt.Errorf("parsing eval spec YAML (%s): %w", path, err)
 	}
-	version, err := validateSchemaVersion("eval.yaml", path, header.SchemaVersion)
+	version, err := ValidateSchemaVersion("eval.yaml", path, header.SchemaVersion)
 	if err != nil {
 		return nil, err
 	}

internal/storage/azure_blob.go

@@ -239,7 +239,7 @@ func (abs *AzureBlobStore) Download(ctx context.Context, runID string) (*models.
 		return nil, fmt.Errorf("azure blob download: reading blob: %w", err)
 	}
 
-	outcome, err := models.ParseEvaluationOutcome(data, runID)
+	outcome, err := models.ParseEvaluationOutcome(data, blobPath)
 	if err != nil {
 		return nil, fmt.Errorf("azure blob download: unmarshaling outcome: %w", err)
 	}