fix: correct handling of removed targets (#9824)

# Which Problems Are Solved

In Actions v2, if a target is removed, which is still used in an
execution, the target is still listed when list executions.

# How the Problems Are Solved

Removed targets are now also removed from the executions.

# Additional Changes

To be sure the list executions include a check if the target is still
existing.

# Additional Context

None

Co-authored-by: Livio Spring <livio.a@gmail.com>
This commit is contained in:
Stefan Benz
2025-04-30 14:58:10 +02:00
committed by GitHub
parent 48c1f7e49f
commit a05f7ce3fc
5 changed files with 141 additions and 19 deletions

View File

@@ -1,11 +1,13 @@
package query package query
import ( import (
"cmp"
"context" "context"
"database/sql" "database/sql"
_ "embed" _ "embed"
"encoding/json" "encoding/json"
"errors" "errors"
"slices"
"time" "time"
sq "github.com/Masterminds/squirrel" sq "github.com/Masterminds/squirrel"
@@ -301,13 +303,15 @@ func executionTargetsUnmarshal(data []byte) ([]*exec.Target, error) {
} }
targets := make([]*exec.Target, len(executionTargets)) targets := make([]*exec.Target, len(executionTargets))
// position starts with 1 slices.SortFunc(executionTargets, func(a, b *executionTarget) int {
for _, item := range executionTargets { return cmp.Compare(a.Position, b.Position)
})
for i, item := range executionTargets {
if item.Target != "" { if item.Target != "" {
targets[item.Position-1] = &exec.Target{Type: domain.ExecutionTargetTypeTarget, Target: item.Target} targets[i] = &exec.Target{Type: domain.ExecutionTargetTypeTarget, Target: item.Target}
} }
if item.Include != "" { if item.Include != "" {
targets[item.Position-1] = &exec.Target{Type: domain.ExecutionTargetTypeInclude, Target: item.Include} targets[i] = &exec.Target{Type: domain.ExecutionTargetTypeInclude, Target: item.Include}
} }
} }
return targets, nil return targets, nil

View File

@@ -1,11 +1,15 @@
SELECT instance_id, SELECT et.instance_id,
execution_id, et.execution_id,
JSONB_AGG( JSONB_AGG(
JSON_OBJECT( JSON_OBJECT(
'position' : position, 'position' : et.position,
'include' : include, 'include' : et.include,
'target' : target_id 'target' : et.target_id
) )
) as targets ) as targets
FROM projections.executions1_targets FROM projections.executions1_targets AS et
GROUP BY instance_id, execution_id INNER JOIN projections.targets2 AS t
ON et.instance_id = t.instance_id
AND et.target_id IS NOT NULL
AND et.target_id = t.id
GROUP BY et.instance_id, et.execution_id

View File

