RHINENG-26121: update manager to use account_advisory table

dev/test_data.sql

@@ -7,6 +7,7 @@ DELETE FROM deleted_system;
 DELETE FROM repo;
 DELETE FROM timestamp_kv;
 DELETE FROM advisory_account_data;
+DELETE FROM account_advisory;
 DELETE FROM package_account_data;
 DELETE FROM package;
 DELETE FROM package_name;
@@ -203,6 +204,7 @@ INSERT INTO timestamp_kv (name, value) VALUES
 ('last_eval_repo_based', '2018-04-05T01:23:45+02:00');
 
 SELECT refresh_all_cached_counts();
+SELECT refresh_account_advisory_caches_multi(NULL, NULL);
 
 ALTER TABLE advisory_metadata ALTER COLUMN id RESTART WITH 100;
 ALTER TABLE system_inventory ALTER COLUMN id RESTART WITH 100;

manager/config/config.go

@@ -26,4 +26,6 @@ var (
 
 	// Expose templates API (feature flag)
 	EnableTemplates = utils.PodConfig.GetBool("templates_api", true)
+	// Use precomputed per-workspace advisory counts from account_advisory table
+	EnableAccountAdvisoryReadPath = utils.PodConfig.GetBool("account_advisory", true)
 )

manager/controllers/advisories.go

@@ -89,14 +89,9 @@ func advisoriesCommon(c *gin.Context) (*gorm.DB, *ListMeta, []string, error) {
 		return nil, nil, nil, err
 	}
 
