opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-24 15:04:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

feat: make dynamic parameters opt-in by default for new templates (#19006)

Browse Source 
resolves #18975 

---------

Co-authored-by: Steven Masley <stevenmasley@gmail.com>
Co-authored-by: blink-so[bot] <211532188+blink-so[bot]@users.noreply.github.com>
This commit is contained in:
Jaayden Halko
2025-07-28 20:41:49 -05:00
committed by GitHub
co-authored by Steven Masley blink-so[bot]
parent faac75389b
commit 1320b8d5be
35 changed files with 476 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/database/dbgen/dbgen.go
+1 -1
View File
@@ -147,7 +147,7 @@ func Template(t testing.TB, db database.Store, seed database.Template) database.
DisplayName: takeFirst(seed.DisplayName, testutil.GetRandomName(t)),
AllowUserCancelWorkspaceJobs: seed.AllowUserCancelWorkspaceJobs,
MaxPortSharingLevel: takeFirst(seed.MaxPortSharingLevel, database.AppSharingLevelOwner),
UseClassicParameterFlow: takeFirst(seed.UseClassicParameterFlow, true),
UseClassicParameterFlow: takeFirst(seed.UseClassicParameterFlow, false),
})
require.NoError(t, err, "insert template")
coderd/database/dump.sql
Generated
+1 -1
View File
@@ -1750,7 +1750,7 @@ CREATE TABLE templates (
deprecated text DEFAULT ''::text NOT NULL,
activity_bump bigint DEFAULT '3600000000000'::bigint NOT NULL,
max_port_sharing_level app_sharing_level DEFAULT 'owner'::app_sharing_level NOT NULL,
use_classic_parameter_flow boolean DEFAULT true NOT NULL
use_classic_parameter_flow boolean DEFAULT false NOT NULL
);
COMMENT ON COLUMN templates.default_ttl IS 'The default duration for autostop for workspaces created from this template.';
coderd/database/migrations/000352_default_dynamic_templates.down.sql
+1
View File
@@ -0,0 +1 @@
ALTER TABLE templates ALTER COLUMN use_classic_parameter_flow SET DEFAULT true;
coderd/database/migrations/000352_default_dynamic_templates.up.sql
+1
View File
@@ -0,0 +1 @@
ALTER TABLE templates ALTER COLUMN use_classic_parameter_flow SET DEFAULT false;
coderd/insights_test.go
+10 -8
View File
@@ -665,10 +665,11 @@ func TestTemplateInsights_Golden(t *testing.T) {
// where we can control the template ID.
// createdTemplate := coderdtest.CreateTemplate(t, client, firstUser.OrganizationID, version.ID)
createdTemplate := dbgen.Template(t, db, database.Template{
ID: template.id,
ActiveVersionID: version.ID,
OrganizationID: firstUser.OrganizationID,
CreatedBy: firstUser.UserID,
ID: template.id,
ActiveVersionID: version.ID,
OrganizationID: firstUser.OrganizationID,
CreatedBy: firstUser.UserID,
UseClassicParameterFlow: true, // Required for testing classic parameter flow behavior
GroupACL: database.TemplateACL{
firstUser.OrganizationID.String(): db2sdk.TemplateRoleActions(codersdk.TemplateRoleUse),
},
@@ -1556,10 +1557,11 @@ func TestUserActivityInsights_Golden(t *testing.T) {
// where we can control the template ID.
// createdTemplate := coderdtest.CreateTemplate(t, client, firstUser.OrganizationID, version.ID)
createdTemplate := dbgen.Template(t, db, database.Template{
ID: template.id,
ActiveVersionID: version.ID,
OrganizationID: firstUser.OrganizationID,
CreatedBy: firstUser.UserID,
ID: template.id,
ActiveVersionID: version.ID,
OrganizationID: firstUser.OrganizationID,
CreatedBy: firstUser.UserID,
UseClassicParameterFlow: true, // Required for parameter usage tracking in this test
GroupACL: database.TemplateACL{
firstUser.OrganizationID.String(): db2sdk.TemplateRoleActions(codersdk.TemplateRoleUse),
},
coderd/parameters_test.go
+34 -3
View File
@@ -3,10 +3,12 @@ package coderd_test
import (
"context"
"os"
"sync"
"testing"
"github.com/google/uuid"
"github.com/stretchr/testify/require"
"go.uber.org/atomic"
"golang.org/x/xerrors"
"github.com/coder/coder/v2/coderd"
@@ -199,8 +201,15 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) {
modulesArchive, err := terraform.GetModulesArchive(os.DirFS("testdata/parameters/modules"))
require.NoError(t, err)
c := atomic.NewInt32(0)
reject := &dbRejectGitSSHKey{Store: db, hook: func(d *dbRejectGitSSHKey) {
if c.Add(1) > 1 {
// Second call forward, reject
d.SetReject(true)
}
}}
setup := setupDynamicParamsTest(t, setupDynamicParamsTestParams{
db: &dbRejectGitSSHKey{Store: db},
db: reject,
ps: ps,
provisionerDaemonVersion: provProto.CurrentVersion.String(),
mainTF: dynamicParametersTerraformSource,
@@ -444,8 +453,30 @@ func setupDynamicParamsTest(t *testing.T, args setupDynamicParamsTestParams) dyn
// that is generally impossible to force an error.
type dbRejectGitSSHKey struct {
database.Store
rejectMu sync.RWMutex
reject bool
hook func(d *dbRejectGitSSHKey)
}
func (*dbRejectGitSSHKey) GetGitSSHKey(_ context.Context, _ uuid.UUID) (database.GitSSHKey, error) {
return database.GitSSHKey{}, xerrors.New("forcing a fake error")
// SetReject toggles whether GetGitSSHKey should return an error or passthrough to the underlying store.
func (d *dbRejectGitSSHKey) SetReject(reject bool) {
d.rejectMu.Lock()
defer d.rejectMu.Unlock()
d.reject = reject
}
func (d *dbRejectGitSSHKey) GetGitSSHKey(ctx context.Context, userID uuid.UUID) (database.GitSSHKey, error) {
if d.hook != nil {
d.hook(d)
}
d.rejectMu.RLock()
reject := d.reject
d.rejectMu.RUnlock()
if reject {
return database.GitSSHKey{}, xerrors.New("forcing a fake error")
}
return d.Store.GetGitSSHKey(ctx, userID)
}
coderd/templates.go
+2 -2
View File
@@ -197,8 +197,8 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque
return
}
// Default is true until dynamic parameters are promoted to stable.
useClassicParameterFlow := ptr.NilToDefault(createTemplate.UseClassicParameterFlow, true)
// Default is false as dynamic parameters are now the preferred approach.
useClassicParameterFlow := ptr.NilToDefault(createTemplate.UseClassicParameterFlow, false)
// Make a temporary struct to represent the template. This is used for
// auditing if any of the following checks fail. It will be overwritten when
coderd/templates_test.go
+2 -2
View File
@@ -77,7 +77,7 @@ func TestPostTemplateByOrganization(t *testing.T) {
assert.Equal(t, expected.Name, got.Name)
assert.Equal(t, expected.Description, got.Description)
assert.Equal(t, expected.ActivityBumpMillis, got.ActivityBumpMillis)
assert.Equal(t, expected.UseClassicParameterFlow, true) // Current default is true
assert.Equal(t, expected.UseClassicParameterFlow, false) // Current default is false
require.Len(t, auditor.AuditLogs(), 3)
assert.Equal(t, database.AuditActionCreate, auditor.AuditLogs()[0].Action)
@@ -1551,7 +1551,7 @@ func TestPatchTemplateMeta(t *testing.T) {
user := coderdtest.CreateFirstUser(t, client)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
require.True(t, template.UseClassicParameterFlow, "default is true")
require.False(t, template.UseClassicParameterFlow, "default is false")
bTrue := true
bFalse := false
coderd/templateversions.go
+1 -1
View File
@@ -1471,7 +1471,7 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht
return
}
var dynamicTemplate bool
dynamicTemplate := true // Default to using dynamic templates
if req.TemplateID != uuid.Nil {
tpl, err := api.Database.GetTemplateByID(ctx, req.TemplateID)
if httpapi.Is404Error(err) {
coderd/templateversions_test.go
+19 -40
View File
@@ -275,6 +275,7 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
files map[string]string
reqTags map[string]string
wantTags map[string]string
variables []codersdk.VariableValue
expectError string
}{
{
@@ -290,6 +291,7 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
default = "1"
}
data "coder_parameter" "b" {
name = "b"
type = string
default = "2"
}
@@ -311,6 +313,7 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
default = "1"
}
data "coder_parameter" "b" {
name = "b"
type = string
default = "2"
}
@@ -335,6 +338,7 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
default = "1"
}
data "coder_parameter" "b" {
name = "b"
type = string
default = "2"
}
@@ -365,6 +369,7 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
default = "1"
}
data "coder_parameter" "b" {
name = "b"
type = string
default = "2"
}
@@ -395,6 +400,7 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
default = "1"
}
data "coder_parameter" "b" {
name = "b"
type = string
default = "2"
}
@@ -429,11 +435,12 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
}
}`,
},
reqTags: map[string]string{"a": "b"},
wantTags: map[string]string{"owner": "", "scope": "organization", "a": "b"},
reqTags: map[string]string{"a": "b"},
wantTags: map[string]string{"owner": "", "scope": "organization", "a": "b"},
variables: []codersdk.VariableValue{{Name: "a", Value: "b"}},
},
{
name: "main.tf with disallowed workspace tag value",
name: "main.tf with resource reference",
files: map[string]string{
`main.tf`: `
variable "a" {
@@ -441,6 +448,7 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
default = "1"
}
data "coder_parameter" "b" {
name = "b"
type = string
default = "2"
}
@@ -461,38 +469,8 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
}
}`,
},
expectError: `Unknown variable; There is no variable named "null_resource".`,
},
{
name: "main.tf with disallowed function in tag value",
files: map[string]string{
`main.tf`: `
variable "a" {
type = string
default = "1"
}
data "coder_parameter" "b" {
type = string
default = "2"
}
data "coder_parameter" "unrelated" {
name = "unrelated"
type = "list(string)"
default = jsonencode(["a", "b"])
}
resource "null_resource" "test" {
name = "foo"
}
data "coder_workspace_tags" "tags" {
tags = {
"foo": "bar",
"a": var.a,
"b": data.coder_parameter.b.value,
"test": pathexpand("~/file.txt"),
}
}`,
},
expectError: `function "pathexpand" may not be used here`,
reqTags: map[string]string{"foo": "bar", "a": "1", "b": "2", "test": "foo"},
wantTags: map[string]string{"owner": "", "scope": "organization", "foo": "bar", "a": "1", "b": "2", "test": "foo"},
},
// We will allow coder_workspace_tags to set the scope on a template version import job
// BUT the user ID will be ultimately determined by the API key in the scope.
@@ -618,11 +596,12 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
// Create a template version from the archive
tvName := testutil.GetRandomNameHyphenated(t)
tv, err := templateAdmin.CreateTemplateVersion(ctx, owner.OrganizationID, codersdk.CreateTemplateVersionRequest{
Name: tvName,
StorageMethod: codersdk.ProvisionerStorageMethodFile,
Provisioner: codersdk.ProvisionerTypeTerraform,
FileID: fi.ID,
ProvisionerTags: tt.reqTags,
Name: tvName,
StorageMethod: codersdk.ProvisionerStorageMethodFile,
Provisioner: codersdk.ProvisionerTypeTerraform,
FileID: fi.ID,
ProvisionerTags: tt.reqTags,
UserVariableValues: tt.variables,
})
if tt.expectError == "" {
coderd/workspaces_test.go
+6 -4
View File
@@ -431,9 +431,9 @@ func TestWorkspace(t *testing.T) {
// Test Utility variables
templateVersionParameters := []*proto.RichParameter{
{Name: "param1", Type: "string", Required: false},
{Name: "param2", Type: "string", Required: false},
{Name: "param3", Type: "string", Required: false},
{Name: "param1", Type: "string", Required: false, DefaultValue: "default1"},
{Name: "param2", Type: "string", Required: false, DefaultValue: "default2"},
{Name: "param3", Type: "string", Required: false, DefaultValue: "default3"},
}
presetParameters := []*proto.PresetParameter{
{Name: "param1", Value: "value1"},
@@ -3842,7 +3842,9 @@ func TestWorkspaceWithEphemeralRichParameters(t *testing.T) {
}},
})
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(request *codersdk.CreateTemplateRequest) {
request.UseClassicParameterFlow = ptr.Ref(true) // TODO: Remove this when dynamic parameters handles this case
})
// Create workspace with default values
workspace := coderdtest.CreateWorkspace(t, client, template.ID)