From 21b845b79e2210d72ba383fd10174657f80a7833 Mon Sep 17 00:00:00 2001 From: Guiheux Steven Date: Tue, 12 May 2020 10:31:25 +0200 Subject: [PATCH] fix(api): Allow to get every service through getServiceHandler (#5173) --- contrib/grpcplugins/action/clair/main.go | 3 +- contrib/grpcplugins/grpcplugins.go | 18 ++++---- engine/api/api.go | 22 +++------- engine/api/api_routes.go | 2 +- engine/api/services.go | 44 ++++++++++++++----- engine/api/services/dao.go | 16 +++++-- engine/api/services/external.go | 25 +++++++++-- engine/api/services_test.go | 33 ++++++++++++++ engine/config.go | 2 +- engine/worker/internal/handler_service.go | 4 +- sdk/cdsclient/client_services.go | 8 ++-- sdk/cdsclient/interface.go | 2 +- .../mock_cdsclient/interface_mock.go | 4 +- sdk/services.go | 28 +++++------- 14 files changed, 140 insertions(+), 71 deletions(-) diff --git a/contrib/grpcplugins/action/clair/main.go b/contrib/grpcplugins/action/clair/main.go index ef6a6611a9..075254df67 100644 --- a/contrib/grpcplugins/action/clair/main.go +++ b/contrib/grpcplugins/action/clair/main.go @@ -38,10 +38,11 @@ func (actPlugin *clairActionPlugin) Manifest(ctx context.Context, _ *empty.Empty func (actPlugin *clairActionPlugin) Run(ctx context.Context, q *actionplugin.ActionQuery) (*actionplugin.ActionResult, error) { // get clair configuration fmt.Printf("Retrieve clair configuration...") - serv, err := grpcplugins.GetExternalServices(actPlugin.HTTPPort, "clair") + servs, err := grpcplugins.GetServices(actPlugin.HTTPPort, "clair") if err != nil { return actionplugin.Fail("Unable to get clair configuration: %s", err) } + serv := servs[0] viper.Set("clair.uri", serv.URL) viper.Set("clair.port", serv.Port) viper.Set("clair.healthPort", serv.HealthPort) diff --git a/contrib/grpcplugins/grpcplugins.go b/contrib/grpcplugins/grpcplugins.go index 73a27130a2..0293b0a7e9 100644 --- a/contrib/grpcplugins/grpcplugins.go +++ b/contrib/grpcplugins/grpcplugins.go @@ -38,34 +38,34 @@ func SendVulnerabilityReport(workerHTTPPort int32, report sdk.VulnerabilityWorke return nil } -// GetExternalServices call worker to get external service configuration -func GetExternalServices(workerHTTPPort int32, serviceType string) (sdk.ExternalService, error) { +// GetServices call worker to get external service configuration +func GetServices(workerHTTPPort int32, serviceType string) ([]sdk.ServiceConfiguration, error) { if workerHTTPPort == 0 { - return sdk.ExternalService{}, nil + return nil, nil } req, err := http.NewRequest("GET", fmt.Sprintf("http://127.0.0.1:%d/services/%s", workerHTTPPort, serviceType), nil) if err != nil { - return sdk.ExternalService{}, fmt.Errorf("get service from worker /services: %v", err) + return nil, fmt.Errorf("get service from worker /services: %v", err) } resp, err := http.DefaultClient.Do(req) if err != nil { - return sdk.ExternalService{}, fmt.Errorf("cannot get service from worker /services: %v", err) + return nil, fmt.Errorf("cannot get service from worker /services: %v", err) } if resp.StatusCode >= 300 { - return sdk.ExternalService{}, fmt.Errorf("cannot get services from worker /services: HTTP %d", resp.StatusCode) + return nil, fmt.Errorf("cannot get services from worker /services: HTTP %d", resp.StatusCode) } b, err := ioutil.ReadAll(resp.Body) if err != nil { - return sdk.ExternalService{}, fmt.Errorf("cannot read body /services: %v", err) + return nil, fmt.Errorf("cannot read body /services: %v", err) } - var serv sdk.ExternalService + var serv []sdk.ServiceConfiguration if err := json.Unmarshal(b, &serv); err != nil { - return serv, fmt.Errorf("cannot unmarshal body /services: %v", err) + return nil, fmt.Errorf("cannot unmarshal body /services: %v", err) } return serv, nil } diff --git a/engine/api/api.go b/engine/api/api.go index 5e0a6d1f52..ce93263c7a 100644 --- a/engine/api/api.go +++ b/engine/api/api.go @@ -181,9 +181,9 @@ type Configuration struct { Token string `toml:"token" comment:"Token shared between Izanami and CDS to be able to send webhooks from izanami" json:"-"` } `toml:"izanami" comment:"Feature flipping provider: https://maif.github.io/izanami" json:"izanami"` } `toml:"features" comment:"###########################\n CDS Features flipping Settings \n##########################" json:"features"` - Services []ServiceConfiguration `toml:"services" comment:"###########################\n CDS Services Settings \n##########################" json:"services"` - DefaultOS string `toml:"defaultOS" default:"linux" comment:"if no model and os/arch is specified in your job's requirements then spawn worker on this operating system (example: freebsd, linux, windows)" json:"defaultOS"` - DefaultArch string `toml:"defaultArch" default:"amd64" comment:"if no model and no os/arch is specified in your job's requirements then spawn worker on this architecture (example: amd64, arm, 386)" json:"defaultArch"` + Services []sdk.ServiceConfiguration `toml:"services" comment:"###########################\n CDS Services Settings \n##########################" json:"services"` + DefaultOS string `toml:"defaultOS" default:"linux" comment:"if no model and os/arch is specified in your job's requirements then spawn worker on this operating system (example: freebsd, linux, windows)" json:"defaultOS"` + DefaultArch string `toml:"defaultArch" default:"amd64" comment:"if no model and no os/arch is specified in your job's requirements then spawn worker on this architecture (example: amd64, arm, 386)" json:"defaultArch"` Graylog struct { AccessToken string `toml:"accessToken" json:"-"` Stream string `toml:"stream" json:"-"` @@ -196,18 +196,6 @@ type Configuration struct { CDN cdn.Configuration `toml:"cdn" json:"cdn" comment:"###########################\n CDN settings.\n##########################"` } -// ServiceConfiguration is the configuration of external service -type ServiceConfiguration struct { - Name string `toml:"name" json:"name"` - URL string `toml:"url" json:"url"` - Port string `toml:"port" json:"port"` - Path string `toml:"path" json:"path"` - HealthURL string `toml:"healthUrl" json:"healthUrl"` - HealthPort string `toml:"healthPort" json:"healthPort"` - HealthPath string `toml:"healthPath" json:"healthPath"` - Type string `toml:"type" json:"type"` -} - // DefaultValues is the struc for API Default configuration default values type DefaultValues struct { ServerSecretsKey string @@ -782,10 +770,10 @@ func (a *API) Serve(ctx context.Context) error { // Init Services services.Initialize(ctx, a.DBConnectionFactory, a.PanicDump()) - externalServices := make([]sdk.ExternalService, 0, len(a.Config.Services)) + externalServices := make([]services.ExternalService, 0, len(a.Config.Services)) for _, s := range a.Config.Services { log.Info(ctx, "Managing external service %s %s", s.Name, s.Type) - serv := sdk.ExternalService{ + serv := services.ExternalService{ Service: sdk.Service{ CanonicalService: sdk.CanonicalService{ Name: s.Name, diff --git a/engine/api/api_routes.go b/engine/api/api_routes.go index 2b5d6d83df..6bf67fe72a 100644 --- a/engine/api/api_routes.go +++ b/engine/api/api_routes.go @@ -423,7 +423,7 @@ func (api *API) InitRouter() { // Engine µServices r.Handle("/services/register", Scope(sdk.AuthConsumerScopeService), r.POST(api.postServiceRegisterHandler, MaintenanceAware())) r.Handle("/services/heartbeat", Scope(sdk.AuthConsumerScopeService), r.POST(api.postServiceHearbeatHandler)) - r.Handle("/services/{type}", Scope(sdk.AuthConsumerScopeService), r.GET(api.getExternalServiceHandler)) + r.Handle("/services/{type}", Scope(sdk.AuthConsumerScopeService), r.GET(api.getServiceHandler)) // Templates r.Handle("/template", Scope(sdk.AuthConsumerScopeTemplate), r.GET(api.getTemplatesHandler), r.POST(api.postTemplateHandler)) diff --git a/engine/api/services.go b/engine/api/services.go index 26dd5f18bd..9b28339007 100644 --- a/engine/api/services.go +++ b/engine/api/services.go @@ -2,6 +2,7 @@ package api import ( "context" + "encoding/base64" "encoding/json" "net/http" "time" @@ -16,25 +17,46 @@ import ( "github.com/ovh/cds/sdk/log" ) -func (api *API) getExternalServiceHandler() service.Handler { +func (api *API) getServiceHandler() service.Handler { return func(ctx context.Context, w http.ResponseWriter, r *http.Request) error { vars := mux.Vars(r) typeService := vars["type"] + var servicesConf []sdk.ServiceConfiguration for _, s := range api.Config.Services { if s.Type == typeService { - extService := sdk.ExternalService{ - HealthPath: s.HealthPath, - HealthPort: s.HealthPort, - Path: s.Path, - HealthURL: s.HealthURL, - Port: s.Port, - URL: s.URL, - } - return service.WriteJSON(w, extService, http.StatusOK) + servicesConf = append(servicesConf, s) } } - return sdk.WrapError(sdk.ErrNotFound, "service %s not found", typeService) + if len(servicesConf) != 0 { + return service.WriteJSON(w, servicesConf, http.StatusOK) + } + + // Try to load from DB + var srvs []sdk.Service + var err error + if isAdmin(ctx) || isMaintainer(ctx) { + srvs, err = services.LoadAllByType(ctx, api.mustDB(), typeService) + } else { + c := getAPIConsumer(ctx) + srvs, err = services.LoadAllByTypeAndUserID(ctx, api.mustDB(), typeService, c.AuthentifiedUserID) + } + if err != nil { + return err + } + for _, s := range srvs { + servicesConf = append(servicesConf, sdk.ServiceConfiguration{ + URL: s.HTTPURL, + Name: s.Name, + ID: s.ID, + PublicKey: base64.StdEncoding.EncodeToString(s.PublicKey), + Type: s.Type, + }) + } + if len(servicesConf) == 0 { + return sdk.WrapError(sdk.ErrNotFound, "service %s not found", typeService) + } + return service.WriteJSON(w, servicesConf, http.StatusOK) } } diff --git a/engine/api/services/dao.go b/engine/api/services/dao.go index fee639a7b4..8c9df7121f 100644 --- a/engine/api/services/dao.go +++ b/engine/api/services/dao.go @@ -69,11 +69,21 @@ func LoadAll(ctx context.Context, db gorp.SqlExecutor) ([]sdk.Service, error) { } // LoadAllByType returns all services with given type. -func LoadAllByType(ctx context.Context, db gorp.SqlExecutor, stype string) ([]sdk.Service, error) { - if ss, ok := internalCache.getFromCache(stype); ok { +func LoadAllByType(ctx context.Context, db gorp.SqlExecutor, typeService string) ([]sdk.Service, error) { + if ss, ok := internalCache.getFromCache(typeService); ok { return ss, nil } - query := gorpmapping.NewQuery(`SELECT * FROM service WHERE type = $1`).Args(stype) + query := gorpmapping.NewQuery(`SELECT * FROM service WHERE type = $1`).Args(typeService) + return getAll(ctx, db, query) +} + +// LoadAllByType returns all services that users can see with given type. +func LoadAllByTypeAndUserID(ctx context.Context, db gorp.SqlExecutor, typeService string, userID string) ([]sdk.Service, error) { + query := gorpmapping.NewQuery(` + SELECT service.* + FROM service + JOIN auth_consumer on auth_consumer.id = service.auth_consumer_id + WHERE service.type = $1 AND auth_consumer.user_id = $2`).Args(typeService, userID) return getAll(ctx, db, query) } diff --git a/engine/api/services/external.go b/engine/api/services/external.go index c83fefef8c..b3c93ef293 100644 --- a/engine/api/services/external.go +++ b/engine/api/services/external.go @@ -2,6 +2,7 @@ package services import ( "context" + "encoding/json" "fmt" "net/url" "time" @@ -11,8 +12,26 @@ import ( "github.com/ovh/cds/sdk/log" ) +// ExternalService represents an external service +type ExternalService struct { + sdk.Service `json:"-"` + HealthURL string `json:"health_url"` + HealthPort string `json:"health_port"` + HealthPath string `json:"health_path"` + Port string `json:"port"` + URL string `json:"url"` + Path string `json:"path"` +} + +func (e ExternalService) ServiceConfig() sdk.ServiceConfig { + b, _ := json.Marshal(e) + var cfg sdk.ServiceConfig + json.Unmarshal(b, &cfg) // nolint + return cfg +} + // Pings browses all external services and ping them -func Pings(ctx context.Context, dbFunc func() *gorp.DbMap, ss []sdk.ExternalService) { +func Pings(ctx context.Context, dbFunc func() *gorp.DbMap, ss []ExternalService) { tickPing := time.NewTicker(1 * time.Minute) db := dbFunc() for { @@ -42,7 +61,7 @@ func Pings(ctx context.Context, dbFunc func() *gorp.DbMap, ss []sdk.ExternalServ } } -func ping(ctx context.Context, db gorp.SqlExecutor, s sdk.ExternalService) error { +func ping(ctx context.Context, db gorp.SqlExecutor, s ExternalService) error { // Select for update serv, err := LoadByNameForUpdateAndSkipLocked(context.Background(), db, s.Name) if err != nil && !sdk.ErrorIs(err, sdk.ErrNotFound) { @@ -93,7 +112,7 @@ func ping(ctx context.Context, db gorp.SqlExecutor, s sdk.ExternalService) error } // InitExternal initializes external services -func InitExternal(ctx context.Context, db *gorp.DbMap, ss []sdk.ExternalService) error { +func InitExternal(ctx context.Context, db *gorp.DbMap, ss []ExternalService) error { for _, s := range ss { tx, err := db.Begin() if err != nil { diff --git a/engine/api/services_test.go b/engine/api/services_test.go index 185d9afbfc..eccae26085 100644 --- a/engine/api/services_test.go +++ b/engine/api/services_test.go @@ -20,6 +20,7 @@ func TestServicesHandlers(t *testing.T) { defer end() admin, jwtRaw := assets.InsertAdminUser(t, api.mustDB()) + _, jwtLambda := assets.InsertLambdaUser(t, api.mustDB()) data := sdk.AuthConsumer{ Name: sdk.RandomString(10), @@ -82,5 +83,37 @@ func TestServicesHandlers(t *testing.T) { api.Router.Mux.ServeHTTP(rec, req) require.Equal(t, 204, rec.Code) + // Get service with lambda user => 404 + uri = api.Router.GetRoute(http.MethodGet, api.getServiceHandler, map[string]string{ + "type": services.TypeHatchery, + }) + require.NotEmpty(t, uri) + req = assets.NewJWTAuthentifiedRequest(t, jwtLambda, http.MethodGet, uri, data) + rec = httptest.NewRecorder() + api.Router.Mux.ServeHTTP(rec, req) + require.Equal(t, 404, rec.Code) + + // lambda user Insert a service + uri = api.Router.GetRoute(http.MethodPost, api.postServiceRegisterHandler, nil) + require.NotEmpty(t, uri) + req = assets.NewJWTAuthentifiedRequest(t, jwtLambda, http.MethodPost, uri, srv) + rec = httptest.NewRecorder() + api.Router.Mux.ServeHTTP(rec, req) + require.Equal(t, 200, rec.Code) + + // Get service with lambda user => 404 + uri = api.Router.GetRoute(http.MethodGet, api.getServiceHandler, map[string]string{ + "type": services.TypeHatchery, + }) + require.NotEmpty(t, uri) + req = assets.NewJWTAuthentifiedRequest(t, jwtLambda, http.MethodGet, uri, data) + rec = httptest.NewRecorder() + api.Router.Mux.ServeHTTP(rec, req) + require.Equal(t, 200, rec.Code) + + var servs []sdk.ServiceConfiguration + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &servs)) + require.Equal(t, 1, len(servs)) + require.NoError(t, services.Delete(api.mustDB(), &srv)) } diff --git a/engine/config.go b/engine/config.go index 0c4d6c1d99..6bd471f2c4 100644 --- a/engine/config.go +++ b/engine/config.go @@ -68,7 +68,7 @@ func configBootstrap(args []string) Configuration { conf.API = &api.Configuration{} conf.API.Name = "cds-api-" + namesgenerator.GetRandomNameCDS(0) defaults.SetDefaults(conf.API) - conf.API.Services = append(conf.API.Services, api.ServiceConfiguration{ + conf.API.Services = append(conf.API.Services, sdk.ServiceConfiguration{ Name: "sample-service", URL: "https://ovh.github.io", Port: "443", diff --git a/engine/worker/internal/handler_service.go b/engine/worker/internal/handler_service.go index bffc905e36..9b833680b0 100644 --- a/engine/worker/internal/handler_service.go +++ b/engine/worker/internal/handler_service.go @@ -15,12 +15,12 @@ func serviceHandler(ctx context.Context, wk *CurrentWorker) http.HandlerFunc { serviceType := vars["type"] log.Debug("Getting service configuration...") - serviceConfig, err := wk.Client().ServiceConfigurationGet(ctx, serviceType) + servicesConfig, err := wk.Client().ServiceConfigurationGet(ctx, serviceType) if err != nil { log.Warning(ctx, "unable to get data: %v", err) writeError(w, r, fmt.Errorf("unable to get service configuration")) } - writeJSON(w, serviceConfig, http.StatusOK) + writeJSON(w, servicesConfig, http.StatusOK) return } } diff --git a/sdk/cdsclient/client_services.go b/sdk/cdsclient/client_services.go index d9a4b28837..5853ffd04e 100644 --- a/sdk/cdsclient/client_services.go +++ b/sdk/cdsclient/client_services.go @@ -33,11 +33,11 @@ func (c *client) ServiceRegister(ctx context.Context, s sdk.Service) (*sdk.Servi return &s, nil } -func (c *client) ServiceConfigurationGet(ctx context.Context, t string) (*sdk.ExternalService, error) { - var serviceConf sdk.ExternalService - _, err := c.GetJSON(ctx, fmt.Sprintf("/services/%s", t), &serviceConf) +func (c *client) ServiceConfigurationGet(ctx context.Context, t string) ([]sdk.ServiceConfiguration, error) { + var servicesConf []sdk.ServiceConfiguration + _, err := c.GetJSON(ctx, fmt.Sprintf("/services/%s", t), &servicesConf) if err != nil { return nil, err } - return &serviceConf, nil + return servicesConf, nil } diff --git a/sdk/cdsclient/interface.go b/sdk/cdsclient/interface.go index 4ff465e66b..c12d2e5d82 100644 --- a/sdk/cdsclient/interface.go +++ b/sdk/cdsclient/interface.go @@ -383,7 +383,7 @@ type WorkerInterface interface { ProjectIntegrationGet(projectKey string, integrationName string, clearPassword bool) (sdk.ProjectIntegration, error) QueueClient Requirements() ([]sdk.Requirement, error) - ServiceConfigurationGet(context.Context, string) (*sdk.ExternalService, error) + ServiceConfigurationGet(context.Context, string) ([]sdk.ServiceConfiguration, error) WorkerClient WorkflowRunArtifacts(projectKey string, name string, number int64) ([]sdk.WorkflowNodeRunArtifact, error) WorkflowCachePush(projectKey, integrationName, ref string, tarContent io.Reader, size int) error diff --git a/sdk/cdsclient/mock_cdsclient/interface_mock.go b/sdk/cdsclient/mock_cdsclient/interface_mock.go index 4f5821f61c..c329318570 100644 --- a/sdk/cdsclient/mock_cdsclient/interface_mock.go +++ b/sdk/cdsclient/mock_cdsclient/interface_mock.go @@ -8350,10 +8350,10 @@ func (mr *MockWorkerInterfaceMockRecorder) Requirements() *gomock.Call { } // ServiceConfigurationGet mocks base method -func (m *MockWorkerInterface) ServiceConfigurationGet(arg0 context.Context, arg1 string) (*sdk.ExternalService, error) { +func (m *MockWorkerInterface) ServiceConfigurationGet(arg0 context.Context, arg1 string) ([]sdk.ServiceConfiguration, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ServiceConfigurationGet", arg0, arg1) - ret0, _ := ret[0].(*sdk.ExternalService) + ret0, _ := ret[0].([]sdk.ServiceConfiguration) ret1, _ := ret[1].(error) return ret0, ret1 } diff --git a/sdk/services.go b/sdk/services.go index 31402cdc61..2e986ec30f 100644 --- a/sdk/services.go +++ b/sdk/services.go @@ -57,20 +57,16 @@ func (c *ServiceConfig) Scan(src interface{}) error { return WrapError(json.Unmarshal(source, c), "cannot unmarshal ServiceConfig") } -// ExternalService represents an external service -type ExternalService struct { - Service `json:"-"` - HealthURL string `json:"health_url"` - HealthPort string `json:"health_port"` - HealthPath string `json:"health_path"` - Port string `json:"port"` - URL string `json:"url"` - Path string `json:"path"` -} - -func (e ExternalService) ServiceConfig() ServiceConfig { - b, _ := json.Marshal(e) - var cfg ServiceConfig - json.Unmarshal(b, &cfg) // nolint - return cfg +// ServiceConfiguration is the configuration of service +type ServiceConfiguration struct { + Name string `toml:"name" json:"name"` + URL string `toml:"url" json:"url"` + Port string `toml:"port" json:"port"` + Path string `toml:"path" json:"path"` + HealthURL string `toml:"healthUrl" json:"health_url"` + HealthPort string `toml:"healthPort" json:"health_port"` + HealthPath string `toml:"healthPath" json:"health_path"` + Type string `toml:"type" json:"type"` + PublicKey string `json:"publicKey"` + ID int64 `json:"id"` }