diff --git a/changelogs/unreleased/10258-Lyndon-Li b/changelogs/unreleased/10258-Lyndon-Li new file mode 100644 index 000000000..d73f8f4c6 --- /dev/null +++ b/changelogs/unreleased/10258-Lyndon-Li @@ -0,0 +1 @@ +Cap the unzip of metadata download to avoid OOM kill \ No newline at end of file diff --git a/pkg/cmd/util/downloadrequest/downloadrequest.go b/pkg/cmd/util/downloadrequest/downloadrequest.go index f0956b1cb..209b6c37c 100644 --- a/pkg/cmd/util/downloadrequest/downloadrequest.go +++ b/pkg/cmd/util/downloadrequest/downloadrequest.go @@ -40,6 +40,7 @@ import ( // not found var ErrNotFound = errors.New("file not found") var ErrDownloadRequestDownloadURLTimeout = errors.New("download request download url timeout, check velero server logs for errors. backup storage location may not be available") +var unzipLimit int64 = 1024 * 1024 * 1024 // 1GB limit func Stream( ctx context.Context, @@ -202,17 +203,35 @@ func download( return errors.Errorf("request failed: %v", string(body)) } - reader := resp.Body + var r io.Reader = resp.Body + var gzipReader *gzip.Reader if kind != veleroV1api.DownloadTargetKindBackupContents { // need to decompress logs - gzipReader, err := gzip.NewReader(resp.Body) + var err error + gzipReader, err = gzip.NewReader(resp.Body) if err != nil { return err } defer gzipReader.Close() - reader = gzipReader + + r = io.LimitReader(gzipReader, unzipLimit) } - _, err = io.Copy(w, reader) - return err + _, err = io.Copy(w, r) + if err != nil { + return err + } + + if gzipReader != nil { + var buf [1]byte + n, err := gzipReader.Read(buf[:]) + if n > 0 || err == nil { + return errors.Errorf("decompressed data exceeds the limit") + } + if err != io.EOF { + return err + } + } + + return nil } diff --git a/pkg/cmd/util/downloadrequest/downloadrequest_test.go b/pkg/cmd/util/downloadrequest/downloadrequest_test.go index 995e83dc6..36a02413a 100644 --- a/pkg/cmd/util/downloadrequest/downloadrequest_test.go +++ b/pkg/cmd/util/downloadrequest/downloadrequest_test.go @@ -463,6 +463,7 @@ func TestDownload(t *testing.T) { expectedContent string expectedError bool errorType error + expectedErrMsg string }{ { name: "successful download with gzip for logs", @@ -474,6 +475,16 @@ func TestDownload(t *testing.T) { expectedContent: testContent, expectedError: false, }, + { + name: "error decompressed data exceeds the limit", + serverHandler: func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write(compressedContent.Bytes()) + }, + target: velerov1api.DownloadTargetKindBackupLog, + expectedError: true, + expectedErrMsg: "decompressed data exceeds the limit", + }, { name: "successful download without gzip for backup contents", serverHandler: func(w http.ResponseWriter, r *http.Request) { @@ -506,6 +517,12 @@ func TestDownload(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + originalLimit := unzipLimit + if tc.expectedErrMsg == "decompressed data exceeds the limit" { + unzipLimit = 10 + } + defer func() { unzipLimit = originalLimit }() + server := httptest.NewServer(tc.serverHandler) defer server.Close() @@ -525,6 +542,9 @@ func TestDownload(t *testing.T) { if tc.errorType != nil { assert.Equal(t, tc.errorType, err) } + if tc.expectedErrMsg != "" { + assert.Contains(t, err.Error(), tc.expectedErrMsg) + } } else { require.NoError(t, err) assert.Equal(t, tc.expectedContent, buf.String())