@@ -22,9 +22,10 @@ var (
` COUNT(*) OVER ()` + ` COUNT(*) OVER ()` +
` FROM projections.executions1` + ` FROM projections.executions1` +
` JOIN (` + ` JOIN (` +
`SELECT instance_id, execution_id, JSONB_AGG( JSON_OBJECT( 'position' : position, 'include' : include, 'target' : target_id ) ) as targets` + `SELECT et.instance_id, et.execution_id, JSONB_AGG( JSON_OBJECT( 'position' : et.position, 'include' : et.include, 'target' : et.target_id ) ) as targets` +
` FROM projections.executions1_targets` + ` FROM projections.executions1_targets AS et` +
` GROUP BY instance_id, execution_id` + ` INNER JOIN projections.targets2 AS t ON et.instance_id = t.instance_id AND et.target_id IS NOT NULL AND et.target_id = t.id` +
` GROUP BY et.instance_id, et.execution_id` +
`)` + `)` +
` AS execution_targets` + ` AS execution_targets` +
` ON execution_targets.instance_id = projections.executions1.instance_id` + ` ON execution_targets.instance_id = projections.executions1.instance_id` +
@@ -45,9 +46,10 @@ var (
` execution_targets.targets` + ` execution_targets.targets` +
` FROM projections.executions1` + ` FROM projections.executions1` +
` JOIN (` + ` JOIN (` +
`SELECT instance_id, execution_id, JSONB_AGG( JSON_OBJECT( 'position' : position, 'include' : include, 'target' : target_id ) ) as targets` + `SELECT et.instance_id, et.execution_id, JSONB_AGG( JSON_OBJECT( 'position' : et.position, 'include' : et.include, 'target' : et.target_id ) ) as targets` +
` FROM projections.executions1_targets` + ` FROM projections.executions1_targets AS et` +
` GROUP BY instance_id, execution_id` + ` INNER JOIN projections.targets2 AS t ON et.instance_id = t.instance_id AND et.target_id IS NOT NULL AND et.target_id = t.id` +
` GROUP BY et.instance_id, et.execution_id` +
`)` + `)` +
` AS execution_targets` + ` AS execution_targets` +
` ON execution_targets.instance_id = projections.executions1.instance_id` + ` ON execution_targets.instance_id = projections.executions1.instance_id` +
@@ -179,6 +181,63 @@ func Test_ExecutionPrepares(t *testing.T) {
}, },
}, },
}, },
{
name: "prepareExecutionsQuery multiple result, removed target, position missing",
prepare: prepareExecutionsQuery,
want: want{
sqlExpectations: mockQueries(
regexp.QuoteMeta(prepareExecutionsStmt),
prepareExecutionsCols,
[][]driver.Value{
{
"ro",
"id-1",
testNow,
testNow,
[]byte(`[{"position" : 1, "target" : "target"}, {"position" : 3, "include" : "include"}]`),
},
{
"ro",
"id-2",
testNow,
testNow,
[]byte(`[{"position" : 2, "target" : "target"}, {"position" : 1, "include" : "include"}]`),
},
},
),
},
object: &Executions{
SearchResponse: SearchResponse{
Count: 2,
},
Executions: []*Execution{
{
ObjectDetails: domain.ObjectDetails{
ID: "id-1",
EventDate: testNow,
CreationDate: testNow,
ResourceOwner: "ro",
},
Targets: []*exec.Target{
{Type: domain.ExecutionTargetTypeTarget, Target: "target"},
{Type: domain.ExecutionTargetTypeInclude, Target: "include"},
},
},
{
ObjectDetails: domain.ObjectDetails{
ID: "id-2",
EventDate: testNow,
CreationDate: testNow,
ResourceOwner: "ro",
},
Targets: []*exec.Target{
{Type: domain.ExecutionTargetTypeInclude, Target: "include"},
{Type: domain.ExecutionTargetTypeTarget, Target: "target"},
},
},
},
},
},
{ {
name: "prepareExecutionsQuery sql err", name: "prepareExecutionsQuery sql err",
prepare: prepareExecutionsQuery, prepare: prepareExecutionsQuery,

View File

@@ -9,6 +9,7 @@ import (
"github.com/zitadel/zitadel/internal/eventstore/handler/v2" "github.com/zitadel/zitadel/internal/eventstore/handler/v2"
exec "github.com/zitadel/zitadel/internal/repository/execution" exec "github.com/zitadel/zitadel/internal/repository/execution"
"github.com/zitadel/zitadel/internal/repository/instance" "github.com/zitadel/zitadel/internal/repository/instance"
"github.com/zitadel/zitadel/internal/repository/target"
) )
const ( const (
@@ -78,6 +79,15 @@ func (p *executionProjection) Reducers() []handler.AggregateReducer {
}, },
}, },
}, },
{
Aggregate: target.AggregateType,
EventReducers: []handler.EventReducer{
{
Event: target.RemovedEventType,
Reduce: p.reduceTargetRemoved,
},
},
},
{ {
Aggregate: instance.AggregateType, Aggregate: instance.AggregateType,
EventReducers: []handler.EventReducer{ EventReducers: []handler.EventReducer{
@@ -152,6 +162,21 @@ func (p *executionProjection) reduceExecutionSet(event eventstore.Event) (*handl
return handler.NewMultiStatement(e, stmts...), nil return handler.NewMultiStatement(e, stmts...), nil
} }
func (p *executionProjection) reduceTargetRemoved(event eventstore.Event) (*handler.Statement, error) {
e, err := assertEvent[*target.RemovedEvent](event)
if err != nil {
return nil, err
}
return handler.NewDeleteStatement(
e,
[]handler.Condition{
handler.NewCond(ExecutionTargetInstanceIDCol, e.Aggregate().InstanceID),
handler.NewCond(ExecutionTargetTargetIDCol, e.Aggregate().ID),
},
handler.WithTableSuffix(ExecutionTargetSuffix),
), nil
}
func (p *executionProjection) reduceExecutionRemoved(event eventstore.Event) (*handler.Statement, error) { func (p *executionProjection) reduceExecutionRemoved(event eventstore.Event) (*handler.Statement, error) {
e, err := assertEvent[*exec.RemovedEvent](event) e, err := assertEvent[*exec.RemovedEvent](event)
if err != nil { if err != nil {

View File

@@ -7,6 +7,7 @@ import (
"github.com/zitadel/zitadel/internal/eventstore/handler/v2" "github.com/zitadel/zitadel/internal/eventstore/handler/v2"
exec "github.com/zitadel/zitadel/internal/repository/execution" exec "github.com/zitadel/zitadel/internal/repository/execution"
"github.com/zitadel/zitadel/internal/repository/instance" "github.com/zitadel/zitadel/internal/repository/instance"
"github.com/zitadel/zitadel/internal/repository/target"
"github.com/zitadel/zitadel/internal/zerrors" "github.com/zitadel/zitadel/internal/zerrors"
) )
@@ -79,6 +80,35 @@ func TestExecutionProjection_reduces(t *testing.T) {
}, },
}, },
}, },
{
name: "reduceTargetRemoved",
args: args{
event: getEvent(
testEvent(
target.RemovedEventType,
target.AggregateType,
[]byte(`{}`),
),
eventstore.GenericEventMapper[target.RemovedEvent],
),
},
reduce: (&executionProjection{}).reduceTargetRemoved,
want: wantReduce{
aggregateType: eventstore.AggregateType("target"),
sequence: 15,
executer: &testExecuter{
executions: []execution{
{
expectedStmt: "DELETE FROM projections.executions1_targets WHERE (instance_id = $1) AND (target_id = $2)",
expectedArgs: []interface{}{
"instance-id",
"agg-id",
},
},
},
},
},
},
{ {
name: "reduceExecutionRemoved", name: "reduceExecutionRemoved",
args: args{ args: args{