-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[Spring Cloud] Add buildpack-binding command for Enterprise tier. #4302
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
Changes from 8 commits
dd89387
ba6ad2b
e384b9b
b18fecc
5226d73
0a7aa27
d015c88
9aed17d
7941c6f
7f3ed5d
e689778
135aa2c
d374d52
f1683a1
59d3690
5211830
bc47c30
cfbcab3
20ed905
627334d
818094c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -14,15 +14,17 @@ | |||||||
| validate_tracing_parameters_asc_create, validate_tracing_parameters_asc_update, | ||||||||
| validate_app_insights_parameters, validate_instance_count, validate_java_agent_parameters, | ||||||||
| validate_jar) | ||||||||
| from ._validators_enterprise import (only_support_enterprise, | ||||||||
| validate_git_uri, validate_acs_patterns, | ||||||||
| validate_routes) | ||||||||
| from ._validators_enterprise import (only_support_enterprise, validate_git_uri, validate_acs_patterns, validate_routes, | ||||||||
| validate_buildpack_binding_exist, validate_buildpack_binding_not_exist, | ||||||||
| validate_buildpack_binding_properties, validate_buildpack_binding_secrets) | ||||||||
| from ._app_validator import (fulfill_deployment_param, active_deployment_exist, active_deployment_exist_under_app, | ||||||||
| ensure_not_active_deployment, validate_deloy_path, validate_deloyment_create_path, | ||||||||
| validate_cpu, validate_memory) | ||||||||
| from ._utils import ApiType | ||||||||
|
|
||||||||
| from .vendored_sdks.appplatform.v2020_07_01.models import RuntimeVersion, TestKeyType | ||||||||
| from .vendored_sdks.appplatform.v2022_01_01_preview.models \ | ||||||||
| import _app_platform_management_client_enums as v20220101_preview_AppPlatformEnums | ||||||||
|
|
||||||||
| name_type = CLIArgumentType(options_list=[ | ||||||||
| '--name', '-n'], help='The primary resource name', validator=validate_name) | ||||||||
|
|
@@ -424,6 +426,12 @@ def prepare_logs_argument(c): | |||||||
| with self.argument_context('spring-cloud {}'.format(scope)) as c: | ||||||||
| c.argument('service', service_name_type, validator=only_support_enterprise) | ||||||||
|
|
||||||||
| with self.argument_context('spring-cloud service-registry bind') as c: | ||||||||
| c.argument('app', app_name_type, help='Name of app.', validator=validate_app_name) | ||||||||
|
|
||||||||
| with self.argument_context('spring-cloud service-registry unbind') as c: | ||||||||
| c.argument('app', app_name_type, help='Name of app.', validator=validate_app_name) | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Duplicate with line 437, please delete line 429-433 |
||||||||
|
|
||||||||
| for scope in ['bind', 'unbind']: | ||||||||
| with self.argument_context('spring-cloud service-registry {}'.format(scope)) as c: | ||||||||
| c.argument('app', app_name_type, help='Name of app.', validator=validate_app_name) | ||||||||
|
|
@@ -500,3 +508,55 @@ def prepare_logs_argument(c): | |||||||
| c.argument('app_name', type=str, help="The Azure Spring Cloud app name to configure the route.") | ||||||||
| c.argument('routes_json', type=str, help="The JSON array of API routes.", validator=validate_routes) | ||||||||
| c.argument('routes_file', type=str, help="The file path of JSON array of API routes.", validator=validate_routes) | ||||||||
|
|
||||||||
| for scope in ['spring-cloud build-service builder buildpack-binding create']: | ||||||||
| with self.argument_context(scope) as c: | ||||||||
| c.argument('type', | ||||||||
| arg_type=get_enum_type(v20220101_preview_AppPlatformEnums.BindingType), | ||||||||
| help='Required type for buildpack binding.') | ||||||||
| c.argument('properties', | ||||||||
| help='Non-sensitive properties for launchProperties. Format "key[=value]".', | ||||||||
| nargs='*', | ||||||||
| validator=validate_buildpack_binding_properties) | ||||||||
| c.argument('secrets', | ||||||||
| help='Sensitive properties for launchProperties. ' | ||||||||
| 'Once put, it will be encrypted and never return to user. ' | ||||||||
| 'Format "key[=value]".', | ||||||||
| nargs='*', | ||||||||
| validator=validate_buildpack_binding_secrets) | ||||||||
| c.argument('name', help='Name for buildpack binding.', validator=validate_buildpack_binding_not_exist) | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated. |
||||||||
| c.argument('builder_name', help='The name for builder.', default="default") | ||||||||
| c.argument('service', service_name_type, validator=only_support_enterprise) | ||||||||
|
|
||||||||
|
|
||||||||
| for scope in ['spring-cloud build-service builder buildpack-binding set']: | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's better to merge
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One question, is this the convention to merge such kinds of statements? If so, I can align this. Could you please help to enlighten me what is the best practice here?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi Li,
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure, updated. |
||||||||
| with self.argument_context(scope) as c: | ||||||||
| c.argument('type', | ||||||||
| arg_type=get_enum_type(v20220101_preview_AppPlatformEnums.BindingType), | ||||||||
| help='Required type for buildpack binding.') | ||||||||
| c.argument('properties', | ||||||||
| help='Non-sensitive properties for launchProperties. Format "key[=value]".', | ||||||||
| nargs='*', | ||||||||
| validator=validate_buildpack_binding_properties) | ||||||||
| c.argument('secrets', | ||||||||
| help='Sensitive properties for launchProperties. ' | ||||||||
| 'Once put, it will be encrypted and never return to user. ' | ||||||||
| 'Format "key[=value]".', | ||||||||
| nargs='*', | ||||||||
| validator=validate_buildpack_binding_secrets) | ||||||||
| c.argument('name', help='Name for buildpack binding.', validator=validate_buildpack_binding_exist) | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated. |
||||||||
| c.argument('builder_name', help='The name for builder.', default="default") | ||||||||
| c.argument('service', service_name_type, validator=only_support_enterprise) | ||||||||
|
|
||||||||
|
|
||||||||
| for scope in ['spring-cloud build-service builder buildpack-binding show', | ||||||||
| 'spring-cloud build-service builder buildpack-binding delete']: | ||||||||
| with self.argument_context(scope) as c: | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| c.argument('name', help='Name for buildpack binding.', validator=validate_buildpack_binding_exist) | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated. |
||||||||
| c.argument('builder_name', help='The name for builder.', default="default") | ||||||||
| c.argument('service', service_name_type, validator=only_support_enterprise) | ||||||||
|
|
||||||||
| for scope in ['spring-cloud build-service builder buildpack-binding list']: | ||||||||
| with self.argument_context(scope) as c: | ||||||||
| c.argument('builder_name', help='The name for builder.', default="default") | ||||||||
| c.argument('service', service_name_type, validator=only_support_enterprise) | ||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,9 @@ | |
| # pylint: disable=too-few-public-methods, unused-argument, redefined-builtin | ||
|
|
||
| from re import match | ||
|
|
||
| from azure.cli.core.commands.validators import validate_tag | ||
| from azure.core.exceptions import ResourceNotFoundError | ||
| from azure.cli.core.util import CLIError | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Please remove the useless import
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated. |
||
| from azure.cli.core.azclierror import (ArgumentUsageError, ClientRequestError, | ||
| InvalidArgumentValueError, | ||
| MutuallyExclusiveArgumentError) | ||
|
|
@@ -15,8 +17,11 @@ | |
| from ._resource_quantity import validate_cpu as validate_and_normalize_cpu | ||
| from ._resource_quantity import \ | ||
| validate_memory as validate_and_normalize_memory | ||
| from ._util_enterprise import is_enterprise_tier | ||
| from ._util_enterprise import ( | ||
| is_enterprise_tier, get_client | ||
| ) | ||
| from ._validators import validate_instance_count | ||
| from .buildpack_binding import (DEFAULT_BUILD_SERVICE_NAME) | ||
|
|
||
| logger = get_logger(__name__) | ||
|
|
||
|
|
@@ -116,3 +121,48 @@ def validate_api_portal_instance_count(namespace): | |
| if namespace.api_portal_instance_count is not None: | ||
| if namespace.api_portal_instance_count < 1: | ||
| raise InvalidArgumentValueError("--api-portal-instance-count must be greater than 0") | ||
|
|
||
|
|
||
| def validate_buildpack_binding_properties(namespace): | ||
| """ Extracts multiple space-separated properties in key[=value] format """ | ||
| if isinstance(namespace.properties, list): | ||
| properties_dict = {} | ||
| for item in namespace.properties: | ||
| properties_dict.update(validate_tag(item)) | ||
| namespace.properties = properties_dict | ||
|
|
||
|
|
||
| def validate_buildpack_binding_secrets(namespace): | ||
| """ Extracts multiple space-separated secrets in key[=value] format """ | ||
| if isinstance(namespace.secrets, list): | ||
| secrets_dict = {} | ||
| for item in namespace.secrets: | ||
| secrets_dict.update(validate_tag(item)) | ||
| namespace.secrets = secrets_dict | ||
|
|
||
|
|
||
| def validate_buildpack_binding_not_exist(cmd, namespace): | ||
| client = get_client(cmd) | ||
| try: | ||
| binding_resource = client.buildpack_binding.get(namespace.resource_group, | ||
| namespace.service, | ||
| DEFAULT_BUILD_SERVICE_NAME, | ||
| namespace.builder_name, | ||
| namespace.name) | ||
| if binding_resource is not None: | ||
| raise CLIError('buildpack Binding {} in builder {} already exists ' | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please use a specific error type instead of CLIError ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated. |
||
| 'in resource group {}, service {}. You can edit it by set command.' | ||
| .format(namespace.name, namespace.resource_group, namespace.service, namespace.builder_name)) | ||
| except ResourceNotFoundError: | ||
| # Excepted case | ||
| pass | ||
|
|
||
|
|
||
| def validate_buildpack_binding_exist(cmd, namespace): | ||
| client = get_client(cmd) | ||
| # If not exists exception will be raised | ||
| client.buildpack_binding.get(namespace.resource_group, | ||
| namespace.service, | ||
| DEFAULT_BUILD_SERVICE_NAME, | ||
| namespace.builder_name, | ||
| namespace.name) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the description here
Showrather thanlist?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.