Skip to content

Commit d0ab91c

Browse files
aslilacaslilac
authored
fix: reduce size of terraform modules archive (#17749)
1 parent 10b44a5 commit d0ab91c

File tree

29 files changed

+816
-386
lines changed

29 files changed

+816
-386
lines changed

‎.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ site/stats/
5050
*.tfplan
5151
*.lock.hcl
5252
.terraform/
53+
!provisioner/terraform/testdata/modules-source-caching/.terraform/
5354

5455
**/.coderv2/*
5556
**/__debug_bin

‎coderd/database/dbauthz/dbauthz.go

+5-6
Original file line numberDiff line numberDiff line change
@@ -12,21 +12,19 @@ import (
1212
"time"
1313

1414
"github.com/google/uuid"
15-
"golang.org/x/xerrors"
16-
1715
"github.com/open-policy-agent/opa/topdown"
16+
"golang.org/x/xerrors"
1817

1918
"cdr.dev/slog"
2019

21-
"github.com/coder/coder/v2/coderd/prebuilds"
22-
"github.com/coder/coder/v2/coderd/rbac/policy"
23-
"github.com/coder/coder/v2/coderd/rbac/rolestore"
24-
2520
"github.com/coder/coder/v2/coderd/database"
2621
"github.com/coder/coder/v2/coderd/database/dbtime"
2722
"github.com/coder/coder/v2/coderd/httpapi/httpapiconstraints"
2823
"github.com/coder/coder/v2/coderd/httpmw/loggermw"
24+
"github.com/coder/coder/v2/coderd/prebuilds"
2925
"github.com/coder/coder/v2/coderd/rbac"
26+
"github.com/coder/coder/v2/coderd/rbac/policy"
27+
"github.com/coder/coder/v2/coderd/rbac/rolestore"
3028
"github.com/coder/coder/v2/coderd/util/slice"
3129
"github.com/coder/coder/v2/provisionersdk"
3230
)
@@ -347,6 +345,7 @@ var (
347345
rbac.ResourceNotificationPreference.Type: {policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete},
348346
rbac.ResourceNotificationTemplate.Type: {policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete},
349347
rbac.ResourceCryptoKey.Type: {policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete},
348+
rbac.ResourceFile.Type: {policy.ActionCreate, policy.ActionRead},
350349
}),
351350
Org: map[string][]rbac.Permission{},
352351
User: []rbac.Permission{},

