Skip to content

Commit 3499e98

Browse files
waveywaveswaveywaves
committed
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.
1 parent 852c443 commit 3499e98

File tree

11 files changed

+176
-136
lines changed

11 files changed

+176
-136
lines changed

‎cmd/entrypoint/main.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ import (
3333
featureFlags "github.com/tektoncd/pipeline/pkg/apis/config"
3434
"github.com/tektoncd/pipeline/pkg/apis/pipeline"
3535
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
36-
"github.com/tektoncd/pipeline/pkg/credentials"
3736
"github.com/tektoncd/pipeline/pkg/credentials/dockercreds"
3837
"github.com/tektoncd/pipeline/pkg/credentials/gitcreds"
38+
credwriter "github.com/tektoncd/pipeline/pkg/credentials/writer"
3939
"github.com/tektoncd/pipeline/pkg/entrypoint"
4040
"github.com/tektoncd/pipeline/pkg/spire"
4141
"github.com/tektoncd/pipeline/pkg/spire/config"
@@ -96,7 +96,7 @@ func main() {
9696
// from secret volume mounts to /tekton/creds. This is done to support the expansion
9797
// of a variable, $(credentials.path), that resolves to a single place with all the
9898
// stored credentials.
99-
builders := []credentials.Builder{dockercreds.NewBuilder(), gitcreds.NewBuilder()}
99+
builders := []credwriter.Builder{dockercreds.NewBuilder(), gitcreds.NewBuilder()}
100100
for _, c := range builders {
101101
if err := c.Write(pipeline.CredsDir); err != nil {
102102
log.Printf("Error initializing credentials: %s", err)
@@ -164,7 +164,7 @@ func main() {
164164

165165
// Copy any creds injected by the controller into the $HOME directory of the current
166166
// user so that they're discoverable by git / ssh.
167-
if err := credentials.CopyCredsToHome(credentials.CredsInitCredentials); err != nil {
167+
if err := credwriter.CopyCredsToHome(credwriter.CredsInitCredentials); err != nil {
168168
log.Printf("non-fatal error copying credentials: %q", err)
169169
}
170170

‎pkg/credentials/dockercreds/creds.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ import (
2525
"path/filepath"
2626
"strings"
2727

28-
"github.com/tektoncd/pipeline/pkg/credentials"
28+
credmatcher "github.com/tektoncd/pipeline/pkg/credentials/matcher"
29+
credwriter "github.com/tektoncd/pipeline/pkg/credentials/writer"
2930
corev1 "k8s.io/api/core/v1"
3031
)
3132

@@ -117,7 +118,7 @@ type entry struct {
117118
}
118119

119120
func newEntry(secret string) (*entry, error) {
120-
secretPath := credentials.VolumeName(secret)
121+
secretPath := credmatcher.VolumeName(secret)
121122

122123
ub, err := os.ReadFile(filepath.Join(secretPath, corev1.BasicAuthUsernameKey))
123124
if err != nil {
@@ -143,7 +144,12 @@ func newEntry(secret string) (*entry, error) {
143144
type basicDockerBuilder struct{}
144145

145146
// NewBuilder returns a new builder for Docker credentials.
146-
func NewBuilder() credentials.Builder { return &basicDockerBuilder{} }
147+
func NewBuilder() interface {
148+
credmatcher.Builder
149+
credwriter.Builder
150+
} {
151+
return &basicDockerBuilder{}
152+
}
147153

148154
// MatchingAnnotations extracts flags for the credential helper
149155
// from the supplied secret and returns a slice (of length 0 or
@@ -152,7 +158,7 @@ func (*basicDockerBuilder) MatchingAnnotations(secret *corev1.Secret) []string {
152158
var flags []string
153159
switch secret.Type {
154160
case corev1.SecretTypeBasicAuth:
155-
for _, v := range credentials.SortAnnotations(secret.Annotations, annotationPrefix) {
161+
for _, v := range credwriter.SortAnnotations(secret.Annotations, annotationPrefix) {
156162
flags = append(flags, fmt.Sprintf("-basic-docker=%s=%s", secret.Name, v))
157163
}
158164
case corev1.SecretTypeDockerConfigJson:
@@ -218,7 +224,7 @@ func (*basicDockerBuilder) Write(directory string) error {
218224
}
219225

220226
func authsFromDockerCfg(secret string) (map[string]entry, error) {
221-
secretPath := credentials.VolumeName(secret)
227+
secretPath := credmatcher.VolumeName(secret)
222228
m := make(map[string]entry)
223229
data, err := os.ReadFile(filepath.Join(secretPath, corev1.DockerConfigKey))
224230
if err != nil {
@@ -229,7 +235,7 @@ func authsFromDockerCfg(secret string) (map[string]entry, error) {
229235
}
230236

231237
func authsFromDockerConfig(secret string) (map[string]entry, error) {
232-
secretPath := credentials.VolumeName(secret)
238+
secretPath := credmatcher.VolumeName(secret)
233239
m := make(map[string]entry)
234240
c := configFile{}
235241
data, err := os.ReadFile(filepath.Join(secretPath, corev1.DockerConfigJsonKey))

‎pkg/credentials/dockercreds/creds_test.go

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,14 @@ import (
2323
"testing"
2424

2525
"github.com/google/go-cmp/cmp"
26-
"github.com/tektoncd/pipeline/pkg/credentials"
26+
credmatcher "github.com/tektoncd/pipeline/pkg/credentials/matcher"
2727
corev1 "k8s.io/api/core/v1"
2828
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2929
)
3030

3131
func TestFlagHandling(t *testing.T) {
32-
credentials.VolumePath = t.TempDir()
33-
dir := credentials.VolumeName("foo")
32+
credmatcher.VolumePath = t.TempDir()
33+
dir := credmatcher.VolumeName("foo")
3434
if err := os.MkdirAll(dir, os.ModePerm); err != nil {
3535
t.Fatalf("os.MkdirAll(%s) = %v", dir, err)
3636
}
@@ -50,12 +50,12 @@ func TestFlagHandling(t *testing.T) {
5050
t.Fatalf("flag.CommandLine.Parse() = %v", err)
5151
}
5252

53-
t.Setenv("HOME", credentials.VolumePath)
54-
if err := NewBuilder().Write(credentials.VolumePath); err != nil {
53+
t.Setenv("HOME", credmatcher.VolumePath)
54+
if err := NewBuilder().Write(credmatcher.VolumePath); err != nil {
5555
t.Fatalf("Write() = %v", err)
5656
}
5757

58-
b, err := os.ReadFile(filepath.Join(credentials.VolumePath, ".docker", "config.json"))
58+
b, err := os.ReadFile(filepath.Join(credmatcher.VolumePath, ".docker", "config.json"))
5959
if err != nil {
6060
t.Fatalf("os.ReadFile(.docker/config.json) = %v", err)
6161
}
@@ -68,8 +68,8 @@ func TestFlagHandling(t *testing.T) {
6868
}
6969

7070
func TestFlagHandlingTwice(t *testing.T) {
71-
credentials.VolumePath = t.TempDir()
72-
fooDir := credentials.VolumeName("foo")
71+
credmatcher.VolumePath = t.TempDir()
72+
fooDir := credmatcher.VolumeName("foo")
7373
if err := os.MkdirAll(fooDir, os.ModePerm); err != nil {
7474
t.Fatalf("os.MkdirAll(%s) = %v", fooDir, err)
7575
}
@@ -79,7 +79,7 @@ func TestFlagHandlingTwice(t *testing.T) {
7979
if err := os.WriteFile(filepath.Join(fooDir, corev1.BasicAuthPasswordKey), []byte("blah"), 0o777); err != nil {
8080
t.Fatalf("os.WriteFile(password) = %v", err)
8181
}
82-
barDir := credentials.VolumeName("bar")
82+
barDir := credmatcher.VolumeName("bar")
8383
if err := os.MkdirAll(barDir, os.ModePerm); err != nil {
8484
t.Fatalf("os.MkdirAll(%s) = %v", barDir, err)
8585
}
@@ -100,12 +100,12 @@ func TestFlagHandlingTwice(t *testing.T) {
100100
t.Fatalf("flag.CommandLine.Parse() = %v", err)
101101
}
102102

103-
t.Setenv("HOME", credentials.VolumePath)
104-
if err := NewBuilder().Write(credentials.VolumePath); err != nil {
103+
t.Setenv("HOME", credmatcher.VolumePath)
104+
if err := NewBuilder().Write(credmatcher.VolumePath); err != nil {
105105
t.Fatalf("Write() = %v", err)
106106
}
107107

108-
b, err := os.ReadFile(filepath.Join(credentials.VolumePath, ".docker", "config.json"))
108+
b, err := os.ReadFile(filepath.Join(credmatcher.VolumePath, ".docker", "config.json"))
109109
if err != nil {
110110
t.Fatalf("os.ReadFile(.docker/config.json) = %v", err)
111111
}
@@ -118,8 +118,8 @@ func TestFlagHandlingTwice(t *testing.T) {
118118
}
119119

120120
func TestFlagHandlingMissingFiles(t *testing.T) {
121-
credentials.VolumePath = t.TempDir()
122-
dir := credentials.VolumeName("not-found")
121+
credmatcher.VolumePath = t.TempDir()
122+
dir := credmatcher.VolumeName("not-found")
123123
if err := os.MkdirAll(dir, os.ModePerm); err != nil {
124124
t.Fatalf("os.MkdirAll(%s) = %v", dir, err)
125125
}
@@ -132,8 +132,8 @@ func TestFlagHandlingMissingFiles(t *testing.T) {
132132
}
133133

134134
func TestFlagHandlingURLCollision(t *testing.T) {
135-
credentials.VolumePath = t.TempDir()
136-
dir := credentials.VolumeName("foo")
135+
credmatcher.VolumePath = t.TempDir()
136+
dir := credmatcher.VolumeName("foo")
137137
if err := os.MkdirAll(dir, os.ModePerm); err != nil {
138138
t.Fatalf("os.MkdirAll(%s) = %v", dir, err)
139139
}
@@ -244,8 +244,8 @@ func TestMatchingAnnotations(t *testing.T) {
244244
}
245245

246246
func TestMultipleFlagHandling(t *testing.T) {
247-
credentials.VolumePath = t.TempDir()
248-
fooDir := credentials.VolumeName("foo")
247+
credmatcher.VolumePath = t.TempDir()
248+
fooDir := credmatcher.VolumeName("foo")
249249
if err := os.MkdirAll(fooDir, os.ModePerm); err != nil {
250250
t.Fatalf("os.MkdirAll(%s) = %v", fooDir, err)
251251
}
@@ -256,31 +256,31 @@ func TestMultipleFlagHandling(t *testing.T) {
256256
t.Fatalf("os.WriteFile(password) = %v", err)
257257
}
258258

259-
barDir := credentials.VolumeName("bar")
259+
barDir := credmatcher.VolumeName("bar")
260260
if err := os.MkdirAll(barDir, os.ModePerm); err != nil {
261261
t.Fatalf("os.MkdirAll(%s) = %v", barDir, err)
262262
}
263263
if err := os.WriteFile(filepath.Join(barDir, corev1.DockerConfigJsonKey), []byte(`{"auths":{"https://index.docker.io/v1":{"auth":"fooisbar"}}}`), 0o777); err != nil {
264264
t.Fatalf("os.WriteFile(username) = %v", err)
265265
}
266266

267-
blubbDir := credentials.VolumeName("blubb")
267+
blubbDir := credmatcher.VolumeName("blubb")
268268
if err := os.MkdirAll(blubbDir, os.ModePerm); err != nil {
269269
t.Fatalf("os.MkdirAll(%s) = %v", blubbDir, err)
270270
}
271271
if err := os.WriteFile(filepath.Join(blubbDir, corev1.DockerConfigJsonKey), []byte(`{"auths":{"us.icr.io":{"auth":"fooisblubb"}}}`), 0o777); err != nil {
272272
t.Fatalf("os.WriteFile(username) = %v", err)
273273
}
274274

275-
bazDir := credentials.VolumeName("baz")
275+
bazDir := credmatcher.VolumeName("baz")
276276
if err := os.MkdirAll(bazDir, os.ModePerm); err != nil {
277277
t.Fatalf("os.MkdirAll(%s) = %v", bazDir, err)
278278
}
279279
if err := os.WriteFile(filepath.Join(bazDir, corev1.DockerConfigKey), []byte(`{"https://my.registry/v1":{"auth":"fooisbaz"}}`), 0o777); err != nil {
280280
t.Fatalf("os.WriteFile(username) = %v", err)
281281
}
282282

283-
blaDir := credentials.VolumeName("bla")
283+
blaDir := credmatcher.VolumeName("bla")
284284
if err := os.MkdirAll(blaDir, os.ModePerm); err != nil {
285285
t.Fatalf("os.MkdirAll(%s) = %v", blaDir, err)
286286
}
@@ -301,12 +301,12 @@ func TestMultipleFlagHandling(t *testing.T) {
301301
t.Fatalf("flag.CommandLine.Parse() = %v", err)
302302
}
303303

304-
t.Setenv("HOME", credentials.VolumePath)
305-
if err := NewBuilder().Write(credentials.VolumePath); err != nil {
304+
t.Setenv("HOME", credmatcher.VolumePath)
305+
if err := NewBuilder().Write(credmatcher.VolumePath); err != nil {
306306
t.Fatalf("Write() = %v", err)
307307
}
308308

309-
b, err := os.ReadFile(filepath.Join(credentials.VolumePath, ".docker", "config.json"))
309+
b, err := os.ReadFile(filepath.Join(credmatcher.VolumePath, ".docker", "config.json"))
310310
if err != nil {
311311
t.Fatalf("os.ReadFile(.docker/config.json) = %v", err)
312312
}
@@ -321,8 +321,8 @@ func TestMultipleFlagHandling(t *testing.T) {
321321
// TestNoAuthProvided confirms that providing zero secrets results in no docker
322322
// credential file being written to disk.
323323
func TestNoAuthProvided(t *testing.T) {
324-
credentials.VolumePath = t.TempDir()
325-
fooDir := credentials.VolumeName("foo")
324+
credmatcher.VolumePath = t.TempDir()
325+
fooDir := credmatcher.VolumeName("foo")
326326
if err := os.MkdirAll(fooDir, os.ModePerm); err != nil {
327327
t.Fatalf("os.MkdirAll(%s) = %v", fooDir, err)
328328
}
@@ -333,11 +333,11 @@ func TestNoAuthProvided(t *testing.T) {
333333
if err != nil {
334334
t.Fatalf("flag.CommandLine.Parse() = %v", err)
335335
}
336-
t.Setenv("HOME", credentials.VolumePath)
337-
if err := NewBuilder().Write(credentials.VolumePath); err != nil {
336+
t.Setenv("HOME", credmatcher.VolumePath)
337+
if err := NewBuilder().Write(credmatcher.VolumePath); err != nil {
338338
t.Fatalf("Write() = %v", err)
339339
}
340-
_, err = os.ReadFile(filepath.Join(credentials.VolumePath, ".docker", "config.json"))
340+
_, err = os.ReadFile(filepath.Join(credmatcher.VolumePath, ".docker", "config.json"))
341341
if err == nil || !os.IsNotExist(err) {
342342
t.Errorf("expected does not exist error but received: %v", err)
343343
}

‎pkg/credentials/gitcreds/basic.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
"path/filepath"
2424
"strings"
2525

26-
"github.com/tektoncd/pipeline/pkg/credentials"
26+
credmatcher "github.com/tektoncd/pipeline/pkg/credentials/matcher"
2727
corev1 "k8s.io/api/core/v1"
2828
)
2929

@@ -121,7 +121,7 @@ func (be *basicEntry) escapedUsername() string {
121121
}
122122

123123
func newBasicEntry(u, secret string) (*basicEntry, error) {
124-
secretPath := credentials.VolumeName(secret)
124+
secretPath := credmatcher.VolumeName(secret)
125125

126126
ub, err := os.ReadFile(filepath.Join(secretPath, corev1.BasicAuthUsernameKey))
127127
if err != nil {

‎pkg/credentials/gitcreds/creds.go

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ import (
2020
"flag"
2121
"fmt"
2222

23-
"github.com/tektoncd/pipeline/pkg/credentials"
23+
credmatcher "github.com/tektoncd/pipeline/pkg/credentials/matcher"
24+
credwriter "github.com/tektoncd/pipeline/pkg/credentials/writer"
2425
corev1 "k8s.io/api/core/v1"
2526
)
2627

@@ -48,38 +49,42 @@ func flags(fs *flag.FlagSet) {
4849
fs.Var(&sshConfig, sshFlag, "List of secret=url pairs.")
4950
}
5051

51-
type gitConfigBuilder struct{}
52+
type gitBuilder struct{}
5253

5354
// NewBuilder returns a new builder for Git credentials.
54-
func NewBuilder() credentials.Builder { return &gitConfigBuilder{} }
55+
func NewBuilder() interface {
56+
credmatcher.Builder
57+
credwriter.Builder
58+
} {
59+
return &gitBuilder{}
60+
}
5561

5662
// MatchingAnnotations extracts flags for the credential helper
5763
// from the supplied secret and returns a slice (of length 0 or
5864
// greater) of applicable domains.
59-
func (*gitConfigBuilder) MatchingAnnotations(secret *corev1.Secret) []string {
60-
var flagName string
65+
func (*gitBuilder) MatchingAnnotations(secret *corev1.Secret) []string {
6166
var flags []string
6267
switch secret.Type {
6368
case corev1.SecretTypeBasicAuth:
64-
flagName = basicAuthFlag
69+
for _, v := range credwriter.SortAnnotations(secret.Annotations, annotationPrefix) {
70+
flags = append(flags, fmt.Sprintf("-%s=%s=%s", basicAuthFlag, secret.Name, v))
71+
}
6572

6673
case corev1.SecretTypeSSHAuth:
67-
flagName = sshFlag
74+
for _, v := range credwriter.SortAnnotations(secret.Annotations, annotationPrefix) {
75+
flags = append(flags, fmt.Sprintf("-%s=%s=%s", sshFlag, secret.Name, v))
76+
}
6877

6978
case corev1.SecretTypeOpaque, corev1.SecretTypeServiceAccountToken, corev1.SecretTypeDockercfg, corev1.SecretTypeDockerConfigJson, corev1.SecretTypeTLS, corev1.SecretTypeBootstrapToken:
7079
return flags
7180

7281
default:
7382
return flags
7483
}
75-
76-
for _, v := range credentials.SortAnnotations(secret.Annotations, annotationPrefix) {
77-
flags = append(flags, fmt.Sprintf("-%s=%s=%s", flagName, secret.Name, v))
78-
}
7984
return flags
8085
}
8186

82-
func (*gitConfigBuilder) Write(directory string) error {
87+
func (*gitBuilder) Write(directory string) error {
8388
if err := basicConfig.Write(directory); err != nil {
8489
return err
8590
}

0 commit comments

Comments
 (0)