rancher / rancher-docs

Rancher Documentation
https://ranchermanager.docs.rancher.com/
Apache License 2.0
59 stars 205 forks source link

Incorrect docs of port object in NavLink #1409

Open lindhe opened 3 months ago

lindhe commented 3 months ago

The docs seems to indicate that the .spec.toService.port field of a NavLink object is a string:

https://github.com/rancher/rancher-docs/blob/8bd7b4cffbbac6ccebc579b616b1faed0f85c0c1/versioned_docs/version-2.8/how-to-guides/new-user-guides/authentication-permissions-and-global-configuration/custom-branding.md?plain=1#L134

https://github.com/rancher/rancher-docs/blob/8bd7b4cffbbac6ccebc579b616b1faed0f85c0c1/versioned_docs/version-2.8/how-to-guides/new-user-guides/authentication-permissions-and-global-configuration/custom-branding.md?plain=1#L189

But API spec seems to disagree:

$ kubectl explain navlinks.ui.cattle.io.spec.toService.port
GROUP:      ui.cattle.io
KIND:       NavLink
VERSION:    v1

FIELD: port <Object>

DESCRIPTION:
    <empty>

I'm running Rancher v2.8.5. I have no idea when the API broke. I'm pretty sure it was a string back in v2.7.8.

I'm guessing the docs should be updated to adhere to the API spec. I am unsure of how exactly, since the API spec just says "Object" but no specifics. 😕

lindhe commented 3 months ago

Wait a minute… Creating a NavLink via the Rancher GUI also sets port: "80". Is the API spec wrong??

martyav commented 3 months ago

@lindhe we took a look at https://github.com/rancher/rancher/blob/release/v2.8.5/pkg/apis/ui.cattle.io/v1/types.go#L33 and technically the port field is an intstr, which is an Object, but the intent is clearly to accept values that are entered as either integers or strings. And the GUI must only be sending strings.

lindhe commented 3 months ago

Ah, I see. Alright. I'm guessing the API docs are generated directly from the code, so there's probably not much to do about the type then. Unless there is some hint we can use to help the API docs?

In that case, perhaps we can at least add a description so it's possible to read the information that way?

martyav commented 3 months ago

I brought it up to the API team