‎coderd/database/dbgen/dbgen.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -999,9 +999,10 @@ func TemplateVersionTerraformValues(t testing.TB, db database.Store, orig databa
999999
t.Helper()
10001000

10011001
params := database.InsertTemplateVersionTerraformValuesByJobIDParams{
1002-
JobID: takeFirst(orig.JobID, uuid.New()),
1003-
CachedPlan: takeFirstSlice(orig.CachedPlan, []byte("{}")),
1004-
UpdatedAt: takeFirst(orig.UpdatedAt, dbtime.Now()),
1002+
JobID: takeFirst(orig.JobID, uuid.New()),
1003+
CachedPlan: takeFirstSlice(orig.CachedPlan, []byte("{}")),
1004+
CachedModuleFiles: orig.CachedModuleFiles,
1005+
UpdatedAt: takeFirst(orig.UpdatedAt, dbtime.Now()),
10051006
}
10061007

10071008
err := db.InsertTemplateVersionTerraformValuesByJobID(genCtx, params)

‎coderd/database/dbmem/dbmem.go

+1
Original file line numberDiff line numberDiff line change
@@ -9315,6 +9315,7 @@ func (q *FakeQuerier) InsertTemplateVersionTerraformValuesByJobID(_ context.Cont
93159315
row := database.TemplateVersionTerraformValue{
93169316
TemplateVersionID: templateVersion.ID,
93179317
CachedPlan: arg.CachedPlan,
9318+
CachedModuleFiles: arg.CachedModuleFiles,
93189319
UpdatedAt: arg.UpdatedAt,
93199320
}
93209321
q.templateVersionTerraformValues = append(q.templateVersionTerraformValues, row)

‎coderd/database/dump.sql

+5-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/foreign_key_constraint.go

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/migrations/000320_terraform_cached_modules.down.sql

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE template_version_terraform_values DROP COLUMN cached_module_files;

‎coderd/database/migrations/000320_terraform_cached_modules.up.sql

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE template_version_terraform_values ADD COLUMN cached_module_files uuid references files(id);

‎coderd/database/models.go

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

+20-7
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/templateversionterraformvalues.sql

+2
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,13 @@ INSERT INTO
1111
template_version_terraform_values (
1212
template_version_id,
1313
cached_plan,
14+
cached_module_files,
1415
updated_at
1516
)
1617
VALUES
1718
(
1819
(select id from template_versions where job_id = @job_id),
1920
@cached_plan,
21+
@cached_module_files,
2022
@updated_at
2123
);

‎coderd/files/cache.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,11 @@ func (c *Cache) Acquire(ctx context.Context, fileID uuid.UUID) (fs.FS, error) {
6363
// mutex has been released, or we would continue to hold the lock until the
6464
// entire file has been fetched, which may be slow, and would prevent other
6565
// files from being fetched in parallel.
66-
return c.prepare(ctx, fileID).Load()
66+
it, err := c.prepare(ctx, fileID).Load()
67+
if err != nil {
68+
c.Release(fileID)
69+
}
70+
return it, err
6771
}
6872

6973
func (c *Cache) prepare(ctx context.Context, fileID uuid.UUID) *lazy.ValueWithError[fs.FS] {

‎coderd/parameters.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -69,14 +69,14 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http
6969
}
7070

7171
fs, err := api.FileCache.Acquire(fileCtx, fileID)
72-
defer api.FileCache.Release(fileID)
7372
if err != nil {
7473
httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{
7574
Message: "Internal error fetching template version Terraform.",
7675
Detail: err.Error(),
7776
})
7877
return
7978
}
79+
defer api.FileCache.Release(fileID)
8080

8181
// Having the Terraform plan available for the evaluation engine is helpful
8282
// for populating values from data blocks, but isn't strictly required. If

‎coderd/provisionerdserver/provisionerdserver.go

+59-5
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ package provisionerdserver
22

33
import (
44
"context"
5+
"crypto/sha256"
56
"database/sql"
7+
"encoding/hex"
68
"encoding/json"
79
"errors"
810
"fmt"
@@ -50,6 +52,10 @@ import (
5052
sdkproto "github.com/coder/coder/v2/provisionersdk/proto"
5153
)
5254

55+
const (
56+
tarMimeType = "application/x-tar"
57+
)
58+
5359
const (
5460
// DefaultAcquireJobLongPollDur is the time the (deprecated) AcquireJob rpc waits to try to obtain a job before
5561
// canceling and returning an empty job.
@@ -1426,11 +1432,59 @@ func (s *server) CompleteJob(ctx context.Context, completed *proto.CompletedJob)
14261432
return nil, xerrors.Errorf("update template version external auth providers: %w", err)
14271433
}
14281434

1429-
if len(jobType.TemplateImport.Plan) > 0 {
1430-
err := s.Database.InsertTemplateVersionTerraformValuesByJobID(ctx, database.InsertTemplateVersionTerraformValuesByJobIDParams{
1431-
JobID: jobID,
1432-
CachedPlan: jobType.TemplateImport.Plan,
1433-
UpdatedAt: now,
1435+
plan := jobType.TemplateImport.Plan
1436+
moduleFiles := jobType.TemplateImport.ModuleFiles
1437+
// If there is a plan, or a module files archive we need to insert a
1438+
// template_version_terraform_values row.
1439+
if len(plan) > 0 || len(moduleFiles) > 0 {
1440+
// ...but the plan and the module files archive are both optional! So
1441+
// we need to fallback to a valid JSON object if the plan was omitted.
1442+
if len(plan) == 0 {
1443+
plan = []byte("{}")
1444+
}
1445+
1446+
// ...and we only want to insert a files row if an archive was provided.
1447+
var fileID uuid.NullUUID
1448+
if len(moduleFiles) > 0 {
1449+
hashBytes := sha256.Sum256(moduleFiles)
1450+
hash := hex.EncodeToString(hashBytes[:])
1451+
1452+
// nolint:gocritic // Requires reading "system" files
1453+
file, err := s.Database.GetFileByHashAndCreator(dbauthz.AsSystemRestricted(ctx), database.GetFileByHashAndCreatorParams{Hash: hash, CreatedBy: uuid.Nil})
1454+
switch {
1455+
case err == nil:
1456+
// This set of modules is already cached, which means we can reuse them
1457+
fileID = uuid.NullUUID{
1458+
Valid: true,
1459+
UUID: file.ID,
1460+
}
1461+
case !xerrors.Is(err, sql.ErrNoRows):
1462+
return nil, xerrors.Errorf("check for cached modules: %w", err)
1463+
default:
1464+
// nolint:gocritic // Requires creating a "system" file
1465+
file, err = s.Database.InsertFile(dbauthz.AsSystemRestricted(ctx), database.InsertFileParams{
1466+
ID: uuid.New(),
1467+
Hash: hash,
1468+
CreatedBy: uuid.Nil,
1469+
CreatedAt: dbtime.Now(),
1470+
Mimetype: tarMimeType,
1471+
Data: moduleFiles,
1472+
})
1473+
if err != nil {
1474+
return nil, xerrors.Errorf("insert template version terraform modules: %w", err)
1475+
}
1476+
fileID = uuid.NullUUID{
1477+
Valid: true,
1478+
UUID: file.ID,
1479+
}
1480+
}
1481+
}
1482+
1483+
err = s.Database.InsertTemplateVersionTerraformValuesByJobID(ctx, database.InsertTemplateVersionTerraformValuesByJobIDParams{
1484+
JobID: jobID,
1485+
UpdatedAt: now,
1486+
CachedPlan: plan,
1487+
CachedModuleFiles: fileID,
14341488
})
14351489
if err != nil {
14361490
return nil, xerrors.Errorf("insert template version terraform data: %w", err)

‎provisioner/echo/serve.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ var (
5252
PlanComplete = []*proto.Response{{
5353
Type: &proto.Response_Plan{
5454
Plan: &proto.PlanComplete{
55-
Plan: []byte("{}"),
55+
Plan: []byte("{}"),
56+
ModuleFiles: []byte{},
5657
},
5758
},
5859
}}
@@ -249,6 +250,7 @@ func TarWithOptions(ctx context.Context, logger slog.Logger, responses *Response
249250
Parameters: resp.GetApply().GetParameters(),
250251
ExternalAuthProviders: resp.GetApply().GetExternalAuthProviders(),
251252
Plan: []byte("{}"),
253+
ModuleFiles: []byte{},
252254
}},
253255
})
254256
}

‎provisioner/terraform/executor.go

+18-2
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,13 @@ import (
1919
tfjson "github.com/hashicorp/terraform-json"
2020
"go.opentelemetry.io/otel/attribute"
2121
"golang.org/x/xerrors"
22+
protobuf "google.golang.org/protobuf/proto"
2223

2324
"cdr.dev/slog"
2425

2526
"github.com/coder/coder/v2/coderd/database"
2627
"github.com/coder/coder/v2/coderd/tracing"
28+
"github.com/coder/coder/v2/codersdk/drpcsdk"
2729
"github.com/coder/coder/v2/provisionersdk/proto"
2830
)
2931

@@ -307,14 +309,28 @@ func (e *executor) plan(ctx, killCtx context.Context, env, vars []string, logr l
307309

308310
graphTimings.ingest(createGraphTimingsEvent(timingGraphComplete))
309311

310-
return &proto.PlanComplete{
312+
moduleFiles, err := getModulesArchive(os.DirFS(e.workdir))
313+
if err != nil {
314+
// TODO: we probably want to persist this error or make it louder eventually
315+
e.logger.Warn(ctx, "failed to archive terraform modules", slog.Error(err))
316+
}
317+
318+
msg := &proto.PlanComplete{
311319
Parameters: state.Parameters,
312320
Resources: state.Resources,
313321
ExternalAuthProviders: state.ExternalAuthProviders,
314322
Timings: append(e.timings.aggregate(), graphTimings.aggregate()...),
315323
Presets: state.Presets,
316324
Plan: plan,
317-
}, nil
325+
ModuleFiles: moduleFiles,
326+
}
327+
328+
if protobuf.Size(msg) > drpcsdk.MaxMessageSize {
329+
e.logger.Warn(ctx, "cannot persist terraform modules, message payload too big", slog.F("archive_size", len(msg.ModuleFiles)))
330+
msg.ModuleFiles = nil
331+
}
332+
333+
return msg, nil
318334
}
319335

320336
func onlyDataResources(sm tfjson.StateModule) tfjson.StateModule {

0 commit comments

Comments
 (0)