diff --git a/docs/docs.go b/docs/docs.go index e08eb1ff..7538dbaa 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -63,6 +63,98 @@ const docTemplate = `{ } } }, + "/api/pipeline/actions/ack": { + "put": { + "description": "Leave an open action out of the search for a number of days, or until the worst result of its pipelines is no longer the given one", + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "Pipeline Actions" + ], + "summary": "Acknowledge an open action", + "responses": { + "204": { + "description": "No Content" + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/server.DefaultResponseModel" + } + }, + "401": { + "description": "Unauthorized", + "schema": { + "$ref": "#/definitions/server.DefaultResponseModel" + } + }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/server.DefaultResponseModel" + } + }, + "500": { + "description": "Internal Server Error", + "schema": { + "$ref": "#/definitions/server.DefaultResponseModel" + } + } + } + }, + "delete": { + "description": "Bring an acknowledged open action back in the search", + "produces": [ + "application/json" + ], + "tags": [ + "Pipeline Actions" + ], + "summary": "Unacknowledge an open action", + "parameters": [ + { + "type": "string", + "description": "Link of the action", + "name": "url", + "in": "query", + "required": true + } + ], + "responses": { + "204": { + "description": "No Content" + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/server.DefaultResponseModel" + } + }, + "401": { + "description": "Unauthorized", + "schema": { + "$ref": "#/definitions/server.DefaultResponseModel" + } + }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/server.DefaultResponseModel" + } + }, + "500": { + "description": "Internal Server Error", + "schema": { + "$ref": "#/definitions/server.DefaultResponseModel" + } + } + } + } + }, "/api/pipeline/actions/search": { "post": { "description": "Search the actions left open by the latest report of every pipeline, one entry per action URL along with the pipelines feeding it", @@ -1323,9 +1415,30 @@ const docTemplate = `{ } } }, + "database.OpenActionAcknowledgement": { + "type": "object", + "properties": { + "by": { + "description": "By is who acknowledged the action. It is empty when authentication is off.", + "type": "string" + }, + "until": { + "description": "Until is when the acknowledgement expires. It lifts earlier if the worst result changes.", + "type": "string" + } + } + }, "database.OpenActionData": { "type": "object", "properties": { + "acknowledgement": { + "description": "Acknowledgement is only set on acknowledged actions, which the search returns\nwhen SearchOpenActionsParams.Acknowledged is true.", + "allOf": [ + { + "$ref": "#/definitions/database.OpenActionAcknowledgement" + } + ] + }, "branch": { "description": "Branch is the target branch of that same scm, which the pull request is merged into.", "type": "string" @@ -2327,6 +2440,10 @@ const docTemplate = `{ "server.SearchOpenActionsResponse": { "type": "object", "properties": { + "acknowledged_count": { + "description": "AcknowledgedCount counts the acknowledged actions, whatever the request asked for.", + "type": "integer" + }, "data": { "type": "array", "items": { @@ -2334,6 +2451,7 @@ const docTemplate = `{ } }, "total_count": { + "description": "TotalCount counts the actions matching the request, across every page.", "type": "integer" } } diff --git a/docs/swagger.json b/docs/swagger.json index 2ad3eb41..fd6ae273 100644 --- a/docs/swagger.json +++ b/docs/swagger.json @@ -55,6 +55,98 @@ } } }, + "/api/pipeline/actions/ack": { + "put": { + "description": "Leave an open action out of the search for a number of days, or until the worst result of its pipelines is no longer the given one", + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "Pipeline Actions" + ], + "summary": "Acknowledge an open action", + "responses": { + "204": { + "description": "No Content" + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/server.DefaultResponseModel" + } + }, + "401": { + "description": "Unauthorized", + "schema": { + "$ref": "#/definitions/server.DefaultResponseModel" + } + }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/server.DefaultResponseModel" + } + }, + "500": { + "description": "Internal Server Error", + "schema": { + "$ref": "#/definitions/server.DefaultResponseModel" + } + } + } + }, + "delete": { + "description": "Bring an acknowledged open action back in the search", + "produces": [ + "application/json" + ], + "tags": [ + "Pipeline Actions" + ], + "summary": "Unacknowledge an open action", + "parameters": [ + { + "type": "string", + "description": "Link of the action", + "name": "url", + "in": "query", + "required": true + } + ], + "responses": { + "204": { + "description": "No Content" + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/server.DefaultResponseModel" + } + }, + "401": { + "description": "Unauthorized", + "schema": { + "$ref": "#/definitions/server.DefaultResponseModel" + } + }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/server.DefaultResponseModel" + } + }, + "500": { + "description": "Internal Server Error", + "schema": { + "$ref": "#/definitions/server.DefaultResponseModel" + } + } + } + } + }, "/api/pipeline/actions/search": { "post": { "description": "Search the actions left open by the latest report of every pipeline, one entry per action URL along with the pipelines feeding it", @@ -1315,9 +1407,30 @@ } } }, + "database.OpenActionAcknowledgement": { + "type": "object", + "properties": { + "by": { + "description": "By is who acknowledged the action. It is empty when authentication is off.", + "type": "string" + }, + "until": { + "description": "Until is when the acknowledgement expires. It lifts earlier if the worst result changes.", + "type": "string" + } + } + }, "database.OpenActionData": { "type": "object", "properties": { + "acknowledgement": { + "description": "Acknowledgement is only set on acknowledged actions, which the search returns\nwhen SearchOpenActionsParams.Acknowledged is true.", + "allOf": [ + { + "$ref": "#/definitions/database.OpenActionAcknowledgement" + } + ] + }, "branch": { "description": "Branch is the target branch of that same scm, which the pull request is merged into.", "type": "string" @@ -2319,6 +2432,10 @@ "server.SearchOpenActionsResponse": { "type": "object", "properties": { + "acknowledged_count": { + "description": "AcknowledgedCount counts the acknowledged actions, whatever the request asked for.", + "type": "integer" + }, "data": { "type": "array", "items": { @@ -2326,6 +2443,7 @@ } }, "total_count": { + "description": "TotalCount counts the actions matching the request, across every page.", "type": "integer" } } diff --git a/docs/swagger.yaml b/docs/swagger.yaml index d7cc5112..25fc7386 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -70,8 +70,25 @@ definitions: $ref: '#/definitions/transformer.Transformer' type: array type: object + database.OpenActionAcknowledgement: + properties: + by: + description: By is who acknowledged the action. It is empty when authentication + is off. + type: string + until: + description: Until is when the acknowledgement expires. It lifts earlier if + the worst result changes. + type: string + type: object database.OpenActionData: properties: + acknowledgement: + allOf: + - $ref: '#/definitions/database.OpenActionAcknowledgement' + description: |- + Acknowledgement is only set on acknowledged actions, which the search returns + when SearchOpenActionsParams.Acknowledged is true. branch: description: Branch is the target branch of that same scm, which the pull request is merged into. @@ -819,11 +836,17 @@ definitions: type: object server.SearchOpenActionsResponse: properties: + acknowledged_count: + description: AcknowledgedCount counts the acknowledged actions, whatever the + request asked for. + type: integer data: items: $ref: '#/definitions/database.OpenActionData' type: array total_count: + description: TotalCount counts the actions matching the request, across every + page. type: integer type: object server.SearchPipelineReportsSummaryRequest: @@ -1288,6 +1311,68 @@ paths: summary: Ping the API tags: - Ping + /api/pipeline/actions/ack: + delete: + description: Bring an acknowledged open action back in the search + parameters: + - description: Link of the action + in: query + name: url + required: true + type: string + produces: + - application/json + responses: + "204": + description: No Content + "400": + description: Bad Request + schema: + $ref: '#/definitions/server.DefaultResponseModel' + "401": + description: Unauthorized + schema: + $ref: '#/definitions/server.DefaultResponseModel' + "403": + description: Forbidden + schema: + $ref: '#/definitions/server.DefaultResponseModel' + "500": + description: Internal Server Error + schema: + $ref: '#/definitions/server.DefaultResponseModel' + summary: Unacknowledge an open action + tags: + - Pipeline Actions + put: + consumes: + - application/json + description: Leave an open action out of the search for a number of days, or + until the worst result of its pipelines is no longer the given one + produces: + - application/json + responses: + "204": + description: No Content + "400": + description: Bad Request + schema: + $ref: '#/definitions/server.DefaultResponseModel' + "401": + description: Unauthorized + schema: + $ref: '#/definitions/server.DefaultResponseModel' + "403": + description: Forbidden + schema: + $ref: '#/definitions/server.DefaultResponseModel' + "500": + description: Internal Server Error + schema: + $ref: '#/definitions/server.DefaultResponseModel' + summary: Acknowledge an open action + tags: + - Pipeline Actions /api/pipeline/actions/search: post: consumes: diff --git a/pkg/database/action.go b/pkg/database/action.go index 81a21b15..c6287cd1 100644 --- a/pkg/database/action.go +++ b/pkg/database/action.go @@ -4,10 +4,17 @@ import ( "context" "encoding/json" "fmt" + "slices" + "strings" "time" + "github.com/stephenafamo/bob" "github.com/stephenafamo/bob/dialect/psql" + "github.com/stephenafamo/bob/dialect/psql/dialect" + "github.com/stephenafamo/bob/dialect/psql/dm" + "github.com/stephenafamo/bob/dialect/psql/im" "github.com/stephenafamo/bob/dialect/psql/sm" + "github.com/updatecli/updatecli/pkg/core/result" ) // OpenActionPipeline is a pipeline whose latest report carries an open action. @@ -39,6 +46,17 @@ type OpenActionData struct { UpdatedAt time.Time `json:"updated_at"` // Pipelines contains every pipeline feeding the action, most recent first. Pipelines []OpenActionPipeline `json:"pipelines"` + // Acknowledgement is only set on acknowledged actions, which the search returns + // when SearchOpenActionsParams.Acknowledged is true. + Acknowledgement *OpenActionAcknowledgement `json:"acknowledgement,omitempty"` +} + +// OpenActionAcknowledgement records who acknowledged an open action, and until when. +type OpenActionAcknowledgement struct { + // Until is when the acknowledgement expires. It lifts earlier if the worst result changes. + Until time.Time `json:"until"` + // By is who acknowledged the action. It is empty when authentication is off. + By string `json:"by"` } // SearchOpenActionsParams contains the filters used to search the actions left open. @@ -54,8 +72,10 @@ type SearchOpenActionsParams struct { // Results restricts the search to the actions fed by at least one pipeline whose // latest result is one of them. An empty list does not filter anything out. Results []string - Limit int - Page int + // Acknowledged returns only the acknowledged actions instead of only the others. + Acknowledged bool + Limit int + Page int } // SearchOpenActions returns the actions left open by the latest report of every pipeline, @@ -70,7 +90,10 @@ type SearchOpenActionsParams struct { // the two. So once the pull request is merged, it is still listed from the previous report // until that report falls out of the searched time range. The reports search reads the // latest reports the same way and shares this limit. -func SearchOpenActions(params SearchOpenActionsParams) ([]OpenActionData, int, error) { +// +// It returns one page of actions, the total number of actions matching the request, +// and the number of acknowledged actions. +func SearchOpenActions(params SearchOpenActionsParams) ([]OpenActionData, int, int, error) { filteredReports := psql.Select( sm.From("pipelineReports"), sm.Columns("id", "pipeline_id", "pipeline_result", "updated_at"), @@ -84,7 +107,7 @@ func SearchOpenActions(params SearchOpenActionsParams) ([]OpenActionData, int, e StartTime: params.StartTime, EndTime: params.EndTime, }); err != nil { - return nil, 0, fmt.Errorf("applying updated_at range filter: %w", err) + return nil, 0, 0, fmt.Errorf("applying updated_at range filter: %w", err) } if len(params.Labels) > 0 { @@ -93,14 +116,14 @@ func SearchOpenActions(params SearchOpenActionsParams) ([]OpenActionData, int, e Query: &filteredReports, Labels: params.Labels, }); err != nil { - return nil, 0, fmt.Errorf("applying label filter: %w", err) + return nil, 0, 0, fmt.Errorf("applying label filter: %w", err) } } // Like the scm summary, the scm narrows the reports before the latest one of every // pipeline is picked. if err := applyScmFilter(params.Ctx, &filteredReports, params.ScmID); err != nil { - return nil, 0, fmt.Errorf("applying scm filter: %w", err) + return nil, 0, 0, fmt.Errorf("applying scm filter: %w", err) } // The latest report of every pipeline is picked from the columns alone, the payload is @@ -169,17 +192,15 @@ func SearchOpenActions(params SearchOpenActionsParams) ([]OpenActionData, int, e // // updated_at is a timestamp without time zone holding UTC, it is marked as such before // being written to json so that it reads back as RFC3339. - query := psql.Select( - sm.With("filtered_reports").As(filteredReports), - sm.With("latest_reports").As(latestReports), - sm.With("open_actions").As(openActions), + groupedActions := psql.Select( sm.Columns( "url", - "(array_agg(title ORDER BY updated_at DESC, id))[1]", - "(array_agg(repository ORDER BY updated_at DESC, id))[1]", - "(array_agg(branch ORDER BY updated_at DESC, id))[1]", + psql.Raw("(array_agg(title ORDER BY updated_at DESC, id))[1]").As("title"), + psql.Raw("(array_agg(repository ORDER BY updated_at DESC, id))[1]").As("repository"), + psql.Raw("(array_agg(branch ORDER BY updated_at DESC, id))[1]").As("branch"), psql.Raw("max(updated_at)").As("updated_at"), - "jsonb_agg(jsonb_build_object('id', id, 'name', name, 'result', pipeline_result, 'updated_at', updated_at AT TIME ZONE 'UTC') ORDER BY updated_at DESC, id)", + psql.Raw("jsonb_agg(jsonb_build_object('id', id, 'name', name, 'result', pipeline_result, 'updated_at', updated_at AT TIME ZONE 'UTC') ORDER BY updated_at DESC, id)").As("pipelines"), + worstResultExpr().As("worst_result"), ), sm.From("open_actions"), sm.GroupBy("url"), @@ -190,36 +211,81 @@ func SearchOpenActions(params SearchOpenActionsParams) ([]OpenActionData, int, e // pipelines before grouping them would instead drop the others from the action, and // take its title from whichever pipeline is left. if len(params.Results) > 0 { - query.Apply(sm.Having(psql.F("bool_or", resultInExpr(params.Results)))) + groupedActions.Apply(sm.Having(psql.F("bool_or", resultInExpr(params.Results)))) } - // Total counter query must be built before applying pagination. - totalCountQuery := psql.Select(sm.From(query).As("actions"), sm.Columns("count(*)")) + // An acknowledgement holds while it has not expired and the action's worst result + // still matches the one stored with it. The worst result comes from the reports this + // search reads, so a time range covering older reports can bring back an action that + // was acknowledged later. + // + // until is a UTC wall-clock timestamp, which is what the driver sends for a UTC time. + actions := psql.Raw(`grouped_actions AS g + LEFT JOIN action_acks AS k + ON k.url = g.url AND k.until > ? AND k.worst_result = g.worst_result`, + time.Now().UTC(), + ) + + with := []bob.Mod[*dialect.SelectQuery]{ + sm.With("filtered_reports").As(filteredReports), + sm.With("latest_reports").As(latestReports), + sm.With("open_actions").As(openActions), + sm.With("grouped_actions").As(groupedActions), + } - totalCountQueryString, totalCountArgs, err := totalCountQuery.Build(params.Ctx) + // Count before pagination. The acknowledged count lets the list say how many + // actions it hides. + countQuery := psql.Select(append(with, + sm.Columns( + "count(*) FILTER (WHERE k.url IS NULL)", + "count(*) FILTER (WHERE k.url IS NOT NULL)", + ), + sm.From(actions), + )...) + + countQueryString, countArgs, err := countQuery.Build(params.Ctx) if err != nil { - return nil, 0, fmt.Errorf("building total count query failed: %s\n\t%s", totalCountQueryString, err) + return nil, 0, 0, fmt.Errorf("building count query failed: %s\n\t%s", countQueryString, err) } - totalCount := 0 - if err := DB.QueryRow(params.Ctx, totalCountQueryString, totalCountArgs...).Scan(&totalCount); err != nil { - return nil, 0, fmt.Errorf("counting open actions: %w", err) + unacknowledgedCount, acknowledgedCount := 0, 0 + if err := DB.QueryRow(params.Ctx, countQueryString, countArgs...).Scan(&unacknowledgedCount, &acknowledgedCount); err != nil { + return nil, 0, 0, fmt.Errorf("counting open actions: %w", err) } - query.Apply( - sm.OrderBy("updated_at").Desc(), - sm.OrderBy("url"), - ) + totalCount := unacknowledgedCount + acknowledged := "k.url IS NULL" + if params.Acknowledged { + totalCount = acknowledgedCount + acknowledged = "k.url IS NOT NULL" + } + + query := psql.Select(append(with, + sm.Columns( + "g.url", + "g.title", + "g.repository", + "g.branch", + "g.updated_at", + "g.pipelines", + "k.until", + "k.acknowledged_by", + ), + sm.From(actions), + sm.Where(psql.Raw(acknowledged)), + sm.OrderBy("g.updated_at").Desc(), + sm.OrderBy("g.url"), + )...) applyPagination(&query, params.Limit, params.Page) queryString, args, err := query.Build(params.Ctx) if err != nil { - return nil, 0, fmt.Errorf("building query failed: %s\n\t%s", queryString, err) + return nil, 0, 0, fmt.Errorf("building query failed: %s\n\t%s", queryString, err) } rows, err := DB.Query(params.Ctx, queryString, args...) if err != nil { - return nil, 0, fmt.Errorf("query failed: %q\n\t%s", queryString, err) + return nil, 0, 0, fmt.Errorf("query failed: %q\n\t%s", queryString, err) } defer rows.Close() @@ -227,6 +293,8 @@ func SearchOpenActions(params SearchOpenActionsParams) ([]OpenActionData, int, e for rows.Next() { action := OpenActionData{} pipelines := []byte{} + var until *time.Time + var by *string if err := rows.Scan( &action.URL, @@ -235,20 +303,109 @@ func SearchOpenActions(params SearchOpenActionsParams) ([]OpenActionData, int, e &action.Branch, &action.UpdatedAt, &pipelines, + &until, + &by, ); err != nil { - return nil, 0, fmt.Errorf("parsing result: %s", err) + return nil, 0, 0, fmt.Errorf("parsing result: %s", err) } if err := json.Unmarshal(pipelines, &action.Pipelines); err != nil { - return nil, 0, fmt.Errorf("parsing pipelines of %q: %s", action.URL, err) + return nil, 0, 0, fmt.Errorf("parsing pipelines of %q: %s", action.URL, err) + } + + if until != nil { + action.Acknowledgement = &OpenActionAcknowledgement{Until: until.UTC(), By: *by} } dataset = append(dataset, action) } if err := rows.Err(); err != nil { - return nil, 0, fmt.Errorf("reading open actions: %w", err) + return nil, 0, 0, fmt.Errorf("reading open actions: %w", err) + } + + return dataset, totalCount, acknowledgedCount, nil +} + +// worstResultExpr selects the worst latest result among the pipelines feeding an action, +// ranked by ActionResults. An unrecognized result is ranked and returned as ResultUnknown. +// +// The results are passed as arguments because bob treats "?" in raw SQL as a placeholder. +func worstResultExpr() psql.Expression { + known := make([]any, 0, len(ActionResults)) + whens := "" + for rank, result := range ActionResults { + if result == ResultUnknown { + continue + } + known = append(known, result) + whens += fmt.Sprintf(" WHEN ? THEN %d", rank) + } + + args := []any{} + args = append(args, known...) + args = append(args, ResultUnknown) + args = append(args, known...) + args = append(args, slices.Index(ActionResults, ResultUnknown)) + + return psql.Raw(fmt.Sprintf( + "(array_agg(CASE WHEN pipeline_result IN (%s) THEN pipeline_result ELSE ? END ORDER BY CASE pipeline_result%s ELSE ? END, updated_at DESC, id))[1]", + strings.TrimSuffix(strings.Repeat("?, ", len(known)), ", "), whens, + ), args...) +} + +// ResultUnknown replaces any result Udash does not recognize. +const ResultUnknown = "?" + +// ActionResults lists the results an action can be acknowledged with, worst first: +// failure, attention (changed), unknown, skipped, success. +var ActionResults = []string{result.FAILURE, result.ATTENTION, ResultUnknown, result.SKIPPED, result.SUCCESS} + +// AcknowledgeAction hides an open action until the given time, or until its worst result +// differs from worstResult. Acknowledging an action again replaces the previous +// acknowledgement. +func AcknowledgeAction(ctx context.Context, url string, until time.Time, worstResult, by string) error { + query := psql.Insert( + im.Into("action_acks", "url", "until", "worst_result", "acknowledged_by", "created_at"), + im.Values( + psql.Arg(url), + psql.Arg(until.UTC()), + psql.Arg(worstResult), + psql.Arg(by), + psql.Arg(time.Now().UTC()), + ), + im.OnConflict("url").DoUpdate( + im.SetExcluded("until", "worst_result", "acknowledged_by", "created_at"), + ), + ) + + queryString, args, err := query.Build(ctx) + if err != nil { + return fmt.Errorf("building query failed: %s\n\t%s", queryString, err) + } + + if _, err := DB.Exec(ctx, queryString, args...); err != nil { + return fmt.Errorf("acknowledging %q: %w", url, err) + } + + return nil +} + +// UnacknowledgeAction removes the acknowledgement of an action, if there is one. +func UnacknowledgeAction(ctx context.Context, url string) error { + query := psql.Delete( + dm.From("action_acks"), + dm.Where(psql.Quote("url").EQ(psql.Arg(url))), + ) + + queryString, args, err := query.Build(ctx) + if err != nil { + return fmt.Errorf("building query failed: %s\n\t%s", queryString, err) + } + + if _, err := DB.Exec(ctx, queryString, args...); err != nil { + return fmt.Errorf("unacknowledging %q: %w", url, err) } - return dataset, totalCount, nil + return nil } diff --git a/pkg/database/database_test.go b/pkg/database/database_test.go index fad98636..e646943b 100644 --- a/pkg/database/database_test.go +++ b/pkg/database/database_test.go @@ -501,7 +501,7 @@ func TestDatabase(t *testing.T) { // search only keeps the actions seeded here, other tests share the database. search := func(results []string) map[string]OpenActionData { t.Helper() - data, totalCount, err := SearchOpenActions(SearchOpenActionsParams{ + data, totalCount, _, err := SearchOpenActions(SearchOpenActionsParams{ Ctx: ctx, Results: results, Options: ReportSearchOptions{Days: 1}, @@ -590,7 +590,7 @@ func TestDatabase(t *testing.T) { require.NoError(t, err) require.Len(t, scms, 1) - byScm, totalCount, err := SearchOpenActions(SearchOpenActionsParams{ + byScm, totalCount, _, err := SearchOpenActions(SearchOpenActionsParams{ Ctx: ctx, ScmID: scms[0].ID.String(), Options: ReportSearchOptions{Days: 1}, diff --git a/pkg/database/gc.go b/pkg/database/gc.go index 2eacfd23..5092693c 100644 --- a/pkg/database/gc.go +++ b/pkg/database/gc.go @@ -39,12 +39,13 @@ type GCResult struct { ConfigSources int64 ConfigConditions int64 ConfigTargets int64 + ActionAcks int64 } func (r GCResult) String() string { return fmt.Sprintf( - "%d reports, %d scms, %d labels, %d config sources, %d config conditions, %d config targets", - r.Reports, r.SCMs, r.Labels, r.ConfigSources, r.ConfigConditions, r.ConfigTargets, + "%d reports, %d scms, %d labels, %d config sources, %d config conditions, %d config targets, %d action acknowledgements", + r.Reports, r.SCMs, r.Labels, r.ConfigSources, r.ConfigConditions, r.ConfigTargets, r.ActionAcks, ) } @@ -121,10 +122,18 @@ var gcTables = []gcTable{ stale: unreferenced("t.updated_at", "r.config_target_ids ? t.id::text"), result: func(r *GCResult) *int64 { return &r.ConfigTargets }, }, + { + // An expired acknowledgement no longer hides anything, but it is kept for the + // report retention period as a record of who handled the action. + name: "action_acks", + stale: "t.until < $1", + result: func(r *GCResult) *int64 { return &r.ActionAcks }, + }, } // GarbageCollect deletes the reports older than the retention, then the scms, labels -// and resource configs no remaining report references. +// and resource configs no remaining report references, and the action acknowledgements +// that expired before the retention cutoff. // // Only one garbage collection deletes at a time across every udash instance sharing // the database, the others return ErrGCLocked. A dry run does not take the lock. diff --git a/pkg/database/gc_test.go b/pkg/database/gc_test.go index 8817f545..29e902bd 100644 --- a/pkg/database/gc_test.go +++ b/pkg/database/gc_test.go @@ -256,4 +256,37 @@ func TestGarbageCollect(t *testing.T) { require.NoError(t, err) assert.Equal(t, s.want, run) }) + + t.Run("deletes the action acknowledgements expired before the retention", func(t *testing.T) { + t.Cleanup(func() { + _, err := DB.Exec(ctx, "DELETE FROM action_acks") + require.NoError(t, err) + }) + + acknowledged := map[string]time.Time{ + "https://example.com/expired-long-ago": old, + "https://example.com/expired-recently": recent, + "https://example.com/still-running": now.AddDate(0, 0, 7), + } + for url, until := range acknowledged { + require.NoError(t, AcknowledgeAction(ctx, url, until, result.SUCCESS, "")) + } + + run, err := GarbageCollect(ctx, GCParams{MaxHistoryDays: maxHistoryDays, BatchSize: 5000}) + require.NoError(t, err) + assert.Equal(t, int64(1), run.ActionAcks) + + urls := []string{} + rows, err := DB.Query(ctx, "SELECT url FROM action_acks ORDER BY url") + require.NoError(t, err) + defer rows.Close() + for rows.Next() { + url := "" + require.NoError(t, rows.Scan(&url)) + urls = append(urls, url) + } + require.NoError(t, rows.Err()) + + assert.Equal(t, []string{"https://example.com/expired-recently", "https://example.com/still-running"}, urls) + }) } diff --git a/pkg/database/migrations/000016_create_action_acks.down.sql b/pkg/database/migrations/000016_create_action_acks.down.sql new file mode 100644 index 00000000..7969aee0 --- /dev/null +++ b/pkg/database/migrations/000016_create_action_acks.down.sql @@ -0,0 +1,3 @@ +BEGIN; +DROP TABLE IF EXISTS action_acks; +COMMIT; diff --git a/pkg/database/migrations/000016_create_action_acks.up.sql b/pkg/database/migrations/000016_create_action_acks.up.sql new file mode 100644 index 00000000..2aaa1712 --- /dev/null +++ b/pkg/database/migrations/000016_create_action_acks.up.sql @@ -0,0 +1,16 @@ +BEGIN; + +-- An acknowledged action is hidden from the open actions list, for every viewer, until +-- `until` passes or the worst result of its pipelines no longer equals worst_result. +-- +-- Actions are identified by url, as in the open actions search. +CREATE TABLE IF NOT EXISTS action_acks ( + url text PRIMARY KEY, + -- until and created_at hold the UTC wall clock, like pipelineReports.updated_at. + until timestamp NOT NULL, + worst_result text NOT NULL, + acknowledged_by text NOT NULL DEFAULT '', + created_at timestamp NOT NULL +); + +COMMIT; diff --git a/pkg/server/action_ack_test.go b/pkg/server/action_ack_test.go new file mode 100644 index 00000000..6b8c1128 --- /dev/null +++ b/pkg/server/action_ack_test.go @@ -0,0 +1,87 @@ +package server + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/updatecli/udash/pkg/database" + "github.com/updatecli/udash/test" +) + +// actionAckTestServer serves the acknowledgement routes behind a stubbed identity, wired +// as newGinEngine wires them. +func actionAckTestServer(t *testing.T, identity Principal) *httptest.Server { + t.Helper() + + gin.SetMode(gin.TestMode) + r := gin.New() + + group := r.Group("/api/pipeline") + group.Use(publicReadOnly(fakeIdentityAuth(identity))) + registerActionAckRoutes(group, true) + + server := httptest.NewServer(r) + t.Cleanup(server.Close) + + return server +} + +func TestActionAcknowledgementAuth(t *testing.T) { + ctx := context.Background() + + postgresContainer, err := test.SetupDatabase(t, ctx) + require.NoError(t, err) + + dbURL, err := postgresContainer.ConnectionString(ctx, "sslmode=disable") + require.NoError(t, err) + + require.NoError(t, database.Connect(database.Options{URI: dbURL})) + require.NoError(t, database.RunMigrationUp()) + + body := map[string]any{"url": "https://example.com/testing/pull/42", "days": 1, "result": "✔"} + + t.Run("an anonymous visitor cannot acknowledge, even when anybody may read", func(t *testing.T) { + srv := actionAckTestServer(t, Principal{Subject: "user-viewer", Permission: PermissionViewer}) + + status, _ := doJSON(t, http.MethodPut, srv.URL+"/api/pipeline/actions/ack", "", body) + assert.Equal(t, http.StatusUnauthorized, status) + + status, _ = doJSON(t, http.MethodDelete, srv.URL+"/api/pipeline/actions/ack?url=x", "", nil) + assert.Equal(t, http.StatusUnauthorized, status) + }) + + t.Run("an identity without any permission cannot acknowledge", func(t *testing.T) { + srv := actionAckTestServer(t, Principal{Subject: "user-nobody"}) + + status, _ := doJSON(t, http.MethodPut, srv.URL+"/api/pipeline/actions/ack", "session", body) + assert.Equal(t, http.StatusForbidden, status) + }) + + t.Run("a viewer acknowledges under their name", func(t *testing.T) { + t.Cleanup(func() { + _, err := database.DB.Exec(ctx, "DELETE FROM action_acks") + require.NoError(t, err) + }) + + srv := actionAckTestServer(t, Principal{Subject: "user-viewer", Name: "Pat", Permission: PermissionViewer}) + + status, _ := doJSON(t, http.MethodPut, srv.URL+"/api/pipeline/actions/ack", "session", body) + require.Equal(t, http.StatusNoContent, status) + + by := "" + require.NoError(t, database.DB.QueryRow(ctx, "SELECT acknowledged_by FROM action_acks").Scan(&by)) + assert.Equal(t, "Pat", by) + + status, _ = doJSON(t, http.MethodDelete, srv.URL+"/api/pipeline/actions/ack?url="+"https%3A%2F%2Fexample.com%2Ftesting%2Fpull%2F42", "session", nil) + require.Equal(t, http.StatusNoContent, status) + + count := 0 + require.NoError(t, database.DB.QueryRow(ctx, "SELECT count(*) FROM action_acks").Scan(&count)) + assert.Zero(t, count) + }) +} diff --git a/pkg/server/action_handlers.go b/pkg/server/action_handlers.go index e4e09952..f74c712d 100644 --- a/pkg/server/action_handlers.go +++ b/pkg/server/action_handlers.go @@ -1,7 +1,10 @@ package server import ( + "fmt" "net/http" + "slices" + "time" "github.com/gin-gonic/gin" "github.com/sirupsen/logrus" @@ -9,10 +12,16 @@ import ( ) type SearchOpenActionsResponse struct { - Data []database.OpenActionData `json:"data"` - TotalCount int `json:"total_count"` + Data []database.OpenActionData `json:"data"` + // TotalCount counts the actions matching the request, across every page. + TotalCount int `json:"total_count"` + // AcknowledgedCount counts the acknowledged actions, whatever the request asked for. + AcknowledgedCount int `json:"acknowledged_count"` } +// acknowledgeDays lists the accepted acknowledgement durations, in days. +var acknowledgeDays = []int{1, 7, 30} + // SearchOpenActions returns the actions left open, such as pull requests waiting to be merged // @Summary Search open actions // @Description Search the actions left open by the latest report of every pipeline, one entry per action URL along with the pipelines feeding it @@ -45,6 +54,8 @@ func SearchOpenActions(c *gin.Context) { // is one of them, such as "✔", "✗", "⚠" or "-". An empty list does not filter // anything out. Results []string `json:"results,omitempty"` + // Acknowledged returns only the acknowledged actions instead of only the others. + Acknowledged bool `json:"acknowledged"` } queryParams := queryData{} @@ -64,17 +75,18 @@ func SearchOpenActions(c *gin.Context) { return } - dataset, totalCount, err := database.SearchOpenActions( + dataset, totalCount, acknowledgedCount, err := database.SearchOpenActions( database.SearchOpenActionsParams{ - Ctx: c, - Options: database.ReportSearchOptions{Days: monitoringDurationDays}, - StartTime: queryParams.StartTime, - EndTime: queryParams.EndTime, - Labels: queryParams.Labels, - ScmID: queryParams.ScmID, - Results: queryParams.Results, - Limit: queryParams.Limit, - Page: queryParams.Page, + Ctx: c, + Options: database.ReportSearchOptions{Days: monitoringDurationDays}, + StartTime: queryParams.StartTime, + EndTime: queryParams.EndTime, + Labels: queryParams.Labels, + ScmID: queryParams.ScmID, + Results: queryParams.Results, + Acknowledged: queryParams.Acknowledged, + Limit: queryParams.Limit, + Page: queryParams.Page, }, ) if err != nil { @@ -84,7 +96,101 @@ func SearchOpenActions(c *gin.Context) { } c.JSON(http.StatusOK, SearchOpenActionsResponse{ - Data: dataset, - TotalCount: totalCount, + Data: dataset, + TotalCount: totalCount, + AcknowledgedCount: acknowledgedCount, }) } + +// AcknowledgeAction hides an open action, such as a pull request somebody is handling +// @Summary Acknowledge an open action +// @Description Leave an open action out of the search for a number of days, or until the worst result of its pipelines is no longer the given one +// @Tags Pipeline Actions +// @Accept json +// @Produce json +// @Success 204 +// @Failure 400 {object} DefaultResponseModel +// @Failure 401 {object} DefaultResponseModel +// @Failure 403 {object} DefaultResponseModel +// @Failure 500 {object} DefaultResponseModel +// @Router /api/pipeline/actions/ack [put] +func AcknowledgeAction(c *gin.Context) { + type queryData struct { + // URL is the link of the action + URL string `json:"url" binding:"required"` + // Days is how long the action stays acknowledged: 1, 7 or 30 + Days int `json:"days" binding:"required"` + // Result is the worst result shown by the search when the action was + // acknowledged, such as "✗", "⚠", "?", "-" or "✔" + Result string `json:"result" binding:"required"` + } + + queryParams := queryData{} + if err := c.ShouldBindJSON(&queryParams); err != nil { + c.JSON(http.StatusBadRequest, DefaultResponseModel{Err: err.Error()}) + return + } + + if !slices.Contains(acknowledgeDays, queryParams.Days) { + c.JSON(http.StatusBadRequest, DefaultResponseModel{Err: fmt.Sprintf("days must be one of %v", acknowledgeDays)}) + return + } + + if !slices.Contains(database.ActionResults, queryParams.Result) { + c.JSON(http.StatusBadRequest, DefaultResponseModel{Err: fmt.Sprintf("result must be one of %v", database.ActionResults)}) + return + } + + until := time.Now().UTC().AddDate(0, 0, queryParams.Days) + if err := database.AcknowledgeAction(c, queryParams.URL, until, queryParams.Result, acknowledgerFromContext(c)); err != nil { + logrus.Errorf("acknowledging an open action: %s", err) + respondWithError(c, err) + return + } + + c.Status(http.StatusNoContent) +} + +// UnacknowledgeAction removes the acknowledgement of an open action +// @Summary Unacknowledge an open action +// @Description Bring an acknowledged open action back in the search +// @Tags Pipeline Actions +// @Produce json +// @Param url query string true "Link of the action" +// @Success 204 +// @Failure 400 {object} DefaultResponseModel +// @Failure 401 {object} DefaultResponseModel +// @Failure 403 {object} DefaultResponseModel +// @Failure 500 {object} DefaultResponseModel +// @Router /api/pipeline/actions/ack [delete] +func UnacknowledgeAction(c *gin.Context) { + url := c.Query("url") + if url == "" { + c.JSON(http.StatusBadRequest, DefaultResponseModel{Err: "url is required"}) + return + } + + if err := database.UnacknowledgeAction(c, url); err != nil { + logrus.Errorf("unacknowledging an open action: %s", err) + respondWithError(c, err) + return + } + + c.Status(http.StatusNoContent) +} + +// acknowledgerFromContext returns the name shown to other viewers for whoever +// acknowledged an action. It is empty for an unauthenticated request, which is every +// request when authentication is off. +func acknowledgerFromContext(c *gin.Context) string { + principal, ok := principalFromContext(c) + if !ok { + return "" + } + + if principal.Name != "" { + return principal.Name + } + + return principal.Subject +} diff --git a/pkg/server/endpoints.go b/pkg/server/endpoints.go index 6d8554ce..cf8f547e 100644 --- a/pkg/server/endpoints.go +++ b/pkg/server/endpoints.go @@ -113,6 +113,20 @@ func publicReadOnly(auth gin.HandlerFunc) gin.HandlerFunc { } } +// registerActionAckRoutes registers the acknowledgement routes on group. When +// authenticated is true, group must already authenticate its requests, and the routes +// require a signed-in viewer even on a public instance, since an acknowledgement changes +// what every viewer sees. +func registerActionAckRoutes(group gin.IRoutes, authenticated bool) { + guard := []gin.HandlerFunc{} + if authenticated { + guard = append(guard, requirePermission(PermissionViewer)) + } + + group.PUT("/actions/ack", append(guard, AcknowledgeAction)...) + group.DELETE("/actions/ack", append(guard, UnacknowledgeAction)...) +} + func newGinEngine(opts Options) (*gin.Engine, error) { r := gin.Default() @@ -237,6 +251,8 @@ func newGinEngine(opts Options) (*gin.Engine, error) { write = append(write, requireScope(ScopeReportsWrite)) } + registerActionAckRoutes(apiPipeline, auth != nil) + apiPipeline.POST("/reports", append(write, CreatePipelineReport)...) apiPipeline.PUT("/reports/:id", append(write, UpdatePipelineReport)...) apiPipeline.DELETE("/reports/:id", append(write, DeletePipelineReport)...) diff --git a/pkg/server/endpoints_test.go b/pkg/server/endpoints_test.go index 23f417d1..1fa4d144 100644 --- a/pkg/server/endpoints_test.go +++ b/pkg/server/endpoints_test.go @@ -1100,6 +1100,116 @@ func TestEndpoints(t *testing.T) { assert.Equal(t, "https://example.com/testing/pull/42", scoped.Data[0].URL) require.Len(t, scoped.Data[0].Pipelines, 1) assert.Equal(t, successWithOpenPR, scoped.Data[0].Pipelines[0].ID) + + t.Run("acknowledged", func(t *testing.T) { + t.Cleanup(func() { + _, err := database.DB.Exec(ctx, "DELETE FROM action_acks") + require.NoError(t, err) + }) + + type acknowledgedSearch struct { + Data []struct { + URL string `json:"url"` + Acknowledgement *struct { + Until time.Time `json:"until"` + By string `json:"by"` + } `json:"acknowledgement"` + } `json:"data"` + TotalCount int `json:"total_count"` + AcknowledgedCount int `json:"acknowledged_count"` + } + + search := func(t *testing.T, acknowledged bool) acknowledgedSearch { + t.Helper() + blob := acknowledgedSearch{} + resp := doPostRequest(t, srv, "/api/pipeline/actions/search", map[string]any{"acknowledged": acknowledged}) + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode) + require.NoError(t, json.NewDecoder(resp.Body).Decode(&blob)) + return blob + } + + urls := func(blob acknowledgedSearch) []string { + found := []string{} + for _, action := range blob.Data { + found = append(found, action.URL) + } + return found + } + + acknowledge := func(t *testing.T, body map[string]any) int { + t.Helper() + status, _ := doJSON(t, http.MethodPut, srv.URL+"/api/pipeline/actions/ack", "", body) + return status + } + + pr42 := "https://example.com/testing/pull/42" + pr43 := "https://example.com/testing/pull/43" + + // Both pipelines feeding the pull request succeeded, so its worst result is ✔. + require.Equal(t, http.StatusNoContent, acknowledge(t, map[string]any{"url": pr42, "days": 7, "result": "✔"})) + + open := search(t, false) + assert.Equal(t, []string{pr43}, urls(open)) + assert.Equal(t, 1, open.TotalCount) + assert.Equal(t, 1, open.AcknowledgedCount) + + acknowledged := search(t, true) + assert.Equal(t, []string{pr42}, urls(acknowledged)) + assert.Equal(t, 1, acknowledged.TotalCount) + require.NotNil(t, acknowledged.Data[0].Acknowledgement) + assert.WithinDuration(t, time.Now().AddDate(0, 0, 7), acknowledged.Data[0].Acknowledgement.Until, time.Minute) + assert.Empty(t, acknowledged.Data[0].Acknowledgement.By, "nobody is signed in without authentication") + + t.Run("comes back once its worst result changes", func(t *testing.T) { + // The pipeline of the second manifest now fails. + failingID, err := database.InsertReport(ctx, reports.Report{ + Name: "second manifest of the same pull request", + Result: "✗", + ID: "second manifest of the same pull request", + PipelineID: "venom", + Actions: map[string]*reports.Action{ + "default": {ID: "default", Title: "Bump venom", Link: pr42}, + }, + }, database.Publisher{}) + require.NoError(t, err) + + open := search(t, false) + assert.Equal(t, []string{pr42, pr43}, urls(open)) + assert.Equal(t, 0, open.AcknowledgedCount) + + // The acknowledgement is kept and applies again once the result is back to ✔. + deleteReport(t, failingID) + assert.Equal(t, []string{pr43}, urls(search(t, false))) + }) + + t.Run("comes back once it expires", func(t *testing.T) { + _, err := database.DB.Exec(ctx, "UPDATE action_acks SET until = $1", time.Now().UTC().Add(-time.Minute)) + require.NoError(t, err) + + open := search(t, false) + assert.Equal(t, []string{pr42, pr43}, urls(open)) + assert.Equal(t, 0, open.AcknowledgedCount) + }) + + t.Run("comes back once unacknowledged", func(t *testing.T) { + require.Equal(t, http.StatusNoContent, acknowledge(t, map[string]any{"url": pr42, "days": 1, "result": "✔"})) + assert.Equal(t, []string{pr43}, urls(search(t, false))) + + status, _ := doJSON(t, http.MethodDelete, srv.URL+"/api/pipeline/actions/ack?url="+url.QueryEscape(pr42), "", nil) + require.Equal(t, http.StatusNoContent, status) + assert.Equal(t, []string{pr42, pr43}, urls(search(t, false))) + }) + + t.Run("rejects what it cannot acknowledge", func(t *testing.T) { + assert.Equal(t, http.StatusBadRequest, acknowledge(t, map[string]any{"url": pr42, "days": 3, "result": "✔"})) + assert.Equal(t, http.StatusBadRequest, acknowledge(t, map[string]any{"url": pr42, "days": 7, "result": "ok"})) + assert.Equal(t, http.StatusBadRequest, acknowledge(t, map[string]any{"days": 7, "result": "✔"})) + + status, _ := doJSON(t, http.MethodDelete, srv.URL+"/api/pipeline/actions/ack", "", nil) + assert.Equal(t, http.StatusBadRequest, status) + }) + }) }) t.Run("POST /api/pipeline/scms/search", func(t *testing.T) {