From 41178ffd1027c9ce5dae5f81ff390e9a611c6d23 Mon Sep 17 00:00:00 2001 From: aviat cohen Date: Tue, 6 Jan 2026 13:23:06 +0000 Subject: [PATCH 1/3] =?UTF-8?q?Set=20the=20creation=E2=80=91method=20param?= =?UTF-8?q?eters=20to=20be=20optional?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../unreleased/fixed-20260106-132043.yaml | 3 + src/fabric_cli/utils/fab_cmd_mkdir_utils.py | 82 ++++++----- tests/test_utils/test_fab_cmd_mkdir_utils.py | 137 +++++++++++++++++- 3 files changed, 187 insertions(+), 35 deletions(-) create mode 100644 .changes/unreleased/fixed-20260106-132043.yaml diff --git a/.changes/unreleased/fixed-20260106-132043.yaml b/.changes/unreleased/fixed-20260106-132043.yaml new file mode 100644 index 000000000..685948d82 --- /dev/null +++ b/.changes/unreleased/fixed-20260106-132043.yaml @@ -0,0 +1,3 @@ +kind: fixed +body: Set the creation‑method parameters to be optional +time: 2026-01-06T13:20:43.213068962Z diff --git a/src/fabric_cli/utils/fab_cmd_mkdir_utils.py b/src/fabric_cli/utils/fab_cmd_mkdir_utils.py index 97c06de90..1796a4b3a 100644 --- a/src/fabric_cli/utils/fab_cmd_mkdir_utils.py +++ b/src/fabric_cli/utils/fab_cmd_mkdir_utils.py @@ -545,36 +545,39 @@ def get_connection_config_from_params(payload, con_type, con_type_def, params): parsed_params = [] missing_params = [] if not provided_params: - # Get required and optional parameters from the creation method - req_params_str = ", ".join( - [p["name"] for p in creation_method["parameters"] if p["required"]] - ) - opt_params_str = ", ".join( - [p["name"] for p in creation_method["parameters"] if not p["required"]] - ) - raise FabricCLIError( - f"Parameters are required for the connection creation method. Required parameters are: {req_params_str}. Optional parameters are: {opt_params_str}", - fab_constant.ERROR_INVALID_INPUT, - ) - for param in creation_method["parameters"]: - p_name = param["name"] - if p_name.lower() not in provided_params and param["required"]: - c_method = creation_method["name"] - missing_params.append(p_name) - if p_name.lower() in provided_params: - parsed_params.append( - { - "dataType": param["dataType"], - "name": p_name, - "value": provided_params[p_name.lower()], - } + # Check if the creation method actually requires parameters + required_params = [p["name"] for p in creation_method["parameters"] if p["required"]] + if required_params: + # Get required and optional parameters from the creation method + req_params_str = ", ".join(required_params) + opt_params_str = ", ".join( + [p["name"] for p in creation_method["parameters"] if not p["required"]] ) - for param in provided_params: - if param not in [p["name"].lower() for p in creation_method["parameters"]]: - c_method = creation_method["name"] - utils_ui.print_warning( - f"Parameter {param} is not used by the creation method {c_method} and will be ignored" + raise FabricCLIError( + f"Parameters are required for the connection creation method. Required parameters are: {req_params_str}. Optional parameters are: {opt_params_str}", + fab_constant.ERROR_INVALID_INPUT, ) + # If no required parameters, continue with empty parsed_params + else: + for param in creation_method["parameters"]: + p_name = param["name"] + if p_name.lower() not in provided_params and param["required"]: + c_method = creation_method["name"] + missing_params.append(p_name) + if p_name.lower() in provided_params: + parsed_params.append( + { + "dataType": param["dataType"], + "name": p_name, + "value": provided_params[p_name.lower()], + } + ) + for param in provided_params: + if param not in [p["name"].lower() for p in creation_method["parameters"]]: + c_method = creation_method["name"] + utils_ui.print_warning( + f"Parameter {param} is not used by the creation method {c_method} and will be ignored" + ) if missing_params: missing_params_str = ", ".join(missing_params) @@ -586,8 +589,11 @@ def get_connection_config_from_params(payload, con_type, con_type_def, params): connection_request["connectionDetails"] = { "type": con_type, "creationMethod": creation_method["name"], - "parameters": parsed_params, } + + # Only add parameters if there are any + if parsed_params: + connection_request["connectionDetails"]["parameters"] = parsed_params """ Check that the provided credential type is supported by the connection type: @@ -657,12 +663,20 @@ def get_connection_config_from_params(payload, con_type, con_type_def, params): "singleSignOnType": singleSignOnType, "connectionEncryption": encryption, "skipTestConnection": skipTestConnection, - "credentials": connection_params, } - connection_request["credentialDetails"]["credentials"]["credentialType"] = cred_type - - if is_on_premises_gateway: - connection_request["credentialDetails"]["credentials"]["values"] = connection_params.get("values") + + # Only add credentials if there are any parameters or it's not a type that requires no credentials + if connection_params or cred_type not in ["Anonymous", "WindowsWithoutImpersonation", "WorkspaceIdentity"]: + connection_request["credentialDetails"]["credentials"] = connection_params + connection_request["credentialDetails"]["credentials"]["credentialType"] = cred_type + + if is_on_premises_gateway: + connection_request["credentialDetails"]["credentials"]["values"] = connection_params.get("values") + else: + # For credential types that don't need parameters, just add the credentialType + connection_request["credentialDetails"]["credentials"] = { + "credentialType": cred_type + } return connection_request diff --git a/tests/test_utils/test_fab_cmd_mkdir_utils.py b/tests/test_utils/test_fab_cmd_mkdir_utils.py index b0fa180a9..987d71e3a 100644 --- a/tests/test_utils/test_fab_cmd_mkdir_utils.py +++ b/tests/test_utils/test_fab_cmd_mkdir_utils.py @@ -9,7 +9,142 @@ from fabric_cli.core import fab_constant from fabric_cli.core.fab_exceptions import FabricCLIError from fabric_cli.errors import ErrorMessages -from fabric_cli.utils.fab_cmd_mkdir_utils import find_mpe_connection +from fabric_cli.utils.fab_cmd_mkdir_utils import ( + find_mpe_connection, + get_connection_config_from_params, +) + + +def test_fabric_data_pipelines_workspace_identity_no_params_success(): + """Test FabricDataPipelines with WorkspaceIdentity credential type when no parameters are required.""" + # Arrange + payload = { + "description": "Created by fab", + "displayName": "test-connection", + "connectivityType": "ShareableCloud" + } + + con_type = "FabricDataPipelines" + con_type_def = { + "type": "FabricDataPipelines", + "creationMethods": [ + { + "name": "FabricDataPipelines.Actions", + "parameters": [] # No parameters required for this creation method + } + ], + "supportedCredentialTypes": ["WorkspaceIdentity"] + } + + params = { + "connectiondetails": { + "type": "FabricDataPipelines", + "creationmethod": "FabricDataPipelines.Actions" + # No parameters provided since none are required + }, + "credentialdetails": { + "type": "WorkspaceIdentity" + # No credential parameters provided since WorkspaceIdentity doesn't require any + } + } + + result = get_connection_config_from_params(payload, con_type, con_type_def, params) + + # Assert + assert result["privacyLevel"] == "None" + assert result["connectionDetails"]["type"] == "FabricDataPipelines" + assert result["connectionDetails"]["creationMethod"] == "FabricDataPipelines.Actions" + assert "parameters" not in result["connectionDetails"] + assert result["credentialDetails"]["credentials"]["credentialType"] == "WorkspaceIdentity" + assert len(result["credentialDetails"]["credentials"].keys()) == 1 + + +def test_connection_with_required_params_missing_failure(): + """Test that connection creation fails when required parameters are missing.""" + # Arrange + payload = { + "description": "Created by fab", + "displayName": "test-connection", + "connectivityType": "ShareableCloud" + } + + con_type = "SQL" + con_type_def = { + "type": "SQL", + "creationMethods": [ + { + "name": "SQL", + "parameters": [ + {"name": "server", "required": True, "dataType": "Text"}, + {"name": "database", "required": True, "dataType": "Text"} + ] + } + ], + "supportedCredentialTypes": ["Basic"] + } + + params = { + "connectiondetails": { + "type": "SQL", + "creationmethod": "SQL" + # No parameters provided, but they are required + }, + "credentialdetails": { + "type": "Basic", + "username": "testuser", + "password": "testpass" + } + } + + with pytest.raises(FabricCLIError) as exc_info: + get_connection_config_from_params(payload, con_type, con_type_def, params) + + assert "Parameters are required for the connection creation method" in str(exc_info.value.message) + assert "server, database" in str(exc_info.value.message) + + +def test_workspace_identity_with_unsupported_params_ignored_success(): + """Test that WorkspaceIdentity ignores unsupported credential parameters with warning.""" + # Arrange + payload = { + "description": "Created by fab", + "displayName": "test-connection", + "connectivityType": "ShareableCloud" + } + + con_type = "FabricDataPipelines" + con_type_def = { + "type": "FabricDataPipelines", + "creationMethods": [ + { + "name": "FabricDataPipelines.Actions", + "parameters": [] + } + ], + "supportedCredentialTypes": ["WorkspaceIdentity"] + } + + params = { + "connectiondetails": { + "type": "FabricDataPipelines", + "creationmethod": "FabricDataPipelines.Actions" + }, + "credentialdetails": { + "type": "WorkspaceIdentity", + "username": "should_be_ignored", # This should be ignored for WorkspaceIdentity + "password": "should_be_ignored" # This should be ignored for WorkspaceIdentity + } + } + + # Act + with patch('fabric_cli.utils.fab_ui.print_warning') as mock_warning: + result = get_connection_config_from_params(payload, con_type, con_type_def, params) + + mock_warning.assert_called_once() + assert "username" in str(mock_warning.call_args) + assert "password" in str(mock_warning.call_args) + + assert result["credentialDetails"]["credentials"]["credentialType"] == "WorkspaceIdentity" class TestFindMpeConnection: From c925b2792e3d4c782565a10cff805e4ec1c6aec4 Mon Sep 17 00:00:00 2001 From: Aviat Cohen Date: Sun, 18 Jan 2026 09:46:30 +0000 Subject: [PATCH 2/3] update changie log --- .changes/unreleased/fixed-20260106-132043.yaml | 3 --- .changes/unreleased/fixed-20260118-094510.yaml | 6 ++++++ 2 files changed, 6 insertions(+), 3 deletions(-) delete mode 100644 .changes/unreleased/fixed-20260106-132043.yaml create mode 100644 .changes/unreleased/fixed-20260118-094510.yaml diff --git a/.changes/unreleased/fixed-20260106-132043.yaml b/.changes/unreleased/fixed-20260106-132043.yaml deleted file mode 100644 index 685948d82..000000000 --- a/.changes/unreleased/fixed-20260106-132043.yaml +++ /dev/null @@ -1,3 +0,0 @@ -kind: fixed -body: Set the creation‑method parameters to be optional -time: 2026-01-06T13:20:43.213068962Z diff --git a/.changes/unreleased/fixed-20260118-094510.yaml b/.changes/unreleased/fixed-20260118-094510.yaml new file mode 100644 index 000000000..ff201ee26 --- /dev/null +++ b/.changes/unreleased/fixed-20260118-094510.yaml @@ -0,0 +1,6 @@ +kind: fixed +body: Set the creation‑method parameters to be optional +time: 2026-01-18T09:45:10.759002358Z +custom: + Author: aviatco + AuthorLink: https://github.com/aviatco From c7f9e73419f870298caf52963560669cfc756265 Mon Sep 17 00:00:00 2001 From: Aviat Cohen Date: Mon, 19 Jan 2026 09:55:13 +0000 Subject: [PATCH 3/3] fix pr comments --- src/fabric_cli/utils/fab_cmd_mkdir_utils.py | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/fabric_cli/utils/fab_cmd_mkdir_utils.py b/src/fabric_cli/utils/fab_cmd_mkdir_utils.py index 1796a4b3a..e75ecd2ed 100644 --- a/src/fabric_cli/utils/fab_cmd_mkdir_utils.py +++ b/src/fabric_cli/utils/fab_cmd_mkdir_utils.py @@ -663,20 +663,14 @@ def get_connection_config_from_params(payload, con_type, con_type_def, params): "singleSignOnType": singleSignOnType, "connectionEncryption": encryption, "skipTestConnection": skipTestConnection, + "credentials": connection_params, } - - # Only add credentials if there are any parameters or it's not a type that requires no credentials - if connection_params or cred_type not in ["Anonymous", "WindowsWithoutImpersonation", "WorkspaceIdentity"]: - connection_request["credentialDetails"]["credentials"] = connection_params - connection_request["credentialDetails"]["credentials"]["credentialType"] = cred_type + + connection_request["credentialDetails"]["credentials"]["credentialType"] = cred_type - if is_on_premises_gateway: - connection_request["credentialDetails"]["credentials"]["values"] = connection_params.get("values") - else: - # For credential types that don't need parameters, just add the credentialType - connection_request["credentialDetails"]["credentials"] = { - "credentialType": cred_type - } + if is_on_premises_gateway: + connection_request["credentialDetails"]["credentials"]["values"] = connection_params.get( + "values") return connection_request