Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i32 for port number #136

Closed
sombralibre opened this issue Feb 9, 2023 · 5 comments
Closed

i32 for port number #136

sombralibre opened this issue Feb 9, 2023 · 5 comments

Comments

@sombralibre
Copy link

Hi there, just a question about why does the struct that represent the ServiceBackendPort uses an i32 instead of an u16 for hold the port number; I've checked the definition for several versions of kubernetes and the definition is the same:

Is there something that I missed?

I means whether the definition of the data wide for port number in the TCP/IP is 16 bits, It doesn't make sense that the port would be represented for an u16?

What would happens if someone don't pay attention to this, and ended passing a value over 65535 or a negative one? It will not be safe enough I assume.

@Arnavion
Copy link
Owner

Arnavion commented Feb 9, 2023

Is there something that I missed?

Yes, you missed that this code is auto-generated from the OpenAPI spec. It's not like I made the conscious decision to represent port numbers as i32. :)

So, to that end, the OpenAPI spec says:

    "io.k8s.api.networking.v1.ServiceBackendPort": {
      "description": "ServiceBackendPort is the service port being referenced.",
      "properties": {
        "name": {
          "description": "name is the name of the port on the Service. This is a mutually exclusive setting with \"Number\".",
          "type": "string"
        },
        "number": {
          "description": "number is the numerical port number (e.g. 80) on the Service. This is a mutually exclusive setting with \"Name\".",
          "format": "int32",
          "type": "integer"
        }
      },
      "type": "object"
    }

"format": "int32"

At this point you might say it's a spec bug and would be resolved by specifying a better format. But even the API server code says:

https://github.com/kubernetes/kubernetes/blob/9b09d0600a69a2eb36b0d136465ccc3c179dacdb/pkg/apis/networking/types.go#L624-L636

Number int32

🤷 Complain to upstream.

@sombralibre
Copy link
Author

Yes, you're right, just after create this issue I went to the OpenApi specs and found the same.

I will scale the question to the openapi people.

Thanks a lot man.

@Arnavion
Copy link
Owner

Arnavion commented Feb 9, 2023

Well, OpenAPI supports specifying integer ranges using minimum and maximum, but the Kubernetes OpenAPI spec is downstream of their golang code so they'd have to invent new annotations to express that. Besides, ServiceBackendPort::number is far from the only field that has this problem; searching for : Option<i32>, in this repo reveals all sorts of time durations and replica counts and percentages, and upstream would have to fix all of them for completeness.

@Arnavion
Copy link
Owner

Arnavion commented Feb 9, 2023

Upstream issue: kubernetes/kubernetes#105533

@sombralibre
Copy link
Author

Upstream issue: kubernetes/kubernetes#105533

👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants