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

Upjet shouldn't generate types with float fields #442

Open
negz opened this issue Oct 16, 2024 · 1 comment
Open

Upjet shouldn't generate types with float fields #442

negz opened this issue Oct 16, 2024 · 1 comment
Labels
bug Something isn't working v2
Milestone

Comments

@negz
Copy link
Member

negz commented Oct 16, 2024

What happened?

In crossplane-contrib/provider-upjet-gcp#623 various fields that appear to really be integers (e.g. port) are generated with type *float64. This led to a request to add paved.GetFloat() - some context and discussion in crossplane/crossplane-runtime#778.

You're not supposed to use floats in Kubernetes APIs. From https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#primitive-types:

Avoid floating-point values as much as possible, and never use them in spec. Floating-point values cannot be reliably round-tripped (encoded and re-decoded) without changing, and have varying precision and representations across languages and architectures.

See for example https://github.com/kubernetes-sigs/json, which deserializes JSON numbers into int64 where possible.

How can we reproduce it?

Take a look at the generated types in crossplane-contrib/provider-upjet-gcp#623.

@negz negz added the bug Something isn't working label Oct 16, 2024
@jeanduplessis jeanduplessis added this to the 2.0 milestone Oct 18, 2024
@rickard-von-essen
Copy link
Contributor

Guessing this is the code part that is the root of this (with a comment about the problem by @turkenh)

case typ.Equals(cty.Number):
// TODO(turkenh): Figure out handling floats with IntOrString on type
// builder side
return schemav2.TypeFloat

Relevant parts of config/schema.json for google_redis_cluster in provider-upjet-gcp.

"discovery_endpoints": {
  "type": [
    "list",
    [
      "object",
      {
        "address": "string",
        "port": "number",

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working v2
Projects
None yet
Development

No branches or pull requests

3 participants