Merge pull request #26 from stacklok/oci-skills-migrate-ocispec-types

Migrate custom OCI types to ocispec in oci/skills

Author: JAORMX

env/env.go

@@ -3,7 +3,7 @@
 
 package env
 
-//go:generate mockgen -source=env.go -destination=mocks/mock_reader.go -package=mocks Reader
+//go:generate mockgen -copyright_file=../.github/license-header.txt -source=env.go -destination=mocks/mock_reader.go -package=mocks Reader
 
 import "os"
 

env/mocks/mock_reader.go

@@ -3,13 +3,14 @@
 
 package skills
 
-//go:generate mockgen -source=interfaces.go -destination=mocks/mock_interfaces.go -package=mocks
+//go:generate mockgen -copyright_file=../../.github/license-header.txt -source=interfaces.go -destination=mocks/mock_interfaces.go -package=mocks
 
 import (
 	"context"
 	"time"
 
 	"github.com/opencontainers/go-digest"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
 // RegistryClient provides remote OCI registry operations for skills.
@@ -34,7 +35,7 @@ type PackageOptions struct {
 
 	// Platforms specifies target platforms for the image index.
 	// If empty, defaults to DefaultPlatforms.
-	Platforms []Platform
+	Platforms []ocispec.Platform
 }
 
 // PackageResult contains the result of packaging a skill.
@@ -44,5 +45,5 @@ type PackageResult struct {
 	ConfigDigest   digest.Digest
 	LayerDigest    digest.Digest
 	Config         *SkillConfig
-	Platforms      []Platform
+	Platforms      []ocispec.Platform
 }

oci/skills/interfaces.go

@@ -7,6 +7,8 @@ import (
 	"encoding/json"
 	"fmt"
 	"strings"
+
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
 // Artifact type for skill identification.
@@ -15,29 +17,8 @@ const (
 	ArtifactTypeSkill = "dev.toolhive.skills.v1"
 )
 
-// OCI Image Index media type.
-const (
-	// MediaTypeImageIndex is the OCI image index media type.
-	MediaTypeImageIndex = "application/vnd.oci.image.index.v1+json"
-)
-
-// Standard OCI media types for Kubernetes image volume compatibility.
-const (
-	// MediaTypeImageManifest is the OCI image manifest media type.
-	MediaTypeImageManifest = "application/vnd.oci.image.manifest.v1+json"
-
-	// MediaTypeImageConfig is the standard OCI image config media type.
-	MediaTypeImageConfig = "application/vnd.oci.image.config.v1+json"
-
-	// MediaTypeImageLayer is the standard OCI image layer media type.
-	MediaTypeImageLayer = "application/vnd.oci.image.layer.v1.tar+gzip"
-)
-
 // Annotation keys for skill metadata in manifests.
 const (
-	// AnnotationCreated is the OCI standard annotation for creation time.
-	AnnotationCreated = "org.opencontainers.image.created"
-
 	// AnnotationSkillName is the annotation key for skill name.
 	AnnotationSkillName = "dev.toolhive.skills.name"
 
@@ -84,35 +65,8 @@ type SkillConfig struct {
 	Files         []string          `json:"files"`
 }
 
-// ImageConfig represents a standard OCI image configuration.
-// This structure is required for Kubernetes image volume compatibility.
-type ImageConfig struct {
-	Architecture string          `json:"architecture"`
-	OS           string          `json:"os"`
-	Config       ImageConfigData `json:"config,omitempty"`
-	RootFS       RootFS          `json:"rootfs"`
-	History      []HistoryEntry  `json:"history,omitempty"`
-}
-
-// ImageConfigData contains container configuration including labels.
-type ImageConfigData struct {
-	Labels map[string]string `json:"Labels,omitempty"`
-}
-
-// RootFS describes the rootfs of the image.
-type RootFS struct {
-	Type    string   `json:"type"`
-	DiffIDs []string `json:"diff_ids"`
-}
-
-// HistoryEntry describes a layer in the image history.
-type HistoryEntry struct {
-	Created   string `json:"created,omitempty"`
-	CreatedBy string `json:"created_by,omitempty"`
-}
-
 // SkillConfigFromImageConfig extracts SkillConfig from OCI image config labels.
-func SkillConfigFromImageConfig(imgConfig *ImageConfig) (*SkillConfig, error) {
+func SkillConfigFromImageConfig(imgConfig *ocispec.Image) (*SkillConfig, error) {
 	if imgConfig == nil {
 		return nil, fmt.Errorf("image config is nil")
 	}
@@ -149,56 +103,44 @@ func SkillConfigFromImageConfig(imgConfig *ImageConfig) (*SkillConfig, error) {
 	return config, nil
 }
 
-// Platform represents a target platform for OCI artifacts.
-type Platform struct {
-	Architecture string `json:"architecture"`
-	OS           string `json:"os"`
-}
-
-// String returns the platform in "os/arch" format.
-func (p Platform) String() string {
-	return p.OS + "/" + p.Architecture
+// PlatformString returns the platform in "os/arch" or "os/arch/variant" format.
+func PlatformString(p ocispec.Platform) string {
+	s := p.OS + "/" + p.Architecture
+	if p.Variant != "" {
+		s += "/" + p.Variant
+	}
+	return s
 }
 
-// ParsePlatform parses a platform string in "os/arch" format.
-func ParsePlatform(s string) (Platform, error) {
+// ParsePlatform parses a platform string in "os/arch" or "os/arch/variant" format.
+func ParsePlatform(s string) (ocispec.Platform, error) {
 	parts := strings.Split(s, "/")
-	if len(parts) != 2 {
-		return Platform{}, fmt.Errorf("invalid platform format: %q (expected os/arch)", s)
+	if len(parts) < 2 || len(parts) > 3 {
+		return ocispec.Platform{}, fmt.Errorf("invalid platform format: %q (expected os/arch or os/arch/variant)", s)
 	}
 	osName := strings.TrimSpace(parts[0])
 	arch := strings.TrimSpace(parts[1])
 	if osName == "" || arch == "" {
-		return Platform{}, fmt.Errorf("invalid platform format: %q (os and arch cannot be empty)", s)
+		return ocispec.Platform{}, fmt.Errorf("invalid platform format: %q (os and arch cannot be empty)", s)
 	}
-	return Platform{OS: osName, Architecture: arch}, nil
+	p := ocispec.Platform{OS: osName, Architecture: arch}
+	if len(parts) == 3 {
+		variant := strings.TrimSpace(parts[2])
+		if variant == "" {
+			return ocispec.Platform{}, fmt.Errorf("invalid platform format: %q (variant cannot be empty)", s)
+		}
+		p.Variant = variant
+	}
+	return p, nil
 }
 
 // DefaultPlatforms are the default platforms for skill artifacts.
 // These cover most Kubernetes clusters.
-var DefaultPlatforms = []Platform{
+var DefaultPlatforms = []ocispec.Platform{
 	{OS: "linux", Architecture: "amd64"},
 	{OS: "linux", Architecture: "arm64"},
 }
 
-// ImageIndex represents an OCI image index (multi-platform manifest list).
-type ImageIndex struct {
-	SchemaVersion int               `json:"schemaVersion"`
-	MediaType     string            `json:"mediaType"`
-	ArtifactType  string            `json:"artifactType,omitempty"`
-	Manifests     []IndexDescriptor `json:"manifests"`
-	Annotations   map[string]string `json:"annotations,omitempty"`
-}
-
-// IndexDescriptor describes a manifest in an image index.
-type IndexDescriptor struct {
-	MediaType   string            `json:"mediaType"`
-	Digest      string            `json:"digest"`
-	Size        int64             `json:"size"`
-	Platform    *Platform         `json:"platform,omitempty"`
-	Annotations map[string]string `json:"annotations,omitempty"`
-}
-
 // ParseRequiresAnnotation parses skill dependency references from manifest annotations.
 // Returns nil if the annotation is missing or invalid.
 func ParseRequiresAnnotation(annotations map[string]string) []string {

oci/skills/mediatypes.go

@@ -6,6 +6,7 @@ package skills
 import (
 	"testing"
 
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -15,16 +16,16 @@ func TestSkillConfigFromImageConfig(t *testing.T) {
 
 	tests := []struct {
 		name      string
-		config    *ImageConfig
+		config    *ocispec.Image
 		wantName  string
 		wantErr   bool
 		wantTools []string
 		wantFiles []string
 	}{
 		{
 			name: "all fields populated",
-			config: &ImageConfig{
-				Config: ImageConfigData{
+			config: &ocispec.Image{
+				Config: ocispec.ImageConfig{
 					Labels: map[string]string{
 						LabelSkillName:         "my-skill",
 						LabelSkillDescription:  "A test skill",
@@ -41,8 +42,8 @@ func TestSkillConfigFromImageConfig(t *testing.T) {
 		},
 		{
 			name: "minimal config",
-			config: &ImageConfig{
-				Config: ImageConfigData{
+			config: &ocispec.Image{
+				Config: ocispec.ImageConfig{
 					Labels: map[string]string{
 						LabelSkillName: "minimal-skill",
 					},
@@ -57,15 +58,15 @@ func TestSkillConfigFromImageConfig(t *testing.T) {
 		},
 		{
 			name: "nil labels",
-			config: &ImageConfig{
-				Config: ImageConfigData{Labels: nil},
+			config: &ocispec.Image{
+				Config: ocispec.ImageConfig{Labels: nil},
 			},
 			wantErr: true,
 		},
 		{
 			name: "missing name",
-			config: &ImageConfig{
-				Config: ImageConfigData{
+			config: &ocispec.Image{
+				Config: ocispec.ImageConfig{
 					Labels: map[string]string{
 						LabelSkillDescription: "no name",
 					},
@@ -75,8 +76,8 @@ func TestSkillConfigFromImageConfig(t *testing.T) {
 		},
 		{
 			name: "invalid allowed tools JSON",
-			config: &ImageConfig{
-				Config: ImageConfigData{
+			config: &ocispec.Image{
+				Config: ocispec.ImageConfig{
 					Labels: map[string]string{
 						LabelSkillName:         "bad-tools",
 						LabelSkillAllowedTools: "not-json",
@@ -87,8 +88,8 @@ func TestSkillConfigFromImageConfig(t *testing.T) {
 		},
 		{
 			name: "invalid files JSON",
-			config: &ImageConfig{
-				Config: ImageConfigData{
+			config: &ocispec.Image{
+				Config: ocispec.ImageConfig{
 					Labels: map[string]string{
 						LabelSkillName:  "bad-files",
 						LabelSkillFiles: "not-json",
@@ -126,18 +127,23 @@ func TestParsePlatform(t *testing.T) {
 	tests := []struct {
 		name    string
 		input   string
-		want    Platform
+		want    ocispec.Platform
 		wantErr bool
 	}{
 		{
 			name:  "linux/amd64",
 			input: "linux/amd64",
-			want:  Platform{OS: "linux", Architecture: "amd64"},
+			want:  ocispec.Platform{OS: "linux", Architecture: "amd64"},
 		},
 		{
 			name:  "linux/arm64",
 			input: "linux/arm64",
-			want:  Platform{OS: "linux", Architecture: "arm64"},
+			want:  ocispec.Platform{OS: "linux", Architecture: "arm64"},
+		},
+		{
+			name:  "linux/arm/v7",
+			input: "linux/arm/v7",
+			want:  ocispec.Platform{OS: "linux", Architecture: "arm", Variant: "v7"},
 		},
 		{
 			name:    "no slash",
@@ -146,7 +152,7 @@ func TestParsePlatform(t *testing.T) {
 		},
 		{
 			name:    "too many parts",
-			input:   "linux/amd64/v8",
+			input:   "linux/amd64/v8/extra",
 			wantErr: true,
 		},
 		{
@@ -159,6 +165,11 @@ func TestParsePlatform(t *testing.T) {
 			input:   "linux/",
 			wantErr: true,
 		},
+		{
+			name:    "empty variant",
+			input:   "linux/arm/",
+			wantErr: true,
+		},
 	}
 
 	for _, tt := range tests {
@@ -179,8 +190,45 @@ func TestParsePlatform(t *testing.T) {
 func TestPlatformString(t *testing.T) {
 	t.Parallel()
 
-	p := Platform{OS: "linux", Architecture: "amd64"}
-	assert.Equal(t, "linux/amd64", p.String())
+	tests := []struct {
+		name     string
+		platform ocispec.Platform
+		want     string
+	}{
+		{
+			name:     "os/arch",
+			platform: ocispec.Platform{OS: "linux", Architecture: "amd64"},
+			want:     "linux/amd64",
+		},
+		{
+			name:     "os/arch/variant",
+			platform: ocispec.Platform{OS: "linux", Architecture: "arm", Variant: "v7"},
+			want:     "linux/arm/v7",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			assert.Equal(t, tt.want, PlatformString(tt.platform))
+		})
+	}
+}
+
+func TestParsePlatform_PlatformString_Roundtrip(t *testing.T) {
+	t.Parallel()
+
+	platforms := []ocispec.Platform{
+		{OS: "linux", Architecture: "amd64"},
+		{OS: "linux", Architecture: "arm64"},
+		{OS: "linux", Architecture: "arm", Variant: "v7"},
+	}
+
+	for _, p := range platforms {
+		parsed, err := ParsePlatform(PlatformString(p))
+		require.NoError(t, err)
+		assert.Equal(t, p, parsed)
+	}
 }
 
 func TestParseRequiresAnnotation(t *testing.T) {
@@ -238,6 +286,6 @@ func TestDefaultPlatforms(t *testing.T) {
 	t.Parallel()
 
 	require.Len(t, DefaultPlatforms, 2)
-	assert.Equal(t, Platform{OS: "linux", Architecture: "amd64"}, DefaultPlatforms[0])
-	assert.Equal(t, Platform{OS: "linux", Architecture: "arm64"}, DefaultPlatforms[1])
+	assert.Equal(t, ocispec.Platform{OS: "linux", Architecture: "amd64"}, DefaultPlatforms[0])
+	assert.Equal(t, ocispec.Platform{OS: "linux", Architecture: "arm64"}, DefaultPlatforms[1])
 }