diff --git a/base/core/gintesting.go b/base/core/gintesting.go deleted file mode 100644 index ec5f06def..000000000 --- a/base/core/gintesting.go +++ /dev/null @@ -1,56 +0,0 @@ -package core - -import ( - "app/base/utils" - "app/manager/middlewares" - - "github.com/gin-gonic/gin" -) - -const LatestAPIVersion = 3 - -type ContextKV struct { - Key string - Value any -} - -var V3APICtx = ContextKV{Key: utils.KeyApiver, Value: 3} - -var WorkspacesTestCtx = ContextKV{ - Key: utils.KeyInventoryWorkspaces, - Value: []string{ - "00000000-0000-0000-0000-000000000001", - "00000000-0000-0000-0000-000000000002", - "00000000-0000-0000-0000-999999999999", - }, -} - -func InitRouter(handler gin.HandlerFunc, contextKVs ...ContextKV) *gin.Engine { - return InitRouterWithPath(handler, "/", contextKVs...) -} - -func InitRouterWithParams(handler gin.HandlerFunc, account int, method, path string, - contextKVs ...ContextKV) *gin.Engine { - router := gin.Default() - router.Use(middlewares.RequestResponseLogger()) - router.Use(middlewares.MockAuthenticator(account)) - router.Use(middlewares.DatabaseWithContext()) - router.Use(func(c *gin.Context) { - // set default api version for tests to latest - c.Set(utils.KeyApiver, LatestAPIVersion) - c.Set(utils.KeyInventoryWorkspaces, WorkspacesTestCtx.Value) - for _, kv := range contextKVs { - c.Set(kv.Key, kv.Value) - } - }) - router.Handle(method, path, handler) - return router -} - -func InitRouterWithPath(handler gin.HandlerFunc, path string, contextKVs ...ContextKV) *gin.Engine { - return InitRouterWithParams(handler, 1, "GET", path, contextKVs...) -} - -func InitRouterWithAccount(handler gin.HandlerFunc, path string, account int, contextKVs ...ContextKV) *gin.Engine { - return InitRouterWithParams(handler, account, "GET", path, contextKVs...) -} diff --git a/base/core/probes_test.go b/base/core/probes_test.go index 93223c246..89af7bb89 100644 --- a/base/core/probes_test.go +++ b/base/core/probes_test.go @@ -6,9 +6,16 @@ import ( "net/http/httptest" "testing" + "github.com/gin-gonic/gin" "github.com/stretchr/testify/assert" ) +func InitRouter(handler gin.HandlerFunc) *gin.Engine { + router := gin.New() + router.GET("/", handler) + return router +} + func TestLiveness(t *testing.T) { w := httptest.NewRecorder() req, _ := http.NewRequest("GET", "/", nil) diff --git a/manager/controllers/advisories.go b/manager/controllers/advisories.go index 344679ca2..f6f3fc25e 100644 --- a/manager/controllers/advisories.go +++ b/manager/controllers/advisories.go @@ -80,7 +80,7 @@ type AdvisoriesResponse struct { } func advisoriesCommon(c *gin.Context) (*gorm.DB, *ListMeta, []string, error) { - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) account := c.GetInt(utils.KeyAccount) workspaceIDs := c.GetStringSlice(utils.KeyInventoryWorkspaces) var query *gorm.DB diff --git a/manager/controllers/advisories_export.go b/manager/controllers/advisories_export.go index 3d3da055d..b6af4933e 100644 --- a/manager/controllers/advisories_export.go +++ b/manager/controllers/advisories_export.go @@ -34,7 +34,7 @@ func AdvisoriesExportHandler(c *gin.Context) { if err != nil { return } - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) query, err := resolveAdvisoriesQuery(db, account, workspaceIDs, filters) if err != nil { diff --git a/manager/controllers/advisory_detail.go b/manager/controllers/advisory_detail.go index 3fea2c2b7..fc673a6ad 100644 --- a/manager/controllers/advisory_detail.go +++ b/manager/controllers/advisory_detail.go @@ -77,7 +77,7 @@ func AdvisoryDetailHandler(c *gin.Context) { var err error var resp *AdvisoryDetailResponse - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) resp, err = getAdvisory(db, advisoryName, false) if err != nil { if errors.Is(err, gorm.ErrRecordNotFound) { diff --git a/manager/controllers/advisory_detail_test.go b/manager/controllers/advisory_detail_test.go index 2d9d9f9e2..961f1d02e 100644 --- a/manager/controllers/advisory_detail_test.go +++ b/manager/controllers/advisory_detail_test.go @@ -14,7 +14,7 @@ import ( func TestAdvisoryDetailDefault(t *testing.T) { core.SetupTest(t) - w := CreateRequestRouterWithPath("GET", "/:advisory_id", "RH-9", "", nil, "", AdvisoryDetailHandler, core.V3APICtx) + w := CreateRequestRouterWithPath("GET", "/:advisory_id", "RH-9", "", nil, "", AdvisoryDetailHandler, V3APICtx) var output AdvisoryDetailResponse CheckResponse(t, w, http.StatusOK, &output) @@ -43,7 +43,7 @@ func (r *AdvisoryDetailResponse) checkRH9Fields(t *testing.T) { func TestAdvisoryDetailCVE(t *testing.T) { core.SetupTest(t) - w := CreateRequestRouterWithPath("GET", "/:advisory_id", "RH-3", "", nil, "", AdvisoryDetailHandler, core.V3APICtx) + w := CreateRequestRouterWithPath("GET", "/:advisory_id", "RH-3", "", nil, "", AdvisoryDetailHandler, V3APICtx) var output AdvisoryDetailResponse CheckResponse(t, w, http.StatusOK, &output) @@ -56,13 +56,13 @@ func TestAdvisoryNoIdProvided(t *testing.T) { core.SetupTest(t) var errResp utils.ErrorResponse w := CreateRequest("GET", "/", nil, "", AdvisoryDetailHandler, - core.V3APICtx) + V3APICtx) CheckResponse(t, w, http.StatusBadRequest, &errResp) assert.Equal(t, "advisory_id param not found", errResp.Error) w = CreateRequest("GET", "/", nil, "", AdvisoryDetailHandler, - core.V3APICtx) + V3APICtx) CheckResponse(t, w, http.StatusBadRequest, &errResp) assert.Equal(t, "advisory_id param not found", errResp.Error) } @@ -71,12 +71,12 @@ func TestAdvisoryNotFound(t *testing.T) { core.SetupTest(t) var errResp utils.ErrorResponse - w := CreateRequestRouterWithPath("GET", "/:advisory_id", "foo", "", nil, "", AdvisoryDetailHandler, core.V3APICtx) + w := CreateRequestRouterWithPath("GET", "/:advisory_id", "foo", "", nil, "", AdvisoryDetailHandler, V3APICtx) CheckResponse(t, w, http.StatusNotFound, &errResp) assert.Equal(t, "advisory not found", errResp.Error) - w = CreateRequestRouterWithPath("GET", "/:advisory_id", "foo", "", nil, "", AdvisoryDetailHandler, core.V3APICtx) + w = CreateRequestRouterWithPath("GET", "/:advisory_id", "foo", "", nil, "", AdvisoryDetailHandler, V3APICtx) CheckResponse(t, w, http.StatusNotFound, &errResp) assert.Equal(t, "advisory not found", errResp.Error) @@ -84,7 +84,7 @@ func TestAdvisoryNotFound(t *testing.T) { func testReq() *httptest.ResponseRecorder { return CreateRequestRouterWithPath("GET", "/:advisory_id", "RH-9", "", nil, "", AdvisoryDetailHandler, - core.V3APICtx) + V3APICtx) } func TestAdvisoryDetailCached(t *testing.T) { @@ -121,13 +121,13 @@ func TestAdvisoryDetailFiltering(t *testing.T) { var errResp utils.ErrorResponse w := CreateRequestRouterWithPath("GET", "/:advisory_id", "RH-9", "?filter[filter]=abcd", nil, "", - AdvisoryDetailHandler, core.V3APICtx) + AdvisoryDetailHandler, V3APICtx) CheckResponse(t, w, http.StatusBadRequest, &errResp) assert.Equal(t, FilterNotSupportedMsg, errResp.Error) w = CreateRequestRouterWithPath("GET", "/:advisory_id", "RH-9", "?filter[filter]=abcd", nil, "", - AdvisoryDetailHandler, core.V3APICtx) + AdvisoryDetailHandler, V3APICtx) CheckResponse(t, w, http.StatusBadRequest, &errResp) assert.Equal(t, FilterNotSupportedMsg, errResp.Error) diff --git a/manager/controllers/advisory_systems.go b/manager/controllers/advisory_systems.go index 7aec2e518..6bf20be1f 100644 --- a/manager/controllers/advisory_systems.go +++ b/manager/controllers/advisory_systems.go @@ -80,7 +80,7 @@ func advisorySystemsCommon(c *gin.Context) (*gorm.DB, *ListMeta, []string, error return nil, nil, nil, errors.New("advisory_id param not found") } - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) var exists int64 err := db.Model(&models.AdvisoryMetadata{}). Where("name = ? ", advisoryName).Count(&exists).Error diff --git a/manager/controllers/advisory_systems_export.go b/manager/controllers/advisory_systems_export.go index e0744b6dc..f9488b336 100644 --- a/manager/controllers/advisory_systems_export.go +++ b/manager/controllers/advisory_systems_export.go @@ -66,7 +66,7 @@ func AdvisorySystemsExportHandler(c *gin.Context) { return } - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) var exists int64 err := db.Model(&models.AdvisoryMetadata{}). Where("name = ? ", advisoryName).Count(&exists).Error diff --git a/manager/controllers/package_detail.go b/manager/controllers/package_detail.go index 69a801f83..f4a20e119 100644 --- a/manager/controllers/package_detail.go +++ b/manager/controllers/package_detail.go @@ -43,7 +43,7 @@ type PackageDetailAttributes struct { var PackageSelect = database.MustGetSelect(&PackageDetailAttributes{}) func packageLatestHandler(c *gin.Context, packageName string) { - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) query := database.PackageByName(db, packageName, database.JoinPackageDetails) var pkg PackageDetailAttributes // Perform 'soft-filtering' by ordering on boolean column first @@ -66,7 +66,7 @@ func packageLatestHandler(c *gin.Context, packageName string) { } func packageEvraHandler(c *gin.Context, nevra *utils.Nevra) { - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) if !packageNameIsValid(db, nevra.Name) { c.JSON(http.StatusBadRequest, utils.ErrorResponse{Error: "invalid package name"}) return @@ -112,7 +112,7 @@ func PackageDetailHandler(c *gin.Context) { return } - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) if packageNameIsValid(db, parameter) { packageLatestHandler(c, parameter) return diff --git a/manager/controllers/package_systems.go b/manager/controllers/package_systems.go index 708cd38f1..d9d3fea8e 100644 --- a/manager/controllers/package_systems.go +++ b/manager/controllers/package_systems.go @@ -129,7 +129,7 @@ func packageSystemsCommon(db *gorm.DB, c *gin.Context) (*gorm.DB, *ListMeta, []s // @Failure 500 {object} utils.ErrorResponse // @Router /packages/{package_name}/systems [get] func PackageSystemsListHandler(c *gin.Context) { - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) query, meta, params, err := packageSystemsCommon(db, c) if err != nil { @@ -187,7 +187,7 @@ func PackageSystemsListHandler(c *gin.Context) { // @Failure 500 {object} utils.ErrorResponse // @Router /ids/packages/{package_name}/systems [get] func PackageSystemsListIDsHandler(c *gin.Context) { - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) query, meta, _, err := packageSystemsCommon(db, c) if err != nil { return diff --git a/manager/controllers/package_systems_export.go b/manager/controllers/package_systems_export.go index 1a8043ea0..8effb26f6 100644 --- a/manager/controllers/package_systems_export.go +++ b/manager/controllers/package_systems_export.go @@ -46,7 +46,7 @@ func PackageSystemsExportHandler(c *gin.Context) { return } - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) var packageIDs []int if err := database.PackageByName(db, packageName).Pluck("p.id", &packageIDs).Error; err != nil { utils.LogAndRespError(c, err, "database error") diff --git a/manager/controllers/package_versions.go b/manager/controllers/package_versions.go index 07df60435..a6537429c 100644 --- a/manager/controllers/package_versions.go +++ b/manager/controllers/package_versions.go @@ -77,7 +77,7 @@ func PackageVersionsListHandler(c *gin.Context) { return } - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) var packageNameIDs []int if err := packagesNameID(db, packageName).Pluck("pn.id", &packageNameIDs).Error; err != nil { utils.LogAndRespError(c, err, "database error") diff --git a/manager/controllers/packages.go b/manager/controllers/packages.go index fa66dff19..3e72676db 100644 --- a/manager/controllers/packages.go +++ b/manager/controllers/packages.go @@ -126,7 +126,7 @@ func PackagesListHandler(c *gin.Context) { return } - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) useCache := shouldUseCache(db, account, filters, workspaceIDs) if !useCache { db.Exec("SET work_mem TO '?'", utils.CoreCfg.DBWorkMem) diff --git a/manager/controllers/packages_export.go b/manager/controllers/packages_export.go index e027af7b0..dc24f04aa 100644 --- a/manager/controllers/packages_export.go +++ b/manager/controllers/packages_export.go @@ -33,7 +33,7 @@ func PackagesExportHandler(c *gin.Context) { return } - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) useCache := shouldUseCache(db, account, filters, workspaceIDs) if !useCache { db.Exec("SET work_mem TO '?'", utils.CoreCfg.DBWorkMem) diff --git a/manager/controllers/status.go b/manager/controllers/status.go index be3e1e660..a6373b10b 100644 --- a/manager/controllers/status.go +++ b/manager/controllers/status.go @@ -1,8 +1,8 @@ package controllers import ( - "app/base/database" "app/base/utils" + "app/manager/middlewares" "net/http" "github.com/gin-gonic/gin" @@ -12,7 +12,11 @@ import ( // @Success 200 {int} http.StatusOK // @Failure 503 {object} utils.ErrorResponse func Status(c *gin.Context) { - sqlDB, _ := database.DB(c.Request.Context()).DB() + sqlDB, err := middlewares.DBWriter(c).DB() + if err != nil { + utils.LogAndRespStatusError(c, http.StatusServiceUnavailable, err, "Database not connected") + return + } if err := sqlDB.Ping(); err != nil { utils.LogAndRespStatusError(c, http.StatusServiceUnavailable, err, "Database not connected") } else { diff --git a/manager/controllers/system_advisories.go b/manager/controllers/system_advisories.go index f04fdd177..02a0499d5 100644 --- a/manager/controllers/system_advisories.go +++ b/manager/controllers/system_advisories.go @@ -86,7 +86,7 @@ func systemAdvisoriesCommon(c *gin.Context) (*gorm.DB, *ListMeta, []string, erro return nil, nil, nil, err } // Error handled method itself - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) var exists int64 err = db.Model(&models.SystemInventory{}).Where("inventory_id = ?", inventoryID). Count(&exists).Error diff --git a/manager/controllers/system_advisories_export.go b/manager/controllers/system_advisories_export.go index 057e486cd..8b6d206bb 100644 --- a/manager/controllers/system_advisories_export.go +++ b/manager/controllers/system_advisories_export.go @@ -42,7 +42,7 @@ func SystemAdvisoriesExportHandler(c *gin.Context) { return } - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) var exists int64 err = db.Model(&models.SystemInventory{}).Where("inventory_id = ?", inventoryID). Count(&exists).Error diff --git a/manager/controllers/system_detail.go b/manager/controllers/system_detail.go index 29d8e4c8a..b3027ee03 100644 --- a/manager/controllers/system_detail.go +++ b/manager/controllers/system_detail.go @@ -58,7 +58,7 @@ func SystemDetailHandler(c *gin.Context) { } var systemDetail SystemDetailLookup - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) query := database.Systems(db, account, workspaceIDs, database.JoinTemplates). Select(database.MustGetSelect(&systemDetail)). Where("si.inventory_id = ?", inventoryID) @@ -163,7 +163,7 @@ func systemJSONsCommon(c *gin.Context, column string) *models.SystemInventory { } var system models.SystemInventory - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) query := database.Systems(db, account, workspaceIDs). Select(column). Where("si.inventory_id = ?", inventoryID) diff --git a/manager/controllers/system_packages.go b/manager/controllers/system_packages.go index 5335248e8..bafbff0a8 100644 --- a/manager/controllers/system_packages.go +++ b/manager/controllers/system_packages.go @@ -103,7 +103,7 @@ func SystemPackagesHandler(c *gin.Context) { return } var loaded []SystemPackageDBLoad - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) q := systemPackageQuery(db, account, workspaceIDs, inventoryID) q, meta, params, err := ListCommon(q, c, filters, SystemPackagesOpts) if err != nil { diff --git a/manager/controllers/system_packages_export.go b/manager/controllers/system_packages_export.go index 51d514ac4..2dab02d81 100644 --- a/manager/controllers/system_packages_export.go +++ b/manager/controllers/system_packages_export.go @@ -46,7 +46,7 @@ func SystemPackagesExportHandler(c *gin.Context) { } var loaded []SystemPackageDBLoad - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) q := systemPackageQuery(db, account, workspaceIDs, inventoryID) q, err = ExportListCommon(q, c, SystemPackagesOpts) if err != nil { diff --git a/manager/controllers/systems.go b/manager/controllers/systems.go index 7bcc30ca0..607c62674 100644 --- a/manager/controllers/systems.go +++ b/manager/controllers/systems.go @@ -195,7 +195,7 @@ func validateSystemsListIDs(ids []string) error { func systemsCommon(c *gin.Context, ids []string) (*gorm.DB, *ListMeta, []string, error) { account := c.GetInt(utils.KeyAccount) workspaceIDs := c.GetStringSlice(utils.KeyInventoryWorkspaces) - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) query := querySystems(db, account, workspaceIDs) filters, err := ParseAllFilters(c, SystemOpts) if err != nil { diff --git a/manager/controllers/systems_advisories_view.go b/manager/controllers/systems_advisories_view.go index 0423b6b82..c2aed7664 100644 --- a/manager/controllers/systems_advisories_view.go +++ b/manager/controllers/systems_advisories_view.go @@ -68,7 +68,7 @@ var advisoriesSystemsViewOpts = ListOpts{ func totalItems(c *gin.Context, tx *gorm.DB, cols string) (int, error) { var count int64 - err := middlewares.DBFromContext(c).Table("(?) AS cq", tx.Select(cols)).Count(&count).Error + err := middlewares.DB(c).Table("(?) AS cq", tx.Select(cols)).Count(&count).Error return int(count), err } @@ -181,7 +181,7 @@ func queryDB(c *gin.Context, endpoint string) ([]systemsAdvisoriesDBLoad, *ListM } acc := c.GetInt(utils.KeyAccount) workspaceIDs := c.GetStringSlice(utils.KeyInventoryWorkspaces) - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) // backward compatibility, put limit/offset from json into querystring if req.Limit != nil { c.Request.URL.RawQuery += fmt.Sprintf("&limit=%d", *req.Limit) diff --git a/manager/controllers/systems_export.go b/manager/controllers/systems_export.go index 0dde06d7f..668bebc5c 100644 --- a/manager/controllers/systems_export.go +++ b/manager/controllers/systems_export.go @@ -62,7 +62,7 @@ import ( func SystemsExportHandler(c *gin.Context) { account := c.GetInt(utils.KeyAccount) workspaceIDs := c.GetStringSlice(utils.KeyInventoryWorkspaces) - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) query := querySystems(db, account, workspaceIDs) filters, err := ParseAllFilters(c, SystemOpts) if err != nil { diff --git a/manager/controllers/systemtags.go b/manager/controllers/systemtags.go index 3ef2fdafd..04ecd5ea1 100644 --- a/manager/controllers/systemtags.go +++ b/manager/controllers/systemtags.go @@ -74,7 +74,7 @@ func SystemTagListHandler(c *gin.Context) { account := c.GetInt(utils.KeyAccount) workspaceIDs := c.GetStringSlice(utils.KeyInventoryWorkspaces) - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) // https://stackoverflow.com/questions/33474778/how-to-group-result-by-array-column-in-postgres sq := database.Systems(db, account, workspaceIDs). Select("jsonb_array_elements(si.tags) AS tag") diff --git a/manager/controllers/template_subscribed_systems_update.go b/manager/controllers/template_subscribed_systems_update.go index 7c785bdff..dd3e81147 100644 --- a/manager/controllers/template_subscribed_systems_update.go +++ b/manager/controllers/template_subscribed_systems_update.go @@ -29,7 +29,7 @@ func TemplateSubscribedSystemsUpdateHandler(c *gin.Context) { templateUUID := c.Param("template_id") workspaceIDs := c.GetStringSlice(utils.KeyInventoryWorkspaces) - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) account, orgID, systemUUID, err := getSubscribedSystem(c, db) if err != nil { diff --git a/manager/controllers/template_subscribed_systems_update_test.go b/manager/controllers/template_subscribed_systems_update_test.go index 6f1e8416d..5b6ca106f 100644 --- a/manager/controllers/template_subscribed_systems_update_test.go +++ b/manager/controllers/template_subscribed_systems_update_test.go @@ -54,8 +54,8 @@ func TestUpdateTemplateSubscribedSystems(t *testing.T) { w := CreateRequestRouterWithParams("PATCH", "/:template_id/subscribed-systems", templateUUID, "", nil, "", TemplateSubscribedSystemsUpdateHandler, templateAccount, - core.ContextKV{Key: utils.KeySystem, Value: subscriptionUUID}, - core.ContextKV{Key: utils.KeyOrgID, Value: orgID}) + ContextKV{Key: utils.KeySystem, Value: subscriptionUUID}, + ContextKV{Key: utils.KeyOrgID, Value: orgID}) assert.Equal(t, http.StatusOK, w.Code) database.CheckTemplateSystems(t, templateAccount, templateUUID, []uuid.UUID{testInventoryID4}) @@ -69,7 +69,7 @@ func TestUpdateTemplateSubscribedSystemsInvalid(t *testing.T) { w := CreateRequestRouterWithParams("PATCH", "/:template_id/subscribed-systems", templateUUID, "", nil, "", TemplateSubscribedSystemsUpdateHandler, templateAccount, - core.ContextKV{Key: utils.KeySystem, Value: subscriptionInvalidUUID}) + ContextKV{Key: utils.KeySystem, Value: subscriptionInvalidUUID}) assert.Equal(t, http.StatusNotFound, w.Code) database.CheckTemplateSystems(t, templateAccount, templateUUID, []uuid.UUID{}) diff --git a/manager/controllers/template_systems.go b/manager/controllers/template_systems.go index d5ff3de70..247cb7359 100644 --- a/manager/controllers/template_systems.go +++ b/manager/controllers/template_systems.go @@ -79,7 +79,7 @@ func getTemplate(c *gin.Context, tx *gorm.DB, account int, uuid string) (*models func templateSystemsQuery(c *gin.Context, account int, workspaceIDs []string) (*gorm.DB, Filters, error) { templateUUID := c.Param("template_id") - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) template, err := getTemplate(c, db, account, templateUUID) if err != nil { diff --git a/manager/controllers/template_systems_delete.go b/manager/controllers/template_systems_delete.go index d28da56fd..a9a0c6ee0 100644 --- a/manager/controllers/template_systems_delete.go +++ b/manager/controllers/template_systems_delete.go @@ -37,7 +37,7 @@ func TemplateSystemsDeleteHandler(c *gin.Context) { return } - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) err := checkTemplateSystems(c, db, account, nil, req.Systems, workspaceIDs) if err != nil { diff --git a/manager/controllers/template_systems_update.go b/manager/controllers/template_systems_update.go index 4ccc34d97..1d5d07e76 100644 --- a/manager/controllers/template_systems_update.go +++ b/manager/controllers/template_systems_update.go @@ -69,7 +69,7 @@ func TemplateSystemsUpdateHandler(c *gin.Context) { return } - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) template, err := getTemplate(c, db, account, templateUUID) if err != nil { // respose set in getTemplateID() diff --git a/manager/controllers/template_systems_update_test.go b/manager/controllers/template_systems_update_test.go index 1d86e9358..41fd306c9 100644 --- a/manager/controllers/template_systems_update_test.go +++ b/manager/controllers/template_systems_update_test.go @@ -240,7 +240,7 @@ func TestUpdateTemplateSystemsCandlepin404(t *testing.T) { }) defer database.DeleteTemplate(t, templateAccount, templateUUID) w := CreateRequestRouterWithParams("PUT", templatePath, templateUUID, "", bytes.NewBufferString(data), "", - TemplateSystemsUpdateHandler, templateAccount, core.ContextKV{Key: utils.KeyOrgID, Value: orgID}) + TemplateSystemsUpdateHandler, templateAccount, ContextKV{Key: utils.KeyOrgID, Value: orgID}) assert.Equal(t, http.StatusBadRequest, w.Code) database.CheckTemplateSystems(t, templateAccount, templateUUID, @@ -254,7 +254,7 @@ func TestUpdateTemplateSystemsCandlepin404(t *testing.T) { ] }` w = CreateRequestRouterWithParams("PUT", templatePath, templateUUID, "", bytes.NewBufferString(data), "", - TemplateSystemsUpdateHandler, templateAccount, core.ContextKV{Key: utils.KeyOrgID, Value: orgID}) + TemplateSystemsUpdateHandler, templateAccount, ContextKV{Key: utils.KeyOrgID, Value: orgID}) assert.Equal(t, http.StatusBadRequest, w.Code) database.CheckTemplateSystems(t, templateAccount, templateUUID, diff --git a/manager/controllers/templates.go b/manager/controllers/templates.go index d58a92306..9dd69c14f 100644 --- a/manager/controllers/templates.go +++ b/manager/controllers/templates.go @@ -83,7 +83,7 @@ func TemplatesListHandler(c *gin.Context) { return } - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) query := templatesQuery(db, filters, account, workspaceIDs) query, meta, params, err := ListCommon(query, c, filters, TemplateOpts) diff --git a/manager/controllers/test_utils.go b/manager/controllers/test_utils.go index 83e1d938e..77d9372b7 100644 --- a/manager/controllers/test_utils.go +++ b/manager/controllers/test_utils.go @@ -1,7 +1,6 @@ package controllers import ( - "app/base/core" "encoding/json" "io" "net/http" @@ -9,12 +8,18 @@ import ( "regexp" "testing" + "app/base/utils" + "app/manager/middlewares" + "github.com/gin-gonic/gin" "github.com/google/uuid" "github.com/stretchr/testify/assert" ) const InvalidContentTypeErr = `{"error":"Invalid content type 'test-format', use 'application/json' or 'text/csv'"}` +const LatestAPIVersion = 3 + +var V3APICtx = ContextKV{Key: utils.KeyApiver, Value: 3} var ( testInventoryID1 = uuid.MustParse("00000000-0000-0000-0000-000000000001") @@ -22,6 +27,49 @@ var ( testInventoryID4 = uuid.MustParse("00000000-0000-0000-0000-000000000004") ) +type ContextKV struct { + Key string + Value any +} + +var WorkspacesTestCtx = ContextKV{ + Key: utils.KeyInventoryWorkspaces, + Value: []string{ + "00000000-0000-0000-0000-000000000001", + "00000000-0000-0000-0000-000000000002", + "00000000-0000-0000-0000-999999999999", + }, +} + +func InitRouter(handler gin.HandlerFunc, contextKVs ...ContextKV) *gin.Engine { + return InitRouterWithPath(handler, "/", contextKVs...) +} + +func InitRouterWithParams(handler gin.HandlerFunc, account int, method, path string, + contextKVs ...ContextKV) *gin.Engine { + router := gin.Default() + router.Use(middlewares.RequestResponseLogger()) + router.Use(middlewares.MockAuthenticator(account)) + router.Use(func(c *gin.Context) { + // set default api version for tests to latest + c.Set(utils.KeyApiver, LatestAPIVersion) + c.Set(utils.KeyInventoryWorkspaces, WorkspacesTestCtx.Value) + for _, kv := range contextKVs { + c.Set(kv.Key, kv.Value) + } + }) + router.Handle(method, path, handler) + return router +} + +func InitRouterWithPath(handler gin.HandlerFunc, path string, contextKVs ...ContextKV) *gin.Engine { + return InitRouterWithParams(handler, 1, "GET", path, contextKVs...) +} + +func InitRouterWithAccount(handler gin.HandlerFunc, path string, account int, contextKVs ...ContextKV) *gin.Engine { + return InitRouterWithParams(handler, account, "GET", path, contextKVs...) +} + func ParseResponseBody(t *testing.T, bytes []byte, out interface{}) { // don't use sonic.Unmarshal as some tests receive empty output err := json.Unmarshal(bytes, &out) @@ -51,34 +99,34 @@ func prepareRequest(method string, url string, body io.Reader, ct string) (w *ht // Create simple request func CreateRequest(method string, url string, body io.Reader, contentType string, handler gin.HandlerFunc, - contextKVs ...core.ContextKV) ( + contextKVs ...ContextKV) ( w *httptest.ResponseRecorder) { w, req := prepareRequest(method, url, body, contentType) - core.InitRouter(handler, contextKVs...).ServeHTTP(w, req) + InitRouter(handler, contextKVs...).ServeHTTP(w, req) return w } // Create request and initialize router with params func CreateRequestRouterWithParams(method, routerPath, param, queryString string, body io.Reader, contentType string, - handler gin.HandlerFunc, routerAccount int, contextKVs ...core.ContextKV) (w *httptest.ResponseRecorder) { + handler gin.HandlerFunc, routerAccount int, contextKVs ...ContextKV) (w *httptest.ResponseRecorder) { w, req := prepareRequest(method, getURLFromRouterPath(routerPath, param, queryString), body, contentType) - core.InitRouterWithParams(handler, routerAccount, method, routerPath, contextKVs...).ServeHTTP(w, req) + InitRouterWithParams(handler, routerAccount, method, routerPath, contextKVs...).ServeHTTP(w, req) return w } // Create request and initialize router with path func CreateRequestRouterWithPath(method, routerPath, param, queryString string, body io.Reader, contentType string, - handler gin.HandlerFunc, contextKVs ...core.ContextKV) (w *httptest.ResponseRecorder) { + handler gin.HandlerFunc, contextKVs ...ContextKV) (w *httptest.ResponseRecorder) { w, req := prepareRequest(method, getURLFromRouterPath(routerPath, param, queryString), body, contentType) - core.InitRouterWithPath(handler, routerPath, contextKVs...).ServeHTTP(w, req) + InitRouterWithPath(handler, routerPath, contextKVs...).ServeHTTP(w, req) return w } // Create request and initialize router with account func CreateRequestRouterWithAccount(method, routerPath, param, queryString string, body io.Reader, contentType string, - handler gin.HandlerFunc, routerAccount int, contextKVs ...core.ContextKV) (w *httptest.ResponseRecorder) { + handler gin.HandlerFunc, routerAccount int, contextKVs ...ContextKV) (w *httptest.ResponseRecorder) { w, req := prepareRequest(method, getURLFromRouterPath(routerPath, param, queryString), body, contentType) - core.InitRouterWithAccount(handler, routerPath, routerAccount, contextKVs...).ServeHTTP(w, req) + InitRouterWithAccount(handler, routerPath, routerAccount, contextKVs...).ServeHTTP(w, req) return w } diff --git a/manager/controllers/utils.go b/manager/controllers/utils.go index acaf33ec0..7dc94207c 100644 --- a/manager/controllers/utils.go +++ b/manager/controllers/utils.go @@ -227,7 +227,7 @@ func UpdateMetaLinks(c *gin.Context, meta *ListMeta, total int, subTotals map[st if total == 0 { var hasSystems bool account := c.GetInt(utils.KeyAccount) - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) db.Raw("SELECT EXISTS (SELECT 1 FROM system_inventory WHERE rh_account_id = ?)", account).Scan(&hasSystems) meta.HasSystems = &hasSystems } diff --git a/manager/middlewares/db.go b/manager/middlewares/db.go index a4daff6ac..add5801be 100644 --- a/manager/middlewares/db.go +++ b/manager/middlewares/db.go @@ -9,24 +9,18 @@ import ( "gorm.io/gorm" ) -const DBKey = "DB" -const DBReadReplicaKey = "DBReadReplica" - -// Apply gin context to database so queries within context are canceled when request is aborted -func DatabaseWithContext() gin.HandlerFunc { - return func(c *gin.Context) { - c.Set(DBKey, database.DB(c.Request.Context())) - c.Set(DBReadReplicaKey, database.DBReadReplica(c.Request.Context())) - c.Next() +// DB returns a handle bound to the current request context. +// GET uses the read replica when one is configured. +func DB(c *gin.Context) *gorm.DB { + if useReadReplica(c) { + return database.DBReadReplica(c.Request.Context()) } + return database.DB(c.Request.Context()) } -// DB handler stored in request context -func DBFromContext(c *gin.Context) *gorm.DB { - if useReadReplica(c) { - return c.MustGet(DBReadReplicaKey).(*gorm.DB) - } - return c.MustGet(DBKey).(*gorm.DB) +// DBWriter returns the primary handle bound to the current request context. +func DBWriter(c *gin.Context) *gorm.DB { + return database.DB(c.Request.Context()) } func useReadReplica(c *gin.Context) bool { diff --git a/manager/middlewares/db_test.go b/manager/middlewares/db_test.go new file mode 100644 index 000000000..2fa5d3f5f --- /dev/null +++ b/manager/middlewares/db_test.go @@ -0,0 +1,180 @@ +package middlewares + +import ( + "app/base/database" + "app/base/utils" + "context" + "go/ast" + "go/parser" + "go/token" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/gorm" +) + +func ginWithMethod(method string) *gin.Context { + c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c.Request = httptest.NewRequest(method, "/", nil) + return c +} + +func TestUseReadReplica(t *testing.T) { + origEnabled := utils.CoreCfg.DBReadReplicaEnabled + origHost := utils.CoreCfg.DBReadReplicaHost + origPort := utils.CoreCfg.DBReadReplicaPort + t.Cleanup(func() { + utils.CoreCfg.DBReadReplicaEnabled = origEnabled + utils.CoreCfg.DBReadReplicaHost = origHost + utils.CoreCfg.DBReadReplicaPort = origPort + }) + + utils.CoreCfg.DBReadReplicaHost = "replica.example" + utils.CoreCfg.DBReadReplicaPort = 5432 + + utils.CoreCfg.DBReadReplicaEnabled = false + assert.False(t, useReadReplica(ginWithMethod(http.MethodGet))) + + utils.CoreCfg.DBReadReplicaEnabled = true + assert.True(t, useReadReplica(ginWithMethod(http.MethodGet))) + assert.False(t, useReadReplica(ginWithMethod(http.MethodPost))) + assert.False(t, useReadReplica(ginWithMethod(http.MethodPut))) + assert.False(t, useReadReplica(ginWithMethod(http.MethodPatch))) + assert.False(t, useReadReplica(ginWithMethod(http.MethodDelete))) + + utils.CoreCfg.DBReadReplicaHost = "" + assert.False(t, useReadReplica(ginWithMethod(http.MethodGet))) +} + +func TestDBWriterUsesRequestContext(t *testing.T) { + utils.SkipWithoutDB(t) + database.Configure() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + c := ginWithMethod(http.MethodPost) + c.Request = c.Request.WithContext(ctx) + + db := DBWriter(c) + require.NotNil(t, db) + assert.Equal(t, ctx, db.Statement.Context) + + writerPool := poolOf(t, db) + primaryPool := poolOf(t, database.DB(ctx)) + assert.Same(t, primaryPool, writerPool) +} + +func TestDBGetWithoutReplicaUsesPrimaryAndRequestContext(t *testing.T) { + utils.SkipWithoutDB(t) + database.Configure() + + origEnabled := utils.CoreCfg.DBReadReplicaEnabled + t.Cleanup(func() { utils.CoreCfg.DBReadReplicaEnabled = origEnabled }) + utils.CoreCfg.DBReadReplicaEnabled = false + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + c := ginWithMethod(http.MethodGet) + c.Request = c.Request.WithContext(ctx) + + db := DB(c) + require.NotNil(t, db) + assert.Equal(t, ctx, db.Statement.Context) + assert.Same(t, poolOf(t, database.DB(ctx)), poolOf(t, db)) +} + +func poolOf(t *testing.T, db *gorm.DB) any { + t.Helper() + sqlDB, err := db.DB() + require.NoError(t, err) + return sqlDB +} + +func TestHandlersDoNotCallDatabaseDBDirectly(t *testing.T) { + _, file, _, ok := runtime.Caller(0) + require.True(t, ok) + root := filepath.Clean(filepath.Join(filepath.Dir(file), "../..")) + roots := []string{ + filepath.Join(root, "manager", "controllers"), + filepath.Join(root, "turnpike", "controllers"), + } + var offenders []string + for _, start := range roots { + err := filepath.WalkDir(start, func(path string, d os.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() || !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") { + return nil + } + offenders = append(offenders, directDatabaseDBCalls(t, path)...) + return nil + }) + require.NoError(t, err) + } + assert.Empty(t, offenders) +} + +func directDatabaseDBCalls(t *testing.T, path string) []string { + t.Helper() + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, path, nil, 0) + require.NoError(t, err) + dbName := importName(f, "app/base/database") + baseName := importName(f, "app/base") + if dbName == "" { + return nil + } + var hits []string + ast.Inspect(f, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok || (sel.Sel.Name != "DB" && sel.Sel.Name != "DBReadReplica") { + return true + } + pkg, ok := sel.X.(*ast.Ident) + if !ok || pkg.Name != dbName { + return true + } + if sel.Sel.Name == "DB" && len(call.Args) == 1 && isBaseContext(call.Args[0], baseName) { + return true + } + pos := fset.Position(call.Pos()) + hits = append(hits, pos.String()) + return true + }) + return hits +} + +func importName(f *ast.File, path string) string { + for _, imp := range f.Imports { + if strings.Trim(imp.Path.Value, `"`) != path { + continue + } + if imp.Name != nil { + return imp.Name.Name + } + return filepath.Base(path) + } + return "" +} + +func isBaseContext(arg ast.Expr, baseName string) bool { + sel, ok := arg.(*ast.SelectorExpr) + if !ok || sel.Sel.Name != "Context" || baseName == "" { + return false + } + pkg, ok := sel.X.(*ast.Ident) + return ok && pkg.Name == baseName +} diff --git a/manager/routes/routes.go b/manager/routes/routes.go index 346c8a2fd..c0ab87a3c 100644 --- a/manager/routes/routes.go +++ b/manager/routes/routes.go @@ -15,7 +15,6 @@ func InitAPI(api *gin.RouterGroup, config docs.EndpointsConfig) { // nolint: fun api.Use(middlewares.CheckReferer()) api.Use(middlewares.SetAPIVersion(api.BasePath())) api.Use(middlewares.Deprecate(deprecations.DeprecateLimit())) - api.Use(middlewares.DatabaseWithContext()) userAuth := api.Group("/") if utils.CoreCfg.KesselEnabled { diff --git a/turnpike/admin_api.go b/turnpike/admin_api.go index c9d1d1f87..3be922675 100644 --- a/turnpike/admin_api.go +++ b/turnpike/admin_api.go @@ -32,7 +32,6 @@ func RunAdminAPI() { utils.LogInfo("port", utils.CoreCfg.PublicPort, "Manager-admin starting") app := gin.New() app.Use(middlewares.RequestResponseLogger()) - app.Use(middlewares.DatabaseWithContext()) middlewares.SetAdminSwagger(app) core.InitProbes(app) diff --git a/turnpike/controllers/admin.go b/turnpike/controllers/admin.go index a51cd3eea..25e421139 100644 --- a/turnpike/controllers/admin.go +++ b/turnpike/controllers/admin.go @@ -30,7 +30,7 @@ func Syncapi(c *gin.Context) { utils.LogInfo("manual syncing called...") sync.Configure() vmaasExportedTS := sync.VmaasDBExported() - err := sync.SyncData(database.DB(c.Request.Context()), nil, vmaasExportedTS) + err := sync.SyncData(middlewares.DBWriter(c), nil, vmaasExportedTS) if err != nil { utils.LogError("err", err, "manual called syncing failed") c.JSON(http.StatusInternalServerError, gin.H{"err": err.Error()}) @@ -221,7 +221,7 @@ func SystemDeleteHandler(c *gin.Context) { } var systemInventoryID []uuid.UUID - db := middlewares.DBFromContext(c) + db := middlewares.DB(c) tx := db.Begin() defer tx.Rollback() diff --git a/turnpike/controllers/database.go b/turnpike/controllers/database.go index c39a067a7..fd3d82f7d 100644 --- a/turnpike/controllers/database.go +++ b/turnpike/controllers/database.go @@ -1,8 +1,8 @@ package controllers import ( - "app/base/database" "app/base/utils" + "app/manager/middlewares" "fmt" "net/http" @@ -30,7 +30,7 @@ func RepackRecreateHandler(c *gin.Context) { "GRANT USAGE ON SCHEMA repack TO vmaas_sync", } for _, query := range runSequence { - if err := database.DB(c.Request.Context()).Exec(query).Error; err != nil { + if err := middlewares.DBWriter(c).Exec(query).Error; err != nil { c.JSON(http.StatusInternalServerError, gin.H{"err": err.Error()}) return } @@ -52,7 +52,7 @@ func RepackRecreateHandler(c *gin.Context) { func GetActiveSessionsHandler(c *gin.Context) { param := c.Param("search") data := make([]Session, 0) - q := database.DB(c.Request.Context()).Table("pg_stat_activity").Select("pid, query") + q := middlewares.DBWriter(c).Table("pg_stat_activity").Select("pid, query") if param != "" { q.Where("query like ?", fmt.Sprint("%", param, "%")) } @@ -83,7 +83,7 @@ func TerminateSessionHandler(c *gin.Context) { c.JSON(http.StatusBadRequest, utils.ErrorResponse{Error: "pid param not found"}) return } - err := database.DB(c.Request.Context()).Exec("select pg_terminate_backend(?)", param).Error + err := middlewares.DBWriter(c).Exec("select pg_terminate_backend(?)", param).Error if err != nil { utils.LogError("error", err, "DB query failed") c.JSON(http.StatusInternalServerError, gin.H{"err": err.Error()})