cleanup: breakup the pkg/credentials into writer and matcher

The credentials package contains the a matcher and a writer which out of
which only the writer is used in cmd/entrypoint. In an effort to isolate
usage and not indirectly import the corev1 api which the matcher uses
for MatchingAnnotations, we are breaking up the credentials builder interface
into two builders for writer and matcher.

This ensures that the entrypoint only uses the writer and not the matcher,
and we are only using either the writer or the matcher functionality
as necessary and not importing unnecessary deps.

cleanup: use better names for the credentials interfaces

cleanup: use CredsDir from entrypoint pkg instead of pipeline

cleanup: remove corev1 usage from credentials package

cleanup: add goling gosec exception for Secret type constants

Author: waveywaves

cmd/entrypoint/main.go

@@ -30,11 +30,10 @@ import (
 
 	"github.com/tektoncd/pipeline/cmd/entrypoint/subcommands"
 	v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1/types"
-	"github.com/tektoncd/pipeline/pkg/credentials"
 	"github.com/tektoncd/pipeline/pkg/credentials/dockercreds"
 	"github.com/tektoncd/pipeline/pkg/credentials/gitcreds"
+	credwriter "github.com/tektoncd/pipeline/pkg/credentials/writer"
 	"github.com/tektoncd/pipeline/pkg/entrypoint"
-	"github.com/tektoncd/pipeline/pkg/entrypoint/pipeline"
 	"github.com/tektoncd/pipeline/pkg/platforms"
 	"github.com/tektoncd/pipeline/pkg/termination"
 )
