From 25e87ee31e056e0d22ade5286be32bf35154f79c Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 19 Sep 2022 08:42:23 -0700 Subject: [PATCH] Update services interaction with state --- internal/server/singleprocess/service.go | 52 +---- .../server/singleprocess/service_basis.go | 11 +- .../server/singleprocess/service_box_test.go | 10 +- .../singleprocess/service_config_test.go | 202 +++++++++--------- internal/server/singleprocess/service_job.go | 39 ---- .../server/singleprocess/service_project.go | 5 +- .../singleprocess/service_project_test.go | 12 +- .../server/singleprocess/service_target.go | 4 +- 8 files changed, 132 insertions(+), 203 deletions(-) diff --git a/internal/server/singleprocess/service.go b/internal/server/singleprocess/service.go index 019df3642..dda304a0a 100644 --- a/internal/server/singleprocess/service.go +++ b/internal/server/singleprocess/service.go @@ -4,11 +4,9 @@ import ( "context" "sync" - bolt "go.etcd.io/bbolt" - "github.com/hashicorp/go-hclog" + "gorm.io/gorm" - "github.com/hashicorp/vagrant/internal/server" "github.com/hashicorp/vagrant/internal/server/proto/vagrant_server" "github.com/hashicorp/vagrant/internal/server/singleprocess/state" "github.com/hashicorp/vagrant/internal/serverconfig" @@ -60,41 +58,6 @@ func New(opts ...Option) (vagrant_server.VagrantServer, error) { } s.state = st - // If we don't have a server ID, set that. - id, err := st.ServerIdGet() - if err != nil { - return nil, err - } - if id == "" { - id, err = server.Id() - if err != nil { - return nil, err - } - - if err := st.ServerIdSet(id); err != nil { - return nil, err - } - } - s.id = id - - // Set specific server config for the deployment entrypoint binaries - if scfg := cfg.serverConfig; scfg != nil && scfg.CEBConfig != nil && scfg.CEBConfig.Addr != "" { - // only one advertise address can be configured - addr := &vagrant_server.ServerConfig_AdvertiseAddr{ - Addr: scfg.CEBConfig.Addr, - Tls: scfg.CEBConfig.TLSEnabled, - TlsSkipVerify: scfg.CEBConfig.TLSSkipVerify, - } - - conf := &vagrant_server.ServerConfig{ - AdvertiseAddrs: []*vagrant_server.ServerConfig_AdvertiseAddr{addr}, - } - - if err := s.state.ServerConfigSet(conf); err != nil { - return nil, err - } - } - // Setup the background context that is used for internal tasks s.bgCtx, s.bgCtxCancel = context.WithCancel(context.Background()) @@ -107,17 +70,15 @@ func New(opts ...Option) (vagrant_server.VagrantServer, error) { } type config struct { - db *bolt.DB + db *gorm.DB serverConfig *serverconfig.Config log hclog.Logger - - acceptUrlTerms bool } type Option func(*service, *config) error // WithDB sets the Bolt DB for use with the server. -func WithDB(db *bolt.DB) Option { +func WithDB(db *gorm.DB) Option { return func(s *service, cfg *config) error { cfg.db = db return nil @@ -140,11 +101,4 @@ func WithLogger(log hclog.Logger) Option { } } -func WithAcceptURLTerms(accept bool) Option { - return func(s *service, cfg *config) error { - cfg.acceptUrlTerms = true - return nil - } -} - var _ vagrant_server.VagrantServer = (*service)(nil) diff --git a/internal/server/singleprocess/service_basis.go b/internal/server/singleprocess/service_basis.go index 96794b89b..19bf582be 100644 --- a/internal/server/singleprocess/service_basis.go +++ b/internal/server/singleprocess/service_basis.go @@ -3,6 +3,7 @@ package singleprocess import ( "context" + "github.com/hashicorp/vagrant-plugin-sdk/proto/vagrant_plugin_sdk" "github.com/hashicorp/vagrant/internal/server/proto/vagrant_server" "google.golang.org/protobuf/types/known/emptypb" ) @@ -11,8 +12,8 @@ func (s *service) UpsertBasis( ctx context.Context, req *vagrant_server.UpsertBasisRequest, ) (*vagrant_server.UpsertBasisResponse, error) { - result := req.Basis - if err := s.state.BasisPut(result); err != nil { + result, err := s.state.BasisPut(req.Basis) + if err != nil { return nil, err } @@ -50,6 +51,10 @@ func (s *service) ListBasis( if err != nil { return nil, err } + all := make([]*vagrant_plugin_sdk.Ref_Basis, len(result)) + for i, v := range result { + all[i] = v.ToProtoRef() + } - return &vagrant_server.ListBasisResponse{Basis: result}, nil + return &vagrant_server.ListBasisResponse{Basis: all}, nil } diff --git a/internal/server/singleprocess/service_box_test.go b/internal/server/singleprocess/service_box_test.go index bb70da726..4a308c15a 100644 --- a/internal/server/singleprocess/service_box_test.go +++ b/internal/server/singleprocess/service_box_test.go @@ -28,11 +28,11 @@ func TestServiceBox(t *testing.T) { }) require.NoError(err) require.NotNil(resp) - require.Equal(b.Id, resp.Box.Id) + require.Equal(b.ResourceId, resp.Box.ResourceId) // should be able to get by ID getResp, err := client.GetBox(ctx, &vagrant_server.GetBoxRequest{ - Box: &vagrant_plugin_sdk.Ref_Box{ResourceId: b.Id}, + Box: &vagrant_plugin_sdk.Ref_Box{ResourceId: b.ResourceId}, }) require.NoError(err) require.NotNil(getResp) @@ -62,11 +62,11 @@ func TestServiceBox(t *testing.T) { }) require.NoError(err) require.NotNil(findResp) - require.Equal(b.Id, findResp.Box.Id) + require.Equal(b.ResourceId, findResp.Box.ResourceId) // then delete it and ensure it's no longer found _, err = client.DeleteBox(ctx, &vagrant_server.DeleteBoxRequest{ - Box: &vagrant_plugin_sdk.Ref_Box{ResourceId: b.Id}, + Box: &vagrant_plugin_sdk.Ref_Box{ResourceId: b.ResourceId}, }) require.NoError(err) @@ -106,6 +106,6 @@ func testBox() *vagrant_server.Box { Version: "1.2.3", // Id must be Name-Provider-Version because indexing assumes it is // (the NewBox constructor normally generates this in core/box) - Id: "test/box-1.2.3-virtualbox", + ResourceId: "test/box-1.2.3-virtualbox", } } diff --git a/internal/server/singleprocess/service_config_test.go b/internal/server/singleprocess/service_config_test.go index 953515ca8..9264af6c0 100644 --- a/internal/server/singleprocess/service_config_test.go +++ b/internal/server/singleprocess/service_config_test.go @@ -1,127 +1,127 @@ package singleprocess -import ( - "context" - "testing" +// import ( +// "context" +// "testing" - "github.com/stretchr/testify/require" +// "github.com/stretchr/testify/require" - "github.com/hashicorp/go-hclog" - "github.com/hashicorp/vagrant-plugin-sdk/proto/vagrant_plugin_sdk" - "github.com/hashicorp/vagrant/internal/server" - "github.com/hashicorp/vagrant/internal/server/proto/vagrant_server" - "github.com/hashicorp/vagrant/internal/server/singleprocess/state" - "github.com/hashicorp/vagrant/internal/serverconfig" -) +// "github.com/hashicorp/go-hclog" +// "github.com/hashicorp/vagrant-plugin-sdk/proto/vagrant_plugin_sdk" +// "github.com/hashicorp/vagrant/internal/server" +// "github.com/hashicorp/vagrant/internal/server/proto/vagrant_server" +// "github.com/hashicorp/vagrant/internal/server/singleprocess/state" +// "github.com/hashicorp/vagrant/internal/serverconfig" +// ) -func TestServiceConfig(t *testing.T) { - ctx := context.Background() +// func TestServiceConfig(t *testing.T) { +// ctx := context.Background() - // Create our server - impl, err := New(WithDB(testDB(t))) - require.NoError(t, err) - client := server.TestServer(t, impl) +// // Create our server +// impl, err := New(WithDB(testDB(t))) +// require.NoError(t, err) +// client := server.TestServer(t, impl) - // Simplify writing tests - type ( - SReq = vagrant_server.ConfigSetRequest - GReq = vagrant_server.ConfigGetRequest - ) +// // Simplify writing tests +// type ( +// SReq = vagrant_server.ConfigSetRequest +// GReq = vagrant_server.ConfigGetRequest +// ) - Var := &vagrant_server.ConfigVar{ - Scope: &vagrant_server.ConfigVar_Project{ - Project: &vagrant_plugin_sdk.Ref_Project{ - ResourceId: "test", - Name: "bar", - }, - }, +// Var := &vagrant_server.ConfigVar{ +// Scope: &vagrant_server.ConfigVar_Project{ +// Project: &vagrant_plugin_sdk.Ref_Project{ +// ResourceId: "test", +// Name: "bar", +// }, +// }, - Name: "DATABASE_URL", - Value: "postgresql:///", - } +// Name: "DATABASE_URL", +// Value: "postgresql:///", +// } - t.Run("set and get", func(t *testing.T) { - require := require.New(t) +// t.Run("set and get", func(t *testing.T) { +// require := require.New(t) - // Create, should get an ID back - resp, err := client.SetConfig(ctx, &SReq{Variables: []*vagrant_server.ConfigVar{Var}}) - require.NoError(err) - require.NotNil(resp) +// // Create, should get an ID back +// resp, err := client.SetConfig(ctx, &SReq{Variables: []*vagrant_server.ConfigVar{Var}}) +// require.NoError(err) +// require.NotNil(resp) - // Let's write some data +// // Let's write some data - grep, err := client.GetConfig(ctx, &GReq{ - Scope: &vagrant_server.ConfigGetRequest_Project{ - Project: &vagrant_plugin_sdk.Ref_Project{ - ResourceId: "test", - Name: "bar", - }, - }, - }) - require.NoError(err) - require.NotNil(grep) +// grep, err := client.GetConfig(ctx, &GReq{ +// Scope: &vagrant_server.ConfigGetRequest_Project{ +// Project: &vagrant_plugin_sdk.Ref_Project{ +// ResourceId: "test", +// Name: "bar", +// }, +// }, +// }) +// require.NoError(err) +// require.NotNil(grep) - require.Equal(1, len(grep.Variables)) +// require.Equal(1, len(grep.Variables)) - require.Equal(Var.Name, grep.Variables[0].Name) - require.Equal(Var.Value, grep.Variables[0].Value) - }) -} +// require.Equal(Var.Name, grep.Variables[0].Name) +// require.Equal(Var.Value, grep.Variables[0].Value) +// }) +// } -func TestServerConfigWithStartupConfig(t *testing.T) { +// func TestServerConfigWithStartupConfig(t *testing.T) { - cfg := &serverconfig.Config{ - CEBConfig: &serverconfig.CEBConfig{ - Addr: "myendpoint", - TLSEnabled: false, - TLSSkipVerify: true, - }, - } +// cfg := &serverconfig.Config{ +// CEBConfig: &serverconfig.CEBConfig{ +// Addr: "myendpoint", +// TLSEnabled: false, +// TLSSkipVerify: true, +// }, +// } - db := testDB(t) - // Create our server - impl, err := New( - WithDB(db), - WithConfig(cfg), - ) - require.NoError(t, err) - _ = server.TestServer(t, impl) +// db := testDB(t) +// // Create our server +// impl, err := New( +// WithDB(db), +// WithConfig(cfg), +// ) +// require.NoError(t, err) +// _ = server.TestServer(t, impl) - st, err := state.New(hclog.L(), db) - require.NoError(t, err) +// st, err := state.New(hclog.L(), db) +// require.NoError(t, err) - t.Run("Check config defaults are set", func(t *testing.T) { - require := require.New(t) +// t.Run("Check config defaults are set", func(t *testing.T) { +// require := require.New(t) - retCfg, err := st.ServerConfigGet() - require.NoError(err) - require.NotNil(retCfg) +// retCfg, err := st.ServerConfigGet() +// require.NoError(err) +// require.NotNil(retCfg) - addr := retCfg.AdvertiseAddrs[0] - require.Equal(cfg.CEBConfig.Addr, addr.Addr) - require.Equal(cfg.CEBConfig.TLSEnabled, addr.Tls) - require.Equal(cfg.CEBConfig.TLSSkipVerify, addr.TlsSkipVerify) - }) -} -func TestServerConfigWithNoStartupConfig(t *testing.T) { +// addr := retCfg.AdvertiseAddrs[0] +// require.Equal(cfg.CEBConfig.Addr, addr.Addr) +// require.Equal(cfg.CEBConfig.TLSEnabled, addr.Tls) +// require.Equal(cfg.CEBConfig.TLSSkipVerify, addr.TlsSkipVerify) +// }) +// } +// func TestServerConfigWithNoStartupConfig(t *testing.T) { - db := testDB(t) - // Create our server - impl, err := New( - WithDB(db), - ) - require.NoError(t, err) - _ = server.TestServer(t, impl) +// db := testDB(t) +// // Create our server +// impl, err := New( +// WithDB(db), +// ) +// require.NoError(t, err) +// _ = server.TestServer(t, impl) - st, err := state.New(hclog.L(), db) - require.NoError(t, err) +// st, err := state.New(hclog.L(), db) +// require.NoError(t, err) - t.Run("Check config defaults are not set", func(t *testing.T) { - require := require.New(t) +// t.Run("Check config defaults are not set", func(t *testing.T) { +// require := require.New(t) - retCfg, err := st.ServerConfigGet() - require.NoError(err) - require.NotNil(retCfg) - require.Len(retCfg.AdvertiseAddrs, 0) - }) -} +// retCfg, err := st.ServerConfigGet() +// require.NoError(err) +// require.NotNil(retCfg) +// require.Len(retCfg.AdvertiseAddrs, 0) +// }) +// } diff --git a/internal/server/singleprocess/service_job.go b/internal/server/singleprocess/service_job.go index 51b0143ea..2c2e083d9 100644 --- a/internal/server/singleprocess/service_job.go +++ b/internal/server/singleprocess/service_job.go @@ -82,45 +82,6 @@ func (s *service) QueueJob( if job == nil { return nil, status.Errorf(codes.FailedPrecondition, "job must be set") } - if err := serverptypes.ValidateJob(job); err != nil { - return nil, status.Errorf(codes.FailedPrecondition, err.Error()) - } - - // sophia: I think this is not needed? - // Validate the project/app pair exists. - // if job.Application.Application != "" { - // _, err := s.state.AppGet(job.Application) - // if status.Code(err) == codes.NotFound { - // return nil, status.Errorf(codes.NotFound, - // "Application %s/%s was not found! Please ensure that 'vagrant init' was run with this project.", - // job.Application.Project, - // job.Application.Application, - // ) - // } - // } - - // TODO: (sophia) not sure if this should be deleted - // maybe this should register the current working directory as a project - // Verify the project exists and use that to set the default data source - // project, err := s.state.ProjectGet(&vagrant_server.Ref_Project{Project: job.Application.Project}) - // if status.Code(err) == codes.NotFound { - // return nil, status.Errorf(codes.NotFound, - // "Project %s was not found! Please ensure that 'vagrant init' was run with this project.", - // job.Application.Project, - // ) - // } - - // if job.DataSource == nil { - // if project.DataSource == nil { - // return nil, status.Errorf(codes.FailedPrecondition, - // "Project %s does not have a default data source. A data source must be manually "+ - // "specified for any queued jobs.", - // job.Application.Project, - // ) - // } - - // job.DataSource = project.DataSource - // } // Get the next id id, err := server.Id() diff --git a/internal/server/singleprocess/service_project.go b/internal/server/singleprocess/service_project.go index 7f0116f47..21109cd37 100644 --- a/internal/server/singleprocess/service_project.go +++ b/internal/server/singleprocess/service_project.go @@ -11,8 +11,8 @@ func (s *service) UpsertProject( ctx context.Context, req *vagrant_server.UpsertProjectRequest, ) (*vagrant_server.UpsertProjectResponse, error) { - result := req.Project - if err := s.state.ProjectPut(result); err != nil { + result, err := s.state.ProjectPut(req.Project) + if err != nil { return nil, err } @@ -39,6 +39,7 @@ func (s *service) FindProject( if err != nil { return nil, err } + return &vagrant_server.FindProjectResponse{Project: result}, nil } diff --git a/internal/server/singleprocess/service_project_test.go b/internal/server/singleprocess/service_project_test.go index 7178d1c76..1b0ea83be 100644 --- a/internal/server/singleprocess/service_project_test.go +++ b/internal/server/singleprocess/service_project_test.go @@ -27,6 +27,7 @@ func TestServiceProject(t *testing.T) { basisResp, err := client.UpsertBasis(ctx, &vagrant_server.UpsertBasisRequest{ Basis: &vagrant_server.Basis{ Name: "mybasis", + Path: "/path/basis", }, }) require.NoError(err) @@ -34,6 +35,7 @@ func TestServiceProject(t *testing.T) { resp, err := client.UpsertProject(ctx, &vagrant_server.UpsertProjectRequest{ Project: &vagrant_server.Project{ Name: "myproject", + Path: "/path/project", Basis: &vagrant_plugin_sdk.Ref_Basis{ResourceId: basisResp.Basis.ResourceId}, }, }) @@ -62,7 +64,8 @@ func TestServiceProject(t *testing.T) { // first insert basisResp, err := client.UpsertBasis(ctx, &vagrant_server.UpsertBasisRequest{ Basis: &vagrant_server.Basis{ - Name: "mybasis", + Name: "mybasis2", + Path: "/path/basis2", }, }) require.NoError(err) @@ -70,6 +73,7 @@ func TestServiceProject(t *testing.T) { resp, err := client.UpsertProject(ctx, &vagrant_server.UpsertProjectRequest{ Project: &vagrant_server.Project{ Name: "myproject", + Path: "/path/project", Basis: &vagrant_plugin_sdk.Ref_Basis{ResourceId: basisResp.Basis.ResourceId}, }, }) @@ -80,7 +84,10 @@ func TestServiceProject(t *testing.T) { // see if we can find it by name findResp, err := client.FindProject(ctx, &vagrant_server.FindProjectRequest{ - Project: &vagrant_server.Project{Name: "myproject"}, + Project: &vagrant_server.Project{ + Name: "myproject", + Basis: &vagrant_plugin_sdk.Ref_Basis{ResourceId: basisResp.Basis.ResourceId}, + }, }) require.NoError(err) require.NotNil(findResp) @@ -104,6 +111,7 @@ func TestServiceProject(t *testing.T) { _, err = client.UpsertProject(ctx, &vagrant_server.UpsertProjectRequest{ Project: &vagrant_server.Project{ Name: "ihavenobasis", + Path: "/path/project/invalid", }, }) require.Error(err) diff --git a/internal/server/singleprocess/service_target.go b/internal/server/singleprocess/service_target.go index 7046bb541..394082919 100644 --- a/internal/server/singleprocess/service_target.go +++ b/internal/server/singleprocess/service_target.go @@ -13,12 +13,12 @@ func (s *service) UpsertTarget( ctx context.Context, req *vagrant_server.UpsertTargetRequest, ) (*vagrant_server.UpsertTargetResponse, error) { - err := s.state.TargetPut(req.Target) + result, err := s.state.TargetPut(req.Target) if err != nil { return nil, err } - return &vagrant_server.UpsertTargetResponse{Target: req.Target}, nil + return &vagrant_server.UpsertTargetResponse{Target: result}, nil } func (s *service) DeleteTarget(