fix: 🐛 Validate deployment parameters with source type

src/spring-cloud/azext_spring_cloud/_deployment_source_factory.py

@@ -5,14 +5,27 @@
 
 # pylint: disable=wrong-import-order
 from .vendored_sdks.appplatform.v2022_01_01_preview import models
+from azure.cli.core.azclierror import (ArgumentUsageError)
+from ._utils import convert_argument_to_parameter_list
 
 
 class BaseSource:
     def fulfilled_options_from_original_source_info(self, **_):
         return {}
 
+    def validate_source(self, **_):
+        pass
+
 
 class JarSource(BaseSource):
+    def validate_source(self, **kwargs):
+        invalid_input = {k: v for k, v in kwargs.items() if k in ['main_entry', 'target_module'] and v is not None}
+        if any(invalid_input):
+            invalid_input_str = convert_argument_to_parameter_list(invalid_input.keys())
+            runtime_version = kwargs.get('runtime_version') or kwargs.get('deployment_resource').properties.source.runtime_version
+            raise ArgumentUsageError('{} cannot be set when --runtime-version is {}.'
+                                     .format(invalid_input_str, runtime_version))
+
     def format_source(self, deployable_path=None, runtime_version=None, version=None, jvm_options=None, **_):
         if all(x is None for x in [deployable_path, runtime_version, version, jvm_options]):
             return
