Skip to content

Commit d37e19c

Browse files
richardltrichardlt
authored
feat(api): clean workflow run secrets (#6263)
1 parent c33aa76 commit d37e19c

File tree

6 files changed

+190
-6
lines changed

6 files changed

+190
-6
lines changed

‎engine/api/api.go

+9-1
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,10 @@ type Configuration struct {
6666
} `toml:"url" comment:"#####################\n CDS URLs Settings \n####################" json:"url"`
6767
HTTP service.HTTPRouterConfiguration `toml:"http" json:"http"`
6868
Secrets struct {
69-
SkipProjectSecretsOnRegion []string `toml:"skipProjectSecretsOnRegion" json:"-"`
69+
SkipProjectSecretsOnRegion []string `toml:"skipProjectSecretsOnRegion" json:"skipProjectSecretsOnRegion" comment:"For given region, CDS will not automatically inject project's secrets when running a job."`
70+
SnapshotRetentionDelay int64 `toml:"snapshotRetentionDelay" json:"snapshotRetentionDelay" comment:"Retention delay for workflow run secrets snapshot (in days), set to 0 will keep secrets until workflow run deletion. Removing secrets will activate the read only mode on a workflow run."`
71+
SnapshotCleanInterval int64 `toml:"snapshotCleanInterval" json:"snapshotCleanInterval" comment:"Interval for secret snapshot clean (in minutes), default: 10"`
72+
SnapshotCleanBatchSize int64 `toml:"snapshotCleanBatchSize" json:"snapshotCleanBatchSize" comment:"Batch size for secret snapshot clean, default: 100"`
7073
} `toml:"secrets" json:"secrets"`
7174
Database database.DBConfiguration `toml:"database" comment:"################################\n Postgresql Database settings \n###############################" json:"database"`
7275
Cache struct {
@@ -748,6 +751,11 @@ func (a *API) Serve(ctx context.Context) error {
748751
a.GoRoutines.RunWithRestart(ctx, "workflow.ResyncWorkflowRunResultsRoutine", func(ctx context.Context) {
749752
workflow.ResyncWorkflowRunResultsRoutine(ctx, a.mustDB)
750753
})
754+
if a.Config.Secrets.SnapshotRetentionDelay > 0 {
755+
a.GoRoutines.RunWithRestart(ctx, "workflow.CleanSecretsSnapshot", func(ctx context.Context) {
756+
a.cleanWorkflowRunSecrets(ctx)
757+
})
758+
}
751759

752760
log.Info(ctx, "Bootstrapping database...")
753761
defaultValues := sdk.DefaultValues{

‎engine/api/workflow/dao_run.go

+30-5
Original file line numberDiff line numberDiff line change
@@ -353,27 +353,27 @@ func LoadRunsSummaries(ctx context.Context, db gorp.SqlExecutor, projectkey, wor
353353
SELECT workflow.id FROM workflow
354354
JOIN project ON project.id = workflow.project_id
355355
WHERE workflow.name = $2 AND project.projectkey = $1
356-
),
356+
),
357357
runs as (
358-
SELECT %s
358+
SELECT %s
359359
FROM workflow_run wr
360360
JOIN workflowID ON workflowID.id = wr.workflow_id
361361
WHERE wr.to_delete = false
362362
),
363363
tags as (
364-
SELECT workflow_run_id, tag || '=' || value "all_tags"
364+
SELECT workflow_run_id, tag || '=' || value "all_tags"
365365
FROM workflow_run_tag
366366
JOIN runs ON runs.id = workflow_run_id
367367
),
368368
aggTags as (
369-
SELECT workflow_run_id, string_agg(all_tags, ',') as tags
369+
SELECT workflow_run_id, string_agg(all_tags, ',') as tags
370370
FROM tags
371371
GROUP BY workflow_run_id
372372
)
373373
SELECT runs.*
374374
FROM runs
375375
JOIN aggTags ON aggTags.workflow_run_id = runs.id
376-
WHERE string_to_array($5, ',') <@ string_to_array(aggTags.tags, ',')
376+
WHERE string_to_array($5, ',') <@ string_to_array(aggTags.tags, ',')
377377
ORDER BY runs.start DESC OFFSET $4 LIMIT $3`, selectedColumn)
378378
var tags []string
379379
for k, v := range tagFilter {
@@ -1095,3 +1095,28 @@ func stopRunsBlocked(ctx context.Context, db *gorp.DbMap) error {
10951095
}
10961096
return nil
10971097
}
1098+
1099+
// LoadRunsIDsCreatedBefore returns the first workflow runs created before given date.
1100+
func LoadRunsIDsCreatedBefore(ctx context.Context, db gorp.SqlExecutor, date time.Time, limit int64) ([]int64, error) {
1101+
var ids []int64
1102+
query := `
1103+
SELECT id
1104+
FROM workflow_run
1105+
WHERE read_only = false AND start < $1
1106+
ORDER BY start ASC
1107+
LIMIT $2
1108+
`
1109+
if _, err := db.Select(&ids, query, date, limit); err != nil {
1110+
return nil, sdk.WithStack(err)
1111+
}
1112+
return ids, nil
1113+
}
1114+
1115+
// SetRunReadOnly set read only flag of a workflow run, this run cannot be restarted anymore.
1116+
func SetRunReadOnlyByID(ctx context.Context, db gorpmapper.SqlExecutorWithTx, workflowRunID int64) error {
1117+
query := `UPDATE workflow_run SET read_only = true WHERE id = $1`
1118+
if _, err := db.Exec(query, workflowRunID); err != nil {
1119+
return sdk.WrapError(err, "unable to set read only for workflow run with id %d", workflowRunID)
1120+
}
1121+
return nil
1122+
}

‎engine/api/workflow/dao_run_secret.go

+17
Original file line numberDiff line numberDiff line change
@@ -49,3 +49,20 @@ func loadRunSecretWithDecryption(ctx context.Context, db gorp.SqlExecutor, runID
4949
}
5050
return secrets, nil
5151
}
52+
53+
func CountRunSecretsByWorkflowRunID(ctx context.Context, db gorp.SqlExecutor, workflowRunID int64) (int64, error) {
54+
query := `SELECT COUNT(1) FROM workflow_run_secret WHERE workflow_run_id = $1`
55+
count, err := db.SelectInt(query, workflowRunID)
56+
if err != nil {
57+
return 0, sdk.WrapError(err, "unable to count workflow run secret for workflow run id %d", workflowRunID)
58+
}
59+
return count, nil
60+
}
61+
62+
func DeleteRunSecretsByWorkflowRunID(ctx context.Context, db gorpmapper.SqlExecutorWithTx, workflowRunID int64) error {
63+
query := `DELETE FROM workflow_run_secret WHERE workflow_run_id = $1`
64+
if _, err := db.Exec(query, workflowRunID); err != nil {
65+
return sdk.WrapError(err, "unable to delete workflow run secret for workflow run id %d", workflowRunID)
66+
}
67+
return nil
68+
}

‎engine/api/workflow_run_secrets.go

+64
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
package api
2+
3+
import (
4+
"context"
5+
"time"
6+
7+
"github.com/go-gorp/gorp"
8+
"github.com/rockbears/log"
9+
10+
"github.com/ovh/cds/engine/api/workflow"
11+
"github.com/ovh/cds/sdk"
12+
)
13+
14+
func (api *API) cleanWorkflowRunSecrets(ctx context.Context) {
15+
// Load workflow run older than now - snapshot retention delay
16+
maxRetentionDate := time.Now().Add(-time.Hour * time.Duration(24*api.Config.Secrets.SnapshotRetentionDelay))
17+
18+
db := api.mustDB()
19+
20+
delay := 10 * time.Minute
21+
if api.Config.Secrets.SnapshotRetentionDelay > 0 {
22+
delay = time.Duration(api.Config.Secrets.SnapshotRetentionDelay) * time.Minute
23+
}
24+
25+
limit := int64(100)
26+
if api.Config.Secrets.SnapshotCleanBatchSize > 0 {
27+
limit = api.Config.Secrets.SnapshotCleanBatchSize
28+
}
29+
30+
log.Info(ctx, "Starting workflow run secrets clean routine")
31+
32+
ticker := time.NewTicker(delay)
33+
34+
for range ticker.C {
35+
runIDs, err := workflow.LoadRunsIDsCreatedBefore(ctx, db, maxRetentionDate, limit)
36+
if err != nil {
37+
log.ErrorWithStackTrace(ctx, err)
38+
continue
39+
}
40+
for _, id := range runIDs {
41+
if err := api.cleanWorkflowRunSecretsForRun(ctx, db, id); err != nil {
42+
log.ErrorWithStackTrace(ctx, err)
43+
}
44+
}
45+
}
46+
}
47+
48+
func (api *API) cleanWorkflowRunSecretsForRun(ctx context.Context, db *gorp.DbMap, workflowRunID int64) error {
49+
tx, err := db.Begin()
50+
if err != nil {
51+
return sdk.WithStack(err)
52+
}
53+
defer tx.Rollback() // nolint
54+
if err := workflow.SetRunReadOnlyByID(ctx, tx, workflowRunID); err != nil {
55+
return sdk.WithStack(err)
56+
}
57+
if err := workflow.DeleteRunSecretsByWorkflowRunID(ctx, tx, workflowRunID); err != nil {
58+
return sdk.WithStack(err)
59+
}
60+
if err := tx.Commit(); err != nil {
61+
return sdk.WithStack(err)
62+
}
63+
return nil
64+
}

‎engine/api/workflow_run_secrets_test.go

+65
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
package api
2+
3+
import (
4+
"context"
5+
"testing"
6+
"time"
7+
8+
"github.com/stretchr/testify/require"
9+
10+
"github.com/ovh/cds/engine/api/authentication"
11+
"github.com/ovh/cds/engine/api/project"
12+
"github.com/ovh/cds/engine/api/test/assets"
13+
"github.com/ovh/cds/engine/api/workflow"
14+
"github.com/ovh/cds/sdk"
15+
)
16+
17+
func Test_cleanSecretsSnapshotForRun(t *testing.T) {
18+
ctx, cancel := context.WithCancel(context.Background())
19+
defer cancel()
20+
21+
api, db, _ := newTestAPI(t)
22+
23+
u, _ := assets.InsertAdminUser(t, db)
24+
consumer, _ := authentication.LoadConsumerByTypeAndUserID(context.TODO(), db, sdk.ConsumerLocal, u.ID, authentication.LoadConsumerOptions.WithAuthentifiedUser)
25+
projectKey := sdk.RandomString(10)
26+
p := assets.InsertTestProject(t, db, api.Cache, projectKey, projectKey)
27+
28+
require.NoError(t, project.InsertVariable(db, p.ID, &sdk.ProjectVariable{
29+
Type: sdk.SecretVariable,
30+
Name: "my-secret",
31+
Value: "my-value",
32+
}, u))
33+
34+
w := assets.InsertTestWorkflow(t, db, api.Cache, p, sdk.RandomString(10))
35+
wr, err := workflow.CreateRun(db.DbMap, w, sdk.WorkflowRunPostHandlerOption{
36+
Hook: &sdk.WorkflowNodeRunHookEvent{},
37+
})
38+
require.NoError(t, err)
39+
api.initWorkflowRun(ctx, p.Key, w, wr, sdk.WorkflowRunPostHandlerOption{
40+
Manual: &sdk.WorkflowNodeRunManual{},
41+
AuthConsumerID: consumer.ID,
42+
})
43+
44+
runIDs, err := workflow.LoadRunsIDsCreatedBefore(ctx, db, time.Now(), 100)
45+
require.NoError(t, err)
46+
require.Contains(t, runIDs, wr.ID)
47+
48+
runIDs, err = workflow.LoadRunsIDsCreatedBefore(ctx, db, wr.Start, 100)
49+
require.NoError(t, err)
50+
require.NotContains(t, runIDs, wr.ID)
51+
52+
count, err := workflow.CountRunSecretsByWorkflowRunID(ctx, db, wr.ID)
53+
require.NoError(t, err)
54+
require.Equal(t, int64(1), count)
55+
56+
require.NoError(t, api.cleanWorkflowRunSecretsForRun(ctx, db.DbMap, wr.ID))
57+
58+
result, err := workflow.LoadRunByID(ctx, db, wr.ID, workflow.LoadRunOptions{})
59+
require.NoError(t, err)
60+
require.True(t, result.ReadOnly)
61+
62+
count, err = workflow.CountRunSecretsByWorkflowRunID(ctx, db, wr.ID)
63+
require.NoError(t, err)
64+
require.Equal(t, int64(0), count)
65+
}

‎engine/sql/api/251_clean_workflow_run_secrets.sql

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
-- +migrate Up
2+
SELECT create_index('workflow_run', 'idx_workflow_run_start', 'read_only,start');
3+
4+
-- +migrate Down
5+
DROP INDEX idx_workflow_run_start;

0 commit comments

Comments
 (0)