Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions base/mqueue/template_event.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type TemplateResponse struct {
Date time.Time `json:"date"` // Latest date to include snapshots for
RepositoryUUIDS []string `json:"repository_uuids"` // Repositories added to the template
EnvironmentID string `json:"client_environment_id"` // Environment ID used by subscription-manager & candlepin
AdvisoryIDs []string `json:"advisory_ids"` // List of advisory names (RHSA-…, RHBA-…)
}

type TemplateEvent struct {
Expand Down
2 changes: 2 additions & 0 deletions conf/listener.env
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,6 @@ DB_PASSWD=listener
CREATED_SYSTEMS_TOPIC=patchman.evaluator.user-evaluation

# template_advisory_eval=false — recalc template systems on advisory change/delete (enable with evaluator pods)
# templates_with_advisories=false — read advisory_ids from template create/update messages.
# Leave false until content-sources publishes advisory_ids; otherwise stored template_advisory rows are cleared.
POD_CONFIG=
33 changes: 26 additions & 7 deletions docs/md/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ archive is uploaded, it upserts **`system_inventory`** (including `vmaas_json` w
Kafka message (`patchman.evaluator.upload` topic) to evaluate the system with the `evaluator-upload` component. This
component also handles system deleting events (`platform.inventory.events` Kafka topic).
It consumes template events from Content Sources (`platform.content-sources.template`), upserts **`template`** metadata,
and on `template-updated` syncs **`template_advisory`** rows from the Content Sources API. When template advisories
and syncs **`template_advisory`**. With `templates_with_advisories` unset or false, only `template-updated` syncs, by
calling Content Sources `GET /templates/{uuid}/advisories/ids`. With `templates_with_advisories=true`, `template-created`
and `template-updated` sync from `advisory_ids` on the message and do not call that API. When template advisories
change and `template_advisory_eval` is enabled, it sends a bulk re-evaluation message to
`patchman.evaluator.user-evaluation`.
See [component environment variables](../../conf/listener.env)
Expand Down Expand Up @@ -79,14 +81,31 @@ and the [major migration runbook](major-migration-runbook.md). Using container C

### Template content update flow

When Content Sources publishes a `template-updated` event:
Content Sources publishes `template-created`, `template-updated`, and `template-deleted` on
`platform.content-sources.template`. **listener** always upserts **`template`** metadata on create and update.
`template-deleted` unassigns systems, deletes **`template_advisory`**, then deletes **`template`**.

1. **listener** upserts **`template`** metadata and calls Content Sources `GET /templates/{uuid}/advisories/ids`.
2. **listener** diffs the response against **`template_advisory`** and inserts or deletes rows.
3. If advisories changed and `template_advisory_eval=true`, **listener** looks up systems assigned to the
Advisory sync on create and update depends on `templates_with_advisories` (listener `POD_CONFIG`, default `false`):

- **false** — only `template-updated` syncs. **listener** calls Content Sources
`GET /templates/{uuid}/advisories/ids` when `CONTENT_SOURCES_ADDRESS` and `CONTENT_SOURCES_USER` are set.
`template-created` does not sync advisories.
- **true** — `template-created` and `template-updated` use `advisory_ids` from each object in the message `data`
array (advisory names such as `RHBA-2024:1234`). The advisories API is not called. A missing key or an empty
array is an empty snapshot and removes stored **`template_advisory`** rows for that template.

Leave the flag false until Content Sources sends `advisory_ids` on every create and update. Enabling it earlier
treats the omitted field as an empty snapshot. Do not enable it on the listener alone in `deploy/clowdapp.yaml`
as part of this change.

After either path:

1. **listener** diffs the snapshot against **`template_advisory`** and inserts or deletes rows. Names not yet in
**`advisory_metadata`** are skipped.
2. If advisories changed and `template_advisory_eval=true`, **listener** looks up systems assigned to the
template and sends `PlatformEvent{system_ids}` to `patchman.evaluator.user-evaluation`.
4. **evaluator-user-evaluation** re-evaluates each system in the message.
5. For template-assigned systems, **evaluator** skips yum updates, calls VMaaS, and marks advisories in
3. **evaluator-user-evaluation** re-evaluates each system in the message.
4. For template-assigned systems, **evaluator** skips yum updates, calls VMaaS, and marks advisories in
**`template_advisory`** as installable; other applicable advisories remain applicable.

### `template_advisory_eval` rollout
Expand Down
3 changes: 3 additions & 0 deletions listener/listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ var (
excludedHostTypes map[string]bool
enableBypass bool
enableTemplateAdvisoryEval bool
templatesWithAdvisories bool
uploadEvalTimeout time.Duration
deletionThreshold time.Duration
useTraceLevel bool
Expand Down Expand Up @@ -92,6 +93,8 @@ func configureListener() {
enableBypass = utils.PodConfig.GetBool("bypass", false)
// Template advisory evaluation: recalc on advisory content change and template delete
enableTemplateAdvisoryEval = utils.PodConfig.GetBool("template_advisory_eval", false)
// Use advisory_ids from the template kafka message instead of the content-sources API.
templatesWithAdvisories = utils.PodConfig.GetBool("templates_with_advisories", false)
// How long to collect upload messages before grouping them and sending to evaluator
uploadEvalTimeout = time.Duration(utils.PodConfig.GetInt("upload_eval_timeout_ms", 500)) * time.Millisecond
// Ignore a system if there was a delete message in the last X hours
Expand Down
9 changes: 7 additions & 2 deletions listener/template_advisories.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"gorm.io/gorm/clause"
)

// nolint: funlen
func syncTemplateAdvisories(ctx context.Context, accountID int, templateID int64, templateUUID, orgID string) error {
current, err := callCSTemplateAdvisories(ctx, templateUUID)
if err != nil {
Expand All @@ -25,13 +24,19 @@ func syncTemplateAdvisories(ctx context.Context, accountID int, templateID int64
}
return errors.Wrap(err, "fetching current template advisories from content-sources")
}
return applyTemplateAdvisories(ctx, accountID, templateID, templateUUID, orgID, current.AdvisoryIDs)
}

// applyTemplateAdvisories diffs advisory names against template_advisory and writes the change.
// advisoryIDs is the full snapshot. Nil and empty both mean no advisories.
func applyTemplateAdvisories(ctx context.Context, accountID int, templateID int64, templateUUID, orgID string,
advisoryIDs []string) error {
stored, err := lookUpTemplateAdvisories(database.DB(ctx), accountID, templateID)
if err != nil {
return errors.Wrap(err, "looking up stored template advisories")
}

toAdd, toRemove := diffTemplateAdvisories(current.AdvisoryIDs, stored)
toAdd, toRemove := diffTemplateAdvisories(advisoryIDs, stored)
Comment on lines +32 to +39

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High · Concurrent snapshots mix advisory rows

When two handlers process different snapshots for the same template concurrently, applyTemplateAdvisories reads and diffs the stored rows before opening its transaction, so both handlers can apply additions based on the same old set and leave rows from both snapshots; callers see an advisory set matching neither snapshot.

Serialize updates per template and read and diff the stored rows within that serialized transaction.

Prompt for AI agents
In `listener/template_advisories.go` at lines 32-39:

**Concurrent snapshots mix advisory rows**

When two handlers process different snapshots for the same template concurrently, `applyTemplateAdvisories` reads and diffs the stored rows before opening its transaction, so both handlers can apply additions based on the same old set and leave rows from both snapshots; callers see an advisory set matching neither snapshot.

Serialize updates per template and read and diff the stored rows within that serialized transaction.

if len(toAdd) == 0 && len(toRemove) == 0 {
utils.LogInfo("template_uuid", templateUUID, "template advisories unchanged")
return nil
Expand Down
129 changes: 129 additions & 0 deletions listener/template_advisories_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ import (
"app/base/core"
"app/base/database"
"app/base/models"
"app/base/mqueue"
"app/base/utils"
"testing"

"github.com/bytedance/sonic"
"github.com/redhatinsights/platform-go-middlewares/v2/identity"
"github.com/stretchr/testify/assert"
)
Expand Down Expand Up @@ -167,3 +169,130 @@ func TestSyncTemplateAdvisories_TemplateNotFound(t *testing.T) {
// sync was skipped and no advisories were changed
database.CheckTemplateAdvisories(t, templateID, []int64{1, 2})
}

func setTemplateAdvisoryFlags(t *testing.T, withAdvisories bool) {
t.Helper()
originalWith := templatesWithAdvisories
originalEval := enableTemplateAdvisoryEval
templatesWithAdvisories = withAdvisories
enableTemplateAdvisoryEval = false
t.Cleanup(func() {
templatesWithAdvisories = originalWith
enableTemplateAdvisoryEval = originalEval
})
}

func templateAdvisoryCount(t *testing.T, templateID int64) int64 {
t.Helper()
var count int64
err := database.TestDB().Model(&models.TemplateAdvisory{}).
Where("template_id = ?", templateID).
Count(&count).Error
assert.Nil(t, err)
return count
}

func loadTemplateByUUID(t *testing.T, templateUUID string) models.Template {
t.Helper()
var stored models.Template
err := database.TestDB().
Where("rh_account_id = ? AND uuid = ?::uuid", accountID, templateUUID).
First(&stored).Error
assert.Nil(t, err)
return stored
}

func TestTemplateMessageAdvisoryIDs(t *testing.T) {
utils.SkipWithoutDB(t)
core.SetupTestEnvironment()
setTemplateAdvisoryFlags(t, true)

// Prove this path does not call Content Sources.
contentSourcesClient = nil
contentSourcesBaseURL = ""

templateUUID := "88888888-0000-0000-0000-000000000099"
t.Cleanup(func() {
database.DeleteTemplate(t, accountID, templateUUID)
})

createEvent := createTempateMsg(t, "template-created", orgID, 1)
createEvent.Data[0].UUID = templateUUID
createEvent.Data[0].EnvironmentID = "88888888000000000000000000000099"
createEvent.Data[0].AdvisoryIDs = []string{"RH-1", "RH-2"}
msg, err := sonic.Marshal(createEvent)
assert.Nil(t, err)

err = TemplatesMessageHandler(base.Context, mqueue.KafkaMessage{Value: msg})
assert.Nil(t, err)

stored := loadTemplateByUUID(t, templateUUID)
database.CheckTemplateAdvisories(t, stored.ID, []int64{1, 2})
assert.Equal(t, int64(2), templateAdvisoryCount(t, stored.ID))

updateEvent := createEvent
updateEvent.Type = "com.redhat.console.repositories.template-updated"
updateEvent.Data[0].AdvisoryIDs = []string{"RH-1", "RH-3"}
msg, err = sonic.Marshal(updateEvent)
assert.Nil(t, err)

err = TemplatesMessageHandler(base.Context, mqueue.KafkaMessage{Value: msg})
assert.Nil(t, err)

database.CheckTemplateAdvisories(t, stored.ID, []int64{1, 3})
assert.Equal(t, int64(2), templateAdvisoryCount(t, stored.ID))
var removed int64
err = database.TestDB().Model(&models.TemplateAdvisory{}).
Where("template_id = ? AND advisory_id = ?", stored.ID, 2).
Count(&removed).Error
assert.Nil(t, err)
assert.Equal(t, int64(0), removed)

updateEvent.Data[0].AdvisoryIDs = []string{}
msg, err = sonic.Marshal(updateEvent)
assert.Nil(t, err)
err = TemplatesMessageHandler(base.Context, mqueue.KafkaMessage{Value: msg})
assert.Nil(t, err)
assert.Equal(t, int64(0), templateAdvisoryCount(t, stored.ID))
}

func TestTemplateUpdateStillUsesContentSourcesAPI(t *testing.T) {
utils.SkipWithoutDB(t)
core.SetupTestEnvironment()
setupContentSourcesClient(t)
setTemplateAdvisoryFlags(t, false)

templateUUID := "88888888-0000-0000-0000-000000000098"
t.Cleanup(func() {
database.DeleteTemplate(t, accountID, templateUUID)
})

createEvent := createTempateMsg(t, "template-created", orgID, 1)
createEvent.Data[0].UUID = templateUUID
createEvent.Data[0].EnvironmentID = "88888888000000000000000000000098"
// Even if a producer sends the key, flag off must ignore it.
createEvent.Data[0].AdvisoryIDs = []string{"RH-2"}
msg, err := sonic.Marshal(createEvent)
assert.Nil(t, err)

err = TemplatesMessageHandler(base.Context, mqueue.KafkaMessage{Value: msg})
assert.Nil(t, err)

stored := loadTemplateByUUID(t, templateUUID)
assert.Equal(t, int64(0), templateAdvisoryCount(t, stored.ID))

database.CreateTemplateAdvisories(t, accountID, stored.ID, []int64{2})

updateEvent := createEvent
updateEvent.Type = "com.redhat.console.repositories.template-updated"
updateEvent.Data[0].AdvisoryIDs = []string{"RH-2"}
msg, err = sonic.Marshal(updateEvent)
assert.Nil(t, err)

err = TemplatesMessageHandler(base.Context, mqueue.KafkaMessage{Value: msg})
assert.Nil(t, err)

// Platform mock returns RH-1 and RH-3, so the message's RH-2 is not the source.
database.CheckTemplateAdvisories(t, stored.ID, []int64{1, 3})
assert.Equal(t, int64(2), templateAdvisoryCount(t, stored.ID))
}
49 changes: 49 additions & 0 deletions listener/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,3 +179,52 @@ func TestTemplateEmptyDescription(t *testing.T) {
// cleanup
deleteTemplatesInDB(t, after)
}

func TestProcessTemplateEventAdvisoryIDs(t *testing.T) {
raw := []byte(`{
"id": "1",
"type": "com.redhat.console.repositories.template-updated",
"source": "urn:redhat:source:console:app:repositories",
"subject": "urn:redhat:subject:console:rhel:template-updated",
"time": "2024-01-02T03:04:05Z",
"redhatorgid": "org_1",
"data": [{
"uuid": "77777777-0000-0000-0000-000000000000",
"name": "Template0",
"org_id": "org_1",
"arch": "x86_64",
"version": "8",
"date": "2024-01-02T03:04:05Z",
"repository_uuids": [],
"client_environment_id": "77777777000000000000000000000000",
"advisory_ids": ["RHBA-2024:1234", "RHSA-2024:5678"]
}]
}`)

eType, event, err := processTemplateEvent(raw)
assert.Nil(t, err)
assert.Equal(t, "template-updated", eType)
assert.Equal(t, []string{"RHBA-2024:1234", "RHSA-2024:5678"}, event.Data[0].AdvisoryIDs)

omitted := []byte(`{
"id": "1",
"type": "com.redhat.console.repositories.template-created",
"source": "urn:redhat:source:console:app:repositories",
"subject": "urn:redhat:subject:console:rhel:template-created",
"time": "2024-01-02T03:04:05Z",
"redhatorgid": "org_1",
"data": [{
"uuid": "77777777-0000-0000-0000-000000000000",
"name": "Template0",
"org_id": "org_1",
"arch": "x86_64",
"version": "8",
"date": "2024-01-02T03:04:05Z",
"repository_uuids": [],
"client_environment_id": "77777777000000000000000000000000"
}]
}`)
_, event, err = processTemplateEvent(omitted)
assert.Nil(t, err)
assert.Nil(t, event.Data[0].AdvisoryIDs)
}
12 changes: 9 additions & 3 deletions listener/templates.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,16 @@ func TemplateUpdate(ctx context.Context, template mqueue.TemplateResponse, event
return errors.Wrap(err, "creating/updating template from message")
}

if contentSourcesBaseURL != "" && utils.CoreCfg.ContentSourcesUser != "" && eventType == TemplateEventUpdate {
ctx := identity.WithIdentity(base.Context,
switch {
case templatesWithAdvisories:
err = applyTemplateAdvisories(ctx, accountID, row.ID, template.UUID, template.OrgID, template.AdvisoryIDs)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High · Failed advisory sync stays stale

When an advisory lookup or database write fails during a message-based sync, TemplateUpdate returns the error, but TemplatesMessageHandler only logs it and returns nil, so MakeRetryingHandler acknowledges the message and the stored advisory state remains stale or partial.

Propagate the per-template error from TemplatesMessageHandler so the retry handler can retry the message.

Also at listener/templates.go:157-158.

Prompt for AI agents
In `listener/templates.go` at line 156:

**Failed advisory sync stays stale**

When an advisory lookup or database write fails during a message-based sync, `TemplateUpdate` returns the error, but `TemplatesMessageHandler` only logs it and returns nil, so `MakeRetryingHandler` acknowledges the message and the stored advisory state remains stale or partial.

Propagate the per-template error from `TemplatesMessageHandler` so the retry handler can retry the message.

Also at `listener/templates.go:157-158`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High · Older snapshots overwrite newer advisories

When an older template event is processed after a newer event, TemplateUpdate applies each message's AdvisoryIDs as the full snapshot without comparing event times or revisions, so applyTemplateAdvisories replaces the newer advisory set with stale rows.

Compare the event time or revision with the latest processed value and ignore stale snapshots.

Also at listener/templates.go:157.

Prompt for AI agents
In `listener/templates.go` at line 156:

**Older snapshots overwrite newer advisories**

When an older template event is processed after a newer event, `TemplateUpdate` applies each message's `AdvisoryIDs` as the full snapshot without comparing event times or revisions, so `applyTemplateAdvisories` replaces the newer advisory set with stale rows.

Compare the event time or revision with the latest processed value and ignore stale snapshots.

Also at `listener/templates.go:157`.

if err != nil {
return errors.Wrap(err, "syncing template advisories from message")
}
case contentSourcesBaseURL != "" && utils.CoreCfg.ContentSourcesUser != "" && eventType == TemplateEventUpdate:
csCtx := identity.WithIdentity(base.Context,
utils.XRHIDForOrg(template.OrgID, utils.CoreCfg.ContentSourcesUser))
err = syncTemplateAdvisories(ctx, accountID, row.ID, template.UUID, template.OrgID)
err = syncTemplateAdvisories(csCtx, accountID, row.ID, template.UUID, template.OrgID)
if err != nil {
return errors.Wrap(err, "syncing template advisories")
}
Expand Down
Loading