This is an automated email from the ASF dual-hosted git repository.
ka94 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-devlake.git
The following commit(s) were added to refs/heads/main by this push:
new 8762ae4f8 5080 Let test_connection report status from tool API (#5422)
8762ae4f8 is described below
commit 8762ae4f823131fd93944df9fcb27ae99e0cf322
Author: Camille Teruel <[email protected]>
AuthorDate: Fri Jun 9 21:00:32 2023 +0200
5080 Let test_connection report status from tool API (#5422)
* fix: AzureDevOps API returns 203 instead of 401
When token is invalid, AzureDevOps API recently started returning 302s that
redirect to a sign-in page
with a status 203. Add a response hook to transform those 203 into 401.
* feat: Let test_connection report HTTP status code
Change test_connection to return a TestConnectionResult that contains a
message and a status code
that are used on go-side to build responses to GET
'plugins/<plugin>/test-connection'.
---------
Co-authored-by: Camille Teruel <[email protected]>
---
backend/python/README.md | 16 +++++-------
.../python/plugins/azuredevops/azuredevops/api.py | 10 +++++++-
.../python/plugins/azuredevops/azuredevops/main.py | 30 ++++++++++++----------
backend/python/pydevlake/pydevlake/__init__.py | 2 +-
backend/python/pydevlake/pydevlake/ipc.py | 4 +--
backend/python/pydevlake/pydevlake/message.py | 18 +++++++++++++
backend/python/pydevlake/pydevlake/plugin.py | 3 ++-
backend/python/test/fakeplugin/fakeplugin/main.py | 13 ++++++++--
.../services/remote/plugin/connection_api.go | 21 +++++++++++----
9 files changed, 82 insertions(+), 35 deletions(-)
diff --git a/backend/python/README.md b/backend/python/README.md
index a93056b50..317d5dc73 100644
--- a/backend/python/README.md
+++ b/backend/python/README.md
@@ -57,7 +57,7 @@ class MyPlugin(dl.Plugin):
def remote_scopes(self, connection, group_id: str) ->
Iterable[MyPluginToolScope]:
...
- def test_connection(self, connection: MyPluginConnection):
+ def test_connection(self, connection: MyPluginConnection) ->
dl.TestConnectionResult:
...
@@ -191,19 +191,17 @@ class MyPlugin(dl.Plugin):
The `test_connection` method is used to test if a given connection is valid.
It should check that the connection credentials are valid.
-If the connection is not valid, it should raise an exception.
+It should make an authenticated request to the API and return a
`TestConnectionResult`.
+There is a convenience static method `from_api_response` to create a
`TestConnectionResult` object from an API response.
```python
class MyPlugin(dl.Plugin):
...
- def test_connection(self, connection: MyPluginConnection):
- api = ...
- response = ...
- if response.status != 401:
- raise Exception("Invalid credentials")
- if response.status != 200:
- raise Exception(f"Connection error {response}")
+ def test_connection(self, connection: MyPluginConnection) ->
dl.TestConnectionResult:
+ api = ... # Create API client
+ response = ... # Make authenticated request to API
+ return dl.TestConnection.from_api_response(response)
```
diff --git a/backend/python/plugins/azuredevops/azuredevops/api.py
b/backend/python/plugins/azuredevops/azuredevops/api.py
index f187f4125..0ceb0b5f4 100644
--- a/backend/python/plugins/azuredevops/azuredevops/api.py
+++ b/backend/python/plugins/azuredevops/azuredevops/api.py
@@ -16,7 +16,7 @@
from typing import Optional
import base64
-from pydevlake.api import API, request_hook, Paginator, Request
+from pydevlake.api import API, APIException, Paginator, Request, Response,
request_hook, response_hook
class AzurePaginator(Paginator):
@@ -43,6 +43,14 @@ class AzureDevOpsAPI(API):
def set_api_version(self, request: Request):
request.query_args['api-version'] = "7.0"
+ @response_hook
+ def change_203_to_401(self, response: Response):
+ # When the token is invalid, Azure DevOps returns a 302 that resolves
to a sign-in page with status 203
+ # We want to change that to a 401 and raise an exception
+ if response.status == 203:
+ response.status = 401
+ raise APIException(response)
+
def my_profile(self):
req =
Request('https://app.vssps.visualstudio.com/_apis/profile/profiles/me')
return self.send(req)
diff --git a/backend/python/plugins/azuredevops/azuredevops/main.py
b/backend/python/plugins/azuredevops/azuredevops/main.py
index cc5f28e2c..14172ac1c 100644
--- a/backend/python/plugins/azuredevops/azuredevops/main.py
+++ b/backend/python/plugins/azuredevops/azuredevops/main.py
@@ -22,7 +22,7 @@ from azuredevops.streams.jobs import Jobs
from azuredevops.streams.pull_request_commits import GitPullRequestCommits
from azuredevops.streams.pull_requests import GitPullRequests
-from pydevlake import Plugin, RemoteScopeGroup, DomainType, ScopeConfigPair
+from pydevlake import Plugin, RemoteScopeGroup, DomainType, ScopeConfigPair,
TestConnectionResult
from pydevlake.domain_layer.code import Repo
from pydevlake.domain_layer.devops import CicdScope
from pydevlake.pipeline_tasks import gitextractor, refdiff
@@ -106,20 +106,22 @@ class AzureDevOpsPlugin(Plugin):
defaultBranch=props.get('defaultBranch', 'main')
)
- def test_connection(self, connection: AzureDevOpsConnection):
+ def test_connection(self, connection: AzureDevOpsConnection) ->
TestConnectionResult:
api = AzureDevOpsAPI(connection)
- if connection.organization is None:
- try:
- api.my_profile()
- except APIException as e:
- if e.response.status == 401:
- raise Exception(f"Invalid token {e}. You may need to set
organization name in connection or edit your token to set organization to 'All
accessible organizations'")
- raise
- else:
- try:
- api.projects(connection.organization)
- except APIException as e:
- raise Exception(f"Invalid token: {e}")
+ message = None
+ hint = None
+ try:
+ if connection.organization is None:
+ hint = "You may need to edit your token to set organization to
'All accessible organizations"
+ res = api.my_profile()
+ else:
+ hint = "Organization name may be incorrect or your token may
not have access to the organization."
+ res = api.projects(connection.organization)
+ except APIException as e:
+ res = e.response
+ if res.status == 401:
+ message = f"Invalid token. {hint}"
+ return TestConnectionResult.from_api_response(res, message)
def extra_tasks(self, scope: GitRepository, scope_config:
GitRepositoryConfig, connection: AzureDevOpsConnection):
if DomainType.CODE in scope_config.entity_types and not
scope.is_external():
diff --git a/backend/python/pydevlake/pydevlake/__init__.py
b/backend/python/pydevlake/pydevlake/__init__.py
index aff946fce..5adc1ba9e 100644
--- a/backend/python/pydevlake/pydevlake/__init__.py
+++ b/backend/python/pydevlake/pydevlake/__init__.py
@@ -35,7 +35,7 @@ def Field(*args, primary_key: bool=False, source:
Optional[str]=None, **kwargs):
from .model import ToolModel, ToolScope, DomainScope, Connection, ScopeConfig,
DomainType, domain_id
from .logger import logger
-from .message import RemoteScopeGroup
+from .message import RemoteScopeGroup, TestConnectionResult
from .plugin import Plugin, ScopeConfigPair
from .stream import Stream, Substream
from .context import Context
diff --git a/backend/python/pydevlake/pydevlake/ipc.py
b/backend/python/pydevlake/pydevlake/ipc.py
index 7a9fd237f..1db11d88f 100644
--- a/backend/python/pydevlake/pydevlake/ipc.py
+++ b/backend/python/pydevlake/pydevlake/ipc.py
@@ -26,7 +26,7 @@ from sqlalchemy.engine import Engine
from pydevlake.context import Context
from pydevlake.message import Message
-from pydevlake.model import DomainType, SubtaskRun
+from pydevlake.model import SubtaskRun
def plugin_method(func):
@@ -85,7 +85,7 @@ class PluginCommands:
if "name" not in connection:
connection["name"] = "Test connection"
connection = self._plugin.connection_type(**connection)
- self._plugin.test_connection(connection)
+ return self._plugin.test_connection(connection)
@plugin_method
def make_pipeline(self, scope_config_pairs: list[tuple[dict, dict]],
connection: dict):
diff --git a/backend/python/pydevlake/pydevlake/message.py
b/backend/python/pydevlake/pydevlake/message.py
index 54e078235..20d320439 100644
--- a/backend/python/pydevlake/pydevlake/message.py
+++ b/backend/python/pydevlake/pydevlake/message.py
@@ -21,6 +21,7 @@ import jsonref
from pydevlake.model import ToolScope
from pydevlake.migration import MigrationScript
+from pydevlake.api import Response
class Message(BaseModel):
@@ -111,3 +112,20 @@ class RemoteScope(RemoteScopeTreeNode):
class RemoteScopes(Message):
__root__: list[RemoteScopeTreeNode]
+
+
+class TestConnectionResult(Message):
+ success: bool
+ message: str
+ status: int
+
+ @staticmethod
+ def from_api_response(response: Response, message: str = None):
+ success = response.status == 200
+ if not message:
+ message = "Connection successful" if success else "Connection
failed"
+ return TestConnectionResult(
+ success=success,
+ message=message,
+ status=response.status
+ )
diff --git a/backend/python/pydevlake/pydevlake/plugin.py
b/backend/python/pydevlake/pydevlake/plugin.py
index 3bcfd2dda..253abacd3 100644
--- a/backend/python/pydevlake/pydevlake/plugin.py
+++ b/backend/python/pydevlake/pydevlake/plugin.py
@@ -22,6 +22,7 @@ import sys
import fire
import pydevlake.message as msg
+from pydevlake.api import APIException
from pydevlake.subtasks import Subtask
from pydevlake.logger import logger
from pydevlake.ipc import PluginCommands
@@ -68,7 +69,7 @@ class Plugin(ABC):
return None
@abstractmethod
- def test_connection(self, connection: Connection):
+ def test_connection(self, connection: Connection) ->
msg.TestConnectionResult:
"""
Test if the the connection with the datasource can be established with
the given connection.
Must raise an exception if the connection can't be established.
diff --git a/backend/python/test/fakeplugin/fakeplugin/main.py
b/backend/python/test/fakeplugin/fakeplugin/main.py
index 0c2461f71..ad3e625a2 100644
--- a/backend/python/test/fakeplugin/fakeplugin/main.py
+++ b/backend/python/test/fakeplugin/fakeplugin/main.py
@@ -20,7 +20,7 @@ import json
from pydantic import SecretStr
-from pydevlake import Plugin, Connection, Stream, ToolModel, ToolScope,
ScopeConfig, RemoteScopeGroup, DomainType, Field
+from pydevlake import Plugin, Connection, Stream, ToolModel, ToolScope,
ScopeConfig, RemoteScopeGroup, DomainType, Field, TestConnectionResult
from pydevlake.domain_layer.devops import CicdScope, CICDPipeline, CICDStatus,
CICDResult, CICDType
VALID_TOKEN = "this_is_a_valid_token"
@@ -147,7 +147,16 @@ class FakePlugin(Plugin):
def test_connection(self, connection: FakeConnection):
if connection.token.get_secret_value() != VALID_TOKEN:
- raise Exception("Invalid token")
+ return TestConnectionResult(
+ success=False,
+ message="Invalid token",
+ status=401
+ )
+ return TestConnectionResult(
+ success=True,
+ message="Connection successful",
+ status=200
+ )
@property
def streams(self):
diff --git a/backend/server/services/remote/plugin/connection_api.go
b/backend/server/services/remote/plugin/connection_api.go
index 81b49f5c7..93cdc8751 100644
--- a/backend/server/services/remote/plugin/connection_api.go
+++ b/backend/server/services/remote/plugin/connection_api.go
@@ -18,6 +18,7 @@ limitations under the License.
package plugin
import (
+ "fmt"
"net/http"
"github.com/apache/incubator-devlake/core/errors"
@@ -26,17 +27,27 @@ import (
"github.com/apache/incubator-devlake/server/services/remote/bridge"
)
+type TestConnectionResult struct {
+ Success bool `json:"success"`
+ Message string `json:"message"`
+ Status int `json:"status"`
+}
+
func (pa *pluginAPI) TestConnection(input *plugin.ApiResourceInput)
(*plugin.ApiResourceOutput, errors.Error) {
- err := pa.invoker.Call("test-connection", bridge.DefaultContext,
input.Body).Err
+ var result TestConnectionResult
+ err := pa.invoker.Call("test-connection", bridge.DefaultContext,
input.Body).Get(&result)
if err != nil {
body := shared.ApiBody{
Success: false,
- Message: err.Error(),
+ Message: fmt.Sprintf("Error while testing connection:
%s", err.Error()),
}
- return &plugin.ApiResourceOutput{Body: body, Status: 400}, nil
+ return &plugin.ApiResourceOutput{Body: body, Status: 500}, nil
} else {
- body := shared.ApiBody{Success: true}
- return &plugin.ApiResourceOutput{Body: body, Status: 200}, nil
+ body := shared.ApiBody{
+ Success: result.Success,
+ Message: result.Message,
+ }
+ return &plugin.ApiResourceOutput{Body: body, Status:
result.Status}, nil
}
}
