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

spring-cloud: add support to enable/disable MSI #1523

Merged
merged 6 commits into from
Apr 16, 2020
Merged

spring-cloud: add support to enable/disable MSI #1523

merged 6 commits into from
Apr 16, 2020

Conversation

leonard520
Copy link
Contributor

@leonard520 leonard520 commented Apr 8, 2020


This checklist is used to make sure that common guidelines for a pull request are followed.

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally?

For new extensions:

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your PR is merged into master branch, a new PR will be created to update src/index.json automatically.
The precondition is to put your code inside this repo and upgrade the version in the PR but do not modify src/index.json.


helps['spring-cloud app identity remove'] = """
type: command
short-summary: Remove managed service identity from a app.
Copy link

Choose a reason for hiding this comment

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

examples

Copy link

Choose a reason for hiding this comment

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

s/a/an

@@ -45,6 +45,8 @@ def load_arguments(self, _):
with self.argument_context('spring-cloud app create') as c:
c.argument(
'is_public', arg_type=get_three_state_flag(), help='If true, assign public domain', default=False)
c.argument('assign_identity', arg_type=get_three_state_flag(),
help='Generate and assign an Azure Active Directory Identity for this app for use with key management services like Azure KeyVault.')
Copy link

Choose a reason for hiding this comment

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

change the help text here too?


none = "None"
system_assigned = "SystemAssigned"
user_assigned = "UserAssigned"
Copy link

@xgugeng xgugeng Apr 8, 2020

Choose a reason for hiding this comment

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

keep none and system_assigned only, since the other two is not supported yet. Never mind, our backend handles the other two well

@yungezz yungezz added the Spring Cloud Spring Cloud related issues label Apr 8, 2020
@yungezz
Copy link
Member

yungezz commented Apr 8, 2020

hi @fengzhou-msft could you pls help to look at it? thanks

Copy link
Member

@yungezz yungezz left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -425,6 +439,37 @@ def app_tail_log(cmd, client, resource_group, service, name, instance=None, foll
raise exceptions[0]


def app_identity_assign(cmd, client, resource_group, service, name):
app_resource = models.AppResource()
identity = models.ManagedIdentityProperties(type="systemassigned")
Copy link
Member

Choose a reason for hiding this comment

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

do you have plan to support other type? if yes better to expose type as a param with default value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not yet. But we can refactor code if we will support in future.

"""

helps['spring-cloud app identity assign'] = """
type: command
short-summary: Enable managed service identity on an app.
examples:
- name: Enable managed service identity on an app.
Copy link

@xgugeng xgugeng Apr 9, 2020

Choose a reason for hiding this comment

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

The example provided is for system-assigned MSI only

@@ -45,6 +45,8 @@ def load_arguments(self, _):
with self.argument_context('spring-cloud app create') as c:
c.argument(
'is_public', arg_type=get_three_state_flag(), help='If true, assign public domain', default=False)
c.argument('assign_identity', arg_type=get_three_state_flag(),
help='Manage an app\'s managed service identity.')
Copy link

Choose a reason for hiding this comment

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

If true, assign managed service identity.

Xiaoyun Ding added 2 commits April 9, 2020 16:45
@leonard520
Copy link
Contributor Author

@hellokangning @sophiaso @yungezz Please kindly help to review. Thanks.

@yonzhan yonzhan added this to the S168 milestone Apr 13, 2020
@yonzhan
Copy link
Collaborator

yonzhan commented Apr 13, 2020

add to S168

@fengzhou-msft fengzhou-msft merged commit 666bab7 into Azure:master Apr 16, 2020
qwordy pushed a commit that referenced this pull request Apr 30, 2020
qwordy added a commit that referenced this pull request May 27, 2020
* spring-cloud: add support to enable/disable MSI (#1523)

* Update src/spring-cloud/HISTORY.md

Co-authored-by: Feiyue Yu <[email protected]>

Co-authored-by: Xiaoyun Ding <[email protected]>
Co-authored-by: Feiyue Yu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Spring Cloud Spring Cloud related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants