setting content type header for successful proxied requests

[diogoasouza]

This PR uses mime.TypeByExtension to set the Content-Type header of proxied extension requests. If it fails to get the type, we use the one returned by the request.

[bigkevmcd]

Is there a test we could add for this?

[diogoasouza]

Is there a test we could add for this?

Maybe an integration test, I'll check it!

[diogoasouza]

Added a test @bigkevmcd, can you review it?

[rohitsakala]

I was trying to test this but I am getting an error.

I installed top-level-product and set nocache to true and then I tried hitting this url https://localhost:8443/v1/uiplugins/top-level-product/0.1.0/ where I get 400: Invalid request

Am I doing something wrong here in testing this ?

[bigkevmcd]

This is good, but you could test the code in the catalog package itself.

func TestProxyRequest_content_type(t *testing.T) {
    response := "testing:\n  value: test\n"
    ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        w.Header().Set("Content-Disposition", "attachment; filename=testing.yaml")
        if _, err := w.Write([]byte(response)); err != nil {
            t.Fatal(err)
        }   
    })) 
    defer ts.Close()

    req := httptest.NewRequest(http.MethodGet, "http://example.com", nil)
    w := httptest.NewRecorder()

    proxyRequest(ts.URL, "/testing.yaml", w, req, func(string) bool { return false })

    resp := w.Result()
    body, _ := io.ReadAll(resp.Body)

    if resp.StatusCode != http.StatusOK {
        t.Errorf("got StatusCode %v, want %v", resp.StatusCode, http.StatusOK)
    }

    wantContent := "application/yaml"
    if ct := resp.Header.Get("Content-Type"); ct != wantContent {
        t.Errorf("got Content-Type %s, want %s", ct, wantContent)

    }   
    if string(body) != response {
        t.Errorf("read body: %s", body)
    }   
}

Note that I had to modify proxyRequest to accept a denied func(string) bool parameter and pass in denyList and then provide one that allows localhost addresses to be able to proxy through the test server.

But, you could redesign the UIPluginHandler as a struct which contained the denied addresses and had the index handler and pluginHandler as methods which would deal with this.

[bigkevmcd]

High-level integration tests tell you that something isn't working, lower-level tests can usually tell you what isn't working.

[diogoasouza]

I added this test. I changed the file to javascript though, since the mime package and http.DetectContentType both don't recognize yaml files

[diogoasouza]

I was trying to test this but I am getting an error.

I installed top-level-product and set nocache to true and then I tried hitting this url https://localhost:8443/v1/uiplugins/top-level-product/0.1.0/ where I get 400: Invalid request

Am I doing something wrong here in testing this ?

The endpoint is https://localhost:8443/v1/uiplugins/top-level-product/0.1.0/<path_to_file>. Since you provided an empty path, 400 was expected in this case. You can try https://localhost:8443/v1/uiplugins/top-level-product/0.1.0/ + any entry form here https://github.com/rancher/ui-plugin-examples/blob/main/extensions/top-level-product/0.1.0/files.txt

[bigkevmcd]

You need to store the old one, and put it back, otherwise any additional tests would have undefined behaviour.

This is why passing in the verifier is a better option .

[diogoasouza]

That's true, I updated the code

[bigkevmcd]

Additional tests should really be added for cases where the mime-type doesn't match, but this is definitely better than it was.