Skip to content

Commit 0a174a9

Browse files
JAORMXclaude
andcommitted
Migrate custom OCI types to ocispec in oci/skills package
Replace the dual type system where the packager used ocispec types internally but the store, registry, and helpers used custom types (ImageConfig, ImageIndex, IndexDescriptor, Platform). This eliminates friction like needing JSON re-serialization between the packager's ocispec.Image and the store's ImageConfig. Removed 7 custom struct types and 5 redundant media type constants in favor of their ocispec equivalents. Updated ParsePlatform to support os/arch/variant format, and replaced Platform.String() method with PlatformString() free function. Also fixed platform variant propagation in packager config and index construction, and added -copyright_file to mockgen directives so generated mocks include SPDX headers. Closes #24 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 3a54b8b commit 0a174a9

12 files changed

Lines changed: 167 additions & 173 deletions

env/env.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
package env
55

6-
//go:generate mockgen -source=env.go -destination=mocks/mock_reader.go -package=mocks Reader
6+
//go:generate mockgen -copyright_file=../.github/license-header.txt -source=env.go -destination=mocks/mock_reader.go -package=mocks Reader
77

88
import "os"
99

env/mocks/mock_reader.go

Lines changed: 3 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

oci/skills/interfaces.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,14 @@
33

44
package skills
55

6-
//go:generate mockgen -source=interfaces.go -destination=mocks/mock_interfaces.go -package=mocks
6+
//go:generate mockgen -copyright_file=../../.github/license-header.txt -source=interfaces.go -destination=mocks/mock_interfaces.go -package=mocks
77

88
import (
99
"context"
1010
"time"
1111

1212
"github.com/opencontainers/go-digest"
13+
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
1314
)
1415

1516
// RegistryClient provides remote OCI registry operations for skills.
@@ -34,7 +35,7 @@ type PackageOptions struct {
3435

3536
// Platforms specifies target platforms for the image index.
3637
// If empty, defaults to DefaultPlatforms.
37-
Platforms []Platform
38+
Platforms []ocispec.Platform
3839
}
3940

4041
// PackageResult contains the result of packaging a skill.
@@ -44,5 +45,5 @@ type PackageResult struct {
4445
ConfigDigest digest.Digest
4546
LayerDigest digest.Digest
4647
Config *SkillConfig
47-
Platforms []Platform
48+
Platforms []ocispec.Platform
4849
}

oci/skills/mediatypes.go

Lines changed: 25 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"encoding/json"
88
"fmt"
99
"strings"
10+
11+
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
1012
)
1113

1214
// Artifact type for skill identification.
@@ -15,29 +17,8 @@ const (
1517
ArtifactTypeSkill = "dev.toolhive.skills.v1"
1618
)
1719

