Repository: Azure/ARO-RP
Pull request #1219 (2020-12-08 22:18:49)

Minor fixes
* az aro: don't honour AZURE_FP_CLIENT_ID when RP_MODE is unset
* update RP error message
* remove ehashman from CODEOWNERS



Patch:
diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
index e0782477c..cfb1dc4a0 100644
--- a/.github/CODEOWNERS
+++ b/.github/CODEOWNERS
@@ -1,1 +1,1 @@
-* @ehashman @jim-minter @mjudeikis @olga-mir
+* @jim-minter @mjudeikis @olga-mir
diff --git a/pkg/api/validate/openshiftcluster_validatedynamic.go b/pkg/api/validate/openshiftcluster_validatedynamic.go
index 51f419480..7151af27d 100644
--- a/pkg/api/validate/openshiftcluster_validatedynamic.go
+++ b/pkg/api/validate/openshiftcluster_validatedynamic.go
@@ -178,7 +178,7 @@ func (dv *openShiftClusterDynamicValidator) validateVnetPermissions(ctx context.
 		"Microsoft.Network/virtualNetworks/subnets/write",
 	}, authorizer, client)
 	if err == wait.ErrWaitTimeout {
-		return api.NewCloudError(http.StatusBadRequest, code, "", "The %s does not have Contributor permission on vnet '%s'.", typ, vnetID)
+		return api.NewCloudError(http.StatusBadRequest, code, "", "The %s does not have Network Contributor permission on vnet '%s'.", typ, vnetID)
 	}
 	if detailedErr, ok := err.(autorest.DetailedError); ok &&
 		detailedErr.StatusCode == http.StatusNotFound {
@@ -231,7 +231,7 @@ func (dv *openShiftClusterDynamicValidator) validateRouteTablePermissionsSubnet(
 		"Microsoft.Network/routeTables/write",
 	}, authorizer, client)
 	if err == wait.ErrWaitTimeout {
-		return api.NewCloudError(http.StatusBadRequest, code, "", "The %s does not have Contributor permission on route table '%s'.", typ, *s.RouteTable.ID)
+		return api.NewCloudError(http.StatusBadRequest, code, "", "The %s does not have Network Contributor permission on route table '%s'.", typ, *s.RouteTable.ID)
 	}
 	if detailedErr, ok := err.(autorest.DetailedError); ok &&
 		detailedErr.StatusCode == http.StatusNotFound {
diff --git a/python/az/aro/azext_aro/custom.py b/python/az/aro/azext_aro/custom.py
index bd28e4184..f26f30a2c 100644
--- a/python/az/aro/azext_aro/custom.py
+++ b/python/az/aro/azext_aro/custom.py
@@ -74,7 +74,11 @@ def aro_create(cmd,  # pylint: disable=too-many-locals
     if not client_sp:
         client_sp = aad.create_service_principal(client_id)
 
-    rp_client_id = os.environ.get('AZURE_FP_CLIENT_ID', FP_CLIENT_ID)
+    if rp_mode_production():
+        rp_client_id = FP_CLIENT_ID
+    else:
+        rp_client_id = os.environ.get('AZURE_FP_CLIENT_ID', FP_CLIENT_ID)
+
     rp_client_sp = aad.get_service_principal(rp_client_id)
     if not rp_client_sp:
         raise CLIError("RP service principal not found.")
@@ -156,7 +160,11 @@ def aro_delete(cmd, client, resource_group_name, resource_name, no_wait=False):
         logger.info(e.message)
 
     aad = AADManager(cmd.cli_ctx)
-    rp_client_id = os.environ.get('AZURE_FP_CLIENT_ID', FP_CLIENT_ID)
+
+    if rp_mode_production():
+        rp_client_id = FP_CLIENT_ID
+    else:
+        rp_client_id = os.environ.get('AZURE_FP_CLIENT_ID', FP_CLIENT_ID)
 
     # Best effort - assume the role assignments on the SP exist if exception raised
     try:
@@ -217,7 +225,11 @@ def aro_update(cmd, client, resource_group_name, resource_name, no_wait=False):
         logger.info(e.message)
 
     aad = AADManager(cmd.cli_ctx)
-    rp_client_id = os.environ.get('AZURE_FP_CLIENT_ID', FP_CLIENT_ID)
+
+    if rp_mode_production():
+        rp_client_id = FP_CLIENT_ID
+    else:
+        rp_client_id = os.environ.get('AZURE_FP_CLIENT_ID', FP_CLIENT_ID)
 
     # Best effort - assume the role assignments on the SP exist if exception raised
     try:
@@ -269,6 +281,10 @@ def rp_mode_development():
     return os.environ.get('RP_MODE', '').lower() == 'development'
 
 
+def rp_mode_production():
+    return os.environ.get('RP_MODE', '') == ''
+
+
 def generate_random_id():
     random_id = (random.choice('abcdefghijklmnopqrstuvwxyz') +
                  ''.join(random.choice('abcdefghijklmnopqrstuvwxyz1234567890')


Test patch:
diff --git a/pkg/api/validate/openshiftcluster_validatedynamic_test.go b/pkg/api/validate/openshiftcluster_validatedynamic_test.go
index d90ee60ff..a5d53efe4 100644
--- a/pkg/api/validate/openshiftcluster_validatedynamic_test.go
+++ b/pkg/api/validate/openshiftcluster_validatedynamic_test.go
@@ -535,7 +535,7 @@ func TestValidateVnetPermissions(t *testing.T) {
 						nil,
 					)
 			},
-			wantErr: "400: InvalidResourceProviderPermissions: : The resource provider does not have Contributor permission on vnet '/subscriptions/0000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Microsoft.Network/virtualNetworks/testVnet'.",
+			wantErr: "400: InvalidResourceProviderPermissions: : The resource provider does not have Network Contributor permission on vnet '/subscriptions/0000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Microsoft.Network/virtualNetworks/testVnet'.",
 		},
 		{
 			name: "fail: not found",
@@ -638,7 +638,7 @@ func TestValidateRouteTablePermissionsSubnet(t *testing.T) {
 					)
 			},
 			subnet:  masterSubnet,
-			wantErr: "400: InvalidResourceProviderPermissions: : The resource provider does not have Contributor permission on route table '/subscriptions/0000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Microsoft.Network/routeTables/testRT'.",
+			wantErr: "400: InvalidResourceProviderPermissions: : The resource provider does not have Network Contributor permission on route table '/subscriptions/0000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Microsoft.Network/routeTables/testRT'.",
 		},
 		{
 			name: "fail: not found",