@@ -37,6 +50,14 @@ def fulfilled_options_from_original_source_info(self, deployment_resource,
 
 
 class NetCoreZipSource(BaseSource):
+    def validate_source(self, **kwargs):
+        invalid_input = {k: v for k, v in kwargs.items() if k in ['jvm_options'] and v is not None}
+        if any(invalid_input):
+            invalid_input_str = convert_argument_to_parameter_list(invalid_input.keys())
+            runtime_version = kwargs.get('runtime_version') or kwargs.get('deployment_resource').properties.source.runtime_version
+            raise ArgumentUsageError('{} cannot be set when --runtime-version is {}.'
+                                     .format(invalid_input_str, runtime_version))
+
     def format_source(self, deployable_path=None, main_entry=None, version=None, runtime_version=None, **_):
         if all(x is None for x in [deployable_path, main_entry, version]):
             return None
@@ -61,6 +82,13 @@ def fulfilled_options_from_original_source_info(self, deployment_resource,
 
 
 class CustomContainerSource(BaseSource):
+    def validate_source(self, **kwargs):
+        invalid_input = {k: v for k, v in kwargs.items() if k in ['jvm_options', 'main_entry', 'target_module'] and v is not None}
+        if any(invalid_input):
+            invalid_input_str = convert_argument_to_parameter_list(invalid_input.keys())
+            raise ArgumentUsageError('{} cannot be set when --container-image is set.'
+                                     .format(invalid_input_str))
+
     def format_source(self, version=None, **kwargs):
         container = self._format_container(**kwargs)
         if all(x is None for x in [container, version]):
@@ -101,6 +129,13 @@ def format_source(self, deployable_path=None, version=None, **_):
 
 
 class SourceBuild(BaseSource):
+    def validate_source(self, **kwargs):
+        invalid_input = {k: v for k, v in kwargs.items() if k in ['jvm_options', 'main_entry'] and v is not None}
+        if any(invalid_input):
+            invalid_input_str = convert_argument_to_parameter_list(invalid_input.keys())
+            raise ArgumentUsageError('{} cannot be set when built from source.'
+                                     .format(invalid_input_str))
+
     def format_source(self, deployable_path=None, target_module=None, runtime_version=None, version=None, **_):
         if all(x is None for x in [deployable_path, target_module, runtime_version, version]):
             return None

src/spring-cloud/azext_spring_cloud/_params.py

@@ -9,7 +9,7 @@
 from azure.cli.core.commands.parameters import (name_type, get_location_type, resource_group_name_type)
 from ._validators import (validate_env, validate_cosmos_type, validate_resource_id, validate_location,
                           validate_name, validate_app_name, validate_deployment_name, validate_log_lines,
-                          validate_log_limit, validate_log_since, validate_sku, validate_jvm_options,
+                          validate_log_limit, validate_log_since, validate_sku, normalize_sku, validate_jvm_options,
                           validate_vnet, validate_vnet_required_parameters, validate_node_resource_group,
                           validate_tracing_parameters_asc_create, validate_tracing_parameters_asc_update,
                           validate_app_insights_parameters, validate_instance_count, validate_java_agent_parameters,
@@ -38,7 +38,7 @@
     validator=validate_env, help="Space-separated environment variables in 'key[=value]' format.", nargs='*')
 service_name_type = CLIArgumentType(options_list=['--service', '-s'], help='Name of Azure Spring Cloud, you can configure the default service using az configure --defaults spring-cloud=<name>.', configured_default='spring-cloud')
 app_name_type = CLIArgumentType(help='App name, you can configure the default app using az configure --defaults spring-cloud-app=<name>.', validator=validate_app_name, configured_default='spring-cloud-app')
-sku_type = CLIArgumentType(arg_type=get_enum_type(['Basic', 'Standard', 'Enterprise']), validator=validate_sku, help='Name of SKU. Enterprise is still in Preview.')
+sku_type = CLIArgumentType(arg_type=get_enum_type(['Basic', 'Standard', 'Enterprise']), help='Name of SKU. Enterprise is still in Preview.')
 source_path_type = CLIArgumentType(nargs='?', const='.',
                                    help="Deploy the specified source folder. The folder will be packed into tar, uploaded, and built using kpack. Default to the current folder if no value provided.",
                                    arg_group='Source Code deploy')
@@ -59,7 +59,7 @@ def load_arguments(self, _):
     # https://dev.azure.com/msazure/AzureDMSS/_workitems/edit/11002857/
     with self.argument_context('spring-cloud create') as c:
         c.argument('location', arg_type=get_location_type(self.cli_ctx), validator=validate_location)
-        c.argument('sku', arg_type=sku_type, default='Standard')
+        c.argument('sku', arg_type=sku_type, default='Standard', validator=validate_sku)
         c.argument('reserved_cidr_range', arg_group='VNet Injection', help='Comma-separated list of IP address ranges in CIDR format. The IP ranges are reserved to host underlying Azure Spring Cloud infrastructure, which should be 3 at least /16 unused IP ranges, must not overlap with any Subnet IP ranges.', validator=validate_vnet_required_parameters)
         c.argument('vnet', arg_group='VNet Injection', help='The name or ID of an existing Virtual Network into which to deploy the Spring Cloud instance.', validator=validate_vnet_required_parameters)
         c.argument('app_subnet', arg_group='VNet Injection', help='The name or ID of an existing subnet in "vnet" into which to deploy the Spring Cloud app. Required when deploying into a Virtual Network. Smaller subnet sizes are supported, please refer: https://aka.ms/azure-spring-cloud-smaller-subnet-vnet-docs', validator=validate_vnet_required_parameters)
@@ -140,7 +140,7 @@ def load_arguments(self, _):
                    help='(Enterprise Tier Only) Number of API portal instances.')
 
     with self.argument_context('spring-cloud update') as c:
-        c.argument('sku', arg_type=sku_type)
+        c.argument('sku', arg_type=sku_type, validator=normalize_sku)
         c.argument('app_insights_key',
                    help="Connection string (recommended) or Instrumentation key of the existing Application Insights.",
                    validator=validate_tracing_parameters_asc_update,

src/spring-cloud/azext_spring_cloud/_validators.py

@@ -48,7 +48,12 @@ def validate_sku(cmd, namespace):
         _validate_terms(cmd, namespace)
     else:
         _check_tanzu_components_not_enable(cmd, namespace)
-    namespace.sku = models.Sku(name=_get_sku_name(namespace.sku), tier=namespace.sku)
+    normalize_sku(cmd, namespace)
+
+
+def normalize_sku(cmd, namespace):
+    if namespace.sku:
+        namespace.sku = models.Sku(name=_get_sku_name(namespace.sku), tier=namespace.sku)
 
 
 def _validate_saas_provider(cmd, namespace):

src/spring-cloud/azext_spring_cloud/app.py

@@ -75,6 +75,7 @@ def app_create(cmd, client, resource_group, service, name,
         'enable_temporary_disk': True,
         'enable_persistent_storage': enable_persistent_storage,
         'persistent_storage': persistent_storage,
+        'public': assign_endpoint,
         'loaded_public_certificate_file': loaded_public_certificate_file
     }
     create_deployment_kwargs = {
@@ -88,6 +89,7 @@ def app_create(cmd, client, resource_group, service, name,
         'jvm_options': jvm_options,
     }
     update_app_kwargs = {
+        'enable_persistent_storage': enable_persistent_storage,
         'public': assign_endpoint,
     }
 
@@ -186,12 +188,13 @@ def app_update(cmd, client, resource_group, service, name,
                              .fulfilled_options_from_original_source_info(**deployment_kwargs, **basic_kwargs))
 
     app_resource = app_factory.format_resource(**app_kwargs, **basic_kwargs)
+    deployment_factory.source_factory.validate_source(**deployment_kwargs, **basic_kwargs)
     deployment_resource = deployment_factory.format_resource(**deployment_kwargs, **basic_kwargs)
 
     pollers = [
         client.apps.begin_update(resource_group, service, name, app_resource)
     ]
-    if deployment_kwargs:
+    if deployment:
         pollers.append(client.deployments.begin_update(resource_group,
                                                        service,
                                                        name,

src/spring-cloud/azext_spring_cloud/tests/latest/test_asc_app.py

@@ -120,6 +120,19 @@ def test_app_deploy_net(self, file_mock):
         self.assertEqual('NetCore_31', resource.properties.source.runtime_version)
         self.assertEqual('test', resource.properties.source.net_core_main_entry_path)
 
+    @mock.patch('azext_spring_cloud._deployment_uploadable_factory.FileUpload.upload_and_build')
+    def test_app_deploy_net_with_jvm_options(self, file_mock):
+        file_mock.return_value = mock.MagicMock()
+        deployment = self._get_deployment()
+        deployment.properties.source.jvm_options = 'test-options'
+        self._execute('rg', 'asc', 'app', deployment=deployment, artifact_path='my-path', runtime_version='NetCore_31', main_entry='test')
+        resource = self.patch_deployment_resource
+        self.assertEqual('NetCoreZip', resource.properties.source.type)
+        self.assertEqual('my-relative-path', resource.properties.source.relative_path)
+        self.assertIsNone(resource.properties.source.version)
+        self.assertEqual('NetCore_31', resource.properties.source.runtime_version)
+        self.assertEqual('test', resource.properties.source.net_core_main_entry_path)
+
     @mock.patch('azext_spring_cloud._deployment_uploadable_factory.FileUpload.upload_and_build')
     def test_app_continous_deploy_net(self, file_mock):
         file_mock.return_value = mock.MagicMock()
@@ -360,7 +373,7 @@ def _execute(self, *args, **kwargs):
         app_update(_get_test_cmd(), client, *args, **kwargs)
 
         call_args = client.deployments.begin_update.call_args_list
-        if kwargs.get('deployment', None):
+        if len(call_args):
             self.assertEqual(1, len(call_args))
             self.assertEqual(5, len(call_args[0][0]))
             self.assertEqual(args[0:3] + ('default',), call_args[0][0][0:4])
@@ -441,6 +454,13 @@ def test_app_update_custom_container_deployment_with_invalid_source(self):
         self.assertIsNone(resource.properties.source)
         self.assertEqual({'key':'value'}, resource.properties.deployment_settings.environment_variables)
 
+    def test_steeltoe_app_cannot_set_jvm_options(self):
+        deployment=self._get_deployment()
+        deployment.properties.source.type = 'NetCoreZip'
+        deployment.properties.source.runtime_version = 'NetCore_31'
+        deployment.properties.source.net_core_main_entry_path = 'main-entry'
+        with self.assertRaisesRegexp(CLIError, '--jvm-options cannot be set when --runtime-version is NetCore_31.'):
+            self._execute('rg', 'asc', 'app', jvm_options='test-option', deployment=deployment)
 
 class TestAppCreate(BasicTest):
     def __init__(self, methodName: str = ...):
@@ -525,6 +545,8 @@ def test_app_with_persistent_storage(self):
         self._execute('rg', 'asc', 'app', cpu='500m', memory='2Gi', instance_count=1, enable_persistent_storage=True)
         resource = self.put_app_resource
         self.assertEqual(50, resource.properties.persistent_disk.size_in_gb)
+        resource = self.patch_app_resource
+        self.assertEqual(50, resource.properties.persistent_disk.size_in_gb)
 
     def test_app_with_persistent_storage_basic(self):
         client = self._get_basic_mock_client(sku='Basic')