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

[backend] health service response should be consumed by application/json not text/plain #7525

Closed
hsinhoyeh opened this issue Apr 6, 2022 · 2 comments

Comments

@hsinhoyeh
Copy link
Contributor

Environment

  • How did you deploy Kubeflow Pipelines (KFP)? install via kubeflow manifest with version v1.4
  • KFP version: v1.7.0
  • KFP SDK version: v1.7.0 (golang version under backend/api/go_http_client)

Steps to reproduce

  1. start pipeline backend api server by listening port 8888
  2. sent http request with swagger generated code stub with the following code example

package main

import (
	"github.com/go-openapi/strfmt"
	"github.com/go-openapi/runtime"
	healthz_client "github.com/kubeflow/pipelines/backend/api/go_http_client/healthz_client"
	healthz_service "github.com/kubeflow/pipelines/backend/api/go_http_client/healthz_client/healthz_service"
)

func main() {
	healthServiceClient := healthz_client.New(
		client.New(
			"localhost:8888",
			healthz_client.DefaultBasePath,
			[]string{"http"}),
		strfmt.NewFormats(),
	)
	
	clientAuthInfoWriter := runtime.ClientAuthInfoWriterFunc(func(clientRequest runtime.ClientRequest, registry strfmt.Registry) error {
		return nil
	})
	
	resp, err := p.HealthServiceClient.HealthzService.GetHealthz(healthz_service.NewGetHealthzParams, clientAuthInfoWriter)
        if err != nil {
            // should not be here
        }
}


  1. When I was sent healthz endpoint (/apis/v1beta1/healthz) with go_http_client(swagger generated code stubs), the generated code stub failed to decode the response with the error message &{false} (*healthz_model.APIGetHealthzResponse) is not supported by the TextConsumer, can be resolved by supporting TextUnmarshaler interface

the http log are displayed below:

GET /apis/v1beta1/healthz HTTP/1.1
Host: 127.0.0.1:8888
User-Agent: Go-http-client/1.1
Accept: application/json
Accept-Encoding: gzip


HTTP/1.1 200 OK
Content-Length: 96
Content-Type: text/plain; charset=utf-8
Date: Wed, 06 Apr 2022 06:41:40 GMT

{"commit_sha":"6ccf5bcd0b9db955c91caab85fa130714527f414", "tag_name":"1.7.0", "multi_user":true}

however, the message was wrapped into a json struct but the content type was specified with text/plain.
This triggered APIGetHealthzResponse failed to decode the response, as it failed to implement TextUnmarshaler interface.

The server code (on v1.7.0 as well as on master) also confirmed this

topMux.HandleFunc("/apis/v1beta1/healthz", func(w http.ResponseWriter, r *http.Request) {
, without proper set the content-type, the default one will determined by http.DetectContentType which will be 'text/plain'

Expected result

the golang example mentioned earier with swagger generated stub should work well.

Impacted by this bug? Give it a 👍. We prioritise the issues with the most 👍.

if this was confirmed as bug, I would like to submit a patch for this. Thanks

@zijianjoy
Copy link
Collaborator

@hsinhoyeh Thank you and we are happy to review the patch PR from you. cc @chensun

@hsinhoyeh
Copy link
Contributor Author

Hi, @zijianjoy I have sent my PR here #7532 thanks. also cc @chensun

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

2 participants