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

Schema for multimaps is wrong #4487

Closed
xRodney opened this issue Oct 10, 2022 · 0 comments · Fixed by #4501
Closed

Schema for multimaps is wrong #4487

xRodney opened this issue Oct 10, 2022 · 0 comments · Fixed by #4501
Labels
bug component/crd-generator Related to the CRD generator
Milestone

Comments

@xRodney
Copy link
Contributor

xRodney commented Oct 10, 2022

Describe the bug

Hello again,

after #4413 will have been merged, crd generator will no longer fail on multimaps, which are present in keycloak-operator crds.

Unfortunately, they will still be generated wrong.

Fabric8 Kubernetes Client version

SNAPSHOT

Steps to reproduce

(same as in #4357)

@Group("map.fabric8.io")
@Version("v1alpha1")
public class ContainingMaps extends  CustomResource<ContainingMapsSpec, Void> {}

public class ContainingMapsSpec {
  private MyMultiMap<String, String> test3;
}

public class MyMultiMap<K, V> extends HashMap<K, List<V>> {}

The field test3 is treated as if it was Map<String, String>:

      openAPIV3Schema:
        properties:
          status:
            type: object
          spec:
            properties:
              test3:
                additionalProperties:
                  type: string
                type: object

Expected behavior

The field test3 should be treated as if it was Map<String, List<String>>:

 openAPIV3Schema:
        properties:
          status:
            type: object
          spec:
            properties:
              test3:
                additionalProperties:
                  items:
                    type: string
                  type: array
                type: object



Runtime

other (please specify in additional context)

Kubernetes API Server version

other (please specify in additional context)

Environment

other (please specify in additional context)

Fabric8 Kubernetes Client Logs

No response

Additional context

Compared to #4413, this is rather low severity, as it can easily be worked around using @SchemaFrom and/or @SchemaSwap.

In my assessment, this should be improved in sundrio and I have already filed an issue there (sundrio/sundrio#341) and a PR (sundrio/sundrio#342). If I'm right and my contribution is merged, here we should only upgrade sundrio and maybe add a test and a changelog entry.

@manusa manusa added component/crd-generator Related to the CRD generator bug labels Oct 11, 2022
xRodney added a commit to xRodney/kubernetes-client that referenced this issue Oct 12, 2022
Upgraded sundrio and added a test to validate that multimaps (like `class MultiMap<K,V> implements Map<K,List<V>>`) are generated correctly.

Fixes fabric8io#4487
xRodney added a commit to xRodney/kubernetes-client that referenced this issue Oct 12, 2022
Upgraded sundrio and added a test to validate that multimaps (like `class MultiMap<K,V> implements Map<K,List<V>>`) are generated correctly.

Fixes fabric8io#4487
xRodney added a commit to xRodney/kubernetes-client that referenced this issue Oct 14, 2022
Upgraded sundrio and added a test to validate that multimaps (like `class MultiMap<K,V> implements Map<K,List<V>>`) are generated correctly.

Fixes fabric8io#4487
xRodney added a commit to xRodney/kubernetes-client that referenced this issue Oct 14, 2022
…ses), schema for multimaps is wrong

Logic in sundrio that is responsible for replacing generic type arguments on methods and properties with their concrete instantiations from subclasses contained a bug, in which it looped infinitely if the same type argument name was used at multiple classes in the hierarchy. A common occurence were multimaps.

A very similar code was also present in crd-generator, also suffering from the same bug.

Sundrio has been updated to account for this situation, and updated utilities from sundrio have been introduced to crd-generator, which replace the previous implementation of the same algorithm.

A test has been added to validate that multimaps (like `class MultiMap<K,V> implements Map<K,List<V>>`) are generated correctly.

Fixes fabric8io#4357
Fixes fabric8io#4487
xRodney added a commit to xRodney/kubernetes-client that referenced this issue Oct 18, 2022
…es), schema for multimaps is wrong

Logic in sundrio that is responsible for replacing generic type arguments on methods and properties with their concrete instantiations from subclasses contained a bug, in which it looped infinitely if the same type argument name was used at multiple classes in the hierarchy. A common occurence were multimaps.

A very similar code was also present in crd-generator, also suffering from the same bug.

