Repository navigation
[Version Pinning] Add linter rules for conflicting cross-file bicep.version constraints and incompatible entrypoint - #20337
Conversation
|
Test this change out locally with the following install scripts (Action run 37543655094) VSCode
Azure CLI
|
| yield break; | ||
| } | ||
|
|
||
| (VersionRange Range, string File)? combined = null; |
There was a problem hiding this comment.
Could we include the entrypoint's constraint in the running intersection? EnumerateAllLocalModuleModelsTransitively() only yields descendants, so starting combined at null excludes the entrypoint entirely.
For example, an entrypoint requiring >=0.20 with a single module requiring <0.17 produces no diagnostic from this rule.
There was a problem hiding this comment.
Fixed by starting the running intersection with the entrypoint's own constraint before looping over referenced modules.
|
|
||
| namespace Bicep.Core.Analyzers.Linter.Rules; | ||
|
|
||
| public sealed class NoLooserVersionConstraintInEntrypointRule : LinterRuleBase |
There was a problem hiding this comment.
I realize this is the wording I used in the spec, but could we reword it around entrypoint compatibility rather than “looser” constraints?
The goal is that a tool reading only the entrypoint's bicep.version constraint must not select a version that a referenced module rejects. That requires the entrypoint's allowed versions to be a subset of every referenced module's allowed versions—not merely checking whether a module has a strictly narrower range.
For example:
| Entrypoint constraint | Module constraint | Expected |
|---|---|---|
>=0.20.0 |
>=0.20.0 |
Pass: equal ranges. |
>=0.25.0 |
>=0.20.0 |
Pass: every version the entrypoint permits satisfies the module. |
>=0.20.0 |
>=0.25.0 |
Report: the tool could select 0.21.0, which the module rejects. |
>=0.20.0, <0.30.0 |
>=0.25.0, <0.40.0 |
Report: partial overlap still permits an incompatible selection such as 0.21.0. |
<0.20.0 |
>=0.25.0 |
Report: no entrypoint-permitted version satisfies the module; this also constitutes a conflict. |
| None specified | >=0.25.0 |
Report: the entrypoint does not exclude incompatible versions. |
The partial-overlap case is why “looser” is misleading: neither range contains the other, but selecting solely from the entrypoint's range is still unsafe.
Suggested description:
The entrypoint's 'bicep.version' constraint must only allow versions that satisfy every referenced Bicep file's constraint.
Suggested diagnostic:
The entrypoint's 'bicep.version' constraint ({0}) allows versions that do not satisfy the constraint '{1}' required by referenced file '{2}'. Tools that select a Bicep version using only the entrypoint's constraint may select an incompatible version.
Perhaps rename the rule to no-incompatible-entrypoint-version and update the corresponding schema/resource wording.
There was a problem hiding this comment.
Reworded as per your suggestion and renamed the rule NoLooserVersionConstraintInEntrypointRule to NoIncompatibleEntrypointVersionRule.
| namespace Bicep.Core.UnitTests.Diagnostics.LinterRuleTests; | ||
|
|
||
| [TestClass] | ||
| public class NoIncompatibleEntrypointVersionRuleTests |
There was a problem hiding this comment.
would be good to include a test for extends
| public NoIncompatibleEntrypointVersionRule() : base( | ||
| code: Code, | ||
| description: CoreResources.NoIncompatibleEntrypointVersionRule_Description, | ||
| LinterRuleCategory.DeploymentError) |
There was a problem hiding this comment.
Thinking through this more, we should default no-incompatible-entrypoint-version to Warning, not Error. The rule catches a risk in version selection by an external tool, rather than proving that the current compilation cannot work. In particular, it reports an error whenever the entrypoint has no constraint and a local module has one, even if the Bicep version actually in use satisfies that module. By contrast, no-conflicting-version-constraints identifies ranges with no common version, which Error makes more sense there because no version can satisfy any constraint.
There was a problem hiding this comment.
Yes, it Makes sense to default to warning. Should we introduce a new LinterRuleCategory with a Warning default or just use overrideCategoryDefaultDiagnosticLevel to set this rule's default to Warning while keeping it in DeploymentError ?
Additonally the second option would need a small change in a test ( RulesShouldNotSpecifyOverriddenDiagnosticLevel_UnlessDifferingFromCategoryDefault in LinterAnalyzerTests.cs ) that currently only allows this override to turn a rule Off and not to another level like Warning, Its failure message says there may be valid reasons to use a different level, so I think this case qualifies for the change. I just wanted to call it out since we'd be changing an existing guardrail.
There was a problem hiding this comment.
Honestly none of the existing linter rule categories quite fit these rules. I'd rather create a new category e.g. VersionConstraintCompatibility, default it to Warning and override it to error for the no-conflicting-version-constraints rule.
There was a problem hiding this comment.
It looks like the guardrail test still only allows it be overridden to turn a rule Off and not to another level.(e.g Error).
| BestPractice, | ||
| DeploymentError, | ||
|
|
||
| /// Informs the user that something may go wrong at deployment time due to an external tool's choices (e.g. version selection). |
There was a problem hiding this comment.
This comment is specific to one rule. Change to "Rules concerning compatibility of bicep version constraints across Bicep files."
| ruleBase.DefaultDiagnosticLevel.Should().BeOneOf(new[] { DiagnosticLevel.Off, DiagnosticLevel.Error }, | ||
| "I think the reason for overriding the default diagnostic level of a rule's category should only be to turn it to Off by default, or to Error for a rule such as " + | ||
| "no-conflicting-version-constraints that proves no version can satisfy the constraints (if there turn out to be other valid reasons, this test will need to be changed)"); |
There was a problem hiding this comment.
Can we just remove this assertion altogether? The first assertion already enforces what this test’s name promises: an override must differ from the category default. The second assertion encodes a broader policy about which levels may be used, and adding Error makes that policy less clear without making it specific to this rule.
|
Please update the PR description as well to account for the changes. |
Description
Summary
Adds two new linter rules catching cases where bicep.version constraints across an entrypoint and its local module references are inconsistent.
New rules
no-conflicting-version-constraintsFlags when the bicep.version constraints declared across the entrypoint and all Bicep files reachable from it (via local module references) cannot be satisfied by any single Bicep version i.e., their ranges don't overlap. Example: entrypoint requires >=0.20.0 while a referenced module requires <0.17.0 . Category: VersionConstraintCompatibility , overridden to Error by default, since an unsatisfiable constraint set means deployment will always fail regardless of which Bicep version is used.
no-incompatible-entrypoint-versionFlags when the entrypoint's bicep.version constraint allows a version that one of its referenced files' constraints would reject , including when the entrypoint declares no constraint at all (treated as the loosest possible constraint, since it permits every version). This matters because external tools that only inspect the entrypoint's config could install a Bicep version incompatible with a referenced file's stricter requirement. Category: VersionConstraintCompatibility , default severity Warning.
Changes
• NoConflictingVersionConstraintsRule.cs / NoIncompatibleEntrypointVersionRule.cs -- new rule implementations
• CoreResources.resx / CoreResources.Designer.cs -- new description/message resources
• bicepconfig.schema.json -- schema entries for both new rule codes
• Unit tests for both rules, covering conflicting ranges, entrypoint incompatible version constraints.
Checklist
Microsoft Reviewers: Open in CodeFlow