[Network]: Command Onboard for Private Endpoint and Private Link Service #10258
Conversation
mmyyrroonn
requested review from
tjprescott,
yugangw-msft,
yonzhan,
zikalino and
Juliehzl
mmyyrroonn
force-pushed
the
NetworkPrivateEndpointsLinkService
branch
from
d5752bd to
03cb2f4
Compare
…ble-polices for subnet
mmyyrroonn
force-pushed
the
NetworkPrivateEndpointsLinkService
branch
from
03cb2f4 to
ec29b16
Compare
| @@ -1130,6 +1170,8 @@ def load_arguments(self, _): | |||
| c.argument('service_endpoints', nargs='+', min_api='2017-06-01') | |||
| c.argument('service_endpoint_policy', nargs='+', min_api='2018-07-01', help='Space-separated list of names or IDs of service endpoint policies to apply.', validator=validate_service_endpoint_policy) | |||
| c.argument('delegations', nargs='+', min_api='2017-08-01', help='Space-separated list of services to whom the subnet should be delegated. (e.g. Microsoft.Sql/servers)', validator=validate_delegations) | |||
| c.argument('private_endpoint_network_policies', arg_type=get_enum_type(['Enabled', 'Disabled']), min_api='2019-04-01', help='Enable or Disable private endpoint on the subnet.') | |||
There was a problem hiding this comment.
wouldn't it be better to have --enable-private-endpoint-network-policies and --enable-private-link-service-network-policies without enum?
There was a problem hiding this comment.
good idea. I will solve this.
| short-summary: Update a private link service endpoint connection. | ||
| examples: | ||
| - name: Update the endpoint connections status of private link service | ||
| text: az network private-link-service connection update -g MyResourceGroup -n tttt.f072a430-2d82-4470-ab30-d23fcfee58d1 --service-name MyPLSName --connection-status Rejected |
There was a problem hiding this comment.
then name seems complex for some reason "tttt.f072a430-2d82-4470-ab30-d23fcfee58d1"
| short-summary: Create a private endpoint. | ||
| examples: | ||
| - name: Create a private endpoint. | ||
| text: az network private-endpoint create -g MyResourceGroup -n MyPE --vnet-name MyVnetName --subnet MySubnet --private-connection-resource-id MyPLSId --connection-name tttt -l centralus |
There was a problem hiding this comment.
just wondering about this one:
--private-connection-resource-id MyPLSId
shouldn't that be resource id?
There was a problem hiding this comment.
yes. I will change this example.
| @@ -1130,6 +1170,8 @@ def load_arguments(self, _): | |||
| c.argument('service_endpoints', nargs='+', min_api='2017-06-01') | |||
| c.argument('service_endpoint_policy', nargs='+', min_api='2018-07-01', help='Space-separated list of names or IDs of service endpoint policies to apply.', validator=validate_service_endpoint_policy) | |||
| c.argument('delegations', nargs='+', min_api='2017-08-01', help='Space-separated list of services to whom the subnet should be delegated. (e.g. Microsoft.Sql/servers)', validator=validate_delegations) | |||
| c.argument('disable_private_endpoint_network_policies', arg_type=get_three_state_flag(positive_label='Disabled', negative_label='Enabled'), min_api='2019-04-01', help='Disable private endpoint network policies on the subnet.') | |||
There was a problem hiding this comment.
hmm... so we don't need enum, true, false would be just right
| # Create PLS | ||
| self.cmd('network vnet create -g {rg} -n {vnet} --subnet-name {subnet1} -l {location}') | ||
| self.cmd('network lb create -g {rg} -l {location} -n {lb} --public-ip-address {ip} --sku {sku}') | ||
| self.cmd('network vnet subnet update -g {rg} -n {subnet1} --vnet-name {vnet} --disable-private-endpoint-network-policies') |
There was a problem hiding this comment.
@zikalino User can just use --disable-private-endpoint-network-policies without Disabled or Enabled. I think true and false are also enum type.
There was a problem hiding this comment.
sorry, I didn't think straight when reading this code :-)
There was a problem hiding this comment.
😃 We can talk about this before the release. I agree this kind of usage --disable-xxx-xxx Disabled is a little bit strange. So we prefer just using the --enable-xxx-xxx true or --enable-xxx-xxx false right? Because for this parameter, the default value is Enabled and user should be able to disable it. Maybe --enable-private-endpoint-network-policies false is better?
There was a problem hiding this comment.
@zikalino I think --enable-private-endpoint-network-polices true is better than --disable-private-endpoint-network policies false. However, it's a little bit complex to use --enable-private-endpoint-network-policies false than --disable-private-endpoint-network-policies. We can make a contract and I will open another PR to fix this. 😄
Fixes #9474
Fixes #9475
This checklist is used to make sure that common guidelines for a pull request are followed.
The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).
I adhere to the Command Guidelines.