Skip to content

Commit 611f892

Browse files
authored
Fix push artifact bug (#14)
* Fix bug * Added unit test
1 parent c646b70 commit 611f892

4 files changed

Lines changed: 40 additions & 8 deletions

File tree

cmd/run.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ downloads; use --no-cache to force re-download. Any flags provided via
5757
5858
# Execute target from remote Makefile artifact, bypassing cache
5959
remake run -f ghcr.io/myorg/myrepo:latest --no-cache deploy`,
60-
Args: cobra.MinimumNArgs(1),
6160
RunE: func(cmd *cobra.Command, args []string) error {
6261
app.Cfg.NoCache = noCache
6362
return app.Run(context.Background(), file, makeFlags, args)

internal/cache/oci_cache.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func (c *OCIRepository) Push(ctx context.Context, reference string, data []byte)
5454
if strings.Contains(reference, "://") && !strings.HasPrefix(reference, "oci://") {
5555
return fmt.Errorf("invalid OCI reference: %s", reference)
5656
}
57-
raw := strings.TrimPrefix(reference, "oci://")
57+
raw := strings.ToLower(strings.TrimPrefix(reference, "oci://"))
5858
ref, err := parseRef(raw, name.WithDefaultRegistry(c.cfg.DefaultRegistry))
5959
if err != nil {
6060
return err
@@ -115,7 +115,7 @@ func (c *OCIRepository) Pull(ctx context.Context, reference string) (string, err
115115
if strings.Contains(reference, "://") && !strings.HasPrefix(reference, "oci://") {
116116
return "", fmt.Errorf("invalid OCI reference: %s", reference)
117117
}
118-
raw := strings.TrimPrefix(reference, "oci://")
118+
raw := strings.ToLower(strings.TrimPrefix(reference, "oci://"))
119119

120120
ref, err := name.ParseReference(raw, name.WithDefaultRegistry(c.cfg.DefaultRegistry))
121121
if err != nil {

internal/client/client_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -537,6 +537,24 @@ func TestOCIClientPushFileStoreCloseError(t *testing.T) {
537537
}
538538
}
539539

540+
func TestOCIClientPushAbsPathError(t *testing.T) {
541+
// Stub absPathFunc to simulate failure
542+
origAbs := absPathFunc
543+
defer func() { absPathFunc = origAbs }()
544+
absPathFunc = func(path string) (string, error) {
545+
return "", fmt.Errorf("abs error")
546+
}
547+
548+
cfg := &config.Config{DefaultRegistry: "example.com"}
549+
client := NewOCIClient(cfg)
550+
551+
// Call Push: path value doesn't matter, stub will error first
552+
err := client.Push(context.Background(), "oci://example.com/repo:tag", "somepath")
553+
if err == nil || !strings.Contains(err.Error(), "failed to resolve absolute path somepath: abs error") {
554+
t.Errorf("expected abs path error, got %v", err)
555+
}
556+
}
557+
540558
func TestOCIClientPushPackManifestError(t *testing.T) {
541559
tmpFile, err := os.CreateTemp("", "test*.txt")
542560
if err != nil {

internal/client/oci_client.go

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ var (
4949
packManifest = oras.PackManifest
5050
copyFunc = oras.Copy
5151
contentFetcher = content.FetchAll
52+
absPathFunc = filepath.Abs
5253
)
5354

5455
// OCIClient provides an implementation of Client for OCI registries.
@@ -87,19 +88,23 @@ func (c *OCIClient) Login(ctx context.Context, registry, user, pass string) erro
8788
// Push uploads the local file at path as an OCI artifact to the given reference.
8889
// It tags the artifact with the reference identifier and pushes it to the remote repository.
8990
func (c *OCIClient) Push(ctx context.Context, reference, path string) error {
91+
// Validate and parse reference
9092
if strings.Contains(reference, "://") && !strings.HasPrefix(reference, "oci://") {
9193
return fmt.Errorf("invalid OCI reference: %s", reference)
9294
}
93-
raw := strings.TrimPrefix(reference, "oci://")
95+
raw := strings.ToLower(strings.TrimPrefix(reference, "oci://"))
9496
ref, err := name.ParseReference(raw, name.WithDefaultRegistry(c.cfg.DefaultRegistry))
9597
if err != nil {
9698
return err
9799
}
98100
repoRef := ref.Context()
101+
99102
repo, err := newRepository(repoRef.RegistryStr() + "/" + repoRef.RepositoryStr())
100103
if err != nil {
101104
return err
102105
}
106+
107+
// Authenticate if credentials present
103108
key := config.NormalizeKey(repoRef.RegistryStr())
104109
user := viper.GetString("registries." + key + ".username")
105110
pass := viper.GetString("registries." + key + ".password")
@@ -111,19 +116,28 @@ func (c *OCIClient) Push(ctx context.Context, reference, path string) error {
111116
}
112117
}
113118

114-
dir := filepath.Dir(path)
119+
// Resolve absolute path and split directory
120+
absPath, err := absPathFunc(path)
121+
if err != nil {
122+
return fmt.Errorf("failed to resolve absolute path %s: %w", path, err)
123+
}
124+
dir := filepath.Dir(absPath)
125+
126+
// Prepare a file store rooted at the file's directory
115127
fs, err := newFileStore(dir)
116128
if err != nil {
117-
return err
129+
return fmt.Errorf("creating file store: %w", err)
118130
}
119131
defer func() { _ = fs.Close() }()
120132

133+
// Add the file using its absolute path to ensure tests find it
121134
mediaType := "application/vnd.remake.file"
122-
fileDesc, err := fs.Add(ctx, path, mediaType, "")
135+
fileDesc, err := fs.Add(ctx, absPath, mediaType, "")
123136
if err != nil {
124137
return fmt.Errorf("adding file to store: %w", err)
125138
}
126139

140+
// Pack manifest using injected function
127141
artifactType := "application/vnd.remake.artifact"
128142
opts := oras.PackManifestOptions{Layers: []v1.Descriptor{fileDesc}}
129143
manifestDesc, err := packManifest(ctx, fs, oras.PackManifestVersion1_1, artifactType, opts)
@@ -137,6 +151,7 @@ func (c *OCIClient) Push(ctx context.Context, reference, path string) error {
137151
tag := ref.Identifier()
138152
_ = fs.Tag(ctx, manifestDesc, tag)
139153

154+
// Push to remote using injected function
140155
if _, err := copyFunc(ctx, fs, tag, repo, tag, oras.DefaultCopyOptions); err != nil {
141156
return fmt.Errorf("pushing to remote: %w", err)
142157
}
@@ -149,7 +164,7 @@ func (c *OCIClient) Pull(ctx context.Context, reference string) ([]byte, error)
149164
if strings.Contains(reference, "://") && !strings.HasPrefix(reference, "oci://") {
150165
return nil, fmt.Errorf("invalid OCI reference: %s", reference)
151166
}
152-
raw := strings.TrimPrefix(reference, "oci://")
167+
raw := strings.ToLower(strings.TrimPrefix(reference, "oci://"))
153168
ref, err := name.ParseReference(raw, name.WithDefaultRegistry(c.cfg.DefaultRegistry))
154169
if err != nil {
155170
return nil, err

0 commit comments

Comments
 (0)