From d1aed25ee6db873dea2a689ec445cbb1e8a2b344 Mon Sep 17 00:00:00 2001 From: Will Jones Date: Thu, 19 Dec 2024 15:11:08 +0000 Subject: [PATCH] Centralize SDK generation logic Presently, the majority of Java code generation is driven by the `pulumi-java-gen` binary, since Java usage began before we had time to implement the `Generate*` family of language host gRPC methods. Recently, these gRPC methods were implemented, to support (among other things) conformance testing. Unfortunately, while both routes end in `pkg/codegen/java`'s `Generate*` functions, each had accumulated its own special "setup logic" ahead of the call into `pkg/codegen`. This commit attempts to sort this out, pushing all that logic into `pkg/codegen` so that both routes behave identically. As a result of this, we should be able to deprecate `pulumi-java-gen` more safely when the time comes, remove direct build-time dependencies on `pulumi-java` from `pulumi/pulumi` and fix some issues that have arisen as a result of the historic differences, such as #1404 (which looks like it may have already been fixed, but this should cement it), and #1508 Closes #1404 Fixes #1508 --- pkg/cmd/pulumi-java-gen/generate.go | 27 +++- pkg/cmd/pulumi-language-java/main.go | 69 ++++----- .../pulumi-java-gen => codegen/java}/dedup.go | 52 ++++--- pkg/codegen/java/dedup_test.go | 138 ++++++++++++++++++ pkg/codegen/java/gen.go | 53 +++++++ pkg/codegen/java/gen_test.go | 2 +- pkg/codegen/java/package_info.go | 12 ++ 7 files changed, 286 insertions(+), 67 deletions(-) rename pkg/{cmd/pulumi-java-gen => codegen/java}/dedup.go (67%) create mode 100644 pkg/codegen/java/dedup_test.go diff --git a/pkg/cmd/pulumi-java-gen/generate.go b/pkg/cmd/pulumi-java-gen/generate.go index 15832b9a945..8ba1511862f 100644 --- a/pkg/cmd/pulumi-java-gen/generate.go +++ b/pkg/cmd/pulumi-java-gen/generate.go @@ -16,12 +16,16 @@ package main import ( "fmt" + "os" "path/filepath" "github.com/blang/semver" + "github.com/hashicorp/hcl/v2" javagen "github.com/pulumi/pulumi-java/pkg/codegen/java" pschema "github.com/pulumi/pulumi/pkg/v3/codegen/schema" + "github.com/pulumi/pulumi/sdk/v3/go/common/diag" + "github.com/pulumi/pulumi/sdk/v3/go/common/util/cmdutil" ) type generateJavaOptions struct { @@ -53,10 +57,11 @@ func generateJava(cfg generateJavaOptions) error { return fmt.Errorf("failed to read schema from %s: %w", cfg.Schema, err) } - pkgSpec, err := dedupTypes(rawPkgSpec) + pkgSpec, diags, err := javagen.DeduplicateTypes(rawPkgSpec) if err != nil { return fmt.Errorf("failed to dedup types in schema from %s: %w", cfg.Schema, err) } + printDiagnostics(diags) pkg, err := pschema.ImportSpec(*pkgSpec, nil) if err != nil { @@ -80,7 +85,13 @@ func generateJava(cfg generateJavaOptions) error { if err != nil { return err } - files, err := javagen.GeneratePackage("pulumi-java-gen", pkg, extraFiles, cfg.Local) + files, err := javagen.GeneratePackage( + "pulumi-java-gen", + pkg, + extraFiles, + nil, /*localDependencies*/ + cfg.Local, + ) if err != nil { return err } @@ -99,3 +110,15 @@ func generateJava(cfg generateJavaOptions) error { return nil } + +// printDiagnostics prints the given diagnostics to stdout and stderr. +func printDiagnostics(diagnostics hcl.Diagnostics) { + sink := diag.DefaultSink(os.Stdout, os.Stderr, diag.FormatOptions{Color: cmdutil.GetGlobalColorization()}) + for _, diagnostic := range diagnostics { + if diagnostic.Severity == hcl.DiagError { + sink.Errorf(diag.Message("", "%s"), diagnostic) + } else { + sink.Warningf(diag.Message("", "%s"), diagnostic) + } + } +} diff --git a/pkg/cmd/pulumi-language-java/main.go b/pkg/cmd/pulumi-language-java/main.go index ade29127e1e..e78071b00d7 100644 --- a/pkg/cmd/pulumi-language-java/main.go +++ b/pkg/cmd/pulumi-language-java/main.go @@ -19,6 +19,7 @@ import ( "time" pbempty "github.com/golang/protobuf/ptypes/empty" + "github.com/hashicorp/hcl/v2" "github.com/pkg/errors" hclsyntax "github.com/pulumi/pulumi/pkg/v3/codegen/hcl2/syntax" "github.com/pulumi/pulumi/pkg/v3/codegen/pcl" @@ -32,7 +33,6 @@ import ( "github.com/pulumi/pulumi/sdk/v3/go/common/util/rpcutil" "github.com/pulumi/pulumi/sdk/v3/go/common/workspace" pulumirpc "github.com/pulumi/pulumi/sdk/v3/proto/go" - "golang.org/x/exp/maps" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" "google.golang.org/protobuf/types/known/structpb" @@ -703,57 +703,40 @@ func (host *javaLanguageHost) GeneratePackage( return nil, err } - pkg, diags, err := schema.BindSpec(spec, loader) + diags := hcl.Diagnostics{} + + // Historically, Java has "deduplicated" PackageSpecs to reduce sets of multiple types whose names differ only in + // case down to just one type that is then shared (assuming that, apart from name, the types are otherwise + // identical). We thus perform that deduplication here before we bind the schema and resolve any references. + dedupedSpec, dedupeDiags, err := codegen.DeduplicateTypes(&spec) if err != nil { return nil, err } - rpcDiagnostics := plugin.HclDiagnosticsToRPCDiagnostics(diags) - if diags.HasErrors() { + diags = diags.Extend(dedupeDiags) + if dedupeDiags.HasErrors() { return &pulumirpc.GeneratePackageResponse{ - Diagnostics: rpcDiagnostics, + Diagnostics: plugin.HclDiagnosticsToRPCDiagnostics(diags), }, nil } - if pkg.Description == "" { - pkg.Description = " " - } - if pkg.Repository == "" { - pkg.Repository = "https://example.com" - } - - // Presently, we only support generating Java SDKs which use Gradle as a build system. Specify that here, as well as - // the set of dependencies that all generated SDKs rely on. - pkgInfo := codegen.PackageInfo{ - BuildFiles: "gradle", - Dependencies: map[string]string{ - "com.google.code.gson:gson": "2.8.9", - "com.google.code.findbugs:jsr305": "3.0.2", - }, + pkg, bindDiags, err := schema.BindSpec(*dedupedSpec, loader) + if err != nil { + return nil, err } - - repositories := map[string]bool{} - - for name, dep := range req.LocalDependencies { - parts := strings.Split(dep, ":") - if len(parts) < 3 { - return nil, fmt.Errorf( - "invalid dependency for %s %s; must be of the form groupId:artifactId:version[:repositoryPath]", - name, dep, - ) - } - - k := parts[0] + ":" + parts[1] - pkgInfo.Dependencies[k] = parts[2] - - if len(parts) == 4 { - repositories[parts[3]] = true - } + diags = diags.Extend(bindDiags) + if bindDiags.HasErrors() { + return &pulumirpc.GeneratePackageResponse{ + Diagnostics: plugin.HclDiagnosticsToRPCDiagnostics(diags), + }, nil } - pkgInfo.Repositories = maps.Keys(repositories) - pkg.Language["java"] = pkgInfo - - files, err := codegen.GeneratePackage("pulumi-language-java", pkg, req.ExtraFiles, req.Local) + files, err := codegen.GeneratePackage( + "pulumi-language-java", + pkg, + req.ExtraFiles, + req.LocalDependencies, + req.Local, + ) if err != nil { return nil, err } @@ -772,7 +755,7 @@ func (host *javaLanguageHost) GeneratePackage( } return &pulumirpc.GeneratePackageResponse{ - Diagnostics: rpcDiagnostics, + Diagnostics: plugin.HclDiagnosticsToRPCDiagnostics(diags), }, nil } diff --git a/pkg/cmd/pulumi-java-gen/dedup.go b/pkg/codegen/java/dedup.go similarity index 67% rename from pkg/cmd/pulumi-java-gen/dedup.go rename to pkg/codegen/java/dedup.go index eb5267e6332..6b96b3ed602 100644 --- a/pkg/cmd/pulumi-java-gen/dedup.go +++ b/pkg/codegen/java/dedup.go @@ -1,6 +1,6 @@ -// Copyright 2022, Pulumi Corporation. All rights reserved. +// Copyright 2024, Pulumi Corporation. All rights reserved. -package main +package java import ( "bytes" @@ -9,15 +9,16 @@ import ( "reflect" "strings" - pschema "github.com/pulumi/pulumi/pkg/v3/codegen/schema" + "github.com/hashicorp/hcl/v2" + "github.com/pulumi/pulumi/pkg/v3/codegen/schema" ) -// Detects cases when identical types have similar names modulo case -// such as `azure-native:network:IpAllocationMethod` vs -// `azure-native:network:IPAllocationMethod`, deterministically picks -// one of these names, and rewrites the schema as if there was only -// one such type. -func dedupTypes(spec *pschema.PackageSpec) (*pschema.PackageSpec, error) { +// DeduplicateTypes detects multiple types in a PackageSpec whose names are the same modulo case, such as +// `azure-native:network:IpAllocationMethod` and `azure-native:network:IPAllocationMethod`, deterministically picks one +// of these names, and rewrites the schema as if there was only one such type. +func DeduplicateTypes(spec *schema.PackageSpec) (*schema.PackageSpec, hcl.Diagnostics, error) { + diags := hcl.Diagnostics{} + normalizedTokens := map[string]string{} for typeToken := range spec.Types { key := strings.ToUpper(typeToken) @@ -41,12 +42,12 @@ func dedupTypes(spec *pschema.PackageSpec) (*pschema.PackageSpec, error) { var buf bytes.Buffer if err := json.NewEncoder(&buf).Encode(spec); err != nil { - return nil, err + return nil, nil, err } var rawSchema interface{} if err := json.NewDecoder(bytes.NewReader(buf.Bytes())).Decode(&rawSchema); err != nil { - return nil, err + return nil, nil, err } types := map[string]interface{}{} @@ -64,13 +65,19 @@ func dedupTypes(spec *pschema.PackageSpec) (*pschema.PackageSpec, error) { transformJSONTree(stripDescription, types[newToken]), ) if eq { - fmt.Printf("WARN renaming %s to %s in the schema\n", - oldToken, newToken) + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: fmt.Sprintf("Renaming '%s' to '%s' in the schema", oldToken, newToken), + }) delete(types, oldToken) } else { - fmt.Printf("WARN not renaming %s to %s in the schema "+ - "because they differ structurally\n", - oldToken, newToken) + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: fmt.Sprintf( + "Not renaming '%s' to '%s' in the schema because they differ structurally", + oldToken, newToken, + ), + }) } } @@ -89,7 +96,10 @@ func dedupTypes(spec *pschema.PackageSpec) (*pschema.PackageSpec, error) { return node } if r, isRenamed := renamedRefs[s]; isRenamed { - fmt.Printf("Rewritten %s to %s\n", s, r) + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: fmt.Sprintf("Rewrote reference '%s' to '%s'", s, r), + }) return r } return node @@ -100,15 +110,15 @@ func dedupTypes(spec *pschema.PackageSpec) (*pschema.PackageSpec, error) { buf.Reset() if err := json.NewEncoder(&buf).Encode(&rawSchema); err != nil { - return nil, err + return nil, nil, err } - var fixedSpec pschema.PackageSpec + var fixedSpec schema.PackageSpec if err := json.NewDecoder(bytes.NewReader(buf.Bytes())).Decode(&fixedSpec); err != nil { - return nil, err + return nil, nil, err } - return &fixedSpec, nil + return &fixedSpec, diags, nil } func transformJSONTree(t func(interface{}) interface{}, tree interface{}) interface{} { diff --git a/pkg/codegen/java/dedup_test.go b/pkg/codegen/java/dedup_test.go new file mode 100644 index 00000000000..60a3f3e5aa6 --- /dev/null +++ b/pkg/codegen/java/dedup_test.go @@ -0,0 +1,138 @@ +// Copyright 2024, Pulumi Corporation. All rights reserved. + +package java + +import ( + "testing" + + "github.com/hashicorp/hcl/v2" + "github.com/pulumi/pulumi/pkg/v3/codegen/schema" + "github.com/stretchr/testify/require" +) + +func TestDeduplicateTypes(t *testing.T) { + cases := []struct { + name string + input *schema.PackageSpec + expectedSpec *schema.PackageSpec + expectedDiags hcl.Diagnostics + }{ + { + name: "no duplicates", + input: &schema.PackageSpec{}, + expectedSpec: &schema.PackageSpec{}, + expectedDiags: hcl.Diagnostics{}, + }, + { + name: "duplicates, lowercase", + input: &schema.PackageSpec{ + Types: map[string]schema.ComplexTypeSpec{ + "azure-native:network:ipallocationmethod": {}, + "azure-native:network:IpAllocationMethod": {}, + }, + }, + expectedSpec: &schema.PackageSpec{ + Types: map[string]schema.ComplexTypeSpec{ + "azure-native:network:IpAllocationMethod": {}, + }, + }, + expectedDiags: hcl.Diagnostics{ + &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Renaming 'azure-native:network:ipallocationmethod' to " + + "'azure-native:network:IpAllocationMethod' in the schema", + }, + }, + }, + { + name: "duplicates, uppercase", + input: &schema.PackageSpec{ + Types: map[string]schema.ComplexTypeSpec{ + "azure-native:network:IPAllocationMethod": {}, + "azure-native:network:IpAllocationMethod": {}, + }, + }, + expectedSpec: &schema.PackageSpec{ + Types: map[string]schema.ComplexTypeSpec{ + "azure-native:network:IPAllocationMethod": {}, + }, + }, + expectedDiags: hcl.Diagnostics{ + &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Renaming 'azure-native:network:IpAllocationMethod' to " + + "'azure-native:network:IPAllocationMethod' in the schema", + }, + }, + }, + { + name: "multiple duplicates", + input: &schema.PackageSpec{ + Types: map[string]schema.ComplexTypeSpec{ + "azure-native:network:ipallocationmethod": {}, + "azure-native:network:IPAllocationMethod": {}, + "azure-native:network:IpAllocationMethod": {}, + }, + }, + expectedSpec: &schema.PackageSpec{ + Types: map[string]schema.ComplexTypeSpec{ + "azure-native:network:IPAllocationMethod": {}, + }, + }, + expectedDiags: hcl.Diagnostics{ + &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Renaming 'azure-native:network:ipallocationmethod' to " + + "'azure-native:network:IPAllocationMethod' in the schema", + }, + &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Renaming 'azure-native:network:IpAllocationMethod' to " + + "'azure-native:network:IPAllocationMethod' in the schema", + }, + }, + }, + { + name: "multiple duplicates and non-duplicates", + input: &schema.PackageSpec{ + Types: map[string]schema.ComplexTypeSpec{ + "azure-native:network:other": {}, + "azure-native:network:ipsallocationmethod": {}, + "azure-native:network:ip_allocationmethod": {}, + "azure-native:network:ipallocationmethod": {}, + "azure-native:network:IPAllocationMethod": {}, + "azure-native:network:IpAllocationMethod": {}, + }, + }, + expectedSpec: &schema.PackageSpec{ + Types: map[string]schema.ComplexTypeSpec{ + "azure-native:network:other": {}, + "azure-native:network:ipsallocationmethod": {}, + "azure-native:network:ip_allocationmethod": {}, + "azure-native:network:IPAllocationMethod": {}, + }, + }, + expectedDiags: hcl.Diagnostics{ + &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Renaming 'azure-native:network:ipallocationmethod' to " + + "'azure-native:network:IPAllocationMethod' in the schema", + }, + &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Renaming 'azure-native:network:IpAllocationMethod' to " + + "'azure-native:network:IPAllocationMethod' in the schema", + }, + }, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + actualSpec, actualDiags, err := DeduplicateTypes(c.input) + require.NoError(t, err) + require.ElementsMatch(t, c.expectedDiags, actualDiags) + require.Equal(t, c.expectedSpec, actualSpec) + }) + } +} diff --git a/pkg/codegen/java/gen.go b/pkg/codegen/java/gen.go index b0f8c8255ba..568b3985bdb 100644 --- a/pkg/codegen/java/gen.go +++ b/pkg/codegen/java/gen.go @@ -17,6 +17,7 @@ import ( "github.com/pulumi/pulumi/pkg/v3/codegen" "github.com/pulumi/pulumi/pkg/v3/codegen/schema" "github.com/pulumi/pulumi/sdk/v3/go/common/util/contract" + "golang.org/x/exp/maps" "github.com/pulumi/pulumi-java/pkg/codegen/java/names" ) @@ -2043,9 +2044,19 @@ func generateModuleContextMap(tool string, pkg *schema.Package) (map[string]*mod panic(fmt.Sprintf("Failed to cast `pkg.Language[\"java\"]`=%v to `PackageInfo`", raw)) } } + javaInfo = javaInfo. WithDefaultDependencies(). WithJavaSdkDependencyDefault(DefaultSdkVersion) + + // All packages that SupportPack (which in some sense reflects the latest version of the schema) should use + // Gradle if no build system has been explicitly specified. + if p.SupportPack() { + if javaInfo.BuildFiles == "" { + javaInfo.BuildFiles = "gradle" + } + } + info = &javaInfo infos[def] = info } @@ -2223,13 +2234,55 @@ func GeneratePackage( tool string, pkg *schema.Package, extraFiles map[string][]byte, + localDependencies map[string]string, local bool, ) (map[string][]byte, error) { + // Presently, Gradle is the primary build system we support for generated SDKs. Later on, when we validate the + // package in order to produce build system artifacts, we'll need a description and repository. To this end, we + // ensure there are non-empty values for these fields here. + if pkg.Description == "" { + pkg.Description = " " + } + if pkg.Repository == "" { + pkg.Repository = "https://example.com" + } + modules, info, err := generateModuleContextMap(tool, pkg) if err != nil { return nil, err } + // We need to ensure that local dependencies are reflected in the lists of dependencies and repositories in the Java + // PackageInfo. + pkgOverrides := PackageInfo{} + + dependencies := map[string]string{} + repositories := map[string]bool{} + for name, dep := range localDependencies { + // A local dependency has the form groupId:artifactId:version[:repositoryPath]. We'll parse this and add an + // entry to the dependency map for groupId:artifactId -> version, and add the repositoryPath to the list of + // repositories if it's present. + parts := strings.Split(dep, ":") + if len(parts) < 3 { + return nil, fmt.Errorf( + "invalid dependency for %s %s; must be of the form groupId:artifactId:version[:repositoryPath]", + name, dep, + ) + } + + k := parts[0] + ":" + parts[1] + dependencies[k] = parts[2] + + if len(parts) == 4 { + repositories[parts[3]] = true + } + } + + pkgOverrides.Dependencies = dependencies + pkgOverrides.Repositories = maps.Keys(repositories) + + pkg.Language["java"] = info.With(pkgOverrides) + // Generate each module. files := fs{} for p, f := range extraFiles { diff --git a/pkg/codegen/java/gen_test.go b/pkg/codegen/java/gen_test.go index 31598a912ed..9e9047fcf42 100644 --- a/pkg/codegen/java/gen_test.go +++ b/pkg/codegen/java/gen_test.go @@ -227,7 +227,7 @@ func TestGeneratePackage(t *testing.T) { pkg.Language = map[string]interface{}{ "java": testCase.packageInfo, } - return GeneratePackage(tool, pkg, extraFiles, false) + return GeneratePackage(tool, pkg, extraFiles, nil, false) }, Language: "java", TestCases: []*test.SDKTest{testCase.sdkTest}, diff --git a/pkg/codegen/java/package_info.go b/pkg/codegen/java/package_info.go index faf39ed68bb..454e0c18155 100644 --- a/pkg/codegen/java/package_info.go +++ b/pkg/codegen/java/package_info.go @@ -16,6 +16,7 @@ package java import ( "github.com/blang/semver" + "golang.org/x/exp/maps" ) const defaultBasePackage = "com.pulumi." @@ -135,6 +136,17 @@ func (i PackageInfo) With(overrides PackageInfo) PackageInfo { result.Dependencies[k] = v } } + if len(overrides.Repositories) > 0 { + repositories := map[string]bool{} + for _, repo := range result.Repositories { + repositories[repo] = true + } + for _, repo := range overrides.Repositories { + repositories[repo] = true + } + + result.Repositories = maps.Keys(repositories) + } if overrides.GradleNexusPublishPluginVersion != "" { result.GradleNexusPublishPluginVersion = overrides.GradleNexusPublishPluginVersion }