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

Consume the pattern property in swagger specs during Bicep type generation to enable better regex validation #15325

Open
asilverman opened this issue Oct 14, 2024 · 6 comments

Comments

@asilverman
Copy link
Contributor

Description

As a user of Bicep, I would like to have more informative static analysis for the policyAssignmentName property when defining a new resource of this type. Currently, the only static analysis feedback I receive is that this property is of type string, which lacks the detailed validation I expect based on the Azure REST API specs.

Image

Issue:

When instantiating a policy assignment resource in Bicep, the static analysis for the name property only checks that it’s a string. However, according to the Azure REST API specifications, the policyAssignmentName property must adhere to a specific pattern:

{
  "name": "policyAssignmentName",
  "in": "path",
  "required": true,
  "type": "string",
  "pattern": "^[^<>*%&:\\?.+/]*[^<>*%&:\\?.+/ ]+$",
  "description": "The name of the policy assignment."
}

Source: Azure REST API Specs

Additionally, the official Azure documentation specifies the following restrictions:

- Character limit: 
  - 1-128 for display name
  - 1-64 for resource name
  - 1-24 for resource name at management group scope

- Valid characters for resource name:
  - Can't use: <>*%&:\?.+/ or control characters.
  - Can't end with a period or space.

Source: Policy Assignments Documentation

Expected Behavior:

I would like Bicep’s static analysis to validate the policyAssignmentName property against the pattern specified in the API specs and the character limits/restrictions outlined in the official documentation. Ideally, the IDE would surface an error or warning if invalid characters are used or if the length exceeds the allowed range.

Current Behavior:

  • The current static analysis only shows that the name property is of type string.
  • No validation is performed regarding character limits or invalid characters until runtime, which results in deployment failures.

Request:

Please enhance the static analysis in Bicep to:

  1. Validate the policyAssignmentName against the pattern defined in the REST API specs.
  2. Enforce character length limits based on the scope (e.g., resource name at management group scope).
  3. Provide real-time feedback in IDEs, helping developers avoid deployment failures by catching these errors earlier in the development process.

This enhancement would improve the developer experience by reducing the number of invalid deployments caused by improperly formatted policy assignment names.

@alex-frankel
Copy link
Collaborator

@jeskew / @stephaniezyen -- I believe we are now capable of consuming more naming restrictions like this as part of closing #1679. Is the current issue that we cannot deal with Regular Expressions? If so, we should rename the issue to better capture that.

@asilverman
Copy link
Contributor Author

asilverman commented Oct 15, 2024

@alex-frankel FWIW while the azure-rest-api-specs documentation allows you to express a size constraint on your resource model, it's unclear how to specify a conditional size constraint that is dependent on the value of another parameter (in this case the target scope).

The documentation for Microsoft.Authorization/policyAssignments seems to fall on this special case:

The image below shows the description for the name property. Notice below how it says the size restriction is '1-24' characters at management group scope, but for any other scope the size restriction is '1-64'

  Image

So, I think an interesting first question is if the API Governance team has a solution for this type of conditional sizing restrictions.

The other question is related to the Bicep resource types generation, in this case the swagger does define the pattern constraint on the model but it's not being consumed or used by the Bicep type system to prevent the author from specifying a value that will result in a deployment failure.

I think both questions are important, and that the latter question is the more actionable from the perspective of the Bicep language ATM

@alex-frankel alex-frankel changed the title Enhance Static Analysis for policyAssignmentName Property of Microsoft.Authorization policyAssignments resource in Bicep Consume the pattern property in swagger specs during Bicep type generation to enable better regex validation Oct 24, 2024
@alex-frankel alex-frankel removed their assignment Oct 24, 2024
@alex-frankel
Copy link
Collaborator

Renamed this issue to better track the ask.

For the conditional naming rules, that will be an ask to the swagger team / API review board, so I'd suggest reaching out to them. It may actually be best to open it in the TypeSpec repo. TypeSpec is what all new specs are written in: https://github.com/microsoft/typespec

@jeskew
Copy link
Contributor

jeskew commented Oct 24, 2024

The pattern property is picked up from Swagger models and added to the serialized representation of string types, so we wouldn't need to make any changes to type generation to get this working.

One thing to consider here is that regexes in Swagger use a syntax that is not identical to what .NET's regex engine supports. There is a compatibility mode that can be enabled, but it is not compatible with .NET's non-backtracking engine.

@alex-frankel
Copy link
Collaborator

One thing to consider here is that regexes in Swagger use a syntax that is not identical to what .NET's regex engine supports. There is a compatibility mode that can be enabled, but it is not compatible with .NET's non-backtracking engine.

Is this a blocking issue then?

@jeskew
Copy link
Contributor

jeskew commented Oct 24, 2024

One thing to consider here is that regexes in Swagger use a syntax that is not identical to what .NET's regex engine supports. There is a compatibility mode that can be enabled, but it is not compatible with .NET's non-backtracking engine.

Is this a blocking issue then?

I don't think so; this was more of a note for whoever implements this. We can still use timeouts with the alternative syntax compatibility mode, but string types that have a pattern constraint will need to know if it came from Swagger (in which case it needs to run in compatibility mode with a timeout) or from a user-defined type (in which case it needs to run in the non-backtracking engine to ensure compatibility with ARM).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

No branches or pull requests

3 participants