diff --git a/appconfig/appconfig.go b/appconfig/appconfig.go index dbdbf028e..8820cf882 100644 --- a/appconfig/appconfig.go +++ b/appconfig/appconfig.go @@ -267,56 +267,61 @@ func (ld *Loader) loadDefaultConfig(ctx context.Context, client *github.Client, // getFileContents returns the content of the file at path on ref in owner/repo // if it exists. Returns an empty slice and false if the file does not exist. func getFileContents(ctx context.Context, client *github.Client, owner, repo, ref, path string) ([]byte, bool, error) { + // GetContents returns encoded content for files < 1MB and a download URL + // for files between 1MB and 100MB. It returns an error for files >100MB, + // but if an app has a configuration file that large, there are probably + // other issues... file, _, _, err := client.Repositories.GetContents(ctx, owner, repo, path, &github.RepositoryContentGetOptions{ Ref: ref, }) if err != nil { - switch { - case isNotFound(err): + if isNotFound(err) { return nil, false, nil - case isTooLargeError(err): - b, err := getLargeFileContents(ctx, client, owner, repo, ref, path) - return b, true, err } return nil, false, errors.Wrap(err, "failed to read file") } - // file will be nil if the path exists but is a directory + // The file will be nil if the path exists but is a directory if file == nil { return nil, false, nil } + // If decoding the content fails, ignore the error and try the download URL + // instead. The most likely error is if the file is larger than 1MB. content, err := file.GetContent() - if err != nil { - return nil, true, errors.Wrap(err, "failed to decode file content") + if err == nil { + return []byte(content), true, nil } - return []byte(content), true, nil -} + downloadURL := file.GetDownloadURL() + if downloadURL == "" { + return nil, true, errors.New("download url is empty") + } -// getLargeFileContents is similar to getFileContents, but works for files up -// to 100MB. Unlike getFileContents, it returns an error if the file does not -// exist. -func getLargeFileContents(ctx context.Context, client *github.Client, owner, repo, ref, path string) ([]byte, error) { - body, res, err := client.Repositories.DownloadContents(ctx, owner, repo, path, &github.RepositoryContentGetOptions{ - Ref: ref, - }) + req, err := http.NewRequestWithContext(ctx, "GET", downloadURL, nil) + if err != nil { + return nil, true, errors.Wrap(err, "failed to create download request") + } + + res, err := client.Client().Do(req) if err != nil { - return nil, errors.Wrap(err, "failed to read file") + return nil, true, errors.Wrap(err, "failed to download file") } + defer func() { - _ = body.Close() + _, _ = io.Copy(io.Discard, res.Body) + _ = res.Body.Close() }() if res.StatusCode != http.StatusOK { - return nil, errors.Errorf("failed to read file: unexpected status code %d", res.StatusCode) + return nil, true, errors.Errorf("failed to download file: unexpected status code %d", res.StatusCode) } - b, err := io.ReadAll(body) + b, err := io.ReadAll(res.Body) if err != nil { - return nil, errors.Wrap(err, "failed to read file") + return nil, true, errors.Wrap(err, "failed to read file") } - return b, nil + return b, true, nil } func isNotFound(err error) bool { @@ -325,14 +330,3 @@ func isNotFound(err error) bool { } return false } - -func isTooLargeError(err error) bool { - if rerr, ok := err.(*github.ErrorResponse); ok { - for _, err := range rerr.Errors { - if err.Code == "too_large" { - return true - } - } - } - return false -} diff --git a/appconfig/appconfig_test.go b/appconfig/appconfig_test.go index 84eb77ad4..0ea8e29f6 100644 --- a/appconfig/appconfig_test.go +++ b/appconfig/appconfig_test.go @@ -149,7 +149,6 @@ func makeTestClient() *github.Client { "/repos/test/local-file/contents/.github/test-app.v2.yml": "404.yml", "/repos/test/local-file-large/contents/.github/test-app.yml": "local-file-large-contents.yml", - "/repos/test/local-file-large/contents/.github": "local-file-large-dir-contents.yml", "/test/local-file-large/develop/.github/test-app.yml": "local-file-large-download.yml", "/repos/test/remote-ref/contents/.github/test-app.yml": "remote-ref-contents.yml", diff --git a/appconfig/testdata/local-file-large-contents.yml b/appconfig/testdata/local-file-large-contents.yml index 3373cfe4c..97f1346d8 100644 --- a/appconfig/testdata/local-file-large-contents.yml +++ b/appconfig/testdata/local-file-large-contents.yml @@ -1,7 +1,11 @@ -- status: 422 +- status: 200 body: | { - "errors": [ - {"code": "too_large"} - ] + "name": "test-app.yml", + "path": ".github/test-app.yml", + "size": 15000000, + "download_url": "https://raw.githubusercontent.com/test/local-file-large/develop/.github/test-app.yml?token=downloadtoken", + "type": "file", + "content": "", + "encoding": "none" } diff --git a/appconfig/testdata/local-file-large-dir-contents.yml b/appconfig/testdata/local-file-large-dir-contents.yml deleted file mode 100644 index bf75a2275..000000000 --- a/appconfig/testdata/local-file-large-dir-contents.yml +++ /dev/null @@ -1,10 +0,0 @@ -- status: 200 - body: | - [ - { - "type": "file", - "name": "test-app.yml", - "path": ".github/test-app.yml", - "download_url": "https://raw.githubusercontent.com/test/local-file-large/develop/.github/test-app.yml" - } - ]