18-
// OCI Image Index media type.
19-
const (
20-
// MediaTypeImageIndex is the OCI image index media type.
21-
MediaTypeImageIndex = "application/vnd.oci.image.index.v1+json"
22-
)
23-
24-
// Standard OCI media types for Kubernetes image volume compatibility.
25-
const (
26-
// MediaTypeImageManifest is the OCI image manifest media type.
27-
MediaTypeImageManifest = "application/vnd.oci.image.manifest.v1+json"
28-
29-
// MediaTypeImageConfig is the standard OCI image config media type.
30-
MediaTypeImageConfig = "application/vnd.oci.image.config.v1+json"
31-
32-
// MediaTypeImageLayer is the standard OCI image layer media type.
33-
MediaTypeImageLayer = "application/vnd.oci.image.layer.v1.tar+gzip"
34-
)
35-
3620
// Annotation keys for skill metadata in manifests.
3721
const (
38-
// AnnotationCreated is the OCI standard annotation for creation time.
39-
AnnotationCreated = "org.opencontainers.image.created"
40-
4122
// AnnotationSkillName is the annotation key for skill name.
4223
AnnotationSkillName = "dev.toolhive.skills.name"
4324

@@ -84,35 +65,8 @@ type SkillConfig struct {
8465
Files []string `json:"files"`
8566
}
8667

87-
// ImageConfig represents a standard OCI image configuration.
88-
// This structure is required for Kubernetes image volume compatibility.
89-
type ImageConfig struct {
90-
Architecture string `json:"architecture"`
91-
OS string `json:"os"`
92-
Config ImageConfigData `json:"config,omitempty"`
93-
RootFS RootFS `json:"rootfs"`
94-
History []HistoryEntry `json:"history,omitempty"`
95-
}
96-
97-
// ImageConfigData contains container configuration including labels.
98-
type ImageConfigData struct {
99-
Labels map[string]string `json:"Labels,omitempty"`
100-
}
101-
102-
// RootFS describes the rootfs of the image.
103-
type RootFS struct {
104-
Type string `json:"type"`
105-
DiffIDs []string `json:"diff_ids"`
106-
}
107-
108-
// HistoryEntry describes a layer in the image history.
109-
type HistoryEntry struct {
110-
Created string `json:"created,omitempty"`
111-
CreatedBy string `json:"created_by,omitempty"`
112-
}
113-
11468
// SkillConfigFromImageConfig extracts SkillConfig from OCI image config labels.
115-
func SkillConfigFromImageConfig(imgConfig *ImageConfig) (*SkillConfig, error) {
69+
func SkillConfigFromImageConfig(imgConfig *ocispec.Image) (*SkillConfig, error) {
11670
if imgConfig == nil {
11771
return nil, fmt.Errorf("image config is nil")
11872
}
@@ -149,56 +103,44 @@ func SkillConfigFromImageConfig(imgConfig *ImageConfig) (*SkillConfig, error) {
149103
return config, nil
150104
}
151105

152-
// Platform represents a target platform for OCI artifacts.
153-
type Platform struct {
154-
Architecture string `json:"architecture"`
155-
OS string `json:"os"`
156-
}
157-
158-
// String returns the platform in "os/arch" format.
159-
func (p Platform) String() string {
160-
return p.OS + "/" + p.Architecture
106+
// PlatformString returns the platform in "os/arch" or "os/arch/variant" format.
107+
func PlatformString(p ocispec.Platform) string {
108+
s := p.OS + "/" + p.Architecture
109+
if p.Variant != "" {
110+
s += "/" + p.Variant
111+
}
112+
return s
161113
}
162114

163-
// ParsePlatform parses a platform string in "os/arch" format.
164-
func ParsePlatform(s string) (Platform, error) {
115+
// ParsePlatform parses a platform string in "os/arch" or "os/arch/variant" format.
116+
func ParsePlatform(s string) (ocispec.Platform, error) {
165117
parts := strings.Split(s, "/")
166-
if len(parts) != 2 {
167-
return Platform{}, fmt.Errorf("invalid platform format: %q (expected os/arch)", s)
118+
if len(parts) < 2 || len(parts) > 3 {
119+
return ocispec.Platform{}, fmt.Errorf("invalid platform format: %q (expected os/arch or os/arch/variant)", s)
168120
}
169121
osName := strings.TrimSpace(parts[0])
170122
arch := strings.TrimSpace(parts[1])
171123
if osName == "" || arch == "" {
172-
return Platform{}, fmt.Errorf("invalid platform format: %q (os and arch cannot be empty)", s)
124+
return ocispec.Platform{}, fmt.Errorf("invalid platform format: %q (os and arch cannot be empty)", s)
173125
}
174-
return Platform{OS: osName, Architecture: arch}, nil
126+
p := ocispec.Platform{OS: osName, Architecture: arch}
127+
if len(parts) == 3 {
128+
variant := strings.TrimSpace(parts[2])
129+
if variant == "" {
130+
return ocispec.Platform{}, fmt.Errorf("invalid platform format: %q (variant cannot be empty)", s)
131+
}
132+
p.Variant = variant
133+
}
134+
return p, nil
175135
}
176136

177137
// DefaultPlatforms are the default platforms for skill artifacts.
178138
// These cover most Kubernetes clusters.
179-
var DefaultPlatforms = []Platform{
139+
var DefaultPlatforms = []ocispec.Platform{
180140
{OS: "linux", Architecture: "amd64"},
181141
{OS: "linux", Architecture: "arm64"},
182142
}
183143

184-
// ImageIndex represents an OCI image index (multi-platform manifest list).
185-
type ImageIndex struct {
186-
SchemaVersion int `json:"schemaVersion"`
187-
MediaType string `json:"mediaType"`
188-
ArtifactType string `json:"artifactType,omitempty"`
189-
Manifests []IndexDescriptor `json:"manifests"`
190-
Annotations map[string]string `json:"annotations,omitempty"`
191-
}
192-
193-
// IndexDescriptor describes a manifest in an image index.
194-
type IndexDescriptor struct {
195-
MediaType string `json:"mediaType"`
196-
Digest string `json:"digest"`
197-
Size int64 `json:"size"`
198-
Platform *Platform `json:"platform,omitempty"`
199-
Annotations map[string]string `json:"annotations,omitempty"`
200-
}
201-
202144
// ParseRequiresAnnotation parses skill dependency references from manifest annotations.
203145
// Returns nil if the annotation is missing or invalid.
204146
func ParseRequiresAnnotation(annotations map[string]string) []string {

oci/skills/mediatypes_test.go

Lines changed: 69 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package skills
66
import (
77
"testing"
88

9+
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
910
"github.com/stretchr/testify/assert"
1011
"github.com/stretchr/testify/require"
1112
)
@@ -15,16 +16,16 @@ func TestSkillConfigFromImageConfig(t *testing.T) {
1516

1617
tests := []struct {
1718
name string
18-
config *ImageConfig
19+
config *ocispec.Image
1920
wantName string
2021
wantErr bool
2122
wantTools []string
2223
wantFiles []string
2324
}{
2425
{
2526
name: "all fields populated",
26-
config: &ImageConfig{
27-
Config: ImageConfigData{
27+
config: &ocispec.Image{
28+
Config: ocispec.ImageConfig{
2829
Labels: map[string]string{
2930
LabelSkillName: "my-skill",
3031
LabelSkillDescription: "A test skill",
@@ -41,8 +42,8 @@ func TestSkillConfigFromImageConfig(t *testing.T) {
4142
},
4243
{
4344
name: "minimal config",
44-
config: &ImageConfig{
45-
Config: ImageConfigData{
45+
config: &ocispec.Image{
46+
Config: ocispec.ImageConfig{
4647
Labels: map[string]string{
4748
LabelSkillName: "minimal-skill",
4849
},
@@ -57,15 +58,15 @@ func TestSkillConfigFromImageConfig(t *testing.T) {
5758
},
5859
{
5960
name: "nil labels",
60-
config: &ImageConfig{
61-
Config: ImageConfigData{Labels: nil},
61+
config: &ocispec.Image{
62+
Config: ocispec.ImageConfig{Labels: nil},
6263
},
6364
wantErr: true,
6465
},
6566
{
6667
name: "missing name",
67-
config: &ImageConfig{
68-
Config: ImageConfigData{
68+
config: &ocispec.Image{
69+
Config: ocispec.ImageConfig{
6970
Labels: map[string]string{
7071
LabelSkillDescription: "no name",
7172
},
@@ -75,8 +76,8 @@ func TestSkillConfigFromImageConfig(t *testing.T) {
7576
},
7677
{
7778
name: "invalid allowed tools JSON",
78-
config: &ImageConfig{
79-
Config: ImageConfigData{
79+
config: &ocispec.Image{
80+
Config: ocispec.ImageConfig{
8081
Labels: map[string]string{
8182
LabelSkillName: "bad-tools",
8283
LabelSkillAllowedTools: "not-json",
@@ -87,8 +88,8 @@ func TestSkillConfigFromImageConfig(t *testing.T) {
8788
},
8889
{
8990
name: "invalid files JSON",
90-
config: &ImageConfig{
91-
Config: ImageConfigData{
91+
config: &ocispec.Image{
92+
Config: ocispec.ImageConfig{
9293
Labels: map[string]string{
9394
LabelSkillName: "bad-files",
9495
LabelSkillFiles: "not-json",
@@ -126,18 +127,23 @@ func TestParsePlatform(t *testing.T) {
126127
tests := []struct {
127128
name string
128129
input string
129-
want Platform
130+
want ocispec.Platform
130131
wantErr bool
131132
}{
132133
{
133134
name: "linux/amd64",
134135
input: "linux/amd64",
135-
want: Platform{OS: "linux", Architecture: "amd64"},
136+
want: ocispec.Platform{OS: "linux", Architecture: "amd64"},
136137
},
137138
{
138139
name: "linux/arm64",
139140
input: "linux/arm64",
140-
want: Platform{OS: "linux", Architecture: "arm64"},
141+
want: ocispec.Platform{OS: "linux", Architecture: "arm64"},
142+
},
143+
{
144+
name: "linux/arm/v7",
145+
input: "linux/arm/v7",
146+
want: ocispec.Platform{OS: "linux", Architecture: "arm", Variant: "v7"},
141147
},
142148
{
143149
name: "no slash",
@@ -146,7 +152,7 @@ func TestParsePlatform(t *testing.T) {
146152
},
147153
{
148154
name: "too many parts",
149-
input: "linux/amd64/v8",
155+
input: "linux/amd64/v8/extra",
150156
wantErr: true,
151157
},
152158
{
@@ -159,6 +165,11 @@ func TestParsePlatform(t *testing.T) {
159165
input: "linux/",
160166
wantErr: true,
161167
},
168+
{
169+
name: "empty variant",
170+
input: "linux/arm/",
171+
wantErr: true,
172+
},
162173
}
163174

164175
for _, tt := range tests {
@@ -179,8 +190,45 @@ func TestParsePlatform(t *testing.T) {
179190
func TestPlatformString(t *testing.T) {
180191
t.Parallel()
181192

182-
p := Platform{OS: "linux", Architecture: "amd64"}
183-
assert.Equal(t, "linux/amd64", p.String())
193+
tests := []struct {
194+
name string
195+
platform ocispec.Platform
196+
want string
197+
}{
198+
{
199+
name: "os/arch",
200+
platform: ocispec.Platform{OS: "linux", Architecture: "amd64"},
201+
want: "linux/amd64",
202+
},
203+
{
204+
name: "os/arch/variant",
205+
platform: ocispec.Platform{OS: "linux", Architecture: "arm", Variant: "v7"},
206+
want: "linux/arm/v7",
207+
},
208+
}
209+
210+
for _, tt := range tests {
211+
t.Run(tt.name, func(t *testing.T) {
212+
t.Parallel()
213+
assert.Equal(t, tt.want, PlatformString(tt.platform))
214+
})
215+
}
216+
}
217+
218+
func TestParsePlatform_PlatformString_Roundtrip(t *testing.T) {
219+
t.Parallel()
220+
221+
platforms := []ocispec.Platform{
222+
{OS: "linux", Architecture: "amd64"},
223+
{OS: "linux", Architecture: "arm64"},
224+
{OS: "linux", Architecture: "arm", Variant: "v7"},
225+
}
226+
227+
for _, p := range platforms {
228+
parsed, err := ParsePlatform(PlatformString(p))
229+
require.NoError(t, err)
230+
assert.Equal(t, p, parsed)
231+
}
184232
}
185233

186234
func TestParseRequiresAnnotation(t *testing.T) {
@@ -238,6 +286,6 @@ func TestDefaultPlatforms(t *testing.T) {
238286
t.Parallel()
239287

240288
require.Len(t, DefaultPlatforms, 2)
241-
assert.Equal(t, Platform{OS: "linux", Architecture: "amd64"}, DefaultPlatforms[0])
242-
assert.Equal(t, Platform{OS: "linux", Architecture: "arm64"}, DefaultPlatforms[1])
289+
assert.Equal(t, ocispec.Platform{OS: "linux", Architecture: "amd64"}, DefaultPlatforms[0])
290+
assert.Equal(t, ocispec.Platform{OS: "linux", Architecture: "arm64"}, DefaultPlatforms[1])
243291
}

0 commit comments

Comments
 (0)