GC Tool is mixing v1 and v2 URLs

[adutra]

What happened:

Since #5660 GC tool switched to V2 api by default, but it seems to be mixing V1 and V2 URLs:

GET /api/v1/trees/main%402e1cfa82035c26cbbbdae632cea070514eb8b773f616aaeaf668e2f0be
8f10d/history?fetch=ALL HTTP/1.1" 405

The above v1 URL does not exist, but there is a matching v2 one. The request is simply trying to read the commitlog for the given reference, using v2 getCommitLog REST endpoint, but the URI prefix is wrong.

As a consequence the router is mistakenly matching the request against the v1 assignReference REST endpoint, which expects a PUT method, not GET – hence the 405 HTTP response (method not allowed).

Here is the DEBUG log showing that:

DEBUG [io.qua.mic.run.bin.RequestMetricInfo] (vert.x-eventloop-thread-1) Path /api/vi/trees/main%402e1cfa82b035c26cbbbdae632cea070514eb8b773f616aaeaf668e2f0be8f10d/history matched pattern /api/v1/trees/.*/ using /api/v1/trees/{referenceType}/{ref}

What you expected to happen:

GC tool should use v2 URL prefixes by default.

I suggest that we:

1. Change the default value of NessieOptions#nessieUri to be a V2 URI prefix;
2. Add a warning if the value of this field does not end with api/v2.

How to reproduce it (as minimally and precisely as possible):

I think this is happening by default if user uses the default URI, or passes a v1 URI using the --uri option.

[dimas-b]

Add a warning if the value of this field does not end with api/v2.

This may be an overkill. URLs are not required to have this suffix in custom deployments.

[dimas-b]

Since GC requires API v2 now, I guess we could validate the URL using the /config end point and check some of the V2-specific attributes, e.g. NessieConfiguration.getSpecVersion()

[adutra]

Since GC requires API v2 now, I guess we could validate the URL using the /config end point and check some of the V2-specific attributes, e.g. NessieConfiguration.getSpecVersion()

While this should work to distinguish a URL mismatch between v1 and v2 endpoints, I think this is a little brittle. E.g. when a v3 is introduced, both v2 and v3 will support NessieConfiguration.getSpecVersion() and thus it won't be possible to detect a mismatch between v2 and v3 endpoints, for instance creating a v3 client with NessieApiV3, but with a v2 url prefix.

I will provide a quick fix using your suggestion, which should be good enough for now, but I think we should come up with a more robust way to detect this kind of mismatch in the future.

[dimas-b]

We could still use the spec version value, I guess, but I agree, this needs some more thinking.

[adutra]

We could still use the spec version value

Could we? Imagine a V3 server returning:

• spec version: 3.0.0
• min API version: 2
• max API version: 3

We want to detect the situation where a client is created with NessieApiV3 but with a v2 URI:

NessieApi client =
    HttpClientBuilder.builder()
        .withUri("http://localhost:19120/api/v2")
        .build(NessieApiV3.class);

The above client will break as soon as it tries to invoke a v3 endpoint that does not exist under the /api/v2 root, but getConfig() will actually work, since the endpoint exists in all versions. So getConfig().getSpecVersion() in my example will return "3.0.0" as expected, failing to detect that the client is not well configured.

[snazy]

It’s effectively just a wrong default IIUC, let’s fix that one.

We don’t have any v3 yet, not even an idea how it could look like.