From f90e016d105d3ed8e11ed0ceb61494878eb2de4c Mon Sep 17 00:00:00 2001 From: Justin Van Patten Date: Fri, 19 May 2023 17:31:21 -0700 Subject: [PATCH 1/4] Make all type names PascalCase --- go.mod | 2 +- pkg/codegen/schema.go | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index d9f4f44..65616a3 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/pulumi/crd2pulumi go 1.19 require ( + github.com/iancoleman/strcase v0.2.0 github.com/pulumi/pulumi/pkg/v3 v3.59.1-0.20230323225522-946074865b11 github.com/pulumi/pulumi/sdk/v3 v3.59.1-0.20230323225522-946074865b11 github.com/spf13/cobra v1.6.1 @@ -119,7 +120,6 @@ require ( github.com/hashicorp/vault/api v1.8.2 // indirect github.com/hashicorp/vault/sdk v0.6.1 // indirect github.com/hashicorp/yamux v0.1.1 // indirect - github.com/iancoleman/strcase v0.2.0 // indirect github.com/ijc/Gotty v0.0.0-20170406111628-a8b993ba6abd // indirect github.com/imdario/mergo v0.3.13 // indirect github.com/inconshreveable/mousetrap v1.0.1 // indirect diff --git a/pkg/codegen/schema.go b/pkg/codegen/schema.go index 0d14a28..624fb81 100644 --- a/pkg/codegen/schema.go +++ b/pkg/codegen/schema.go @@ -19,12 +19,11 @@ import ( "sort" "strconv" + "github.com/iancoleman/strcase" "github.com/pulumi/crd2pulumi/internal/slices" "github.com/pulumi/crd2pulumi/internal/unstruct" pschema "github.com/pulumi/pulumi/pkg/v3/codegen/schema" "github.com/pulumi/pulumi/sdk/v3/go/common/tokens" - "golang.org/x/text/cases" - "golang.org/x/text/language" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -141,7 +140,7 @@ func AddType(schema map[string]any, name string, types map[string]pschema.Comple propertyDescription, _, _ := unstructured.NestedString(propertySchema, "description") defaultValue, _, _ := unstructured.NestedFieldNoCopy(propertySchema, "default") propertySpecs[propertyName] = pschema.PropertySpec{ - TypeSpec: GetTypeSpec(propertySchema, name+cases.Title(language.Und).String(propertyName), types), + TypeSpec: GetTypeSpec(propertySchema, name+strcase.ToCamel(propertyName), types), Description: propertyDescription, Default: defaultValue, } From a47fea29190c16914bd8a31120945960fc33316e Mon Sep 17 00:00:00 2001 From: Ramon Quitales Date: Thu, 25 May 2023 13:13:21 -0700 Subject: [PATCH 2/4] Refactor tests --- tests/crds_test.go | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/tests/crds_test.go b/tests/crds_test.go index d4eac64..70ec77a 100644 --- a/tests/crds_test.go +++ b/tests/crds_test.go @@ -15,6 +15,7 @@ package tests import ( + "fmt" "io/fs" "io/ioutil" "os" @@ -31,14 +32,18 @@ const gkeManagedCertsUrl = "https://raw.githubusercontent.com/GoogleCloudPlatfor // execCrd2Pulumi runs the crd2pulumi binary in a temporary directory func execCrd2Pulumi(t *testing.T, lang, path string) { - tmpdir, err := ioutil.TempDir("", "") + tmpdir, err := ioutil.TempDir("", "crd2pulumi_test") assert.Nil(t, err, "expected to create a temp dir for the CRD output") - defer os.RemoveAll(tmpdir) - langFlag := "--" + lang + "Path" + t.Cleanup(func() { + t.Logf("removing temp dir %q for %s test", tmpdir, lang) + os.RemoveAll(tmpdir) + }) + langFlag := fmt.Sprintf("--%sPath", lang) // e.g. --dotnetPath binaryPath, err := filepath.Abs("../bin/crd2pulumi") if err != nil { - panic(err) + t.Fatalf("unable to create absolute path to binary: %s", err) } + t.Logf("%s %s=%s %s: running", binaryPath, langFlag, tmpdir, path) crdCmd := exec.Command(binaryPath, langFlag, tmpdir, "--force", path) crdOut, err := crdCmd.CombinedOutput() @@ -51,7 +56,12 @@ func TestCRDsFromFile(t *testing.T) { filepath.WalkDir("crds", func(path string, d fs.DirEntry, err error) error { if !d.IsDir() && (filepath.Ext(path) == ".yml" || filepath.Ext(path) == ".yaml") { for _, lang := range languages { - execCrd2Pulumi(t, lang, path) + lang := lang + name := fmt.Sprintf("%s-%s", lang, filepath.Base(path)) + t.Run(name, func(t *testing.T) { + t.Parallel() + execCrd2Pulumi(t, lang, path) + }) } } return nil @@ -61,6 +71,10 @@ func TestCRDsFromFile(t *testing.T) { // TestCRDsFromUrl pulls the CRD YAML file from a URL and generates it in each language func TestCRDsFromUrl(t *testing.T) { for _, lang := range languages { - execCrd2Pulumi(t, lang, gkeManagedCertsUrl) + lang := lang + t.Run(lang, func(t *testing.T) { + t.Parallel() + execCrd2Pulumi(t, lang, gkeManagedCertsUrl) + }) } } From f2d468942162305dda16594df340076aa83b5be1 Mon Sep 17 00:00:00 2001 From: Ramon Quitales Date: Thu, 25 May 2023 15:01:28 -0700 Subject: [PATCH 3/4] Add tests for camelCased type generation --- .../crds/underscored-types/networkpolicy.yaml | 123 ++++++++++++++++++ tests/crds_test.go | 48 ++++++- 2 files changed, 167 insertions(+), 4 deletions(-) create mode 100644 tests/crds/underscored-types/networkpolicy.yaml diff --git a/tests/crds/underscored-types/networkpolicy.yaml b/tests/crds/underscored-types/networkpolicy.yaml new file mode 100644 index 0000000..14faeee --- /dev/null +++ b/tests/crds/underscored-types/networkpolicy.yaml @@ -0,0 +1,123 @@ +# Ensure generated nested types are not underscored. +# See https://github.com/pulumi/crd2pulumi/issues/107 +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + myinfo: abcdefghijkl + generation: 4 + labels: + creator.ac.com: myinfo + name: networkpolicies.juice.box.com +spec: + conversion: + strategy: None + group: juice.box.com + names: + kind: NetworkPolicy + listKind: NetworkPolicyList + plural: networkpolicies + shortNames: + - anp + - anps + singular: networkpolicy + preserveUnknownFields: true + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + properties: + spec: + type: object + description: NetworkPolicySpec is a specification of the network + entitlements for a pod. + properties: + apps_incoming: + description: apps_incoming specifies which applications are permitted + to establish a TCP connection to a POD. + items: + properties: + app: + pattern: ^(__kubernetes__|plb\.juice-plb\.juice-prod|((([A-Za-z0-9]+[-A-Za-z0-9]?)*[A-Za-z0-9])\.){2}(kk|kube))$ + type: string + cluster: + description: cluster that this policy applies to. Defaults to + the local cluster. Setting cluster to 'ALL' will match all + clusters + type: string + required: + - app + type: object + type: array + apps_outgoing: + description: apps_outgoing specifies what applications a pod may attempt + to make TCP connections to. + items: + properties: + app: + pattern: ^(__kubernetes__|plb\.juice-plb\.juice-prod|((([A-Za-z0-9]+[-A-Za-z0-9]?)*[A-Za-z0-9])\.){2}(kk|kube))$ + type: string + cluster: + description: cluster that this policy applies to. Defaults to + the local cluster. Setting cluster to 'ALL' will match all + clusters + type: string + required: + - app + type: object + type: array + namespaces_incoming: + description: namespaces_incoming specifies which kubernetes namespace + are permitted to establish incoming TCP sessions. + items: + properties: + cluster: + description: cluster that this policy applies to. Defaults to + the local cluster. Setting cluster to 'ALL' will match all + clusters + type: string + namespace: + pattern: ^(((([A-Za-z0-9]+[-A-Za-z0-9]?)*[A-Za-z0-9])\.)(kk|kube))$ + type: string + required: + - namespace + type: object + type: array + namespaces_outgoing: + description: namespaces_outgoing specifies which kubernetes namespace + are permitted to establish outgoing TCP sessions. + items: + properties: + cluster: + description: cluster that this policy applies to. Defaults to + the local cluster. Setting cluster to 'ALL' will match all + clusters + type: string + namespace: + pattern: ^(((([A-Za-z0-9]+[-A-Za-z0-9]?)*[A-Za-z0-9])\.)(kk|kube))$ + type: string + required: + - namespace + type: object + type: array + selector: + additionalProperties: + type: string + description: selector is a set of label selectors + type: object + required: + - selector + served: true + storage: true +status: + acceptedNames: + kind: NetworkPolicy + listKind: NetworkPolicyList + plural: networkpolicies + shortNames: + - anp + - anps + singular: networkpolicy + storedVersions: + - v1alpha1 \ No newline at end of file diff --git a/tests/crds_test.go b/tests/crds_test.go index 70ec77a..6e10b43 100644 --- a/tests/crds_test.go +++ b/tests/crds_test.go @@ -31,7 +31,7 @@ var languages = []string{"dotnet", "go", "nodejs", "python"} const gkeManagedCertsUrl = "https://raw.githubusercontent.com/GoogleCloudPlatform/gke-managed-certs/master/deploy/managedcertificates-crd.yaml" // execCrd2Pulumi runs the crd2pulumi binary in a temporary directory -func execCrd2Pulumi(t *testing.T, lang, path string) { +func execCrd2Pulumi(t *testing.T, lang, path string, additionalValidation func(t *testing.T, path string)) { tmpdir, err := ioutil.TempDir("", "crd2pulumi_test") assert.Nil(t, err, "expected to create a temp dir for the CRD output") t.Cleanup(func() { @@ -48,7 +48,14 @@ func execCrd2Pulumi(t *testing.T, lang, path string) { crdCmd := exec.Command(binaryPath, langFlag, tmpdir, "--force", path) crdOut, err := crdCmd.CombinedOutput() t.Logf("%s %s=%s %s: output=\n%s", binaryPath, langFlag, tmpdir, path, crdOut) - assert.Nil(t, err, "expected crd2pulumi for '%s=%s %s' to succeed", langFlag, tmpdir, path) + if err != nil { + t.Fatalf("expected crd2pulumi for '%s=%s %s' to succeed", langFlag, tmpdir, path) + } + + // Run additional validation if provided. + if additionalValidation != nil { + additionalValidation(t, tmpdir) + } } // TestCRDsFromFile enumerates all CRD YAML files, and generates them in each language. @@ -60,7 +67,7 @@ func TestCRDsFromFile(t *testing.T) { name := fmt.Sprintf("%s-%s", lang, filepath.Base(path)) t.Run(name, func(t *testing.T) { t.Parallel() - execCrd2Pulumi(t, lang, path) + execCrd2Pulumi(t, lang, path, nil) }) } } @@ -74,7 +81,40 @@ func TestCRDsFromUrl(t *testing.T) { lang := lang t.Run(lang, func(t *testing.T) { t.Parallel() - execCrd2Pulumi(t, lang, gkeManagedCertsUrl) + execCrd2Pulumi(t, lang, gkeManagedCertsUrl, nil) }) } } + +// TestCRDsWithUnderscore tests that CRDs with underscores field names are camelCased for the +// generated types. Currently this test only runs for Python, and we're hardcoding the field name +// detection logic in the test for simplicity. This is brittle and we should improve this in the +// future. +// TODO: properly detect field names in the generated Python code instead of grep'ing for them. +func TestCRDsWithUnderscore(t *testing.T) { + // Callback function to run additional validation on the generated Python code after running + // crd2pulumi. + validateUnderscore := func(t *testing.T, path string) { + // Ensure inputs are camelCased. + filename := filepath.Join(path, "pulumi_crds", "juice", "v1alpha1", "_inputs.py") + t.Logf("validating underscored field names in %s", filename) + pythonInputs, err := ioutil.ReadFile(filename) + if err != nil { + t.Fatalf("expected to read generated Python code: %s", err) + } + assert.Contains(t, string(pythonInputs), "NetworkPolicySpecAppsIncomingArgs", "expected to find camelCased field name in generated Python code") + assert.NotContains(t, string(pythonInputs), "NetworkPolicySpecApps_incomingArgs", "expected to not find underscored field name in generated Python code") + + // Ensure outputs are camelCased. + filename = filepath.Join(path, "pulumi_crds", "juice", "v1alpha1", "outputs.py") + t.Logf("validating underscored field names in %s", filename) + pythonInputs, err = ioutil.ReadFile(filename) + if err != nil { + t.Fatalf("expected to read generated Python code: %s", err) + } + assert.Contains(t, string(pythonInputs), "NetworkPolicySpecAppsIncoming", "expected to find camelCased field name in generated Python code") + assert.NotContains(t, string(pythonInputs), "NetworkPolicySpecApps_incoming", "expected to not find underscored field name in generated Python code") + } + + execCrd2Pulumi(t, "python", "crds/underscored-types/networkpolicy.yaml", validateUnderscore) +} From 278ca28be3649a1a608bb324c993dfa0e2059c97 Mon Sep 17 00:00:00 2001 From: Ramon Quitales Date: Thu, 25 May 2023 15:04:20 -0700 Subject: [PATCH 4/4] Add changelog entry --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d791525..351e1d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ CHANGELOG ========= +## Unreleased +- Remove underscores in generated nested types (https://github.com/pulumi/crd2pulumi/pull/114) + ## 1.2.4 (2023-03-23) - Requires Go 1.19 or higher now to build - Fix issue [#108](https://github.com/pulumi/crd2pulumi/issues/108) - crd2pulumi generator splits types apart into duplicate entires in pulumiTypes.go and pulumiTypes1.go