Refactor provisionerd to dynamically update parameter schemas

coder · kylecarbs · Feb 12, 2022 · Feb 10, 2022 · Feb 10, 2022 · Feb 10, 2022
commit a6ce22db9c657b99322e9f3de348fef1909ff231
diff --git a/coderd/provisionerdaemons.go b/coderd/provisionerdaemons.go
@@ -113,9 +113,6 @@ type workspaceProvisionJob struct {
 type projectVersionImportJob struct {
 	OrganizationID string    `json:"organization_id"`
 	ProjectID      uuid.UUID `json:"project_id"`
-
-	SkipParameterSchemas bool `json:"skip_parameter_schemas"`
-	SkipResources        bool `json:"skip_resources"`
 }
 
 // Implementation of the provisioner daemon protobuf server.
@@ -276,9 +273,7 @@ func (server *provisionerdServer) AcquireJob(ctx context.Context, _ *proto.Empty
 
 		protoJob.Type = &proto.AcquiredJob_ProjectImport_{
 			ProjectImport: &proto.AcquiredJob_ProjectImport{
-				ParameterValues:      protoParameters,
-				SkipParameterSchemas: input.SkipParameterSchemas,
-				SkipResources:        input.SkipResources,
+				ParameterValues: protoParameters,
 			},
 		}
 	}
@@ -324,35 +319,89 @@ func (server *provisionerdServer) UpdateJob(stream proto.DRPCProvisionerDaemon_U
 		if err != nil {
 			return xerrors.Errorf("update job: %w", err)
 		}
-		insertParams := database.InsertProvisionerJobLogsParams{
-			JobID: parsedID,
-		}
-		for _, log := range update.Logs {
-			logLevel, err := convertLogLevel(log.Level)
+		if len(update.Logs) > 0 {
+			insertParams := database.InsertProvisionerJobLogsParams{
+				JobID: parsedID,
+			}
+			for _, log := range update.Logs {
+				logLevel, err := convertLogLevel(log.Level)
+				if err != nil {
+					return xerrors.Errorf("convert log level: %w", err)
+				}
+				logSource, err := convertLogSource(log.Source)
+				if err != nil {
+					return xerrors.Errorf("convert log source: %w", err)
+				}
+				insertParams.ID = append(insertParams.ID, uuid.New())
+				insertParams.CreatedAt = append(insertParams.CreatedAt, time.UnixMilli(log.CreatedAt))
+				insertParams.Level = append(insertParams.Level, logLevel)
+				insertParams.Source = append(insertParams.Source, logSource)
+				insertParams.Output = append(insertParams.Output, log.Output)
+			}
+			logs, err := server.Database.InsertProvisionerJobLogs(context.Background(), insertParams)
 			if err != nil {
-				return xerrors.Errorf("convert log level: %w", err)
+				return xerrors.Errorf("insert job logs: %w", err)
 			}
-			logSource, err := convertLogSource(log.Source)
+			data, err := json.Marshal(logs)
 			if err != nil {
-				return xerrors.Errorf("convert log source: %w", err)
+				return xerrors.Errorf("marshal job log: %w", err)
+			}
+			err = server.Pubsub.Publish(provisionerJobLogsChannel(parsedID), data)
+			if err != nil {
+				return xerrors.Errorf("publish job log: %w", err)
 			}
-			insertParams.ID = append(insertParams.ID, uuid.New())
-			insertParams.CreatedAt = append(insertParams.CreatedAt, time.UnixMilli(log.CreatedAt))
-			insertParams.Level = append(insertParams.Level, logLevel)
-			insertParams.Source = append(insertParams.Source, logSource)
-			insertParams.Output = append(insertParams.Output, log.Output)
-		}
-		logs, err := server.Database.InsertProvisionerJobLogs(context.Background(), insertParams)
-		if err != nil {
-			return xerrors.Errorf("insert job logs: %w", err)
-		}
-		data, err := json.Marshal(logs)
-		if err != nil {
-			return xerrors.Errorf("marshal job log: %w", err)
 		}
-		err = server.Pubsub.Publish(provisionerJobLogsChannel(parsedID), data)
-		if err != nil {
-			return xerrors.Errorf("publish job log: %w", err)
+
+		if update.GetProjectImport() != nil {
+			// Validate that all parameters send from the provisioner daemon
+			// follow the protocol.
+			parameterSchemas := make([]database.InsertParameterSchemaParams, 0, len(update.GetProjectImport().ParameterSchemas))
+			for _, protoParameter := range update.GetProjectImport().ParameterSchemas {
+				validationTypeSystem, err := convertValidationTypeSystem(protoParameter.ValidationTypeSystem)
+				if err != nil {
+					return xerrors.Errorf("convert validation type system for %q: %w", protoParameter.Name, err)
+				}
+
+				parameterSchema := database.InsertParameterSchemaParams{
+					ID:                   uuid.New(),
+					CreatedAt:            database.Now(),
+					JobID:                job.ID,
+					Name:                 protoParameter.Name,
+					Description:          protoParameter.Description,
+					RedisplayValue:       protoParameter.RedisplayValue,
+					ValidationError:      protoParameter.ValidationError,
+					ValidationCondition:  protoParameter.ValidationCondition,
+					ValidationValueType:  protoParameter.ValidationValueType,
+					ValidationTypeSystem: validationTypeSystem,
+
+					DefaultSourceScheme:      database.ParameterSourceSchemeNone,
+					DefaultDestinationScheme: database.ParameterDestinationSchemeNone,
+
+					AllowOverrideDestination: protoParameter.AllowOverrideDestination,
+					AllowOverrideSource:      protoParameter.AllowOverrideSource,
+				}
+
+				// It's possible a parameter doesn't define a default source!
+				if protoParameter.DefaultSource != nil {
+					parameterSourceScheme, err := convertParameterSourceScheme(protoParameter.DefaultSource.Scheme)
+					if err != nil {
+						return xerrors.Errorf("convert parameter source scheme: %w", err)
+					}
+					parameterSchema.DefaultSourceScheme = parameterSourceScheme
+					parameterSchema.DefaultSourceValue = protoParameter.DefaultSource.Value
+				}
+
+				// It's possible a parameter doesn't define a default destination!
+				if protoParameter.DefaultDestination != nil {
+					parameterDestinationScheme, err := convertParameterDestinationScheme(protoParameter.DefaultDestination.Scheme)
+					if err != nil {
+						return xerrors.Errorf("convert parameter destination scheme: %w", err)
+					}
+					parameterSchema.DefaultDestinationScheme = parameterDestinationScheme
+				}
+
+				parameterSchemas = append(parameterSchemas, parameterSchema)
+			}
 		}
 	}
 }
@@ -412,80 +461,18 @@ func (server *provisionerdServer) CompleteJob(ctx context.Context, completed *pr
 			return nil, xerrors.Errorf("unmarshal job data: %w", err)
 		}
 
-		// Validate that all parameters send from the provisioner daemon
-		// follow the protocol.
-		parameterSchemas := make([]database.InsertParameterSchemaParams, 0, len(jobType.ProjectImport.ParameterSchemas))
-		for _, protoParameter := range jobType.ProjectImport.ParameterSchemas {
-			validationTypeSystem, err := convertValidationTypeSystem(protoParameter.ValidationTypeSystem)
-			if err != nil {
-				return nil, xerrors.Errorf("convert validation type system for %q: %w", protoParameter.Name, err)
-			}
-
-			parameterSchema := database.InsertParameterSchemaParams{
-				ID:                   uuid.New(),
-				CreatedAt:            database.Now(),
-				JobID:                job.ID,
-				Name:                 protoParameter.Name,
-				Description:          protoParameter.Description,
-				RedisplayValue:       protoParameter.RedisplayValue,
-				ValidationError:      protoParameter.ValidationError,
-				ValidationCondition:  protoParameter.ValidationCondition,
-				ValidationValueType:  protoParameter.ValidationValueType,
-				ValidationTypeSystem: validationTypeSystem,
-
-				DefaultSourceScheme:      database.ParameterSourceSchemeNone,
-				DefaultDestinationScheme: database.ParameterDestinationSchemeNone,
-
-				AllowOverrideDestination: protoParameter.AllowOverrideDestination,
-				AllowOverrideSource:      protoParameter.AllowOverrideSource,
-			}
-
-			// It's possible a parameter doesn't define a default source!
-			if protoParameter.DefaultSource != nil {
-				parameterSourceScheme, err := convertParameterSourceScheme(protoParameter.DefaultSource.Scheme)
-				if err != nil {
-					return nil, xerrors.Errorf("convert parameter source scheme: %w", err)
-				}
-				parameterSchema.DefaultSourceScheme = parameterSourceScheme
-				parameterSchema.DefaultSourceValue = protoParameter.DefaultSource.Value
-			}
-
-			// It's possible a parameter doesn't define a default destination!
-			if protoParameter.DefaultDestination != nil {
-				parameterDestinationScheme, err := convertParameterDestinationScheme(protoParameter.DefaultDestination.Scheme)
-				if err != nil {
-					return nil, xerrors.Errorf("convert parameter destination scheme: %w", err)
-				}
-				parameterSchema.DefaultDestinationScheme = parameterDestinationScheme
-			}
-
-			parameterSchemas = append(parameterSchemas, parameterSchema)
-		}
-
-		// This must occur in a transaction in case of failure.
-		err = server.Database.InTx(func(db database.Store) error {
-			err = db.UpdateProvisionerJobWithCompleteByID(ctx, database.UpdateProvisionerJobWithCompleteByIDParams{
-				ID:        jobID,
-				UpdatedAt: database.Now(),
-				CompletedAt: sql.NullTime{
-					Time:  database.Now(),
-					Valid: true,
-				},
-			})
-			if err != nil {
-				return xerrors.Errorf("update provisioner job: %w", err)
-			}
-			// This could be a bulk-insert operation to improve performance.
-			// See the "InsertWorkspaceHistoryLogs" query.
-			for _, parameterSchema := range parameterSchemas {
-				_, err = db.InsertParameterSchema(ctx, parameterSchema)
-				if err != nil {
-					return xerrors.Errorf("insert parameter schema %q: %w", parameterSchema.Name, err)
-				}
-			}
-			server.Logger.Debug(ctx, "marked import job as completed", slog.F("job_id", jobID))
-			return nil
+		err = server.Database.UpdateProvisionerJobWithCompleteByID(ctx, database.UpdateProvisionerJobWithCompleteByIDParams{
+			ID:        jobID,
+			UpdatedAt: database.Now(),
+			CompletedAt: sql.NullTime{
+				Time:  database.Now(),
+				Valid: true,
+			},
 		})
+		if err != nil {
+			return nil, xerrors.Errorf("update provisioner job: %w", err)
+		}
+		server.Logger.Debug(ctx, "marked import job as completed", slog.F("job_id", jobID))
 		if err != nil {
 			return nil, xerrors.Errorf("complete job: %w", err)
 		}

diff --git a/coderd/provisionerjobs.go b/coderd/provisionerjobs.go
@@ -46,13 +46,10 @@ type ProvisionerJob struct {
 }
 
 type CreateProjectImportJobRequest struct {
-	StorageMethod database.ProvisionerStorageMethod `json:"storage_method" validate:"oneof=file,required"`
-	StorageSource string                            `json:"storage_source" validate:"required"`
-	Provisioner   database.ProvisionerType          `json:"provisioner" validate:"oneof=terraform echo,required"`
-
-	ParameterValues      []CreateParameterValueRequest `json:"parameter_values"`
-	SkipParameterSchemas bool                          `json:"skip_parameter_schemas"`
-	SkipResources        bool                          `json:"skip_resources"`
+	StorageMethod   database.ProvisionerStorageMethod `json:"storage_method" validate:"oneof=file,required"`
+	StorageSource   string                            `json:"storage_source" validate:"required"`
+	Provisioner     database.ProvisionerType          `json:"provisioner" validate:"oneof=terraform echo,required"`
+	ParameterValues []CreateParameterValueRequest     `json:"parameter_values"`
 }
 
 func (*api) provisionerJobByOrganization(rw http.ResponseWriter, r *http.Request) {
@@ -84,10 +81,7 @@ func (api *api) postProvisionerImportJobByOrganization(rw http.ResponseWriter, r
 	}
 
 	input, err := json.Marshal(projectVersionImportJob{
-		// AdditionalParameters: req.AdditionalParameters,
-		OrganizationID:       organization.ID,
-		SkipParameterSchemas: req.SkipParameterSchemas,
-		SkipResources:        req.SkipResources,
+		OrganizationID: organization.ID,
 	})
 	if err != nil {
 		httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{

diff --git a/coderd/provisionerjobs_test.go b/coderd/provisionerjobs_test.go
@@ -2,6 +2,7 @@ package coderd_test
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 	"testing"
 	"time"
@@ -70,6 +71,7 @@ func TestPostProvisionerImportJobByOrganization(t *testing.T) {
 					},
 				},
 			}},
+			Provision: echo.ProvisionComplete,
 		})
 		require.NoError(t, err)
 		file, err := client.UploadFile(context.Background(), codersdk.ContentTypeTar, data)
@@ -84,10 +86,10 @@ func TestPostProvisionerImportJobByOrganization(t *testing.T) {
 				SourceScheme:      database.ParameterSourceSchemeData,
 				DestinationScheme: database.ParameterDestinationSchemeProvisionerVariable,
 			}},
-			SkipResources: true,
 		})
 		require.NoError(t, err)
-		coderdtest.AwaitProvisionerJob(t, client, user.Organization, job.ID)
+		job = coderdtest.AwaitProvisionerJob(t, client, user.Organization, job.ID)
+		fmt.Printf("Job %+v\n", job)
 		values, err := client.ProvisionerJobParameterValues(context.Background(), user.Organization, job.ID)
 		require.NoError(t, err)
 		require.Equal(t, "somevalue", values[0].SourceValue)