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

[AppService] functionapp create: Check if storage account is network restricted #30605

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from
Open
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,8 @@ def load_arguments(self, _):
"string. For the system assigned managed-identity authentication type, this parameter is not applicable and should be left empty.", is_preview=True)
c.argument('zone_redundant', arg_type=get_three_state_flag(),
help='Enable zone redundancy for high availability. Applies to Flex Consumption SKU only.', is_preview=True)
c.argument('configure_networking_later', options_list=['--configure-networking-later', '--cnl'], arg_type=get_three_state_flag(),
help='Use this option if you want to configure networking later for an app using network-restricted storage.')

with self.argument_context('functionapp deployment config set') as c:
c.argument('deployment_storage_name', options_list=['--deployment-storage-name', '--dsn'], help="The deployment storage account name.", is_preview=True)
Expand Down
46 changes: 44 additions & 2 deletions src/azure-cli/azure/cli/command_modules/appservice/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -4773,7 +4773,7 @@ def create_functionapp(cmd, resource_group_name, name, storage_account, plan=Non
always_ready_instances=None, maximum_instance_count=None, instance_memory=None,
flexconsumption_location=None, deployment_storage_name=None,
deployment_storage_container_name=None, deployment_storage_auth_type=None,
deployment_storage_auth_value=None, zone_redundant=False):
deployment_storage_auth_value=None, zone_redundant=False, configure_networking_later=None):
# pylint: disable=too-many-statements, too-many-branches

if functions_version is None and flexconsumption_location is None:
Expand Down Expand Up @@ -4990,6 +4990,22 @@ def create_functionapp(cmd, resource_group_name, name, storage_account, plan=Non
else SiteConfigPropertiesDictionary()
app_settings_dict = matched_runtime.app_settings_dict if not flexconsumption_location else {}

if is_storage_account_network_restricted(cmd.cli_ctx, resource_group_name, storage_account):
if consumption_plan_location is not None:
raise ValidationError('The Consumption plan does not support storage accounts with network restrictions. '
'If you wish to use virtual networks, please create your app on a different hosting '
'plan.')

if not vnet and not configure_networking_later:
raise ValidationError('The storage account you selected "{}" has networking restrictions. No virtual '
'networking was configured so your app will not start. Please try again with '
'virtual networking integration by adding the --vnet and --subnet flags. If '
'you wish to do this at a later time, use the --configure-networking-later '
'flag instead.'.format(storage_account))
if vnet and configure_networking_later:
raise ValidationError('The --vnet and --configure-networking-later flags are mutually exclusive.')
functionapp_def.vnet_content_share_enabled = True

con_string = _validate_and_get_connection_string(cmd.cli_ctx, resource_group_name, storage_account)

if environment is not None:
Expand Down Expand Up @@ -5128,7 +5144,10 @@ def create_functionapp(cmd, resource_group_name, name, storage_account, plan=Non
if (plan_info is not None and is_plan_elastic_premium(cmd, plan_info)) or consumption_plan_location is not None:
site_config.app_settings.append(NameValuePair(name='WEBSITE_CONTENTAZUREFILECONNECTIONSTRING',
value=con_string))
site_config.app_settings.append(NameValuePair(name='WEBSITE_CONTENTSHARE', value=_get_content_share_name(name)))
content_share_name = _get_content_share_name(name)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this function may cause the dismatch between resource name and the recording file.

def _get_content_share_name(app_name):
# content share name should be up to 63 characters long, lowercase letter and digits, and random
# so take the first 50 characters of the app name and add the last 12 digits of a random uuid
share_name = app_name[0:50]
suffix = str(uuid.uuid4()).rsplit('-', maxsplit=1)[-1]
return share_name.lower() + suffix

Copy link
Contributor Author

@kamperiadis kamperiadis Jan 24, 2025

Choose a reason for hiding this comment

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

Hi @yanzhudd We actually do intend to create a new content share resource with a new name on behalf of the customer. Does this mean we should just make the tests run live? Or what would you require for this PR to be merged?

Copy link
Contributor

@yanzhudd yanzhudd Jan 26, 2025

Choose a reason for hiding this comment

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

yes, you can mark the tests as live only, and please remove the related recording files as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you @yanzhudd Those tests are already marked as live_only and there are no recording files for them.

site_config.app_settings.append(NameValuePair(name='WEBSITE_CONTENTSHARE', value=content_share_name))
if is_storage_account_network_restricted(cmd.cli_ctx, resource_group_name, storage_account):
create_file_share(cmd.cli_ctx, resource_group_name, storage_account, content_share_name)

create_app_insights = False

Expand Down Expand Up @@ -5357,6 +5376,16 @@ def _validate_cpu_momory_functionapp(cpu=None, memory=None):
return


def create_file_share(cli_ctx, resource_group_name, storage_account, share_name):

storage_client = get_mgmt_service_client(cli_ctx, StorageManagementClient)
from azure.mgmt.storage.models import FileShare

file_share = FileShare()

return storage_client.file_shares.create(resource_group_name, storage_account, share_name, file_share=file_share)


def _get_extension_version_functionapp(functions_version):
if functions_version is not None:
return '~{}'.format(functions_version)
Expand Down Expand Up @@ -5703,6 +5732,19 @@ def _validate_and_get_connection_string(cli_ctx, resource_group_name, storage_ac
return connection_string


def is_storage_account_network_restricted(cli_ctx, resource_group_name, storage_account):
sa_resource_group = resource_group_name
if is_valid_resource_id(storage_account):
sa_resource_group = parse_resource_id(storage_account)['resource_group']
storage_account = parse_resource_id(storage_account)['name']
storage_client = get_mgmt_service_client(cli_ctx, StorageManagementClient)
storage_properties = storage_client.storage_accounts.get_properties(sa_resource_group,
storage_account)
return storage_properties.public_network_access == 'Disabled' or \
(storage_properties.public_network_access == 'Enabled' and
storage_properties.network_rule_set.default_action == 'Deny')


def list_consumption_locations(cmd):
client = web_client_factory(cmd.cli_ctx)
regions = client.list_geo_regions(sku='Dynamic')
Expand Down

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Loading