Skip to content

Commit

Permalink
Refactors IsSrcRemoteFileURL to only validate the URL is valid
Browse files Browse the repository at this point in the history
`IsSrcRemoteFileURL` was doing a `http.Get` call to make sure the URL was valid, but not surfacing any errors.
Because the error from the http.Get call is not handled, some useful information can be buried.
It also means kaniko will download the file twice during a build, once to validate, and once to actually add the file
to the image.
Removing the http.Get call and validating the URL is valid, and has the correct schema and hostname will stop
the double handling, and allow any errors to be surfaced through the error handing in the file download function.

Fixes #1590

Signed-off-by: Angus Williams <anguswilliams@gmail.com>
  • Loading branch information
anguswilliams committed Jun 13, 2023
1 parent 0790e8b commit 1e0ad79
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 7 deletions.
9 changes: 2 additions & 7 deletions pkg/util/command_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package util

import (
"fmt"
"net/http"
"net/url"
"os"
"os/user"
Expand Down Expand Up @@ -291,12 +290,8 @@ func IsSrcsValid(srcsAndDest instructions.SourcesAndDest, resolvedSources []stri
}

func IsSrcRemoteFileURL(rawurl string) bool {
_, err := url.ParseRequestURI(rawurl)
if err != nil {
return false
}
_, err = http.Get(rawurl) //nolint:noctx
return err == nil
u, err := url.ParseRequestURI(rawurl)
return err == nil && u.Scheme != "" && u.Host != ""
}

func UpdateConfigEnv(envVars []instructions.KeyValuePair, config *v1.Config, replacementEnvs []string) error {
Expand Down
41 changes: 41 additions & 0 deletions pkg/util/command_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -855,3 +855,44 @@ func TestLookupUser(t *testing.T) {
}

}

func TestIsSrcRemoteFileURL(t *testing.T) {
type args struct {
rawurl string
}
tests := []struct {
name string
args args
want bool
}{
{
name: "valid https url",
args: args{rawurl: "https://google.com?foo=bar"},
want: true,
},
{
name: "valid http url",
args: args{rawurl: "http://example.com/foobar.tar.gz"},
want: true,
},
{
name: "invalid url",
args: args{rawurl: "http:/not-a-url.com"},
want: false,
},
{
name: "invalid url filepath",
args: args{rawurl: "/is/a/filepath"},
want: false,
},
}
for _, tt := range tests {
t.Run(
tt.name, func(t *testing.T) {
if got := IsSrcRemoteFileURL(tt.args.rawurl); got != tt.want {
t.Errorf("IsSrcRemoteFileURL() = %v, want %v", got, tt.want)
}
},
)
}
}

0 comments on commit 1e0ad79

Please sign in to comment.