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

Bicep types are not generated for resources that use "name" as a polymorphic discriminator #657

Closed
anthony-c-martin opened this issue Oct 15, 2020 · 8 comments · Fixed by #1950
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@anthony-c-martin
Copy link
Member

anthony-c-martin commented Oct 15, 2020

First reported under #656.

Some RPs define multiple resources of the same resourceType, with a fixed set of (or enum-valued) name values, with a different body shape depending on the value of name. This is problematic for child resources, because Bicep expects the fully-qualified name of the type.

resource myWebConfig 'Microsoft.Web/sites/config@2020-06-01' = {
  // because we're forced to use string interpolation, 'name' is not a compile-time value, and we cannot use it as a discriminator key to select the right type for the resource body
  name: '${myWebsite.name}/authsettings'
}
@ghost ghost added the Needs: Triage 🔍 label Oct 15, 2020
@anthony-c-martin
Copy link
Member Author

anthony-c-martin commented Oct 15, 2020

Some possible solutions:

  1. Special-case suffix-matching for interpolated strings when picking a path in an object discriminator (e.g. as long as the string ends in /authsettings, we can reliably pick a path).
  2. Introduce a parent syntax so that child resources only need specify the child name - e.g.:
resource myWebConfig 'Microsoft.Web/sites/config@2020-06-01' = {
  parent: myWebsite // reference to parent resource
  name: 'authsettings' // this is now a constant type and can be used as a discriminator key
}
  1. Create a merged object with all possible properties. This will be problematic for Microsoft.Web/sites/config in particular, because one of the polymorphic types is an open dictionary.

@alex-frankel alex-frankel added discussion This is a discussion issue and not a change proposal. and removed Needs: Triage 🔍 labels Oct 19, 2020
@alex-frankel alex-frankel added this to the v0.3 milestone Oct 21, 2020
@alex-frankel alex-frankel added the enhancement New feature or request label Oct 21, 2020
@slavizh
Copy link
Contributor

slavizh commented Oct 23, 2020

btw as this is very much related to app services APIs is there any plan to have better intellisense for that API. Overall that API is tone of the worse as it is one thing build on top of another and they continue to do that (new logic app). For example depending on the kind value which is string and there you can have one value or multiple values separated by comma the properties for Microsoft.Web/sites resource will be different. Meaning that there could be some properties that are relevant for web apps only and some others only for container web apps and third for container web apps with linux, etc.

@bmoore-msft
Copy link
Contributor

These are a pain for tooling... IDK if I completely follow your proposal but I think it has legs. Basically, for a singleton we'd have to give some indication, somewhere that's that what is it. Ideally, we could to this in the tooling/metadata rather than have the user specify. IOW, we know that web's "config" resource has additional information to specify it's "shape" - as @slavizh mentions this is usually "kind" or "sku" but the web api's predate those properties. We could emulate that behavior in these singleton's - either by the user specifying or by tooling keying off of the name somehow.

Not sure we need to solve this in V1 but good to keep in mind as we look at things like #127

@alex-frankel
Copy link
Collaborator

At minimum, we should treat this as "any" type so that it does not throw an error/warning

@TSunny007 TSunny007 removed their assignment Jan 4, 2021
@miqm
Copy link
Collaborator

miqm commented Jan 20, 2021

any update/ETA on this?

@alex-frankel
Copy link
Collaborator

No work has started. I think plan is to revisit this once #127 is done or as part of implementation of that feature. If using the new child resource syntax, we should be able to properly validate these resources

@miqm
Copy link
Collaborator

miqm commented Feb 10, 2021

I noticed, that this might be also a problem in type generation itself, see Azure/bicep-types-az#101

@anthony-c-martin
Copy link
Member Author

I noticed, that this might be also a problem in type generation itself, see Azure/bicep-types-az#101

You're correct - there's also some work we need to do type generation for this issue.

@alex-frankel alex-frankel removed the discussion This is a discussion issue and not a change proposal. label Mar 8, 2021
@anthony-c-martin anthony-c-martin self-assigned this Mar 17, 2021
@ghost ghost locked as resolved and limited conversation to collaborators May 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants