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

Support (logical) name for resources and config variables #546

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

Zaid-Ajaj
Copy link
Contributor

@Zaid-Ajaj Zaid-Ajaj commented Jan 18, 2024

Description

Fixes #539, #477

This PR adds support for resources and config variables to have a logical name other than their lexical name which is used to reference the resources or variables in the YAML program.

Added as a top-level field:

name: simple-yaml
runtime: yaml
resources:
  bucket:
    type: aws:s3:Bucket
    name: my-bucket
    properties:
      website:
        indexDocument: index.html

Where the bucket is the lexical name that is used for the program evaluator and my-bucket is the name that is used when registering the resource. In TypeScript it would look like this:

const bucket = new Bucket("my-bucket", ...)

Similarly, configuration variables can now have a logical name as well where at runtime, it will be the key used to query the config data provided by Pulumi CLI whereas their lexical name is just how they are defined and referenced in the program.

Program-gen for YAML has been updated. Take this PCL:

config configLexicalName string {
  __logicalName = "cC-Charlie_charlie.😃⁉️"
}

resource resourceLexicalName "random:index/randomPet:RandomPet" {
  // not necessarily a valid logical name, just testing that it passes through to codegen unmodified
  __logicalName = "aA-Alpha_alpha.🤯⁉️"

  prefix = configLexicalName
}

output outputLexicalName {
  __logicalName = "bB-Beta_beta.💜⁉"
  value = resourceLexicalName.id
}

Becomes:

configuration:
  configLexicalName:
    type: string
    name: "cC-Charlie_charlie.\U0001F603⁉️"
resources:
  resourceLexicalName:
    type: random:RandomPet
    name: "aA-Alpha_alpha.\U0001F92F⁉️"
    properties:
      prefix: ${configLexicalName}
outputs:
  "bB-Beta_beta.\U0001F49C⁉": ${resourceLexicalName.id}

Outputs just keep using their logical name because their lexical name cannot be referenced anywhere

@Zaid-Ajaj Zaid-Ajaj requested a review from a team January 18, 2024 18:16
@Frassle
Copy link
Member

Frassle commented Jan 18, 2024

Outputs just keep using their logical name because their lexical name cannot be referenced anywhere

Why do we even support __logicalName on outputs in PCL? I don't think any language supports referencing of outputs, this looks like something we should clean up.

@Frassle
Copy link
Member

Frassle commented Jan 18, 2024

Why do we even support __logicalName on outputs in PCL?

My first thought was maybe block labels couldn't be arbitary strings, and that's true for the first label but that's always going to be "output". The second block label is fine to be a string literal: https://github.com/hashicorp/hcl/blob/main/hclsyntax/spec.md#structural-elements

We should sanitize this.

@Zaid-Ajaj
Copy link
Contributor Author

Why do we even support __logicalName on outputs in PCL? I don't think any language supports referencing of outputs, this looks like something we should clean up.

Indeed 💯

@Zaid-Ajaj Zaid-Ajaj changed the title Support logicalName for resources and config variables Support (logical) name for resources and config variables Jan 18, 2024
github-merge-queue bot pushed a commit to pulumi/pulumi that referenced this pull request Jan 18, 2024
Prompted by pulumi/pulumi-yaml#546

At some point we added `__logicalName` to outputs to match
resources/config. But outputs don't actually need lexical and logical
names, they only have logical names. In the cases where `__logicalName`
was set the lexical name was totally ignored.

We can simply just use the block label (block labels can be arbitrary
strings) and eventually deprecate the `__logicalName` option on
`output`.
Copy link
Member

@Frassle Frassle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM great work for getting this done so quickly, will really help unblock providers. Glad it wasn't too gnarly.

@Zaid-Ajaj Zaid-Ajaj merged commit 70edebe into main Jan 18, 2024
5 checks passed
@Zaid-Ajaj Zaid-Ajaj deleted the zaid/support-logical-names branch January 18, 2024 23:03
github-merge-queue bot pushed a commit to pulumi/pulumi that referenced this pull request Jan 23, 2024
Upgrade pulumi-yaml to the latest version, which includes
pulumi/pulumi-yaml#546
github-merge-queue bot pushed a commit to pulumi/pulumi that referenced this pull request Jan 23, 2024
Upgrade pulumi-yaml to the latest version, which includes
pulumi/pulumi-yaml#546
@AaronFriel
Copy link
Contributor

@Frassle outputs have logical names because they can have names that are not legal identifiers using the export facility.

pulumi/pulumi#9464 (comment)

@Frassle
Copy link
Member

Frassle commented Mar 12, 2024

Yeh we worked that out, but outputs only have one name. Unlike resources which have a source name and a logical name outputs just have a logical name. See pulumi/pulumi#15180 for the follow up on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broken YAML generated by the converter
3 participants