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

fix: kurtosis run considers every nonexistent path to be a URL and fails with a suspicious error #1706

Merged
merged 1 commit into from Nov 7, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 20 additions & 13 deletions cli/cli/commands/run/run.go
Expand Up @@ -12,6 +12,7 @@ import (
"os/signal"
"path"
"path/filepath"
"regexp"
"strings"

"github.com/kurtosis-tech/kurtosis/api/golang/core/lib/starlark_run_config"
Expand Down Expand Up @@ -104,6 +105,8 @@ const (

imageDownloadFlagKey = "image-download"
defaultImageDownload = "missing"

httpProtocolRegexStr = "^(http|https)://"
)

var StarlarkRunCmd = &engine_consuming_kurtosis_command.EngineConsumingKurtosisCommand{
Expand Down Expand Up @@ -706,20 +709,8 @@ func validateSerializedArgs(serializedArgs string) error {

func getArgsFromFilepathOrURL(packageArgsFile string) (string, error) {
var packageArgsFileBytes []byte
isFileURL := true
_, err := os.Stat(packageArgsFile)
if err == nil {
isFileURL = false
packageArgsFileBytes, err = os.ReadFile(packageArgsFile)
if err != nil {
return "", stacktrace.Propagate(err, "attempted to read file provided by flag '%v' with path '%v' but failed", packageArgsFileFlagKey, packageArgsFile)
}
}
if err != nil && !os.IsNotExist(err) {
return "", stacktrace.Propagate(err, "An error occurred checking for argument's file existence on '%s'", packageArgsFile)
}

if isFileURL {
if isHttpUrl(packageArgsFile) {
argsFileURL, parseErr := url.Parse(packageArgsFile)
if parseErr != nil {
return "", stacktrace.Propagate(parseErr, "An error occurred while parsing file args URL '%s'", argsFileURL)
Expand All @@ -734,6 +725,16 @@ func getArgsFromFilepathOrURL(packageArgsFile string) (string, error) {
return "", stacktrace.Propagate(readAllErr, "An error occurred reading the args file content")
}
packageArgsFileBytes = responseBodyBytes
} else {
_, err := os.Stat(packageArgsFile)
if err != nil {
return "", stacktrace.Propagate(err, "An error occurred checking for argument's file existence on '%s'", packageArgsFile)
}

packageArgsFileBytes, err = os.ReadFile(packageArgsFile)
if err != nil {
return "", stacktrace.Propagate(err, "attempted to read file provided by flag '%v' with path '%v' but failed", packageArgsFileFlagKey, packageArgsFile)
}
}

packageArgsFileStr := string(packageArgsFileBytes)
Expand All @@ -742,4 +743,10 @@ func getArgsFromFilepathOrURL(packageArgsFile string) (string, error) {
}

return packageArgsFileStr, nil

}

func isHttpUrl(maybeHttpUrl string) bool {
httpProtocolRegex := regexp.MustCompile(httpProtocolRegexStr)
return httpProtocolRegex.MatchString(maybeHttpUrl)
}
32 changes: 32 additions & 0 deletions cli/cli/commands/run/run_test.go
Expand Up @@ -42,3 +42,35 @@ func TestValidateArgs_invalid(t *testing.T) {
err = validatePackageArgs(testCtx, testParsedFlags, parsedArgs)
require.NotNil(t, err)
}

func TestIsHttpUrl_ValidHTTP(t *testing.T) {
fileUrl := "http://www.mysite.com/myfile.json"

isHttpUrlResult := isHttpUrl(fileUrl)

require.True(t, isHttpUrlResult)
}

func TestIsHttpUrl_ValidHTTPS(t *testing.T) {
fileUrl := "https://www.mysite.com/myfile.json"

isHttpUrlResult := isHttpUrl(fileUrl)

require.True(t, isHttpUrlResult)
}

func TestIsHttpUrl_NoValidBecauseIsAbsoluteFilepath(t *testing.T) {
fileUrl := "/my-folder/myfile.json"

isHttpUrlResult := isHttpUrl(fileUrl)

require.False(t, isHttpUrlResult)
}

func TestIsHttpUrl_NoValidBecauseIsRelativeFilepath(t *testing.T) {
fileUrl := "../my-folder/myfile.json"

isHttpUrlResult := isHttpUrl(fileUrl)

require.False(t, isHttpUrlResult)
}