Skip to content

Commit

Permalink
Split HTTPGetAction.Path into path and query (#14273)
Browse files Browse the repository at this point in the history
* Split config.Path into path and query

* Add path and query test

* use Go's code

* use url.Parse
  • Loading branch information
nak3 authored Aug 30, 2023
1 parent cea3201 commit 358ec13
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 9 deletions.
18 changes: 10 additions & 8 deletions pkg/queue/health/probe.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,8 @@ var transport = func() *http.Transport {
return t
}()

func getURL(config HTTPProbeConfigOptions) url.URL {
return url.URL{
Scheme: string(config.Scheme),
Host: net.JoinHostPort(config.Host, config.Port.String()),
Path: config.Path,
}
func getURL(config HTTPProbeConfigOptions) (*url.URL, error) {
return url.Parse(string(config.Scheme) + "://" + net.JoinHostPort(config.Host, config.Port.String()) + config.Path)
}

// http2UpgradeProbe checks that an HTTP with HTTP2 upgrade request
Expand All @@ -107,7 +103,10 @@ func http2UpgradeProbe(config HTTPProbeConfigOptions) (int, error) {
Transport: transport,
Timeout: config.Timeout,
}
url := getURL(config)
url, err := getURL(config)
if err != nil {
return 0, fmt.Errorf("error constructing probe url %w", err)
}
req, err := http.NewRequest(http.MethodOptions, url.String(), nil)
if err != nil {
return 0, fmt.Errorf("error constructing probe request %w", err)
Expand Down Expand Up @@ -161,7 +160,10 @@ func HTTPProbe(config HTTPProbeConfigOptions) error {
Transport: autoDowngradingTransport(config),
Timeout: config.Timeout,
}
url := getURL(config)
url, err := getURL(config)
if err != nil {
return fmt.Errorf("error constructing probe url %w", err)
}
req, err := http.NewRequest(http.MethodGet, url.String(), nil)
if err != nil {
return fmt.Errorf("error constructing probe request %w", err)
Expand Down
9 changes: 8 additions & 1 deletion pkg/queue/health/probe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ func TestHTTPProbeSuccess(t *testing.T) {
Value: "Testval",
}
var gotPath string
var gotQuery string
const expectedPath = "/health"
const expectedQuery = "foo=bar"
const configPath = expectedPath + "?" + expectedQuery
server := newTestServer(t, func(w http.ResponseWriter, r *http.Request) {
if v := r.Header.Get(expectedHeader.Name); v != "" {
gotHeader = corev1.HTTPHeader{Name: expectedHeader.Name, Value: v}
Expand All @@ -72,11 +75,12 @@ func TestHTTPProbeSuccess(t *testing.T) {
gotKubeletHeader = true
}
gotPath = r.URL.Path
gotQuery = r.URL.RawQuery
w.WriteHeader(http.StatusOK)
})

action := newHTTPGetAction(t, server.URL)
action.Path = expectedPath
action.Path = configPath
action.HTTPHeaders = []corev1.HTTPHeader{expectedHeader}

config := HTTPProbeConfigOptions{
Expand All @@ -98,6 +102,9 @@ func TestHTTPProbeSuccess(t *testing.T) {
if !cmp.Equal(gotPath, expectedPath) {
t.Errorf("Path = %s, want: %s", gotPath, expectedPath)
}
if !cmp.Equal(gotQuery, expectedQuery) {
t.Errorf("Query = %s, want: %s", gotQuery, expectedQuery)
}
// Close the server so probing fails afterwards.
server.Close()
if err := HTTPProbe(config); err == nil {
Expand Down
43 changes: 43 additions & 0 deletions test/e2e/readinessport_test.go → test/e2e/readiness_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,3 +80,46 @@ func TestReadinessAlternatePort(t *testing.T) {
}

}

func TestReadinessPathAndQuery(t *testing.T) {
t.Parallel()

clients := Setup(t)

names := test.ResourceNames{
Service: test.ObjectNameForTest(t),
Image: test.Readiness,
}

test.EnsureTearDown(t, clients, &names)

t.Log("Creating a new Service")

resources, err := v1test.CreateServiceReady(t, clients, &names,
v1opts.WithReadinessProbe(
&corev1.Probe{
ProbeHandler: corev1.ProbeHandler{
HTTPGet: &corev1.HTTPGetAction{
Path: "/query?probe=ok",
},
},
}))
if err != nil {
t.Fatalf("Failed to create initial Service: %v: %v", names.Service, err)
}

url := resources.Route.Status.URL.URL()
if _, err := pkgTest.CheckEndpointState(
context.Background(),
clients.KubeClient,
t.Logf,
url,
spoof.MatchesAllOf(spoof.IsStatusOK, spoof.MatchesBody(test.HelloWorldText)),
"HelloWorldServesText",
test.ServingFlags.ResolvableDomain,
test.AddRootCAtoTransport(context.Background(), t.Logf, clients, test.ServingFlags.HTTPS),
); err != nil {
t.Fatalf("The endpoint %s for Route %s didn't serve the expected text %q: %v", url, names.Route, test.HelloWorldText, err)
}

}
8 changes: 8 additions & 0 deletions test/test_images/readiness/readiness.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ func main() {
mainServer.HandleFunc("/healthz", handleHealthz)
}

mainServer.HandleFunc("/query", handleQuery)
http.ListenAndServe(":8080", mainServer)
}

Expand Down Expand Up @@ -118,6 +119,13 @@ func handleHealthz(w http.ResponseWriter, r *http.Request) {
fmt.Fprint(w, test.HelloWorldText)
}

func handleQuery(w http.ResponseWriter, r *http.Request) {
if r.URL.Query().Get("probe") == "ok" {
fmt.Fprint(w, test.HelloWorldText)
}
http.Error(w, "no query", http.StatusInternalServerError)
}

func handleMain(w http.ResponseWriter, r *http.Request) {
fmt.Fprint(w, test.HelloWorldText)
}

0 comments on commit 358ec13

Please sign in to comment.