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

Misspelled ExtraTypes dangling ref in schema #2463

Closed
t0yv0 opened this issue Apr 13, 2023 · 5 comments
Closed

Misspelled ExtraTypes dangling ref in schema #2463

t0yv0 opened this issue Apr 13, 2023 · 5 comments
Labels
area/providers impact/breaking Fixing this issue will require a breaking change impact/usability Something that impacts users' ability to use the product easily and intuitively kind/bug Some behavior is incorrect or out of spec resolution/duplicate This issue is a duplicate of another issue

Comments

@t0yv0
Copy link
Member

t0yv0 commented Apr 13, 2023

What happened?

jq .config.variables.region $(find . -name 'schema.json')                                                                                                                                                                                                                      ~/code/pulumi-aws-2
{
  "type": "string",
  "$ref": "#/types/aws:index/region:Region",
  "description": "The region where AWS operations will take place. Examples are us-east-1, us-west-2, etc.\n",
  "defaultInfo": {
    "environment": [
      "AWS_REGION",
      "AWS_DEFAULT_REGION"
    ]
  }
}

Note how it references #/types/aws:index/region:Region but there is no such type in the schema.

Instead we have slightly differently cased enum in here coming from ExtraTypes:

jq '.types|keys' $(find . -name 'schema.json') | grep Region | grep 'aws:index' | grep -v Filter                                                                                                                                                                               ~/code/pulumi-aws-2
  "aws:index/Region:Region",

Expected Behavior

No dangling refs in schema

Steps to reproduce

See JQ code above

Output of pulumi about

N/A

Additional context

Having to account for dangling refs complicates consuming code a bit.

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).

@t0yv0 t0yv0 added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Apr 13, 2023
@t0yv0
Copy link
Member Author

t0yv0 commented Apr 13, 2023

AFAIK the SDK projections ignore this ref and treat region as a *string. It might be that some user code out there relies on the ExtraType. Paths forward here

  • drop the dangling "$ref", will not break anything
  • drop ExtraType, possibly breaking user code we do not see
  • fix the ref typo - would changing a string to a string-based Enum be a non-breaking change over all language SDKs? not sure at the moment

@t0yv0
Copy link
Member Author

t0yv0 commented Apr 14, 2023

I think it's an authoring issue - https://github.com/pulumi/pulumi-aws/blob/07d08a0a50a459620181440fbb00ba497843f6c6/provider/resources.go#L3788-#L3788 and https://github.com/pulumi/pulumi-aws/blob/07d08a0a50a459620181440fbb00ba497843f6c6/provider/resources.go#L450-#L450

I've not done too deeply though - but I think the tokens originate from code in resoures.go, awsTypeDefaultFile seems suspect. It's hard to change this though as a change would be breaking. I've worked around it so no need to do it now

@kpitzen
Copy link
Contributor

kpitzen commented Apr 14, 2023

This, currently, seems like a breaking change from an SDK perspective, though we can likely ease that in the next major version via aliasing, similar to what we're doing in AZ Native 2.0. I'll mark this as such - thank you for reporting this @t0yv0 !

@kpitzen kpitzen added impact/usability Something that impacts users' ability to use the product easily and intuitively area/providers impact/breaking Fixing this issue will require a breaking change and removed needs-triage Needs attention from the triage team labels Apr 14, 2023
@t0yv0
Copy link
Member Author

t0yv0 commented Apr 14, 2023

Yes. The only non-breaking way would be to remove the "ref" from config - but maybe I'm mistaken there too.

@mikhailshilkov mikhailshilkov added the resolution/duplicate This issue is a duplicate of another issue label Nov 23, 2023
@mikhailshilkov
Copy link
Member

This is tracked more broadly in #2565

@mikhailshilkov mikhailshilkov closed this as not planned Won't fix, can't repro, duplicate, stale Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/providers impact/breaking Fixing this issue will require a breaking change impact/usability Something that impacts users' ability to use the product easily and intuitively kind/bug Some behavior is incorrect or out of spec resolution/duplicate This issue is a duplicate of another issue
Projects
None yet
Development

No branches or pull requests

3 participants