mirror of
https://github.com/coder/coder.git
synced 2026-06-02 20:48:20 +00:00
fix: mark presets as validation_failed to prevent endless prebuild retries (#22085)
## Description - Updates `wsbuilder` to return a `BuildError` with `http.StatusBadRequest` to signify a "validation error" on missing or invalid parameters - Adds a short-circuit in `prebuilds.StoreReconciler` to mark presets for which creating a build returns a "validation error" as "validation failed" and skip further attempts to reconcile. - Adds a test to verify the above - Introduces a new Prometheus metric `coderd_prebuilt_workspaces_preset_validation_failed` to track the above Closes: https://github.com/coder/coder/issues/21237 --------- Co-authored-by: Cian Johnston <cian@coder.com>
This commit is contained in:
@@ -0,0 +1,64 @@
|
||||
package wsbuilder_test
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"testing"
|
||||
|
||||
"github.com/hashicorp/hcl/v2"
|
||||
"github.com/stretchr/testify/require"
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/dynamicparameters"
|
||||
"github.com/coder/coder/v2/coderd/wsbuilder"
|
||||
)
|
||||
|
||||
func TestBuildErrorResponseDelegation(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
t.Run("plain_error", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
be := wsbuilder.BuildError{
|
||||
Status: http.StatusBadRequest,
|
||||
Message: "bad",
|
||||
Wrapped: xerrors.New("oops"),
|
||||
}
|
||||
|
||||
status, resp := be.Response()
|
||||
require.Equal(t, http.StatusBadRequest, status)
|
||||
require.Equal(t, "bad", resp.Message)
|
||||
require.Contains(t, resp.Detail, "oops")
|
||||
require.Empty(t, resp.Validations)
|
||||
})
|
||||
|
||||
t.Run("responder_error", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
inner := &dynamicparameters.DiagnosticError{
|
||||
Message: "resolve parameters",
|
||||
KeyedDiagnostics: map[string]hcl.Diagnostics{
|
||||
"param1": {
|
||||
{
|
||||
Severity: hcl.DiagError,
|
||||
Summary: "required parameter",
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
be := wsbuilder.BuildError{
|
||||
Status: http.StatusBadRequest,
|
||||
Message: "build error wrapper",
|
||||
Wrapped: inner,
|
||||
}
|
||||
|
||||
status, resp := be.Response()
|
||||
|
||||
// Should delegate to the inner DiagnosticError's response.
|
||||
innerStatus, innerResp := inner.Response()
|
||||
require.Equal(t, innerStatus, status)
|
||||
require.Equal(t, innerResp.Message, resp.Message)
|
||||
require.Len(t, resp.Validations, 1)
|
||||
require.Equal(t, "param1", resp.Validations[0].Field)
|
||||
})
|
||||
}
|
||||
@@ -24,6 +24,7 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/dynamicparameters"
|
||||
"github.com/coder/coder/v2/coderd/files"
|
||||
"github.com/coder/coder/v2/coderd/httpapi"
|
||||
"github.com/coder/coder/v2/coderd/httpapi/httperror"
|
||||
"github.com/coder/coder/v2/coderd/prebuilds"
|
||||
"github.com/coder/coder/v2/coderd/provisionerdserver"
|
||||
"github.com/coder/coder/v2/coderd/rbac"
|
||||
@@ -280,6 +281,13 @@ func (e BuildError) Unwrap() error {
|
||||
}
|
||||
|
||||
func (e BuildError) Response() (int, codersdk.Response) {
|
||||
// If the wrapped error knows how to produce its own response
|
||||
// (e.g. DiagnosticError with Validations), prefer that over
|
||||
// the generic BuildError response.
|
||||
if inner, ok := httperror.IsResponder(e.Wrapped); ok {
|
||||
return inner.Response()
|
||||
}
|
||||
|
||||
return e.Status, codersdk.Response{
|
||||
Message: e.Message,
|
||||
Detail: e.Error(),
|
||||
@@ -875,7 +883,7 @@ func (b *Builder) getDynamicParameters() (names, values []string, err error) {
|
||||
b.richParameterValues,
|
||||
presetParameterValues)
|
||||
if err != nil {
|
||||
return nil, nil, xerrors.Errorf("resolve parameters: %w", err)
|
||||
return nil, nil, BuildError{http.StatusBadRequest, "resolve parameters", err}
|
||||
}
|
||||
|
||||
names = make([]string, 0, len(buildValues))
|
||||
@@ -1112,7 +1120,7 @@ func (b *Builder) getDynamicProvisionerTags() (map[string]string, error) {
|
||||
output, diags := render.Render(b.ctx, b.workspace.OwnerID, vals)
|
||||
tagErr := dynamicparameters.CheckTags(output, diags)
|
||||
if tagErr != nil {
|
||||
return nil, tagErr
|
||||
return nil, BuildError{http.StatusBadRequest, "workspace tags validation failed", tagErr}
|
||||
}
|
||||
|
||||
for k, v := range output.WorkspaceTags.Tags() {
|
||||
|
||||
Reference in New Issue
Block a user