Sundrio has been updated to account for this situation, and updated utilities from sundrio have been introduced to crd-generator, which replace the previous implementation of the same algorithm.

A test has been added to validate that multimaps (like `class MultiMap<K,V> implements Map<K,List<V>>`) are generated correctly.

Fixes fabric8io#4357
Fixes fabric8io#4487
xRodney added a commit to xRodney/kubernetes-client that referenced this issue Oct 18, 2022
…es), schema for multimaps is wrong

Logic in sundrio that is responsible for replacing generic type arguments on methods and properties with their concrete instantiations from subclasses contained a bug, in which it looped infinitely if the same type argument name was used at multiple classes in the hierarchy. A common occurence were multimaps.

A very similar code was also present in crd-generator, also suffering from the same bug.

Sundrio has been updated to account for this situation, and updated utilities from sundrio have been introduced to crd-generator, which replace the previous implementation of the same algorithm.

A test has been added to validate that multimaps (like `class MultiMap<K,V> implements Map<K,List<V>>`) are generated correctly.

Fixes fabric8io#4357
Fixes fabric8io#4487
xRodney added a commit to xRodney/kubernetes-client that referenced this issue Oct 18, 2022
…es), schema for multimaps is wrong

Logic in sundrio that is responsible for replacing generic type arguments on methods and properties with their concrete instantiations from subclasses contained a bug, in which it looped infinitely if the same type argument name was used at multiple classes in the hierarchy. A common occurence were multimaps.

A very similar code was also present in crd-generator, also suffering from the same bug.

Sundrio has been updated to account for this situation, and updated utilities from sundrio have been introduced to crd-generator, which replace the previous implementation of the same algorithm.

A test has been added to validate that multimaps (like `class MultiMap<K,V> implements Map<K,List<V>>`) are generated correctly.

Fixes fabric8io#4357
Fixes fabric8io#4487
xRodney added a commit to xRodney/kubernetes-client that referenced this issue Oct 19, 2022
…es), schema for multimaps is wrong

Logic in sundrio that is responsible for replacing generic type arguments on methods and properties with their concrete instantiations from subclasses contained a bug, in which it looped infinitely if the same type argument name was used at multiple classes in the hierarchy. A common occurence were multimaps.

A very similar code was also present in crd-generator, also suffering from the same bug.

Sundrio has been updated to account for this situation, and updated utilities from sundrio have been introduced to crd-generator, which replace the previous implementation of the same algorithm.

A test has been added to validate that multimaps (like `class MultiMap<K,V> implements Map<K,List<V>>`) are generated correctly.

Fixes fabric8io#4357
Fixes fabric8io#4487
manusa pushed a commit to xRodney/kubernetes-client that referenced this issue Oct 19, 2022
…es), schema for multimaps is wrong

Logic in sundrio that is responsible for replacing generic type arguments on methods and properties with their concrete instantiations from subclasses contained a bug, in which it looped infinitely if the same type argument name was used at multiple classes in the hierarchy. A common occurence were multimaps.

A very similar code was also present in crd-generator, also suffering from the same bug.

Sundrio has been updated to account for this situation, and updated utilities from sundrio have been introduced to crd-generator, which replace the previous implementation of the same algorithm.

A test has been added to validate that multimaps (like `class MultiMap<K,V> implements Map<K,List<V>>`) are generated correctly.

Fixes fabric8io#4357
Fixes fabric8io#4487
@manusa manusa added this to the 6.2.0 milestone Oct 19, 2022
manusa pushed a commit that referenced this issue Oct 19, 2022
…es), schema for multimaps is wrong

Logic in sundrio that is responsible for replacing generic type arguments on methods and properties with their concrete instantiations from subclasses contained a bug, in which it looped infinitely if the same type argument name was used at multiple classes in the hierarchy. A common occurence were multimaps.

A very similar code was also present in crd-generator, also suffering from the same bug.

Sundrio has been updated to account for this situation, and updated utilities from sundrio have been introduced to crd-generator, which replace the previous implementation of the same algorithm.

A test has been added to validate that multimaps (like `class MultiMap<K,V> implements Map<K,List<V>>`) are generated correctly.

Fixes #4357
Fixes #4487
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component/crd-generator Related to the CRD generator
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants