From 26b28ea88c558c6a5e9fdfac6357913000f8ce1a Mon Sep 17 00:00:00 2001 From: Olivier Vernin Date: Fri, 9 Oct 2026 13:45:54 +0200 Subject: [PATCH 1/4] feat: acknowledge open actions A new action_acks table records, per action URL, until when it is acknowledged, the worst result it had then and who acknowledged it. An acknowledgement holds while it has not expired and the worst result is unchanged, so an action that gets worse comes back right away. POST /api/pipeline/actions/search hides acknowledged actions by default, lists only them with "acknowledged": true, and always returns acknowledged_count. PUT /api/pipeline/actions/ack acknowledges an action for 1, 7 or 30 days and DELETE removes the acknowledgement. Both require the viewer permission when authentication is configured. --- pkg/database/action.go | 221 +++++++++++++++--- pkg/database/database_test.go | 4 +- .../000016_create_action_acks.down.sql | 3 + .../000016_create_action_acks.up.sql | 16 ++ pkg/server/action_ack_test.go | 87 +++++++ pkg/server/action_handlers.go | 134 +++++++++-- pkg/server/endpoints.go | 16 ++ pkg/server/endpoints_test.go | 110 +++++++++ 8 files changed, 543 insertions(+), 48 deletions(-) create mode 100644 pkg/database/migrations/000016_create_action_acks.down.sql create mode 100644 pkg/database/migrations/000016_create_action_acks.up.sql create mode 100644 pkg/server/action_ack_test.go diff --git a/pkg/database/action.go b/pkg/database/action.go index 81a21b15..1ab779db 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 unrecognised 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 recognise. +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/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) { From a2f02c332a74cbefaee47e558243059b082609cc Mon Sep 17 00:00:00 2001 From: Olivier Vernin Date: Fri, 9 Oct 2026 13:46:11 +0200 Subject: [PATCH 2/4] feat(gc): delete expired action acknowledgements The garbage collector removes action_acks rows that expired before the retention, like the other tables it prunes. --- pkg/database/gc.go | 15 ++++++++++++--- pkg/database/gc_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) 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) + }) } From 968652d788b241ffe7d7cffe335bd7cc968497b8 Mon Sep 17 00:00:00 2001 From: Olivier Vernin Date: Fri, 9 Oct 2026 13:46:12 +0200 Subject: [PATCH 3/4] docs: regenerate swagger for action acknowledgements --- docs/docs.go | 118 ++++++++++++++++++++++++++++++++++++++++++++++ docs/swagger.json | 118 ++++++++++++++++++++++++++++++++++++++++++++++ docs/swagger.yaml | 85 +++++++++++++++++++++++++++++++++ 3 files changed, 321 insertions(+) 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: From 62549586a1da55f14f7b02315dfcdc0b06373e83 Mon Sep 17 00:00:00 2001 From: Olivier Vernin Date: Sat, 10 Oct 2026 13:02:17 +0200 Subject: [PATCH 4/4] chore: use american english Signed-off-by: Olivier Vernin --- pkg/database/action.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/database/action.go b/pkg/database/action.go index 1ab779db..c6287cd1 100644 --- a/pkg/database/action.go +++ b/pkg/database/action.go @@ -328,7 +328,7 @@ func SearchOpenActions(params SearchOpenActionsParams) ([]OpenActionData, int, i } // worstResultExpr selects the worst latest result among the pipelines feeding an action, -// ranked by ActionResults. An unrecognised result is ranked and returned as ResultUnknown. +// 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 { @@ -354,7 +354,7 @@ func worstResultExpr() psql.Expression { ), args...) } -// ResultUnknown replaces any result Udash does not recognise. +// ResultUnknown replaces any result Udash does not recognize. const ResultUnknown = "?" // ActionResults lists the results an action can be acknowledged with, worst first: