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.

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

commonsteps: StepDownload timeout override #272

Merged
merged 1 commit into from
Jan 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
83 changes: 51 additions & 32 deletions multistep/commonsteps/step_download.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,40 +64,59 @@ type StepDownload struct {
// The timeout must be long enough to accommodate large/slow downloads.
const defaultGetterReadTimeout time.Duration = 30 * time.Minute

var defaultGetterClient = getter.Client{
// Disable writing and reading through symlinks.
DisableSymlinks: true,
// The order of the Getters in the list may affect the result
// depending if the Request.Src is detected as valid by multiple getters
Getters: []getter.Getter{
&getter.GitGetter{
Timeout: defaultGetterReadTimeout,
Detectors: []getter.Detector{
new(getter.GitHubDetector),
new(getter.GitDetector),
new(getter.BitBucketDetector),
new(getter.GitLabDetector),
var getterReadTimeout = defaultGetterReadTimeout
var defaultGetterClient = getter.Client{}

func init() {
prepareGetterClient()
}

// prepareGetterClient is run in the init function but is actually extracted
// to make testing easier by providing a way to re prepare the client
func prepareGetterClient() {
mogrogan marked this conversation as resolved.
Show resolved Hide resolved
getterReadTimeout := defaultGetterReadTimeout
if env, exists := os.LookupEnv("PACKER_GETTER_READ_TIMEOUT"); exists {
Copy link
Contributor

Choose a reason for hiding this comment

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

Note to myself: we'll need to add that new envvar to the docs somewhere

parsedDuration, err := time.ParseDuration(env)
if err != nil {
panic(err)
}
getterReadTimeout = parsedDuration
}
defaultGetterClient = getter.Client{
// Disable writing and reading through symlinks.
DisableSymlinks: true,
// The order of the Getters in the list may affect the result
// depending if the Request.Src is detected as valid by multiple getters
Getters: []getter.Getter{
&getter.GitGetter{
Timeout: getterReadTimeout,
Detectors: []getter.Detector{
new(getter.GitHubDetector),
new(getter.GitDetector),
new(getter.BitBucketDetector),
new(getter.GitLabDetector),
},
},
&getter.HgGetter{
Timeout: getterReadTimeout,
},
new(getter.SmbClientGetter),
new(getter.SmbMountGetter),
&getter.HttpGetter{
Netrc: true,
XTerraformGetDisabled: true,
HeadFirstTimeout: getterReadTimeout,
ReadTimeout: getterReadTimeout,
},
new(getter.FileGetter),
&gcs.Getter{
Timeout: getterReadTimeout,
},
&s3.Getter{
Timeout: getterReadTimeout,
},
},
&getter.HgGetter{
Timeout: defaultGetterReadTimeout,
},
new(getter.SmbClientGetter),
new(getter.SmbMountGetter),
&getter.HttpGetter{
Netrc: true,
XTerraformGetDisabled: true,
HeadFirstTimeout: defaultGetterReadTimeout,
ReadTimeout: defaultGetterReadTimeout,
},
new(getter.FileGetter),
&gcs.Getter{
Timeout: defaultGetterReadTimeout,
},
&s3.Getter{
Timeout: defaultGetterReadTimeout,
},
},
}
}

func (s *StepDownload) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction {
Expand Down
46 changes: 46 additions & 0 deletions multistep/commonsteps/step_download_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ import (
"path/filepath"
"reflect"
"runtime"
"strings"
"testing"
"time"

"github.com/google/go-cmp/cmp"
urlhelper "github.com/hashicorp/go-getter/v2/helper/url"
Expand Down Expand Up @@ -287,6 +289,50 @@ func TestStepDownload_download(t *testing.T) {
os.RemoveAll(step.TargetPath)
}

func TestStepDownload_short_timeout(t *testing.T) {
if runtime.GOOS == "windows" {
t.Log("skipping download test on windows right now.")
return
}

os.Setenv("PACKER_GETTER_READ_TIMEOUT", "1ns")
prepareGetterClient()

// we need to re prepare the getterClient with default behavior once the test
// is completed
defer prepareGetterClient()
mogrogan marked this conversation as resolved.
Show resolved Hide resolved
defer os.Unsetenv("PACKER_GETTER_READ_TIMEOUT")

srvr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
<-time.After(time.Microsecond * 100)
w.Write([]byte("should not receive this"))
}))

defer srvr.Close()
step := &StepDownload{
Checksum: "sha1:f572d396fae9206628714fb2ce00f72e94f2258f",
Description: "ISO",
ResultKey: "iso_path",
Url: nil,
}
ui := &packersdk.BasicUi{
Reader: new(bytes.Buffer),
Writer: new(bytes.Buffer),
PB: &packersdk.NoopProgressTracker{},
}

_, err := step.download(context.TODO(), ui, srvr.URL+"/root/another.txt?")

contextDeadlineErrorMsg := "context deadline exceeded"
if err == nil {
t.Fatalf("Bad: expected a '%s' error", contextDeadlineErrorMsg)
}
if !strings.Contains(err.Error(), contextDeadlineErrorMsg) {
t.Fatalf("Bad: expected a '%s' error, got %s", contextDeadlineErrorMsg, err.Error())
}

}

func TestStepDownload_WindowsParseSourceURL(t *testing.T) {
if runtime.GOOS != "windows" {
t.Skip("skip windows specific tests")
Expand Down
Loading