Expose Kong capabilities through a function in the client

[mmorel-35]

Creates a function to expose kong's informations like it's version, if it's an enterprise edition , which database is used.
There is also a Credentials attribute to inform if they support tags which is not the case under 1.4.0 version.

As this is calculated here, deck won't require semver as a direct dependency to calculate it himself.

I also made the kong client aware of the workspace. This allows the Root to be / by default and /kong when a workspace is used.

I reused getSemVerVer of KIC as a version cleaner

[rainest]

My initial thoughts:

• Adding workspace to the client builder is a significant breaking change. It will affect everything that uses this library, even though the required changes downstream are pretty small. We probably want to plan around when we release this as such. I don't think we can get the root behavior in here without it.
• I recommend keeping the semver type in place rather than converting it to a string. I don't think the dependency is a huge concern and any downstream version-gated behavior would need to use semver comparisons. Those will likely persist because not everything will have a go-kong.Kong capability entry and/or because a user may not want to upgrade to a newer go-kong version that adds one immediately.
• mtls-auth credentials are weird and didn't get tag support when other credentials did (the first version they support tags is 2.3.2.0-enterprise-edition); we make use of this in the controller. We can add that in a follow-up.
• This has use case overlap with the existing Enterprise feature enabled checks that we use in go-kong tests, but doesn't conflict with that implementation. We may want to figure out a way to unify that with this in the future.

[mmorel-35]

• I have removed the workspace from the client constructor and used a setter to define it
• I kept Version as a semVer
• The structure has been completed with RBAC and Portal to use them on the part you quoted
• Concerning the tag support on each credentials, I'm open to suggestions on how to structure it. There shall be the state and the minimum version that support it so the client can trace it in an error log if he sees that it's not supported.

[rainest]

I have removed the workspace from the client constructor and used a setter to define it

This doesn't quite get away from the breaking change, since client users would still need to remove any existing workspace path they're putting in the base URL and call SetWorkspace() for the root request to work properly. An alternative did occur to me though: the fallback logic approach originally used in decK is still viable with some adjustments. The client can determine whether it's in a workspace or not and get the root info without explicit configuration like so:

1. Attempt to request GET /kong first. If this returns a 200, the base URL includes a workspace.
2. If the initial request for GET /kong returns a 404, request GET /
3. If neither returns a successful response, error out.

One of those will work if the base URL is a Kong admin API. If we also need to know the workspace we're in, we can split the base URL path and use the last path segment.

Concerning the tag support on each credentials, I'm open to suggestions on how to structure it. There shall be the state and the minimum version that support it so the client can trace it in an error log if he sees that it's not supported.

I should have been a bit clearer on this: mtls-auth is the only credential type that's like this due to some weird history in its development, so it needs a unique capability value. We shouldn't see any other such oddball credentials in the future; the rest can share a single capabilities value, and should for simplicity.

[mmorel-35]

If we use the last path segment, a HEAD /workspaces/last_segment which returns a 200 should confirm the existence of the last_segment as a workspace?

Any idea for the naming of the field for tag support of the credentials which aren't mtlsauth ?

[rainest]

Any idea for the naming of the field for tag support of the credentials which aren't mtlsauth ?

No strong opinion really. The original

	Credentials struct {
		hasTagSupport bool
		minVersion    string
	}

is fine by me.

If we use the last path segment, a HEAD /workspaces/last_segment which returns a 200 should confirm the existence of the last_segment as a workspace?

Looks like it, though the endpoint is a bit odd. If you're sending a request to a workspace other than the root/default workspace, those requests can see the workspace you're in, but not others:

$ http lHEAD ocalhost:8001/other/workspaces/default 
HTTP/1.1 404 Not Found

$ http HEAD localhost:8001/other/workspaces/other 
HTTP/1.1 200 OK

The root can see everything, so localhost:8001/workspaces/other is a 200.

[mmorel-35]

@hbagdi , this seems to be the most DRY way to expose the Version and the calculed tag supports as this is repeated between projects (deck and kic to start with). This way the client doesn't need to know the logic but just use boolean properties.

[hbagdi]

@hbagdi , this seems to be the most DRY way to expose the Version and the calculed tag supports as this is repeated between projects (deck and kic to start with). This way the client doesn't need to know the logic but just use boolean properties.

The versioning part could make sense. The tag part is going to go away in KIC and decK as they will drop support for Kong 1.x so we shouldn't be putting that in.

[hbagdi]

Let's not do this here since it is a lossy conversion.
The user of the library can interpret the version if needed.

[rainest]

How about providing both the cleaned semver and the original string with all our extra content? Elsewhere we're likely going to be performing semver comparisons for the most part, and I think it makes sense to have a standard conversion rather than each user implementing their own cleanSemVer() variant.

[hbagdi]

I think a better approach would be:

info, err := client.Root() // most powerful
version := utils.versionFromInfo(info) // package-level global function to fetch version from the info map
semanticVersion := utils.ParseSemanticVersion(version) // package-level function

This makes the code cleaner and keeps each function responsible for one and only one thing.

[hbagdi]

I don't think this pattern is good.
We should instead give a utility function to the user which takes the output / or /kong and answers the questions around support for tags, enterprise, etc.
The kong packages is intended for talking to the API. Interpreting these specifics should not be part of this package.

[mmorel-35]

If I summarize, there is nothing left to take from that attempt. The PR can be closed?

[hbagdi]

@mmorel-35 Apologies for mixed reviews from our end on this one.
We met internally and here is what we think we should be doing here:

• We don't want to add capabilities feature just yet. Most of it won't be needed as we drop support for Kong 1.x series.
• We would like the API to be cleaner and here is how we want it to look like: Expose Kong capabilities through a function in the client #48 (comment)

Again, apologies for the delay and confusion here. Let us know if you have questions.

[codecov-commenter]

Codecov Report

❗ No coverage uploaded for pull request base (main@66d6286). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #48   +/-   ##
=======================================
  Coverage        ?   39.64%           
=======================================
  Files           ?       40           
  Lines           ?     2525           
  Branches        ?        0           
=======================================
  Hits            ?     1001           
  Misses          ?     1277           
  Partials        ?      247           

Flag	Coverage Δ
2.0.5	39.64% <0.00%> (?)
2.1.4	39.40% <0.00%> (?)
2.2.2	39.40% <0.00%> (?)
2.3.3	39.40% <0.00%> (?)
2.4.0	39.40% <0.00%> (?)
community	39.64% <0.00%> (?)
integration	39.64% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 66d6286...3505ee2. Read the comment docs.

[mmorel-35]

@hbagdi , is this what you want ?

[hbagdi]

@mmorel-35 We are still interested in adding / and /kong endpoints for looking up info. If you could send that in a separate PR, that would be totally cool.

[hbagdi]

Thank you for working on this!