microsoftgraph / msgraph-bicep-types

Repo contains Microsoft Graph resource types to integrate with bicep templates.
MIT License
46 stars 7 forks source link

securityIdentifier property in groups #118

Closed slavizh closed 5 months ago

slavizh commented 6 months ago

Is your feature request related to a problem? Please describe. There is the property securityIdentifier in Microsoft.Graph/groups@v1.0. Description of the property is 'Security identifier of the group, used in Windows scenarios'. If I do not provide the property at all and query graph after the creation of the group the value is not null but some value. Is this property a read-only or it is possible to actually change if if you want. To me it sounds like it should be read-only.

Do not know if this is a bug thus logged it as feature by asking a question.

slavizh commented 6 months ago

May be the same thing can be said for onPremisesProvisioningErrors property. It does not seems it is some property you can set up.

dkershaw10 commented 6 months ago

All onPremises properties are read-only. It says that in the docs for all the onPremises properties except onPremisesProvisioningErrors - so that's a doc bug.

I'm not familiar with securityIdentifier and how that differs from onPremisesSecurityIdentifier. My guess is this is also a doc bug, and should be read-only.

dkershaw10 commented 6 months ago

Currently we aren't documenting any of the read-only properties. We're discussing internally whether we should do this, and if we do, what the doc format will look like.

slavizh commented 6 months ago

In general read-only properties should be documented so you can output them if needed. For example in most cases ID is something that is needed in output. However the read-only ones should not appear when you configure resource as option to set them. Unfortunately this is not the only RP that has this problem but as it is in preview and more Graph resources can be added in time if you set a standard for the other teams now will make the API implementation of bicep easier to use.

dkershaw10 commented 6 months ago

Thanks @slavizh. Yeah - we actually arrived at the same conclusion. Jason has a PR to include the read-only properties, but we're just checking in with our content team on whether the documentation makes read-only properties clear enough. Will update shortly.

dkershaw10 commented 6 months ago

@slavizh we've not fixed securityIdentifier (will get to that), but the reference docs have been updated with read-only properties (in the Properties value section). Please take a look and let us know if this satisfies your needs.

slavizh commented 6 months ago

@dkershaw10 no worries. This is not so important but a nice case for adopting properly which properties are read-only or not.

dkershaw10 commented 5 months ago

@jason-dou Let's mark securityIdentifier as read-only

jason-dou commented 5 months ago

The change to mark the properties as read-only is merged. The Graph Bicep type description is expected to be updated in the upcoming Bicep version. Will also update the reference doc