address pr comments

nvflare/apis/fl_constant.py

@@ -575,6 +575,6 @@ class ConnPropKey:
 
 
 class ConnectionSecurity:
-    INSECURE = "insecure"
+    CLEAR = "clear"
     TLS = "tls"
     MTLS = "mtls"

nvflare/fuel/f3/cellnet/connector_manager.py

@@ -93,7 +93,7 @@ def __init__(self, communicator: Communicator, secure: bool, comm_configurator:
         # default conn sec
         conn_sec = self.int_resources.get(DriverParams.CONNECTION_SECURITY)
         if not conn_sec:
-            self.int_resources[DriverParams.CONNECTION_SECURITY] = ConnectionSecurity.INSECURE
+            self.int_resources[DriverParams.CONNECTION_SECURITY] = ConnectionSecurity.CLEAR
 
         self.logger.debug(f"internal scheme={self.int_scheme}, resources={self.int_resources}")
         self.logger.debug(f"adhoc scheme={self.adhoc_scheme}, resources={self.adhoc_resources}")

nvflare/fuel/f3/cellnet/core_cell.py

@@ -334,7 +334,7 @@ def __init__(
         # If configured, use it; otherwise keep the original value of 'secure'.
         conn_security = credentials.get(DriverParams.CONNECTION_SECURITY.value)
         if conn_security:
-            if conn_security == ConnectionSecurity.INSECURE:
+            if conn_security == ConnectionSecurity.CLEAR:
                 secure = False
             else:
                 secure = True

nvflare/fuel/f3/comm_config_utils.py

@@ -27,7 +27,7 @@ def requires_secure_connection(resources: dict):
     conn_sec = resources.get(DriverParams.CONNECTION_SECURITY.value)
     if conn_sec:
         # if connection security is specified, it takes precedence over the "secure" flag
-        if conn_sec == ConnectionSecurity.INSECURE:
+        if conn_sec == ConnectionSecurity.CLEAR:
             return False
         else:
             return True

nvflare/fuel/utils/url_utils.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 
 _SECURE_SCHEME_MAPPING = {"tcp": "stcp", "grpc": "grpcs", "http": "https"}
+_CLEAR_SCHEME_MAPPING = {"stcp": "tcp", "grpcs": "grpc", "https": "http"}
 
 
 def make_url(scheme: str, address, secure: bool) -> str:
@@ -29,12 +30,28 @@ def make_url(scheme: str, address, secure: bool) -> str:
     Returns:
 
     """
-    secure_scheme = _SECURE_SCHEME_MAPPING.get(scheme)
-    if not secure_scheme:
-        raise ValueError(f"unsupported scheme '{scheme}'")
-
     if secure:
+        if scheme in _SECURE_SCHEME_MAPPING.values():
+            # already secure scheme
+            secure_scheme = scheme
+        else:
+            secure_scheme = _SECURE_SCHEME_MAPPING.get(scheme)
+
+        if not secure_scheme:
+            raise ValueError(f"unsupported scheme '{scheme}'")
+
         scheme = secure_scheme
+    else:
+        if scheme in _CLEAR_SCHEME_MAPPING.values():
+            # already clear scheme
+            clear_scheme = scheme
+        else:
+            clear_scheme = _CLEAR_SCHEME_MAPPING.get(scheme)
+
+        if not clear_scheme:
+            raise ValueError(f"unsupported scheme '{scheme}'")
+
+        scheme = clear_scheme
 
     if isinstance(address, str):
         if not address:

nvflare/lighter/constants.py

@@ -11,6 +11,7 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
+from nvflare.apis.fl_constant import ConnectionSecurity
 
 
 class WorkDir:
@@ -66,10 +67,9 @@ class ProvisionMode:
 
 
 class ConnSecurity:
-    CLEAR = "clear"
-    INSECURE = "insecure"
-    TLS = "tls"
-    MTLS = "mtls"
+    CLEAR = ConnectionSecurity.CLEAR
+    TLS = ConnectionSecurity.TLS
+    MTLS = ConnectionSecurity.MTLS
 
 
 class AdminRole:

nvflare/lighter/impl/static_file.py

@@ -115,7 +115,7 @@ def _build_overseer(self, overseer: Participant, ctx: ProvisionContext):
 
     @staticmethod
     def _build_conn_properties(site: Participant, ctx: ProvisionContext, site_config: dict):
-        valid_values = [ConnSecurity.CLEAR, ConnSecurity.INSECURE, ConnSecurity.TLS, ConnSecurity.MTLS]
+        valid_values = [ConnSecurity.CLEAR, ConnSecurity.TLS, ConnSecurity.MTLS]
         conn_security = site.get_prop_fb(PropKey.CONN_SECURITY)
         if conn_security:
             assert isinstance(conn_security, str)
@@ -124,8 +124,6 @@ def _build_conn_properties(site: Participant, ctx: ProvisionContext, site_config
             if conn_security not in valid_values:
                 raise ValueError(f"invalid connection_security '{conn_security}': must be in {valid_values}")
 
-            if conn_security in [ConnSecurity.CLEAR, ConnSecurity.INSECURE]:
-                conn_security = ConnSecurity.INSECURE
             site_config["connection_security"] = conn_security
 
         custom_ca_cert = site.get_prop_fb(PropKey.CUSTOM_CA_CERT)

nvflare/private/fed/app/fl_conf.py

@@ -306,7 +306,7 @@ def _determine_conn_props(self, client_name, config_data: dict):
                 addr = relay_config.get(ConnPropKey.ADDRESS)
                 relay_conn_security = relay_config.get(ConnPropKey.CONNECTION_SECURITY)
                 secure = True
-                if relay_conn_security == ConnectionSecurity.INSECURE:
+                if relay_conn_security == ConnectionSecurity.CLEAR:
                     secure = False
                 relay_url = make_url(scheme, addr, secure)
                 print(f"connect to server via relay: {relay_url=} {relay_fqcn=}")

nvflare/private/fed/app/relay/relay.py

@@ -138,7 +138,7 @@ def main(args):
     secure_conn = True
     if conn_security:
         credentials[DriverParams.CONNECTION_SECURITY.value] = conn_security
-        if conn_security == ConnectionSecurity.INSECURE:
+        if conn_security == ConnectionSecurity.CLEAR:
             secure_conn = False
     parent_url = make_url(parent_scheme, parent_address, secure_conn)
 

tests/unit_test/fuel/f3/comm_config_utils_test.py

@@ -19,7 +19,7 @@
 
 CS = DriverParams.CONNECTION_SECURITY.value
 S = DriverParams.SECURE.value
-IS = ConnectionSecurity.INSECURE
+IS = ConnectionSecurity.CLEAR
 T = ConnectionSecurity.TLS
 M = ConnectionSecurity.MTLS
 

tests/unit_test/fuel/utils/url_utils_test.py

@@ -36,6 +36,12 @@ class TestUrlUtils:
             ("tcp", ["xyz.com", 1234], False, "tcp://xyz.com:1234"),
             ("tcp", {"host": "xyz.com"}, False, "tcp://xyz.com"),
             ("tcp", {"host": "xyz.com", "port": 1234}, False, "tcp://xyz.com:1234"),
+            ("stcp", {"host": "xyz.com"}, False, "tcp://xyz.com"),
+            ("https", {"host": "xyz.com"}, False, "http://xyz.com"),
+            ("grpcs", {"host": "xyz.com"}, False, "grpc://xyz.com"),
+            ("stcp", {"host": "xyz.com"}, True, "stcp://xyz.com"),
+            ("https", {"host": "xyz.com"}, True, "https://xyz.com"),
+            ("grpcs", {"host": "xyz.com"}, True, "grpcs://xyz.com"),
         ],
     )
     def test_make_url(self, scheme, address, secure, expected):