mirror of https://github.com/knative/pkg.git
Change URIFromDestinationV1 to return apis.URL instead of string (#909)
This commit is contained in:
Ville Aikas
Knative Prow Robot
parent
7fbd5952bb
commit
cac31abb7f
17
apis/url.go
17
apis/url.go
|
|
@ -109,3 +109,20 @@ func (u *URL) URL() *url.URL {
|
|||
url := url.URL(*u)
|
||||
return &url
|
||||
}
|
||||
|
||||
// ResolveReference calls the underlying ResolveReference method
|
||||
// and returns an apis.URL
|
||||
func (u *URL) ResolveReference(ref *URL) *URL {
|
||||
if ref == nil {
|
||||
return u
|
||||
}
|
||||
// Turn both u / ref to url.URL
|
||||
uRef := url.URL(*ref)
|
||||
uu := url.URL(*u)
|
||||
|
||||
newU := uu.ResolveReference(&uRef)
|
||||
|
||||
// Turn new back to apis.URL
|
||||
ret := URL(*newU)
|
||||
return &ret
|
||||
}
|
||||
|
|
|
|||
143
apis/url_test.go
143
apis/url_test.go
|
|
@ -460,3 +460,146 @@ func TestURLString(t *testing.T) {
|
|||
})
|
||||
}
|
||||
}
|
||||
|
||||
// These are lifted from the net/url url_test.go
|
||||
var resolveReferenceTests = []struct {
|
||||
base, rel, expected string
|
||||
}{
|
||||
// Absolute URL references
|
||||
{"http://foo.com?a=b", "https://bar.com/", "https://bar.com/"},
|
||||
{"http://foo.com/", "https://bar.com/?a=b", "https://bar.com/?a=b"},
|
||||
{"http://foo.com/", "https://bar.com/?", "https://bar.com/?"},
|
||||
{"http://foo.com/bar", "mailto:foo@example.com", "mailto:foo@example.com"},
|
||||
|
||||
// Path-absolute references
|
||||
{"http://foo.com/bar", "/baz", "http://foo.com/baz"},
|
||||
{"http://foo.com/bar?a=b#f", "/baz", "http://foo.com/baz"},
|
||||
{"http://foo.com/bar?a=b", "/baz?", "http://foo.com/baz?"},
|
||||
{"http://foo.com/bar?a=b", "/baz?c=d", "http://foo.com/baz?c=d"},
|
||||
|
||||
// Multiple slashes
|
||||
{"http://foo.com/bar", "http://foo.com//baz", "http://foo.com//baz"},
|
||||
{"http://foo.com/bar", "http://foo.com///baz/quux", "http://foo.com///baz/quux"},
|
||||
|
||||
// Scheme-relative
|
||||
{"https://foo.com/bar?a=b", "//bar.com/quux", "https://bar.com/quux"},
|
||||
|
||||
// Path-relative references:
|
||||
|
||||
// ... current directory
|
||||
{"http://foo.com", ".", "http://foo.com/"},
|
||||
{"http://foo.com/bar", ".", "http://foo.com/"},
|
||||
{"http://foo.com/bar/", ".", "http://foo.com/bar/"},
|
||||
|
||||
// ... going down
|
||||
{"http://foo.com", "bar", "http://foo.com/bar"},
|
||||
{"http://foo.com/", "bar", "http://foo.com/bar"},
|
||||
{"http://foo.com/bar/baz", "quux", "http://foo.com/bar/quux"},
|
||||
|
||||
// ... going up
|
||||
{"http://foo.com/bar/baz", "../quux", "http://foo.com/quux"},
|
||||
{"http://foo.com/bar/baz", "../../../../../quux", "http://foo.com/quux"},
|
||||
{"http://foo.com/bar", "..", "http://foo.com/"},
|
||||
{"http://foo.com/bar/baz", "./..", "http://foo.com/"},
|
||||
// ".." in the middle (issue 3560)
|
||||
{"http://foo.com/bar/baz", "quux/dotdot/../tail", "http://foo.com/bar/quux/tail"},
|
||||
{"http://foo.com/bar/baz", "quux/./dotdot/../tail", "http://foo.com/bar/quux/tail"},
|
||||
{"http://foo.com/bar/baz", "quux/./dotdot/.././tail", "http://foo.com/bar/quux/tail"},
|
||||
{"http://foo.com/bar/baz", "quux/./dotdot/./../tail", "http://foo.com/bar/quux/tail"},
|
||||
{"http://foo.com/bar/baz", "quux/./dotdot/dotdot/././../../tail", "http://foo.com/bar/quux/tail"},
|
||||
{"http://foo.com/bar/baz", "quux/./dotdot/dotdot/./.././../tail", "http://foo.com/bar/quux/tail"},
|
||||
{"http://foo.com/bar/baz", "quux/./dotdot/dotdot/dotdot/./../../.././././tail", "http://foo.com/bar/quux/tail"},
|
||||
{"http://foo.com/bar/baz", "quux/./dotdot/../dotdot/../dot/./tail/..", "http://foo.com/bar/quux/dot/"},
|
||||
|
||||
// Remove any dot-segments prior to forming the target URI.
|
||||
// http://tools.ietf.org/html/rfc3986#section-5.2.4
|
||||
{"http://foo.com/dot/./dotdot/../foo/bar", "../baz", "http://foo.com/dot/baz"},
|
||||
|
||||
// Triple dot isn't special
|
||||
{"http://foo.com/bar", "...", "http://foo.com/..."},
|
||||
|
||||
// Fragment
|
||||
{"http://foo.com/bar", ".#frag", "http://foo.com/#frag"},
|
||||
{"http://example.org/", "#!$&%27()*+,;=", "http://example.org/#!$&%27()*+,;="},
|
||||
|
||||
// Paths with escaping (issue 16947).
|
||||
{"http://foo.com/foo%2fbar/", "../baz", "http://foo.com/baz"},
|
||||
{"http://foo.com/1/2%2f/3%2f4/5", "../../a/b/c", "http://foo.com/1/a/b/c"},
|
||||
{"http://foo.com/1/2/3", "./a%2f../../b/..%2fc", "http://foo.com/1/2/b/..%2fc"},
|
||||
{"http://foo.com/1/2%2f/3%2f4/5", "./a%2f../b/../c", "http://foo.com/1/2%2f/3%2f4/a%2f../c"},
|
||||
{"http://foo.com/foo%20bar/", "../baz", "http://foo.com/baz"},
|
||||
{"http://foo.com/foo", "../bar%2fbaz", "http://foo.com/bar%2fbaz"},
|
||||
{"http://foo.com/foo%2dbar/", "./baz-quux", "http://foo.com/foo%2dbar/baz-quux"},
|
||||
|
||||
// RFC 3986: Normal Examples
|
||||
// http://tools.ietf.org/html/rfc3986#section-5.4.1
|
||||
{"http://a/b/c/d;p?q", "g:h", "g:h"},
|
||||
{"http://a/b/c/d;p?q", "g", "http://a/b/c/g"},
|
||||
{"http://a/b/c/d;p?q", "./g", "http://a/b/c/g"},
|
||||
{"http://a/b/c/d;p?q", "g/", "http://a/b/c/g/"},
|
||||
{"http://a/b/c/d;p?q", "/g", "http://a/g"},
|
||||
{"http://a/b/c/d;p?q", "//g", "http://g"},
|
||||
{"http://a/b/c/d;p?q", "?y", "http://a/b/c/d;p?y"},
|
||||
{"http://a/b/c/d;p?q", "g?y", "http://a/b/c/g?y"},
|
||||
{"http://a/b/c/d;p?q", "#s", "http://a/b/c/d;p?q#s"},
|
||||
{"http://a/b/c/d;p?q", "g#s", "http://a/b/c/g#s"},
|
||||
{"http://a/b/c/d;p?q", "g?y#s", "http://a/b/c/g?y#s"},
|
||||
{"http://a/b/c/d;p?q", ";x", "http://a/b/c/;x"},
|
||||
{"http://a/b/c/d;p?q", "g;x", "http://a/b/c/g;x"},
|
||||
{"http://a/b/c/d;p?q", "g;x?y#s", "http://a/b/c/g;x?y#s"},
|
||||
{"http://a/b/c/d;p?q", "", "http://a/b/c/d;p?q"},
|
||||
{"http://a/b/c/d;p?q", ".", "http://a/b/c/"},
|
||||
{"http://a/b/c/d;p?q", "./", "http://a/b/c/"},
|
||||
{"http://a/b/c/d;p?q", "..", "http://a/b/"},
|
||||
{"http://a/b/c/d;p?q", "../", "http://a/b/"},
|
||||
{"http://a/b/c/d;p?q", "../g", "http://a/b/g"},
|
||||
{"http://a/b/c/d;p?q", "../..", "http://a/"},
|
||||
{"http://a/b/c/d;p?q", "../../", "http://a/"},
|
||||
{"http://a/b/c/d;p?q", "../../g", "http://a/g"},
|
||||
|
||||
// RFC 3986: Abnormal Examples
|
||||
// http://tools.ietf.org/html/rfc3986#section-5.4.2
|
||||
{"http://a/b/c/d;p?q", "../../../g", "http://a/g"},
|
||||
{"http://a/b/c/d;p?q", "../../../../g", "http://a/g"},
|
||||
{"http://a/b/c/d;p?q", "/./g", "http://a/g"},
|
||||
{"http://a/b/c/d;p?q", "/../g", "http://a/g"},
|
||||
{"http://a/b/c/d;p?q", "g.", "http://a/b/c/g."},
|
||||
{"http://a/b/c/d;p?q", ".g", "http://a/b/c/.g"},
|
||||
{"http://a/b/c/d;p?q", "g..", "http://a/b/c/g.."},
|
||||
{"http://a/b/c/d;p?q", "..g", "http://a/b/c/..g"},
|
||||
{"http://a/b/c/d;p?q", "./../g", "http://a/b/g"},
|
||||
{"http://a/b/c/d;p?q", "./g/.", "http://a/b/c/g/"},
|
||||
{"http://a/b/c/d;p?q", "g/./h", "http://a/b/c/g/h"},
|
||||
{"http://a/b/c/d;p?q", "g/../h", "http://a/b/c/h"},
|
||||
{"http://a/b/c/d;p?q", "g;x=1/./y", "http://a/b/c/g;x=1/y"},
|
||||
{"http://a/b/c/d;p?q", "g;x=1/../y", "http://a/b/c/y"},
|
||||
{"http://a/b/c/d;p?q", "g?y/./x", "http://a/b/c/g?y/./x"},
|
||||
{"http://a/b/c/d;p?q", "g?y/../x", "http://a/b/c/g?y/../x"},
|
||||
{"http://a/b/c/d;p?q", "g#s/./x", "http://a/b/c/g#s/./x"},
|
||||
{"http://a/b/c/d;p?q", "g#s/../x", "http://a/b/c/g#s/../x"},
|
||||
|
||||
// Extras.
|
||||
{"https://a/b/c/d;p?q", "//g?q", "https://g?q"},
|
||||
{"https://a/b/c/d;p?q", "//g#s", "https://g#s"},
|
||||
{"https://a/b/c/d;p?q", "//g/d/e/f?y#s", "https://g/d/e/f?y#s"},
|
||||
{"https://a/b/c/d;p#s", "?y", "https://a/b/c/d;p?y"},
|
||||
{"https://a/b/c/d;p?q#s", "?y", "https://a/b/c/d;p?y"},
|
||||
}
|
||||
|
||||
func TestResolveReference(t *testing.T) {
|
||||
mustParse := func(url string) *URL {
|
||||
u, err := ParseURL(url)
|
||||
if err != nil {
|
||||
t.Fatalf("Parse(%q) got err %v", url, err)
|
||||
}
|
||||
return u
|
||||
}
|
||||
|
||||
for _, tc := range resolveReferenceTests {
|
||||
apisURL := mustParse(tc.base)
|
||||
apisRel := mustParse(tc.rel)
|
||||
if got := apisURL.ResolveReference(apisRel).String(); got != tc.expected {
|
||||
t.Errorf("URL(%q).ResolveReference(%q)\ngot %q\nwant %q", tc.base, tc.rel, got, tc.expected)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -89,7 +89,7 @@ func (r *URIResolver) URIFromDestination(dest duckv1beta1.Destination, parent in
|
|||
if dest.URI.URL().IsAbs() {
|
||||
return "", errors.New("absolute URI is not allowed when Ref or [apiVersion, kind, name] exists")
|
||||
}
|
||||
return url.URL().ResolveReference(dest.URI.URL()).String(), nil
|
||||
return url.ResolveReference(dest.URI).String(), nil
|
||||
}
|
||||
return url.URL().String(), nil
|
||||
}
|
||||
|
|
@ -97,7 +97,7 @@ func (r *URIResolver) URIFromDestination(dest duckv1beta1.Destination, parent in
|
|||
if dest.URI != nil {
|
||||
// IsAbs check whether the URL has a non-empty scheme. Besides the non non-empty scheme, we also require dest.URI has a non-empty host
|
||||
if !dest.URI.URL().IsAbs() || dest.URI.Host == "" {
|
||||
return "", fmt.Errorf("URI is not absolute(both scheme and host should be non-empty): %v", dest.URI.String())
|
||||
return "", fmt.Errorf("URI is not absolute(both scheme and host should be non-empty): %q", dest.URI.String())
|
||||
}
|
||||
return dest.URI.String(), nil
|
||||
}
|
||||
|
|
@ -105,31 +105,31 @@ func (r *URIResolver) URIFromDestination(dest duckv1beta1.Destination, parent in
|
|||
return "", errors.New("destination missing Ref, [apiVersion, kind, name] and URI, expected at least one")
|
||||
}
|
||||
|
||||
// URIFromDestinationV1 resolves a v1.Destination into a URI string.
|
||||
func (r *URIResolver) URIFromDestinationV1(dest duckv1.Destination, parent interface{}) (string, error) {
|
||||
// URIFromDestinationV1 resolves a v1.Destination into a URL.
|
||||
func (r *URIResolver) URIFromDestinationV1(dest duckv1.Destination, parent interface{}) (*apis.URL, error) {
|
||||
if dest.Ref != nil {
|
||||
url, err := r.URIFromObjectReference(dest.Ref, parent)
|
||||
if err != nil {
|
||||
return "", err
|
||||
return nil, err
|
||||
}
|
||||
if dest.URI != nil {
|
||||
if dest.URI.URL().IsAbs() {
|
||||
return "", errors.New("absolute URI is not allowed when Ref or [apiVersion, kind, name] exists")
|
||||
return nil, errors.New("absolute URI is not allowed when Ref or [apiVersion, kind, name] exists")
|
||||
}
|
||||
return url.URL().ResolveReference(dest.URI.URL()).String(), nil
|
||||
return url.ResolveReference(dest.URI), nil
|
||||
}
|
||||
return url.URL().String(), nil
|
||||
return url, nil
|
||||
}
|
||||
|
||||
if dest.URI != nil {
|
||||
// IsAbs check whether the URL has a non-empty scheme. Besides the non non-empty scheme, we also require dest.URI has a non-empty host
|
||||
if !dest.URI.URL().IsAbs() || dest.URI.Host == "" {
|
||||
return "", fmt.Errorf("URI is not absolute(both scheme and host should be non-empty): %v", dest.URI.String())
|
||||
return nil, fmt.Errorf("URI is not absolute(both scheme and host should be non-empty): %q", dest.URI.String())
|
||||
}
|
||||
return dest.URI.String(), nil
|
||||
return dest.URI, nil
|
||||
}
|
||||
|
||||
return "", errors.New("destination missing Ref and URI, expected at least one")
|
||||
return nil, errors.New("destination missing Ref and URI, expected at least one")
|
||||
}
|
||||
|
||||
// URIFromObjectReference resolves an ObjectReference to a URI string.
|
||||
|
|
|
|||
|
|
@ -82,14 +82,14 @@ func TestGetURI_DestinationV1Beta1(t *testing.T) {
|
|||
Host: "example.com",
|
||||
},
|
||||
},
|
||||
wantErr: fmt.Sprintf("URI is not absolute(both scheme and host should be non-empty): %v", "//example.com"),
|
||||
wantErr: fmt.Sprintf("URI is not absolute(both scheme and host should be non-empty): %q", "//example.com"),
|
||||
}, "URI with no host": {
|
||||
dest: duckv1beta1.Destination{
|
||||
URI: &apis.URL{
|
||||
Scheme: "http",
|
||||
},
|
||||
},
|
||||
wantErr: fmt.Sprintf("URI is not absolute(both scheme and host should be non-empty): %v", "http:"),
|
||||
wantErr: fmt.Sprintf("URI is not absolute(both scheme and host should be non-empty): %q", "http:"),
|
||||
},
|
||||
"Ref and [apiVersion, kind, name] both exists": {
|
||||
objects: []runtime.Object{
|
||||
|
|
@ -382,14 +382,14 @@ func TestGetURI_DestinationV1(t *testing.T) {
|
|||
Host: "example.com",
|
||||
},
|
||||
},
|
||||
wantErr: fmt.Sprintf("URI is not absolute(both scheme and host should be non-empty): %v", "//example.com"),
|
||||
wantErr: fmt.Sprintf("URI is not absolute(both scheme and host should be non-empty): %q", "//example.com"),
|
||||
}, "URI with no host": {
|
||||
dest: duckv1.Destination{
|
||||
URI: &apis.URL{
|
||||
Scheme: "http",
|
||||
},
|
||||
},
|
||||
wantErr: fmt.Sprintf("URI is not absolute(both scheme and host should be non-empty): %v", "http:"),
|
||||
wantErr: fmt.Sprintf("URI is not absolute(both scheme and host should be non-empty): %q", "http:"),
|
||||
},
|
||||
"happy ref": {
|
||||
objects: []runtime.Object{
|
||||
|
|
@ -533,7 +533,7 @@ func TestGetURI_DestinationV1(t *testing.T) {
|
|||
t.Errorf("unexpected error: %v", gotErr.Error())
|
||||
}
|
||||
}
|
||||
if diff := cmp.Diff(tc.wantURI, uri); diff != "" {
|
||||
if diff := cmp.Diff(tc.wantURI, uri.String()); diff != "" {
|
||||
t.Errorf("unexpected object (-want, +got) = %v", diff)
|
||||
}
|
||||
})
|
||||
|
|
|
|||
Loading…
Reference in New Issue