diff --git a/s2i/builder.go b/s2i/builder.go index 940db89875..90d96d5486 100644 --- a/s2i/builder.go +++ b/s2i/builder.go @@ -1,14 +1,22 @@ package s2i import ( + "archive/tar" "context" + "encoding/json" "errors" "fmt" + "io" + "io/fs" "net/url" "os" + "path/filepath" + "github.com/docker/docker/api/types" dockerClient "github.com/docker/docker/client" - + "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" @@ -17,7 +25,7 @@ import ( "github.com/openshift/source-to-image/pkg/scm/git" fn "knative.dev/kn-plugin-func" - docker "knative.dev/kn-plugin-func/docker" + "knative.dev/kn-plugin-func/docker" ) var ( @@ -36,10 +44,17 @@ var DefaultBuilderImages = map[string]string{ "quarkus": "registry.access.redhat.com/ubi8/openjdk-17", } +// 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 { verbose bool impl build.Builder // S2I builder implementation (aka "Strategy") + cli DockerClient } type Option func(*Builder) @@ -60,6 +75,12 @@ func WithImpl(s build.Builder) Option { } } +func WithDockerClient(cli DockerClient) Option { + return func(b *Builder) { + b.cli = cli + } +} + // NewBuilder creates a new instance of a Builder with static defaults. func NewBuilder(options ...Option) *Builder { b := &Builder{} @@ -70,6 +91,8 @@ func NewBuilder(options ...Option) *Builder { } func (b *Builder) Build(ctx context.Context, f fn.Function) (err error) { + // TODO this function currently doesn't support private s2i builder images since credentials are not set + // Builder image from the Function if defined, default otherwise. builderImage, err := builderImage(f) if err != nil { @@ -87,6 +110,27 @@ func (b *Builder) Build(ctx context.Context, f fn.Function) (err error) { cfg.RuntimeImagePullPolicy = api.DefaultRuntimeImagePullPolicy cfg.DockerConfig = s2idocker.GetDefaultDockerConfig() + tmp, err := os.MkdirTemp("", "s2i-build") + if err != nil { + return fmt.Errorf("cannot create temporary dir for s2i build: %w", err) + } + defer os.RemoveAll(tmp) + + cfg.AsDockerfile = filepath.Join(tmp, "Dockerfile") + + if b.cli == nil { + b.cli, _, err = docker.NewClient(dockerClient.DefaultDockerHost) + if err != nil { + return fmt.Errorf("cannot create docker client: %w", err) + } + } + + scriptURL, err := s2iScriptURL(ctx, b.cli, cfg.BuilderImage) + if err != nil { + return fmt.Errorf("cannot get s2i script url: %w", err) + } + 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 @@ -115,20 +159,9 @@ func (b *Builder) Build(ctx context.Context, f fn.Function) (err error) { // Create the S2I builder instance if not overridden if b.impl == nil { - var client dockerClient.CommonAPIClient - client, _, err = docker.NewClient(dockerClient.DefaultDockerHost) - if err != nil { - return - } - defer client.Close() - - if isPodman(ctx, client) { - client = podmanDockerClient{client} - } - - b.impl, _, err = strategies.Strategy(client, cfg, build.Overrides{}) + b.impl, _, err = strategies.Strategy(nil, cfg, build.Overrides{}) if err != nil { - return + return fmt.Errorf("cannot create s2i builder: %w", err) } } @@ -143,7 +176,150 @@ func (b *Builder) Build(ctx context.Context, f fn.Function) (err error) { fmt.Println(message) } } - return + + pr, pw := io.Pipe() + + 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) + } + + hdr, err := tar.FileInfoHeader(fi, p) + if err != nil { + return fmt.Errorf("cannot create tar header: %w", err) + } + hdr.Name = p + + 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) + } + _, 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.Image}, + } + + resp, err := b.cli.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 + } + + errMsg, err := parseBuildResponse(resp.Body, out) + if err != nil { + return fmt.Errorf("cannot parse response body: %w", err) + } + if errMsg != "" { + return fmt.Errorf("cannot build the app: %s", errMsg) + } + + return nil +} + +func parseBuildResponse(r io.Reader, w io.Writer) (errorMessage string, err error) { + obj := struct { + ErrorDetail struct { + Message string `json:"message"` + } `json:"errorDetail"` + Stream string `json:"stream"` + }{} + d := json.NewDecoder(r) + for { + err = d.Decode(&obj) + if err != nil { + if errors.Is(err, io.EOF) { + break + } + return "", err + } + if obj.ErrorDetail.Message != "" { + errorMessage = obj.ErrorDetail.Message + return errorMessage, nil + } + if obj.Stream != "" { + _, err = w.Write([]byte(obj.Stream)) + if err != nil { + return "", err + } + } + } + return "", nil +} + +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) + } + 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 + } + } + + 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 } // builderImage for Function diff --git a/s2i/builder_test.go b/s2i/builder_test.go index 81061d25e5..9c4ee974e6 100644 --- a/s2i/builder_test.go +++ b/s2i/builder_test.go @@ -1,10 +1,29 @@ package s2i_test import ( + "archive/tar" + "bytes" "context" "errors" + "fmt" + "io" + "io/ioutil" + "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/openshift/source-to-image/pkg/api" fn "knative.dev/kn-plugin-func" "knative.dev/kn-plugin-func/s2i" @@ -37,9 +56,10 @@ func Test_ErrRuntimeNotSupported(t *testing.T) { // define a Builder Image will default. func Test_ImageDefault(t *testing.T) { var ( - i = &mockImpl{} // mock underlying s2i implementation - b = s2i.NewBuilder(s2i.WithImpl(i)) // Func S2I Builder logic - f = fn.Function{Runtime: "node"} // Function with no builder image set + i = &mockImpl{} // mock underlying s2i implementation + c = mockDocker{} // mock docker client + b = s2i.NewBuilder(s2i.WithImpl(i), s2i.WithDockerClient(c)) // Func S2I Builder logic + f = fn.Function{Runtime: "node"} // Function with no builder image set ) // An implementation of the underlying S2I implementation which verifies @@ -63,9 +83,10 @@ func Test_ImageDefault(t *testing.T) { // image defined on the given Function if provided. func Test_BuilderImageConfigurable(t *testing.T) { var ( - i = &mockImpl{} // mock underlying s2i implementation - b = s2i.NewBuilder(s2i.WithImpl(i)) // Func S2I Builder logic - f = fn.Function{ // Function with a builder image set + i = &mockImpl{} // mock underlying s2i implementation + c = mockDocker{} // mock docker client + b = s2i.NewBuilder(s2i.WithImpl(i), s2i.WithDockerClient(c)) // Func S2I Builder logic + f = fn.Function{ // Function with a builder image set Runtime: "node", BuilderImages: map[string]string{ "s2i": "example.com/user/builder-image", @@ -93,6 +114,7 @@ func Test_BuilderImageConfigurable(t *testing.T) { // Test_Verbose ensures that the verbosity flag is propagated to the // S2I builder implementation. func Test_BuilderVerbose(t *testing.T) { + c := mockDocker{} // mock docker client assert := func(verbose bool) { i := &mockImpl{ BuildFn: func(cfg *api.Config) (r *api.Result, err error) { @@ -101,7 +123,7 @@ func Test_BuilderVerbose(t *testing.T) { } return &api.Result{Messages: []string{"message"}}, nil }} - if err := s2i.NewBuilder(s2i.WithVerbose(verbose), s2i.WithImpl(i)).Build(context.Background(), fn.Function{Runtime: "node"}); err != nil { + if err := s2i.NewBuilder(s2i.WithVerbose(verbose), s2i.WithImpl(i), s2i.WithDockerClient(c)).Build(context.Background(), fn.Function{Runtime: "node"}); err != nil { t.Fatal(err) } } @@ -122,7 +144,8 @@ func Test_BuildEnvs(t *testing.T) { BuildEnvs: []fn.Env{{Name: &envName, Value: &envValue}}, } i = &mockImpl{} - b = s2i.NewBuilder(s2i.WithImpl(i)) + c = mockDocker{} + b = s2i.NewBuilder(s2i.WithImpl(i), s2i.WithDockerClient(c)) ) i.BuildFn = func(cfg *api.Config) (r *api.Result, err error) { for _, v := range cfg.Environment { @@ -140,6 +163,190 @@ 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" + 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 + }, + } + 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}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := fn.Function{ + Runtime: "node", + BuilderImages: map[string]string{ + "s2i": tt.builderImage, + }, + } + + b := s2i.NewBuilder(s2i.WithImpl(impl), s2i.WithDockerClient(cli)) + err = b.Build(context.Background(), f) + if err != nil { + t.Error(err) + } + }) + } + +} + +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() }) + + l, err := net.Listen("tcp", "localhost:0") + if err != nil { + t.Fatal(err) + } + 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 addr +} + +func TestBuildContextUpload(t *testing.T) { + + dockerfileContent := []byte("FROM scratch\nLABEL A=42") + atxtContent := []byte("hello world!\n") + + 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 := ioutil.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 := ioutil.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{}, errors.New("unexpected file") + } + } + return types.ImageBuildResponse{ + Body: io.NopCloser(strings.NewReader(`{"stream": "OK!"}`)), + OSType: "linux", + }, nil + }, + } + + impl := &mockImpl{ + BuildFn: func(config *api.Config) (*api.Result, error) { + err := ioutil.WriteFile(config.AsDockerfile, dockerfileContent, 0644) + if err != nil { + return nil, err + } + err = ioutil.WriteFile(filepath.Join(filepath.Dir(config.AsDockerfile), "a.txt"), atxtContent, 0644) + if err != nil { + return nil, err + } + + return nil, nil + }, + } + + f := fn.Function{ + Runtime: "node", + } + b := s2i.NewBuilder(s2i.WithImpl(impl), s2i.WithDockerClient(cli)) + err := b.Build(context.Background(), f) + if err != nil { + t.Error(err) + } +} + +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"}) + if err == nil || !strings.Contains(err.Error(), "Error: this is expected") { + t.Error("didn't get expected error") + } +} + // mockImpl is a mock implementation of an S2I builder. type mockImpl struct { BuildFn func(*api.Config) (*api.Result, error) @@ -148,3 +355,39 @@ type mockImpl struct { func (i *mockImpl) Build(cfg *api.Config) (*api.Result, error) { return i.BuildFn(cfg) } + +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) ImageInspectWithRaw(ctx context.Context, image string) (types.ImageInspect, []byte, error) { + if m.inspect != nil { + return m.inspect(ctx, image) + } + + return types.ImageInspect{}, nil, nil +} + +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) + } + + _, _ = io.Copy(io.Discard, context) + return types.ImageBuildResponse{ + Body: io.NopCloser(strings.NewReader("")), + OSType: "linux", + }, nil +} + +type notFoundErr struct { +} + +func (n notFoundErr) Error() string { + return "not found" +} + +func (n notFoundErr) NotFound() bool { + return true +} diff --git a/s2i/docker_client_wrapper.go b/s2i/docker_client_wrapper.go deleted file mode 100644 index e67f37256e..0000000000 --- a/s2i/docker_client_wrapper.go +++ /dev/null @@ -1,62 +0,0 @@ -package s2i - -import ( - "context" - "encoding/json" - "fmt" - "strings" - - "github.com/docker/docker/api/types" - "github.com/docker/docker/client" -) - -// Wrapper to workaround https://github.com/containers/podman/issues/13770 -type podmanDockerClient struct { - client.CommonAPIClient -} - -func (p podmanDockerClient) ContainerCommit(ctx context.Context, nameOrID string, opts types.ContainerCommitOptions) (types.IDResponse, error) { - if len(opts.Config.Cmd) > 0 { - bs, err := json.Marshal(opts.Config.Cmd) - if err != nil { - return types.IDResponse{}, err - } - opts.Changes = append(opts.Changes, "CMD "+string(bs)) - } - - if len(opts.Config.Entrypoint) > 0 { - bs, err := json.Marshal(opts.Config.Entrypoint) - if err != nil { - return types.IDResponse{}, err - } - opts.Changes = append(opts.Changes, "ENTRYPOINT "+string(bs)) - } - - if opts.Config.User != "" { - opts.Changes = append(opts.Changes, "USER "+opts.Config.User) - } - - for _, e := range opts.Config.Env { - parts := strings.SplitN(e, "=", 2) - opts.Changes = append(opts.Changes, fmt.Sprintf("ENV %s=%q", parts[0], parts[1])) - } - - for k, v := range opts.Config.Labels { - opts.Changes = append(opts.Changes, fmt.Sprintf("LABEL %q=%q", k, v)) - } - - return p.CommonAPIClient.ContainerCommit(ctx, nameOrID, opts) -} - -func isPodman(ctx context.Context, cli client.CommonAPIClient) bool { - v, err := cli.ServerVersion(ctx) - if err != nil { - return false - } - for _, comp := range v.Components { - if comp.Name == "Podman Engine" { - return true - } - } - return false -} diff --git a/s2i/testData/builder.tar b/s2i/testData/builder.tar new file mode 100644 index 0000000000..8928b96b0f Binary files /dev/null and b/s2i/testData/builder.tar differ