pulumi / pulumi-yaml

YAML language provider for Pulumi
Apache License 2.0
38 stars 11 forks source link

Broken YAML generated by the converter #539

Closed t0yv0 closed 6 months ago

t0yv0 commented 7 months ago

What happened?

There is a surprisingly pesky problem with YAML converter generating invalid YAML with repeated "example:" keys. During programhunt hackathon project with @wongstein and @mnlumi we noted that this is an extremely prevalent error for registry examples (in GCP and AWS), with 255 instances out of 1-2K samples:

error_type
resource, variable, or config value .. not found   780
pulumi stack init                                  260
pulumi stack init: "example:"                      255
unknown                                            170
...

https://github.com/pulumi/programhunt/blob/main/REPORT.md#registry-example-quality

More recently double-checking pulumi-azuread rollout results, we were able to further pin it down to something specifically introduced by adding the TF converter (pulumi convert) to the examples pipeline. Looks like it's either an intrinsic issue or we're not configuring it right to emit the "good" resource names.

It appears that Java is affected as well https://github.com/pulumi/pulumi-converter-terraform/issues/88 but the issue is not nearly as important there as the new program, while less aesthetically pleasing, still has a chance to work.

Example

Original 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 = "jdoe@hashicorp.com"
}

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 */
  ]
}

Converter generates this. Note how "example" cannot be listed as a dictionary key twice, so this is not something that gets accepted by the raw YAML parser even before Pulumi has a chance to do semantic processing. This is broken:

resources:
  example:
    type: azuread:User
    properties:
      displayName: J Doe
      owners:
        - ${current.objectId}
      password: notSecure123
      userPrincipalName: jdoe@hashicorp.com
  example:
    type: azuread:Group
    properties:
      displayName: MyGroup
      owners:
        - ${current.objectId}
      securityEnabled: true
      members:
        - ${example.objectId}
variables:
  current:
    fn::invoke:
      Function: azuread:getClientConfig
      Arguments: {}

Prior to introducing the converter the result that used to be generated is this:

resources:
  exampleUser:
    type: azuread:User
    properties:
      displayName: J Doe
      owners:
        - ${current.objectId}
      password: notSecure123
      userPrincipalName: jdoe@hashicorp.com
  exampleGroup:
    type: azuread:Group
    properties:
      displayName: MyGroup
      owners:
        - ${current.objectId}
      securityEnabled: true
      members:
        - ${exampleUser.objectId}
variables:
  current:
    fn::invoke:
      Function: azuread:getClientConfig
      Arguments: {}

Output of pulumi about

N/A

Additional context

N/A

Contributing

Vote on this issue by adding a 👍 reaction. To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

Frassle commented 7 months ago

Similar to pulumi/pulumi-converter-terraform#88 we need to use __logicalName in conversion to get this right.

Frassle commented 6 months ago

Ok so the converter does use __logicalName already, e.g. https://github.com/pulumi/pulumi-converter-terraform/blob/main/pkg/convert/testdata/programs/assets/pcl/main.pp#L2

And converting the above HCL to PCL with the converter shows this as well:

current = invoke("azuread:index/getClientConfig:getClientConfig", {})

resource "example" "azuread:index/user:User" {
  displayName       = "J Doe"
  owners            = [current.objectId]
  password          = "notSecure123"
  userPrincipalName = "jdoe@hashicorp.com"
}

resource "exampleResource" "azuread:index/group:Group" {
  __logicalName   = "example"
  displayName     = "MyGroup"
  owners          = [current.objectId]
  securityEnabled = true

  members = [example.objectId]
}

So I think this is a yaml codegen bug, not a converter issues. Moving repos.

t0yv0 commented 6 months ago

Thank you!!