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

Major issues since at least 1.5.2 #152

Closed
maxemann96 opened this issue Sep 24, 2024 · 3 comments · Fixed by pulumi/pulumi-kubernetes#3237 or #153
Closed

Major issues since at least 1.5.2 #152

maxemann96 opened this issue Sep 24, 2024 · 3 comments · Fixed by pulumi/pulumi-kubernetes#3237 or #153
Assignees
Labels
area/codegen Affects quality or correctness of generated code area/schema Related to support for CRD or Pulumi schema support impact/regression Something that used to work, but is now broken kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Milestone

Comments

@maxemann96
Copy link

maxemann96 commented Sep 24, 2024

What happended

Since version at least 1.5.2, the produced output is not usable and has some serious issues. To point the issue out, I minimized my setup to Karpenter, Keycloak, MariaDB and Strimzi custom resource definitions. You can use the following script to produce the same output, as I got (BEWARE: This will delete the folders k8s and nodejs from the directory, the script is residing in, if existent!)

# !/bin/sh
set -e

SCRIPT_DIR="$(dirname "$0")"
cd "$SCRIPT_DIR"

rm -Rf "k8s" "nodejs"

mkdir nodejs
mkdir k8s
cd k8s

curl "https://raw.githubusercontent.com/aws/karpenter-provider-aws/v1.0.2/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml" -o Karpenter_karpenter.k8s.aws_ec2nodeclasses.yaml

curl "https://raw.githubusercontent.com/aws/karpenter-provider-aws/v1.0.2/pkg/apis/crds/karpenter.sh_nodeclaims.yaml" -o Karpenter_karpenter.sh_nodeclaims.yaml

curl "https://raw.githubusercontent.com/aws/karpenter-provider-aws/v1.0.2/pkg/apis/crds/karpenter.sh_nodepools.yaml" -o Karpenter_karpenter.sh_nodepools.yaml

curl "https://raw.githubusercontent.com/keycloak/keycloak-k8s-resources/24.0.4/kubernetes/keycloaks.k8s.keycloak.org-v1.yml" -o Keycloak_keycloaks.k8s.keycloak.org-v1.yaml

curl "https://raw.githubusercontent.com/keycloak/keycloak-k8s-resources/24.0.4/kubernetes/keycloakrealmimports.k8s.keycloak.org-v1.yml" -o Keycloak_keycloakrealmimports.k8s.keycloak.org-v1.yaml

for file in $(echo "backups connections databases grants mariadbs maxscales restores sqljobs users"); do
    curl "https://raw.githubusercontent.com/mariadb-operator/mariadb-operator/helm-chart-0.29.0/config/crd/bases/k8s.mariadb.com_$file.yaml" -o "MariaDB_k8s.mariadb.com_$file.yaml"
done;

curl "https://github.com/strimzi/strimzi-kafka-operator/releases/download/0.43.0/strimzi-crds-0.43.0.yaml" -o Strimzi_strimzi-crds-0.43.0.yaml -L

cd ..

crd2pulumi --force --nodejs --nodejsPath="./nodejs" ./k8s/*.yaml

Karpenter

nodejs/karpenter/v1beta1 is generated, but nodejs/karpenter/v1 not. Works with 1.4.0.

Keycloak

The generated pulumi type is kubernetes:k8s.mariadb.com/v2alpha1:Keycloak. There is no mariadb in the Keycloak crds. Works with 1.4.0

Strimzi

From nodejs/types/input.ts, the hyphen is not escaped:

        /**
         * **Currently not supported** JVM Options for pods.
         */
        export interface KafkaBridgeSpecJvmOptions {
            /**
             * A map of -XX options to the JVM.
             */
            -XX?: pulumi.Input<{[key: string]: pulumi.Input<string>}>;

Previous behaviour

Keycloak and MariaDB worked before (don't know, if the contain hyphens, I have another workaround to fix hyphens here, not part of the script above). For strimzi, I opened #103 a while ago, this was closed as fixed with 1.5.0.

Useful information

Output of crd2pulumi version: v1.5.2. Directly downloaded from the GitHub releases page.

@pulumi-bot pulumi-bot added the needs-triage Needs attention from the triage team label Sep 24, 2024
@rquitales
Copy link
Member

Thanks for reporting the issues and apologies for the issues you're running into.

  1. Karpenter: The version used here (
    schemas[sw.Info.Version] = *sw
    ) is incorrectly targeting the generated swagger version, instead of the CRD apiVersion. We'll have to update our code base to address this.
  2. Keycloak: This will require a fix in upstream Pulumi Kubernetes schemagen library. The issue here is that we are not correctly specifying the entire potential group from a swagger definition. As a result, the following two groups k8s.keycloak.org and k8s.mariadb.com will collapse as k8s in our map lookup, resulting in the incorrect type token being generated. We will need to update split[len(split)-3] to strings.Join(split[:len(split)-2], ".") as a potential fix.
  3. Strimzi: The strimzi CRD generation was incorrectly marked as being resolved by the v1.5.0 release. I have re-opened Invalid typescript code produced when generating for strimzi crds  #103 to continue tracking this error. Unfortunately, there is an outstanding issue with generating crds that contain - in field names across all supported languages, and will require changes in upstream codegen libraries that we rely on.

We'll add this to our backlog and issue a fix for the first two issues.

@rquitales rquitales added area/codegen Affects quality or correctness of generated code area/schema Related to support for CRD or Pulumi schema support and removed needs-triage Needs attention from the triage team labels Sep 25, 2024
@rquitales rquitales self-assigned this Sep 25, 2024
@mikhailshilkov mikhailshilkov added the kind/bug Some behavior is incorrect or out of spec label Sep 27, 2024
@mjeffryes mjeffryes added the impact/regression Something that used to work, but is now broken label Sep 27, 2024
@pulumi-bot pulumi-bot added resolution/fixed This issue was fixed labels Sep 30, 2024
rquitales added a commit that referenced this issue Oct 1, 2024
## Proposed Changes

This PR ensures the correct CRD versions are used when parsing.
Previously, the Swagger version generated by `builder.BuildOpenAPIV2`
defaulted to `0.1.0`, but we should be pulling the version directly from
the CRD spec itself. This change ensures that SDKs are generated for all
CRD versions. Additionally, this PR incorporates the latest updates from
p-k to prevent package collisions.

## Related Issues (optional)
Closes: #152
@rquitales
Copy link
Member

I've just cut a v1.5.3 release which contains fixes for the first 2 describe issues. #103 will continue tracking the fix for strimzi CRDs.

@mjeffryes mjeffryes added this to the 0.111 milestone Oct 2, 2024
@pulumi-bot
Copy link

This PR has been shipped in release v1.5.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/codegen Affects quality or correctness of generated code area/schema Related to support for CRD or Pulumi schema support impact/regression Something that used to work, but is now broken kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Projects
None yet
5 participants