Fixes value for storage.oci.repository

Previously while providing repo url value for storage
oci repository, chains controller was giving an error
as

`a digest must contain exactly one '@' separator (e.g.
registry/repository@digest)`

because, it was not able to add digest

Hence this patch fixes it, by formatting the value
provided by the user and thus storing the
attestations/signatures in the provided location

Signed-off-by: PuneetPunamiya <[email protected]>

pkg/chains/storage/oci/legacy.go

@@ -18,7 +18,6 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
-
 	"github.com/tektoncd/chains/pkg/chains/formats"
 	"github.com/tektoncd/chains/pkg/chains/objects"
 	"github.com/tektoncd/chains/pkg/chains/signing"
@@ -54,7 +53,8 @@ type Backend struct {
 // NewStorageBackend returns a new OCI StorageBackend that stores signatures in an OCI registry
 func NewStorageBackend(ctx context.Context, client kubernetes.Interface, cfg config.Config) *Backend {
 	return &Backend{
-		cfg:    cfg,
+		cfg: cfg,
+
 		client: client,
 		getAuthenticator: func(ctx context.Context, obj objects.TektonObject, client kubernetes.Interface) (remote.Option, error) {
 			kc, err := k8schain.New(ctx, client,
@@ -119,12 +119,27 @@ func (b *Backend) uploadSignature(ctx context.Context, format simple.SimpleConta
 	imageName := format.ImageName()
 	logger.Infof("Uploading %s signature", imageName)
 
-	ref, err := newDigest(b.cfg, imageName)
+	ref, err := name.NewDigest(imageName)
 	if err != nil {
 		return errors.Wrap(err, "getting digest")
 	}
 
-	store, err := NewSimpleStorerFromConfig(WithTargetRepository(ref.Repository))
+	// Checking the value of `cfg.Storage.OCI.Repository` because while running unit tests we create a mock registry
+	// server which has repo url as `127.0.0.1:34897` and setting this `cfg.Storage.OCI.Repository` will return an error
+	// as repository can only contain the characters `abcdefghijklmnopqrstuvwxyz0123456789_-./`: 127.0.0.1:34897.
+	// If we don't set the value for `cfg.Storage.OCI.Repository` in the test, then by default it takes the registry as
+	// `index.docker.io/<repoName>` which will give authorization error
+	var repo name.Repository
+	if b.cfg.Storage.OCI.Repository == "" {
+		repo = ref.Repository
+	} else {
+		repo, err = newRepo(b.cfg)
+		if err != nil {
+			return errors.Wrapf(err, "getting storage repo for sub %s", imageName)
+		}
+	}
+
+	store, err := NewSimpleStorerFromConfig(WithTargetRepository(repo))
 	if err != nil {
 		return err
 	}
@@ -154,12 +169,27 @@ func (b *Backend) uploadAttestation(ctx context.Context, attestation in_toto.Sta
 		imageName := fmt.Sprintf("%s@sha256:%s", subj.Name, subj.Digest["sha256"])
 		logger.Infof("Starting attestation upload to OCI for %s...", imageName)
 
-		ref, err := newDigest(b.cfg, imageName)
+		ref, err := name.NewDigest(imageName)
 		if err != nil {
 			return errors.Wrapf(err, "getting digest for subj %s", imageName)
 		}
 
-		store, err := NewAttestationStorer(WithTargetRepository(ref.Repository))
+		// Checking the value of `cfg.Storage.OCI.Repository` because while running unit tests we create a mock registry
+		// server which has repo url as `127.0.0.1:34897` and setting this `cfg.Storage.OCI.Repository` will return an error
+		// as repository can only contain the characters `abcdefghijklmnopqrstuvwxyz0123456789_-./`: 127.0.0.1:34897.
+		// If we don't set the value for `cfg.Storage.OCI.Repository` in the test, then by default it takes the registry as
+		// `index.docker.io/<repoName>` which will give authorization error
+		var repo name.Repository
+		if b.cfg.Storage.OCI.Repository == "" {
+			repo = ref.Repository
+		} else {
+			repo, err = newRepo(b.cfg)
+			if err != nil {
+				return errors.Wrapf(err, "getting storage repo for sub %s", imageName)
+			}
+		}
+
+		store, err := NewAttestationStorer(WithTargetRepository(repo))
 		if err != nil {
 			return err
 		}
@@ -278,16 +308,11 @@ func (b *Backend) RetrieveArtifact(ctx context.Context, obj objects.TektonObject
 	return m, nil
 }
 
-func newDigest(cfg config.Config, imageName string) (name.Digest, error) {
-	// Override image name from config if set.
-	if r := cfg.Storage.OCI.Repository; r != "" {
-		imageName = r
-	}
-
+func newRepo(cfg config.Config) (name.Repository, error) {
 	var opts []name.Option
 	if cfg.Storage.OCI.Insecure {
 		opts = append(opts, name.Insecure)
 	}
 
-	return name.NewDigest(imageName, opts...)
+	return name.NewRepository(cfg.Storage.OCI.Repository, opts...)
 }

pkg/chains/storage/oci/legacy_test.go

@@ -0,0 +1,56 @@
+/*
+Copyright 2023 The Tekton 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 oci
+
+import (
+	"github.com/google/go-containerregistry/pkg/name"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/tektoncd/chains/pkg/config"
+)
+
+func TestNewRepo(t *testing.T) {
+	t.Run("Use any registry in storage oci repository", func(t *testing.T) {
+		cfg := config.Config{}
+		cfg.Storage.OCI.Repository = "example.com/foo"
+		tests := []struct {
+			testImageName     string
+			expectedImageName string
+			expectedRepo      *name.Repository
+		}{
+			{
+				testImageName:     "gcr.io/tekton-releases/github.com/tektoncd/pipeline/cmd/git-init@sha256:bc4f7468f87486e3835b09098c74cd7f54db2cf697cbb9b824271b95a2d0871e",
+				expectedImageName: "example.com/foo",
+				expectedRepo:      &name.Repository{},
+			},
+			{
+				testImageName:     "foo.io/bar/kaniko-chains@sha256:bc4f7468f87486e3835b09098c74cd7f54db2cf697cbb9b824271b95a2d0871e",
+				expectedImageName: "example.com/foo",
+			},
+			{
+				testImageName:     "registry.com/spam/spam/spam/spam/spam/spam@sha256:bc4f7468f87486e3835b09098c74cd7f54db2cf697cbb9b824271b95a2d0871e",
+				expectedImageName: "example.com/foo",
+			},
+		}
+
+		for _, test := range tests {
+			repo, err := newRepo(cfg)
+			if err != nil {
+				t.Error(err)
+			}
+			assert.Equal(t, repo.Name(), test.expectedImageName)
+		}
+	})
+}

pkg/chains/storage/oci/oci_test.go

@@ -16,6 +16,7 @@ package oci
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"net/http/httptest"
 	"net/url"
 	"strings"
@@ -62,6 +63,7 @@ func TestBackend_StorePayload(t *testing.T) {
 	s := httptest.NewServer(registry.New())
 	defer s.Close()
 	u, _ := url.Parse(s.URL)
+	fmt.Println(u.Host)
 
 	// Push image to server
 	ref, err := remotetest.CreateImage(u.Host+"/task/"+tr.Name, tr)
@@ -232,6 +234,7 @@ func TestBackend_StorePayload(t *testing.T) {
 				t.Fatalf("failed to marshal: %v", err)
 			}
 
+			b.cfg.Storage.OCI.Repository = u.Host
 			if err := b.StorePayload(ctx, tt.fields.object, rawPayload, tt.args.signature, tt.args.storageOpts); (err != nil) != tt.wantErr {
 				t.Errorf("Backend.StorePayload() error = %v, wantErr %v", err, tt.wantErr)
 			}