Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add networksecurityperimeter and managedidentitywithdelegation common types #1505

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

AlitzelMendez
Copy link
Member

issue: #1160

@azure-sdk
Copy link
Collaborator

azure-sdk commented Sep 10, 2024

All changed packages have been documented.

  • @azure-tools/typespec-azure-resource-manager
Show changes

@azure-tools/typespec-azure-resource-manager - feature ✏️

Added common-types managed identity with delegation and network security perimeter

"items": {
"$ref": "#/definitions/NetworkSecurityPerimeterConfiguration"
},
"x-ms-identifiers": []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wasn't there before
image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to address this one but I am unsure if it is a problem on my definition or when the generation of the common types is happening.

For what I saw, "x-ms-identifiers" is included when there is not an id for the array, in this case there is an id coming from resource:

model NetworkSecurityPerimeterConfiguration extends ProxyResource {
(is not directly in there but it is pulled) even if I tried to add an id, I am not able to do so because it is already inherited from the other model, but when the common-type is getting generated it includes the x-ms-identifiers like it didn't have the id, am I missing something?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is related to this bug: #1358 as the same logic is likely to be used

If so, we probably want to fix this before we merge this PR

@azure-sdk
Copy link
Collaborator

You can try these changes here

🛝 Playground 🌐 Website 📚 Next docs

* The default NetworkSecurityPerimeterConfigurationName.
*/
@added(Versions.v5)
model NetworkSecurityPerimeterConfigurationNameParameter {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to decide how to handle the segment name in this parameter model. We might wanto to do this: #1358

Or there may be a static value that is always be used. This depends on usage inside specs

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mark, I think that is the link for the bug that you mentioned above, could you please share the link for the suggestion?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I meant a template, like this:

/**
* The name of the private endpoint connection associated with the Azure resource.
* @template Segment The resource type name for private endpoint connections (default is privateEndpointConnections)
*/
model PrivateEndpointConnectionParameter<Segment extends valueof string = "privateEndpointConnections"> {
/** The name of the private endpoint connection associated with the Azure resource. */
@path
@TypeSpec.Rest.segment(Segment)
@key("privateEndpointConnectionName")
name: string;
}

* The default NetworkSecurityPerimeterConfigurationName.
*/
@added(Versions.v5)
model NetworkSecurityPerimeterConfigurationNameParameter {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I meant a template, like this:

/**
* The name of the private endpoint connection associated with the Azure resource.
* @template Segment The resource type name for private endpoint connections (default is privateEndpointConnections)
*/
model PrivateEndpointConnectionParameter<Segment extends valueof string = "privateEndpointConnections"> {
/** The name of the private endpoint connection associated with the Azure resource. */
@path
@TypeSpec.Rest.segment(Segment)
@key("privateEndpointConnectionName")
name: string;
}

Copy link
Member

@markcowl markcowl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved pending resolution of the two remaining issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants