resolve comments

gravitational · Jul 11, 2023 · a32c7b4 · a32c7b4
1 parent 5c7897b
commit a32c7b4
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 14 deletions.
diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go
@@ -4121,12 +4121,8 @@ func (a *ServerWithRoles) SetClusterNetworkingConfig(ctx context.Context, newNet
 	}
 
 	return a.authServer.SetClusterNetworkingConfig(ctx, newNetConfig)
-}
 
-func cloudTenantNetworkingError(field string) string {
-	return fmt.Sprintf("cloud tenants cannot update %q", field)
 }
-
 func (a *ServerWithRoles) validateCloudNetworkConfigUpdate(newConfig, oldConfig types.ClusterNetworkingConfig) error {
 	if a.hasBuiltinRole(types.RoleAdmin) {
 		return nil
@@ -4136,21 +4132,23 @@ func (a *ServerWithRoles) validateCloudNetworkConfigUpdate(newConfig, oldConfig
 		return nil
 	}
 
+	const cloudUpdateFailureMsg = "cloud tenants cannot update %q"
+
 	if newConfig.GetProxyListenerMode() != oldConfig.GetProxyListenerMode() {
-		return trace.BadParameter(cloudTenantNetworkingError("proxy_listener_mode"))
+		return trace.BadParameter(cloudUpdateFailureMsg, "proxy_listener_mode")
 	}
-	newtst, newerr := newConfig.GetTunnelStrategyType()
-	oldtst, olderr := oldConfig.GetTunnelStrategyType()
-	if newerr != olderr || newtst != oldtst {
-		return trace.BadParameter(cloudTenantNetworkingError("tunnel_strategy"))
+	newtst, _ := newConfig.GetTunnelStrategyType()
+	oldtst, _ := oldConfig.GetTunnelStrategyType()
+	if newtst != oldtst {
+		return trace.BadParameter(cloudUpdateFailureMsg, "tunnel_strategy")
 	}
 
 	if newConfig.GetKeepAliveInterval() != oldConfig.GetKeepAliveInterval() {
-		return trace.BadParameter(cloudTenantNetworkingError("keep_alive_interval"))
+		return trace.BadParameter(cloudUpdateFailureMsg, "keep_alive_interval")
 	}
 
 	if newConfig.GetKeepAliveCountMax() != oldConfig.GetKeepAliveCountMax() {
-		return trace.BadParameter(cloudTenantNetworkingError("keep_alive_count_max"))
+		return trace.BadParameter(cloudUpdateFailureMsg, "keep_alive_count_max")
 	}
 
 	return nil

diff --git a/lib/auth/auth_with_roles_test.go b/lib/auth/auth_with_roles_test.go
@@ -1141,7 +1141,8 @@ func TestAuthPreferenceRBAC(t *testing.T) {
 func TestClusterNetworkingCloudUpdates(t *testing.T) {
 	srv := newTestTLSServer(t)
 	ctx := context.Background()
-	srv.Auth().SetClusterNetworkingConfig(ctx, types.DefaultClusterNetworkingConfig())
+	err := srv.Auth().SetClusterNetworkingConfig(ctx, types.DefaultClusterNetworkingConfig())
+	require.NoError(t, err)
 
 	user, _, err := CreateUserAndRole(srv.Auth(), "username", []string{}, []types.Rule{
 		{
@@ -1170,11 +1171,42 @@ func TestClusterNetworkingCloudUpdates(t *testing.T) {
 			name:         "non admin user cannot set keep_alive_interval",
 			cloud:        true,
 			identity:     TestUser(user.GetName()),
-			expectSetErr: cloudTenantNetworkingError("keep_alive_interval"),
+			expectSetErr: "keep_alive_interval",
 			clusterNetworkingConfig: newClusterNetworkingConf(t, types.ClusterNetworkingConfigSpecV2{
 				KeepAliveInterval: types.Duration(time.Second * 20),
 			}),
 		},
+		{
+			name:         "non admin user cannot set tunnel_strategy",
+			cloud:        true,
+			identity:     TestUser(user.GetName()),
+			expectSetErr: "tunnel_strategy",
+			clusterNetworkingConfig: newClusterNetworkingConf(t, types.ClusterNetworkingConfigSpecV2{
+				TunnelStrategy: &types.TunnelStrategyV1{
+					Strategy: &types.TunnelStrategyV1_ProxyPeering{
+						ProxyPeering: types.DefaultProxyPeeringTunnelStrategy(),
+					},
+				},
+			}),
+		},
+		{
+			name:         "non admin user cannot set proxy_listener_mode",
+			cloud:        true,
+			identity:     TestUser(user.GetName()),
+			expectSetErr: "proxy_listener_mode",
+			clusterNetworkingConfig: newClusterNetworkingConf(t, types.ClusterNetworkingConfigSpecV2{
+				ProxyListenerMode: types.ProxyListenerMode_Multiplex,
+			}),
+		},
+		{
+			name:         "non admin user cannot set keep_alive_count_max",
+			cloud:        true,
+			identity:     TestUser(user.GetName()),
+			expectSetErr: "keep_alive_count_max",
+			clusterNetworkingConfig: newClusterNetworkingConf(t, types.ClusterNetworkingConfigSpecV2{
+				KeepAliveCountMax: 55,
+			}),
+		},
 		{
 			name:     "non admin user can set client_idle_timeout",
 			cloud:    true,
@@ -1214,7 +1246,7 @@ func TestClusterNetworkingCloudUpdates(t *testing.T) {
 			err = client.SetClusterNetworkingConfig(ctx, tc.clusterNetworkingConfig)
 			if err != nil {
 				require.NotEmpty(t, tc.expectSetErr)
-				require.ErrorContains(t, err, tc.expectSetErr)
+				require.ErrorContains(t, err, fmt.Sprintf("%q", tc.expectSetErr))
 			} else {
 				require.Empty(t, tc.expectSetErr)
 			}