Skip to content

Commit 5436968

Browse files
csweichelcsweichel
committed
[initializer] Add digest support and unit tests
to the new FileDownloadInitializer
1 parent c10fb46 commit 5436968

File tree

7 files changed

+231
-52
lines changed

7 files changed

+231
-52
lines changed

‎components/content-service/go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ require (
2323
github.com/prometheus/client_golang v1.9.0
2424
github.com/spf13/cobra v1.1.1
2525
golang.org/x/oauth2 v0.0.0-20210427180440-81ed05c6b58c
26+
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
2627
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1
2728
google.golang.org/api v0.46.0
2829
google.golang.org/grpc v1.37.0

‎components/content-service/go.sum

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -899,6 +899,7 @@ golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a/go.mod h1:RxMgew5VJxzue5/jJ
899899
golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
900900
golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
901901
golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
902+
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c h1:5KslGYwFpkhGh+Q16bwMP3cOontH8FOep7tGV86Y7SQ=
902903
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
903904
golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
904905
golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=

‎components/content-service/pkg/initializer/download.go

Lines changed: 65 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package initializer
77
import (
88
"context"
99
"io"
10-
"io/ioutil"
1110
"net/http"
1211
"os"
1312
"path/filepath"
@@ -17,83 +16,111 @@ import (
1716
"github.com/gitpod-io/gitpod/common-go/tracing"
1817
csapi "github.com/gitpod-io/gitpod/content-service/api"
1918
"github.com/gitpod-io/gitpod/content-service/pkg/archive"
19+
"github.com/opencontainers/go-digest"
2020
"github.com/opentracing/opentracing-go"
21+
"golang.org/x/sync/errgroup"
2122
"golang.org/x/xerrors"
2223
)
2324

24-
type FileInfo struct {
25-
url string
26-
// filePath is relative to the FileDownloadInitializer's TargetLocation, e.g. if TargetLocation is in `/workspace/myrepo`
27-
// a filePath of `foobar/file` would produce a file in `/workspace/myrepo/foobar/file`.
28-
// filePath must include the filename. The FileDownloadInitializer will create any parent directories
25+
type fileInfo struct {
26+
URL string
27+
28+
// Path is relative to the FileDownloadInitializer's TargetLocation, e.g. if TargetLocation is in `/workspace/myrepo`
29+
// a Path of `foobar/file` would produce a file in `/workspace/myrepo/foobar/file`.
30+
// Path must include the filename. The FileDownloadInitializer will create any parent directories
2931
// necessary to place the file.
30-
filePath string
31-
// digest is a hash of the file content in the OCI digest format (see https://github.com/opencontainers/image-spec/blob/master/descriptor.md#digests).
32+
Path string
33+
34+
// Digest is a hash of the file content in the OCI Digest format (see https://github.com/opencontainers/image-spec/blob/master/descriptor.md#digests).
3235
// This information is used to compute subsequent
3336
// content versions, and to validate the file content was downloaded correctly.
34-
digest string
37+
Digest digest.Digest
3538
}
3639

37-
type FileDownloadInitializer struct {
38-
FilesInfos []FileInfo
40+
type fileDownloadInitializer struct {
41+
FilesInfos []fileInfo
3942
TargetLocation string
43+
HTTPClient *http.Client
44+
RetryTimeout time.Duration
4045
}
4146

4247
// Run initializes the workspace
43-
func (ws *FileDownloadInitializer) Run(ctx context.Context, mappings []archive.IDMapping) (src csapi.WorkspaceInitSource, err error) {
48+
func (ws *fileDownloadInitializer) Run(ctx context.Context, mappings []archive.IDMapping) (src csapi.WorkspaceInitSource, err error) {
4449
span, ctx := opentracing.StartSpanFromContext(ctx, "FileDownloadInitializer.Run")
4550
defer tracing.FinishSpan(span, &err)
4651

4752
for _, info := range ws.FilesInfos {
48-
contents, err := downloadFile(ctx, info.url)
49-
if err != nil {
50-
tracing.LogError(span, xerrors.Errorf("cannot download file '%s' from '%s': %w", info.filePath, info.url, err))
51-
}
52-
53-
fullPath := filepath.Join(ws.TargetLocation, info.filePath)
54-
err = os.MkdirAll(filepath.Dir(fullPath), 0755)
55-
if err != nil {
56-
tracing.LogError(span, xerrors.Errorf("cannot mkdir %s: %w", filepath.Dir(fullPath), err))
57-
}
58-
err = ioutil.WriteFile(fullPath, contents, 0755)
53+
err := ws.downloadFile(ctx, info)
5954
if err != nil {
60-
tracing.LogError(span, xerrors.Errorf("cannot write %s: %w", fullPath, err))
55+
tracing.LogError(span, xerrors.Errorf("cannot download file '%s' from '%s': %w", info.Path, info.URL, err))
56+
return src, err
6157
}
6258
}
63-
return src, nil
59+
return csapi.WorkspaceInitFromOther, nil
6460
}
6561

66-
func downloadFile(ctx context.Context, url string) (content []byte, err error) {
62+
func (ws *fileDownloadInitializer) downloadFile(ctx context.Context, info fileInfo) (err error) {
6763
//nolint:ineffassign
6864
span, ctx := opentracing.StartSpanFromContext(ctx, "downloadFile")
6965
defer tracing.FinishSpan(span, &err)
70-
span.LogKV("url", url)
66+
span.LogKV("url", info.URL)
7167

72-
dl := func() (content []byte, err error) {
73-
req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
68+
fn := filepath.Join(ws.TargetLocation, info.Path)
69+
err = os.MkdirAll(filepath.Dir(fn), 0755)
70+
if err != nil {
71+
tracing.LogError(span, xerrors.Errorf("cannot mkdir %s: %w", filepath.Dir(fn), err))
72+
}
73+
74+
fd, err := os.OpenFile(fn, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0644)
75+
if err != nil {
76+
return err
77+
}
78+
79+
dl := func() (err error) {
80+
req, err := http.NewRequestWithContext(ctx, "GET", info.URL, nil)
7481
if err != nil {
75-
return nil, err
82+
return err
7683
}
7784
_ = opentracing.GlobalTracer().Inject(span.Context(), opentracing.HTTPHeaders, opentracing.HTTPHeadersCarrier(req.Header))
7885

79-
resp, err := http.DefaultClient.Do(req)
86+
resp, err := ws.HTTPClient.Do(req)
8087
if err != nil {
81-
return nil, err
88+
return err
8289
}
8390
defer resp.Body.Close()
8491
if resp.StatusCode != http.StatusOK {
85-
return nil, xerrors.Errorf("non-OK OTS response: %s", resp.Status)
92+
return xerrors.Errorf("non-OK download response: %s", resp.Status)
8693
}
8794

88-
return io.ReadAll(resp.Body)
95+
pr, pw := io.Pipe()
96+
body := io.TeeReader(resp.Body, pw)
97+
98+
eg, _ := errgroup.WithContext(ctx)
99+
eg.Go(func() error {
100+
_, err = io.Copy(fd, body)
101+
pw.Close()
102+
return err
103+
})
104+
eg.Go(func() error {
105+
dgst, err := digest.FromReader(pr)
106+
if err != nil {
107+
return err
108+
}
109+
if dgst != info.Digest {
110+
return xerrors.Errorf("digest mismatch")
111+
}
112+
return nil
113+
})
114+
115+
return eg.Wait()
89116
}
90117
for i := 0; i < otsDownloadAttempts; i++ {
91118
span.LogKV("attempt", i)
92119
if i > 0 {
93-
time.Sleep(time.Second)
120+
time.Sleep(ws.RetryTimeout)
94121
}
95122

96-
content, err = dl()
123+
err = dl()
97124
if err == context.Canceled || err == context.DeadlineExceeded {
98125
return
99126
}
@@ -103,8 +130,8 @@ func downloadFile(ctx context.Context, url string) (content []byte, err error) {
103130
log.WithError(err).WithField("attempt", i).Warn("cannot download additional content files")
104131
}
105132
if err != nil {
106-
return nil, err
133+
return err
107134
}
108135

109-
return content, nil
136+
return nil
110137
}

‎components/content-service/pkg/initializer/download_test.go

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
// Copyright (c) 2021 Gitpod GmbH. All rights reserved.
2+
// Licensed under the GNU Affero General Public License (AGPL).
3+
// See License-AGPL.txt in the project root for license information.
4+
5+
package initializer
6+
7+
import (
8+
"bytes"
9+
"context"
10+
"io"
11+
"net/http"
12+
"os"
13+
"testing"
14+
15+
"github.com/gitpod-io/gitpod/content-service/api"
16+
"github.com/opencontainers/go-digest"
17+
)
18+
19+
// RoundTripFunc .
20+
type RoundTripFunc func(req *http.Request) *http.Response
21+
22+
// RoundTrip .
23+
func (f RoundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) {
24+
return f(req), nil
25+
}
26+
27+
func TestFileDownloadInitializer(t *testing.T) {
28+
const defaultContent = "hello world"
29+
30+
type serverSideFile struct {
31+
Path string
32+
Content string
33+
}
34+
tests := []struct {
35+
Name string
36+
Files []fileInfo
37+
ServerSide []serverSideFile
38+
ExpectedError string
39+
}{
40+
{
41+
Name: "happy path",
42+
Files: []fileInfo{
43+
{
44+
URL: "/file1",
45+
Path: "/level/file1",
46+
Digest: digest.FromString(defaultContent),
47+
},
48+
// duplication is intentional
49+
{
50+
URL: "/file1",
51+
Path: "/level/file1",
52+
Digest: digest.FromString(defaultContent),
53+
},
54+
{
55+
URL: "/file2",
56+
Path: "/level/file2",
57+
Digest: digest.FromString(defaultContent),
58+
},
59+
},
60+
ServerSide: []serverSideFile{
61+
{Path: "/file1", Content: defaultContent},
62+
{Path: "/file2", Content: defaultContent},
63+
},
64+
},
65+
{
66+
Name: "digest mismatch",
67+
Files: []fileInfo{
68+
{
69+
URL: "/file1",
70+
Path: "/level/file1",
71+
Digest: digest.FromString(defaultContent + "foobar"),
72+
},
73+
},
74+
ServerSide: []serverSideFile{
75+
{Path: "/file1", Content: defaultContent},
76+
},
77+
ExpectedError: "digest mismatch",
78+
},
79+
{
80+
Name: "file not found",
81+
Files: []fileInfo{
82+
{
83+
URL: "/file1",
84+
Path: "/level/file1",
85+
Digest: digest.FromString(defaultContent + "foobar"),
86+
},
87+
},
88+
ExpectedError: "non-OK download response: Not Found",
89+
},
90+
}
91+
92+
for _, test := range tests {
93+
t.Run(test.Name, func(t *testing.T) {
94+
tmpdir, err := os.MkdirTemp("", "TestFileDownloadInitializer*")
95+
if err != nil {
96+
t.Fatal("cannot create tempdir", err)
97+
}
98+
defer os.RemoveAll(tmpdir)
99+
100+
client := &http.Client{
101+
Transport: RoundTripFunc(func(req *http.Request) *http.Response {
102+
for _, f := range test.ServerSide {
103+
if f.Path != req.URL.Path {
104+
continue
105+
}
106+
107+
return &http.Response{
108+
StatusCode: http.StatusOK,
109+
Body: io.NopCloser(bytes.NewReader([]byte(f.Content))),
110+
Header: make(http.Header),
111+
}
112+
}
113+
114+
return &http.Response{
115+
Status: http.StatusText(http.StatusNotFound),
116+
StatusCode: http.StatusNotFound,
117+
Header: make(http.Header),
118+
}
119+
}),
120+
}
121+
122+
req := &api.FileDownloadInitializer{}
123+
for _, f := range test.Files {
124+
req.Files = append(req.Files, &api.FileDownloadInitializer_FileInfo{
125+
Url: "http://foobar" + f.URL,
126+
FilePath: f.Path,
127+
Digest: string(f.Digest),
128+
})
129+
}
130+
131+
initializer, err := newFileDownloadInitializer(tmpdir, req)
132+
if err != nil {
133+
t.Fatal(err)
134+
}
135+
initializer.HTTPClient = client
136+
initializer.RetryTimeout = 0
137+
138+
src, err := initializer.Run(context.Background(), nil)
139+
if err == nil && src != api.WorkspaceInitFromOther {
140+
t.Error("initializer returned wrong content init source")
141+
}
142+
if err != nil && err.Error() != test.ExpectedError {
143+
t.Fatalf("unexpected error: %v", err)
144+
}
145+
})
146+
}
147+
}

‎components/content-service/pkg/initializer/initializer.go

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"strings"
1616
"time"
1717

18+
"github.com/opencontainers/go-digest"
1819
"github.com/opentracing/opentracing-go"
1920
"golang.org/x/xerrors"
2021
"google.golang.org/grpc/codes"
@@ -145,18 +146,24 @@ func NewFromRequest(ctx context.Context, loc string, rs storage.DirectDownloader
145146
}
146147

147148
// newFileDownloadInitializer creates a download initializer for a request
148-
func newFileDownloadInitializer(loc string, req *csapi.FileDownloadInitializer) (*FileDownloadInitializer, error) {
149-
fileInfos := make([]FileInfo, len(req.Files))
149+
func newFileDownloadInitializer(loc string, req *csapi.FileDownloadInitializer) (*fileDownloadInitializer, error) {
150+
fileInfos := make([]fileInfo, len(req.Files))
150151
for i, f := range req.Files {
151-
fileInfos[i] = FileInfo{
152-
url: f.Url,
153-
filePath: f.FilePath,
154-
digest: f.Digest,
152+
dgst, err := digest.Parse(f.Digest)
153+
if err != nil {
154+
return nil, xerrors.Errorf("invalid digest %s: %w", f.Digest, err)
155+
}
156+
fileInfos[i] = fileInfo{
157+
URL: f.Url,
158+
Path: f.FilePath,
159+
Digest: dgst,
155160
}
156161
}
157-
initializer := &FileDownloadInitializer{
162+
initializer := &fileDownloadInitializer{
158163
FilesInfos: fileInfos,
159164
TargetLocation: filepath.Join(loc, req.TargetLocation),
165+
HTTPClient: http.DefaultClient,
166+
RetryTimeout: 1 * time.Second,
160167
}
161168
return initializer, nil
162169
}

‎components/server/src/workspace/image-source-provider.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { ImageBuilderClientProvider, ResolveBaseImageRequest, BuildRegistryAuthT
99
import { HostContextProvider } from "../auth/host-context-provider";
1010
import { TraceContext } from "@gitpod/gitpod-protocol/lib/util/tracing";
1111
import { CommitContext, WorkspaceImageSource, WorkspaceConfig, WorkspaceImageSourceReference, WorkspaceImageSourceDocker, ImageConfigFile, ExternalImageConfigFile, User, AdditionalContentContext } from "@gitpod/gitpod-protocol";
12-
import { createHmac } from 'crypto';
12+
import { createHash } from 'crypto';
1313

1414
@injectable()
1515
export class ImageSourceProvider {
@@ -93,9 +93,7 @@ export class ImageSourceProvider {
9393
}
9494

9595
protected getContentSHA(contents: string): string {
96-
return createHmac('sha256', '')
97-
.update(contents)
98-
.digest('hex');
96+
return createHash('sha256').update(contents).digest('hex');
9997
}
10098

10199

0 commit comments

Comments
 (0)