chore: add dynamic parameter error if missing metadata from provisioner (#17809)

This commit is contained in:
Steven Masley
2025-05-14 12:21:36 -05:00
committed by GitHub
parent f3bcac2e90
commit 789c4beba7
14 changed files with 163 additions and 25 deletions

View File

@ -1774,6 +1774,7 @@ func (api *API) CreateInMemoryTaggedProvisionerDaemon(dialCtx context.Context, n
logger := api.Logger.Named(fmt.Sprintf("inmem-provisionerd-%s", name))
srv, err := provisionerdserver.NewServer(
api.ctx, // use the same ctx as the API
daemon.APIVersion,
api.AccessURL,
daemon.ID,
defaultOrg.ID,

View File

@ -29,6 +29,7 @@ import (
"github.com/coder/coder/v2/coderd/rbac"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/cryptorand"
"github.com/coder/coder/v2/provisionerd/proto"
"github.com/coder/coder/v2/testutil"
)
@ -1000,10 +1001,11 @@ func TemplateVersionTerraformValues(t testing.TB, db database.Store, orig databa
t.Helper()
params := database.InsertTemplateVersionTerraformValuesByJobIDParams{
JobID: takeFirst(orig.JobID, uuid.New()),
CachedPlan: takeFirstSlice(orig.CachedPlan, []byte("{}")),
CachedModuleFiles: orig.CachedModuleFiles,
UpdatedAt: takeFirst(orig.UpdatedAt, dbtime.Now()),
JobID: takeFirst(orig.JobID, uuid.New()),
CachedPlan: takeFirstSlice(orig.CachedPlan, []byte("{}")),
CachedModuleFiles: orig.CachedModuleFiles,
UpdatedAt: takeFirst(orig.UpdatedAt, dbtime.Now()),
ProvisionerdVersion: takeFirst(orig.ProvisionerdVersion, proto.CurrentVersion.String()),
}
err := db.InsertTemplateVersionTerraformValuesByJobID(genCtx, params)

View File

@ -9343,10 +9343,11 @@ func (q *FakeQuerier) InsertTemplateVersionTerraformValuesByJobID(_ context.Cont
// Insert the new row
row := database.TemplateVersionTerraformValue{
TemplateVersionID: templateVersion.ID,
CachedPlan: arg.CachedPlan,
CachedModuleFiles: arg.CachedModuleFiles,
UpdatedAt: arg.UpdatedAt,
TemplateVersionID: templateVersion.ID,
UpdatedAt: arg.UpdatedAt,
CachedPlan: arg.CachedPlan,
CachedModuleFiles: arg.CachedModuleFiles,
ProvisionerdVersion: arg.ProvisionerdVersion,
}
q.templateVersionTerraformValues = append(q.templateVersionTerraformValues, row)
return nil

View File

@ -1441,9 +1441,12 @@ CREATE TABLE template_version_terraform_values (
template_version_id uuid NOT NULL,
updated_at timestamp with time zone DEFAULT now() NOT NULL,
cached_plan jsonb NOT NULL,
cached_module_files uuid
cached_module_files uuid,
provisionerd_version text DEFAULT ''::text NOT NULL
);
COMMENT ON COLUMN template_version_terraform_values.provisionerd_version IS 'What version of the provisioning engine was used to generate the cached plan and module files.';
CREATE TABLE template_version_variables (
template_version_id uuid NOT NULL,
name text NOT NULL,

View File

@ -0,0 +1 @@
ALTER TABLE template_version_terraform_values DROP COLUMN provisionerd_version;

View File

@ -0,0 +1,4 @@
ALTER TABLE template_version_terraform_values ADD COLUMN IF NOT EXISTS provisionerd_version TEXT NOT NULL DEFAULT '';
COMMENT ON COLUMN template_version_terraform_values.provisionerd_version IS
'What version of the provisioning engine was used to generate the cached plan and module files.';

View File

@ -3225,6 +3225,8 @@ type TemplateVersionTerraformValue struct {
UpdatedAt time.Time `db:"updated_at" json:"updated_at"`
CachedPlan json.RawMessage `db:"cached_plan" json:"cached_plan"`
CachedModuleFiles uuid.NullUUID `db:"cached_module_files" json:"cached_module_files"`
// What version of the provisioning engine was used to generate the cached plan and module files.
ProvisionerdVersion string `db:"provisionerd_version" json:"provisionerd_version"`
}
type TemplateVersionVariable struct {

View File

@ -11702,7 +11702,7 @@ func (q *sqlQuerier) UpdateTemplateVersionExternalAuthProvidersByJobID(ctx conte
const getTemplateVersionTerraformValues = `-- name: GetTemplateVersionTerraformValues :one
SELECT
template_version_terraform_values.template_version_id, template_version_terraform_values.updated_at, template_version_terraform_values.cached_plan, template_version_terraform_values.cached_module_files
template_version_terraform_values.template_version_id, template_version_terraform_values.updated_at, template_version_terraform_values.cached_plan, template_version_terraform_values.cached_module_files, template_version_terraform_values.provisionerd_version
FROM
template_version_terraform_values
WHERE
@ -11717,6 +11717,7 @@ func (q *sqlQuerier) GetTemplateVersionTerraformValues(ctx context.Context, temp
&i.UpdatedAt,
&i.CachedPlan,
&i.CachedModuleFiles,
&i.ProvisionerdVersion,
)
return i, err
}
@ -11727,22 +11728,25 @@ INSERT INTO
template_version_id,
cached_plan,
cached_module_files,
updated_at
updated_at,
provisionerd_version
)
VALUES
(
(select id from template_versions where job_id = $1),
$2,
$3,
$4
$4,
$5
)
`
type InsertTemplateVersionTerraformValuesByJobIDParams struct {
JobID uuid.UUID `db:"job_id" json:"job_id"`
CachedPlan json.RawMessage `db:"cached_plan" json:"cached_plan"`
CachedModuleFiles uuid.NullUUID `db:"cached_module_files" json:"cached_module_files"`
UpdatedAt time.Time `db:"updated_at" json:"updated_at"`
JobID uuid.UUID `db:"job_id" json:"job_id"`
CachedPlan json.RawMessage `db:"cached_plan" json:"cached_plan"`
CachedModuleFiles uuid.NullUUID `db:"cached_module_files" json:"cached_module_files"`
UpdatedAt time.Time `db:"updated_at" json:"updated_at"`
ProvisionerdVersion string `db:"provisionerd_version" json:"provisionerd_version"`
}
func (q *sqlQuerier) InsertTemplateVersionTerraformValuesByJobID(ctx context.Context, arg InsertTemplateVersionTerraformValuesByJobIDParams) error {
@ -11751,6 +11755,7 @@ func (q *sqlQuerier) InsertTemplateVersionTerraformValuesByJobID(ctx context.Con
arg.CachedPlan,
arg.CachedModuleFiles,
arg.UpdatedAt,
arg.ProvisionerdVersion,
)
return err
}

View File

@ -12,12 +12,14 @@ INSERT INTO
template_version_id,
cached_plan,
cached_module_files,
updated_at
updated_at,
provisionerd_version
)
VALUES
(
(select id from template_versions where job_id = @job_id),
@cached_plan,
@cached_module_files,
@updated_at
@updated_at,
@provisionerd_version
);

View File

@ -8,9 +8,11 @@ import (
"time"
"github.com/google/uuid"
"github.com/hashicorp/hcl/v2"
"golang.org/x/sync/errgroup"
"golang.org/x/xerrors"
"github.com/coder/coder/v2/apiversion"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbauthz"
"github.com/coder/coder/v2/coderd/files"
@ -107,6 +109,9 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http
return
}
// If the err is sql.ErrNoRows, an empty terraform values struct is correct.
staticDiagnostics := parameterProvisionerVersionDiagnostic(tf)
owner, err := api.getWorkspaceOwnerData(ctx, user, templateVersion.OrganizationID)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
@ -141,7 +146,7 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http
result, diagnostics := preview.Preview(ctx, input, templateFS)
response := codersdk.DynamicParametersResponse{
ID: -1,
Diagnostics: previewtypes.Diagnostics(diagnostics),
Diagnostics: previewtypes.Diagnostics(diagnostics.Extend(staticDiagnostics)),
}
if result != nil {
response.Parameters = result.Parameters
@ -169,7 +174,7 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http
result, diagnostics := preview.Preview(ctx, input, templateFS)
response := codersdk.DynamicParametersResponse{
ID: update.ID,
Diagnostics: previewtypes.Diagnostics(diagnostics),
Diagnostics: previewtypes.Diagnostics(diagnostics.Extend(staticDiagnostics)),
}
if result != nil {
response.Parameters = result.Parameters
@ -262,3 +267,31 @@ func (api *API) getWorkspaceOwnerData(
Groups: groupNames,
}, nil
}
// parameterProvisionerVersionDiagnostic checks the version of the provisioner
// used to create the template version. If the version is less than 1.5, it
// returns a warning diagnostic. Only versions 1.5+ return the module & plan data
// required.
func parameterProvisionerVersionDiagnostic(tf database.TemplateVersionTerraformValue) hcl.Diagnostics {
missingMetadata := hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "This template version is missing required metadata to support dynamic parameters. Go back to the classic creation flow.",
Detail: "To restore full functionality, please re-import the terraform as a new template version.",
}
if tf.ProvisionerdVersion == "" {
return hcl.Diagnostics{&missingMetadata}
}
major, minor, err := apiversion.Parse(tf.ProvisionerdVersion)
if err != nil || tf.ProvisionerdVersion == "" {
return hcl.Diagnostics{&missingMetadata}
} else if major < 1 || (major == 1 && minor < 5) {
missingMetadata.Detail = "This template version does not support dynamic parameters. " +
"Some options may be missing or incorrect. " +
"Please contact an administrator to update the provisioner and re-import the template version."
return hcl.Diagnostics{&missingMetadata}
}
return nil
}

View File

@ -0,0 +1,77 @@
package coderd
import (
"testing"
"github.com/stretchr/testify/require"
"github.com/coder/coder/v2/coderd/database"
)
func Test_parameterProvisionerVersionDiagnostic(t *testing.T) {
t.Parallel()
testCases := []struct {
version string
warning bool
}{
{
version: "",
warning: true,
},
{
version: "invalid",
warning: true,
},
{
version: "0.4",
warning: true,
},
{
version: "0.5",
warning: true,
},
{
version: "0.6",
warning: true,
},
{
version: "1.4",
warning: true,
},
{
version: "1.5",
warning: false,
},
{
version: "1.6",
warning: false,
},
{
version: "2.0",
warning: false,
},
{
version: "2.5",
warning: false,
},
{
version: "2.6",
warning: false,
},
}
for _, tc := range testCases {
t.Run("Version_"+tc.version, func(t *testing.T) {
t.Parallel()
diags := parameterProvisionerVersionDiagnostic(database.TemplateVersionTerraformValue{
ProvisionerdVersion: tc.version,
})
if tc.warning {
require.Len(t, diags, 1, "expected warning")
} else {
require.Len(t, diags, 0, "expected no warning")
}
})
}
}

View File

@ -95,6 +95,7 @@ type Options struct {
}
type server struct {
apiVersion string
// lifecycleCtx must be tied to the API server's lifecycle
// as when the API server shuts down, we want to cancel any
// long-running operations.
@ -153,7 +154,9 @@ func (t Tags) Valid() error {
return nil
}
func NewServer(lifecycleCtx context.Context,
func NewServer(
lifecycleCtx context.Context,
apiVersion string,
accessURL *url.URL,
id uuid.UUID,
organizationID uuid.UUID,
@ -214,6 +217,7 @@ func NewServer(lifecycleCtx context.Context,
s := &server{
lifecycleCtx: lifecycleCtx,
apiVersion: apiVersion,
AccessURL: accessURL,
ID: id,
OrganizationID: organizationID,
@ -1536,10 +1540,11 @@ func (s *server) CompleteJob(ctx context.Context, completed *proto.CompletedJob)
}
err = s.Database.InsertTemplateVersionTerraformValuesByJobID(ctx, database.InsertTemplateVersionTerraformValuesByJobIDParams{
JobID: jobID,
UpdatedAt: now,
CachedPlan: plan,
CachedModuleFiles: fileID,
JobID: jobID,
UpdatedAt: now,
CachedPlan: plan,
CachedModuleFiles: fileID,
ProvisionerdVersion: s.apiVersion,
})
if err != nil {
return nil, xerrors.Errorf("insert template version terraform data: %w", err)

View File

@ -2993,6 +2993,7 @@ func setup(t *testing.T, ignoreLogErrors bool, ov *overrides) (proto.DRPCProvisi
srv, err := provisionerdserver.NewServer(
ov.ctx,
proto.CurrentVersion.String(),
&url.URL{},
daemon.ID,
defOrg.ID,

View File

@ -336,6 +336,7 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request)
logger.Info(ctx, "starting external provisioner daemon")
srv, err := provisionerdserver.NewServer(
srvCtx,
daemon.APIVersion,
api.AccessURL,
daemon.ID,
authRes.orgID,