Resolve policy definition versions to the version the assignment actually runs

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()
 

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,28 +86,62 @@ func (c *VersionedPolicyCollection[T]) GetVersion(constraintStr *string) (T, err
 		return c.versionlessDefinition, nil
 	}
 
+	// 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
+		}
+
+		return nil, errors.Join(ErrNoVersionFound, fmt.Errorf(
+			"version %s",
+			*constraintStr,
+		))
+	}
+
 	constraint, err := policyVersionConstraintToSemVerConstraint(*constraintStr)
 	if err != nil {
 		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 {

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)
 	})
 }
@@ -236,6 +243,97 @@ 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",
+		},
+		{
+			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 {
+		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()

assets/policyAssignment.go

@@ -26,39 +26,93 @@ 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.
 // The caller is responsible for ensuring that the policy assignment is valid.
 // 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)
 	}
 
 	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
 }
 
 // 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)
 }
 
@@ -135,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")
 	}