feat(policy): add Azure Policy control plane (definitions, set definitions, assignments, exemptions) - #281
feat(policy): add Azure Policy control plane (definitions, set definitions, assignments, exemptions)#281Tal-E wants to merge 1 commit into
Conversation
…tions, assignments, exemptions)
|
🎉 Thanks for your first pull request to floci-az! Your CI checks need a maintainer to approve them before they run. That is GitHub's standard gate on first-time contributors, not a problem with your PR — so if the checks look like they are doing nothing, that is why. Once a maintainer approves, CI and the compatibility suite start automatically. Nothing is needed from you in the meantime. While you wait, a couple of things that make review faster:
Come join us in Slack — it is the fastest way to reach maintainers if you get stuck, or want feedback on an approach before investing more time in it. |
|
| Filename | Overview |
|---|---|
| src/main/java/io/floci/az/services/policy/PolicyHandler.java | Implements policy CRUD, filtering, identities, and integrity checks, but mishandles combined identities and accepts malformed unscoped references. |
| src/main/java/io/floci/az/services/policy/PolicyPath.java | Parses policy extension-resource paths but rejects valid subscription-level resource scopes. |
| src/main/java/io/floci/az/services/policy/PolicyStore.java | Stores policy resources in concurrent maps without atomic cross-resource integrity operations. |
| src/main/java/io/floci/az/core/AzureRoutingFilter.java | Extends provider routing to tenant-rooted paths and preserves guarded policy dispatch. |
| src/test/java/io/floci/az/services/policy/PolicyTest.java | Provides broad lifecycle and routing coverage but omits combined identities, subscription-level resource scopes, and concurrency races. |
| docs/services/policy.md | Documents the new control-plane surface and intentional deviations comprehensively. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Client[Azure CLI / SDK / Terraform] --> Router[AzureRoutingFilter]
Router -->|guarded Microsoft.Authorization policy path| Handler[PolicyHandler]
Router -->|other ARM path| Generic[Generic ARM handler]
Handler --> Path[PolicyPath scope and type parsing]
Handler --> Definitions[Definitions store]
Handler --> Sets[Set definitions store]
Handler --> Assignments[Assignments store]
Handler --> Exemptions[Exemptions store]
Assignments -->|definition reference| Definitions
Assignments -->|initiative reference| Sets
Exemptions -->|assignment reference| Assignments
Reviews (1): Last reviewed commit: "feat(policy): add Azure Policy control p..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if ("SystemAssigned".equalsIgnoreCase(type)) { | ||
| Map<?, ?> previous = existing != null && existing.get("identity") instanceof Map<?, ?> map ? map : Map.of(); | ||
| String principalId = "SystemAssigned".equalsIgnoreCase(String.valueOf(previous.get("type"))) | ||
| && previous.get("principalId") instanceof String kept | ||
| ? kept : UUID.randomUUID().toString(); | ||
| identity.put("principalId", principalId); | ||
| identity.put("tenantId", config.services().entra().defaultTenantId()); | ||
| identity.put("type", "SystemAssigned"); | ||
| return identity; | ||
| } | ||
| if ("UserAssigned".equalsIgnoreCase(type)) { | ||
| identity.put("type", "UserAssigned"); | ||
| Map<String, Object> userAssigned = new LinkedHashMap<>(); | ||
| if (requested.get("userAssignedIdentities") instanceof Map<?, ?> ids) { | ||
| for (Object key : ids.keySet()) { | ||
| String resourceId = String.valueOf(key); | ||
| userAssigned.put(resourceId, identities.findByResourceId(resourceId) | ||
| .map(PolicyHandler::identityIds) | ||
| .orElseGet(() -> Map.<String, Object>of( | ||
| "principalId", TokenIssuer.deterministicGuid("policy-uai-principal:" + resourceId.toLowerCase(Locale.ROOT)), | ||
| "clientId", TokenIssuer.deterministicGuid("policy-uai-client:" + resourceId.toLowerCase(Locale.ROOT))))); | ||
| } | ||
| } | ||
| identity.put("userAssignedIdentities", userAssigned); | ||
| return identity; | ||
| } | ||
| identity.put("type", "None"); |
There was a problem hiding this comment.
Policy assignments can use the combined SystemAssigned,UserAssigned identity type. This code only recognizes each type separately, so a combined payload falls through to None. The stored assignment then loses both identities, breaking clients that manage assignments with mixed identities.
Knowledge Base Used: Compatibility test suite
| private static final String MARKER = "/providers/microsoft.authorization/"; | ||
| private static final Pattern SUBSCRIPTION = Pattern.compile("subscriptions/[^/]+"); | ||
| private static final Pattern RESOURCE_GROUP = Pattern.compile("subscriptions/[^/]+/resourcegroups/[^/]+"); | ||
| private static final Pattern RESOURCE = Pattern.compile("subscriptions/[^/]+/resourcegroups/[^/]+/providers/.+"); |
There was a problem hiding this comment.
The RESOURCE pattern requires a resourceGroups segment. A policy assignment or exemption targeting a subscription-level resource such as /subscriptions/{sub}/providers/{namespace}/{type}/{name} is therefore classified as UNKNOWN and rejected with a 404, even though this feature promises support at arbitrary resource scope.
| private final Map<String, Map<String, Object>> definitions = new ConcurrentHashMap<>(); | ||
| private final Map<String, Map<String, Object>> setDefinitions = new ConcurrentHashMap<>(); | ||
| private final Map<String, Map<String, Object>> assignments = new ConcurrentHashMap<>(); | ||
| private final Map<String, Map<String, Object>> exemptions = new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
Referential-integrity checks and mutations span separate concurrent maps without one atomic operation. For example, exemption creation can confirm that an assignment exists, race with assignment deletion after exemption cleanup has run, and then insert an orphan exemption. An assignment can similarly validate a definition just before another request deletes it. Concurrent requests can therefore violate the promised reference and cascade guarantees.
Knowledge Base Used: Core routing and authentication
| private static boolean isScopedId(String id) { | ||
| String lower = id.toLowerCase(Locale.ROOT); | ||
| return lower.contains("/subscriptions/") || lower.contains("/managementgroups/"); | ||
| } |
There was a problem hiding this comment.
Every reference ID without /subscriptions/ or /managementGroups/ is treated as a built-in and bypasses lookup. Relative IDs such as policyDefinitions/missing and unrelated provider IDs such as /providers/Other.Provider/policyDefinitions/missing can therefore create permanently dangling assignments or initiatives. Restrict this bypass to canonical tenant-rooted Microsoft.Authorization IDs.
|
Thank you, this is a well-scoped control plane and the description is a model: CLI cassettes named per claim, unverified codes listed, deviations on the docs page. Handler-owned routing, (blocking) Your own On Greptile: the 2025-03-01 assignment identity enum is (follow-up, separate PR) Narrow the built-in bypass to That is the only blocker. |
|
Two things since my review. A rebase is now owed, and the cause is my merge batch: #268 landed and both branches add tests at the end of |
2 participants
Summary
Adds the Azure Policy control plane under
Microsoft.Authorization: policy definitions, policy set definitions (initiatives), policy assignments and policy exemptions, at every ARM scope the real service accepts (management group, subscription, resource group, resource; tenant-rooted built-in reads answer empty). This letsaz policy ..., theazurermpolicy resources and the policy SDKs (armpolicy,azure-mgmt-resource,azure-resourcemanager-resources) author and deploy policies against floci-az instead of a real subscription.Scope is deliberately the control plane only: rules are stored and validated structurally, never evaluated. Nothing is denied, audited or remediated, there is no compliance state (
Microsoft.PolicyInsightsis not implemented) and no built-in definitions are seeded. The docs page lists every deviation.What is in the change:
services/policy/:PolicyHandler(filter-laneAzureServiceHandler, guarded provider route onMicrosoft.Authorizationso role assignments, locks and deny assignments keep falling through to the generic ARM handler),PolicyStore(in-memory, keyed by lower-cased resource id like the other ARM control-plane stores) andPolicyPath(scope / type / name parsing for extension resources).$filter=policyType eq '...'. Server-sidepolicyType=Custom,modedefaultIndexed,versiondefault1.0.0,metadata.createdBy/createdOn/updatedBy/updatedOnstamps, generatedpolicyDefinitionReferenceIdanddefinitionVersion=1.*.*on references.atScope(),atExactScope(),atScopeAndBelow()andpolicyDefinitionId eq '...'.scope,enforcementMode,definitionVersion,instanceIdpopulated server-side;SystemAssignedidentities get a stable principal,UserAssignedidentities resolveprincipalId/clientIdfrom the Managed Identity store when the identity exists.atScope(),atExactScope(),excludeExpired(),policyAssignmentId eq '...'; deleted together with their assignment.PolicyDefinitionNotFound,PolicySetDefinitionNotFound), exemptions need an existing assignment (PolicyAssignmentNotFound), and referenced definitions / set definitions cannot be deleted (InvalidDeletePolicyDefinitionRequest,InvalidDeletePolicySetDefinitionRequest). Built-in ids (/providers/Microsoft.Authorization/policyDefinitions/{guid}) are accepted without lookup.routeArmProvidersnow also considers tenant-rootedproviders/Microsoft.X/...paths (matching the/providers/<ns>/marker against the slash-prefixed path), which is how built-in listings and management-group scopes arrive. Subscription-rooted behaviour is unchanged;RoutingTableAssemblyTestgains the new golden entry and the guarded-first invariant now covers both guarded routes.floci-az.services.policy.enabled(FLOCI_AZ_SERVICES_POLICY_ENABLED, defaulttrue), banner line,docs/services/policy.md, nav, service matrix and README rows.PolicyTest(definitions, set definitions, assignments, exemptions, scope filters, management-group and tenant scopes, referential integrity, routing boundaries),PolicyDisabledTest, two new cases inAzureRoutingFilterTest; a Java compat test (PolicyCompatibilityTest, raw REST like the Managed Identity one) and anaz policyBATS file for the az CLI suite.Out of scope, left for follow-ups: rule evaluation / deny effects, PolicyInsights compliance and remediation, built-in definition catalogue, the
versionssub-resource API, pagination of listings.Type of change
fix:)feat:)feat!:orfix!:)Azure Compatibility
azure-rest-api-specspolicystable/2025-03-01/openapi.jsonand exemptionspreview/2022-07-01-preview/policyExemptions.json; anyapi-versionis accepted.Azure/azure-cli(test_resource_policy_default.yaml,test_resource_policyset_default.yaml,test_resource_policyexemption_default.yaml, api-version 2025-11-01):201on definition and assignment PUT,200on set definition / exemption update,metadata.createdBy/createdOnstamps, generated numericpolicyDefinitionReferenceId,definitionVersion1.*.*, assignmentinstanceId, DELETE of assignments and exemptions returning the resource body, and the exactPolicyDefinitionNotFound/PolicySetDefinitionNotFoundmessages.InvalidDeletePolicyDefinitionRequest(definition referenced by a set definition) reproduces the live message reported in AzureRM custom policy deletion fails when included in policy set definition hashicorp/terraform-provider-azurerm#15615.InvalidCreatePolicyDefinitionRequest/InvalidCreatePolicyAssignmentRequest/InvalidCreatePolicyExemptionRequestcodes returned for missing required properties (modelled on the recordedInvalidCreatePolicySetDefinitionRequest), and the assignment-referenced variant ofInvalidDeletePolicyDefinitionRequest. Happy to adjust if a maintainer has live samples.ManagedIdentityCompatibilityTestdoes); the BATS file usesazfrommcr.microsoft.com/azure-cli:latest.Checklist
./mvnw testpasses locally