Skip to content

Commit 35d83ff

Browse files
renzodavid9renzodavid9
committed
feat: tests for new native client and changes in other places to make it work with new implementation
1 parent 6e17b2d commit 35d83ff

File tree

6 files changed

+411
-92
lines changed

6 files changed

+411
-92
lines changed

‎pkg/skaffold/deploy/kubectl/kubectl_test.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import (
2323
"fmt"
2424
"io"
2525
"os"
26-
"path/filepath"
2726
"testing"
2827
"time"
2928

@@ -40,6 +39,11 @@ import (
4039
"github.com/GoogleContainerTools/skaffold/v2/testutil"
4140
)
4241

42+
type gcsClientMock struct{}
43+
44+
func (g gcsClientMock) DownloadRecursive(ctx context.Context, src, dst string) error {
45+
return nil
46+
}
4347
func TestKubectlV1RenderDeploy(t *testing.T) {
4448
tests := []struct {
4549
description string
@@ -520,13 +524,15 @@ func TestGCSManifests(t *testing.T) {
520524
RawK8s: []string{"gs://dev/deployment.yaml"},
521525
},
522526
commands: testutil.
523-
CmdRunOut(fmt.Sprintf("gsutil cp -r %s %s", "gs://dev/deployment.yaml", filepath.Join(manifest.ManifestTmpDir, manifest.ManifestsFromGCS)), "log").
524-
AndRun("kubectl --context kubecontext --namespace testNamespace apply -f -"),
527+
CmdRun("kubectl --context kubecontext --namespace testNamespace apply -f -"),
525528
}}
526529
for _, test := range tests {
527530
testutil.Run(t, test.description, func(t *testutil.T) {
528531
t.Override(&client.Client, deployutil.MockK8sClient)
529532
t.Override(&util.DefaultExecCommand, test.commands)
533+
t.Override(&manifest.GetGCSClient, func() manifest.GCSClient {
534+
return gcsClientMock{}
535+
})
530536
if err := os.MkdirAll(manifest.ManifestTmpDir, os.ModePerm); err != nil {
531537
t.Fatal(err)
532538
}

‎pkg/skaffold/gcs/client/native.go

Lines changed: 118 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,21 @@ import (
3131
"google.golang.org/api/iterator"
3232
)
3333

34-
// Obj contains information about the GCS object.
34+
var GetBucketManager = getBucketManager
35+
36+
// bucketHandler defines the available interactaions with a GCS bucket.
37+
type bucketHandler interface {
38+
// ListObjects list the objects that match the given query.
39+
ListObjects(ctx context.Context, q *storage.Query) ([]string, error)
40+
// DownloadObject download the object with the given uri in the localPath.
41+
DownloadObject(ctx context.Context, localPath, uri string) error
42+
// UploadObject creates a files with the given content with the objName.
43+
UploadObject(ctx context.Context, objName string, content *os.File) error
44+
// Close closes the bucket handler connection.
45+
Close()
46+
}
47+
48+
// uriInfo contains information about the GCS object.
3549
type uriInfo struct {
3650
// Bucket is the name of the GCS bucket.
3751
Bucket string
@@ -48,17 +62,16 @@ type Native struct{}
4862

4963
// Downloads the content that match the given src uri and subfolders.
5064
func (n *Native) DownloadRecursive(ctx context.Context, src, dst string) error {
51-
sc, err := storage.NewClient(ctx)
65+
uriInfo, err := n.parseGCSURI(src)
5266
if err != nil {
53-
return fmt.Errorf("error creating GCS Client: %w", err)
67+
return err
5468
}
55-
defer sc.Close()
5669

57-
uriInfo, err := n.parseGCSURI(src)
70+
bucket, err := GetBucketManager(ctx, uriInfo.Bucket)
5871
if err != nil {
5972
return err
6073
}
61-
bucket := sc.Bucket(uriInfo.Bucket)
74+
defer bucket.Close()
6275

6376
files, err := n.filesToDownload(ctx, bucket, uriInfo)
6477
if err != nil {
@@ -67,7 +80,15 @@ func (n *Native) DownloadRecursive(ctx context.Context, src, dst string) error {
6780

6881
for uri, localPath := range files {
6982
fullPath := filepath.Join(dst, localPath)
70-
if err := n.downloadFile(ctx, bucket, fullPath, uri); err != nil {
83+
84+
dir := filepath.Dir(fullPath)
85+
if _, err := os.Stat(dir); os.IsNotExist(err) {
86+
if err := os.MkdirAll(dir, os.ModePerm); err != nil {
87+
return fmt.Errorf("failed to create directory: %v", err)
88+
}
89+
}
90+
91+
if err := bucket.DownloadObject(ctx, fullPath, uri); err != nil {
7192
return err
7293
}
7394
}
@@ -77,45 +98,36 @@ func (n *Native) DownloadRecursive(ctx context.Context, src, dst string) error {
7798

7899
// Uploads a single file to the given dst.
79100
func (n *Native) UploadFile(ctx context.Context, src, dst string) error {
80-
sc, err := storage.NewClient(ctx)
81-
if err != nil {
82-
return fmt.Errorf("error creating GCS Client: %w", err)
83-
}
84-
defer sc.Close()
85-
86101
f, err := os.Open(src)
87102
if err != nil {
88103
return fmt.Errorf("error opening file: %w", err)
89104
}
90105
defer f.Close()
91106

92-
uinfo, err := n.parseGCSURI(dst)
107+
urinfo, err := n.parseGCSURI(dst)
93108
if err != nil {
94109
return err
95110
}
96-
bucket := sc.Bucket(uinfo.Bucket)
97111

98-
isDirectory, err := n.isGCSDirectory(ctx, bucket, uinfo)
112+
bucket, err := GetBucketManager(ctx, urinfo.Bucket)
99113
if err != nil {
100114
return err
101115
}
102116

103-
dstObj := uinfo.ObjPath
117+
isDirectory, err := n.isGCSDirectory(ctx, bucket, urinfo)
118+
if err != nil {
119+
return err
120+
}
121+
122+
dstObj := urinfo.ObjPath
104123
if isDirectory {
105124
dstObj, err = url.JoinPath(dstObj, filepath.Base(src))
106125
if err != nil {
107126
return err
108127
}
109128
}
110129

111-
wc := bucket.Object(dstObj).NewWriter(ctx)
112-
if _, err = io.Copy(wc, f); err != nil {
113-
return fmt.Errorf("error copying file to GCS: %w", err)
114-
}
115-
if err := wc.Close(); err != nil {
116-
return fmt.Errorf("error closing GCS writer: %w", err)
117-
}
118-
return nil
130+
return bucket.UploadObject(ctx, dstObj, f)
119131
}
120132

121133
func (n *Native) parseGCSURI(uri string) (uriInfo, error) {
@@ -136,10 +148,10 @@ func (n *Native) parseGCSURI(uri string) (uriInfo, error) {
136148
return gcsobj, nil
137149
}
138150

139-
func (n *Native) filesToDownload(ctx context.Context, bucket *storage.BucketHandle, uinfo uriInfo) (map[string]string, error) {
151+
func (n *Native) filesToDownload(ctx context.Context, bucket bucketHandler, urinfo uriInfo) (map[string]string, error) {
140152
uriToLocalPath := map[string]string{}
141153

142-
exactMatches, err := n.listObjects(ctx, bucket, &storage.Query{MatchGlob: uinfo.ObjPath})
154+
exactMatches, err := bucket.ListObjects(ctx, &storage.Query{MatchGlob: urinfo.ObjPath})
143155
if err != nil {
144156
return nil, err
145157
}
@@ -148,58 +160,37 @@ func (n *Native) filesToDownload(ctx context.Context, bucket *storage.BucketHand
148160
uriToLocalPath[match] = filepath.Base(match)
149161
}
150162

151-
recursiveMatches, err := n.recursiveListing(ctx, bucket, uinfo)
163+
recursiveMatches, err := n.recursiveListing(ctx, bucket, urinfo)
152164
if err != nil {
153165
return nil, err
154166
}
155167

156-
for _, match := range recursiveMatches {
157-
uriToLocalPath[match] = match
168+
for uri, match := range recursiveMatches {
169+
uriToLocalPath[uri] = match
158170
}
159171

160172
return uriToLocalPath, nil
161173
}
162174

163-
func (n *Native) listObjects(ctx context.Context, bucket *storage.BucketHandle, q *storage.Query) ([]string, error) {
164-
matches := []string{}
165-
it := bucket.Objects(ctx, q)
166-
167-
for {
168-
attrs, err := it.Next()
169-
if err == iterator.Done {
170-
break
171-
}
172-
173-
if err != nil {
174-
return nil, fmt.Errorf("failed to iterate objects: %v", err)
175-
}
176-
177-
if attrs.Name != "" {
178-
matches = append(matches, attrs.Name)
179-
}
180-
}
181-
return matches, nil
182-
}
183-
184-
func (n *Native) recursiveListing(ctx context.Context, bucket *storage.BucketHandle, uinfo uriInfo) (map[string]string, error) {
175+
func (n *Native) recursiveListing(ctx context.Context, bucket bucketHandler, urinfo uriInfo) (map[string]string, error) {
185176
uriToLocalPath := map[string]string{}
186-
recursiveURI := n.uriForRecursiveSearch(uinfo.ObjPath)
187-
recursiveMatches, err := n.listObjects(ctx, bucket, &storage.Query{MatchGlob: recursiveURI})
177+
recursiveURI := n.uriForRecursiveSearch(urinfo.ObjPath)
178+
recursiveMatches, err := bucket.ListObjects(ctx, &storage.Query{MatchGlob: recursiveURI})
188179
if err != nil {
189180
return nil, err
190181
}
191182

192-
prefixRemovalURI := n.uriForPrefixRemoval(uinfo.Full())
183+
prefixRemovalURI := n.uriForPrefixRemoval(urinfo.Full())
193184
prefixRemovalRegex, err := n.wildcardToRegex(prefixRemovalURI)
194185
if err != nil {
195186
return nil, err
196187
}
197188

198-
shouldRecreateFolders := !strings.Contains(uinfo.ObjPath, "**")
189+
shouldRecreateFolders := !strings.Contains(urinfo.ObjPath, "**")
199190
for _, match := range recursiveMatches {
200191
destPath := filepath.Base(match)
201192
if shouldRecreateFolders {
202-
matchWithBucket := uinfo.Bucket + "/" + match
193+
matchWithBucket := urinfo.Bucket + "/" + match
203194
destPath = string(prefixRemovalRegex.ReplaceAll([]byte(matchWithBucket), []byte("")))
204195
}
205196
uriToLocalPath[match] = destPath
@@ -254,15 +245,69 @@ func (n *Native) wildcardToRegex(wildcard string) (*regexp.Regexp, error) {
254245
return regexp.Compile(regexStr)
255246
}
256247

257-
func (n *Native) downloadFile(ctx context.Context, bucket *storage.BucketHandle, localPath, uri string) error {
258-
dir := filepath.Dir(localPath)
259-
if _, err := os.Stat(dir); os.IsNotExist(err) {
260-
if err := os.MkdirAll(dir, os.ModePerm); err != nil {
261-
return fmt.Errorf("failed to create directory: %v", err)
248+
func (n *Native) isGCSDirectory(ctx context.Context, bucket bucketHandler, urinfo uriInfo) (bool, error) {
249+
if urinfo.ObjPath == "" {
250+
return true, nil
251+
}
252+
253+
if strings.HasSuffix(urinfo.ObjPath, "/") {
254+
return true, nil
255+
}
256+
257+
q := &storage.Query{Prefix: urinfo.ObjPath + "/"}
258+
matches, err := bucket.ListObjects(ctx, q)
259+
if err != nil {
260+
return false, err
261+
}
262+
263+
if len(matches) > 0 {
264+
return true, nil
265+
}
266+
267+
return false, nil
268+
}
269+
270+
func getBucketManager(ctx context.Context, bucketName string) (bucketHandler, error) {
271+
sc, err := storage.NewClient(ctx)
272+
if err != nil {
273+
return nil, fmt.Errorf("error creating GCS Client: %w", err)
274+
}
275+
276+
return nativeBucketHandler{
277+
storageClient: sc,
278+
bucket: sc.Bucket(bucketName),
279+
}, nil
280+
}
281+
282+
// nativeBucketHandler implements a handler using the Cloud client libraries.
283+
type nativeBucketHandler struct {
284+
storageClient *storage.Client
285+
bucket *storage.BucketHandle
286+
}
287+
288+
func (nb nativeBucketHandler) ListObjects(ctx context.Context, q *storage.Query) ([]string, error) {
289+
matches := []string{}
290+
it := nb.bucket.Objects(ctx, q)
291+
292+
for {
293+
attrs, err := it.Next()
294+
if err == iterator.Done {
295+
break
296+
}
297+
298+
if err != nil {
299+
return nil, fmt.Errorf("failed to iterate objects: %v", err)
300+
}
301+
302+
if attrs.Name != "" {
303+
matches = append(matches, attrs.Name)
262304
}
263305
}
306+
return matches, nil
307+
}
264308

265-
reader, err := bucket.Object(uri).NewReader(ctx)
309+
func (nb nativeBucketHandler) DownloadObject(ctx context.Context, localPath, uri string) error {
310+
reader, err := nb.bucket.Object(uri).NewReader(ctx)
266311
if err != nil {
267312
return fmt.Errorf("failed to read object: %v", err)
268313
}
@@ -281,24 +326,17 @@ func (n *Native) downloadFile(ctx context.Context, bucket *storage.BucketHandle,
281326
return nil
282327
}
283328

284-
func (n *Native) isGCSDirectory(ctx context.Context, bucket *storage.BucketHandle, uinfo uriInfo) (bool, error) {
285-
if uinfo.ObjPath == "" {
286-
return true, nil
287-
}
288-
289-
if strings.HasSuffix(uinfo.ObjPath, "/") {
290-
return true, nil
291-
}
292-
293-
q := &storage.Query{Prefix: uinfo.ObjPath + "/"}
294-
matches, err := n.listObjects(ctx, bucket, q)
295-
if err != nil {
296-
return false, err
329+
func (nb nativeBucketHandler) UploadObject(ctx context.Context, objName string, content *os.File) error {
330+
wc := nb.bucket.Object(objName).NewWriter(ctx)
331+
if _, err := io.Copy(wc, content); err != nil {
332+
return fmt.Errorf("error copying file to GCS: %w", err)
297333
}
298-
299-
if len(matches) > 0 {
300-
return true, nil
334+
if err := wc.Close(); err != nil {
335+
return fmt.Errorf("error closing GCS writer: %w", err)
301336
}
337+
return nil
338+
}
302339

303-
return false, nil
340+
func (nb nativeBucketHandler) Close() {
341+
nb.storageClient.Close()
304342
}

0 commit comments

Comments
 (0)