Using the converter renders less informative Java var names

[t0yv0]

Hello!

• Vote on this issue by adding a 👍 reaction
• If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

This feels like a regression in the quality of the generated programs when introducing the converter into the pipeline.

Original PR where this is discovered: pulumi/pulumi-azuread#484

All variables are held equal during the comparison, the change is just introducing the pulumi convert plus the converter at 1.0.11 version into the pipeline.

Example HCL:

data "azuread_client_config" "current" {}

resource "azuread_user" "example" {
  display_name        = "J Doe"
  owners              = [data.azuread_client_config.current.object_id]
  password            = "notSecure123"
  user_principal_name = "[email protected]"
}

resource "azuread_group" "example" {
  display_name     = "MyGroup"
  owners           = [data.azuread_client_config.current.object_id]
  security_enabled = true

  members = [
    azuread_user.example.object_id,
    /* more users */
  ]
}

Java rendering without the converter:

package generated_program;

import com.pulumi.Context;
import com.pulumi.Pulumi;
import com.pulumi.core.Output;
import com.pulumi.azuread.AzureadFunctions;
import com.pulumi.azuread.User;
import com.pulumi.azuread.UserArgs;
import com.pulumi.azuread.Group;
import com.pulumi.azuread.GroupArgs;
import java.util.List;
import java.util.ArrayList;
import java.util.Map;
import java.io.File;
import java.nio.file.Files;
import java.nio.file.Paths;

public class App {
    public static void main(String[] args) {
        Pulumi.run(App::stack);
    }

    public static void stack(Context ctx) {
        final var current = AzureadFunctions.getClientConfig();

        var exampleUser = new User("exampleUser", UserArgs.builder()        
            .displayName("J Doe")
            .owners(current.applyValue(getClientConfigResult -> getClientConfigResult.objectId()))
            .password("notSecure123")
            .userPrincipalName("[email protected]")
            .build());

        var exampleGroup = new Group("exampleGroup", GroupArgs.builder()        
            .displayName("MyGroup")
            .owners(current.applyValue(getClientConfigResult -> getClientConfigResult.objectId()))
            .securityEnabled(true)
            .members(exampleUser.objectId())
            .build());

    }
}

Java rendering with the converter:

package generated_program;

import com.pulumi.Context;
import com.pulumi.Pulumi;
import com.pulumi.core.Output;
import com.pulumi.azuread.AzureadFunctions;
import com.pulumi.azuread.User;
import com.pulumi.azuread.UserArgs;
import com.pulumi.azuread.Group;
import com.pulumi.azuread.GroupArgs;
import java.util.List;
import java.util.ArrayList;
import java.util.Map;
import java.io.File;
import java.nio.file.Files;
import java.nio.file.Paths;

public class App {
    public static void main(String[] args) {
        Pulumi.run(App::stack);
    }

    public static void stack(Context ctx) {
        final var current = AzureadFunctions.getClientConfig();

        var example = new User("example", UserArgs.builder()        
            .displayName("J Doe")
            .owners(current.applyValue(getClientConfigResult -> getClientConfigResult.objectId()))
            .password("notSecure123")
            .userPrincipalName("[email protected]")
            .build());

        var exampleResource = new Group("exampleResource", GroupArgs.builder()        
            .displayName("MyGroup")
            .owners(current.applyValue(getClientConfigResult -> getClientConfigResult.objectId()))
            .securityEnabled(true)
            .members(example.objectId())
            .build());

    }
}

Affected area/feature

[Frassle]

Yeh we need to make use of __logicalName to not change resource names in conversion.

[t0yv0]

Looks like the same problem is affecting not just Java, but python, node and Go. I had a build problem that was masking that. But now it's uncovered.

[Frassle]

This is because the converter is using "Resource" as a suffix to make the resource names unique rather than using the type of the resource. Should be a fairly easy fix.

[t0yv0]

I think this looks done as of this morning!

[justinvp]

@Frassle, what's involved to fix this? Is this a quick fix that you or @Zaid-Ajaj could pick up?

[t0yv0]

pulumi/pulumi-azure#1377 specifically does not have this problem anymore. After it picked up 1.0.13 in pulumi/pulumi-azure@f9d3f3b so I think 1.0.13 must contain a fix.

[Frassle]

#107 would have fixed this. There's a couple of outstanding questions from that implementation though.

Firstly, if variables conflict (i.e. two terraform blocks both called "example" but with different type) should we use the type name on all of them, or should the first be allowed to claim just "example"?

Secondly, should we be suffixing with the type always, not just when there's name conflicts?

Obviously if we answer yes to the second question the first question is redundant.

[t0yv0]

It's aesthetics... I could go either way. I like exampleUser, exampleGroup personally better than example, exampleGroup but it does not matter all that much.