diff --git a/coderd/coderd.go b/coderd/coderd.go index cd8253792c354..c3f45b15e4a30 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -1597,7 +1597,7 @@ type API struct { // passed to dbauthz. AccessControlStore *atomic.Pointer[dbauthz.AccessControlStore] PortSharer atomic.Pointer[portsharing.PortSharer] - FileCache files.Cache + FileCache *files.Cache PrebuildsClaimer atomic.Pointer[prebuilds.Claimer] PrebuildsReconciler atomic.Pointer[prebuilds.ReconciliationOrchestrator] @@ -1722,13 +1722,30 @@ func compressHandler(h http.Handler) http.Handler { return cmp.Handler(h) } +type MemoryProvisionerDaemonOption func(*memoryProvisionerDaemonOptions) + +func MemoryProvisionerWithVersionOverride(version string) MemoryProvisionerDaemonOption { + return func(opts *memoryProvisionerDaemonOptions) { + opts.versionOverride = version + } +} + +type memoryProvisionerDaemonOptions struct { + versionOverride string +} + // CreateInMemoryProvisionerDaemon is an in-memory connection to a provisionerd. // Useful when starting coderd and provisionerd in the same process. func (api *API) CreateInMemoryProvisionerDaemon(dialCtx context.Context, name string, provisionerTypes []codersdk.ProvisionerType) (client proto.DRPCProvisionerDaemonClient, err error) { return api.CreateInMemoryTaggedProvisionerDaemon(dialCtx, name, provisionerTypes, nil) } -func (api *API) CreateInMemoryTaggedProvisionerDaemon(dialCtx context.Context, name string, provisionerTypes []codersdk.ProvisionerType, provisionerTags map[string]string) (client proto.DRPCProvisionerDaemonClient, err error) { +func (api *API) CreateInMemoryTaggedProvisionerDaemon(dialCtx context.Context, name string, provisionerTypes []codersdk.ProvisionerType, provisionerTags map[string]string, opts ...MemoryProvisionerDaemonOption) (client proto.DRPCProvisionerDaemonClient, err error) { + options := &memoryProvisionerDaemonOptions{} + for _, opt := range opts { + opt(options) + } + tracer := api.TracerProvider.Tracer(tracing.TracerName) clientSession, serverSession := drpcsdk.MemTransportPipe() defer func() { @@ -1755,6 +1772,12 @@ func (api *API) CreateInMemoryTaggedProvisionerDaemon(dialCtx context.Context, n return nil, xerrors.Errorf("failed to parse built-in provisioner key ID: %w", err) } + apiVersion := proto.CurrentVersion.String() + if options.versionOverride != "" && flag.Lookup("test.v") != nil { + // This should only be usable for unit testing. To fake a different provisioner version + apiVersion = options.versionOverride + } + //nolint:gocritic // in-memory provisioners are owned by system daemon, err := api.Database.UpsertProvisionerDaemon(dbauthz.AsSystemRestricted(dialCtx), database.UpsertProvisionerDaemonParams{ Name: name, @@ -1764,7 +1787,7 @@ func (api *API) CreateInMemoryTaggedProvisionerDaemon(dialCtx context.Context, n Tags: provisionersdk.MutateTags(uuid.Nil, provisionerTags), LastSeenAt: sql.NullTime{Time: dbtime.Now(), Valid: true}, Version: buildinfo.Version(), - APIVersion: proto.CurrentVersion.String(), + APIVersion: apiVersion, KeyID: keyID, }) if err != nil { diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 85f000939d75e..a25f0576e76be 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -135,6 +135,7 @@ type Options struct { // IncludeProvisionerDaemon when true means to start an in-memory provisionerD IncludeProvisionerDaemon bool + ProvisionerDaemonVersion string ProvisionerDaemonTags map[string]string MetricsCacheRefreshInterval time.Duration AgentStatsRefreshInterval time.Duration @@ -601,7 +602,7 @@ func NewWithAPI(t testing.TB, options *Options) (*codersdk.Client, io.Closer, *c setHandler(rootHandler) var provisionerCloser io.Closer = nopcloser{} if options.IncludeProvisionerDaemon { - provisionerCloser = NewTaggedProvisionerDaemon(t, coderAPI, "test", options.ProvisionerDaemonTags) + provisionerCloser = NewTaggedProvisionerDaemon(t, coderAPI, "test", options.ProvisionerDaemonTags, coderd.MemoryProvisionerWithVersionOverride(options.ProvisionerDaemonVersion)) } client := codersdk.New(serverURL) t.Cleanup(func() { @@ -648,7 +649,7 @@ func NewProvisionerDaemon(t testing.TB, coderAPI *coderd.API) io.Closer { return NewTaggedProvisionerDaemon(t, coderAPI, "test", nil) } -func NewTaggedProvisionerDaemon(t testing.TB, coderAPI *coderd.API, name string, provisionerTags map[string]string) io.Closer { +func NewTaggedProvisionerDaemon(t testing.TB, coderAPI *coderd.API, name string, provisionerTags map[string]string, opts ...coderd.MemoryProvisionerDaemonOption) io.Closer { t.Helper() // t.Cleanup runs in last added, first called order. t.TempDir() will delete @@ -676,7 +677,7 @@ func NewTaggedProvisionerDaemon(t testing.TB, coderAPI *coderd.API, name string, connectedCh := make(chan struct{}) daemon := provisionerd.New(func(dialCtx context.Context) (provisionerdproto.DRPCProvisionerDaemonClient, error) { - return coderAPI.CreateInMemoryTaggedProvisionerDaemon(dialCtx, name, []codersdk.ProvisionerType{codersdk.ProvisionerTypeEcho}, provisionerTags) + return coderAPI.CreateInMemoryTaggedProvisionerDaemon(dialCtx, name, []codersdk.ProvisionerType{codersdk.ProvisionerTypeEcho}, provisionerTags, opts...) }, &provisionerd.Options{ Logger: coderAPI.Logger.Named("provisionerd").Leveled(slog.LevelDebug), UpdateInterval: 250 * time.Millisecond, diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index e152960a26ef7..a0289f222392b 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -1214,8 +1214,8 @@ func (s *MethodTestSuite) TestTemplate() { JobID: job.ID, TemplateID: uuid.NullUUID{UUID: t.ID, Valid: true}, }) - dbgen.TemplateVersionTerraformValues(s.T(), db, database.InsertTemplateVersionTerraformValuesByJobIDParams{ - JobID: job.ID, + dbgen.TemplateVersionTerraformValues(s.T(), db, database.TemplateVersionTerraformValue{ + TemplateVersionID: tv.ID, }) check.Args(tv.ID).Asserts(t, policy.ActionRead) })) diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index 5069ce0cbe0ad..286c80f1c2143 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -998,11 +998,19 @@ func TemplateVersionParameter(t testing.TB, db database.Store, orig database.Tem return version } -func TemplateVersionTerraformValues(t testing.TB, db database.Store, orig database.InsertTemplateVersionTerraformValuesByJobIDParams) { +func TemplateVersionTerraformValues(t testing.TB, db database.Store, orig database.TemplateVersionTerraformValue) database.TemplateVersionTerraformValue { t.Helper() + jobID := uuid.New() + if orig.TemplateVersionID != uuid.Nil { + v, err := db.GetTemplateVersionByID(genCtx, orig.TemplateVersionID) + if err == nil { + jobID = v.JobID + } + } + params := database.InsertTemplateVersionTerraformValuesByJobIDParams{ - JobID: takeFirst(orig.JobID, uuid.New()), + JobID: jobID, CachedPlan: takeFirstSlice(orig.CachedPlan, []byte("{}")), CachedModuleFiles: orig.CachedModuleFiles, UpdatedAt: takeFirst(orig.UpdatedAt, dbtime.Now()), @@ -1011,6 +1019,11 @@ func TemplateVersionTerraformValues(t testing.TB, db database.Store, orig databa err := db.InsertTemplateVersionTerraformValuesByJobID(genCtx, params) require.NoError(t, err, "insert template version parameter") + + v, err := db.GetTemplateVersionTerraformValues(genCtx, orig.TemplateVersionID) + require.NoError(t, err, "get template version values") + + return v } func WorkspaceAgentStat(t testing.TB, db database.Store, orig database.WorkspaceAgentStat) database.WorkspaceAgentStat { diff --git a/coderd/files/cache.go b/coderd/files/cache.go index ccdf9abff2ebb..56e9a715de189 100644 --- a/coderd/files/cache.go +++ b/coderd/files/cache.go @@ -16,7 +16,7 @@ import ( // NewFromStore returns a file cache that will fetch files from the provided // database. -func NewFromStore(store database.Store) Cache { +func NewFromStore(store database.Store) *Cache { fetcher := func(ctx context.Context, fileID uuid.UUID) (fs.FS, error) { file, err := store.GetFileByID(ctx, fileID) if err != nil { @@ -27,7 +27,7 @@ func NewFromStore(store database.Store) Cache { return archivefs.FromTarReader(content), nil } - return Cache{ + return &Cache{ lock: sync.Mutex{}, data: make(map[uuid.UUID]*cacheEntry), fetcher: fetcher, @@ -112,3 +112,12 @@ func (c *Cache) Release(fileID uuid.UUID) { delete(c.data, fileID) } + +// Count returns the number of files currently in the cache. +// Mainly used for unit testing assertions. +func (c *Cache) Count() int { + c.lock.Lock() + defer c.lock.Unlock() + + return len(c.data) +} diff --git a/coderd/parameters.go b/coderd/parameters.go index 8d32096f29522..c3fc4ffdeeede 100644 --- a/coderd/parameters.go +++ b/coderd/parameters.go @@ -18,10 +18,13 @@ import ( "github.com/coder/coder/v2/coderd/files" "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/httpmw" + "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/wsjson" + sdkproto "github.com/coder/coder/v2/provisionersdk/proto" "github.com/coder/preview" previewtypes "github.com/coder/preview/types" + "github.com/coder/terraform-provider-coder/v2/provider" "github.com/coder/websocket" ) @@ -34,9 +37,7 @@ import ( // @Success 101 // @Router /users/{user}/templateversions/{templateversion}/parameters [get] func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http.Request) { - ctx, cancel := context.WithTimeout(r.Context(), 30*time.Minute) - defer cancel() - user := httpmw.UserParam(r) + ctx := r.Context() templateVersion := httpmw.TemplateVersionParam(r) // Check that the job has completed successfully @@ -59,6 +60,33 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http return } + tf, err := api.Database.GetTemplateVersionTerraformValues(ctx, templateVersion.ID) + if err != nil && !xerrors.Is(err, sql.ErrNoRows) { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to retrieve Terraform values for template version", + Detail: err.Error(), + }) + return + } + + major, minor, err := apiversion.Parse(tf.ProvisionerdVersion) + // If the api version is not valid or less than 1.5, we need to use the static parameters + useStaticParams := err != nil || major < 1 || (major == 1 && minor < 6) + if useStaticParams { + api.handleStaticParameters(rw, r, templateVersion.ID) + } else { + api.handleDynamicParameters(rw, r, tf, templateVersion) + } +} + +type previewFunction func(ctx context.Context, values map[string]string) (*preview.Output, hcl.Diagnostics) + +func (api *API) handleDynamicParameters(rw http.ResponseWriter, r *http.Request, tf database.TemplateVersionTerraformValue, templateVersion database.TemplateVersion) { + var ( + ctx = r.Context() + user = httpmw.UserParam(r) + ) + // nolint:gocritic // We need to fetch the templates files for the Terraform // evaluator, and the user likely does not have permission. fileCtx := dbauthz.AsProvisionerd(ctx) @@ -71,6 +99,7 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http return } + // Add the file first. Calling `Release` if it fails is a no-op, so this is safe. templateFS, err := api.FileCache.Acquire(fileCtx, fileID) if err != nil { httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ @@ -85,34 +114,25 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http // for populating values from data blocks, but isn't strictly required. If // we don't have a cached plan available, we just use an empty one instead. plan := json.RawMessage("{}") - tf, err := api.Database.GetTemplateVersionTerraformValues(ctx, templateVersion.ID) - if err == nil { + if len(tf.CachedPlan) > 0 { plan = tf.CachedPlan + } - if tf.CachedModuleFiles.Valid { - moduleFilesFS, err := api.FileCache.Acquire(fileCtx, tf.CachedModuleFiles.UUID) - if err != nil { - httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ - Message: "Internal error fetching Terraform modules.", - Detail: err.Error(), - }) - return - } - defer api.FileCache.Release(tf.CachedModuleFiles.UUID) - templateFS = files.NewOverlayFS(templateFS, []files.Overlay{{Path: ".terraform/modules", FS: moduleFilesFS}}) + if tf.CachedModuleFiles.Valid { + moduleFilesFS, err := api.FileCache.Acquire(fileCtx, tf.CachedModuleFiles.UUID) + if err != nil { + httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ + Message: "Internal error fetching Terraform modules.", + Detail: err.Error(), + }) + return } - } else if !xerrors.Is(err, sql.ErrNoRows) { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Failed to retrieve Terraform values for template version", - Detail: err.Error(), - }) - return - } + defer api.FileCache.Release(tf.CachedModuleFiles.UUID) - // If the err is sql.ErrNoRows, an empty terraform values struct is correct. - staticDiagnostics := parameterProvisionerVersionDiagnostic(tf) + templateFS = files.NewOverlayFS(templateFS, []files.Overlay{{Path: ".terraform/modules", FS: moduleFilesFS}}) + } - owner, err := api.getWorkspaceOwnerData(ctx, user, templateVersion.OrganizationID) + owner, err := getWorkspaceOwnerData(ctx, api.Database, user, templateVersion.OrganizationID) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error fetching workspace owner.", @@ -127,6 +147,129 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http Owner: owner, } + api.handleParameterWebsocket(rw, r, func(ctx context.Context, values map[string]string) (*preview.Output, hcl.Diagnostics) { + // Update the input values with the new values. + // The rest of the input is unchanged. + input.ParameterValues = values + return preview.Preview(ctx, input, templateFS) + }) +} + +func (api *API) handleStaticParameters(rw http.ResponseWriter, r *http.Request, version uuid.UUID) { + ctx := r.Context() + dbTemplateVersionParameters, err := api.Database.GetTemplateVersionParameters(ctx, version) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to retrieve template version parameters", + Detail: err.Error(), + }) + return + } + + params := make([]previewtypes.Parameter, 0, len(dbTemplateVersionParameters)) + for _, it := range dbTemplateVersionParameters { + param := previewtypes.Parameter{ + ParameterData: previewtypes.ParameterData{ + Name: it.Name, + DisplayName: it.DisplayName, + Description: it.Description, + Type: previewtypes.ParameterType(it.Type), + FormType: "", // ooooof + Styling: previewtypes.ParameterStyling{}, + Mutable: it.Mutable, + DefaultValue: previewtypes.StringLiteral(it.DefaultValue), + Icon: it.Icon, + Options: make([]*previewtypes.ParameterOption, 0), + Validations: make([]*previewtypes.ParameterValidation, 0), + Required: it.Required, + Order: int64(it.DisplayOrder), + Ephemeral: it.Ephemeral, + Source: nil, + }, + // Always use the default, since we used to assume the empty string + Value: previewtypes.StringLiteral(it.DefaultValue), + Diagnostics: nil, + } + + if it.ValidationError != "" || it.ValidationRegex != "" || it.ValidationMonotonic != "" { + var reg *string + if it.ValidationRegex != "" { + reg = ptr.Ref(it.ValidationRegex) + } + + var vMin *int64 + if it.ValidationMin.Valid { + vMin = ptr.Ref(int64(it.ValidationMin.Int32)) + } + + var vMax *int64 + if it.ValidationMax.Valid { + vMin = ptr.Ref(int64(it.ValidationMax.Int32)) + } + + var monotonic *string + if it.ValidationMonotonic != "" { + monotonic = ptr.Ref(it.ValidationMonotonic) + } + + param.Validations = append(param.Validations, &previewtypes.ParameterValidation{ + Error: it.ValidationError, + Regex: reg, + Min: vMin, + Max: vMax, + Monotonic: monotonic, + }) + } + + var protoOptions []*sdkproto.RichParameterOption + _ = json.Unmarshal(it.Options, &protoOptions) // Not going to make this fatal + for _, opt := range protoOptions { + param.Options = append(param.Options, &previewtypes.ParameterOption{ + Name: opt.Name, + Description: opt.Description, + Value: previewtypes.StringLiteral(opt.Value), + Icon: opt.Icon, + }) + } + + // Take the form type from the ValidateFormType function. This is a bit + // unfortunate we have to do this, but it will return the default form_type + // for a given set of conditions. + _, param.FormType, _ = provider.ValidateFormType(provider.OptionType(param.Type), len(param.Options), param.FormType) + + param.Diagnostics = previewtypes.Diagnostics(param.Valid(param.Value)) + params = append(params, param) + } + + api.handleParameterWebsocket(rw, r, func(_ context.Context, values map[string]string) (*preview.Output, hcl.Diagnostics) { + for i := range params { + param := ¶ms[i] + paramValue, ok := values[param.Name] + if ok { + param.Value = previewtypes.StringLiteral(paramValue) + } else { + param.Value = param.DefaultValue + } + param.Diagnostics = previewtypes.Diagnostics(param.Valid(param.Value)) + } + + return &preview.Output{ + Parameters: params, + }, hcl.Diagnostics{ + { + // Only a warning because the form does still work. + Severity: hcl.DiagWarning, + Summary: "This template version is missing required metadata to support dynamic parameters.", + Detail: "To restore full functionality, please re-import the terraform as a new template version.", + }, + } + }) +} + +func (api *API) handleParameterWebsocket(rw http.ResponseWriter, r *http.Request, render previewFunction) { + ctx, cancel := context.WithTimeout(r.Context(), 30*time.Minute) + defer cancel() + conn, err := websocket.Accept(rw, r, nil) if err != nil { httpapi.Write(ctx, rw, http.StatusUpgradeRequired, codersdk.Response{ @@ -143,10 +286,10 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http ) // Send an initial form state, computed without any user input. - result, diagnostics := preview.Preview(ctx, input, templateFS) + result, diagnostics := render(ctx, map[string]string{}) response := codersdk.DynamicParametersResponse{ - ID: -1, - Diagnostics: previewtypes.Diagnostics(diagnostics.Extend(staticDiagnostics)), + ID: -1, // Always start with -1. + Diagnostics: previewtypes.Diagnostics(diagnostics), } if result != nil { response.Parameters = result.Parameters @@ -170,11 +313,11 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http // The connection has been closed, so there is no one to write to return } - input.ParameterValues = update.Inputs - result, diagnostics := preview.Preview(ctx, input, templateFS) + + result, diagnostics := render(ctx, update.Inputs) response := codersdk.DynamicParametersResponse{ ID: update.ID, - Diagnostics: previewtypes.Diagnostics(diagnostics.Extend(staticDiagnostics)), + Diagnostics: previewtypes.Diagnostics(diagnostics), } if result != nil { response.Parameters = result.Parameters @@ -188,8 +331,9 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http } } -func (api *API) getWorkspaceOwnerData( +func getWorkspaceOwnerData( ctx context.Context, + db database.Store, user database.User, organizationID uuid.UUID, ) (previewtypes.WorkspaceOwner, error) { @@ -200,7 +344,7 @@ func (api *API) getWorkspaceOwnerData( // nolint:gocritic // This is kind of the wrong query to use here, but it // matches how the provisioner currently works. We should figure out // something that needs less escalation but has the correct behavior. - row, err := api.Database.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx), user.ID) + row, err := db.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx), user.ID) if err != nil { return err } @@ -227,7 +371,10 @@ func (api *API) getWorkspaceOwnerData( var publicKey string g.Go(func() error { - key, err := api.Database.GetGitSSHKey(ctx, user.ID) + // The correct public key has to be sent. This will not be leaked + // unless the template leaks it. + // nolint:gocritic + key, err := db.GetGitSSHKey(dbauthz.AsSystemRestricted(ctx), user.ID) if err != nil { return err } @@ -237,7 +384,11 @@ func (api *API) getWorkspaceOwnerData( var groupNames []string g.Go(func() error { - groups, err := api.Database.GetGroups(ctx, database.GetGroupsParams{ + // The groups need to be sent to preview. These groups are not exposed to the + // user, unless the template does it through the parameters. Regardless, we need + // the correct groups, and a user might not have read access. + // nolint:gocritic + groups, err := db.GetGroups(dbauthz.AsSystemRestricted(ctx), database.GetGroupsParams{ OrganizationID: organizationID, HasMemberID: user.ID, }) @@ -267,31 +418,3 @@ 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 -} diff --git a/coderd/parameters_internal_test.go b/coderd/parameters_internal_test.go deleted file mode 100644 index a02baeae380b6..0000000000000 --- a/coderd/parameters_internal_test.go +++ /dev/null @@ -1,77 +0,0 @@ -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") - } - }) - } -} diff --git a/coderd/parameters_test.go b/coderd/parameters_test.go index f335f60f2b8cf..e7fc77f141efc 100644 --- a/coderd/parameters_test.go +++ b/coderd/parameters_test.go @@ -1,22 +1,31 @@ package coderd_test import ( + "context" "os" "testing" + "github.com/google/uuid" "github.com/stretchr/testify/require" + "golang.org/x/xerrors" + "github.com/coder/coder/v2/coderd" "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/database/pubsub" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/codersdk/wsjson" "github.com/coder/coder/v2/provisioner/echo" "github.com/coder/coder/v2/provisioner/terraform" + provProto "github.com/coder/coder/v2/provisionerd/proto" "github.com/coder/coder/v2/provisionersdk/proto" "github.com/coder/coder/v2/testutil" "github.com/coder/websocket" ) -func TestDynamicParametersOwnerGroups(t *testing.T) { +func TestDynamicParametersOwnerSSHPublicKey(t *testing.T) { t.Parallel() cfg := coderdtest.DeploymentValues(t) @@ -25,9 +34,11 @@ func TestDynamicParametersOwnerGroups(t *testing.T) { owner := coderdtest.CreateFirstUser(t, ownerClient) templateAdmin, templateAdminUser := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin()) - dynamicParametersTerraformSource, err := os.ReadFile("testdata/parameters/groups/main.tf") + dynamicParametersTerraformSource, err := os.ReadFile("testdata/parameters/public_key/main.tf") + require.NoError(t, err) + dynamicParametersTerraformPlan, err := os.ReadFile("testdata/parameters/public_key/plan.json") require.NoError(t, err) - dynamicParametersTerraformPlan, err := os.ReadFile("testdata/parameters/groups/plan.json") + sshKey, err := templateAdmin.GitSSHKey(t.Context(), "me") require.NoError(t, err) files := echo.WithExtraFiles(map[string][]byte{ @@ -56,106 +67,192 @@ func TestDynamicParametersOwnerGroups(t *testing.T) { preview := testutil.RequireReceive(ctx, t, previews) require.Equal(t, -1, preview.ID) require.Empty(t, preview.Diagnostics) - require.Equal(t, "group", preview.Parameters[0].Name) - require.True(t, preview.Parameters[0].Value.Valid()) - require.Equal(t, "Everyone", preview.Parameters[0].Value.Value.AsString()) - - // Send a new value, and see it reflected - err = stream.Send(codersdk.DynamicParametersRequest{ - ID: 1, - Inputs: map[string]string{"group": "Bloob"}, - }) - require.NoError(t, err) - preview = testutil.RequireReceive(ctx, t, previews) - require.Equal(t, 1, preview.ID) - require.Empty(t, preview.Diagnostics) - require.Equal(t, "group", preview.Parameters[0].Name) - require.True(t, preview.Parameters[0].Value.Valid()) - require.Equal(t, "Bloob", preview.Parameters[0].Value.Value.AsString()) - - // Back to default - err = stream.Send(codersdk.DynamicParametersRequest{ - ID: 3, - Inputs: map[string]string{}, - }) - require.NoError(t, err) - preview = testutil.RequireReceive(ctx, t, previews) - require.Equal(t, 3, preview.ID) - require.Empty(t, preview.Diagnostics) - require.Equal(t, "group", preview.Parameters[0].Name) + require.Equal(t, "public_key", preview.Parameters[0].Name) require.True(t, preview.Parameters[0].Value.Valid()) - require.Equal(t, "Everyone", preview.Parameters[0].Value.Value.AsString()) + require.Equal(t, sshKey.PublicKey, preview.Parameters[0].Value.Value.AsString()) } -func TestDynamicParametersOwnerSSHPublicKey(t *testing.T) { +func TestDynamicParametersWithTerraformValues(t *testing.T) { t.Parallel() - cfg := coderdtest.DeploymentValues(t) - cfg.Experiments = []string{string(codersdk.ExperimentDynamicParameters)} - ownerClient := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, DeploymentValues: cfg}) - owner := coderdtest.CreateFirstUser(t, ownerClient) - templateAdmin, templateAdminUser := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin()) + t.Run("OK_Modules", func(t *testing.T) { + t.Parallel() - dynamicParametersTerraformSource, err := os.ReadFile("testdata/parameters/public_key/main.tf") - require.NoError(t, err) - dynamicParametersTerraformPlan, err := os.ReadFile("testdata/parameters/public_key/plan.json") - require.NoError(t, err) - sshKey, err := templateAdmin.GitSSHKey(t.Context(), "me") - require.NoError(t, err) + dynamicParametersTerraformSource, err := os.ReadFile("testdata/parameters/modules/main.tf") + require.NoError(t, err) - files := echo.WithExtraFiles(map[string][]byte{ - "main.tf": dynamicParametersTerraformSource, + modulesArchive, err := terraform.GetModulesArchive(os.DirFS("testdata/parameters/modules")) + require.NoError(t, err) + + setup := setupDynamicParamsTest(t, setupDynamicParamsTestParams{ + provisionerDaemonVersion: provProto.CurrentVersion.String(), + mainTF: dynamicParametersTerraformSource, + modulesArchive: modulesArchive, + plan: nil, + static: nil, + }) + + ctx := testutil.Context(t, testutil.WaitShort) + stream := setup.stream + previews := stream.Chan() + + // Should see the output of the module represented + preview := testutil.RequireReceive(ctx, t, previews) + require.Equal(t, -1, preview.ID) + require.Empty(t, preview.Diagnostics) + + require.Len(t, preview.Parameters, 1) + require.Equal(t, "jetbrains_ide", preview.Parameters[0].Name) + require.True(t, preview.Parameters[0].Value.Valid()) + require.Equal(t, "CL", preview.Parameters[0].Value.AsString()) }) - files.ProvisionPlan = []*proto.Response{{ - Type: &proto.Response_Plan{ - Plan: &proto.PlanComplete{ - Plan: dynamicParametersTerraformPlan, + + // OldProvisioners use the static parameters in the dynamic param flow + t.Run("OldProvisioner", func(t *testing.T) { + t.Parallel() + + const defaultValue = "PS" + setup := setupDynamicParamsTest(t, setupDynamicParamsTestParams{ + provisionerDaemonVersion: "1.4", + mainTF: nil, + modulesArchive: nil, + plan: nil, + static: []*proto.RichParameter{ + { + Name: "jetbrains_ide", + Type: "string", + DefaultValue: defaultValue, + Icon: "", + Options: []*proto.RichParameterOption{ + { + Name: "PHPStorm", + Description: "", + Value: defaultValue, + Icon: "", + }, + { + Name: "Golang", + Description: "", + Value: "GO", + Icon: "", + }, + }, + ValidationRegex: "[PG][SO]", + ValidationError: "Regex check", + }, }, - }, - }} + }) - version := coderdtest.CreateTemplateVersion(t, templateAdmin, owner.OrganizationID, files) - coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdmin, version.ID) - _ = coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, version.ID) + ctx := testutil.Context(t, testutil.WaitShort) + stream := setup.stream + previews := stream.Chan() - ctx := testutil.Context(t, testutil.WaitShort) - stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, templateAdminUser.ID, version.ID) - require.NoError(t, err) - defer stream.Close(websocket.StatusGoingAway) + // Assert the initial state + preview := testutil.RequireReceive(ctx, t, previews) + diagCount := len(preview.Diagnostics) + require.Equal(t, 1, diagCount) + require.Contains(t, preview.Diagnostics[0].Summary, "required metadata to support dynamic parameters") + require.Len(t, preview.Parameters, 1) + require.Equal(t, "jetbrains_ide", preview.Parameters[0].Name) + require.True(t, preview.Parameters[0].Value.Valid()) + require.Equal(t, defaultValue, preview.Parameters[0].Value.AsString()) - previews := stream.Chan() + // Test some inputs + for _, exp := range []string{defaultValue, "GO", "Invalid", defaultValue} { + inputs := map[string]string{} + if exp != defaultValue { + // Let the default value be the default without being explicitly set + inputs["jetbrains_ide"] = exp + } + err := stream.Send(codersdk.DynamicParametersRequest{ + ID: 1, + Inputs: inputs, + }) + require.NoError(t, err) - // Should automatically send a form state with all defaulted/empty values - preview := testutil.RequireReceive(ctx, t, previews) - require.Equal(t, -1, preview.ID) - require.Empty(t, preview.Diagnostics) - require.Equal(t, "public_key", preview.Parameters[0].Name) - require.True(t, preview.Parameters[0].Value.Valid()) - require.Equal(t, sshKey.PublicKey, preview.Parameters[0].Value.Value.AsString()) + preview := testutil.RequireReceive(ctx, t, previews) + diagCount := len(preview.Diagnostics) + require.Equal(t, 1, diagCount) + require.Contains(t, preview.Diagnostics[0].Summary, "required metadata to support dynamic parameters") + + require.Len(t, preview.Parameters, 1) + if exp == "Invalid" { // Try an invalid option + require.Len(t, preview.Parameters[0].Diagnostics, 1) + } else { + require.Len(t, preview.Parameters[0].Diagnostics, 0) + } + require.Equal(t, "jetbrains_ide", preview.Parameters[0].Name) + require.True(t, preview.Parameters[0].Value.Valid()) + require.Equal(t, exp, preview.Parameters[0].Value.AsString()) + } + }) + + t.Run("FileError", func(t *testing.T) { + // Verify files close even if the websocket terminates from an error + t.Parallel() + + db, ps := dbtestutil.NewDB(t) + dynamicParametersTerraformSource, err := os.ReadFile("testdata/parameters/modules/main.tf") + require.NoError(t, err) + + modulesArchive, err := terraform.GetModulesArchive(os.DirFS("testdata/parameters/modules")) + require.NoError(t, err) + + setup := setupDynamicParamsTest(t, setupDynamicParamsTestParams{ + db: &dbRejectGitSSHKey{Store: db}, + ps: ps, + provisionerDaemonVersion: provProto.CurrentVersion.String(), + mainTF: dynamicParametersTerraformSource, + modulesArchive: modulesArchive, + expectWebsocketError: true, + }) + // This is checked in setupDynamicParamsTest. Just doing this in the + // test to make it obvious what this test is doing. + require.Zero(t, setup.api.FileCache.Count()) + }) } -func TestDynamicParametersWithTerraformModules(t *testing.T) { - t.Parallel() +type setupDynamicParamsTestParams struct { + db database.Store + ps pubsub.Pubsub + provisionerDaemonVersion string + mainTF []byte + modulesArchive []byte + plan []byte + static []*proto.RichParameter + expectWebsocketError bool +} + +type dynamicParamsTest struct { + client *codersdk.Client + api *coderd.API + stream *wsjson.Stream[codersdk.DynamicParametersResponse, codersdk.DynamicParametersRequest] +} + +func setupDynamicParamsTest(t *testing.T, args setupDynamicParamsTestParams) dynamicParamsTest { cfg := coderdtest.DeploymentValues(t) cfg.Experiments = []string{string(codersdk.ExperimentDynamicParameters)} - ownerClient := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, DeploymentValues: cfg}) + ownerClient, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{ + Database: args.db, + Pubsub: args.ps, + IncludeProvisionerDaemon: true, + ProvisionerDaemonVersion: args.provisionerDaemonVersion, + DeploymentValues: cfg, + }) + owner := coderdtest.CreateFirstUser(t, ownerClient) templateAdmin, templateAdminUser := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin()) - dynamicParametersTerraformSource, err := os.ReadFile("testdata/parameters/modules/main.tf") - require.NoError(t, err) - modulesArchive, err := terraform.GetModulesArchive(os.DirFS("testdata/parameters/modules")) - require.NoError(t, err) - files := echo.WithExtraFiles(map[string][]byte{ - "main.tf": dynamicParametersTerraformSource, + "main.tf": args.mainTF, }) files.ProvisionPlan = []*proto.Response{{ Type: &proto.Response_Plan{ Plan: &proto.PlanComplete{ - Plan: []byte("{}"), - ModuleFiles: modulesArchive, + Plan: args.plan, + ModuleFiles: args.modulesArchive, + Parameters: args.static, }, }, }} @@ -166,18 +263,35 @@ func TestDynamicParametersWithTerraformModules(t *testing.T) { ctx := testutil.Context(t, testutil.WaitShort) stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, templateAdminUser.ID, version.ID) - require.NoError(t, err) - defer stream.Close(websocket.StatusGoingAway) + if args.expectWebsocketError { + require.Errorf(t, err, "expected error forming websocket") + } else { + require.NoError(t, err) + } - previews := stream.Chan() + t.Cleanup(func() { + if stream != nil { + _ = stream.Close(websocket.StatusGoingAway) + } + // Cache should always have 0 files when the only stream is closed + require.Eventually(t, func() bool { + return api.FileCache.Count() == 0 + }, testutil.WaitShort/5, testutil.IntervalMedium) + }) - // Should see the output of the module represented - preview := testutil.RequireReceive(ctx, t, previews) - require.Equal(t, -1, preview.ID) - require.Empty(t, preview.Diagnostics) + return dynamicParamsTest{ + client: ownerClient, + stream: stream, + api: api, + } +} - require.Len(t, preview.Parameters, 1) - require.Equal(t, "jetbrains_ide", preview.Parameters[0].Name) - require.True(t, preview.Parameters[0].Value.Valid()) - require.Equal(t, "CL", preview.Parameters[0].Value.AsString()) +// dbRejectGitSSHKey is a cheeky way to force an error to occur in a place +// that is generally impossible to force an error. +type dbRejectGitSSHKey struct { + database.Store +} + +func (*dbRejectGitSSHKey) GetGitSSHKey(_ context.Context, _ uuid.UUID) (database.GitSSHKey, error) { + return database.GitSSHKey{}, xerrors.New("forcing a fake error") } diff --git a/codersdk/client.go b/codersdk/client.go index 4492066785d6f..b0fb4d9764b3c 100644 --- a/codersdk/client.go +++ b/codersdk/client.go @@ -359,7 +359,7 @@ func (c *Client) Dial(ctx context.Context, path string, opts *websocket.DialOpti } conn, resp, err := websocket.Dial(ctx, u.String(), opts) - if resp.Body != nil { + if resp != nil && resp.Body != nil { resp.Body.Close() } if err != nil { diff --git a/enterprise/coderd/parameters_test.go b/enterprise/coderd/parameters_test.go new file mode 100644 index 0000000000000..e6bc564e43da2 --- /dev/null +++ b/enterprise/coderd/parameters_test.go @@ -0,0 +1,101 @@ +package coderd_test + +import ( + "os" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/rbac" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" + "github.com/coder/coder/v2/enterprise/coderd/license" + "github.com/coder/coder/v2/provisioner/echo" + "github.com/coder/coder/v2/provisionersdk/proto" + "github.com/coder/coder/v2/testutil" + "github.com/coder/websocket" +) + +func TestDynamicParametersOwnerGroups(t *testing.T) { + t.Parallel() + + cfg := coderdtest.DeploymentValues(t) + cfg.Experiments = []string{string(codersdk.ExperimentDynamicParameters)} + ownerClient, owner := coderdenttest.New(t, + &coderdenttest.Options{ + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureTemplateRBAC: 1, + }, + }, + Options: &coderdtest.Options{IncludeProvisionerDaemon: true, DeploymentValues: cfg}, + }, + ) + templateAdmin, templateAdminUser := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin()) + + // Create the group to be asserted + group := coderdtest.CreateGroup(t, ownerClient, owner.OrganizationID, "bloob", templateAdminUser) + + dynamicParametersTerraformSource, err := os.ReadFile("testdata/parameters/groups/main.tf") + require.NoError(t, err) + dynamicParametersTerraformPlan, err := os.ReadFile("testdata/parameters/groups/plan.json") + require.NoError(t, err) + + files := echo.WithExtraFiles(map[string][]byte{ + "main.tf": dynamicParametersTerraformSource, + }) + files.ProvisionPlan = []*proto.Response{{ + Type: &proto.Response_Plan{ + Plan: &proto.PlanComplete{ + Plan: dynamicParametersTerraformPlan, + }, + }, + }} + + version := coderdtest.CreateTemplateVersion(t, templateAdmin, owner.OrganizationID, files) + coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdmin, version.ID) + _ = coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, version.ID) + + ctx := testutil.Context(t, testutil.WaitShort) + stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, templateAdminUser.ID, version.ID) + require.NoError(t, err) + defer stream.Close(websocket.StatusGoingAway) + + previews := stream.Chan() + + // Should automatically send a form state with all defaulted/empty values + preview := testutil.RequireReceive(ctx, t, previews) + require.Equal(t, -1, preview.ID) + require.Empty(t, preview.Diagnostics) + require.Equal(t, "group", preview.Parameters[0].Name) + require.True(t, preview.Parameters[0].Value.Valid()) + require.Equal(t, database.EveryoneGroup, preview.Parameters[0].Value.Value.AsString()) + + // Send a new value, and see it reflected + err = stream.Send(codersdk.DynamicParametersRequest{ + ID: 1, + Inputs: map[string]string{"group": group.Name}, + }) + require.NoError(t, err) + preview = testutil.RequireReceive(ctx, t, previews) + require.Equal(t, 1, preview.ID) + require.Empty(t, preview.Diagnostics) + require.Equal(t, "group", preview.Parameters[0].Name) + require.True(t, preview.Parameters[0].Value.Valid()) + require.Equal(t, group.Name, preview.Parameters[0].Value.Value.AsString()) + + // Back to default + err = stream.Send(codersdk.DynamicParametersRequest{ + ID: 3, + Inputs: map[string]string{}, + }) + require.NoError(t, err) + preview = testutil.RequireReceive(ctx, t, previews) + require.Equal(t, 3, preview.ID) + require.Empty(t, preview.Diagnostics) + require.Equal(t, "group", preview.Parameters[0].Name) + require.True(t, preview.Parameters[0].Value.Valid()) + require.Equal(t, database.EveryoneGroup, preview.Parameters[0].Value.Value.AsString()) +} diff --git a/coderd/testdata/parameters/groups/main.tf b/enterprise/coderd/testdata/parameters/groups/main.tf similarity index 83% rename from coderd/testdata/parameters/groups/main.tf rename to enterprise/coderd/testdata/parameters/groups/main.tf index 8bed301f33ce5..9356cc2840e91 100644 --- a/coderd/testdata/parameters/groups/main.tf +++ b/enterprise/coderd/testdata/parameters/groups/main.tf @@ -12,7 +12,7 @@ data "coder_parameter" "group" { name = "group" default = try(data.coder_workspace_owner.me.groups[0], "") dynamic "option" { - for_each = concat(data.coder_workspace_owner.me.groups, "bloob") + for_each = data.coder_workspace_owner.me.groups content { name = option.value value = option.value diff --git a/coderd/testdata/parameters/groups/plan.json b/enterprise/coderd/testdata/parameters/groups/plan.json similarity index 100% rename from coderd/testdata/parameters/groups/plan.json rename to enterprise/coderd/testdata/parameters/groups/plan.json diff --git a/go.mod b/go.mod index 9dfa9a5ab7adf..cdbd61574066f 100644 --- a/go.mod +++ b/go.mod @@ -485,7 +485,7 @@ require ( require ( github.com/anthropics/anthropic-sdk-go v0.2.0-beta.3 - github.com/coder/preview v0.0.2-0.20250509141204-fc9484dbe506 + github.com/coder/preview v0.0.2-0.20250510235314-66630669ff6f github.com/fsnotify/fsnotify v1.9.0 github.com/kylecarbs/aisdk-go v0.0.8 github.com/mark3labs/mcp-go v0.27.0 diff --git a/go.sum b/go.sum index 3a6d9d92cfb93..3d635991b7abe 100644 --- a/go.sum +++ b/go.sum @@ -911,8 +911,8 @@ github.com/coder/pq v1.10.5-0.20240813183442-0c420cb5a048 h1:3jzYUlGH7ZELIH4XggX github.com/coder/pq v1.10.5-0.20240813183442-0c420cb5a048/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o= github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0 h1:3A0ES21Ke+FxEM8CXx9n47SZOKOpgSE1bbJzlE4qPVs= github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0/go.mod h1:5UuS2Ts+nTToAMeOjNlnHFkPahrtDkmpydBen/3wgZc= -github.com/coder/preview v0.0.2-0.20250509141204-fc9484dbe506 h1:rQ7Queq1IZwEBjEIk9EJsVx7XHQ+Rvo2h72/A88BnPg= -github.com/coder/preview v0.0.2-0.20250509141204-fc9484dbe506/go.mod h1:wXVvHiSmZv/7Q+Ug5I0B45TGM2U+YAjY4K3aB/6+KKo= +github.com/coder/preview v0.0.2-0.20250510235314-66630669ff6f h1:Q1AcLBpRRR8rf/H+GxcJaZ5Ox4UeIRkDgc6wvvmOJAo= +github.com/coder/preview v0.0.2-0.20250510235314-66630669ff6f/go.mod h1:GfkwIv5gQLpL01qeGU1/YoxoFtt5trzCqnWZLo77clU= github.com/coder/quartz v0.1.3 h1:hA2nI8uUA2fNN9uhXv2I4xZD4aHkA7oH3g2t03v4xf8= github.com/coder/quartz v0.1.3/go.mod h1:vsiCc+AHViMKH2CQpGIpFgdHIEQsxwm8yCscqKmzbRA= github.com/coder/retry v1.5.1 h1:iWu8YnD8YqHs3XwqrqsjoBTAVqT9ml6z9ViJ2wlMiqc=