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

Improve Type checking on related properties #5861

Open
afscrome opened this issue Feb 3, 2022 · 1 comment
Open

Improve Type checking on related properties #5861

afscrome opened this issue Feb 3, 2022 · 1 comment
Labels
enhancement New feature or request type system

Comments

@afscrome
Copy link
Contributor

afscrome commented Feb 3, 2022

Is your feature request related to a problem? Please describe.
I spent a while today investigating why the following template didn't work. The problem ultimately being that the logAnalytics resource was using the wrong type (it should be Microsoft.OperationalInsights/workspace), and thus produced the wrong resource id

resource appInsights 'Microsoft.Insights/components@2020-02-02' = {
  properties: {
    WorkspaceResourceId: logAnalytics.id
  }
}

resource logAnalytics 'Microsoft.OperationalInsights/clusters@2021-06-01' existing = {
  name: workspaceName
}

This feels like an error the bicep type system could capture.

Describe the solution you'd like
Ideally bicep should know that WorkspaceResourceId is looking for a resource id of a Microsoft.OperationalInsights/workspace. The bicep type system could then know that logAnalytics.id is not a resource id of Microsoft.OperationalInsights/workspace and produce a compile error.

This would require some kind of annotation in the API specs so that bicep would know that WorkspaceResoruceId maps to the resoruce id of a Microsoft.OperationalInsights/workspace resource. OpenApi's format property could be useful here e.g.

        "WorkspaceResourceId": {
          "type": "string",
          "format": "Microsoft.OperationalInsights/workspace.id",
          "description": "Resource Id of the log analytics workspace which the data will be ingested to. This property is required to create an application with this API version. Applications from older versions will not have this property."
        },

https://github.com/Azure/azure-rest-api-specs/blob/5582a35deb1bfa4aa22bac8f1d51b7934ead94ac/specification/applicationinsights/resource-manager/Microsoft.Insights/stable/2020-02-02/components_API.json#L585-L588

The problem with this solution is you still need to support arbitrary strings for backwards compatibility. This brings you back to the problem with name for child resources today where the compiler sometimes detects errors, but not always

An alternative solution for scenarios like this is to generate a property that takes the resource type directly.

resource appInsights 'Microsoft.Insights/components@2020-02-02' = {
  properties: {
    Workspace: logAnalytics
  }
}

The bicep compiler would ultimately lower this to use WorkspaceResourceId:

resource appInsights 'Microsoft.Insights/components@2020-02-02' = {
  properties: {
    WorkspaceResourceId: logAnalytics.id
  }
}

This has the advantage of knowing that when you use Workspace, you always get the compiler validation, and when you use WorkspaceResourceId you never get validation (beyond it being a string), but it may cause confusion now there is both Workspace and WorkspaceResourceId.

@afscrome afscrome added the enhancement New feature or request label Feb 3, 2022
@ghost ghost added the Needs: Triage 🔍 label Feb 3, 2022
@alex-frankel
Copy link
Collaborator

We agree that Bicep should be able to capture these sorts of things and it could definitely be added, but the hard part is getting this type information properly catalogued in swagger. From our experience with what-if, this is..hard. We would need a top-down initiative to drive this.

Will leave this open to collect reactions, comments, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request type system
Projects
None yet
Development

No branches or pull requests

3 participants