Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor(x/upgrade): configure go-getter #18470

Merged
merged 5 commits into from
Nov 15, 2023
Merged

refactor(x/upgrade): configure go-getter #18470

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3940a38
refactor: configure go-getter
julienrbrt Nov 15, 2023
3e701ec
changelog
julienrbrt Nov 15, 2023
bb225e2
go mod tidy
julienrbrt Nov 15, 2023
418852d
fixes
julienrbrt Nov 15, 2023
8da3f39
Merge branch 'main' into julien/upgrades
julienrbrt Nov 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions x/upgrade/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ Ref: https://keepachangelog.com/en/1.0.0/

## [Unreleased]

### Improvements

* [#18470](https://github.com/cosmos/cosmos-sdk/pull/18470) Improve go-getter settings.

### State Machine Breaking

* (x/upgrade) [#16244](https://github.com/cosmos/cosmos-sdk/pull/16244) Upgrade module no longer stores the app version but gets and sets the app version stored in the `ParamStore` of baseapp.
Expand Down
2 changes: 1 addition & 1 deletion x/upgrade/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ require (
github.com/cosmos/gogoproto v1.4.11
github.com/golang/protobuf v1.5.3
github.com/grpc-ecosystem/grpc-gateway v1.16.0
github.com/hashicorp/go-cleanhttp v0.5.2
github.com/hashicorp/go-getter v1.7.3
github.com/hashicorp/go-metrics v0.5.2
github.com/spf13/cast v1.5.1
Expand Down Expand Up @@ -101,7 +102,6 @@ require (
github.com/gorilla/websocket v1.5.0 // indirect
github.com/grpc-ecosystem/go-grpc-middleware v1.4.0 // indirect
github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c // indirect
github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
github.com/hashicorp/go-hclog v1.5.0 // indirect
github.com/hashicorp/go-immutable-radix v1.3.1 // indirect
github.com/hashicorp/go-plugin v1.5.2 // indirect
Expand Down
32 changes: 30 additions & 2 deletions x/upgrade/plan/downloader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
package plan

import (
"context"
"errors"
"fmt"
neturl "net/url"
"os"
"path/filepath"
"strings"

"github.com/hashicorp/go-cleanhttp"
"github.com/hashicorp/go-getter"
)

Expand All @@ -24,8 +26,9 @@ import (
// NOTE: This functions does not check the provided url for validity.
func DownloadUpgrade(dstRoot, url, daemonName string) error {
target := filepath.Join(dstRoot, "bin", daemonName)

// First try to download it as a single file. If there's no error, it's okay and we're done.
if err := getter.GetFile(target, url); err != nil {
if err := getFile(url, target); err != nil {
// If it was a checksum error, no need to try as directory.
if _, ok := err.(*getter.ChecksumError); ok {
return err
Comment on lines 26 to 34
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment on line 26 should be updated to reflect the new behavior of the function, which now checks the validity of the URL using the getFile function. This is important for maintainability and clarity of the codebase.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's not what getFile does.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@julienrbrt You're correct, my apologies for the confusion. The getFile function indeed does not perform URL validation; it's responsible for downloading the file using a more secure and configured HTTP client. Thank you for pointing that out. I'll ensure to be more accurate in my future comments.

tac0turtle marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -109,7 +112,8 @@ func DownloadURL(url string) (string, error) {
}
defer os.RemoveAll(tempDir)
tempFile := filepath.Join(tempDir, "content")
if err = getter.GetFile(tempFile, url); err != nil {

if err := getFile(url, tempFile); err != nil {
return "", fmt.Errorf("could not download url \"%s\": %w", url, err)
}
tempFileBz, rerr := os.ReadFile(tempFile)
Expand All @@ -136,3 +140,27 @@ func ValidateURL(urlStr string, mustChecksum bool) error {

return nil
}

// getFile downloads the given url into the provided directory.
func getFile(url, dst string) error {
httpGetter := &getter.HttpGetter{
Client: cleanhttp.DefaultClient(),
XTerraformGetDisabled: true,
}

goGetterGetters := getter.Getters
goGetterGetters["http"] = httpGetter
goGetterGetters["https"] = httpGetter

// https://github.com/hashicorp/go-getter#security-options
getterClient := &getter.Client{
Ctx: context.Background(),
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved
DisableSymlinks: true,
Src: url,
Dst: dst,
Pwd: dst,
Getters: goGetterGetters,
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved
}

return getterClient.Get()
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved
}
Loading