-	// TODO: fix below; the condition is always true since moving from groups to workspaces,
-	// there will always be at least root workspace
-	if config.DisableCachedCounts || HasInventoryFilter(filters) || len(workspaceIDs) != 0 {
-		middlewares.AdvisoryAccountDataCnt.WithLabelValues("miss").Inc()
-		query = buildQueryAdvisoriesTagged(db, filters, account, workspaceIDs)
-	} else {
-		middlewares.AdvisoryAccountDataCnt.WithLabelValues("hit").Inc()
-		query = buildQueryAdvisories(db, account)
+	query, err = resolveAdvisoriesQuery(db, account, workspaceIDs, filters)
+	if err != nil {
+		return nil, nil, nil, err
 	}
 
 	query, meta, params, err := ListCommon(query, c, filters, AdvisoriesOpts)
@@ -218,6 +213,36 @@ func buildQueryAdvisories(db *gorm.DB, account int) *gorm.DB {
 	return query
 }
 
+func resolveAdvisoriesQuery(db *gorm.DB, account int, workspaceIDs []string, filters Filters) (*gorm.DB, error) {
+	if config.EnableAccountAdvisoryReadPath && !hasNonGroupInventoryFilter(filters) {
+		effectiveWorkspaceIDs := workspaceIDs
+		if groupNames := getGroupNameFilterValues(filters); len(groupNames) > 0 {
+			var err error
+			effectiveWorkspaceIDs, err = resolveWorkspaceNameToIDs(db, account, groupNames, workspaceIDs)
+			if err != nil {
+				return nil, err
+			}
+		}
+		middlewares.AdvisoryAccountDataCnt.WithLabelValues("hit").Inc()
+		if len(effectiveWorkspaceIDs) == 0 {
+			query := buildQueryAdvisoriesFromAccountAdvisory(db, account, effectiveWorkspaceIDs)
+			return query.Where("FALSE"), nil
+		}
+		return buildQueryAdvisoriesFromAccountAdvisory(db, account, effectiveWorkspaceIDs), nil
+	}
+	// TODO: fix below;
+	// the condition is always true since moving from groups to workspaces,
+	// there will always be at least root workspace
+	// leaving until RBAC cannot be re-enabled
+	if !config.EnableAccountAdvisoryReadPath && !config.DisableCachedCounts &&
+		!HasInventoryFilter(filters) && len(workspaceIDs) == 0 {
+		middlewares.AdvisoryAccountDataCnt.WithLabelValues("hit").Inc()
+		return buildQueryAdvisories(db, account), nil
+	}
+	middlewares.AdvisoryAccountDataCnt.WithLabelValues("miss").Inc()
+	return buildQueryAdvisoriesTagged(db, filters, account, workspaceIDs), nil
+}
+
 func buildAdvisoryAccountDataQuery(db *gorm.DB, account int, workspaceIDs []string) *gorm.DB {
 	query := database.SystemAdvisories(db, account, workspaceIDs).
 		Select(`sa.advisory_id, si.rh_account_id as rh_account_id,
@@ -242,6 +267,63 @@ func buildQueryAdvisoriesTagged(db *gorm.DB, filters map[string]FilterData, acco
 	return query
 }
 
+func hasNonGroupInventoryFilter(filters Filters) bool {
+	for key, data := range filters {
+		if data.Type == TagFilter {
+			return true
+		}
+		if data.Type == InventoryFilter && key != "group_name" {
+			return true
+		}
+	}
+	return false
+}
+
+func getGroupNameFilterValues(filters Filters) []string {
+	if data, ok := filters["group_name"]; ok && data.Type == InventoryFilter {
+		return data.Values
+	}
+	return nil
+}
+
+func resolveWorkspaceNameToIDs(db *gorm.DB,
+	accountID int,
+	names []string,
+	allowedWorkspaceIDs []string) ([]string, error) {
+	var ids []string
+	query := db.Table("system_inventory si").
+		Distinct().
+		Where("si.rh_account_id = ?", accountID).
+		Where("si.workspace_name IN (?)", names)
+	if len(allowedWorkspaceIDs) > 0 {
+		query = query.Where("si.workspace_id IN (?)", allowedWorkspaceIDs)
+	}
+	err := query.Pluck("workspace_id", &ids).Error
+	return ids, err
+}
+
+func buildAccountAdvisorySubquery(db *gorm.DB, account int, workspaceIDs []string) *gorm.DB {
+	query := db.Table("account_advisory aa_inner").
+		Select(`aa_inner.advisory_id,
+			aa_inner.rh_account_id,
+			SUM(aa_inner.systems_installable) as systems_installable,
+			SUM(aa_inner.systems_applicable) as systems_applicable`).
+		Where("aa_inner.rh_account_id = ?", account)
+	if len(workspaceIDs) > 0 {
+		query = query.Where("aa_inner.workspace_id IN (?)", workspaceIDs)
+	}
+	return query.Group("aa_inner.advisory_id, aa_inner.rh_account_id").
+		Having("SUM(aa_inner.systems_applicable) > 0")
+}
+
+func buildQueryAdvisoriesFromAccountAdvisory(db *gorm.DB, account int, workspaceIDs []string) *gorm.DB {
+	subq := buildAccountAdvisorySubquery(db, account, workspaceIDs)
+	return database.AdvisoryMetadata(db).
+		Select(AdvisoriesSelect).
+		Joins("JOIN (?) aad ON am.id = aad.advisory_id", subq).
+		Joins("LEFT JOIN advisory_severity sev ON am.severity_id = sev.id")
+}
+
 func buildAdvisoriesData(advisories []AdvisoriesDBLookup) ([]AdvisoryItem, int, map[string]int) {
 	var total int
 	subtotals := map[string]int{

manager/controllers/advisories_export.go

@@ -2,11 +2,9 @@ package controllers
 
 import (
 	"app/base/utils"
-	"app/manager/config"
 	"app/manager/middlewares"
 
 	"github.com/gin-gonic/gin"
-	"gorm.io/gorm"
 )
 
 // nolint:lll
@@ -37,14 +35,11 @@ func AdvisoriesExportHandler(c *gin.Context) {
 		return
 	}
 	db := middlewares.DBFromContext(c)
-	var query *gorm.DB
 
-	// TODO: fix below; the condition is always true since moving from groups to workspaces,
-	// there will always be at least root workspace
-	if config.DisableCachedCounts || HasInventoryFilter(filters) || len(workspaceIDs) != 0 {
-		query = buildQueryAdvisoriesTagged(db, filters, account, workspaceIDs)
-	} else {
-		query = buildQueryAdvisories(db, account)
+	query, err := resolveAdvisoriesQuery(db, account, workspaceIDs, filters)
+	if err != nil {
+		utils.LogAndRespError(c, err, "db error")
+		return
 	}
 
 	var advisories []AdvisoriesDBLookup

manager/controllers/advisories_test.go

@@ -2,7 +2,9 @@ package controllers
 
 import (
 	"app/base/core"
+	"app/base/database"
 	"app/base/utils"
+	"app/manager/config"
 	"fmt"
 	"net/http"
 	"testing"
@@ -38,6 +40,15 @@ func testAdvisoriesIDs(t *testing.T, url string) IDsPlainResponse {
 	return output
 }
 
+func TestAdvisoriesLegacyPath(t *testing.T) {
+	core.SetupTest(t)
+	config.EnableAccountAdvisoryReadPath = false
+	defer func() { config.EnableAccountAdvisoryReadPath = true }()
+
+	output := testAdvisories(t, "/")
+	assert.Equal(t, 12, len(output.Data))
+}
+
 func TestAdvisoriesDefault(t *testing.T) {
 	output := testAdvisories(t, "/")
 	assert.Equal(t, 12, len(output.Data))
@@ -386,3 +397,161 @@ func TestAdvisoryMetadataSums(t *testing.T) {
 	assert.Equal(t, bugfix, st["bugfix"])
 	assert.Equal(t, security, st["security"])
 }
+
+func TestAdvisoriesGroupNameSingle(t *testing.T) {
+	output := testAdvisories(t, "/?sort=id&filter[group_name]=group1")
+	// group1 = workspace 00000000-0000-0000-0000-000000000001
+	// Systems 1-6 in group1, advisories 1-8 from system 1, advisory 1 from systems 2-6
+	assert.Equal(t, 8, len(output.Data))
+	assert.Equal(t, "RH-1", output.Data[0].ID)
+	assert.Equal(t, 4, output.Data[0].Attributes.InstallableSystems)
+	assert.Equal(t, 6, output.Data[0].Attributes.ApplicableSystems)
+}
+
+func TestAdvisoriesGroupNameGroup2(t *testing.T) {
+	output := testAdvisories(t, "/?sort=id&filter[group_name]=group2")
+	// group2 = workspace 00000000-0000-0000-0000-000000000002
+	// System 8 in group2 (system 7 has no last_evaluation), advisories 10-13
+	assert.Equal(t, 4, len(output.Data))
+	assert.Equal(t, "CUSTOM-12", output.Data[0].ID)
+	assert.Equal(t, "CUSTOM-13", output.Data[1].ID)
+	assert.Equal(t, "UNSPEC-10", output.Data[2].ID)
+	assert.Equal(t, "UNSPEC-11", output.Data[3].ID)
+}
+
+func TestAdvisoriesGroupNameMultiple(t *testing.T) {
+	output := testAdvisories(t, "/?filter[group_name]=group1,group2")
+	// group1 + group2 = all advisories for account 1
+	assert.Equal(t, 12, len(output.Data))
+}
+
+func TestAdvisoriesGroupNameNoMatch(t *testing.T) {
+	output := testAdvisories(t, "/?filter[group_name]=nonexistent")
+	assert.Equal(t, 0, len(output.Data))
+	assert.Equal(t, 0, output.Meta.TotalItems)
+}
+
+func TestAdvisoriesIDsGroupName(t *testing.T) {
+	output := testAdvisoriesIDs(t, "/?sort=id&filter[group_name]=group2")
+	assert.Equal(t, 4, len(output.IDs))
+}
+
+func TestAdvisoriesExportGroupName(t *testing.T) {
+	core.SetupTest(t)
+	w := CreateRequest("GET", "/?filter[group_name]=group2", nil, "application/json", AdvisoriesExportHandler)
+
+	var output []AdvisoriesDBLookup
+	CheckResponse(t, w, http.StatusOK, &output)
+	assert.Equal(t, 4, len(output))
+}
+
+func TestHasNonGroupInventoryFilter(t *testing.T) {
+	tests := []struct {
+		name     string
+		filters  Filters
+		expected bool
+	}{
+		{
+			name:     "empty filters",
+			filters:  Filters{},
+			expected: false,
+		},
+		{
+			name: "only group_name",
+			filters: Filters{
+				"group_name": {Type: InventoryFilter, Operator: "eq", Values: []string{"group1"}},
+			},
+			expected: false,
+		},
+		{
+			name: "group_name and column filter",
+			filters: Filters{
+				"group_name":         {Type: InventoryFilter, Operator: "eq", Values: []string{"group1"}},
+				"applicable_systems": {Type: ColumnFilter, Operator: "gt", Values: []string{"0"}},
+			},
+			expected: false,
+		},
+		{
+			name: "tag filter",
+			filters: Filters{
+				"ns1/k1": {Type: TagFilter, Operator: "eq", Values: []string{"val1"}},
+			},
+			expected: true,
+		},
+		{
+			name: "sap_system filter",
+			filters: Filters{
+				"system_profile][sap_system": {Type: InventoryFilter, Operator: "eq", Values: []string{"true"}},
+			},
+			expected: true,
+		},
+		{
+			name: "group_name and tag",
+			filters: Filters{
+				"group_name": {Type: InventoryFilter, Operator: "eq", Values: []string{"group1"}},
+				"ns1/k1":     {Type: TagFilter, Operator: "eq", Values: []string{"val1"}},
+			},
+			expected: true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Equal(t, tc.expected, hasNonGroupInventoryFilter(tc.filters))
+		})
+	}
+}
+
+func TestGetGroupNameFilterValues(t *testing.T) {
+	filters := Filters{
+		"group_name": {Type: InventoryFilter, Operator: "eq", Values: []string{"group1", "group2"}},
+	}
+	assert.Equal(t, []string{"group1", "group2"}, getGroupNameFilterValues(filters))
+
+	assert.Nil(t, getGroupNameFilterValues(Filters{}))
+
+	filtersColumnOnly := Filters{
+		"group_name": {Type: ColumnFilter, Operator: "eq", Values: []string{"group1"}},
+	}
+	assert.Nil(t, getGroupNameFilterValues(filtersColumnOnly))
+}
+
+func TestAdvisoriesTagsFallbackWithAccountAdvisory(t *testing.T) {
+	output := testAdvisories(t, "/?sort=id&tags=ns1/k2=val2")
+	assert.Equal(t, 8, len(output.Data))
+	assert.Equal(t, 1, output.Data[0].Attributes.InstallableSystems)
+	assert.Equal(t, 2, output.Data[0].Attributes.ApplicableSystems)
+}
+
+func TestResolveWorkspaceNameToIDs(t *testing.T) {
+	core.SetupTest(t)
+
+	allowedWorkspaces := []string{
+		"00000000-0000-0000-0000-000000000001",
+		"00000000-0000-0000-0000-000000000002",
+		"00000000-0000-0000-0000-999999999999",
+	}
+
+	ids, err := resolveWorkspaceNameToIDs(database.DB, 1, []string{"group1"}, allowedWorkspaces)
+	assert.Nil(t, err)
+	assert.Equal(t, 1, len(ids))
+	assert.Equal(t, "00000000-0000-0000-0000-000000000001", ids[0])
+
+	ids, err = resolveWorkspaceNameToIDs(database.DB, 1, []string{"group2"}, allowedWorkspaces)
+	assert.Nil(t, err)
+	assert.Equal(t, 1, len(ids))
+	assert.Equal(t, "00000000-0000-0000-0000-000000000002", ids[0])
+
+	ids, err = resolveWorkspaceNameToIDs(database.DB, 1, []string{"group1", "group2"}, allowedWorkspaces)
+	assert.Nil(t, err)
+	assert.Equal(t, 2, len(ids))
+
+	ids, err = resolveWorkspaceNameToIDs(database.DB, 1, []string{"nonexistent"}, allowedWorkspaces)
+	assert.Nil(t, err)
+	assert.Equal(t, 0, len(ids))
+
+	ids, err = resolveWorkspaceNameToIDs(database.DB,
+		1, []string{"group1"}, []string{"00000000-0000-0000-0000-000000000002"})
+	assert.Nil(t, err)
+	assert.Equal(t, 0, len(ids))
+}