diff --git a/cmd/modelctl/commands/debug/serve-webui.go b/cmd/modelctl/commands/debug/serve-webui.go index b69ee978..8af949a9 100644 --- a/cmd/modelctl/commands/debug/serve-webui.go +++ b/cmd/modelctl/commands/debug/serve-webui.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/canonical/inference-snaps-cli/v2/cmd/modelctl/common" + "github.com/canonical/inference-snaps-cli/v2/pkg/models" "github.com/canonical/inference-snaps-cli/v2/pkg/webui" "github.com/spf13/cobra" ) @@ -45,7 +46,7 @@ func (cmd *serveWebUiCommand) serveWebUi(_ *cobra.Command, args []string) error config := webui.Config{ OpenAIBaseURL: cmd.baseUrl, - Capabilities: common.SupportedCapabilities(), // set all capabilities for debugging + Capabilities: models.SupportedCapabilities(), // set all capabilities for debugging InstanceName: "debug", EngineName: "unset", } diff --git a/cmd/modelctl/commands/debug/validate.go b/cmd/modelctl/commands/debug/validate.go index 9cc1845f..f22354bc 100644 --- a/cmd/modelctl/commands/debug/validate.go +++ b/cmd/modelctl/commands/debug/validate.go @@ -2,9 +2,12 @@ package debug import ( "fmt" + "path/filepath" "github.com/canonical/inference-snaps-cli/v2/cmd/modelctl/common" "github.com/canonical/inference-snaps-cli/v2/pkg/engines" + "github.com/canonical/inference-snaps-cli/v2/pkg/models" + "github.com/canonical/inference-snaps-cli/v2/pkg/runtimes" "github.com/spf13/cobra" ) @@ -17,9 +20,9 @@ func ValidateCommand(ctx *common.Context) *cobra.Command { cmd.Context = ctx cobraCmd := &cobra.Command{ - Use: "validate-engines", - Short: "Validate engine manifest files", - Args: cobra.MinimumNArgs(1), + Use: "lint-package", + Short: "Validate engines, models, and runtimes manifest files", + Args: cobra.ExactArgs(1), ValidArgsFunction: cobra.NoFileCompletions, RunE: cmd.run, } @@ -28,23 +31,44 @@ func ValidateCommand(ctx *common.Context) *cobra.Command { } func (cmd *validateCommand) run(_ *cobra.Command, args []string) error { - if len(args) == 0 { - return fmt.Errorf("no engine manifest specified") + allManifestsValid := true + snapDir := args[0] + + validators := []struct { + subdir string + manifestFilename string + validate func(string) error + }{ + {"engines", engines.ManifestFilename, engines.Validate}, + {"models", models.ManifestFilename, models.Validate}, + {"runtimes", runtimes.ManifestFilename, runtimes.Validate}, } - allManifestsValid := true - for _, manifestPath := range args { - err := engines.Validate(manifestPath) + for _, v := range validators { + pattern := filepath.Join(snapDir, v.subdir, "*", v.manifestFilename) + manifestPaths, err := filepath.Glob(pattern) if err != nil { + return fmt.Errorf("invalid glob pattern %s: %w", pattern, err) + } + + if len(manifestPaths) == 0 { allManifestsValid = false - fmt.Printf("❌ %s: %s\n", manifestPath, err) - } else { - fmt.Printf("✅ %s\n", manifestPath) + fmt.Printf("❌ %s: no manifests found\n", filepath.Join(snapDir, v.subdir)) + continue + } + + for _, manifestPath := range manifestPaths { + if err := v.validate(manifestPath); err != nil { + allManifestsValid = false + fmt.Printf("❌ %s: %s\n", manifestPath, err) + } else { + fmt.Printf("✅ %s\n", manifestPath) + } } } if !allManifestsValid { - return fmt.Errorf("not all manifests are valid") + return fmt.Errorf("some manifests are invalid") } return nil } diff --git a/cmd/modelctl/common/model.go b/cmd/modelctl/common/model.go index 84c00d27..96c70a4b 100644 --- a/cmd/modelctl/common/model.go +++ b/cmd/modelctl/common/model.go @@ -10,13 +10,6 @@ import ( "github.com/canonical/inference-snaps-cli/v2/pkg/utils" ) -const ( - capabilityText string = "text" - capabilityVision string = "vision" - capabilityTools string = "tools" - capabilityThinking string = "thinking" -) - type ModelDetails struct { ID string `json:"id" yaml:"id"` Name string `json:"name" yaml:"name"` @@ -31,10 +24,6 @@ type ModelDetails struct { Components []string `json:"components" yaml:"components"` } -func SupportedCapabilities() []string { - return []string{capabilityText, capabilityVision, capabilityTools, capabilityThinking} -} - func NewModelDetails(manifest *models.Manifest) (ModelDetails, error) { var modelDetails ModelDetails modelDetails.ID = manifest.ID diff --git a/pkg/models/types.go b/pkg/models/types.go index 51321a50..d7c493ef 100644 --- a/pkg/models/types.go +++ b/pkg/models/types.go @@ -4,6 +4,13 @@ import ( "github.com/canonical/inference-snaps-cli/v2/pkg/engines" ) +const ( + capabilityText string = "text" + capabilityVision string = "vision" + capabilityTools string = "tools" + capabilityThinking string = "thinking" +) + type Manifest struct { ID string `json:"id" yaml:"id"` Name string `json:"name" yaml:"name"` @@ -21,3 +28,7 @@ type Manifest struct { Layout map[string]engines.Layout `json:"layout,omitempty" yaml:"layout,omitempty"` } + +func SupportedCapabilities() []string { + return []string{capabilityText, capabilityVision, capabilityTools, capabilityThinking} +} diff --git a/pkg/models/validate.go b/pkg/models/validate.go new file mode 100644 index 00000000..21e1150c --- /dev/null +++ b/pkg/models/validate.go @@ -0,0 +1,135 @@ +package models + +import ( + "bytes" + "errors" + "fmt" + "net/url" + "os" + "path/filepath" + "regexp" + "slices" + + "github.com/canonical/inference-snaps-cli/v2/pkg/utils" + "go.yaml.in/yaml/v4" +) + +// diskSizePattern matches a human-readable size such as "6G" or "512M". +var diskSizePattern = regexp.MustCompile(`^\d+(\.\d+)?(K|M|G|T)i?B?$`) + +func Validate(manifestFilePath string) error { + + if filepath.Base(manifestFilePath) != ManifestFilename { + return fmt.Errorf("manifest file must be called %s: %s", ManifestFilename, manifestFilePath) + } + + _, err := os.Stat(manifestFilePath) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("manifest file does not exist: %s", manifestFilePath) + } + return fmt.Errorf("getting file info: %v", err) + } + + yamlData, err := os.ReadFile(manifestFilePath) + if err != nil { + return fmt.Errorf("reading file: %v", err) + } + + // Get model ID from the directory name + modelId := modelIdFromPath(manifestFilePath) + + return validateManifestYaml(modelId, yamlData) +} + +func modelIdFromPath(manifestFilePath string) string { + parts := utils.SplitPathIntoDirectories(manifestFilePath) + if len(parts) < 2 { + return "" + } + return parts[len(parts)-2] // second last part: model-id/model.yaml +} + +func validateManifestYaml(expectedName string, yamlData []byte) error { + yamlData = bytes.TrimSpace(yamlData) + if len(yamlData) == 0 { + return errors.New("empty yaml data") + } + + var manifest Manifest + + yamlDecoder := yaml.NewDecoder(bytes.NewReader(yamlData)) + + // Error if there are unknown fields in the yaml + yamlDecoder.KnownFields(true) + + // We depend on the yaml unmarshal to check field types + if err := yamlDecoder.Decode(&manifest); err != nil { + return fmt.Errorf("decoding manifest: %v", err) + } + + return manifest.validate(expectedName) +} + +func (manifest Manifest) validate(expectedModelId string) error { + if manifest.ID == "" { + return fmt.Errorf("required field is not set: id") + } + + if expectedModelId != "" { + if manifest.ID != expectedModelId { + return fmt.Errorf("model directory name should match id in manifest: %s != %s", expectedModelId, manifest.ID) + } + } + + if manifest.Name == "" { + return fmt.Errorf("required field is not set: name") + } + + if manifest.Description == "" { + return fmt.Errorf("required field is not set: description") + } + + if manifest.ModelCardUrl == "" { + return fmt.Errorf("required field is not set: model-card-url") + } + if u, err := url.ParseRequestURI(manifest.ModelCardUrl); err != nil || u.Scheme == "" || u.Host == "" { + return fmt.Errorf("invalid model-card-url: %s", manifest.ModelCardUrl) + } + + if manifest.Quantization == "" { + return fmt.Errorf("required field is not set: quantization") + } + + if len(manifest.Capabilities) == 0 { + return fmt.Errorf("required field is not set: capabilities") + } + for _, cap := range manifest.Capabilities { + if !slices.Contains(SupportedCapabilities(), cap) { + return fmt.Errorf("unsupported capability: %q", cap) + } + } + + if manifest.DiskSize == "" { + return fmt.Errorf("required field is not set: disk-size") + } + if !diskSizePattern.MatchString(manifest.DiskSize) { + return fmt.Errorf("invalid disk-size format: %s", manifest.DiskSize) + } + + if len(manifest.Components) == 0 { + return fmt.Errorf("required field is not set: components") + } + + if len(manifest.Environment) == 0 { + return fmt.Errorf("required field is not set: environment") + } + + for target, layout := range manifest.Layout { + if layout.Symlink == "" { + return fmt.Errorf("layout %q: required field is not set: symlink", target) + } + } + + return nil +} diff --git a/pkg/models/validate_test.go b/pkg/models/validate_test.go new file mode 100644 index 00000000..def5dca2 --- /dev/null +++ b/pkg/models/validate_test.go @@ -0,0 +1,175 @@ +package models + +import ( + "os" + "path/filepath" + "testing" + + "gopkg.in/yaml.v3" +) + +func templateManifest() Manifest { + manifest := Manifest{ + ID: "test", + Name: "test", + Description: "test", + ModelCardUrl: "https://example.com/model-card", + Quantization: "Q4_K_M", + Capabilities: []string{"text"}, + DiskSize: "6G", + Components: []string{"test-component"}, + Environment: []string{"MODEL_FILE=/tmp/model.gguf"}, + } + return manifest +} + +func TestManifestFiles(t *testing.T) { + modelsDir := "../../test_data/models" + + _, err := os.ReadDir(modelsDir) + if err != nil { + t.Fatalf("Failed reading models directory: %v", err) + } + model := "26b-q4-k-m-gguf" + manifestPath := filepath.Join(modelsDir, model, ManifestFilename) + t.Run(model, func(t *testing.T) { + err = Validate(manifestPath) + if err != nil { + t.Fatalf("%s: %v", model, err) + } + }) +} + +func TestManifestYamlUnsupportedCapability(t *testing.T) { + modelsDir := "../../test_data/models" + + _, err := os.ReadDir(modelsDir) + if err != nil { + t.Fatalf("Failed reading models directory: %v", err) + } + model := "30b-a3b-q4-k-m-gguf" + manifestPath := filepath.Join(modelsDir, model, ManifestFilename) + t.Run(model, func(t *testing.T) { + err = Validate(manifestPath) + if err == nil { + t.Fatalf("%s: expected an error for unsupported capability, got nil", model) + } + }) +} +func TestManifestEmpty(t *testing.T) { + data := "" + err := validateManifestYaml("", []byte(data)) + if err == nil { + t.Fatal("Empty yaml should fail") + } +} + +func TestUnknownField(t *testing.T) { + data, _ := yaml.Marshal(templateManifest()) + data = append(data, []byte("unknown-field: test\n")...) + + err := validateManifestYaml("test", data) + if err == nil { + t.Fatal("Unknown field should fail") + } +} + +func TestIdRequired(t *testing.T) { + manifest := templateManifest() + manifest.ID = "" + + err := manifest.validate("test") + if err == nil { + t.Fatal("id field is required") + } +} + +func TestNameRequired(t *testing.T) { + manifest := templateManifest() + manifest.Name = "" + + err := manifest.validate("test") + if err == nil { + t.Fatal("name field is required") + } +} + +func TestDescriptionRequired(t *testing.T) { + manifest := templateManifest() + manifest.Description = "" + + err := manifest.validate("test") + if err == nil { + t.Fatal("description field is required") + } +} + +func TestModelCardUrlRequired(t *testing.T) { + manifest := templateManifest() + manifest.ModelCardUrl = "" + + err := manifest.validate("test") + if err == nil { + t.Fatal("model-card-url field is required") + } +} + +func TestQuantizationRequired(t *testing.T) { + manifest := templateManifest() + manifest.Quantization = "" + + err := manifest.validate("test") + if err == nil { + t.Fatal("quantization field is required") + } +} + +func TestCapabilitiesRequired(t *testing.T) { + manifest := templateManifest() + manifest.Capabilities = nil + + err := manifest.validate("test") + if err == nil { + t.Fatal("capabilities field is required") + } +} + +func TestDiskSizeRequired(t *testing.T) { + manifest := templateManifest() + manifest.DiskSize = "" + + err := manifest.validate("test") + if err == nil { + t.Fatal("disk-size field is required") + } +} + +func TestComponentsRequired(t *testing.T) { + manifest := templateManifest() + manifest.Components = nil + + err := manifest.validate("test") + if err == nil { + t.Fatal("components field is required") + } +} + +func TestEnvironmentRequired(t *testing.T) { + manifest := templateManifest() + manifest.Environment = nil + + err := manifest.validate("test") + if err == nil { + t.Fatal("environment field is required") + } +} + +func TestModelIdMatch(t *testing.T) { + manifest := templateManifest() + manifest.ID = "different-id" + + err := manifest.validate("test") + if err == nil { + t.Fatal("model directory name should match id in manifest") + } +} diff --git a/pkg/runtimes/validate.go b/pkg/runtimes/validate.go new file mode 100644 index 00000000..6d879265 --- /dev/null +++ b/pkg/runtimes/validate.go @@ -0,0 +1,117 @@ +package runtimes + +import ( + "bytes" + "errors" + "fmt" + "os" + "path/filepath" + + "github.com/canonical/inference-snaps-cli/v2/pkg/utils" + "go.yaml.in/yaml/v4" +) + +func Validate(manifestFilePath string) error { + + if filepath.Base(manifestFilePath) != ManifestFilename { + return fmt.Errorf("manifest file must be called %s: %s", ManifestFilename, manifestFilePath) + } + + _, err := os.Stat(manifestFilePath) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("manifest file does not exist: %s", manifestFilePath) + } + return fmt.Errorf("getting file info: %v", err) + } + + yamlData, err := os.ReadFile(manifestFilePath) + if err != nil { + return fmt.Errorf("reading file: %v", err) + } + + // Get runtime name from path + runtimeName := runtimeNameFromPath(manifestFilePath) + + return validateManifestYaml(runtimeName, yamlData) +} + +func runtimeNameFromPath(manifestFilePath string) string { + parts := utils.SplitPathIntoDirectories(manifestFilePath) + if len(parts) < 2 { + return "" + } + return parts[len(parts)-2] // second last part: runtime-name/runtime.yaml +} + +func validateManifestYaml(expectedName string, yamlData []byte) error { + yamlData = bytes.TrimSpace(yamlData) + if len(yamlData) == 0 { + return errors.New("empty yaml data") + } + + var manifest Manifest + + yamlDecoder := yaml.NewDecoder(bytes.NewReader(yamlData)) + + // Error if there are unknown fields in the yaml + yamlDecoder.KnownFields(true) + + // We depend on the yaml unmarshal to check field types + if err := yamlDecoder.Decode(&manifest); err != nil { + return fmt.Errorf("decoding manifest: %v", err) + } + + return manifest.validate(expectedName) +} + +func (manifest Manifest) validate(expectedRuntimeName string) error { + if manifest.Name == "" { + return fmt.Errorf("required field is not set: name") + } + + // Only do runtime name matching test if expected name is set + if expectedRuntimeName != "" { + if manifest.Name != expectedRuntimeName { + return fmt.Errorf("runtime directory name should match name in manifest: %s != %s", expectedRuntimeName, manifest.Name) + } + } + + if len(manifest.Servers) == 0 { + return fmt.Errorf("required field is not set: servers") + } + + for name, server := range manifest.Servers { + if err := server.validate(name); err != nil { + return err + } + } + + if len(manifest.Environment) == 0 { + return fmt.Errorf("required field is not set: environment") + } + + if len(manifest.Components) == 0 { + return fmt.Errorf("required field is not set: components") + } + + for target, layout := range manifest.Layout { + if layout.Symlink == "" { + return fmt.Errorf("layout %q: required field is not set: symlink", target) + } + } + + return nil +} + +func (server Server) validate(name string) error { + if server.Protocol == "" { + return fmt.Errorf("required field is not set for server %s: protocol", name) + } + + if server.BasePath == "" { + return fmt.Errorf("required field is not set for server %s: base-path", name) + } + + return nil +} diff --git a/pkg/runtimes/validate_test.go b/pkg/runtimes/validate_test.go new file mode 100644 index 00000000..0f0a507c --- /dev/null +++ b/pkg/runtimes/validate_test.go @@ -0,0 +1,142 @@ +package runtimes + +import ( + "os" + "path/filepath" + "testing" + + "gopkg.in/yaml.v3" +) + +func templateManifest() Manifest { + manifest := Manifest{ + Name: "test", + Servers: map[string]Server{ + "openai": { + Protocol: "http", + BasePath: "/v1", + }, + }, + Environment: []string{"PATH=$PATH:$SNAP_COMPONENTS/runtime-test/bin"}, + Components: []string{"runtime-test"}, + } + return manifest +} + +func TestManifestFiles(t *testing.T) { + runtimesDir := "../../test_data/runtimes" + + entries, err := os.ReadDir(runtimesDir) + if err != nil { + t.Fatalf("Failed reading runtimes directory: %v", err) + } + + for _, entry := range entries { + if entry.IsDir() { + runtime := entry.Name() + manifestPath := filepath.Join(runtimesDir, runtime, ManifestFilename) + t.Run(runtime, func(t *testing.T) { + err = Validate(manifestPath) + if err != nil { + t.Fatalf("%s: %v", runtime, err) + } + }) + } + } +} + +func TestManifestEmpty(t *testing.T) { + data := "" + err := validateManifestYaml("", []byte(data)) + if err == nil { + t.Fatal("Empty yaml should fail") + } +} + +func TestUnknownField(t *testing.T) { + data, _ := yaml.Marshal(templateManifest()) + data = append(data, []byte("unknown-field: test\n")...) + + err := validateManifestYaml("test", data) + if err == nil { + t.Fatal("Unknown field should fail") + } +} + +func TestNameRequired(t *testing.T) { + manifest := templateManifest() + manifest.Name = "" + + err := manifest.validate("test") + if err == nil { + t.Fatal("name field is required") + } +} + +func TestNameMatch(t *testing.T) { + manifest := templateManifest() + manifest.Name = "different-name" + + err := manifest.validate("test") + if err == nil { + t.Fatal("runtime directory name should match name in manifest") + } +} + +func TestServersRequired(t *testing.T) { + manifest := templateManifest() + manifest.Servers = nil + + err := manifest.validate("test") + if err == nil { + t.Fatal("servers field is required") + } +} + +func TestServerProtocolRequired(t *testing.T) { + manifest := templateManifest() + manifest.Servers = map[string]Server{ + "openai": { + BasePath: "/v1", + }, + } + + err := manifest.validate("test") + if err == nil { + t.Fatal("server protocol field is required") + } +} + +func TestServerBasePathRequired(t *testing.T) { + manifest := templateManifest() + manifest.Servers = map[string]Server{ + "openai": { + Protocol: "http", + }, + } + + err := manifest.validate("test") + if err == nil { + t.Fatal("server base-path field is required") + } +} + +func TestEnvironmentRequired(t *testing.T) { + manifest := templateManifest() + manifest.Environment = nil + + err := manifest.validate("test") + if err == nil { + t.Fatal("environment field is required") + } +} + +func TestComponentsRequired(t *testing.T) { + manifest := templateManifest() + manifest.Components = nil + + err := manifest.validate("test") + if err == nil { + t.Fatal("components field is required") + } +}