diff --git a/pkg/builders/s2i/builder.go b/pkg/builders/s2i/builder.go index 18c8480594..619bc65390 100644 --- a/pkg/builders/s2i/builder.go +++ b/pkg/builders/s2i/builder.go @@ -1,37 +1,21 @@ package s2i import ( - "archive/tar" "context" - "crypto/sha1" - "encoding/hex" "errors" "fmt" - "io" - "io/fs" - "maps" "net/url" "os" "path/filepath" - "regexp" - "runtime" - "slices" "strings" - "github.com/docker/docker/api/types" dockerClient "github.com/docker/docker/client" - "github.com/docker/docker/pkg/jsonmessage" - "github.com/google/go-containerregistry/pkg/name" - v1 "github.com/google/go-containerregistry/pkg/v1" - "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/openshift/source-to-image/pkg/api" "github.com/openshift/source-to-image/pkg/api/validation" "github.com/openshift/source-to-image/pkg/build" "github.com/openshift/source-to-image/pkg/build/strategies" s2idocker "github.com/openshift/source-to-image/pkg/docker" "github.com/openshift/source-to-image/pkg/scm/git" - "golang.org/x/term" - "knative.dev/func/pkg/builders" "knative.dev/func/pkg/docker" fn "knative.dev/func/pkg/functions" @@ -56,18 +40,12 @@ var DefaultBuilderImages = map[string]string{ "typescript": DefaultNodeBuilder, } -// DockerClient is subset of dockerClient.CommonAPIClient required by this package -type DockerClient interface { - ImageBuild(ctx context.Context, context io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) - ImageInspectWithRaw(ctx context.Context, image string) (types.ImageInspect, []byte, error) -} - // Builder of functions using the s2i subsystem. type Builder struct { name string verbose bool impl build.Builder // S2I builder implementation (aka "Strategy") - cli DockerClient + cli s2idocker.Client } type Option func(*Builder) @@ -94,7 +72,7 @@ func WithImpl(s build.Builder) Option { } } -func WithDockerClient(cli DockerClient) Option { +func WithDockerClient(cli s2idocker.Client) Option { return func(b *Builder) { b.cli = cli } @@ -165,13 +143,6 @@ func (b *Builder) Build(ctx context.Context, f fn.Function, platforms []fn.Platf } } - // Build directory - tmp, err := os.MkdirTemp("", "func-s2i-build") - if err != nil { - return fmt.Errorf("cannot create temporary dir for s2i build: %w", err) - } - defer os.RemoveAll(tmp) - // Build Config cfg := &api.Config{ Source: &git.URL{ @@ -185,7 +156,6 @@ func (b *Builder) Build(ctx context.Context, f fn.Function, platforms []fn.Platf PreviousImagePullPolicy: api.DefaultPreviousImagePullPolicy, RuntimeImagePullPolicy: api.DefaultRuntimeImagePullPolicy, DockerConfig: s2idocker.GetDefaultDockerConfig(), - AsDockerfile: filepath.Join(tmp, "Dockerfile"), } // Scaffold @@ -193,19 +163,6 @@ func (b *Builder) Build(ctx context.Context, f fn.Function, platforms []fn.Platf return } - // Extract a an S2I script url from the image if provided and use - // this in the build config. - scriptURL, err := s2iScriptURL(ctx, client, cfg.BuilderImage) - if err != nil { - return fmt.Errorf("cannot get s2i script url: %w", err) - } else if scriptURL != "image:///usr/libexec/s2i" { - // Only set if the label found on the image is NOT the default. - // Otherwise this label, which is essentially a default fallback, will - // take precidence over any scripts provided in ./.s2i/bin, which are - // supposed to be the override to that default. - cfg.ScriptsURL = scriptURL - } - // Excludes // Do not include .git, .env, .func or any language-specific cache directories // (node_modules, etc) in the tar file sent to the builder, as this both @@ -235,7 +192,7 @@ func (b *Builder) Build(ctx context.Context, f fn.Function, platforms []fn.Platf // Create the S2I builder instance if not overridden var impl = b.impl if impl == nil { - impl, _, err = strategies.Strategy(nil, cfg, build.Overrides{}) + impl, _, err = strategies.Strategy(client, cfg, build.Overrides{}) if err != nil { return fmt.Errorf("cannot create s2i builder: %w", err) } @@ -252,184 +209,7 @@ func (b *Builder) Build(ctx context.Context, f fn.Function, platforms []fn.Platf fmt.Fprintln(os.Stderr, message) } } - - pr, pw := io.Pipe() - - // s2i apparently is not excluding the files in --as-dockerfile mode - exclude := regexp.MustCompile(cfg.ExcludeRegExp) - - // if exists, patch dockerfile to using cache mount - if _, e := os.Stat(cfg.AsDockerfile); e == nil { - err = patchDockerfile(cfg.AsDockerfile, f) - if err != nil { - return err - } - } - - const up = ".." + string(os.PathSeparator) - go func() { - tw := tar.NewWriter(pw) - err := filepath.Walk(tmp, func(path string, fi fs.FileInfo, err error) error { - if err != nil { - return err - } - - p, err := filepath.Rel(tmp, path) - if err != nil { - return fmt.Errorf("cannot get relative path: %w", err) - } - if p == "." { - return nil - } - - p = filepath.ToSlash(p) - - if exclude.MatchString(p) { - return nil - } - - lnk := "" - if fi.Mode()&fs.ModeSymlink != 0 { - lnk, err = os.Readlink(path) - if err != nil { - return fmt.Errorf("cannot read link: %w", err) - } - if filepath.IsAbs(lnk) { - lnk, err = filepath.Rel(tmp, lnk) - if err != nil { - return fmt.Errorf("cannot get relative path for symlink: %w", err) - } - if strings.HasPrefix(lnk, up) || lnk == ".." { - return fmt.Errorf("link %q points outside source root", p) - } - } - } - - hdr, err := tar.FileInfoHeader(fi, filepath.ToSlash(lnk)) - if err != nil { - return fmt.Errorf("cannot create tar header: %w", err) - } - hdr.Name = p - - if runtime.GOOS == "windows" { - // Windows does not have execute permission, we assume that all files are executable. - hdr.Mode |= 0111 - } - - err = tw.WriteHeader(hdr) - if err != nil { - return fmt.Errorf("cannot write header to thar stream: %w", err) - } - if fi.Mode().IsRegular() { - var r io.ReadCloser - r, err = os.Open(path) - if err != nil { - return fmt.Errorf("cannot open source file: %w", err) - } - defer r.Close() - - _, err = io.Copy(tw, r) - if err != nil { - return fmt.Errorf("cannot copy file to tar stream :%w", err) - } - } - - return nil - }) - _ = tw.Close() - _ = pw.CloseWithError(err) - }() - - opts := types.ImageBuildOptions{ - Tags: []string{f.Build.Image}, - PullParent: true, - Version: types.BuilderBuildKit, - } - - resp, err := client.ImageBuild(ctx, pr, opts) - if err != nil { - return fmt.Errorf("cannot build the app image: %w", err) - } - defer resp.Body.Close() - - var out io.Writer = io.Discard - if b.verbose { - out = os.Stderr - } - - var isTerminal bool - var fd uintptr - if outF, ok := out.(*os.File); ok { - fd = outF.Fd() - isTerminal = term.IsTerminal(int(outF.Fd())) - } - - return jsonmessage.DisplayJSONMessagesStream(resp.Body, out, fd, isTerminal, nil) -} - -func patchDockerfile(path string, f fn.Function) error { - data, err := os.ReadFile(path) - if err != nil { - return err - } - re := regexp.MustCompile(`RUN (.*assemble)`) - s := sha1.Sum([]byte(f.Root)) - mountCmd := "--mount=type=cache,target=/tmp/artifacts/,uid=1001,id=" + hex.EncodeToString(s[:8]) - replacement := fmt.Sprintf("RUN %s \\\n $1", mountCmd) - newDockerFileStr := re.ReplaceAllString(string(data), replacement) - - return os.WriteFile(path, []byte(newDockerFileStr), 0644) -} - -func s2iScriptURL(ctx context.Context, cli DockerClient, image string) (string, error) { - img, _, err := cli.ImageInspectWithRaw(ctx, image) - if err != nil { - if dockerClient.IsErrNotFound(err) { // image is not in the daemon, get info directly from registry - var ( - ref name.Reference - img v1.Image - cfg *v1.ConfigFile - ) - - ref, err = name.ParseReference(image) - if err != nil { - return "", fmt.Errorf("cannot parse image name: %w", err) - } - if _, ok := ref.(name.Tag); ok && !slices.Contains(slices.Collect(maps.Values(DefaultBuilderImages)), image) { - fmt.Fprintln(os.Stderr, "image referenced by tag which is discouraged: Tags are mutable and can point to a different artifact than the expected one") - } - img, err = remote.Image(ref) - if err != nil { - return "", fmt.Errorf("cannot get image from registry: %w", err) - } - cfg, err = img.ConfigFile() - if err != nil { - return "", fmt.Errorf("cannot get config for image: %w", err) - } - - if cfg.Config.Labels != nil { - if u, ok := cfg.Config.Labels["io.openshift.s2i.scripts-url"]; ok { - return u, nil - } - } - } - return "", err - } - - if img.Config != nil && img.Config.Labels != nil { - if u, ok := img.Config.Labels["io.openshift.s2i.scripts-url"]; ok { - return u, nil - } - } - - //nolint:staticcheck - if img.ContainerConfig != nil && img.ContainerConfig.Labels != nil { - if u, ok := img.ContainerConfig.Labels["io.openshift.s2i.scripts-url"]; ok { - return u, nil - } - } - - return "", nil + return nil } // Builder Image chooses the correct builder image or defaults. diff --git a/pkg/builders/s2i/builder_test.go b/pkg/builders/s2i/builder_test.go index 4a3313130f..3367444f03 100644 --- a/pkg/builders/s2i/builder_test.go +++ b/pkg/builders/s2i/builder_test.go @@ -1,28 +1,17 @@ package s2i_test import ( - "archive/tar" - "bytes" "context" "errors" - "fmt" "io" - "log" - "net" - "net/http" - "os" - "path/filepath" - "strings" "testing" - "github.com/google/go-containerregistry/pkg/name" - "github.com/google/go-containerregistry/pkg/registry" - "github.com/google/go-containerregistry/pkg/v1/remote" - "github.com/google/go-containerregistry/pkg/v1/tarball" - "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/image" + "github.com/docker/docker/api/types/network" "github.com/docker/docker/errdefs" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/openshift/source-to-image/pkg/api" @@ -165,70 +154,6 @@ func Test_BuilderImageConfigurable(t *testing.T) { } } -// Test_BuildImageWithFuncIgnore ensures that ignored files are not added to -// the func image -func Test_BuildImageWithFuncIgnore(t *testing.T) { - - funcIgnoreContent := []byte(`#testing Comments -#testingComments.txt -hello.txt -`) - f := fn.Function{ - Runtime: "node", - } - tempdir := t.TempDir() - f.Root = tempdir - //create a .funcignore file containing the details of the files to be ignored - err := os.WriteFile(filepath.Join(f.Root, ".funcignore"), funcIgnoreContent, 0644) - if err != nil { - t.Fatal(err) - } - - // creating test files which should be ignored - err = os.WriteFile(filepath.Join(f.Root, "hello.txt"), []byte(""), 0644) - if err != nil { - t.Fatal(err) - } - - err = os.WriteFile(filepath.Join(f.Root, "#testingComments.txt"), []byte(""), 0644) - if err != nil { - t.Fatal(err) - } - - cli := mockDocker{ - build: func(ctx context.Context, context io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) { - tr := tar.NewReader(context) - for { - hdr, err := tr.Next() - if err != nil { - if errors.Is(err, io.EOF) { - break - } - return types.ImageBuildResponse{}, err - } - - // If we find the undesired file, return an error - if filepath.Base(hdr.Name) == "hello.txt" { - return types.ImageBuildResponse{}, fmt.Errorf("test failed, found ignonered file %s:", filepath.Base(hdr.Name)) - } - // If we find the undesired file, return an error - if filepath.Base(hdr.Name) == "#tesingComments.txt" { - return types.ImageBuildResponse{}, fmt.Errorf("test failed, found ignonered file %s:", filepath.Base(hdr.Name)) - } - - } - return types.ImageBuildResponse{ - Body: io.NopCloser(strings.NewReader(`{"stream": "OK!"}`)), - OSType: "linux", - }, nil - }, - } - b := s2i.NewBuilder(s2i.WithName(builders.S2I), s2i.WithDockerClient(cli)) - if err := b.Build(context.Background(), f, nil); err != nil { - t.Fatal(err) - } -} - // Test_Verbose ensures that the verbosity flag is propagated to the // S2I builder implementation. func Test_BuilderVerbose(t *testing.T) { @@ -284,228 +209,95 @@ func Test_BuildEnvs(t *testing.T) { } } -func TestS2IScriptURL(t *testing.T) { - testRegistry := startRegistry(t) - - // builder that is only in registry not in daemon - remoteBuilder := testRegistry + "/default/builder:remote" - // builder that is in daemon - localBuilder := "example.com/default/builder:local" - - // begin push testing builder to registry - tag, err := name.NewTag(remoteBuilder) - if err != nil { - t.Fatal(err) - } - - img, err := tarball.ImageFromPath(filepath.Join("testdata", "builder.tar"), nil) - if err != nil { - t.Fatal(err) - } - - err = remote.Write(&tag, img) - if err != nil { - t.Fatal(err) - } - // end push testing builder to registry - - scriptURL := "image:///usr/local/s2i" +func TestBuildFail(t *testing.T) { cli := mockDocker{ inspect: func(ctx context.Context, image string) (types.ImageInspect, []byte, error) { - if image != localBuilder { - return types.ImageInspect{}, nil, notFoundErr{} - } - return types.ImageInspect{ - Config: &container.Config{Labels: map[string]string{"io.openshift.s2i.scripts-url": scriptURL}}, - }, nil, nil + return types.ImageInspect{}, nil, errors.New("this is expected") }, } - impl := &mockImpl{ - BuildFn: func(config *api.Config) (*api.Result, error) { - if config.ScriptsURL != scriptURL { - return nil, fmt.Errorf("unexepeted ScriptURL: %q", config.ScriptsURL) - } - return nil, nil - }, - } - - tests := []struct { - name string - builderImage string - }{ - {name: "builder in daemon", builderImage: localBuilder}, - {name: "builder not in daemon", builderImage: remoteBuilder}, + b := s2i.NewBuilder(s2i.WithDockerClient(cli)) + err := b.Build(context.Background(), fn.Function{Runtime: "node"}, nil) + if err == nil { + t.Error("didn't get expected error") } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - f := fn.Function{ - Runtime: "node", - Build: fn.BuildSpec{ - BuilderImages: map[string]string{ - builders.S2I: tt.builderImage, - }, - }, - } +} - b := s2i.NewBuilder(s2i.WithName(builders.S2I), s2i.WithImpl(impl), s2i.WithDockerClient(cli)) - err = b.Build(context.Background(), f, nil) - if err != nil { - t.Error(err) - } - }) - } +// mockImpl is a mock implementation of an S2I builder. +type mockImpl struct { + BuildFn func(*api.Config) (*api.Result, error) +} +func (i *mockImpl) Build(cfg *api.Config) (*api.Result, error) { + return i.BuildFn(cfg) } -func startRegistry(t *testing.T) (addr string) { - s := http.Server{ - Handler: registry.New(registry.Logger(log.New(io.Discard, "", 0))), - } - t.Cleanup(func() { s.Close() }) +type mockDocker struct { + inspect func(ctx context.Context, image string) (types.ImageInspect, []byte, error) +} - l, err := net.Listen("tcp", "localhost:0") - if err != nil { - t.Fatal(err) +func (m mockDocker) ImageInspectWithRaw(ctx context.Context, image string) (types.ImageInspect, []byte, error) { + if m.inspect != nil { + return m.inspect(ctx, image) } - addr = l.Addr().String() - go func() { - err = s.Serve(l) - if err != nil && !errors.Is(err, net.ErrClosed) { - fmt.Fprintln(os.Stderr, "ERROR: ", err) - } - }() + return types.ImageInspect{}, nil, nil +} - return addr +func (m mockDocker) ImageBuild(ctx context.Context, context io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) { + panic("implement me") } -func TestBuildContextUpload(t *testing.T) { +func (m mockDocker) ContainerAttach(ctx context.Context, container string, options container.AttachOptions) (types.HijackedResponse, error) { + panic("implement me") +} - dockerfileContent := []byte("FROM scratch\nLABEL A=42") - atxtContent := []byte("hello world!\n") +func (m mockDocker) ContainerCommit(ctx context.Context, container string, options container.CommitOptions) (types.IDResponse, error) { + panic("implement me") +} - cli := mockDocker{ - build: func(ctx context.Context, context io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) { - tr := tar.NewReader(context) - for { - hdr, err := tr.Next() - if err != nil { - if errors.Is(err, io.EOF) { - break - } - return types.ImageBuildResponse{}, err - } - switch hdr.Name { - case ".": - case "Dockerfile": - bs, err := io.ReadAll(tr) - if err != nil { - return types.ImageBuildResponse{}, err - } - if !bytes.Equal(bs, dockerfileContent) { - return types.ImageBuildResponse{}, errors.New("bad content for Dockerfile") - } - case "a.txt": - bs, err := io.ReadAll(tr) - if err != nil { - return types.ImageBuildResponse{}, err - } - if !bytes.Equal(bs, atxtContent) { - return types.ImageBuildResponse{}, errors.New("bad content for a.txt") - } - default: - return types.ImageBuildResponse{}, fmt.Errorf("unexpected file or directory: %q", hdr.Name) - } - } - return types.ImageBuildResponse{ - Body: io.NopCloser(strings.NewReader(`{"stream": "OK!"}`)), - OSType: "linux", - }, nil - }, - } +func (m mockDocker) ContainerCreate(ctx context.Context, config *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig, platform *ocispec.Platform, containerName string) (container.CreateResponse, error) { + panic("implement me") +} - impl := &mockImpl{ - BuildFn: func(config *api.Config) (*api.Result, error) { - err := os.WriteFile(config.AsDockerfile, dockerfileContent, 0644) - if err != nil { - return nil, err - } - err = os.WriteFile(filepath.Join(filepath.Dir(config.AsDockerfile), "a.txt"), atxtContent, 0644) - if err != nil { - return nil, err - } - err = os.Mkdir(filepath.Join(filepath.Dir(config.AsDockerfile), "node_modules"), 0755) - if err != nil { - return nil, err - } +func (m mockDocker) ContainerInspect(ctx context.Context, container string) (types.ContainerJSON, error) { + panic("implement me") +} - return nil, nil - }, - } +func (m mockDocker) ContainerRemove(ctx context.Context, container string, options container.RemoveOptions) error { + panic("implement me") +} - f := fn.Function{ - Runtime: "node", - } - b := s2i.NewBuilder(s2i.WithImpl(impl), s2i.WithDockerClient(cli)) - err := b.Build(context.Background(), f, nil) - if err != nil { - t.Error(err) - } +func (m mockDocker) ContainerStart(ctx context.Context, container string, options container.StartOptions) error { + panic("implement me") } -func TestBuildFail(t *testing.T) { - cli := mockDocker{ - build: func(ctx context.Context, context io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) { - return types.ImageBuildResponse{ - Body: io.NopCloser(strings.NewReader(`{"errorDetail": {"message": "Error: this is expected"}}`)), - OSType: "linux", - }, nil - }, - } - impl := &mockImpl{ - BuildFn: func(config *api.Config) (*api.Result, error) { - return &api.Result{Success: true}, nil - }, - } - b := s2i.NewBuilder(s2i.WithImpl(impl), s2i.WithDockerClient(cli)) - err := b.Build(context.Background(), fn.Function{Runtime: "node"}, nil) - if err == nil || !strings.Contains(err.Error(), "Error: this is expected") { - t.Error("didn't get expected error") - } +func (m mockDocker) ContainerKill(ctx context.Context, container, signal string) error { + panic("implement me") } -// mockImpl is a mock implementation of an S2I builder. -type mockImpl struct { - BuildFn func(*api.Config) (*api.Result, error) +func (m mockDocker) ContainerWait(ctx context.Context, container string, condition container.WaitCondition) (<-chan container.WaitResponse, <-chan error) { + panic("implement me") } -func (i *mockImpl) Build(cfg *api.Config) (*api.Result, error) { - return i.BuildFn(cfg) +func (m mockDocker) CopyToContainer(ctx context.Context, container, path string, content io.Reader, opts container.CopyToContainerOptions) error { + panic("implement me") } -type mockDocker struct { - inspect func(ctx context.Context, image string) (types.ImageInspect, []byte, error) - build func(ctx context.Context, context io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) +func (m mockDocker) CopyFromContainer(ctx context.Context, container, srcPath string) (io.ReadCloser, container.PathStat, error) { + panic("implement me") } -func (m mockDocker) ImageInspectWithRaw(ctx context.Context, image string) (types.ImageInspect, []byte, error) { - if m.inspect != nil { - return m.inspect(ctx, image) - } +func (m mockDocker) ImagePull(ctx context.Context, ref string, options image.PullOptions) (io.ReadCloser, error) { + panic("implement me") +} - return types.ImageInspect{}, nil, nil +func (m mockDocker) ImageRemove(ctx context.Context, image string, options image.RemoveOptions) ([]image.DeleteResponse, error) { + panic("implement me") } -func (m mockDocker) ImageBuild(ctx context.Context, context io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) { - if m.build != nil { - return m.build(ctx, context, options) - } +func (m mockDocker) ServerVersion(ctx context.Context) (types.Version, error) { - _, _ = io.Copy(io.Discard, context) - return types.ImageBuildResponse{ - Body: io.NopCloser(strings.NewReader("")), - OSType: "linux", - }, nil + panic("implement me") } type notFoundErr struct {