Express and read the sites identity block - #9
Merged
Conversation
Consuming apps had to hand-roll their own ARM `sites` GET/PUT requests
because neither `WebSitePayload` nor `WebSiteData` could touch the
top-level `identity` block: there was no way to ask ARM for a
system-assigned identity on a Function App, and no way to read back the
`identity.principalId` that a subsequent role assignment needs to grant
that identity storage/Key Vault RBAC. That single missing field was
enough to make the whole typed surface unusable for provisioning, so the
app bypassed it entirely and grew a bespoke request pair alongside it.
- `WebSitePayload` takes a trailing optional `identityType` and emits
`identity: {type: ...}`, matching the existing precedent in
`CognitiveServicesAccountPayload` and `FoundryProjectPayload`.
- `WebSiteData` exposes `identityType`, `identityPrincipalId` and
`identityTenantId`, read via the usual `Field::arrNullableString`
dotted-path helpers.
- `FunctionAppResource::createOrUpdate()` threads `identityType` through.
All three additions are trailing optional/nullable parameters, so this is
non-breaking for existing callers.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the gap that made the consuming app bypass this package for
Microsoft.Web/sitesentirely.The problem
WebSitePayload::toAzureBody()emits onlylocation,kind,propertiesandtags. There is no way to inject a top-level key, andpropertiesnests underproperties, soidentitycould not be smuggled in. On the read side,WebSiteData::fromAzure()read noidentityat all — a repo-wide search forprincipalIdreturned onlyRoleAssignmentData,RoleAssignmentPayloadandUserAssignedIdentityData; no DTO read a top-levelidentityblock off any resource.The app needs
identity.principalIdto grant a Function App's system-assigned managed identity storage RBAC, and hard-fails without it. So it hand-rolled its own ARM requests and skipped this package'ssitessurface.The change
WebSitePayload— trailing?string $identityType = null; emitsidentity: { type }when non-null. Copied verbatim from the existingCognitiveServicesAccountPayload/FoundryProjectPayloadshape, so this is the pattern the codebase already blesses rather than a new one.WebSiteData— trailing nullableidentityType,identityPrincipalId,identityTenantId, read via the existingField::arrNullableString()helpers.FunctionAppResource::createOrUpdate()— trailing optional?string $identityType = null.docs/usage/functions-and-web-apps.md— a short section showingcreateOrUpdate(..., identityType:)→$site->identityPrincipalId→roleAssignments()->create(), which is the exact flow that motivated this.All additions are trailing optional/nullable parameters — non-breaking for both positional and named callers.
Gates
composer inventory:paritygenerate-api-reference.php --checkcomposer testcomposer test:coverage--min=100pint --testNotes
docs/inventory-parity.mdis rewritten byinventory:parityeven in--checkmode; the only delta was theGenerated at:line, so it was reverted rather than committing timestamp-only churn.not->toHaveKey('identity')on the pre-existing body test. The false branch was already executed by existing tests, so coverage was satisfied without it — the assertion was added so the null branch is genuinely pinned rather than merely traversed.composer test:coverageneeds Herd's xdebug.sopassed explicitly via-d zend_extension=....Merging this auto-tags a release (
release.ymlrunsgithub-tag-actionon PR close againstmain,DEFAULT_BUMP: minor), which is why the work was put on a branch rather than committed tomain.🤖 Generated with Claude Code