fix(oci/puller): do not omit previous errors when returning them

cmd/artifact/install/install.go

@@ -219,7 +219,7 @@ func (o *artifactInstallOptions) RunArtifactInstall(ctx context.Context, args []
 			return nil, err
 		}
 
-		artifactConfig, err := puller.PullConfigLayer(ctx, ref)
+		artifactConfig, err := puller.GetArtifactConfig(ctx, ref)
 		if err != nil {
 			return nil, err
 		}

internal/follower/follower.go

@@ -171,7 +171,7 @@ func (f *Follower) follow(ctx context.Context) {
 	f.logger.Info("Found new artifact version", f.logger.Args("followerName", f.ref, "tag", f.tag))
 
 	// Pull config layer to check falco versions
-	artifactConfig, err := f.PullConfigLayer(ctx, f.ref)
+	artifactConfig, err := f.GetArtifactConfig(ctx, f.ref)
 	if err != nil {
 		f.logger.Error("Unable to pull config layer", f.logger.Args("followerName", f.ref, "reason", err.Error()))
 		return

pkg/oci/puller/puller.go

@@ -170,7 +170,7 @@ func (p *Puller) manifestFromRef(ctx context.Context, ref string) (*v1.Manifest,
 
 	desc, manifestReader, err := repo.FetchReference(ctx, ref)
 	if err != nil {
-		return nil, fmt.Errorf("unable to fetch reference %q", ref)
+		return nil, fmt.Errorf("unable to fetch reference %q: %w", ref, err)
 	}
 	defer manifestReader.Close()
 
@@ -212,7 +212,7 @@ func (p *Puller) manifestFromRef(ctx context.Context, ref string) (*v1.Manifest,
 	var manifest v1.Manifest
 	manifestBytes, err := io.ReadAll(manifestReader)
 	if err != nil {
-		return nil, fmt.Errorf("unable to read bytes from manifest reader for ref %q", ref)
+		return nil, fmt.Errorf("unable to read bytes from manifest reader for ref %q: %w", ref, err)
 	}
 
 	if err = json.Unmarshal(manifestBytes, &manifest); err != nil {
@@ -222,8 +222,23 @@ func (p *Puller) manifestFromRef(ctx context.Context, ref string) (*v1.Manifest,
 	return &manifest, nil
 }
 
+// GetArtifactConfig fetches only the config layer from a given ref.
+func (p *Puller) GetArtifactConfig(ctx context.Context, ref string) (*oci.ArtifactConfig, error) {
+	configBytes, err := p.PullConfigLayer(ctx, ref)
+	if err != nil {
+		return nil, err
+	}
+
+	var artifactConfig oci.ArtifactConfig
+	if err = json.Unmarshal(configBytes, &artifactConfig); err != nil {
+		return nil, err
+	}
+
+	return &artifactConfig, nil
+}
+
 // PullConfigLayer fetches only the config layer from a given ref.
-func (p *Puller) PullConfigLayer(ctx context.Context, ref string) (*oci.ArtifactConfig, error) {
+func (p *Puller) PullConfigLayer(ctx context.Context, ref string) ([]byte, error) {
 	repo, err := repository.NewRepository(ref, repository.WithClient(p.Client), repository.WithPlainHTTP(p.plainHTTP))
 	if err != nil {
 		return nil, err
@@ -238,25 +253,24 @@ func (p *Puller) PullConfigLayer(ctx context.Context, ref string) (*oci.Artifact
 
 	descriptor, err := repo.Blobs().Resolve(ctx, configRef)
 	if err != nil {
-		return nil, fmt.Errorf("unable to resolve to get descriptor for config blob %q", configRef)
+		return nil, err
 	}
 
 	rc, err := repo.Fetch(ctx, descriptor)
 	if err != nil {
-		return nil, fmt.Errorf("unable to fetch descriptor with digest: %s", descriptor.Digest.String())
+		return nil, err
 	}
 
 	configBytes, err := io.ReadAll(rc)
 	if err != nil {
 		return nil, err
 	}
 
-	var artifactConfig oci.ArtifactConfig
-	if err = json.Unmarshal(configBytes, &artifactConfig); err != nil {
+	if err := rc.Close(); err != nil {
 		return nil, err
 	}
 
-	return &artifactConfig, nil
+	return configBytes, nil
 }
 
 // CheckAllowedType does a preliminary check on the manifest to state whether we are allowed

pkg/oci/puller/puller_suite_test.go

@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright (C) 2023 The Falco Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package puller_test
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/distribution/distribution/v3/configuration"
+	_ "github.com/distribution/distribution/v3/registry/storage/driver/inmemory"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	v1 "github.com/opencontainers/image-spec/specs-go/v1"
+	"oras.land/oras-go/v2"
+	"oras.land/oras-go/v2/content/file"
+	"oras.land/oras-go/v2/registry/remote"
+	"oras.land/oras-go/v2/registry/remote/auth"
+
+	"github.com/falcosecurity/falcoctl/pkg/oci"
+	"github.com/falcosecurity/falcoctl/pkg/oci/authn"
+	ocipusher "github.com/falcosecurity/falcoctl/pkg/oci/pusher"
+	"github.com/falcosecurity/falcoctl/pkg/oci/repository"
+	testutils "github.com/falcosecurity/falcoctl/pkg/test"
+)
+
+var (
+	localRegistryHost         string
+	localRegistry             *remote.Registry
+	testRuleTarball           = "../../test/data/rules.tar.gz"
+	testPluginTarball         = "../../test/data/plugin.tar.gz"
+	testPluginPlatform1       = "linux/amd64"
+	testPluginPlatform2       = "windows/amd64"
+	testPluginPlatform3       = "linux/arm64"
+	ctx                       = context.Background()
+	destinationDir            string
+	pluginMultiPlatformRef    string
+	rulesRef                  string
+	artifactWithuoutConfigRef string
+)
+
+func TestPuller(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Puller Suite")
+}
+
+var _ = BeforeSuite(func() {
+	var err error
+	config := &configuration.Configuration{}
+	// Get a free port to be used by the registry.
+	port, err := testutils.FreePort()
+	Expect(err).ToNot(HaveOccurred())
+	// Create the registry address to which will bind.
+	config.HTTP.Addr = fmt.Sprintf("localhost:%d", port)
+	localRegistryHost = config.HTTP.Addr
+
+	// Create the oras registry.
+	localRegistry, err = testutils.NewOrasRegistry(localRegistryHost, true)
+	Expect(err).ToNot(HaveOccurred())
+
+	// Start the local registry.
+	go func() {
+		err := testutils.StartRegistry(context.Background(), config)
+		Expect(err).ToNot(BeNil())
+	}()
+
+	// Create the temporary dir where artifacts are saved.
+	destinationDir, err = os.MkdirTemp("", "falcoctl-puller-tests-")
+	Expect(err).ShouldNot(HaveOccurred())
+
+	// Push the artifacts to the registry.
+	// Same artifacts will be used to test the puller code.
+	pusher := ocipusher.NewPusher(authn.NewClient(authn.WithCredentials(&auth.EmptyCredential)), true, nil)
+
+	// Push plugin artifact with multiple architectures.
+	filePathsAndPlatforms := ocipusher.WithFilepathsAndPlatforms([]string{testPluginTarball, testPluginTarball, testPluginTarball},
+		[]string{testPluginPlatform1, testPluginPlatform2, testPluginPlatform3})
+	pluginMultiPlatformRef = localRegistryHost + "/plugins:multiplatform"
+	artConfig := oci.ArtifactConfig{}
+	Expect(artConfig.ParseDependencies("my-dep:1.2.3|my-alt-dep:1.4.5")).ToNot(HaveOccurred())
+	Expect(artConfig.ParseRequirements("my-req:7.8.9")).ToNot(HaveOccurred())
+	artifactConfig := ocipusher.WithArtifactConfig(artConfig)
+
+	// Build options slice.
+	options := []ocipusher.Option{filePathsAndPlatforms, artifactConfig}
+
+	// Push the plugin artifact.
+	_, err = pusher.Push(ctx, oci.Plugin, pluginMultiPlatformRef, options...)
+	Expect(err).ShouldNot(HaveOccurred())
+
+	// Prepare and push rulesfile artifact.
+	filePaths := ocipusher.WithFilepaths([]string{testRuleTarball})
+	artConfig = oci.ArtifactConfig{}
+	Expect(artConfig.ParseDependencies("dep1:1.2.3", "dep2:2.3.1")).ToNot(HaveOccurred())
+	options = []ocipusher.Option{
+		filePaths,
+		ocipusher.WithTags("latest"),
+		ocipusher.WithArtifactConfig(artConfig),
+	}
+	// Push a new artifact
+	rulesRef = localRegistryHost + "/rulesfiles:regular"
+	_, err = pusher.Push(ctx, oci.Rulesfile, rulesRef, options...)
+	Expect(err).ShouldNot(HaveOccurred())
+
+	// Push artifact without config layer.
+	artifactWithuoutConfigRef = localRegistryHost + "/artifact:noconfig"
+	err = pushArtifactWithoutConfigLayer(ctx, artifactWithuoutConfigRef, testRuleTarball, authn.NewClient(authn.WithCredentials(&auth.EmptyCredential)))
+	Expect(err).ShouldNot(HaveOccurred())
+})
+
+func pushArtifactWithoutConfigLayer(ctx context.Context, ref, artifactPath string, client remote.Client) error {
+	repo, err := repository.NewRepository(ref,
+		repository.WithClient(client),
+		repository.WithPlainHTTP(true))
+	if err != nil {
+		return err
+	}
+
+	fileStore, err := file.New(destinationDir)
+	if err != nil {
+		return err
+	}
+
+	// Get absolute path of the artifact.
+
+	path, err := filepath.Abs(artifactPath)
+	if err != nil {
+		return err
+	}
+
+	desc, err := fileStore.Add(ctx, filepath.Base(artifactPath), oci.FalcoRulesfileLayerMediaType, path)
+	if err != nil {
+		return err
+	}
+
+	packOptions := oras.PackOptions{
+		PackImageManifest: true,
+	}
+
+	desc, err = oras.Pack(ctx, fileStore, "", []v1.Descriptor{desc}, packOptions)
+
+	if err != nil {
+		return err
+	}
+
+	if err := oras.CopyGraph(ctx, fileStore, repo, desc, oras.DefaultCopyGraphOptions); err != nil {
+		return err
+	}
+
+	rootReader, err := fileStore.Fetch(ctx, desc)
+	if err != nil {
+		return err
+	}
+	defer rootReader.Close()
+
+	// Tag the root descriptor remotely.
+	err = repo.PushReference(ctx, desc, rootReader, repo.Reference.Reference)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+var _ = AfterSuite(func() {
+	Expect(os.RemoveAll(destinationDir)).Should(Succeed())
+})