diff --git a/integration/deploy_test.go b/integration/deploy_test.go index 07bce976ce4..c0b4dcbee56 100644 --- a/integration/deploy_test.go +++ b/integration/deploy_test.go @@ -17,11 +17,17 @@ limitations under the License. package integration import ( + "errors" + "io" + "os" + "path/filepath" "strings" "testing" "github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/flags" "github.com/GoogleContainerTools/skaffold/integration/skaffold" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/walk" "github.com/GoogleContainerTools/skaffold/testutil" ) @@ -105,3 +111,97 @@ func TestDeployWithInCorrectConfig(t *testing.T) { t.Errorf("failed without saying the reason: %s", output) } } + +// Verify that we can deploy without artifact details (https://github.com/GoogleContainerTools/skaffold/issues/4616) +func TestDeployWithoutWorkspaces(t *testing.T) { + MarkIntegrationTest(t, NeedsGcp) + + ns, _ := SetupNamespace(t) + + outputBytes := skaffold.Build("--quiet").InDir("examples/nodejs").InNs(ns.Name).RunOrFailOutput(t) + // Parse the Build Output + buildArtifacts, err := flags.ParseBuildOutput(outputBytes) + failNowIfError(t, err) + if len(buildArtifacts.Builds) != 1 { + t.Fatalf("expected 1 artifact to be built, but found %d", len(buildArtifacts.Builds)) + } + + tmpDir := testutil.NewTempDir(t) + buildOutputFile := tmpDir.Path("build.out") + tmpDir.Write("build.out", string(outputBytes)) + copyFiles(tmpDir.Root(), "examples/nodejs/skaffold.yaml") + copyFiles(tmpDir.Root(), "examples/nodejs/k8s") + + // Run Deploy using the build output + // See https://github.com/GoogleContainerTools/skaffold/issues/2372 on why status-check=false + skaffold.Deploy("--build-artifacts", buildOutputFile, "--status-check=false").InDir(tmpDir.Root()).InNs(ns.Name).RunOrFail(t) +} + +// Copies a file or directory tree. There are 2x3 cases: +// 1. If _src_ is a file, +// 1. and _dst_ exists and is a file then _src_ is copied into _dst_ +// 2. and _dst_ exists and is a directory, then _src_ is copied as _dst/$(basename src)_ +// 3. and _dst_ does not exist, then _src_ is copied as _dst_. +// 2. If _src_ is a directory, +// 1. and _dst_ exists and is a file, then return an error +// 2. and _dst_ exists and is a directory, then src is copied as _dst/$(basename src)_ +// 3. and _dst_ does not exist, then src is copied as _dst/src[1:]_. +func copyFiles(dst, src string) error { + if util.IsFile(src) { + switch { + case util.IsFile(dst): // copy _src_ to _dst_ + case util.IsDir(dst): // copy _src_ to _dst/src[-1] + dst = filepath.Join(dst, filepath.Base(src)) + default: // copy _src_ to _dst_ + if err := os.MkdirAll(filepath.Dir(dst), os.ModePerm); err != nil { + return err + } + } + in, err := os.Open(src) + if err != nil { + return err + } + out, err := os.Create(dst) + if err != nil { + return err + } + _, err = io.Copy(out, in) + return err + } else if !util.IsDir(src) { + return errors.New("src does not exist") + } + // so src is a directory + if util.IsFile(dst) { + return errors.New("cannot copy directory into file") + } + srcPrefix := src + if util.IsDir(dst) { // src is copied to _dst/$(basename src) + srcPrefix = filepath.Dir(src) + } else if err := os.MkdirAll(filepath.Dir(dst), os.ModePerm); err != nil { + return err + } + return walk.From(src).Unsorted().WhenIsFile().Do(func(path string, _ walk.Dirent) error { + rel, err := filepath.Rel(srcPrefix, path) + if err != nil { + return err + } + in, err := os.Open(path) + if err != nil { + return err + } + defer in.Close() + + destFile := filepath.Join(dst, rel) + if err := os.MkdirAll(filepath.Dir(destFile), os.ModePerm); err != nil { + return err + } + + out, err := os.Create(destFile) + if err != nil { + return err + } + + _, err = io.Copy(out, in) + return err + }) +} diff --git a/pkg/skaffold/runner/build_deploy.go b/pkg/skaffold/runner/build_deploy.go index 03a9d35bea3..cad5dd672c2 100644 --- a/pkg/skaffold/runner/build_deploy.go +++ b/pkg/skaffold/runner/build_deploy.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "io" + "os" "time" "github.com/sirupsen/logrus" @@ -38,6 +39,10 @@ func (r *SkaffoldRunner) BuildAndTest(ctx context.Context, out io.Writer, artifa return []build.Artifact{}, nil } + if err := checkWorkspaces(artifacts); err != nil { + return nil, err + } + tags, err := r.imageTags(ctx, out, artifacts) if err != nil { return nil, err @@ -201,3 +206,20 @@ func (r *SkaffoldRunner) imageTags(ctx context.Context, out io.Writer, artifacts logrus.Infoln("Tags generated in", time.Since(start)) return imageTags, nil } + +func checkWorkspaces(artifacts []*latest.Artifact) error { + for _, a := range artifacts { + if a.Workspace != "" { + if info, err := os.Stat(a.Workspace); err != nil { + // err could be permission-related + if os.IsNotExist(err) { + return fmt.Errorf("image %q context %q does not exist", a.ImageName, a.Workspace) + } + return fmt.Errorf("image %q context %q: %w", a.ImageName, a.Workspace, err) + } else if !info.IsDir() { + return fmt.Errorf("image %q context %q is not a directory", a.ImageName, a.Workspace) + } + } + } + return nil +} diff --git a/pkg/skaffold/runner/build_deploy_test.go b/pkg/skaffold/runner/build_deploy_test.go index 63c0524c7c0..215bbd8ccfc 100644 --- a/pkg/skaffold/runner/build_deploy_test.go +++ b/pkg/skaffold/runner/build_deploy_test.go @@ -128,3 +128,58 @@ func TestBuildAndTestSkipBuild(t *testing.T) { t.CheckDeepEqual([]Actions{{}}, testBench.Actions()) }) } + +func TestCheckWorkspaces(t *testing.T) { + tmpDir := testutil.NewTempDir(t).Touch("file") + tmpFile := tmpDir.Path("file") + + tests := []struct { + description string + artifacts []*latest.Artifact + shouldErr bool + }{ + { + description: "no workspace", + artifacts: []*latest.Artifact{ + { + ImageName: "image", + }, + }, + }, + { + description: "directory that exists", + artifacts: []*latest.Artifact{ + { + ImageName: "image", + Workspace: tmpDir.Root(), + }, + }, + }, + { + description: "error on non-existent location", + artifacts: []*latest.Artifact{ + { + ImageName: "image", + Workspace: "doesnotexist", + }, + }, + shouldErr: true, + }, + { + description: "error on file", + artifacts: []*latest.Artifact{ + { + ImageName: "image", + Workspace: tmpFile, + }, + }, + shouldErr: true, + }, + } + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + err := checkWorkspaces(test.artifacts) + t.CheckError(test.shouldErr, err) + }) + } +} diff --git a/pkg/skaffold/schema/samples_test.go b/pkg/skaffold/schema/samples_test.go index cd2b0ec7314..f050718da5b 100644 --- a/pkg/skaffold/schema/samples_test.go +++ b/pkg/skaffold/schema/samples_test.go @@ -74,15 +74,7 @@ func TestParseSamples(t *testing.T) { } func checkSkaffoldConfig(t *testutil.T, yaml []byte) { - root := t.NewTempDir() - configFile := root.Path("skaffold.yaml") - root.Write("skaffold.yaml", string(yaml)) - // create workspace directories referenced in these samples - for _, d := range []string{"app", "node", "python", "leeroy-web", "leeroy-app", "backend", "base-service", "world-service"} { - root.Mkdir(d) - } - root.Chdir() - + configFile := t.TempFile("skaffold.yaml", yaml) cfg, err := ParseConfigAndUpgrade(configFile, latest.Version) t.CheckNoError(err) diff --git a/pkg/skaffold/schema/validation/validation.go b/pkg/skaffold/schema/validation/validation.go index e0c84479f11..6c3c1a6448c 100644 --- a/pkg/skaffold/schema/validation/validation.go +++ b/pkg/skaffold/schema/validation/validation.go @@ -18,7 +18,6 @@ package validation import ( "fmt" - "os" "reflect" "strings" @@ -37,7 +36,6 @@ var ( // Process checks if the Skaffold pipeline is valid and returns all encountered errors as a concatenated string func Process(config *latest.SkaffoldConfig) error { errs := visitStructs(config, validateYamltags) - errs = append(errs, validateWorkspaces(config.Build.Artifacts)...) errs = append(errs, validateImageNames(config.Build.Artifacts)...) errs = append(errs, validateDockerNetworkMode(config.Build.Artifacts)...) errs = append(errs, validateCustomDependencies(config.Build.Artifacts)...) @@ -58,25 +56,6 @@ func Process(config *latest.SkaffoldConfig) error { return fmt.Errorf(strings.Join(messages, " | ")) } -// validateWorkspaces makes sure the artifact workspaces are valid directories. -func validateWorkspaces(artifacts []*latest.Artifact) (errs []error) { - for _, a := range artifacts { - if a.Workspace != "" { - if info, err := os.Stat(a.Workspace); err != nil { - // err could be permission-related - if os.IsNotExist(err) { - errs = append(errs, fmt.Errorf("image %q context %q does not exist", a.ImageName, a.Workspace)) - } else { - errs = append(errs, fmt.Errorf("image %q context %q: %w", a.ImageName, a.Workspace, err)) - } - } else if !info.IsDir() { - errs = append(errs, fmt.Errorf("image %q context %q is not a directory", a.ImageName, a.Workspace)) - } - } - } - return -} - // validateImageNames makes sure the artifact image names are valid base names, // without tags nor digests. func validateImageNames(artifacts []*latest.Artifact) (errs []error) { diff --git a/pkg/skaffold/schema/validation/validation_test.go b/pkg/skaffold/schema/validation/validation_test.go index e9af5ff37a4..12cb2380f0b 100644 --- a/pkg/skaffold/schema/validation/validation_test.go +++ b/pkg/skaffold/schema/validation/validation_test.go @@ -717,72 +717,6 @@ func TestValidateJibPluginType(t *testing.T) { } } -func TestValidateWorkspaces(t *testing.T) { - tmpDir := testutil.NewTempDir(t).Touch("file") - tmpFile := tmpDir.Path("file") - - tests := []struct { - description string - artifacts []*latest.Artifact - shouldErr bool - }{ - { - description: "no workspace", - artifacts: []*latest.Artifact{ - { - ImageName: "image", - }, - }, - }, - { - description: "directory that exists", - artifacts: []*latest.Artifact{ - { - ImageName: "image", - Workspace: tmpDir.Root(), - }, - }, - }, - { - description: "error on non-existent location", - artifacts: []*latest.Artifact{ - { - ImageName: "image", - Workspace: "doesnotexist", - }, - }, - shouldErr: true, - }, - { - description: "error on file", - artifacts: []*latest.Artifact{ - { - ImageName: "image", - Workspace: tmpFile, - }, - }, - shouldErr: true, - }, - } - for _, test := range tests { - testutil.Run(t, test.description, func(t *testutil.T) { - // disable yamltags validation - t.Override(&validateYamltags, func(interface{}) error { return nil }) - - err := Process( - &latest.SkaffoldConfig{ - Pipeline: latest.Pipeline{ - Build: latest.BuildConfig{ - Artifacts: test.artifacts, - }, - }, - }) - - t.CheckError(test.shouldErr, err) - }) - } -} - func TestValidateLogsConfig(t *testing.T) { tests := []struct { prefix string