From da42594bbe0006d580331b0222f35a46043a58de Mon Sep 17 00:00:00 2001 From: Renzo Rojas Date: Thu, 12 Sep 2024 11:04:39 -0400 Subject: [PATCH] feat: new gcs client using cloud client libraries (#9518) * feat: new gcs client using cloud client libraries * feat: tests for new native client and changes in other places to make it work with new implementation --- integration/remote_config_dependency_test.go | 102 ++++++ pkg/skaffold/deploy/kubectl/kubectl_test.go | 12 +- pkg/skaffold/gcs/client/native.go | 356 ++++++++++++++++++ pkg/skaffold/gcs/client/native_test.go | 365 +++++++++++++++++++ pkg/skaffold/gcs/gsutil.go | 15 +- pkg/skaffold/gcs/gsutil_test.go | 20 +- pkg/skaffold/kubernetes/manifest/gcs.go | 15 +- pkg/skaffold/kubernetes/manifest/util.go | 6 +- 8 files changed, 874 insertions(+), 17 deletions(-) create mode 100644 pkg/skaffold/gcs/client/native.go create mode 100644 pkg/skaffold/gcs/client/native_test.go diff --git a/integration/remote_config_dependency_test.go b/integration/remote_config_dependency_test.go index 706c3b81929..412ff643be4 100644 --- a/integration/remote_config_dependency_test.go +++ b/integration/remote_config_dependency_test.go @@ -120,3 +120,105 @@ requires: }) } } + +func TestRenderWithRemoteGCS(t *testing.T) { + tests := []struct { + description string + configFile string + args []string + shouldErr bool + expectedOutput string + expectedErrMsg string + }{ + { + description: "download all repo with same folders from subfolder", + configFile: `apiVersion: skaffold/v4beta11 +kind: Config +requires: + - googleCloudStorage: + source: gs://skaffold-remote-dependency-e2e-tests/test1/* + path: ./skaffold.yaml +`, + args: []string{"--tag", "fixed", "--default-repo=", "--digest-source", "tag"}, + expectedOutput: `apiVersion: v1 +kind: Pod +metadata: + name: getting-started +spec: + containers: + - image: skaffold-example:fixed + name: getting-started`, + }, + { + description: "download full repo with top sub folder", + configFile: `apiVersion: skaffold/v4beta11 +kind: Config +requires: + - googleCloudStorage: + source: gs://skaffold-remote-dependency-e2e-tests/test1 + path: ./test1/skaffold.yaml +`, + args: []string{"--tag", "fixed", "--default-repo=", "--digest-source", "tag"}, + expectedOutput: `apiVersion: v1 +kind: Pod +metadata: + name: getting-started +spec: + containers: + - image: skaffold-example:fixed + name: getting-started`, + }, + { + description: "download full repo with bucket name as top folder", + configFile: `apiVersion: skaffold/v4beta11 +kind: Config +requires: + - googleCloudStorage: + source: gs://skaffold-remote-dependency-e2e-tests + path: ./skaffold-remote-dependency-e2e-tests/test1/skaffold.yaml +`, + args: []string{"--tag", "fixed", "--default-repo=", "--digest-source", "tag"}, + expectedOutput: `apiVersion: v1 +kind: Pod +metadata: + name: getting-started +spec: + containers: + - image: skaffold-example:fixed + name: getting-started`, + }, + { + description: "download only all yaml files across bucket", + configFile: `apiVersion: skaffold/v4beta11 +kind: Config +requires: + - googleCloudStorage: + source: gs://skaffold-remote-dependency-e2e-tests/test1/**.yaml + path: ./skaffold.yaml +`, + args: []string{"--tag", "fixed", "--default-repo=", "--digest-source", "tag", "-p", "flat-structure"}, + expectedOutput: `apiVersion: v1 +kind: Pod +metadata: + name: getting-started +spec: + containers: + - image: skaffold-example:fixed + name: getting-started`, + }, + } + + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + MarkIntegrationTest(t.T, NeedsGcp) + tmpDirRemoteRepo := t.NewTempDir() + tmpDirTest := t.NewTempDir() + + tmpDirTest.Write("skaffold.yaml", test.configFile) + args := append(test.args, "--remote-cache-dir", tmpDirRemoteRepo.Root()) + output, err := skaffold.Render(args...).InDir(tmpDirTest.Root()).RunWithCombinedOutput(t.T) + t.CheckNoError(err) + t.CheckDeepEqual(test.expectedOutput, string(output), testutil.YamlObj(t.T)) + }) + } +} diff --git a/pkg/skaffold/deploy/kubectl/kubectl_test.go b/pkg/skaffold/deploy/kubectl/kubectl_test.go index f638bb91bb2..5863ad340a0 100644 --- a/pkg/skaffold/deploy/kubectl/kubectl_test.go +++ b/pkg/skaffold/deploy/kubectl/kubectl_test.go @@ -23,7 +23,6 @@ import ( "fmt" "io" "os" - "path/filepath" "testing" "time" @@ -40,6 +39,11 @@ import ( "github.com/GoogleContainerTools/skaffold/v2/testutil" ) +type gcsClientMock struct{} + +func (g gcsClientMock) DownloadRecursive(ctx context.Context, src, dst string) error { + return nil +} func TestKubectlV1RenderDeploy(t *testing.T) { tests := []struct { description string @@ -520,13 +524,15 @@ func TestGCSManifests(t *testing.T) { RawK8s: []string{"gs://dev/deployment.yaml"}, }, commands: testutil. - CmdRunOut(fmt.Sprintf("gsutil cp -r %s %s", "gs://dev/deployment.yaml", filepath.Join(manifest.ManifestTmpDir, manifest.ManifestsFromGCS)), "log"). - AndRun("kubectl --context kubecontext --namespace testNamespace apply -f -"), + CmdRun("kubectl --context kubecontext --namespace testNamespace apply -f -"), }} for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { t.Override(&client.Client, deployutil.MockK8sClient) t.Override(&util.DefaultExecCommand, test.commands) + t.Override(&manifest.GetGCSClient, func() manifest.GCSClient { + return gcsClientMock{} + }) if err := os.MkdirAll(manifest.ManifestTmpDir, os.ModePerm); err != nil { t.Fatal(err) } diff --git a/pkg/skaffold/gcs/client/native.go b/pkg/skaffold/gcs/client/native.go new file mode 100644 index 00000000000..f5f5418e6f2 --- /dev/null +++ b/pkg/skaffold/gcs/client/native.go @@ -0,0 +1,356 @@ +/* +Copyright 2024 The Skaffold 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 client + +import ( + "context" + "errors" + "fmt" + "io" + "net/url" + "os" + "path/filepath" + "regexp" + "strings" + + "cloud.google.com/go/storage" + "google.golang.org/api/iterator" +) + +var GetBucketManager = getBucketManager + +// bucketHandler defines the available interactions with a GCS bucket. +type bucketHandler interface { + // ListObjects lists the objects that match the given query. + ListObjects(ctx context.Context, q *storage.Query) ([]string, error) + // DownloadObject downloads the object with the given uri in the localPath. + DownloadObject(ctx context.Context, localPath, uri string) error + // UploadObject creates a files with the given content with the objName. + UploadObject(ctx context.Context, objName string, content *os.File) error + // Close closes the bucket handler connection. + Close() +} + +// uriInfo contains information about the GCS object URI. +type uriInfo struct { + // Bucket is the name of the GCS bucket. + Bucket string + + // ObjPath is the path, with or without wildcards, of the specified object(s) in the GCS bucket. + ObjPath string +} + +func (o uriInfo) Full() string { + return o.Bucket + "/" + o.ObjPath +} + +type Native struct{} + +// Downloads the content that match the given src uri and subfolders. +func (n *Native) DownloadRecursive(ctx context.Context, src, dst string) error { + uriInfo, err := n.parseGCSURI(src) + if err != nil { + return err + } + + bucket, err := GetBucketManager(ctx, uriInfo.Bucket) + if err != nil { + return err + } + defer bucket.Close() + + files, err := n.filesToDownload(ctx, bucket, uriInfo) + if err != nil { + return err + } + + for uri, localPath := range files { + fullPath := filepath.Join(dst, localPath) + dir := filepath.Dir(fullPath) + if _, err := os.Stat(dir); os.IsNotExist(err) { + if err := os.MkdirAll(dir, os.ModePerm); err != nil { + return fmt.Errorf("failed to create directory: %v", err) + } + } + + if err := bucket.DownloadObject(ctx, fullPath, uri); err != nil { + return err + } + } + + return nil +} + +// Uploads a single file to the given dst. +func (n *Native) UploadFile(ctx context.Context, src, dst string) error { + f, err := os.Open(src) + if err != nil { + return fmt.Errorf("error opening file: %w", err) + } + defer f.Close() + + urinfo, err := n.parseGCSURI(dst) + if err != nil { + return err + } + + bucket, err := GetBucketManager(ctx, urinfo.Bucket) + if err != nil { + return err + } + + isDirectory, err := n.isGCSDirectory(ctx, bucket, urinfo) + if err != nil { + return err + } + + dstObj := urinfo.ObjPath + if isDirectory { + dstObj, err = url.JoinPath(dstObj, filepath.Base(src)) + if err != nil { + return err + } + } + + return bucket.UploadObject(ctx, dstObj, f) +} + +func (n *Native) parseGCSURI(uri string) (uriInfo, error) { + var gcsobj uriInfo + u, err := url.Parse(uri) + if err != nil { + return uriInfo{}, fmt.Errorf("cannot parse URI %q: %w", uri, err) + } + if u.Scheme != "gs" { + return uriInfo{}, fmt.Errorf("URI scheme is %q, must be 'gs'", u.Scheme) + } + if u.Host == "" { + return uriInfo{}, errors.New("bucket name is empty") + } + gcsobj.Bucket = u.Host + // If we do this with the url package it will scape the `?` character, breaking the glob. + gcsobj.ObjPath = strings.TrimLeft(strings.ReplaceAll(uri, "gs://"+u.Host, ""), "/") + + return gcsobj, nil +} + +func (n *Native) filesToDownload(ctx context.Context, bucket bucketHandler, urinfo uriInfo) (map[string]string, error) { + uriToLocalPath := map[string]string{} + + // The exact match is with the original glob expression. This could be: + // 1. a/b/c -> It will return the file `c` under a/b/ if it exists + // 2. a/b/c* -> It will return any file under a/b/ that starts with c, e.g, c1, c-other, etc + // 3. a/b/c** -> It will return any file that starts with 'c', and files inside any folder starting with 'c'. It is doing the recursion already + exactMatches, err := bucket.ListObjects(ctx, &storage.Query{MatchGlob: urinfo.ObjPath}) + if err != nil { + return nil, err + } + + for _, match := range exactMatches { + uriToLocalPath[match] = filepath.Base(match) + } + + // Then, to mimic gsutil behavior, we assume the last part of the glob is a folder, so we complete the + // URI with the necessary wildcard to list the folder recursively. + recursiveMatches, err := n.recursiveListing(ctx, bucket, urinfo) + if err != nil { + return nil, err + } + + for uri, match := range recursiveMatches { + uriToLocalPath[uri] = match + } + + return uriToLocalPath, nil +} + +func (n *Native) recursiveListing(ctx context.Context, bucket bucketHandler, urinfo uriInfo) (map[string]string, error) { + uriToLocalPath := map[string]string{} + recursiveURI := n.uriForRecursiveSearch(urinfo.ObjPath) + recursiveMatches, err := bucket.ListObjects(ctx, &storage.Query{MatchGlob: recursiveURI}) + if err != nil { + return nil, err + } + + prefixRemovalURI := n.uriForPrefixRemoval(urinfo.Full()) + prefixRemovalRegex, err := n.wildcardToRegex(prefixRemovalURI) + if err != nil { + return nil, err + } + + // For glob patterns that have `**` (anywhere), gsutil doesn't recreate the folder structure. + shouldRecreateFolders := !strings.Contains(urinfo.ObjPath, "**") + for _, match := range recursiveMatches { + destPath := filepath.Base(match) + if shouldRecreateFolders { + matchWithBucket := urinfo.Bucket + "/" + match + destPath = string(prefixRemovalRegex.ReplaceAll([]byte(matchWithBucket), []byte(""))) + } + uriToLocalPath[match] = destPath + } + + return uriToLocalPath, nil +} + +// uriForRecursiveSearch returns a modified URI is to cover globs like a/*/d*, to remove its prefix: +// For the case where the bucket has the following files: +// - a/b/d/sub1/file1 +// - a/c/d/sub2/sub3/file2 +// - a/e/d2/file2 +// The resulting files + folders should be: +// - d/sub1/file1 +// - d/sub2/sub3/file2 +// - d2/file2 +func (n *Native) uriForRecursiveSearch(uri string) string { + // when we want to list all the bucket + if uri == "" { + return "**" + } + // uri is a/b** or a/b/** + if strings.HasSuffix(uri, "**") { + return uri + } + // a/b* and a/b/* become a/b** and a/b/** + if strings.HasSuffix(uri, "*") { + return uri + "*" + } + // a/b/ becomes a/b/** + if strings.HasSuffix(uri, "/") { + return uri + "**" + } + // a/b becomes a/b/** + return uri + "/**" +} + +func (n *Native) uriForPrefixRemoval(uri string) string { + if strings.HasSuffix(uri, "/*") { + return strings.TrimSuffix(uri, "*") + } + uri = strings.TrimSuffix(uri, "/") + idx := strings.LastIndex(uri, "/") + return uri[:idx+1] +} + +func (n *Native) wildcardToRegex(wildcard string) (*regexp.Regexp, error) { + // Escape special regex characters that might be present in the wildcard + escaped := regexp.QuoteMeta(wildcard) + + escaped = strings.ReplaceAll(escaped, "\\*", "[^/]*") + escaped = strings.ReplaceAll(escaped, "\\?", "[^/]") // Match any single character except '/' + escaped = strings.ReplaceAll(escaped, "\\[", "[") + escaped = strings.ReplaceAll(escaped, "\\]", "]") + regexStr := "^" + escaped + + return regexp.Compile(regexStr) +} + +func (n *Native) isGCSDirectory(ctx context.Context, bucket bucketHandler, urinfo uriInfo) (bool, error) { + if urinfo.ObjPath == "" { + return true, nil + } + + if strings.HasSuffix(urinfo.ObjPath, "/") { + return true, nil + } + + q := &storage.Query{Prefix: urinfo.ObjPath + "/"} + // GCS doesn't support empty "folders". + matches, err := bucket.ListObjects(ctx, q) + if err != nil { + return false, err + } + + if len(matches) > 0 { + return true, nil + } + + return false, nil +} + +func getBucketManager(ctx context.Context, bucketName string) (bucketHandler, error) { + sc, err := storage.NewClient(ctx) + if err != nil { + return nil, fmt.Errorf("error creating GCS Client: %w", err) + } + + return nativeBucketHandler{ + storageClient: sc, + bucket: sc.Bucket(bucketName), + }, nil +} + +// nativeBucketHandler implements a handler using the Cloud client libraries. +type nativeBucketHandler struct { + storageClient *storage.Client + bucket *storage.BucketHandle +} + +func (nb nativeBucketHandler) ListObjects(ctx context.Context, q *storage.Query) ([]string, error) { + matches := []string{} + it := nb.bucket.Objects(ctx, q) + + for { + attrs, err := it.Next() + if err == iterator.Done { + break + } + + if err != nil { + return nil, fmt.Errorf("failed to iterate objects: %v", err) + } + + if attrs.Name != "" { + matches = append(matches, attrs.Name) + } + } + return matches, nil +} + +func (nb nativeBucketHandler) DownloadObject(ctx context.Context, localPath, uri string) error { + reader, err := nb.bucket.Object(uri).NewReader(ctx) + if err != nil { + return fmt.Errorf("failed to read object: %v", err) + } + defer reader.Close() + + file, err := os.Create(localPath) + if err != nil { + return fmt.Errorf("failed to create file: %v", err) + } + defer file.Close() + + if _, err := io.Copy(file, reader); err != nil { + return fmt.Errorf("failed to copy object to file: %v", err) + } + + return nil +} + +func (nb nativeBucketHandler) UploadObject(ctx context.Context, objName string, content *os.File) error { + wc := nb.bucket.Object(objName).NewWriter(ctx) + if _, err := io.Copy(wc, content); err != nil { + return fmt.Errorf("error copying file to GCS: %w", err) + } + if err := wc.Close(); err != nil { + return fmt.Errorf("error closing GCS writer: %w", err) + } + return nil +} + +func (nb nativeBucketHandler) Close() { + nb.storageClient.Close() +} diff --git a/pkg/skaffold/gcs/client/native_test.go b/pkg/skaffold/gcs/client/native_test.go new file mode 100644 index 00000000000..25a9a20f788 --- /dev/null +++ b/pkg/skaffold/gcs/client/native_test.go @@ -0,0 +1,365 @@ +/* +Copyright 2024 The Skaffold 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 client + +import ( + "context" + "os" + "path/filepath" + "regexp" + "strings" + "testing" + + "cloud.google.com/go/storage" + "github.com/bmatcuk/doublestar" + + "github.com/GoogleContainerTools/skaffold/v2/testutil" +) + +// Regex to transform a/b** to a/b*/** +var escapeDoubleStarWithoutSlashRegex = regexp.MustCompile(`([^/])\*\*`) + +type repoHandlerMock struct { + root string + downloadedFiles map[string]string + uploadedFile string +} + +func (r *repoHandlerMock) filterOnlyFiles(paths []string) ([]string, error) { + matches := []string{} + for _, m := range paths { + fileInfo, err := os.Stat(m) + if err != nil { + return nil, err + } + + if !fileInfo.IsDir() { + withoutPrefix := strings.TrimPrefix(m, r.root+string(filepath.Separator)) + matches = append(matches, filepath.ToSlash(withoutPrefix)) + } + } + return matches, nil +} + +func (r *repoHandlerMock) matchGlob(matchGlob string) ([]string, error) { + glob := escapeDoubleStarWithoutSlashRegex.ReplaceAllString(matchGlob, `${1}*/**`) + glob = filepath.Join(r.root, glob) + globMatches, err := doublestar.Glob(glob) + if err != nil { + return nil, err + } + + return r.filterOnlyFiles(globMatches) +} + +func (r *repoHandlerMock) withPrefix(prefix string) ([]string, error) { + path := filepath.Join(r.root, prefix+"**") + matches, err := doublestar.Glob(path) + if err != nil { + return nil, err + } + + return r.filterOnlyFiles(matches) +} + +func (r *repoHandlerMock) ListObjects(ctx context.Context, q *storage.Query) ([]string, error) { + if q.MatchGlob != "" { + return r.matchGlob(q.MatchGlob) + } + + if q.Prefix != "" { + return r.withPrefix(q.Prefix) + } + + return nil, nil +} + +func (r *repoHandlerMock) DownloadObject(ctx context.Context, localPath, uri string) error { + r.downloadedFiles[uri] = localPath + return nil +} + +func (r *repoHandlerMock) UploadObject(ctx context.Context, objName string, content *os.File) error { + r.uploadedFile = objName + return nil +} + +func (r *repoHandlerMock) Close() {} + +func TestDownloadRecursive(t *testing.T) { + tests := []struct { + name string + uri string + dst string + availableFiles []string + expectedDownloadedFiles map[string]string + }{ + { + name: "exact match with flat output", + uri: "gs://bucket/dir1/manifest1.yaml", + dst: "download", + availableFiles: []string{ + "dir1/manifest1.yaml", + "manifest2.yaml", + "main.go", + }, + expectedDownloadedFiles: map[string]string{ + "dir1/manifest1.yaml": "download/manifest1.yaml", + }, + }, + { + name: "exact match with wildcard, flat output", + uri: "gs://bucket/*/manifest[12].yaml", + dst: "download", + availableFiles: []string{ + "dir1/manifest1.yaml", + "dir1/manifest3.yaml", + "dir2/manifest2.yaml", + }, + expectedDownloadedFiles: map[string]string{ + "dir1/manifest1.yaml": "download/manifest1.yaml", + "dir2/manifest2.yaml": "download/manifest2.yaml", + }, + }, + { + name: "exact match with ? wildcard with flat output", + uri: "gs://bucket/*/manifest?.yaml", + dst: "download", + availableFiles: []string{ + "dir1/manifest1.yaml", + "dir2/manifest2.yaml", + }, + expectedDownloadedFiles: map[string]string{ + "dir1/manifest1.yaml": "download/manifest1.yaml", + "dir2/manifest2.yaml": "download/manifest2.yaml", + }, + }, + { + name: "recursive match with folders creation", + uri: "gs://bucket/dir*", + dst: "download", + availableFiles: []string{ + "dir1/manifest1.yaml", + "dir2/manifest2.yaml", + }, + expectedDownloadedFiles: map[string]string{ + "dir1/manifest1.yaml": "download/dir1/manifest1.yaml", + "dir2/manifest2.yaml": "download/dir2/manifest2.yaml", + }, + }, + { + name: "recursive match with flat output", + uri: "gs://bucket/dir**", + dst: "download", + availableFiles: []string{ + "dir1/manifest1.yaml", + "dir2/manifest2.yaml", + }, + expectedDownloadedFiles: map[string]string{ + "dir1/manifest1.yaml": "download/manifest1.yaml", + "dir2/manifest2.yaml": "download/manifest2.yaml", + }, + }, + { + name: "recursive match from bucket with folders creation", + uri: "gs://bucket", + dst: "download", + availableFiles: []string{ + "dir1/manifest1.yaml", + "dir2/manifest2.yaml", + }, + expectedDownloadedFiles: map[string]string{ + "dir1/manifest1.yaml": "download/bucket/dir1/manifest1.yaml", + "dir2/manifest2.yaml": "download/bucket/dir2/manifest2.yaml", + }, + }, + { + name: "recursive match all bucket content with folders creation", + uri: "gs://bucket/*", + dst: "download", + availableFiles: []string{ + "dir1/manifest1.yaml", + "dir1/sub1/main.go", + "dir2/manifest2.yaml", + }, + expectedDownloadedFiles: map[string]string{ + "dir1/manifest1.yaml": "download/dir1/manifest1.yaml", + "dir1/sub1/main.go": "download/dir1/sub1/main.go", + "dir2/manifest2.yaml": "download/dir2/manifest2.yaml", + }, + }, + { + name: "recursive match all bucket content with flat structure", + uri: "gs://bucket/**", + dst: "download", + availableFiles: []string{ + "dir1/manifest1.yaml", + "dir1/sub1/main.go", + "manifest2.yaml", + }, + expectedDownloadedFiles: map[string]string{ + "dir1/manifest1.yaml": "download/manifest1.yaml", + "dir1/sub1/main.go": "download/main.go", + "manifest2.yaml": "download/manifest2.yaml", + }, + }, + { + name: "recursive match with folder creating and prefix removal", + uri: "gs://bucket/submodule/*/content/*", + dst: "download", + availableFiles: []string{ + "submodule/a/content/dir1/manifest1.yaml", + "submodule/b/content/dir2/manifest2.yaml", + "submodule/c/content/dir3/Dockerfile", + "submodule/dir4/main.go", + }, + expectedDownloadedFiles: map[string]string{ + "submodule/a/content/dir1/manifest1.yaml": "download/dir1/manifest1.yaml", + "submodule/b/content/dir2/manifest2.yaml": "download/dir2/manifest2.yaml", + "submodule/c/content/dir3/Dockerfile": "download/dir3/Dockerfile", + }, + }, + { + name: "recursive match with matching folder creating and prefix removal", + uri: "gs://bucket/submodule/*/content*", + dst: "download", + availableFiles: []string{ + "submodule/a/content1/dir1/manifest1.yaml", + "submodule/b/content2/dir2/manifest2.yaml", + }, + expectedDownloadedFiles: map[string]string{ + "submodule/a/content1/dir1/manifest1.yaml": "download/content1/dir1/manifest1.yaml", + "submodule/b/content2/dir2/manifest2.yaml": "download/content2/dir2/manifest2.yaml", + }, + }, + { + name: "no match", + uri: "gs://bucket/**/*.go", + dst: "download", + availableFiles: []string{ + "submodule/a/content/dir1/manifest1.yaml", + "submodule/b/content/dir2/manifest2.yaml", + "submodule/c/content/dir3/Dockerfile", + }, + expectedDownloadedFiles: map[string]string{}, + }, + } + + for _, test := range tests { + testutil.Run(t, test.name, func(t *testutil.T) { + td := t.NewTempDir() + td.Touch(test.availableFiles...) + root := td.Root() + + rh := &repoHandlerMock{ + root: root, + downloadedFiles: make(map[string]string), + } + t.Override(&GetBucketManager, func(ctx context.Context, bucketName string) (bucketHandler, error) { + return rh, nil + }) + + n := Native{} + err := n.DownloadRecursive(context.TODO(), test.uri, test.dst) + t.CheckNoError(err) + + for uri, local := range test.expectedDownloadedFiles { + test.expectedDownloadedFiles[uri] = filepath.FromSlash(local) + } + + t.CheckMapsMatch(test.expectedDownloadedFiles, rh.downloadedFiles) + }) + } +} + +func TestUploadFile(t *testing.T) { + tests := []struct { + name string + uri string + localFile string + availableFiles []string + expectedCreatedFile string + shouldError bool + }{ + { + name: "upload file to existing folder using local name", + uri: "gs://bucket/folder", + localFile: "manifest.yaml", + availableFiles: []string{ + "folder/main.go", + }, + expectedCreatedFile: "folder/manifest.yaml", + }, + { + name: "upload file to existing folder using new name", + uri: "gs://bucket/folder/newmanifest.yaml", + localFile: "manifest.yaml", + availableFiles: []string{ + "folder/main.go", + }, + expectedCreatedFile: "folder/newmanifest.yaml", + }, + { + name: "upload file to not existing subfolder using local name", + uri: "gs://bucket/folder/newfolder/", + localFile: "manifest.yaml", + availableFiles: []string{ + "folder/main.go", + }, + expectedCreatedFile: "folder/newfolder/manifest.yaml", + }, + { + name: "upload file to not existing subfolder using new name", + uri: "gs://bucket/folder/newfolder/newmanifest.yaml", + localFile: "manifest.yaml", + availableFiles: []string{ + "folder/main.go", + }, + expectedCreatedFile: "folder/newfolder/newmanifest.yaml", + }, + { + name: "upload file to root of bucket", + uri: "gs://bucket", + localFile: "manifest.yaml", + expectedCreatedFile: "manifest.yaml", + }, + } + + for _, test := range tests { + testutil.Run(t, test.name, func(t *testutil.T) { + bucketTd := t.NewTempDir() + bucketTd.Touch(test.availableFiles...) + bucketRoot := bucketTd.Root() + rh := &repoHandlerMock{ + root: bucketRoot, + downloadedFiles: make(map[string]string), + } + t.Override(&GetBucketManager, func(ctx context.Context, bucketName string) (bucketHandler, error) { + return rh, nil + }) + + localFileTd := t.NewTempDir() + localFileTd.Touch(test.localFile) + locaFullPath := filepath.Join(localFileTd.Root(), test.localFile) + + n := Native{} + err := n.UploadFile(context.TODO(), locaFullPath, test.uri) + t.CheckNoError(err) + t.CheckDeepEqual(test.expectedCreatedFile, rh.uploadedFile) + }) + } +} diff --git a/pkg/skaffold/gcs/gsutil.go b/pkg/skaffold/gcs/gsutil.go index 722f2da0dfd..ab1a2ac181c 100644 --- a/pkg/skaffold/gcs/gsutil.go +++ b/pkg/skaffold/gcs/gsutil.go @@ -28,6 +28,7 @@ import ( "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/config" sErrors "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/errors" + "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/gcs/client" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/output/log" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/util" @@ -54,6 +55,16 @@ func (g *Gsutil) Copy(ctx context.Context, src, dst string, recursive bool) erro return nil } +// GetGCSClient returns a GCS client that uses Client libraries. +var GetGCSClient = func() gscClient { + return &client.Native{} +} + +type gscClient interface { + // Downloads the content that match the given src uri and subfolders. + DownloadRecursive(ctx context.Context, src, dst string) error +} + // SyncObjects syncs the target Google Cloud Storage objects with skaffold's local cache and returns the local path to the objects. func SyncObjects(ctx context.Context, g latest.GoogleCloudStorageInfo, opts config.SkaffoldOptions) (string, error) { remoteCacheDir, err := config.GetRemoteCacheDir(opts) @@ -89,8 +100,8 @@ func SyncObjects(ctx context.Context, g latest.GoogleCloudStorageInfo, opts conf } } - gcs := Gsutil{} - if err := gcs.Copy(ctx, g.Source, cacheDir, true); err != nil { + gcs := GetGCSClient() + if err := gcs.DownloadRecursive(ctx, g.Source, cacheDir); err != nil { return "", fmt.Errorf("failed to cache Google Cloud Storage objects from %q: %w", g.Source, err) } return cacheDir, nil diff --git a/pkg/skaffold/gcs/gsutil_test.go b/pkg/skaffold/gcs/gsutil_test.go index 87651bf4b9b..6cd46d547fb 100644 --- a/pkg/skaffold/gcs/gsutil_test.go +++ b/pkg/skaffold/gcs/gsutil_test.go @@ -77,6 +77,14 @@ func TestCopy(t *testing.T) { } } +type gcsClientMock struct { + err error +} + +func (g gcsClientMock) DownloadRecursive(ctx context.Context, src, dst string) error { + return g.err +} + func TestSyncObject(t *testing.T) { source := "gs://my-bucket/dir1/*" path := "configs/skaffold.yaml" @@ -144,13 +152,13 @@ func TestSyncObject(t *testing.T) { _ = syncRemote.Set(test.syncFlag) opts := config.SkaffoldOptions{RemoteCacheDir: td.Root(), SyncRemoteCache: *syncRemote} - var cmd *testutil.FakeCmd - if test.gsutilErr == nil { - cmd = testutil.CmdRunOut(fmt.Sprintf("gsutil cp -r %s %s", source, td.Path(sourceHash)), "logs") - } else { - cmd = testutil.CmdRunOutErr(fmt.Sprintf("gsutil cp -r %s %s", source, td.Path(sourceHash)), "logs", test.gsutilErr) + gcsClient := gcsClientMock{} + if test.gsutilErr != nil { + gcsClient.err = test.gsutilErr } - t.Override(&util.DefaultExecCommand, cmd) + t.Override(&GetGCSClient, func() gscClient { + return gcsClient + }) path, err := SyncObjects(context.Background(), test.g, opts) var expected string diff --git a/pkg/skaffold/kubernetes/manifest/gcs.go b/pkg/skaffold/kubernetes/manifest/gcs.go index 565afa94481..44faa319074 100644 --- a/pkg/skaffold/kubernetes/manifest/gcs.go +++ b/pkg/skaffold/kubernetes/manifest/gcs.go @@ -23,11 +23,20 @@ import ( "path/filepath" "strings" - "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/gcs" + "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/gcs/client" ) var ManifestsFromGCS = "manifests_from_gcs" +type GCSClient interface { + // Downloads the content that match the given src uri and subfolders. + DownloadRecursive(ctx context.Context, src, dst string) error +} + +var GetGCSClient = func() GCSClient { + return &client.Native{} +} + // DownloadFromGCS downloads all provided manifests from a remote GCS bucket, // and returns a relative path pointing to the GCS temp dir. func DownloadFromGCS(manifests []string) (string, error) { @@ -39,8 +48,8 @@ func DownloadFromGCS(manifests []string) (string, error) { if manifest == "" || !strings.HasPrefix(manifest, gcsPrefix) { return "", fmt.Errorf("%v is not a valid GCS path", manifest) } - gcs := gcs.Gsutil{} - if err := gcs.Copy(context.Background(), manifest, dir, true); err != nil { + gcs := GetGCSClient() + if err := gcs.DownloadRecursive(context.Background(), manifest, dir); err != nil { return "", fmt.Errorf("failed to download manifests fom GCS: %w", err) } } diff --git a/pkg/skaffold/kubernetes/manifest/util.go b/pkg/skaffold/kubernetes/manifest/util.go index 77e4ec41d68..6fb56d6888e 100644 --- a/pkg/skaffold/kubernetes/manifest/util.go +++ b/pkg/skaffold/kubernetes/manifest/util.go @@ -24,7 +24,7 @@ import ( "path/filepath" "strings" - "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/gcs" + "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/gcs/client" ) const ( @@ -51,8 +51,8 @@ func Write(manifests string, output string, manifestOut io.Writer) error { if err := dumpToFile(manifests, tempFile); err != nil { return err } - gcs := gcs.Gsutil{} - if err := gcs.Copy(context.Background(), tempFile, output, false); err != nil { + gcs := client.Native{} + if err := gcs.UploadFile(context.Background(), tempFile, output); err != nil { return writeErr(fmt.Errorf("failed to copy rendered manifests to GCS: %w", err)) } return nil