From 9355275036bb6d28e358dac7ca9b037172e7acbc Mon Sep 17 00:00:00 2001 From: Yingjie Date: Wed, 12 Aug 2026 14:09:19 +0800 Subject: [PATCH 1/3] fix(assets): respect prerelease suffix when resolving policy definition versions Azure Policy encodes preview and deprecated state as a semver prerelease suffix ("-preview" / "-deprecated"). Masterminds semver expands a wildcard constraint such as "1.*.*-preview" to a range whose upper bound is a clean version, so the suffix leaks and the range also matches non-preview releases. GetVersion then selected the highest match, resolving "1.*.*-preview" to 1.4.0 instead of the 1.3.0-preview the assignment is actually pinned to. Role assignments were therefore computed against a different version than the one the Policy RP enforces, producing spurious or missing role assignments on upstream version bumps. Replace the highest-match selection in GetVersion with an identifier-aware three-tier resolution: 1. highest match carrying the SAME prerelease suffix 2. highest stable release, so a definition that graduated out of preview still resolves instead of failing with "no version found" 3. highest remaining match, so a set containing only a different suffix (e.g. deprecated-only) still resolves Matching by identifier rather than by "is a prerelease" is required because semver treats "-preview" and "-deprecated" alike, so a "-preview" constraint would otherwise select a higher "-deprecated" version. Rename policyVersionConstraintHasPrerelease to policyVersionConstraintPrerelease and return the identifier instead of a bool. Verified: "1.*.*-preview" over {1.2.0-preview, 1.3.0-preview, 1.4.0-deprecated, 1.5.0} now resolves to 1.3.0-preview. Refs Azure/Azure-Landing-Zones#4190 --- assets/genericVersionCollection.go | 35 ++++++++-- assets/genericVersionCollection_test.go | 85 +++++++++++++++++++++++++ assets/semver.go | 14 ++++ assets/semver_test.go | 22 +++++++ 4 files changed, 149 insertions(+), 7 deletions(-) diff --git a/assets/genericVersionCollection.go b/assets/genericVersionCollection.go index b150794..2cf56d4 100644 --- a/assets/genericVersionCollection.go +++ b/assets/genericVersionCollection.go @@ -91,23 +91,44 @@ func (c *VersionedPolicyCollection[T]) GetVersion(constraintStr *string) (T, err return nil, err } - var resKey *semver.Version + // Azure Policy encodes state as a semver prerelease suffix: "-preview" or "-deprecated". + // A constraint carrying such a suffix must resolve to a version with the same suffix, not to a + // higher stable release or a different suffix. It falls back to the highest stable release when + // no same-suffix version matches (graduated out of preview), then to any other matching version + // as a last resort. See Azure/Azure-Landing-Zones#4190. + wantPrerelease := policyVersionConstraintPrerelease(*constraintStr) + + var bestMatch, bestStable, bestAny *semver.Version for v := range c.versions { if !constraint.Check(&v) { continue } - if resKey == nil { - resKey = &v - continue + if bestAny == nil || v.GreaterThan(bestAny) { + bestAny = &v } - if v.LessThan(resKey) { - continue + // When the constraint has no suffix, wantPrerelease is "" and the first case wins. + switch v.Prerelease() { + case wantPrerelease: + if bestMatch == nil || v.GreaterThan(bestMatch) { + bestMatch = &v + } + case "": + if bestStable == nil || v.GreaterThan(bestStable) { + bestStable = &v + } } + } - resKey = &v + resKey := bestMatch + if resKey == nil { + resKey = bestStable + } + + if resKey == nil { + resKey = bestAny } if resKey == nil { diff --git a/assets/genericVersionCollection_test.go b/assets/genericVersionCollection_test.go index 669b453..60bbe8c 100644 --- a/assets/genericVersionCollection_test.go +++ b/assets/genericVersionCollection_test.go @@ -236,6 +236,91 @@ func TestVersionedPolicyCollection_GetVersion_PrereleaseVersionMatchOnNilVersion assert.NotNil(t, got) } +func TestVersionedPolicyCollection_GetVersion_PrereleaseAwareResolution(t *testing.T) { + newColl := func(versions ...string) *PolicyDefinitionVersions { + pdvs := NewPolicyDefinitionVersions() + for _, v := range versions { + require.NoError(t, pdvs.Add(fakePolicyDefinitionVersioned("MCSB2", v), false)) + } + + return pdvs + } + + fullSet := []string{"1.1.0-preview", "1.2.0-preview", "1.3.0-preview", "1.4.0"} + + tests := []struct { + name string + versions []string + constraint string + want string + }{ + { + name: "preview wildcard prefers highest preview not stable", + versions: fullSet, + constraint: "1.*.*-preview", + want: "1.3.0-preview", + }, + { + name: "preview pinned minor", + versions: fullSet, + constraint: "1.3.*-preview", + want: "1.3.0-preview", + }, + { + name: "clean wildcard resolves stable", + versions: fullSet, + constraint: "1.*.*", + want: "1.4.0", + }, + { + name: "graduated out of preview falls back to stable", + versions: []string{"1.4.0"}, + constraint: "1.*.*-preview", + want: "1.4.0", + }, + { + name: "preview only resolves highest preview", + versions: []string{"1.1.0-preview", "1.2.0-preview"}, + constraint: "1.*.*-preview", + want: "1.2.0-preview", + }, + { + name: "preview wildcard ignores deprecated and higher stable", + versions: []string{"1.2.0-preview", "1.3.0-preview", "1.4.0-deprecated", "1.5.0"}, + constraint: "1.*.*-preview", + want: "1.3.0-preview", + }, + { + name: "deprecated constraint resolves matching deprecated", + versions: []string{"1.3.0-preview", "1.4.0-deprecated", "1.5.0"}, + constraint: "1.*.*-deprecated", + want: "1.4.0-deprecated", + }, + { + name: "clean wildcard ignores deprecated", + versions: []string{"1.4.0-deprecated", "1.5.0"}, + constraint: "1.*.*", + want: "1.5.0", + }, + { + name: "only different suffix falls back to any match", + versions: []string{"1.4.0-deprecated"}, + constraint: "1.*.*-preview", + want: "1.4.0-deprecated", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pdvs := newColl(tt.versions...) + got, err := pdvs.GetVersion(to.Ptr(tt.constraint)) + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, tt.want, *got.GetVersion()) + }) + } +} + func TestVersionedPolicyCollection_Exists(t *testing.T) { t.Run("returns true when versionless definition present", func(t *testing.T) { pdvs := NewPolicyDefinitionVersions() diff --git a/assets/semver.go b/assets/semver.go index 0cdea9b..58f0ffa 100644 --- a/assets/semver.go +++ b/assets/semver.go @@ -54,3 +54,17 @@ func policyVersionConstraintToSemVerConstraint(constraint string) (*semver.Const return sv, nil } + +// policyVersionConstraintPrerelease returns the prerelease identifier carried by a policy version constraint string, +// e.g. "1.*.*-preview" -> "preview", "1.*.*-deprecated" -> "deprecated", "1.*.*" -> "". +// Azure Policy encodes preview/deprecated state as this suffix. +func policyVersionConstraintPrerelease(constraint string) string { + parts := strings.Split(constraint, ".") + if len(parts) != ExpectedVersionComponents { + return "" + } + + _, pre, _ := strings.Cut(parts[len(parts)-1], "-") + + return pre +} diff --git a/assets/semver_test.go b/assets/semver_test.go index 086a2e6..99cd759 100644 --- a/assets/semver_test.go +++ b/assets/semver_test.go @@ -120,3 +120,25 @@ func TestPolicyVersionConstraintToSemVerConstraint(t *testing.T) { }) } } + +func TestPolicyVersionConstraintPrerelease(t *testing.T) { + tests := []struct { + constraint string + want string + }{ + {"1.*.*-preview", "preview"}, + {"1.3.*-preview", "preview"}, + {"1.*.*-deprecated", "deprecated"}, + {">= 0.0.*-preview", "preview"}, + {"1.*.*", ""}, + {"1.2.*", ""}, + {">= 0.0.*", ""}, + {"1.2", ""}, + } + + for _, tt := range tests { + t.Run(tt.constraint, func(t *testing.T) { + assert.Equal(t, tt.want, policyVersionConstraintPrerelease(tt.constraint)) + }) + } +} From 270e641e19c96450ffc59c98216ec8ad653a85fd Mon Sep 17 00:00:00 2001 From: Yingjie Date: Fri, 14 Aug 2026 13:38:56 +0800 Subject: [PATCH 2/3] resolve policy versions against the deployed effective version Option A made a "-preview" constraint resolve to a matching preview rather than to a newer stable release, but it still re-resolves the constraint to the highest match. When a tenant runs an older preview than the highest available one, the client and the Policy RP still disagree, and role assignments are computed against the wrong member set. Azure reports the version an assignment actually runs as the read-only effectiveDefinitionVersion property. Carry it on PolicyAssignment and prefer it over the definitionVersion constraint when resolving the referenced definition, so role assignments follow what the Policy RP enforces instead of what the constraint happens to match today. The value is held in an unexported field rather than in the embedded SDK type. The SDK marshaller emits effectiveDefinitionVersion and latestDefinitionVersion, and assignments are written out as ARM JSON, where Azure rejects these read-only properties. Keeping the value off the embedded type makes leaking it structurally impossible, and a deep copy preserves it, so it survives the copies made when an assignment is stored in and retrieved from AlzLib. Both read-only fields are cleared when an assignment is constructed or unmarshalled. GetVersion now resolves an exact version by direct lookup before treating the string as a constraint. This is required because an effective version such as 1.2.0-preview carries no wildcard and the constraint validator rejects it. It also makes pinning a concrete definitionVersion work, which previously failed validation despite being valid for Azure Policy. Resolution falls back to the constraint when no effective version is known, so behaviour is unchanged for callers that do not supply one, including greenfield deployments where the assignment does not exist yet. Note that populating the effective version requires reading deployed assignments from Azure, which alzlib does not do; consumers such as terraform-provider-alz supply it via SetEffectiveDefinitionVersion. Refs Azure/Azure-Landing-Zones#4190 --- alzlib_test.go | 37 ++++++++++++ assets/genericVersionCollection.go | 13 +++++ assets/genericVersionCollection_test.go | 17 +++++- assets/policyAssignment.go | 60 +++++++++++++++++++- assets/policyAssignment_test.go | 75 +++++++++++++++++++++++++ 5 files changed, 198 insertions(+), 4 deletions(-) diff --git a/alzlib_test.go b/alzlib_test.go index ce52028..c21ded9 100644 --- a/alzlib_test.go +++ b/alzlib_test.go @@ -981,6 +981,43 @@ func TestAssignmentReferencedDefinitionHasParameter(t *testing.T) { assert.True(t, az.AssignmentReferencedDefinitionHasParameter(resID2, nil, "anyParam")) } +func TestPolicyAssignmentEffectiveDefinitionVersionRoundTrip(t *testing.T) { + az := NewAlzLib(&Options{ + AllowOverwrite: true, + Parallelism: defaultParallelism, + UniqueRoleDefinitions: defaultUniqueRoleDefinitions, + }) + + original := assets.NewPolicyAssignment(armpolicy.Assignment{ + Name: to.Ptr("Deploy-MCSB2-Monitoring"), + Properties: &armpolicy.AssignmentProperties{ + DisplayName: to.Ptr("Microsoft Cloud Security Benchmark v2"), + Description: to.Ptr("Microsoft Cloud Security Benchmark v2 policy initiative."), + PolicyDefinitionID: to.Ptr("/providers/Microsoft.Authorization/policySetDefinitions/e3ec7e09"), + DefinitionVersion: to.Ptr("1.*.*-preview"), + }, + }) + require.NoError(t, az.AddPolicyAssignments(original)) + + // Callers get a deep copy, so the effective version must survive being written back. + fetched := az.PolicyAssignment("Deploy-MCSB2-Monitoring") + require.NotNil(t, fetched) + require.Nil(t, fetched.EffectiveDefinitionVersion()) + + fetched.SetEffectiveDefinitionVersion(to.Ptr("1.2.0-preview")) + require.NoError(t, az.AddPolicyAssignments(fetched)) + + stored := az.PolicyAssignment("Deploy-MCSB2-Monitoring") + require.NotNil(t, stored) + require.NotNil(t, stored.EffectiveDefinitionVersion()) + assert.Equal(t, "1.2.0-preview", *stored.EffectiveDefinitionVersion()) + + _, ver, err := stored.ReferencedPolicyDefinitionResourceIDAndVersion() + require.NoError(t, err) + require.NotNil(t, ver) + assert.Equal(t, "1.2.0-preview", *ver) +} + func TestAddPolicyDefinitionsMultipleVersions(t *testing.T) { t.Parallel() diff --git a/assets/genericVersionCollection.go b/assets/genericVersionCollection.go index 2cf56d4..c7f89dc 100644 --- a/assets/genericVersionCollection.go +++ b/assets/genericVersionCollection.go @@ -86,6 +86,19 @@ func (c *VersionedPolicyCollection[T]) GetVersion(constraintStr *string) (T, err return c.versionlessDefinition, nil } + // An exact version resolves directly. This is how an assignment's effective definition version, + // as reported by Azure, and concrete version pins are looked up. + if sv, err := semver.StrictNewVersion(*constraintStr); err == nil { + if pol, ok := c.versions[*sv]; ok { + return pol, nil + } + + return nil, errors.Join(ErrNoVersionFound, fmt.Errorf( + "version %s", + *constraintStr, + )) + } + constraint, err := policyVersionConstraintToSemVerConstraint(*constraintStr) if err != nil { return nil, err diff --git a/assets/genericVersionCollection_test.go b/assets/genericVersionCollection_test.go index 60bbe8c..98aca7e 100644 --- a/assets/genericVersionCollection_test.go +++ b/assets/genericVersionCollection_test.go @@ -190,10 +190,17 @@ func TestVersionedPolicyCollection_GetVersion_Versioned(t *testing.T) { assert.Nil(t, got) }) - t.Run("invalid no wildcard patch", func(t *testing.T) { + t.Run("exact version resolves", func(t *testing.T) { constr := testVersion100 got, err := pdvs.GetVersion(&constr) - require.ErrorContains(t, err, "version constraint should have wildcard in patch version") + require.NoError(t, err) + assert.Equal(t, policy1, got) + }) + + t.Run("exact version not in collection", func(t *testing.T) { + constr := "3.0.0" + got, err := pdvs.GetVersion(&constr) + require.ErrorIs(t, err, ErrNoVersionFound) assert.Nil(t, got) }) } @@ -308,6 +315,12 @@ func TestVersionedPolicyCollection_GetVersion_PrereleaseAwareResolution(t *testi constraint: "1.*.*-preview", want: "1.4.0-deprecated", }, + { + name: "exact preview version pins below the highest preview", + versions: fullSet, + constraint: "1.2.0-preview", + want: "1.2.0-preview", + }, } for _, tt := range tests { diff --git a/assets/policyAssignment.go b/assets/policyAssignment.go index bdabe16..fbe415f 100644 --- a/assets/policyAssignment.go +++ b/assets/policyAssignment.go @@ -26,6 +26,10 @@ const ( // working with policy assignments. type PolicyAssignment struct { armpolicy.Assignment + + // Held outside the embedded SDK type so it is never serialized: Azure rejects this read-only + // property when an assignment is deployed. + effectiveDefinitionVersion *string } // NewPolicyAssignment creates a new PolicyAssignment instance from an armpolicy.Assignment. @@ -33,12 +37,15 @@ type PolicyAssignment struct { // Use either the UnmarshalJSON method, or the ValidatePolicyAssignment function to validate the // assignment. func NewPolicyAssignment(pa armpolicy.Assignment) *PolicyAssignment { - return &PolicyAssignment{pa} + paObj := &PolicyAssignment{Assignment: pa} + paObj.hoistReadOnlyDefinitionVersions() + + return paObj } // NewPolicyAssignmentValidate creates a new PolicyAssignment instance and validates it. func NewPolicyAssignmentValidate(pa armpolicy.Assignment) (*PolicyAssignment, error) { - paObj := &PolicyAssignment{pa} + paObj := NewPolicyAssignment(pa) if err := ValidatePolicyAssignment(paObj); err != nil { return nil, fmt.Errorf("NewPolicyAssignmentValidate: %w", err) } @@ -46,6 +53,46 @@ func NewPolicyAssignmentValidate(pa armpolicy.Assignment) (*PolicyAssignment, er return paObj, nil } +// hoistReadOnlyDefinitionVersions moves the effective version reported by Azure out of the embedded +// SDK type, so it can inform version resolution without being written to deployment artifacts, and +// drops the other read-only version field for the same reason. +func (pa *PolicyAssignment) hoistReadOnlyDefinitionVersions() { + if pa.Properties == nil { + return + } + + if v := pa.Properties.EffectiveDefinitionVersion; v != nil && *v != "" { + pa.effectiveDefinitionVersion = to.Ptr(*v) + } + + pa.Properties.EffectiveDefinitionVersion = nil + pa.Properties.LatestDefinitionVersion = nil +} + +// EffectiveDefinitionVersion returns the exact policy definition version that Azure reports as being +// in effect for this assignment, or nil when it is unknown. +func (pa *PolicyAssignment) EffectiveDefinitionVersion() *string { + if pa.effectiveDefinitionVersion == nil { + return nil + } + + return to.Ptr(*pa.effectiveDefinitionVersion) +} + +// SetEffectiveDefinitionVersion records the exact policy definition version that Azure reports as +// being in effect, so that role assignments are computed against the version the Policy RP actually +// enforces instead of re-resolving the definitionVersion constraint. Passing nil or an empty string +// clears it. +func (pa *PolicyAssignment) SetEffectiveDefinitionVersion(version *string) { + if version == nil || *version == "" { + pa.effectiveDefinitionVersion = nil + + return + } + + pa.effectiveDefinitionVersion = to.Ptr(*version) +} + // IdentityType returns the identity type of the policy assignment. func (pa *PolicyAssignment) IdentityType() armpolicy.ResourceIdentityType { return *pa.Identity.Type @@ -53,12 +100,19 @@ func (pa *PolicyAssignment) IdentityType() armpolicy.ResourceIdentityType { // ReferencedPolicyDefinitionResourceIDAndVersion returns the resource ID and version of the // policy definition referenced by the policy assignment. +// The effective version reported by Azure takes precedence over the definitionVersion constraint, +// so callers resolve the version the assignment actually runs rather than re-resolving the +// constraint to a newer one. See Azure/Azure-Landing-Zones#4190. func (pa *PolicyAssignment) ReferencedPolicyDefinitionResourceIDAndVersion() (*arm.ResourceID, *string, error) { id, err := arm.ParseResourceID(*pa.Properties.PolicyDefinitionID) if err != nil { return nil, nil, fmt.Errorf("PolicyAssignment.ReferencedPolicyDefinitionResourceID: %w", err) } + if pa.effectiveDefinitionVersion != nil { + return id, to.Ptr(*pa.effectiveDefinitionVersion), nil + } + return id, pa.Properties.DefinitionVersion, nil } @@ -105,6 +159,8 @@ func (pa *PolicyAssignment) UnmarshalJSON(data []byte) error { return fmt.Errorf("PolicyAssignment.UnmarshalJSON: %w", err) } + pa.hoistReadOnlyDefinitionVersions() + return ValidatePolicyAssignment(pa) } diff --git a/assets/policyAssignment_test.go b/assets/policyAssignment_test.go index a9d89f2..df2c185 100644 --- a/assets/policyAssignment_test.go +++ b/assets/policyAssignment_test.go @@ -4,6 +4,7 @@ package assets import ( + "encoding/json" "fmt" "reflect" "testing" @@ -11,6 +12,7 @@ import ( "github.com/Azure/alzlib/to" "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armpolicy" + "github.com/brunoga/deep" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -30,6 +32,79 @@ func TestIdentityType(t *testing.T) { } } +func TestPolicyAssignmentEffectiveDefinitionVersion(t *testing.T) { + newAssignment := func() armpolicy.Assignment { + return armpolicy.Assignment{ + Name: to.Ptr("Deploy-MCSB2-Monitoring"), + Properties: &armpolicy.AssignmentProperties{ + PolicyDefinitionID: to.Ptr( + "/providers/Microsoft.Authorization/policySetDefinitions/e3ec7e09-768c-4b64-882c-fcada3772047", + ), + DefinitionVersion: to.Ptr("1.*.*-preview"), + }, + } + } + + t.Run("constraint is used when no effective version is known", func(t *testing.T) { + pa := NewPolicyAssignment(newAssignment()) + + _, ver, err := pa.ReferencedPolicyDefinitionResourceIDAndVersion() + require.NoError(t, err) + require.NotNil(t, ver) + assert.Equal(t, "1.*.*-preview", *ver) + assert.Nil(t, pa.EffectiveDefinitionVersion()) + }) + + t.Run("effective version takes precedence over the constraint", func(t *testing.T) { + pa := NewPolicyAssignment(newAssignment()) + pa.SetEffectiveDefinitionVersion(to.Ptr("1.2.0-preview")) + + _, ver, err := pa.ReferencedPolicyDefinitionResourceIDAndVersion() + require.NoError(t, err) + require.NotNil(t, ver) + assert.Equal(t, "1.2.0-preview", *ver) + }) + + t.Run("empty effective version clears it", func(t *testing.T) { + pa := NewPolicyAssignment(newAssignment()) + pa.SetEffectiveDefinitionVersion(to.Ptr("1.2.0-preview")) + pa.SetEffectiveDefinitionVersion(to.Ptr("")) + + _, ver, err := pa.ReferencedPolicyDefinitionResourceIDAndVersion() + require.NoError(t, err) + require.NotNil(t, ver) + assert.Equal(t, "1.*.*-preview", *ver) + }) + + t.Run("read-only versions are hoisted off the serialized type", func(t *testing.T) { + assignment := newAssignment() + assignment.Properties.EffectiveDefinitionVersion = to.Ptr("1.3.0-preview") + assignment.Properties.LatestDefinitionVersion = to.Ptr("1.4.0") + + pa := NewPolicyAssignment(assignment) + + require.NotNil(t, pa.EffectiveDefinitionVersion()) + assert.Equal(t, "1.3.0-preview", *pa.EffectiveDefinitionVersion()) + assert.Nil(t, pa.Properties.EffectiveDefinitionVersion) + assert.Nil(t, pa.Properties.LatestDefinitionVersion) + + b, err := json.Marshal(pa) + require.NoError(t, err) + assert.NotContains(t, string(b), "effectiveDefinitionVersion") + assert.NotContains(t, string(b), "latestDefinitionVersion") + }) + + t.Run("effective version survives a deep copy", func(t *testing.T) { + pa := NewPolicyAssignment(newAssignment()) + pa.SetEffectiveDefinitionVersion(to.Ptr("1.2.0-preview")) + + cp := deep.MustCopy(pa) + + require.NotNil(t, cp.EffectiveDefinitionVersion()) + assert.Equal(t, "1.2.0-preview", *cp.EffectiveDefinitionVersion()) + }) +} + func TestReferencedPolicyDefinitionResourceId(t *testing.T) { pa := NewPolicyAssignment(armpolicy.Assignment{ Properties: &armpolicy.AssignmentProperties{ From 7c1ff3e7cfae2ab73a863bbd71e002bb998a9e87 Mon Sep 17 00:00:00 2001 From: Yingjie Date: Mon, 17 Aug 2026 20:07:24 +0800 Subject: [PATCH 3/3] validate writable policy assignment version constraints Azure Policy does not permit assignments to pin a definition to a specific patch version. Writable definitionVersion values must keep the patch component as a wildcard so patch updates are always ingested. Distinguish the writable definitionVersion constraint from Azure's read-only effectiveDefinitionVersion. Validate assignment definitionVersion values with the existing policy constraint validator, while retaining exact-version lookup for effective versions reported by Azure. Accept supported wildcard forms such as 1.*.*-preview and 1.3.*-preview, and reject exact patch values such as 1.3.0 and 1.3.0-preview. Clarify the GetVersion documentation and add regression coverage for the supported and unsupported assignment version formats. Refs Azure/Azure-Landing-Zones#4190 --- assets/genericVersionCollection.go | 6 +-- assets/policyAssignment.go | 10 +++++ assets/policyAssignment_test.go | 60 ++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 3 deletions(-) diff --git a/assets/genericVersionCollection.go b/assets/genericVersionCollection.go index c7f89dc..617885e 100644 --- a/assets/genericVersionCollection.go +++ b/assets/genericVersionCollection.go @@ -54,7 +54,7 @@ func (c *VersionedPolicyCollection[T]) Versions() []semver.Version { return vers } -// GetVersion returns a policy version based on the provided constraint string. +// GetVersion returns a policy version based on an exact version or policy version constraint. // If the constraint string is nil, it returns the versionless definition if it exists. // If the constraint string is nil and no versionless definition exists, it returns the latest // version. @@ -86,8 +86,8 @@ func (c *VersionedPolicyCollection[T]) GetVersion(constraintStr *string) (T, err return c.versionlessDefinition, nil } - // An exact version resolves directly. This is how an assignment's effective definition version, - // as reported by Azure, and concrete version pins are looked up. + // Exact lookup supports Azure's read-only effective definition version. Writable assignment + // constraints must continue to use a wildcard patch version. if sv, err := semver.StrictNewVersion(*constraintStr); err == nil { if pol, ok := c.versions[*sv]; ok { return pol, nil diff --git a/assets/policyAssignment.go b/assets/policyAssignment.go index fbe415f..fa18ce7 100644 --- a/assets/policyAssignment.go +++ b/assets/policyAssignment.go @@ -191,6 +191,16 @@ func ValidatePolicyAssignment(pa *PolicyAssignment) error { return NewErrPropertyMustNotBeNil("properties.policyDefinitionID") } + if pa.Properties.DefinitionVersion != nil { + if _, err := policyVersionConstraintToSemVerConstraint(*pa.Properties.DefinitionVersion); err != nil { + return fmt.Errorf( + "ValidatePolicyAssignment: invalid properties.definitionVersion `%s`: %w", + *pa.Properties.DefinitionVersion, + err, + ) + } + } + if pa.Properties.DisplayName == nil { return NewErrPropertyMustNotBeNil("properties.displayName") } diff --git a/assets/policyAssignment_test.go b/assets/policyAssignment_test.go index df2c185..b5fd455 100644 --- a/assets/policyAssignment_test.go +++ b/assets/policyAssignment_test.go @@ -194,6 +194,66 @@ func TestValidatePolicyAssignment(t *testing.T) { }, expectedErr: "", }, + { + name: "Valid definition version autoingests minor and patch updates", + assignment: armpolicy.Assignment{ + Name: to.Ptr("validName"), + Properties: &armpolicy.AssignmentProperties{ + PolicyDefinitionID: to.Ptr( + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Authorization/policyDefinitions/pd1", + ), + DefinitionVersion: to.Ptr("1.*.*-preview"), + DisplayName: to.Ptr("Valid Display Name"), + Description: to.Ptr("Valid Description"), + }, + }, + expectedErr: "", + }, + { + name: "Valid definition version pins a minor path", + assignment: armpolicy.Assignment{ + Name: to.Ptr("validName"), + Properties: &armpolicy.AssignmentProperties{ + PolicyDefinitionID: to.Ptr( + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Authorization/policyDefinitions/pd1", + ), + DefinitionVersion: to.Ptr("1.3.*-preview"), + DisplayName: to.Ptr("Valid Display Name"), + Description: to.Ptr("Valid Description"), + }, + }, + expectedErr: "", + }, + { + name: "Invalid definition version pins a stable patch", + assignment: armpolicy.Assignment{ + Name: to.Ptr("validName"), + Properties: &armpolicy.AssignmentProperties{ + PolicyDefinitionID: to.Ptr( + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Authorization/policyDefinitions/pd1", + ), + DefinitionVersion: to.Ptr("1.3.0"), + DisplayName: to.Ptr("Valid Display Name"), + Description: to.Ptr("Valid Description"), + }, + }, + expectedErr: "version constraint should have wildcard in patch version", + }, + { + name: "Invalid definition version pins a preview patch", + assignment: armpolicy.Assignment{ + Name: to.Ptr("validName"), + Properties: &armpolicy.AssignmentProperties{ + PolicyDefinitionID: to.Ptr( + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Authorization/policyDefinitions/pd1", + ), + DefinitionVersion: to.Ptr("1.3.0-preview"), + DisplayName: to.Ptr("Valid Display Name"), + Description: to.Ptr("Valid Description"), + }, + }, + expectedErr: "version constraint should have wildcard in patch version", + }, { name: "Nil Name", assignment: armpolicy.Assignment{