chore: address minor comments

Signed-off-by: Lorenzo Susini <[email protected]>

Author: loresuso

internal/artifact/install/deps.go

@@ -33,14 +33,19 @@ var (
 )
 
 type depInfo struct {
+	// ref is the remote reference to this artifact
 	ref string
+	// res contains the config layer for this artifact
 	res *oci.RegistryResult
+	// ver represents the semver version of this artifact
 	ver *semver.Version
-	ok  bool
+	// ok is used to mark this dependency as fully processed, with its own
+	// dependencies and alternatives
+	ok bool
 }
 
 func copyDepsMap(in depsMapType) (out depsMapType) {
-	out = make(depsMapType)
+	out = make(depsMapType, len(in))
 	for k, v := range in {
 		out[k] = v
 	}
@@ -91,13 +96,15 @@ func ResolveDeps(resolver artifactConfigResolver, inRefs ...string) (outRefs []s
 
 	for {
 		allOk := true
+		// Since we are updating depMap in this for loop, let's copy the map for iterating it
+		// while we continue inserting new values in the real depMap map.
 		for name, info := range copyDepsMap(depMap) {
 			if info.ok {
 				continue
 			}
 			for _, required := range info.res.Config.Dependencies {
 				// Does already exist in the map?
-				if existing, exists := depMap[required.Name]; exists {
+				if existing, ok := depMap[required.Name]; ok {
 					requiredVer, err := semver.Parse(required.Version)
 					if err != nil {
 						return nil, fmt.Errorf(`invalid artifact config: version %q is not semver compatible`, required.Version)
@@ -120,8 +127,8 @@ func ResolveDeps(resolver artifactConfigResolver, inRefs ...string) (outRefs []s
 				// Are alternatives already in the map?
 				var foundAlternative = false
 				for _, alternative := range required.Alternatives {
-					existing, exists := depMap[alternative.Name]
-					if !exists {
+					existing, ok := depMap[alternative.Name]
+					if !ok {
 						continue
 					}
 

internal/artifact/install/install.go

@@ -16,9 +16,7 @@ package install
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
-	"io"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -107,23 +105,13 @@ func (o *artifactInstallOptions) RunArtifactInstall(ctx context.Context, args []
 			return nil, err
 		}
 
-		configReader, err := puller.PullConfigLayer(ctx, ref)
+		artifactConfig, err := puller.PullConfigLayer(ctx, ref)
 		if err != nil {
 			return nil, err
 		}
 
-		configBytes, err := io.ReadAll(configReader)
-		if err != nil {
-			return nil, err
-		}
-
-		var artifactConfig oci.ArtifactConfig
-		if err = json.Unmarshal(configBytes, &artifactConfig); err != nil {
-			return nil, err
-		}
-
 		return &oci.RegistryResult{
-			Config: artifactConfig,
+			Config: *artifactConfig,
 		}, nil
 	})
 

pkg/oci/puller/puller.go

@@ -157,7 +157,7 @@ func manifestFromDesc(ctx context.Context, target oras.Target, desc *v1.Descript
 }
 
 // PullConfigLayer fetches only the config layer from a given ref.
-func (p *Puller) PullConfigLayer(ctx context.Context, ref string) (io.Reader, error) {
+func (p *Puller) PullConfigLayer(ctx context.Context, ref string) (*oci.ArtifactConfig, error) {
 	repo, err := repository.NewRepository(ref, repository.WithClient(p.Client))
 	if err != nil {
 		return nil, err
@@ -172,10 +172,8 @@ func (p *Puller) PullConfigLayer(ctx context.Context, ref string) (io.Reader, er
 	// Resolve to actual manifest if an index is found.
 	if desc.MediaType == v1.MediaTypeImageIndex {
 		var index v1.Index
-		_, indexReader, err := repo.FetchReference(ctx, ref)
-		if err != nil {
-			return nil, fmt.Errorf("unable to fetch index for ref %q", ref)
-		}
+		indexReader := manifestReader
+		defer indexReader.Close()
 
 		indexBytes, err := io.ReadAll(indexReader)
 		if err != nil {
@@ -228,5 +226,15 @@ func (p *Puller) PullConfigLayer(ctx context.Context, ref string) (io.Reader, er
 		return nil, fmt.Errorf("unable to fetch descriptor with digest: %s", desc.Digest.String())
 	}
 
-	return rc, nil
+	configBytes, err := io.ReadAll(rc)
+	if err != nil {
+		return nil, err
+	}
+
+	var artifactConfig oci.ArtifactConfig
+	if err = json.Unmarshal(configBytes, &artifactConfig); err != nil {
+		return nil, err
+	}
+
+	return &artifactConfig, nil
 }

pkg/oci/types.go

@@ -152,7 +152,7 @@ func (a *ArtifactDependency) SetAlternative(name, version string) {
 // SetDependency stores an artifact dependency in the config.
 //
 // Return the insertion position.
-func (rc *ArtifactConfig) SetDependency(name, version string, alternatives []dependency) int {
+func (rc *ArtifactConfig) SetDependency(name, version string, alternatives []Dependency) int {
 	for i, d := range rc.Dependencies {
 		if d.Name == name {
 			rc.Dependencies[i].Version = version