This is an automated email from the ASF dual-hosted git repository.
hez 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 c92cf69a6 [fix-5085]: Azure bug fixes related to external repo support
(#5448)
c92cf69a6 is described below
commit c92cf69a63589ba34cafea26d9d3687c7afb4fe4
Author: Keon Amini <[email protected]>
AuthorDate: Mon Jun 12 13:35:34 2023 -0500
[fix-5085]: Azure bug fixes related to external repo support (#5448)
* fix: Match scope ids containing "/" in scope API
The string formats of scope ids are defined by external tools.
Some of them might contain characters such as "/".
* fix: Fix failing test
This test is skip in CI because it needs a token.
* fix: Fix build collection for non-external repositories
Non-external repositories don't have a provider field because they are
fetched from a different endpoint than external repositories that doesn't
contain this information.
Set 'tfsgit' as default provider for those repos.
---------
Co-authored-by: Camille Teruel <[email protected]>
---
.../pluginhelper/api/scope_generic_helper.go | 101 +++++++++++----------
.../python/plugins/azuredevops/azuredevops/main.py | 8 +-
.../azuredevops/azuredevops/streams/builds.py | 2 +-
.../python/pydevlake/pydevlake/testing/testing.py | 1 -
.../server/services/remote/plugin/default_api.go | 3 +-
5 files changed, 60 insertions(+), 55 deletions(-)
diff --git a/backend/helpers/pluginhelper/api/scope_generic_helper.go
b/backend/helpers/pluginhelper/api/scope_generic_helper.go
index 5a5e21257..d6b045564 100644
--- a/backend/helpers/pluginhelper/api/scope_generic_helper.go
+++ b/backend/helpers/pluginhelper/api/scope_generic_helper.go
@@ -135,11 +135,11 @@ func (c *GenericScopeApiHelper[Conn, Scope, Tr])
DbHelper() ScopeDatabaseHelper[
}
func (c *GenericScopeApiHelper[Conn, Scope, Tr]) PutScopes(input
*plugin.ApiResourceInput, scopes []*Scope) ([]*ScopeRes[Scope, Tr],
errors.Error) {
- params := c.extractFromReqParam(input)
- if params.connectionId == 0 {
- return nil, errors.BadInput.New("invalid connectionId")
+ params, err := c.extractFromReqParam(input, false)
+ if err != nil {
+ return nil, err
}
- err := c.dbHelper.VerifyConnection(params.connectionId)
+ err = c.dbHelper.VerifyConnection(params.connectionId)
if err != nil {
return nil, errors.Default.Wrap(err, fmt.Sprintf("error
verifying connection for connection ID %d", params.connectionId))
}
@@ -171,14 +171,11 @@ func (c *GenericScopeApiHelper[Conn, Scope, Tr])
PutScopes(input *plugin.ApiReso
}
func (c *GenericScopeApiHelper[Conn, Scope, Tr]) UpdateScope(input
*plugin.ApiResourceInput) (*ScopeRes[Scope, Tr], errors.Error) {
- params := c.extractFromReqParam(input)
- if params.connectionId == 0 {
- return nil, errors.BadInput.New("invalid connectionId")
- }
- if len(params.scopeId) == 0 {
- return nil, errors.BadInput.New("invalid scopeId")
+ params, err := c.extractFromReqParam(input, true)
+ if err != nil {
+ return nil, err
}
- err := c.dbHelper.VerifyConnection(params.connectionId)
+ err = c.dbHelper.VerifyConnection(params.connectionId)
if err != nil {
return nil, err
}
@@ -206,11 +203,11 @@ func (c *GenericScopeApiHelper[Conn, Scope, Tr])
UpdateScope(input *plugin.ApiRe
}
func (c *GenericScopeApiHelper[Conn, Scope, Tr]) GetScopes(input
*plugin.ApiResourceInput) ([]*ScopeRes[Scope, Tr], errors.Error) {
- params := c.extractFromGetReqParam(input)
- if params.connectionId == 0 {
+ params, err := c.extractFromGetReqParam(input, false)
+ if err != nil {
return nil, errors.BadInput.New("invalid path params:
\"connectionId\" not set")
}
- err := c.dbHelper.VerifyConnection(params.connectionId)
+ err = c.dbHelper.VerifyConnection(params.connectionId)
if err != nil {
return nil, errors.Default.Wrap(err, fmt.Sprintf("error
verifying connection for connection ID %d", params.connectionId))
}
@@ -253,14 +250,11 @@ func (c *GenericScopeApiHelper[Conn, Scope, Tr])
GetScopes(input *plugin.ApiReso
}
func (c *GenericScopeApiHelper[Conn, Scope, Tr]) GetScope(input
*plugin.ApiResourceInput) (*ScopeRes[Scope, Tr], errors.Error) {
- params := c.extractFromGetReqParam(input)
- if params == nil || params.connectionId == 0 {
- return nil, errors.BadInput.New("invalid path params:
\"connectionId\" not set")
- }
- if len(params.scopeId) == 0 || params.scopeId == "0" {
- return nil, errors.BadInput.New("invalid path params:
\"scopeId\" not set/invalid")
+ params, err := c.extractFromGetReqParam(input, true)
+ if err != nil {
+ return nil, err
}
- err := c.dbHelper.VerifyConnection(params.connectionId)
+ err = c.dbHelper.VerifyConnection(params.connectionId)
if err != nil {
return nil, errors.Default.Wrap(err, fmt.Sprintf("error
verifying connection for connection ID %d", params.connectionId))
}
@@ -286,14 +280,11 @@ func (c *GenericScopeApiHelper[Conn, Scope, Tr])
GetScope(input *plugin.ApiResou
}
func (c *GenericScopeApiHelper[Conn, Scope, Tr]) DeleteScope(input
*plugin.ApiResourceInput) errors.Error {
- params := c.extractFromDeleteReqParam(input)
- if params == nil || params.connectionId == 0 {
- return errors.BadInput.New("invalid path params:
\"connectionId\" not set")
- }
- if len(params.scopeId) == 0 || params.scopeId == "0" {
- return errors.BadInput.New("invalid path params: \"scopeId\"
not set/invalid")
+ params, err := c.extractFromDeleteReqParam(input)
+ if err != nil {
+ return err
}
- err := c.dbHelper.VerifyConnection(params.connectionId)
+ err = c.dbHelper.VerifyConnection(params.connectionId)
if err != nil {
return errors.Default.Wrap(err, fmt.Sprintf("error verifying
connection for connection ID %d", params.connectionId))
}
@@ -357,56 +348,70 @@ func (c *GenericScopeApiHelper[Conn, Scope, Tr])
mapByScopeId(scopes []*ScopeRes
return scopeMap
}
-func (c *GenericScopeApiHelper[Conn, Scope, Tr]) extractFromReqParam(input
*plugin.ApiResourceInput) *requestParams {
+func (c *GenericScopeApiHelper[Conn, Scope, Tr]) extractFromReqParam(input
*plugin.ApiResourceInput, withScopeId bool) (*requestParams, errors.Error) {
connectionId, err := strconv.ParseUint(input.Params["connectionId"],
10, 64)
- if err != nil || connectionId == 0 {
- connectionId = 0
+ if err != nil {
+ return nil, errors.BadInput.Wrap(err, "Invalid
\"connectionId\"")
+ }
+ if connectionId == 0 {
+ return nil, errors.BadInput.New("\"connectionId\" cannot be 0")
+ }
+ var scopeId string
+ if withScopeId {
+ scopeId = input.Params["scopeId"]
+ // Path params that use `/*param` handlers instead of `/:param`
start with a /, so remove it
+ if scopeId[0] == '/' {
+ scopeId = scopeId[1:]
+ }
}
- scopeId := input.Params["scopeId"]
pluginName := input.Params["plugin"]
return &requestParams{
connectionId: connectionId,
- scopeId: scopeId,
plugin: pluginName,
- }
+ scopeId: scopeId,
+ }, nil
}
-func (c *GenericScopeApiHelper[Conn, Scope, Tr])
extractFromDeleteReqParam(input *plugin.ApiResourceInput) *deleteRequestParams {
- params := c.extractFromReqParam(input)
- var err errors.Error
+func (c *GenericScopeApiHelper[Conn, Scope, Tr])
extractFromDeleteReqParam(input *plugin.ApiResourceInput)
(*deleteRequestParams, errors.Error) {
+ params, err := c.extractFromReqParam(input, true)
+ if err != nil {
+ return nil, err
+ }
var deleteDataOnly bool
{
ddo, ok := input.Query["delete_data_only"]
if ok {
deleteDataOnly, err =
errors.Convert01(strconv.ParseBool(ddo[0]))
- }
- if err != nil {
- deleteDataOnly = false
+ if err != nil {
+ deleteDataOnly = false
+ }
}
}
return &deleteRequestParams{
requestParams: *params,
deleteDataOnly: deleteDataOnly,
- }
+ }, nil
}
-func (c *GenericScopeApiHelper[Conn, Scope, Tr]) extractFromGetReqParam(input
*plugin.ApiResourceInput) *getRequestParams {
- params := c.extractFromReqParam(input)
- var err errors.Error
+func (c *GenericScopeApiHelper[Conn, Scope, Tr]) extractFromGetReqParam(input
*plugin.ApiResourceInput, withScopeId bool) (*getRequestParams, errors.Error) {
+ params, err := c.extractFromReqParam(input, withScopeId)
+ if err != nil {
+ return nil, err
+ }
var loadBlueprints bool
{
lbps, ok := input.Query["blueprints"]
if ok {
loadBlueprints, err =
errors.Convert01(strconv.ParseBool(lbps[0]))
- }
- if err != nil {
- loadBlueprints = false
+ if err != nil {
+ loadBlueprints = false
+ }
}
}
return &getRequestParams{
requestParams: *params,
loadBlueprints: loadBlueprints,
- }
+ }, nil
}
func setScopeFields(p interface{}, connectionId uint64, createdDate
*time.Time, updatedDate *time.Time) {
diff --git a/backend/python/plugins/azuredevops/azuredevops/main.py
b/backend/python/plugins/azuredevops/azuredevops/main.py
index 14172ac1c..15d2593c2 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,
TestConnectionResult
+from pydevlake import Plugin, RemoteScopeGroup, DomainType,
TestConnectionResult
from pydevlake.domain_layer.code import Repo
from pydevlake.domain_layer.devops import CicdScope
from pydevlake.pipeline_tasks import gitextractor, refdiff
@@ -124,14 +124,14 @@ class AzureDevOpsPlugin(Plugin):
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():
+ if DomainType.CODE in scope_config.domain_types and not
scope.is_external():
url = urlparse(scope.remote_url)
url =
url._replace(netloc=f'{url.username}:{connection.token.get_secret_value()}@{url.hostname}')
yield gitextractor(url.geturl(), scope.domain_id(),
connection.proxy)
- def extra_stages(self, scope_config_pairs: list[ScopeConfigPair], _):
+ def extra_stages(self, scope_config_pairs: list[tuple[GitRepository,
GitRepositoryConfig]], _):
for scope, config in scope_config_pairs:
- if DomainType.CODE in config.entity_types:
+ if DomainType.CODE in config.domain_types:
if not scope.is_external():
yield [refdiff(scope.id, config.refdiff)]
diff --git a/backend/python/plugins/azuredevops/azuredevops/streams/builds.py
b/backend/python/plugins/azuredevops/azuredevops/streams/builds.py
index 46d0a9ea9..9b6deda78 100644
--- a/backend/python/plugins/azuredevops/azuredevops/streams/builds.py
+++ b/backend/python/plugins/azuredevops/azuredevops/streams/builds.py
@@ -29,7 +29,7 @@ class Builds(Stream):
def collect(self, state, context) -> Iterable[tuple[object, dict]]:
repo: GitRepository = context.scope
api = AzureDevOpsAPI(context.connection)
- response = api.builds(repo.org_id, repo.project_id, repo.id,
repo.provider)
+ response = api.builds(repo.org_id, repo.project_id, repo.id,
repo.provider or 'tfsgit')
for raw_build in response:
yield raw_build, state
diff --git a/backend/python/pydevlake/pydevlake/testing/testing.py
b/backend/python/pydevlake/pydevlake/testing/testing.py
index fb2a990a1..e31240e1c 100644
--- a/backend/python/pydevlake/pydevlake/testing/testing.py
+++ b/backend/python/pydevlake/pydevlake/testing/testing.py
@@ -172,7 +172,6 @@ def assert_valid_remote_scopes(plugin: Plugin, connection:
Connection, group_id:
def assert_valid_pipeline_plan(plugin: Plugin, connection: Connection,
tool_scope: ToolScope, scope_config: ScopeConfig) -> list[list[PipelineTask]]:
plan = plugin.make_pipeline_plan(
[(tool_scope, scope_config)],
- [domain_type.value for domain_type in DomainType],
connection
)
assert len(plan) > 0, 'Pipeline plan has no stage'
diff --git a/backend/server/services/remote/plugin/default_api.go
b/backend/server/services/remote/plugin/default_api.go
index 817a05d4f..ac100c67f 100644
--- a/backend/server/services/remote/plugin/default_api.go
+++ b/backend/server/services/remote/plugin/default_api.go
@@ -65,7 +65,8 @@ func GetDefaultAPI(
"PUT": papi.PutScope,
"GET": papi.ListScopes,
},
- "connections/:connectionId/scopes/:scopeId": {
+ // Use `*` to match scopeId with `/` in it
+ "connections/:connectionId/scopes/*scopeId": {
"GET": papi.GetScope,
"PATCH": papi.UpdateScope,
"DELETE": papi.DeleteScope,
