From 4ff897dcecdc399a64652d622fdee1bd88b958da Mon Sep 17 00:00:00 2001 From: actiontech-zihan Date: Mon, 10 Aug 2026 22:31:47 +0800 Subject: [PATCH 1/2] feat: allow empty DB service password on add/update Align Add/Update validation with connectable empty-password GBase-8a DSNs; keep other required fields and add unit coverage. Co-authored-by: Cursor --- api/dms/service/v1/db_service.go | 4 +- .../service/v1/db_service_password_test.go | 73 +++++++ api/dms/service/v2/db_service.go | 4 +- .../service/v2/db_service_password_test.go | 73 +++++++ internal/dms/biz/db_service.go | 4 +- internal/dms/biz/db_service_password_test.go | 184 ++++++++++++++++++ 6 files changed, 335 insertions(+), 7 deletions(-) create mode 100644 api/dms/service/v1/db_service_password_test.go create mode 100644 api/dms/service/v2/db_service_password_test.go create mode 100644 internal/dms/biz/db_service_password_test.go diff --git a/api/dms/service/v1/db_service.go b/api/dms/service/v1/db_service.go index 2064029b..b2a0ad40 100644 --- a/api/dms/service/v1/db_service.go +++ b/api/dms/service/v1/db_service.go @@ -28,8 +28,8 @@ type DBService struct { // Required: true User string `json:"user"` // DB Service admin password - // Required: true - Password string `json:"password" validate:"required"` + // Required: false — empty string allowed (e.g. GBase-8a) + Password string `json:"password"` // DB Service business name // Required: true // Deprecated: the business field is replaced with the environmentTag of the v2 interface. diff --git a/api/dms/service/v1/db_service_password_test.go b/api/dms/service/v1/db_service_password_test.go new file mode 100644 index 00000000..2a1b5e6e --- /dev/null +++ b/api/dms/service/v1/db_service_password_test.go @@ -0,0 +1,73 @@ +package v1 + +import ( + "strings" + "testing" + + utilConf "github.com/actiontech/dms/pkg/dms-common/pkg/config" +) + +func TestAddDBServiceReq_EmptyPasswordAllowed(t *testing.T) { + t.Parallel() + + base := func(password string) *AddDBServiceReq { + return &AddDBServiceReq{ + ProjectUid: "700300", + DBService: &DBService{ + Name: "gbase8a_empty_pwd", + DBType: "GBase-8a", + Host: "10.186.16.126", + Port: "5258", + User: "root", + Password: password, + Business: "default", + MaintenanceTimes: nil, + }, + } + } + + t.Run("password_empty_string", func(t *testing.T) { + t.Parallel() + if err := utilConf.Validate(base("")); err != nil { + t.Fatalf("expected empty password to pass Add validation, got: %v", err) + } + }) + + t.Run("password_omitted_zero_value", func(t *testing.T) { + t.Parallel() + req := base("") + req.DBService.Password = "" // JSON omit binds to zero value + if err := utilConf.Validate(req); err != nil { + t.Fatalf("expected omitted/zero password to pass Add validation, got: %v", err) + } + }) +} + +func TestAddDBServiceReq_MissingHostStillRequired(t *testing.T) { + t.Parallel() + + req := &AddDBServiceReq{ + ProjectUid: "700300", + DBService: &DBService{ + Name: "gbase8a_missing_host", + DBType: "GBase-8a", + Host: "", + Port: "5258", + User: "root", + Password: "", + Business: "default", + }, + } + + err := utilConf.Validate(req) + if err == nil { + t.Fatal("expected missing Host to fail validation") + } + msg := err.Error() + if !strings.Contains(msg, "Host") || !strings.Contains(msg, "required") { + t.Fatalf("expected Host required validation error, got: %v", err) + } + if strings.Contains(strings.ToLower(msg), "password") { + t.Fatalf("Host failure must not be attributed to password: %v", err) + } +} diff --git a/api/dms/service/v2/db_service.go b/api/dms/service/v2/db_service.go index 1bc4788b..b1fd1073 100644 --- a/api/dms/service/v2/db_service.go +++ b/api/dms/service/v2/db_service.go @@ -108,8 +108,8 @@ type DBService struct { // Required: true User string `json:"user"` // DB Service admin password - // Required: true - Password string `json:"password" validate:"required"` + // Required: false — empty string allowed (e.g. GBase-8a) + Password string `json:"password"` // DB Service environment tag // Required: true EnvironmentTagUID string `json:"environment_tag_uid" validate:"required"` diff --git a/api/dms/service/v2/db_service_password_test.go b/api/dms/service/v2/db_service_password_test.go new file mode 100644 index 00000000..e397f640 --- /dev/null +++ b/api/dms/service/v2/db_service_password_test.go @@ -0,0 +1,73 @@ +package v2 + +import ( + "strings" + "testing" + + utilConf "github.com/actiontech/dms/pkg/dms-common/pkg/config" +) + +func TestAddDBServiceReq_EmptyPasswordAllowed(t *testing.T) { + t.Parallel() + + base := func(password string) *AddDBServiceReq { + return &AddDBServiceReq{ + ProjectUid: "700300", + DBService: &DBService{ + Name: "gbase8a_empty_pwd", + DBType: "GBase-8a", + Host: "10.186.16.126", + Port: "5258", + User: "root", + Password: password, + EnvironmentTagUID: "2086752861772845056", + MaintenanceTimes: nil, + }, + } + } + + t.Run("password_empty_string", func(t *testing.T) { + t.Parallel() + if err := utilConf.Validate(base("")); err != nil { + t.Fatalf("expected empty password to pass Add validation, got: %v", err) + } + }) + + t.Run("password_omitted_zero_value", func(t *testing.T) { + t.Parallel() + req := base("") + req.DBService.Password = "" // JSON omit binds to zero value + if err := utilConf.Validate(req); err != nil { + t.Fatalf("expected omitted/zero password to pass Add validation, got: %v", err) + } + }) +} + +func TestAddDBServiceReq_MissingHostStillRequired(t *testing.T) { + t.Parallel() + + req := &AddDBServiceReq{ + ProjectUid: "700300", + DBService: &DBService{ + Name: "gbase8a_missing_host", + DBType: "GBase-8a", + Host: "", + Port: "5258", + User: "root", + Password: "", + EnvironmentTagUID: "2086752861772845056", + }, + } + + err := utilConf.Validate(req) + if err == nil { + t.Fatal("expected missing Host to fail validation") + } + msg := err.Error() + if !strings.Contains(msg, "Host") || !strings.Contains(msg, "required") { + t.Fatalf("expected Host required validation error, got: %v", err) + } + if strings.Contains(strings.ToLower(msg), "password") { + t.Fatalf("Host failure must not be attributed to password: %v", err) + } +} diff --git a/internal/dms/biz/db_service.go b/internal/dms/biz/db_service.go index f626d324..44c91911 100644 --- a/internal/dms/biz/db_service.go +++ b/internal/dms/biz/db_service.go @@ -743,9 +743,7 @@ func (d *DBServiceUsecase) UpdateDBServiceByArgs(ctx context.Context, dbServiceU ds.Desc = *updateDBService.Desc } if updateDBService.Password != nil { - if *updateDBService.Password == "" { - return fmt.Errorf("password can't be empty") - } + // empty string allowed (align with Add API; e.g. GBase-8a) ds.Password = *updateDBService.Password } diff --git a/internal/dms/biz/db_service_password_test.go b/internal/dms/biz/db_service_password_test.go new file mode 100644 index 00000000..257db255 --- /dev/null +++ b/internal/dms/biz/db_service_password_test.go @@ -0,0 +1,184 @@ +package biz + +import ( + "context" + "io" + "strings" + "testing" + + pkgConst "github.com/actiontech/dms/internal/dms/pkg/constant" + utilLog "github.com/actiontech/dms/pkg/dms-common/pkg/log" +) + +type fakeDBServiceRepoForPassword struct { + svc *DBService + updated *DBService +} + +func (f *fakeDBServiceRepoForPassword) SaveDBServices(context.Context, []*DBService) error { + return nil +} +func (f *fakeDBServiceRepoForPassword) GetDBServicesByIds(context.Context, []string) ([]*DBService, error) { + return nil, nil +} +func (f *fakeDBServiceRepoForPassword) ListDBServices(context.Context, *ListDBServicesOption) ([]*DBService, int64, error) { + return nil, 0, nil +} +func (f *fakeDBServiceRepoForPassword) DelDBService(context.Context, string) error { return nil } +func (f *fakeDBServiceRepoForPassword) GetDBService(_ context.Context, _ string) (*DBService, error) { + return f.svc, nil +} +func (f *fakeDBServiceRepoForPassword) GetDBServices(context.Context, []pkgConst.FilterCondition) ([]*DBService, error) { + return nil, nil +} +func (f *fakeDBServiceRepoForPassword) CheckDBServiceExist(context.Context, []string) (bool, error) { + return true, nil +} +func (f *fakeDBServiceRepoForPassword) UpdateDBService(_ context.Context, dbService *DBService) error { + f.updated = dbService + return nil +} +func (f *fakeDBServiceRepoForPassword) CountDBService(context.Context) ([]DBTypeCount, error) { + return nil, nil +} +func (f *fakeDBServiceRepoForPassword) GetBusinessByProjectUID(context.Context, string) ([]string, error) { + return nil, nil +} +func (f *fakeDBServiceRepoForPassword) GetFieldDistinctValue(context.Context, DBServiceField, interface{}) error { + return nil +} + +type fakeProjectRepoForPassword struct { + project *Project +} + +func (f *fakeProjectRepoForPassword) SaveProject(context.Context, *Project) error { return nil } +func (f *fakeProjectRepoForPassword) BatchSaveProjects(context.Context, []*Project) error { + return nil +} +func (f *fakeProjectRepoForPassword) ListProjects(context.Context, *ListProjectsOption, string) ([]*Project, int64, error) { + return nil, 0, nil +} +func (f *fakeProjectRepoForPassword) GetProject(context.Context, string) (*Project, error) { + return f.project, nil +} +func (f *fakeProjectRepoForPassword) GetProjectByName(context.Context, string) (*Project, error) { + return f.project, nil +} +func (f *fakeProjectRepoForPassword) GetProjectByNames(context.Context, []string) ([]*Project, error) { + return []*Project{f.project}, nil +} +func (f *fakeProjectRepoForPassword) UpdateProject(context.Context, *Project) error { return nil } +func (f *fakeProjectRepoForPassword) DelProject(context.Context, string) error { return nil } +func (f *fakeProjectRepoForPassword) UpdateDBServiceBusiness(context.Context, string, string, string) error { + return nil +} + +type fakeEnvTagRepoForPassword struct { + tag *EnvironmentTag +} + +func (f *fakeEnvTagRepoForPassword) CreateEnvironmentTag(context.Context, *EnvironmentTag) error { + return nil +} +func (f *fakeEnvTagRepoForPassword) UpdateEnvironmentTag(context.Context, string, string, string) error { + return nil +} +func (f *fakeEnvTagRepoForPassword) DeleteEnvironmentTag(context.Context, string) error { return nil } +func (f *fakeEnvTagRepoForPassword) GetEnvironmentTagByName(context.Context, string, string) (bool, *EnvironmentTag, error) { + return true, f.tag, nil +} +func (f *fakeEnvTagRepoForPassword) GetEnvironmentTagByUID(context.Context, string) (*EnvironmentTag, error) { + return f.tag, nil +} +func (f *fakeEnvTagRepoForPassword) ListEnvironmentTags(context.Context, *ListEnvironmentTagsOption) ([]*EnvironmentTag, int64, error) { + return nil, 0, nil +} + +func newDBServiceUsecaseForEmptyPasswordTest(repo *fakeDBServiceRepoForPassword) *DBServiceUsecase { + logger := utilLog.NewMyLogger(io.Discard) + projectRepo := &fakeProjectRepoForPassword{ + project: &Project{UID: "700300", Status: ProjectStatusActive}, + } + projectUC := &ProjectUsecase{ + repo: projectRepo, + log: utilLog.NewHelper(logger, utilLog.WithMessageKey("biz.project.test")), + } + opUC := NewOpPermissionVerifyUsecase(logger, nil, &mockOpPermissionVerifyRepo{}, &mockUserRepo{users: map[string]*User{}}) + envUC := &EnvironmentTagUsecase{ + environmentTagRepo: &fakeEnvTagRepoForPassword{ + tag: &EnvironmentTag{UID: "env-1", Name: "prod"}, + }, + log: utilLog.NewHelper(logger, utilLog.WithMessageKey("biz.env.test")), + } + pluginUC := &PluginUsecase{registeredPlugins: nil} + return NewDBServiceUsecase(logger, repo, nil, pluginUC, opUC, projectUC, nil, envUC) +} + +func TestUpdateDBServiceByArgs_EmptyPasswordAllowed(t *testing.T) { + repo := &fakeDBServiceRepoForPassword{ + svc: &DBService{ + UID: "ds-1", + Name: "gbase8a", + DBType: "GBase-8a", + Host: "10.186.16.126", + Port: "5258", + User: "root", + Password: "old-secret", + ProjectUID: "700300", + }, + } + uc := newDBServiceUsecaseForEmptyPasswordTest(repo) + empty := "" + err := uc.UpdateDBServiceByArgs(context.Background(), "ds-1", &BizDBServiceArgs{ + DBType: "GBase-8a", + Host: "10.186.16.126", + Port: "5258", + User: "root", + Password: &empty, + EnvironmentTagUID: "env-1", + }, pkgConst.UIDOfUserAdmin) + if err != nil { + t.Fatalf("expected Update with password=\"\" to succeed (not \"password can't be empty\"), got: %v", err) + } + if repo.updated == nil { + t.Fatal("expected UpdateDBService to be called") + } + if repo.updated.Password != "" { + t.Fatalf("expected stored password to be empty string, got %q", repo.updated.Password) + } +} + +func TestUpdateDBServiceByArgs_MissingHostStillRejected(t *testing.T) { + repo := &fakeDBServiceRepoForPassword{ + svc: &DBService{ + UID: "ds-1", + Name: "gbase8a", + DBType: "GBase-8a", + Host: "10.186.16.126", + Port: "5258", + User: "root", + Password: "old-secret", + ProjectUID: "700300", + }, + } + uc := newDBServiceUsecaseForEmptyPasswordTest(repo) + empty := "" + err := uc.UpdateDBServiceByArgs(context.Background(), "ds-1", &BizDBServiceArgs{ + DBType: "GBase-8a", + Host: "", + Port: "5258", + User: "root", + Password: &empty, + EnvironmentTagUID: "env-1", + }, pkgConst.UIDOfUserAdmin) + if err == nil { + t.Fatal("expected missing Host to fail Update") + } + if !strings.Contains(err.Error(), "host") { + t.Fatalf("expected host-related error, got: %v", err) + } + if strings.Contains(err.Error(), "password can't be empty") { + t.Fatalf("must not fail on legacy empty-password check: %v", err) + } +} From a13bcb110ea2dae25c357e97f036768c1a2ebd0a Mon Sep 17 00:00:00 2001 From: actiontech-zihan Date: Tue, 11 Aug 2026 19:37:49 +0800 Subject: [PATCH 2/2] fix: require non-empty DB service password on add/update Revert empty-password allowance; restore Add validate required and reject empty Update password. Co-authored-by: Cursor --- api/dms/service/v1/db_service.go | 4 ++-- .../service/v1/db_service_password_test.go | 24 +++++++++---------- api/dms/service/v2/db_service.go | 4 ++-- .../service/v2/db_service_password_test.go | 24 +++++++++---------- internal/dms/biz/db_service.go | 4 +++- internal/dms/biz/db_service_password_test.go | 21 +++++++--------- 6 files changed, 40 insertions(+), 41 deletions(-) diff --git a/api/dms/service/v1/db_service.go b/api/dms/service/v1/db_service.go index b2a0ad40..2064029b 100644 --- a/api/dms/service/v1/db_service.go +++ b/api/dms/service/v1/db_service.go @@ -28,8 +28,8 @@ type DBService struct { // Required: true User string `json:"user"` // DB Service admin password - // Required: false — empty string allowed (e.g. GBase-8a) - Password string `json:"password"` + // Required: true + Password string `json:"password" validate:"required"` // DB Service business name // Required: true // Deprecated: the business field is replaced with the environmentTag of the v2 interface. diff --git a/api/dms/service/v1/db_service_password_test.go b/api/dms/service/v1/db_service_password_test.go index 2a1b5e6e..b9912386 100644 --- a/api/dms/service/v1/db_service_password_test.go +++ b/api/dms/service/v1/db_service_password_test.go @@ -7,7 +7,7 @@ import ( utilConf "github.com/actiontech/dms/pkg/dms-common/pkg/config" ) -func TestAddDBServiceReq_EmptyPasswordAllowed(t *testing.T) { +func TestAddDBServiceReq_EmptyPasswordRejected(t *testing.T) { t.Parallel() base := func(password string) *AddDBServiceReq { @@ -28,17 +28,20 @@ func TestAddDBServiceReq_EmptyPasswordAllowed(t *testing.T) { t.Run("password_empty_string", func(t *testing.T) { t.Parallel() - if err := utilConf.Validate(base("")); err != nil { - t.Fatalf("expected empty password to pass Add validation, got: %v", err) + err := utilConf.Validate(base("")) + if err == nil { + t.Fatal("expected empty password to fail Add validation") + } + msg := strings.ToLower(err.Error()) + if !strings.Contains(msg, "password") || !strings.Contains(msg, "required") { + t.Fatalf("expected Password required validation error, got: %v", err) } }) - t.Run("password_omitted_zero_value", func(t *testing.T) { + t.Run("password_non_empty_passes_password_rule", func(t *testing.T) { t.Parallel() - req := base("") - req.DBService.Password = "" // JSON omit binds to zero value - if err := utilConf.Validate(req); err != nil { - t.Fatalf("expected omitted/zero password to pass Add validation, got: %v", err) + if err := utilConf.Validate(base("not-empty")); err != nil { + t.Fatalf("expected non-empty password to pass Add validation, got: %v", err) } }) } @@ -54,7 +57,7 @@ func TestAddDBServiceReq_MissingHostStillRequired(t *testing.T) { Host: "", Port: "5258", User: "root", - Password: "", + Password: "not-empty", Business: "default", }, } @@ -67,7 +70,4 @@ func TestAddDBServiceReq_MissingHostStillRequired(t *testing.T) { if !strings.Contains(msg, "Host") || !strings.Contains(msg, "required") { t.Fatalf("expected Host required validation error, got: %v", err) } - if strings.Contains(strings.ToLower(msg), "password") { - t.Fatalf("Host failure must not be attributed to password: %v", err) - } } diff --git a/api/dms/service/v2/db_service.go b/api/dms/service/v2/db_service.go index b1fd1073..1bc4788b 100644 --- a/api/dms/service/v2/db_service.go +++ b/api/dms/service/v2/db_service.go @@ -108,8 +108,8 @@ type DBService struct { // Required: true User string `json:"user"` // DB Service admin password - // Required: false — empty string allowed (e.g. GBase-8a) - Password string `json:"password"` + // Required: true + Password string `json:"password" validate:"required"` // DB Service environment tag // Required: true EnvironmentTagUID string `json:"environment_tag_uid" validate:"required"` diff --git a/api/dms/service/v2/db_service_password_test.go b/api/dms/service/v2/db_service_password_test.go index e397f640..a21b2359 100644 --- a/api/dms/service/v2/db_service_password_test.go +++ b/api/dms/service/v2/db_service_password_test.go @@ -7,7 +7,7 @@ import ( utilConf "github.com/actiontech/dms/pkg/dms-common/pkg/config" ) -func TestAddDBServiceReq_EmptyPasswordAllowed(t *testing.T) { +func TestAddDBServiceReq_EmptyPasswordRejected(t *testing.T) { t.Parallel() base := func(password string) *AddDBServiceReq { @@ -28,17 +28,20 @@ func TestAddDBServiceReq_EmptyPasswordAllowed(t *testing.T) { t.Run("password_empty_string", func(t *testing.T) { t.Parallel() - if err := utilConf.Validate(base("")); err != nil { - t.Fatalf("expected empty password to pass Add validation, got: %v", err) + err := utilConf.Validate(base("")) + if err == nil { + t.Fatal("expected empty password to fail Add validation") + } + msg := strings.ToLower(err.Error()) + if !strings.Contains(msg, "password") || !strings.Contains(msg, "required") { + t.Fatalf("expected Password required validation error, got: %v", err) } }) - t.Run("password_omitted_zero_value", func(t *testing.T) { + t.Run("password_non_empty_passes_password_rule", func(t *testing.T) { t.Parallel() - req := base("") - req.DBService.Password = "" // JSON omit binds to zero value - if err := utilConf.Validate(req); err != nil { - t.Fatalf("expected omitted/zero password to pass Add validation, got: %v", err) + if err := utilConf.Validate(base("not-empty")); err != nil { + t.Fatalf("expected non-empty password to pass Add validation, got: %v", err) } }) } @@ -54,7 +57,7 @@ func TestAddDBServiceReq_MissingHostStillRequired(t *testing.T) { Host: "", Port: "5258", User: "root", - Password: "", + Password: "not-empty", EnvironmentTagUID: "2086752861772845056", }, } @@ -67,7 +70,4 @@ func TestAddDBServiceReq_MissingHostStillRequired(t *testing.T) { if !strings.Contains(msg, "Host") || !strings.Contains(msg, "required") { t.Fatalf("expected Host required validation error, got: %v", err) } - if strings.Contains(strings.ToLower(msg), "password") { - t.Fatalf("Host failure must not be attributed to password: %v", err) - } } diff --git a/internal/dms/biz/db_service.go b/internal/dms/biz/db_service.go index 44c91911..f626d324 100644 --- a/internal/dms/biz/db_service.go +++ b/internal/dms/biz/db_service.go @@ -743,7 +743,9 @@ func (d *DBServiceUsecase) UpdateDBServiceByArgs(ctx context.Context, dbServiceU ds.Desc = *updateDBService.Desc } if updateDBService.Password != nil { - // empty string allowed (align with Add API; e.g. GBase-8a) + if *updateDBService.Password == "" { + return fmt.Errorf("password can't be empty") + } ds.Password = *updateDBService.Password } diff --git a/internal/dms/biz/db_service_password_test.go b/internal/dms/biz/db_service_password_test.go index 257db255..6e76fd92 100644 --- a/internal/dms/biz/db_service_password_test.go +++ b/internal/dms/biz/db_service_password_test.go @@ -115,7 +115,7 @@ func newDBServiceUsecaseForEmptyPasswordTest(repo *fakeDBServiceRepoForPassword) return NewDBServiceUsecase(logger, repo, nil, pluginUC, opUC, projectUC, nil, envUC) } -func TestUpdateDBServiceByArgs_EmptyPasswordAllowed(t *testing.T) { +func TestUpdateDBServiceByArgs_EmptyPasswordRejected(t *testing.T) { repo := &fakeDBServiceRepoForPassword{ svc: &DBService{ UID: "ds-1", @@ -138,14 +138,14 @@ func TestUpdateDBServiceByArgs_EmptyPasswordAllowed(t *testing.T) { Password: &empty, EnvironmentTagUID: "env-1", }, pkgConst.UIDOfUserAdmin) - if err != nil { - t.Fatalf("expected Update with password=\"\" to succeed (not \"password can't be empty\"), got: %v", err) + if err == nil { + t.Fatal("expected Update with password=\"\" to fail") } - if repo.updated == nil { - t.Fatal("expected UpdateDBService to be called") + if !strings.Contains(err.Error(), "password can't be empty") { + t.Fatalf("expected \"password can't be empty\", got: %v", err) } - if repo.updated.Password != "" { - t.Fatalf("expected stored password to be empty string, got %q", repo.updated.Password) + if repo.updated != nil { + t.Fatal("expected UpdateDBService not to be called when password is empty") } } @@ -163,13 +163,13 @@ func TestUpdateDBServiceByArgs_MissingHostStillRejected(t *testing.T) { }, } uc := newDBServiceUsecaseForEmptyPasswordTest(repo) - empty := "" + pwd := "not-empty" err := uc.UpdateDBServiceByArgs(context.Background(), "ds-1", &BizDBServiceArgs{ DBType: "GBase-8a", Host: "", Port: "5258", User: "root", - Password: &empty, + Password: &pwd, EnvironmentTagUID: "env-1", }, pkgConst.UIDOfUserAdmin) if err == nil { @@ -178,7 +178,4 @@ func TestUpdateDBServiceByArgs_MissingHostStillRejected(t *testing.T) { if !strings.Contains(err.Error(), "host") { t.Fatalf("expected host-related error, got: %v", err) } - if strings.Contains(err.Error(), "password can't be empty") { - t.Fatalf("must not fail on legacy empty-password check: %v", err) - } }