Respect the full path of the Oxide API base URL

sudomateo commented 8 months ago

Preliminary checks

[X] I am using the latest version, or the latest version that corresponds to my Oxide installation.
[X] There is no open issue that reports the same problem.

What was the expected behaviour

The expected behavior is that the Oxide API client will respect the full path of the provided base URL. This can be achieved using URL.JoinPath, but I wanted to open this issue instead of a pull request to discuss the desired behavior first.

Here's a test case showing the desired behavior and how the current code currently fails.

func TestURLPath(t *testing.T) {
        tt := map[string]struct {
                baseURL          string
                relativePath     string
                expectedFinalURL string
        }{
                "api behind a context path": {
                        baseURL:          "https://api.oxide.computer/some/api/path",
                        relativePath:     "/v1/system/hardware/disks",
                        expectedFinalURL: "https://api.oxide.computer/some/api/path/v1/system/hardware/disks",
                },
                "api behind a context path with trailing slash": {
                        baseURL:          "https://api.oxide.computer/some/api/path/",
                        relativePath:     "/v1/system/hardware/disks",
                        expectedFinalURL: "https://api.oxide.computer/some/api/path/v1/system/hardware/disks",
                },
                "api behind a context path with query parameters": {
                        baseURL:          "https://api.oxide.computer/some/api/path?foo=bar",
                        relativePath:     "/v1/system/hardware/disks",
                        expectedFinalURL: "https://api.oxide.computer/some/api/path/v1/system/hardware/disks?foo=bar",
                },
                "api at the root path": {
                        baseURL:          "https://api.oxide.computer",
                        relativePath:     "/v1/system/hardware/disks",
                        expectedFinalURL: "https://api.oxide.computer/v1/system/hardware/disks",
                },
                "api at the root path with trailing slash": {
                        baseURL:          "https://api.oxide.computer/",
                        relativePath:     "/v1/system/hardware/disks",
                        expectedFinalURL: "https://api.oxide.computer/v1/system/hardware/disks",
                },
                "api at the root path with query parameters": {
                        baseURL:          "https://api.oxide.computer?foo=bar",
                        relativePath:     "/v1/system/hardware/disks",
                        expectedFinalURL: "https://api.oxide.computer/v1/system/hardware/disks?foo=bar",
                },
        }

        for testName, testCase := range tt {
                t.Run(testName, func(t *testing.T) {
                        baseURL, err := parseBaseURL(testCase.baseURL)
                        if err != nil {
                                t.Fatalf("failed parsing base url: %v", err)
                        }

                        finalURL := resolveRelative(baseURL, testCase.relativePath)

                        assert.Equal(t, testCase.expectedFinalURL, finalURL)
                })
        }
}

What is the current behaviour and what actions did you take to get there

A call to NewClient calls parseBaseURL to verify whether the passed Oxide API host is a valid URL.

https://github.com/oxidecomputer/oxide.go/blob/20c490d2d42338820d3ef2d9077318fb409c1224/oxide/lib.go#L91

Each API method of this client calls resolveRelative to build the final URL for its API request.

https://github.com/oxidecomputer/oxide.go/blob/20c490d2d42338820d3ef2d9077318fb409c1224/oxide/utils.go#L11-L20

Under the hood, resolveRelative uses URL.ResolveReference to build the final URL. When URL.ResolveReference is passed a path with a leading /, the resulting final URL is an absolute URL from the root of the base URL. This means that users of this Oxide API client cannot have their API served from any URL path other than the root.

For example, one cannot use a base URL of https://api.oxide.computer/some/api/path because an API call to /v1/system/hardware/disks would result in a final URL of https://api.oxide.computer/v1/system/hardware/disks, dropping the /some/api/path part of the URL.

Go SDK version

latest

Operating system

Fedora 39

Anything else you would like to add?

No response

karencfv commented 8 months ago

Hey there! Thanks a bunch for taking the time to open up this issue, and meticulously documenting the desired behaviour. This makes it very easy to understand what you're after :)

This functionality is indeed by design. While this feature may be desirable for testing a mock API or something similar, our product is currently designed in such a way that the API is always served from the root URL.

To avoid unintended errors, and to ensure that in the future we can enforce certain versions in the path (/v1/, /v2/, /v3/), it should remain this way for now.

Of course, if in the future we find that this feature is necessary, this issue can always be revisited!

sudomateo commented 8 months ago

This functionality is indeed by design. While this feature may be desirable for testing a mock API or something similar, our product is currently designed in such a way that the API is always served from the root URL.

Thank you for clarifying! My thought was that customers may place a web application firewall or similar in front of the Oxide API and serve it via a different URL path. The on-premises world is so unpredictable 😆.

Of course, if in the future we find that this feature is necessary, this issue can always be revisited!

Awesome, I'll leave this open then. Feel free to modify the issue as necessary. Thank you!

oxidecomputer / oxide.go