Azure / azure-rest-api-specs

The source for REST API specifications for Microsoft Azure.
MIT License
2.61k stars 5.03k forks source link

[EventGrid] API design issues for the `identity` block #13340

Open tombuildsstuff opened 3 years ago

tombuildsstuff commented 3 years ago

πŸ‘‹

The Swagger definitions for Identities within the EventGrid API (this Swagger) appear to disregard the conventions used for identity blocks elsewhere in this codebase, such that a normal Identity block gets generated as:

// ManagedServiceIdentity managed service identity.
type ManagedServiceIdentity struct {
    // Type - Type of managed service identity. Possible values include: 'ManagedServiceIdentityTypeSystemAssigned', 'ManagedServiceIdentityTypeUserAssigned', 'ManagedServiceIdentityTypeSystemAssignedUserAssigned', 'ManagedServiceIdentityTypeNone'
    Type ManagedServiceIdentityType `json:"type,omitempty"`
    // TenantID - READ-ONLY; Tenant of managed service identity.
    TenantID *string `json:"tenantId,omitempty"`
    // PrincipalID - READ-ONLY; Principal Id of managed service identity.
    PrincipalID *string `json:"principalId,omitempty"`
    // UserAssignedIdentities - The list of user assigned identities associated with the resource. The user identity dictionary key references will be ARM resource ids in the form: '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.ManagedIdentity/userAssignedIdentities/{identityName}
    UserAssignedIdentities map[string]*ManagedServiceIdentityUserAssignedIdentitiesValue `json:"userAssignedIdentities"`
}

but EventGrid instead generates:

// EventSubscriptionIdentity the identity information with the event subscription.
type EventSubscriptionIdentity struct {
    // Type - The type of managed identity used. The type 'SystemAssigned, UserAssigned' includes both an implicitly created identity and a set of user-assigned identities. The type 'None' will remove any identity. Possible values include: 'SystemAssigned', 'UserAssigned'
    Type EventSubscriptionIdentityType `json:"type,omitempty"`
    // UserAssignedIdentity - The user identity associated with the resource.
    UserAssignedIdentity *string `json:"userAssignedIdentity,omitempty"`
}

it's also worth noting the value defined above "None" isn't present in as a Constant, even though it's mentioned above:

// EventSubscriptionIdentityType enumerates the values for event subscription identity type.
type EventSubscriptionIdentityType string

const (
    // SystemAssigned ...
    SystemAssigned EventSubscriptionIdentityType = "SystemAssigned"
    // UserAssigned ...
    UserAssigned EventSubscriptionIdentityType = "UserAssigned"
)

Would it be possible for this to be fixed to match other API's? This appears to be both a design issue (in the API) and a process issue (that this slipped through) - do common types not exist for the identity block?

cc @JeffreyRichter

ghost commented 3 years ago

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @jfggdl.

Issue Details
πŸ‘‹ The Swagger definitions for Identities within the EventGrid API ([this Swagger](https://github.com/Azure/azure-rest-api-specs/blob/921957dfa2228c2062b8d2ef832507339c5562d4/specification/eventgrid/resource-manager/Microsoft.EventGrid/preview/2020-04-01-preview/EventGrid.json#L1)) appear to disregard the conventions used for `identity` blocks elsewhere in this codebase, such that a normal Identity block gets generated as: ``` // ManagedServiceIdentity managed service identity. type ManagedServiceIdentity struct { // Type - Type of managed service identity. Possible values include: 'ManagedServiceIdentityTypeSystemAssigned', 'ManagedServiceIdentityTypeUserAssigned', 'ManagedServiceIdentityTypeSystemAssignedUserAssigned', 'ManagedServiceIdentityTypeNone' Type ManagedServiceIdentityType `json:"type,omitempty"` // TenantID - READ-ONLY; Tenant of managed service identity. TenantID *string `json:"tenantId,omitempty"` // PrincipalID - READ-ONLY; Principal Id of managed service identity. PrincipalID *string `json:"principalId,omitempty"` // UserAssignedIdentities - The list of user assigned identities associated with the resource. The user identity dictionary key references will be ARM resource ids in the form: '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.ManagedIdentity/userAssignedIdentities/{identityName} UserAssignedIdentities map[string]*ManagedServiceIdentityUserAssignedIdentitiesValue `json:"userAssignedIdentities"` } ``` but EventGrid instead generates: ``` // EventSubscriptionIdentity the identity information with the event subscription. type EventSubscriptionIdentity struct { // Type - The type of managed identity used. The type 'SystemAssigned, UserAssigned' includes both an implicitly created identity and a set of user-assigned identities. The type 'None' will remove any identity. Possible values include: 'SystemAssigned', 'UserAssigned' Type EventSubscriptionIdentityType `json:"type,omitempty"` // UserAssignedIdentity - The user identity associated with the resource. UserAssignedIdentity *string `json:"userAssignedIdentity,omitempty"` } ``` it's also worth noting the value defined above "None" isn't present in as a Constant, even though it's mentioned above: ``` // EventSubscriptionIdentityType enumerates the values for event subscription identity type. type EventSubscriptionIdentityType string const ( // SystemAssigned ... SystemAssigned EventSubscriptionIdentityType = "SystemAssigned" // UserAssigned ... UserAssigned EventSubscriptionIdentityType = "UserAssigned" ) ``` Would it be possible for this to be fixed to match other API's? This appears to be both a design issue (in the API) and a process issue (that this slipped through) - do common types not exist for the `identity` block? cc @JeffreyRichter
Author: tombuildsstuff
Assignees: catalinaperalta, jhendrixMSFT, akning-ms
Labels: `Event Grid`, `Service Attention`, `needs-triage`, `question`
Milestone: -