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

[AD] az ad app/sp update: Support generic update --set on root level #22798

Merged
merged 1 commit into from
Jun 13, 2022

Conversation

jiasli
Copy link
Member

@jiasli jiasli commented Jun 9, 2022

Related command
az ad app update
az ad sp update

Description
Close #22758, #22777, #22635

az ad sp update has no native arguments, but only generic update arguments:

> az ad sp update -h

Command
    az ad sp update : Update a service principal.

Arguments
    --id    [Required] : Service principal name, or object id.

Generic Update Arguments
    --add              : Add an object to a list of objects by specifying a path and key value
                         pairs.  Example: --add property.listProperty <key=value, string or JSON
                         string>.
    --force-string     : When using 'set' or 'add', preserve string literals instead of attempting
                         to convert to JSON.
    --remove           : Remove a property or an element from a list.  Example: --remove
                         property.list <indexToRemove> OR --remove propertyToRemove.
    --set              : Update an object by specifying a property path and value to set.  Example:
                         --set property1.property2=<value>.

https://docs.microsoft.com/en-us/cli/azure/microsoft-graph-migration#known-issues documented that generic update arguments are not supported by now. Therefore, az ad sp update is basically a no-op.

This PR is an initial attempt to makes generic update work with the new model-less framework by supporting generic update --set for az ad app/sp update on the root level:

ad app update --id {app_id} --set isDeviceOnlyAuthSupported=true
az ad sp update --id 233dd73b-72e3-424a-9367-7588d957267e --set 'tags=["mytag"]'

@ghost ghost requested a review from yonzhan June 9, 2022 06:13
@ghost ghost added the Auto-Assign Auto assign by bot label Jun 9, 2022
@ghost ghost assigned jiasli Jun 9, 2022
@ghost ghost added this to the Jun 2022 (2022-07-05) milestone Jun 9, 2022
@ghost ghost added the RBAC az role label Jun 9, 2022
@ghost ghost requested a review from wangzelin007 June 9, 2022 06:13
@ghost ghost added the Graph az ad label Jun 9, 2022
@ghost ghost requested review from evelyn-ys and calvinhzy June 9, 2022 06:13
@@ -95,7 +95,7 @@ def application_delete(self, id):
result = self._send("DELETE", "/applications/{id}".format(id=id))
return result

def application_patch(self, id, body):
def application_update(self, id, body):
Copy link
Member Author

@jiasli jiasli Jun 9, 2022

Choose a reason for hiding this comment

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

AD Graph SDK uses verb "patch", instead of "update": azure.graphrbac.operations.applications_operations.ApplicationsOperations.patch. I guess it is used to differentiate with update_application of generic update.

We switch to use "update" to align with other update operations (such as azure.graphrbac.operations.users_operations.UsersOperations.update) and the API document https://docs.microsoft.com/en-us/graph/api/application-update.

@jiasli
Copy link
Member Author

jiasli commented Jun 9, 2022

The underlying functionality support of --set is

elif isinstance(instance, dict):
instance[name] = value

@jiasli jiasli marked this pull request as ready for review June 13, 2022 04:24
@jiasli jiasli merged commit 4680da7 into Azure:dev Jun 13, 2022
@jiasli jiasli deleted the sp-update branch June 13, 2022 06:50
@jiasli jiasli changed the title [Role] az ad app/sp update: Support generic update --set on root level [AD] az ad app/sp update: Support generic update --set on root level Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auto-Assign Auto assign by bot Graph az ad RBAC az role
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error in 'az ad sp update --set appRoleAssignmentRequired=true'
3 participants