@@ -92,9 +91,9 @@ func main() {
 	// from secret volume mounts to /tekton/creds. This is done to support the expansion
 	// of a variable, $(credentials.path), that resolves to a single place with all the
 	// stored credentials.
-	builders := []credentials.Builder{dockercreds.NewBuilder(), gitcreds.NewBuilder()}
+	builders := []credwriter.Writer{dockercreds.NewBuilder(), gitcreds.NewBuilder()}
 	for _, c := range builders {
-		if err := c.Write(pipeline.CredsDir); err != nil {
+		if err := c.Write(entrypoint.CredsDir); err != nil {
 			log.Printf("Error initializing credentials: %s", err)
 		}
 	}
@@ -154,7 +153,7 @@ func main() {
 
 	// Copy any creds injected by the controller into the $HOME directory of the current
 	// user so that they're discoverable by git / ssh.
-	if err := credentials.CopyCredsToHome(credentials.CredsInitCredentials); err != nil {
+	if err := credwriter.CopyCredsToHome(credwriter.CredsInitCredentials); err != nil {
 		log.Printf("non-fatal error copying credentials: %q", err)
 	}
 

pkg/credentials/common/constants.go

@@ -0,0 +1,21 @@
+package common
+
+// Secret key constants used in credential files,
+// so as to avoid reliance on corev1.Secret.
+//
+//nolint:gosec // for known Kubernetes secret-type constants, not real credentials
+const (
+	BasicAuthUsernameKey          = "username"
+	BasicAuthPasswordKey          = "password"
+	SSHAuthPrivateKey             = "ssh-privatekey"
+	DockerConfigKey               = ".dockercfg"
+	DockerConfigJsonKey           = ".dockerconfigjson"
+	SecretTypeBasicAuth           = "kubernetes.io/basic-auth"
+	SecretTypeSSHAuth             = "kubernetes.io/ssh-auth"
+	SecretTypeDockerConfigJson    = "kubernetes.io/dockerconfigjson"
+	SecretTypeDockercfg           = "kubernetes.io/dockercfg"
+	SecretTypeServiceAccountToken = "kubernetes.io/service-account-token"
+	SecretTypeOpaque              = "kubernetes.io/opaque"
+	SecretTypeTLS                 = "kubernetes.io/tls"
+	SecretTypeBootstrapToken      = "kubernetes.io/bootstrap-token"
+)

pkg/credentials/dockercreds/creds.go

@@ -25,8 +25,9 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/tektoncd/pipeline/pkg/credentials"
-	corev1 "k8s.io/api/core/v1"
+	"github.com/tektoncd/pipeline/pkg/credentials/common"
+	credmatcher "github.com/tektoncd/pipeline/pkg/credentials/matcher"
+	credwriter "github.com/tektoncd/pipeline/pkg/credentials/writer"
 )
 
 const annotationPrefix = "tekton.dev/docker-"
@@ -117,15 +118,15 @@ type entry struct {
 }
 
 func newEntry(secret string) (*entry, error) {
-	secretPath := credentials.VolumeName(secret)
+	secretPath := credmatcher.VolumeName(secret)
 
-	ub, err := os.ReadFile(filepath.Join(secretPath, corev1.BasicAuthUsernameKey))
+	ub, err := os.ReadFile(filepath.Join(secretPath, common.BasicAuthUsernameKey))
 	if err != nil {
 		return nil, err
 	}
 	username := string(ub)
 
-	pb, err := os.ReadFile(filepath.Join(secretPath, corev1.BasicAuthPasswordKey))
+	pb, err := os.ReadFile(filepath.Join(secretPath, common.BasicAuthPasswordKey))
 	if err != nil {
 		return nil, err
 	}
@@ -143,25 +144,30 @@ func newEntry(secret string) (*entry, error) {
 type basicDockerBuilder struct{}
 
 // NewBuilder returns a new builder for Docker credentials.
-func NewBuilder() credentials.Builder { return &basicDockerBuilder{} }
+func NewBuilder() interface {
+	credmatcher.Matcher
+	credwriter.Writer
+} {
+	return &basicDockerBuilder{}
+}
 
 // MatchingAnnotations extracts flags for the credential helper
 // from the supplied secret and returns a slice (of length 0 or
 // greater) of applicable domains.
-func (*basicDockerBuilder) MatchingAnnotations(secret *corev1.Secret) []string {
+func (*basicDockerBuilder) MatchingAnnotations(secret credmatcher.Secret) []string {
 	var flags []string
-	switch secret.Type {
-	case corev1.SecretTypeBasicAuth:
-		for _, v := range credentials.SortAnnotations(secret.Annotations, annotationPrefix) {
-			flags = append(flags, fmt.Sprintf("-basic-docker=%s=%s", secret.Name, v))
+	switch credmatcher.GetSecretType(secret) {
+	case common.SecretTypeBasicAuth:
+		for _, v := range credwriter.SortAnnotations(secret.GetAnnotations(), annotationPrefix) {
+			flags = append(flags, fmt.Sprintf("-basic-docker=%s=%s", secret.GetName(), v))
 		}
-	case corev1.SecretTypeDockerConfigJson:
-		flags = append(flags, "-docker-config="+secret.Name)
-	case corev1.SecretTypeDockercfg:
-		flags = append(flags, "-docker-cfg="+secret.Name)
+	case common.SecretTypeDockerConfigJson:
+		flags = append(flags, "-docker-config="+secret.GetName())
+	case common.SecretTypeDockercfg:
+		flags = append(flags, "-docker-cfg="+secret.GetName())
 
-	case corev1.SecretTypeOpaque, corev1.SecretTypeServiceAccountToken, corev1.SecretTypeSSHAuth, corev1.SecretTypeTLS, corev1.SecretTypeBootstrapToken:
-		return flags
+	case common.SecretTypeOpaque, common.SecretTypeServiceAccountToken, common.SecretTypeSSHAuth, common.SecretTypeTLS, common.SecretTypeBootstrapToken:
+		fallthrough
 
 	default:
 		return flags
@@ -218,9 +224,9 @@ func (*basicDockerBuilder) Write(directory string) error {
 }
 
 func authsFromDockerCfg(secret string) (map[string]entry, error) {
-	secretPath := credentials.VolumeName(secret)
+	secretPath := credmatcher.VolumeName(secret)
 	m := make(map[string]entry)
-	data, err := os.ReadFile(filepath.Join(secretPath, corev1.DockerConfigKey))
+	data, err := os.ReadFile(filepath.Join(secretPath, common.DockerConfigKey))
 	if err != nil {
 		return m, err
 	}
@@ -229,10 +235,10 @@ func authsFromDockerCfg(secret string) (map[string]entry, error) {
 }
 
 func authsFromDockerConfig(secret string) (map[string]entry, error) {
-	secretPath := credentials.VolumeName(secret)
+	secretPath := credmatcher.VolumeName(secret)
 	m := make(map[string]entry)
 	c := configFile{}
-	data, err := os.ReadFile(filepath.Join(secretPath, corev1.DockerConfigJsonKey))
+	data, err := os.ReadFile(filepath.Join(secretPath, common.DockerConfigJsonKey))
 	if err != nil {
 		return m, err
 	}

pkg/credentials/dockercreds/creds_test.go

@@ -23,14 +23,14 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
-	"github.com/tektoncd/pipeline/pkg/credentials"
+	credmatcher "github.com/tektoncd/pipeline/pkg/credentials/matcher"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func TestFlagHandling(t *testing.T) {
-	credentials.VolumePath = t.TempDir()
-	dir := credentials.VolumeName("foo")
+	credmatcher.VolumePath = t.TempDir()
+	dir := credmatcher.VolumeName("foo")
 	if err := os.MkdirAll(dir, os.ModePerm); err != nil {
 		t.Fatalf("os.MkdirAll(%s) = %v", dir, err)
 	}
@@ -50,12 +50,12 @@ func TestFlagHandling(t *testing.T) {
 		t.Fatalf("flag.CommandLine.Parse() = %v", err)
 	}
 
-	t.Setenv("HOME", credentials.VolumePath)
-	if err := NewBuilder().Write(credentials.VolumePath); err != nil {
+	t.Setenv("HOME", credmatcher.VolumePath)
+	if err := NewBuilder().Write(credmatcher.VolumePath); err != nil {
 		t.Fatalf("Write() = %v", err)
 	}
 
-	b, err := os.ReadFile(filepath.Join(credentials.VolumePath, ".docker", "config.json"))
+	b, err := os.ReadFile(filepath.Join(credmatcher.VolumePath, ".docker", "config.json"))
 	if err != nil {
 		t.Fatalf("os.ReadFile(.docker/config.json) = %v", err)
 	}
@@ -68,8 +68,8 @@ func TestFlagHandling(t *testing.T) {
 }
 
 func TestFlagHandlingTwice(t *testing.T) {
-	credentials.VolumePath = t.TempDir()
-	fooDir := credentials.VolumeName("foo")
+	credmatcher.VolumePath = t.TempDir()
+	fooDir := credmatcher.VolumeName("foo")
 	if err := os.MkdirAll(fooDir, os.ModePerm); err != nil {
 		t.Fatalf("os.MkdirAll(%s) = %v", fooDir, err)
 	}
@@ -79,7 +79,7 @@ func TestFlagHandlingTwice(t *testing.T) {
 	if err := os.WriteFile(filepath.Join(fooDir, corev1.BasicAuthPasswordKey), []byte("blah"), 0o777); err != nil {
 		t.Fatalf("os.WriteFile(password) = %v", err)
 	}
-	barDir := credentials.VolumeName("bar")
+	barDir := credmatcher.VolumeName("bar")
 	if err := os.MkdirAll(barDir, os.ModePerm); err != nil {
 		t.Fatalf("os.MkdirAll(%s) = %v", barDir, err)
 	}
@@ -100,12 +100,12 @@ func TestFlagHandlingTwice(t *testing.T) {
 		t.Fatalf("flag.CommandLine.Parse() = %v", err)
 	}
 
-	t.Setenv("HOME", credentials.VolumePath)
-	if err := NewBuilder().Write(credentials.VolumePath); err != nil {
+	t.Setenv("HOME", credmatcher.VolumePath)
+	if err := NewBuilder().Write(credmatcher.VolumePath); err != nil {
 		t.Fatalf("Write() = %v", err)
 	}
 
-	b, err := os.ReadFile(filepath.Join(credentials.VolumePath, ".docker", "config.json"))
+	b, err := os.ReadFile(filepath.Join(credmatcher.VolumePath, ".docker", "config.json"))
 	if err != nil {
 		t.Fatalf("os.ReadFile(.docker/config.json) = %v", err)
 	}
@@ -118,8 +118,8 @@ func TestFlagHandlingTwice(t *testing.T) {
 }
 
 func TestFlagHandlingMissingFiles(t *testing.T) {
-	credentials.VolumePath = t.TempDir()
-	dir := credentials.VolumeName("not-found")
+	credmatcher.VolumePath = t.TempDir()
+	dir := credmatcher.VolumeName("not-found")
 	if err := os.MkdirAll(dir, os.ModePerm); err != nil {
 		t.Fatalf("os.MkdirAll(%s) = %v", dir, err)
 	}
@@ -132,8 +132,8 @@ func TestFlagHandlingMissingFiles(t *testing.T) {
 }
 
 func TestFlagHandlingURLCollision(t *testing.T) {
-	credentials.VolumePath = t.TempDir()
-	dir := credentials.VolumeName("foo")
+	credmatcher.VolumePath = t.TempDir()
+	dir := credmatcher.VolumeName("foo")
 	if err := os.MkdirAll(dir, os.ModePerm); err != nil {
 		t.Fatalf("os.MkdirAll(%s) = %v", dir, err)
 	}
@@ -244,8 +244,8 @@ func TestMatchingAnnotations(t *testing.T) {
 }
 
 func TestMultipleFlagHandling(t *testing.T) {
-	credentials.VolumePath = t.TempDir()
-	fooDir := credentials.VolumeName("foo")
+	credmatcher.VolumePath = t.TempDir()
+	fooDir := credmatcher.VolumeName("foo")
 	if err := os.MkdirAll(fooDir, os.ModePerm); err != nil {
 		t.Fatalf("os.MkdirAll(%s) = %v", fooDir, err)
 	}
@@ -256,31 +256,31 @@ func TestMultipleFlagHandling(t *testing.T) {
 		t.Fatalf("os.WriteFile(password) = %v", err)
 	}
 
-	barDir := credentials.VolumeName("bar")
+	barDir := credmatcher.VolumeName("bar")
 	if err := os.MkdirAll(barDir, os.ModePerm); err != nil {
 		t.Fatalf("os.MkdirAll(%s) = %v", barDir, err)
 	}
 	if err := os.WriteFile(filepath.Join(barDir, corev1.DockerConfigJsonKey), []byte(`{"auths":{"https://index.docker.io/v1":{"auth":"fooisbar"}}}`), 0o777); err != nil {
 		t.Fatalf("os.WriteFile(username) = %v", err)
 	}
 
-	blubbDir := credentials.VolumeName("blubb")
+	blubbDir := credmatcher.VolumeName("blubb")
 	if err := os.MkdirAll(blubbDir, os.ModePerm); err != nil {
 		t.Fatalf("os.MkdirAll(%s) = %v", blubbDir, err)
 	}
 	if err := os.WriteFile(filepath.Join(blubbDir, corev1.DockerConfigJsonKey), []byte(`{"auths":{"us.icr.io":{"auth":"fooisblubb"}}}`), 0o777); err != nil {
 		t.Fatalf("os.WriteFile(username) = %v", err)
 	}
 
-	bazDir := credentials.VolumeName("baz")
+	bazDir := credmatcher.VolumeName("baz")
 	if err := os.MkdirAll(bazDir, os.ModePerm); err != nil {
 		t.Fatalf("os.MkdirAll(%s) = %v", bazDir, err)
 	}
 	if err := os.WriteFile(filepath.Join(bazDir, corev1.DockerConfigKey), []byte(`{"https://my.registry/v1":{"auth":"fooisbaz"}}`), 0o777); err != nil {
 		t.Fatalf("os.WriteFile(username) = %v", err)
 	}
 
-	blaDir := credentials.VolumeName("bla")
+	blaDir := credmatcher.VolumeName("bla")
 	if err := os.MkdirAll(blaDir, os.ModePerm); err != nil {
 		t.Fatalf("os.MkdirAll(%s) = %v", blaDir, err)
 	}
@@ -301,12 +301,12 @@ func TestMultipleFlagHandling(t *testing.T) {
 		t.Fatalf("flag.CommandLine.Parse() = %v", err)
 	}
 
-	t.Setenv("HOME", credentials.VolumePath)
-	if err := NewBuilder().Write(credentials.VolumePath); err != nil {
+	t.Setenv("HOME", credmatcher.VolumePath)
+	if err := NewBuilder().Write(credmatcher.VolumePath); err != nil {
 		t.Fatalf("Write() = %v", err)
 	}
 
-	b, err := os.ReadFile(filepath.Join(credentials.VolumePath, ".docker", "config.json"))
+	b, err := os.ReadFile(filepath.Join(credmatcher.VolumePath, ".docker", "config.json"))
 	if err != nil {
 		t.Fatalf("os.ReadFile(.docker/config.json) = %v", err)
 	}
@@ -321,8 +321,8 @@ func TestMultipleFlagHandling(t *testing.T) {
 // TestNoAuthProvided confirms that providing zero secrets results in no docker
 // credential file being written to disk.
 func TestNoAuthProvided(t *testing.T) {
-	credentials.VolumePath = t.TempDir()
-	fooDir := credentials.VolumeName("foo")
+	credmatcher.VolumePath = t.TempDir()
+	fooDir := credmatcher.VolumeName("foo")
 	if err := os.MkdirAll(fooDir, os.ModePerm); err != nil {
 		t.Fatalf("os.MkdirAll(%s) = %v", fooDir, err)
 	}
@@ -333,11 +333,11 @@ func TestNoAuthProvided(t *testing.T) {
 	if err != nil {
 		t.Fatalf("flag.CommandLine.Parse() = %v", err)
 	}
-	t.Setenv("HOME", credentials.VolumePath)
-	if err := NewBuilder().Write(credentials.VolumePath); err != nil {
+	t.Setenv("HOME", credmatcher.VolumePath)
+	if err := NewBuilder().Write(credmatcher.VolumePath); err != nil {
 		t.Fatalf("Write() = %v", err)
 	}
-	_, err = os.ReadFile(filepath.Join(credentials.VolumePath, ".docker", "config.json"))
+	_, err = os.ReadFile(filepath.Join(credmatcher.VolumePath, ".docker", "config.json"))
 	if err == nil || !os.IsNotExist(err) {
 		t.Errorf("expected does not exist error but received: %v", err)
 	}

pkg/credentials/gitcreds/basic.go

@@ -23,8 +23,8 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/tektoncd/pipeline/pkg/credentials"
-	corev1 "k8s.io/api/core/v1"
+	"github.com/tektoncd/pipeline/pkg/credentials/common"
+	credmatcher "github.com/tektoncd/pipeline/pkg/credentials/matcher"
 )
 
 // As the flag is read, this status is populated.
@@ -121,15 +121,15 @@ func (be *basicEntry) escapedUsername() string {
 }
 
 func newBasicEntry(u, secret string) (*basicEntry, error) {
-	secretPath := credentials.VolumeName(secret)
+	secretPath := credmatcher.VolumeName(secret)
 
-	ub, err := os.ReadFile(filepath.Join(secretPath, corev1.BasicAuthUsernameKey))
+	ub, err := os.ReadFile(filepath.Join(secretPath, common.BasicAuthUsernameKey))
 	if err != nil {
 		return nil, err
 	}
 	username := string(ub)
 
-	pb, err := os.ReadFile(filepath.Join(secretPath, corev1.BasicAuthPasswordKey))
+	pb, err := os.ReadFile(filepath.Join(secretPath, common.BasicAuthPasswordKey))
 	if err != nil {
 		return nil, err
 	}