Skip to content

Commit c108600

Browse files
SkarlsoSkarlso
authored
Return the concrete and refactor the providers out of handlers (#273)
* Return the concrete * Removed providers from handlers and fixed the cyclic import * linter * removed the init handler * Removed the need to initHandler * Fixed user test
1 parent b419af3 commit c108600

23 files changed

+180
-387
lines changed

‎handlers/auth.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,13 @@ func authMiddleware(authCfg *AuthConfig) echo.MiddlewareFunc {
3636
if hasUsername && hasRoles && roles != nil {
3737
// Look through the perms until we find that the user has this permission
3838
if err := authCfg.checkRole(roles, c.Request().Method, c.Path()); err != nil {
39-
return c.String(http.StatusForbidden, fmt.Sprintf("Permission denied for user."))
39+
return c.String(http.StatusForbidden, "Permission denied for user.")
4040
}
4141

4242
username, okUsername := username.(string)
4343
if !okUsername {
4444
gaia.Cfg.Logger.Error("username is not type string")
45-
return c.String(http.StatusInternalServerError, fmt.Sprintf("Unknown error has occured."))
45+
return c.String(http.StatusInternalServerError, "Unknown error has occurred.")
4646
}
4747

4848
// Currently this lives inside the existing auth middleware. Ideally we would have independent
@@ -58,7 +58,7 @@ func authMiddleware(authCfg *AuthConfig) echo.MiddlewareFunc {
5858
return c.String(http.StatusForbidden, err.Error())
5959
}
6060
gaia.Cfg.Logger.Error("rbacEnforcer error", "error", err.Error())
61-
return c.String(http.StatusInternalServerError, fmt.Sprintf("Unknown error has occurred while validating permissions."))
61+
return c.String(http.StatusInternalServerError, "Unknown error has occurred while validating permissions.")
6262
}
6363
}
6464
return next(c)

‎handlers/handler.go

+29-41
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@ import (
88
"github.com/labstack/echo/middleware"
99

1010
"github.com/gaia-pipeline/gaia"
11-
"github.com/gaia-pipeline/gaia/handlers/providers/pipelines"
12-
"github.com/gaia-pipeline/gaia/handlers/providers/workers"
1311
"github.com/gaia-pipeline/gaia/helper/rolehelper"
1412
)
1513

@@ -51,41 +49,36 @@ func (s *GaiaHandler) InitHandlers(e *echo.Echo) error {
5149

5250
// Pipelines
5351
// Create pipeline provider
54-
pipelineProvider := pipelines.NewPipelineProvider(pipelines.Dependencies{
55-
Scheduler: s.deps.Scheduler,
56-
PipelineService: s.deps.PipelineService,
57-
SettingsStore: s.deps.Store,
58-
})
59-
apiAuthGrp.POST("pipeline", pipelineProvider.CreatePipeline)
60-
apiAuthGrp.POST("pipeline/gitlsremote", pipelineProvider.PipelineGitLSRemote)
61-
apiAuthGrp.GET("pipeline/name", pipelineProvider.PipelineNameAvailable)
62-
apiAuthGrp.GET("pipeline/created", pipelineProvider.CreatePipelineGetAll)
63-
apiAuthGrp.GET("pipeline", pipelineProvider.PipelineGetAll)
64-
apiAuthGrp.GET("pipeline/:pipelineid", pipelineProvider.PipelineGet)
65-
apiAuthGrp.PUT("pipeline/:pipelineid", pipelineProvider.PipelineUpdate)
66-
apiAuthGrp.DELETE("pipeline/:pipelineid", pipelineProvider.PipelineDelete)
67-
apiAuthGrp.POST("pipeline/:pipelineid/start", pipelineProvider.PipelineStart)
68-
apiAuthGrp.PUT("pipeline/:pipelineid/reset-trigger-token", pipelineProvider.PipelineResetToken)
69-
apiAuthGrp.POST("pipeline/:pipelineid/pull", pipelineProvider.PipelinePull)
70-
apiAuthGrp.GET("pipeline/latest", pipelineProvider.PipelineGetAllWithLatestRun)
71-
apiAuthGrp.POST("pipeline/periodicschedules", pipelineProvider.PipelineCheckPeriodicSchedules)
72-
apiGrp.POST("pipeline/githook", pipelineProvider.GitWebHook)
73-
apiGrp.POST("pipeline/:pipelineid/:pipelinetoken/trigger", pipelineProvider.PipelineTrigger)
52+
apiAuthGrp.POST("pipeline", s.deps.PipelineProvider.CreatePipeline)
53+
apiAuthGrp.POST("pipeline/gitlsremote", s.deps.PipelineProvider.PipelineGitLSRemote)
54+
apiAuthGrp.GET("pipeline/name", s.deps.PipelineProvider.PipelineNameAvailable)
55+
apiAuthGrp.GET("pipeline/created", s.deps.PipelineProvider.CreatePipelineGetAll)
56+
apiAuthGrp.GET("pipeline", s.deps.PipelineProvider.PipelineGetAll)
57+
apiAuthGrp.GET("pipeline/:pipelineid", s.deps.PipelineProvider.PipelineGet)
58+
apiAuthGrp.PUT("pipeline/:pipelineid", s.deps.PipelineProvider.PipelineUpdate)
59+
apiAuthGrp.DELETE("pipeline/:pipelineid", s.deps.PipelineProvider.PipelineDelete)
60+
apiAuthGrp.POST("pipeline/:pipelineid/start", s.deps.PipelineProvider.PipelineStart)
61+
apiAuthGrp.PUT("pipeline/:pipelineid/reset-trigger-token", s.deps.PipelineProvider.PipelineResetToken)
62+
apiAuthGrp.POST("pipeline/:pipelineid/pull", s.deps.PipelineProvider.PipelinePull)
63+
apiAuthGrp.GET("pipeline/latest", s.deps.PipelineProvider.PipelineGetAllWithLatestRun)
64+
apiAuthGrp.POST("pipeline/periodicschedules", s.deps.PipelineProvider.PipelineCheckPeriodicSchedules)
65+
apiGrp.POST("pipeline/githook", s.deps.PipelineProvider.GitWebHook)
66+
apiGrp.POST("pipeline/:pipelineid/:pipelinetoken/trigger", s.deps.PipelineProvider.PipelineTrigger)
7467

7568
// Settings
7669
settingsHandler := newSettingsHandler(s.deps.Store)
77-
apiAuthGrp.POST("settings/poll/on", pipelineProvider.SettingsPollOn)
78-
apiAuthGrp.POST("settings/poll/off", pipelineProvider.SettingsPollOff)
79-
apiAuthGrp.GET("settings/poll", pipelineProvider.SettingsPollGet)
70+
apiAuthGrp.POST("settings/poll/on", s.deps.PipelineProvider.SettingsPollOn)
71+
apiAuthGrp.POST("settings/poll/off", s.deps.PipelineProvider.SettingsPollOff)
72+
apiAuthGrp.GET("settings/poll", s.deps.PipelineProvider.SettingsPollGet)
8073
apiAuthGrp.GET("settings/rbac", settingsHandler.rbacGet)
8174
apiAuthGrp.PUT("settings/rbac", settingsHandler.rbacPut)
8275

8376
// PipelineRun
84-
apiAuthGrp.POST("pipelinerun/:pipelineid/:runid/stop", pipelineProvider.PipelineStop)
85-
apiAuthGrp.GET("pipelinerun/:pipelineid/:runid", pipelineProvider.PipelineRunGet)
86-
apiAuthGrp.GET("pipelinerun/:pipelineid", pipelineProvider.PipelineGetAllRuns)
87-
apiAuthGrp.GET("pipelinerun/:pipelineid/latest", pipelineProvider.PipelineGetLatestRun)
88-
apiAuthGrp.GET("pipelinerun/:pipelineid/:runid/log", pipelineProvider.GetJobLogs)
77+
apiAuthGrp.POST("pipelinerun/:pipelineid/:runid/stop", s.deps.PipelineProvider.PipelineStop)
78+
apiAuthGrp.GET("pipelinerun/:pipelineid/:runid", s.deps.PipelineProvider.PipelineRunGet)
79+
apiAuthGrp.GET("pipelinerun/:pipelineid", s.deps.PipelineProvider.PipelineGetAllRuns)
80+
apiAuthGrp.GET("pipelinerun/:pipelineid/latest", s.deps.PipelineProvider.PipelineGetLatestRun)
81+
apiAuthGrp.GET("pipelinerun/:pipelineid/:runid/log", s.deps.PipelineProvider.GetJobLogs)
8982

9083
// Secrets
9184
apiAuthGrp.GET("secrets", ListSecrets)
@@ -109,17 +102,12 @@ func (s *GaiaHandler) InitHandlers(e *echo.Echo) error {
109102
}
110103

111104
// Worker
112-
// initialize the worker provider
113-
workerProvider := workers.NewWorkerProvider(workers.Dependencies{
114-
Scheduler: s.deps.Scheduler,
115-
Certificate: s.deps.Certificate,
116-
})
117-
apiAuthGrp.GET("worker/secret", workerProvider.GetWorkerRegisterSecret)
118-
apiAuthGrp.GET("worker/status", workerProvider.GetWorkerStatusOverview)
119-
apiAuthGrp.GET("worker", workerProvider.GetWorker)
120-
apiAuthGrp.DELETE("worker/:workerid", workerProvider.DeregisterWorker)
121-
apiAuthGrp.POST("worker/secret", workerProvider.ResetWorkerRegisterSecret)
122-
apiGrp.POST("worker/register", workerProvider.RegisterWorker)
105+
apiAuthGrp.GET("worker/secret", s.deps.WorkerProvider.GetWorkerRegisterSecret)
106+
apiAuthGrp.GET("worker/status", s.deps.WorkerProvider.GetWorkerStatusOverview)
107+
apiAuthGrp.GET("worker", s.deps.WorkerProvider.GetWorker)
108+
apiAuthGrp.DELETE("worker/:workerid", s.deps.WorkerProvider.DeregisterWorker)
109+
apiAuthGrp.POST("worker/secret", s.deps.WorkerProvider.ResetWorkerRegisterSecret)
110+
apiGrp.POST("worker/register", s.deps.WorkerProvider.RegisterWorker)
123111

124112
// Middleware
125113
e.Use(middleware.Recover())

‎handlers/rbac_test.go

-21
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,6 @@ func (e *mockRBACSvc) DetachRole(username string, role string) error {
6666
}
6767

6868
func Test_rbacHandler_addRole(t *testing.T) {
69-
handlerService := NewGaiaHandler(Dependencies{})
70-
7169
handler := rbacHandler{
7270
svc: &mockRBACSvc{},
7371
}
@@ -79,7 +77,6 @@ func Test_rbacHandler_addRole(t *testing.T) {
7977
}()
8078

8179
e := echo.New()
82-
_ = handlerService.InitHandlers(e)
8380

8481
t.Run("success (200) if add is successful", func(t *testing.T) {
8582
body := `[
@@ -157,8 +154,6 @@ func Test_rbacHandler_addRole(t *testing.T) {
157154
}
158155

159156
func Test_rbacHandler_deleteRole(t *testing.T) {
160-
handlerService := NewGaiaHandler(Dependencies{})
161-
162157
handler := rbacHandler{
163158
svc: &mockRBACSvc{},
164159
}
@@ -170,7 +165,6 @@ func Test_rbacHandler_deleteRole(t *testing.T) {
170165
}()
171166

172167
e := echo.New()
173-
_ = handlerService.InitHandlers(e)
174168

175169
t.Run("success (200) if role is present", func(t *testing.T) {
176170
req := httptest.NewRequest(http.MethodDelete, "/", nil)
@@ -214,8 +208,6 @@ func Test_rbacHandler_deleteRole(t *testing.T) {
214208
}
215209

216210
func Test_rbacHandler_getAllRoles(t *testing.T) {
217-
handlerService := NewGaiaHandler(Dependencies{})
218-
219211
handler := rbacHandler{
220212
svc: &mockRBACSvc{},
221213
}
@@ -227,7 +219,6 @@ func Test_rbacHandler_getAllRoles(t *testing.T) {
227219
}()
228220

229221
e := echo.New()
230-
_ = handlerService.InitHandlers(e)
231222

232223
req := httptest.NewRequest(http.MethodGet, "/", nil)
233224
rec := httptest.NewRecorder()
@@ -241,8 +232,6 @@ func Test_rbacHandler_getAllRoles(t *testing.T) {
241232
}
242233

243234
func Test_rbacHandler_getUserAttachedRoles(t *testing.T) {
244-
handlerService := NewGaiaHandler(Dependencies{})
245-
246235
handler := rbacHandler{
247236
svc: &mockRBACSvc{},
248237
}
@@ -254,7 +243,6 @@ func Test_rbacHandler_getUserAttachedRoles(t *testing.T) {
254243
}()
255244

256245
e := echo.New()
257-
_ = handlerService.InitHandlers(e)
258246

259247
t.Run("success (200) if user is present", func(t *testing.T) {
260248
req := httptest.NewRequest(http.MethodGet, "/", nil)
@@ -298,8 +286,6 @@ func Test_rbacHandler_getUserAttachedRoles(t *testing.T) {
298286
}
299287

300288
func Test_rbacHandler_getRolesAttachedUsers(t *testing.T) {
301-
handlerService := NewGaiaHandler(Dependencies{})
302-
303289
handler := rbacHandler{
304290
svc: &mockRBACSvc{},
305291
}
@@ -311,7 +297,6 @@ func Test_rbacHandler_getRolesAttachedUsers(t *testing.T) {
311297
}()
312298

313299
e := echo.New()
314-
_ = handlerService.InitHandlers(e)
315300

316301
t.Run("success (200) if role is present", func(t *testing.T) {
317302
req := httptest.NewRequest(http.MethodGet, "/", nil)
@@ -355,8 +340,6 @@ func Test_rbacHandler_getRolesAttachedUsers(t *testing.T) {
355340
}
356341

357342
func Test_rbacHandler_attachRole(t *testing.T) {
358-
handlerService := NewGaiaHandler(Dependencies{})
359-
360343
handler := rbacHandler{
361344
svc: &mockRBACSvc{},
362345
}
@@ -368,7 +351,6 @@ func Test_rbacHandler_attachRole(t *testing.T) {
368351
}()
369352

370353
e := echo.New()
371-
_ = handlerService.InitHandlers(e)
372354

373355
t.Run("success (200) if role is present", func(t *testing.T) {
374356
req := httptest.NewRequest(http.MethodPut, "/", nil)
@@ -426,8 +408,6 @@ func Test_rbacHandler_attachRole(t *testing.T) {
426408
}
427409

428410
func Test_rbacHandler_detachRole(t *testing.T) {
429-
handlerService := NewGaiaHandler(Dependencies{})
430-
431411
handler := rbacHandler{
432412
svc: &mockRBACSvc{},
433413
}
@@ -439,7 +419,6 @@ func Test_rbacHandler_detachRole(t *testing.T) {
439419
}()
440420

441421
e := echo.New()
442-
_ = handlerService.InitHandlers(e)
443422

444423
t.Run("success (200) if role is present", func(t *testing.T) {
445424
req := httptest.NewRequest(http.MethodDelete, "/", nil)

‎handlers/service.go

+9-5
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package handlers
22

33
import (
4+
"github.com/gaia-pipeline/gaia/providers/pipelines"
5+
"github.com/gaia-pipeline/gaia/providers/workers"
46
"github.com/gaia-pipeline/gaia/security"
57
"github.com/gaia-pipeline/gaia/security/rbac"
68
"github.com/gaia-pipeline/gaia/store"
@@ -10,11 +12,13 @@ import (
1012

1113
// Dependencies define dependencies for this service.
1214
type Dependencies struct {
13-
Scheduler service.GaiaScheduler
14-
PipelineService pipeline.Service
15-
Certificate security.CAAPI
16-
RBACService rbac.Service
17-
Store store.GaiaStore
15+
Scheduler service.GaiaScheduler
16+
PipelineService pipeline.Servicer
17+
PipelineProvider pipelines.PipelineProviderer
18+
WorkerProvider workers.WorkerProviderer
19+
Certificate security.CAAPI
20+
RBACService rbac.Service
21+
Store store.GaiaStore
1822
}
1923

2024
// GaiaHandler defines handler functions throughout Gaia.

‎handlers/settings_test.go

+12-19
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,25 @@ import (
1313
"github.com/stretchr/testify/assert"
1414

1515
"github.com/gaia-pipeline/gaia"
16-
"github.com/gaia-pipeline/gaia/handlers/providers/pipelines"
16+
"github.com/gaia-pipeline/gaia/providers/pipelines"
1717
"github.com/gaia-pipeline/gaia/workers/pipeline"
18+
"github.com/gaia-pipeline/gaia/workers/scheduler/service"
1819
)
1920

2021
type status struct {
2122
Status bool
2223
}
2324

25+
type mockScheduleService struct {
26+
service.GaiaScheduler
27+
pipelineRun *gaia.PipelineRun
28+
err error
29+
}
30+
31+
func (ms *mockScheduleService) SchedulePipeline(p *gaia.Pipeline, startReason string, args []*gaia.Argument) (*gaia.PipelineRun, error) {
32+
return ms.pipelineRun, ms.err
33+
}
34+
2435
type mockSettingStoreService struct {
2536
get func() (*gaia.StoreConfig, error)
2637
put func(*gaia.StoreConfig) error
@@ -50,11 +61,6 @@ func TestSetPollerToggle(t *testing.T) {
5061
Scheduler: &mockScheduleService{},
5162
})
5263

53-
handlerService := NewGaiaHandler(Dependencies{
54-
Scheduler: &mockScheduleService{},
55-
PipelineService: pipelineService,
56-
})
57-
5864
get := func() (*gaia.StoreConfig, error) {
5965
return nil, nil
6066
}
@@ -70,7 +76,6 @@ func TestSetPollerToggle(t *testing.T) {
7076
})
7177
// // Initialize echo
7278
e := echo.New()
73-
_ = handlerService.InitHandlers(e)
7479

7580
t.Run("switching it on twice should fail", func(t2 *testing.T) {
7681
req := httptest.NewRequest(echo.POST, "/", nil)
@@ -203,11 +208,6 @@ func TestGettingSettingFromDBTakesPrecedence(t *testing.T) {
203208
Scheduler: &mockScheduleService{},
204209
})
205210

206-
handlerService := NewGaiaHandler(Dependencies{
207-
Scheduler: &mockScheduleService{},
208-
PipelineService: pipelineService,
209-
})
210-
211211
get := func() (*gaia.StoreConfig, error) {
212212
return &gaia.StoreConfig{
213213
Poll: true,
@@ -225,7 +225,6 @@ func TestGettingSettingFromDBTakesPrecedence(t *testing.T) {
225225
})
226226

227227
e := echo.New()
228-
_ = handlerService.InitHandlers(e)
229228

230229
req := httptest.NewRequest(echo.GET, "/", nil)
231230
req.Header.Set("Content-Type", "application/json")
@@ -261,11 +260,6 @@ func TestSettingPollerOnAlsoSavesSettingsInDB(t *testing.T) {
261260
Scheduler: &mockScheduleService{},
262261
})
263262

264-
handlerService := NewGaiaHandler(Dependencies{
265-
Scheduler: &mockScheduleService{},
266-
PipelineService: pipelineService,
267-
})
268-
269263
get := func() (*gaia.StoreConfig, error) {
270264
return &gaia.StoreConfig{
271265
Poll: true,
@@ -285,7 +279,6 @@ func TestSettingPollerOnAlsoSavesSettingsInDB(t *testing.T) {
285279
})
286280

287281
e := echo.New()
288-
_ = handlerService.InitHandlers(e)
289282

290283
req := httptest.NewRequest(echo.POST, "/", nil)
291284
req.Header.Set("Content-Type", "application/json")

0 commit comments

Comments
 (0)