Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitac7af5e

Browse files
committed
feat: validate presets on template import
1 parent3126f21 commitac7af5e

File tree

7 files changed

+226
-50
lines changed

7 files changed

+226
-50
lines changed

‎coderd/dynamicparameters/error.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,14 @@ func tagValidationError(diags hcl.Diagnostics) *DiagnosticError {
2626
}
2727
}
2828

29+
funcpresetValidationError(diags hcl.Diagnostics)*DiagnosticError {
30+
return&DiagnosticError{
31+
Message:"Unable to parse presets",
32+
Diagnostics:diags,
33+
KeyedDiagnostics:make(map[string]hcl.Diagnostics),
34+
}
35+
}
36+
2937
typeDiagnosticErrorstruct {
3038
// Message is the human-readable message that will be returned to the user.
3139
Messagestring
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package dynamicparameters
2+
3+
import (
4+
"github.com/hashicorp/hcl/v2"
5+
6+
"github.com/coder/preview"
7+
)
8+
9+
// CheckPresets extracts the preset related diagnostics from a template version preset
10+
funcCheckPresets(output*preview.Output,diags hcl.Diagnostics)*DiagnosticError {
11+
de:=presetValidationError(diags)
12+
presets:=output.Presets
13+
for_,preset:=rangepresets {
14+
ifhcl.Diagnostics(preset.Diagnostics).HasErrors() {
15+
de.Extend(preset.Name,hcl.Diagnostics(preset.Diagnostics))
16+
}
17+
}
18+
19+
ifde.HasError() {
20+
returnde
21+
}
22+
23+
returnnil
24+
}

‎coderd/parameters_test.go‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package coderd_test
33
import (
44
"context"
55
"os"
6+
"sync"
67
"testing"
78

89
"github.com/google/uuid"
@@ -193,20 +194,23 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) {
193194
t.Parallel()
194195

195196
db,ps:=dbtestutil.NewDB(t)
197+
dbReject:=&dbRejectGitSSHKey{Store:db}
196198
dynamicParametersTerraformSource,err:=os.ReadFile("testdata/parameters/modules/main.tf")
197199
require.NoError(t,err)
198200

199201
modulesArchive,err:=terraform.GetModulesArchive(os.DirFS("testdata/parameters/modules"))
200202
require.NoError(t,err)
201203

202204
setup:=setupDynamicParamsTest(t,setupDynamicParamsTestParams{
203-
db:&dbRejectGitSSHKey{Store:db},
205+
db:dbReject,
204206
ps:ps,
205207
provisionerDaemonVersion:provProto.CurrentVersion.String(),
206208
mainTF:dynamicParametersTerraformSource,
207209
modulesArchive:modulesArchive,
208210
})
209211

212+
dbReject.SetReject(true)
213+
210214
stream:=setup.stream
211215
previews:=stream.Chan()
212216

@@ -412,8 +416,25 @@ func setupDynamicParamsTest(t *testing.T, args setupDynamicParamsTestParams) dyn
412416
// that is generally impossible to force an error.
413417
typedbRejectGitSSHKeystruct {
414418
database.Store
419+
rejectMu sync.RWMutex
420+
rejectbool
421+
}
422+
423+
// SetReject toggles whether GetGitSSHKey should return an error or passthrough to the underlying store.
424+
func (d*dbRejectGitSSHKey)SetReject(rejectbool) {
425+
d.rejectMu.Lock()
426+
deferd.rejectMu.Unlock()
427+
d.reject=reject
415428
}
416429

417-
func (*dbRejectGitSSHKey)GetGitSSHKey(_ context.Context,_ uuid.UUID) (database.GitSSHKey,error) {
418-
return database.GitSSHKey{},xerrors.New("forcing a fake error")
430+
func (d*dbRejectGitSSHKey)GetGitSSHKey(ctx context.Context,userID uuid.UUID) (database.GitSSHKey,error) {
431+
d.rejectMu.RLock()
432+
reject:=d.reject
433+
d.rejectMu.RUnlock()
434+
435+
ifreject {
436+
return database.GitSSHKey{},xerrors.New("forcing a fake error")
437+
}
438+
439+
returnd.Store.GetGitSSHKey(ctx,userID)
419440
}

‎coderd/templateversions.go‎

Lines changed: 56 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616

1717
"github.com/go-chi/chi/v5"
1818
"github.com/google/uuid"
19+
"github.com/hashicorp/hcl/v2"
1920
"github.com/moby/moby/pkg/namesgenerator"
2021
"github.com/sqlc-dev/pqtype"
2122
"golang.org/x/xerrors"
@@ -1582,10 +1583,63 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht
15821583
}
15831584
}
15841585

1586+
varfiles fs.FS
1587+
switchfile.Mimetype {
1588+
case"application/x-tar":
1589+
files=archivefs.FromTarReader(bytes.NewBuffer(file.Data))
1590+
case"application/zip":
1591+
files,err=archivefs.FromZipReader(bytes.NewReader(file.Data),int64(len(file.Data)))
1592+
iferr!=nil {
1593+
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
1594+
Message:"Internal error reading file",
1595+
Detail:"extract zip archive: "+err.Error(),
1596+
})
1597+
return
1598+
}
1599+
default:
1600+
httpapi.Write(ctx,rw,http.StatusBadRequest, codersdk.Response{
1601+
Message:"Unsupported file type",
1602+
Detail:fmt.Sprintf("Mimetype %q is not supported",file.Mimetype),
1603+
})
1604+
return
1605+
}
1606+
ownerData,err:=dynamicparameters.WorkspaceOwner(ctx,api.Database,organization.ID,apiKey.UserID)
1607+
iferr!=nil {
1608+
ifhttpapi.Is404Error(err) {
1609+
httpapi.Write(ctx,rw,http.StatusBadRequest, codersdk.Response{
1610+
Message:"Internal error checking workspace tags",
1611+
Detail:fmt.Sprintf("Owner not found, uuid=%s",apiKey.UserID.String()),
1612+
})
1613+
return
1614+
}
1615+
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
1616+
Message:"Internal error checking workspace tags",
1617+
Detail:"fetch owner data: "+err.Error(),
1618+
})
1619+
return
1620+
}
1621+
1622+
previewInput:= preview.Input{
1623+
PlanJSON:nil,// Template versions are before `terraform plan`
1624+
ParameterValues:nil,// No user-specified parameters
1625+
Owner:*ownerData,
1626+
Logger:stdslog.New(stdslog.DiscardHandler),
1627+
}
1628+
previewOutput,previewDiags:=preview.Preview(ctx,previewInput,files)
1629+
1630+
// Validate presets on template version import to avoid errors that would
1631+
// have caused workspace creation to fail:
1632+
presetErr:=dynamicparameters.CheckPresets(previewOutput,nil)
1633+
ifpresetErr!=nil {
1634+
code,resp:=presetErr.Response()
1635+
httpapi.Write(ctx,rw,code,resp)
1636+
return
1637+
}
1638+
15851639
varparsedTagsmap[string]string
15861640
varokbool
15871641
ifdynamicTemplate {
1588-
parsedTags,ok=api.dynamicTemplateVersionTags(ctx,rw,organization.ID,apiKey.UserID,file)
1642+
parsedTags,ok=api.dynamicTemplateVersionTags(ctx,rw,previewOutput,previewDiags)
15891643
if!ok {
15901644
return
15911645
}
@@ -1762,50 +1816,7 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht
17621816
warnings))
17631817
}
17641818

1765-
func (api*API)dynamicTemplateVersionTags(ctx context.Context,rw http.ResponseWriter,orgID uuid.UUID,owner uuid.UUID,file database.File) (map[string]string,bool) {
1766-
ownerData,err:=dynamicparameters.WorkspaceOwner(ctx,api.Database,orgID,owner)
1767-
iferr!=nil {
1768-
ifhttpapi.Is404Error(err) {
1769-
httpapi.Write(ctx,rw,http.StatusBadRequest, codersdk.Response{
1770-
Message:"Internal error checking workspace tags",
1771-
Detail:fmt.Sprintf("Owner not found, uuid=%s",owner.String()),
1772-
})
1773-
returnnil,false
1774-
}
1775-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
1776-
Message:"Internal error checking workspace tags",
1777-
Detail:"fetch owner data: "+err.Error(),
1778-
})
1779-
returnnil,false
1780-
}
1781-
1782-
varfiles fs.FS
1783-
switchfile.Mimetype {
1784-
case"application/x-tar":
1785-
files=archivefs.FromTarReader(bytes.NewBuffer(file.Data))
1786-
case"application/zip":
1787-
files,err=archivefs.FromZipReader(bytes.NewReader(file.Data),int64(len(file.Data)))
1788-
iferr!=nil {
1789-
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
1790-
Message:"Internal error checking workspace tags",
1791-
Detail:"extract zip archive: "+err.Error(),
1792-
})
1793-
returnnil,false
1794-
}
1795-
default:
1796-
httpapi.Write(ctx,rw,http.StatusBadRequest, codersdk.Response{
1797-
Message:"Unsupported file type for dynamic template version tags",
1798-
Detail:fmt.Sprintf("Mimetype %q is not supported for dynamic template version tags",file.Mimetype),
1799-
})
1800-
returnnil,false
1801-
}
1802-
1803-
output,diags:=preview.Preview(ctx, preview.Input{
1804-
PlanJSON:nil,// Template versions are before `terraform plan`
1805-
ParameterValues:nil,// No user-specified parameters
1806-
Owner:*ownerData,
1807-
Logger:stdslog.New(stdslog.DiscardHandler),
1808-
},files)
1819+
func (*API)dynamicTemplateVersionTags(ctx context.Context,rw http.ResponseWriter,output*preview.Output,diags hcl.Diagnostics) (map[string]string,bool) {
18091820
tagErr:=dynamicparameters.CheckTags(output,diags)
18101821
iftagErr!=nil {
18111822
code,resp:=tagErr.Response()

‎coderd/templateversions_test.go‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -641,6 +641,118 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
641641
})
642642
}
643643
})
644+
645+
t.Run("Presets",func(t*testing.T) {
646+
t.Parallel()
647+
store,ps:=dbtestutil.NewDB(t)
648+
client:=coderdtest.New(t,&coderdtest.Options{
649+
Database:store,
650+
Pubsub:ps,
651+
})
652+
owner:=coderdtest.CreateFirstUser(t,client)
653+
templateAdmin,_:=coderdtest.CreateAnotherUser(t,client,owner.OrganizationID,rbac.RoleTemplateAdmin())
654+
655+
for_,tt:=range []struct {
656+
namestring
657+
filesmap[string]string
658+
expectErrorstring
659+
}{
660+
{
661+
name:"valid preset",
662+
files:map[string]string{
663+
`main.tf`:`
664+
terraform {
665+
required_providers {
666+
coder = {
667+
source = "coder/coder"
668+
version = "2.8.0"
669+
}
670+
}
671+
}
672+
data "coder_parameter" "valid_parameter" {
673+
name = "valid_parameter_name"
674+
default = "valid_option_value"
675+
option {
676+
name = "valid_option_name"
677+
value = "valid_option_value"
678+
}
679+
}
680+
data "coder_workspace_preset" "valid_preset" {
681+
name = "valid_preset"
682+
parameters = {
683+
"valid_parameter_name" = "valid_option_value"
684+
}
685+
}
686+
`,
687+
},
688+
},
689+
{
690+
name:"invalid preset",
691+
files:map[string]string{
692+
`main.tf`:`
693+
terraform {
694+
required_providers {
695+
coder = {
696+
source = "coder/coder"
697+
version = "2.8.0"
698+
}
699+
}
700+
}
701+
data "coder_parameter" "valid_parameter" {
702+
name = "valid_parameter_name"
703+
default = "valid_option_value"
704+
option {
705+
name = "valid_option_name"
706+
value = "valid_option_value"
707+
}
708+
}
709+
data "coder_workspace_preset" "invalid_parameter_name" {
710+
name = "invalid_parameter_name"
711+
parameters = {
712+
"invalid_parameter_name" = "irrelevant_value"
713+
}
714+
}
715+
`,
716+
},
717+
expectError:"Undefined Parameter",
718+
},
719+
} {
720+
t.Run(tt.name,func(t*testing.T) {
721+
t.Parallel()
722+
ctx:=testutil.Context(t,testutil.WaitShort)
723+
724+
// Create an archive from the files provided in the test case.
725+
tarFile:=testutil.CreateTar(t,tt.files)
726+
727+
// Post the archive file
728+
fi,err:=templateAdmin.Upload(ctx,"application/x-tar",bytes.NewReader(tarFile))
729+
require.NoError(t,err)
730+
731+
// Create a template version from the archive
732+
tvName:=testutil.GetRandomNameHyphenated(t)
733+
tv,err:=templateAdmin.CreateTemplateVersion(ctx,owner.OrganizationID, codersdk.CreateTemplateVersionRequest{
734+
Name:tvName,
735+
StorageMethod:codersdk.ProvisionerStorageMethodFile,
736+
Provisioner:codersdk.ProvisionerTypeTerraform,
737+
FileID:fi.ID,
738+
})
739+
740+
iftt.expectError=="" {
741+
require.NoError(t,err)
742+
// Assert the expected provisioner job is created from the template version import
743+
pj,err:=store.GetProvisionerJobByID(ctx,tv.Job.ID)
744+
require.NoError(t,err)
745+
require.NotNil(t,pj)
746+
// Also assert that we get the expected information back from the API endpoint
747+
require.Zero(t,tv.MatchedProvisioners.Count)
748+
require.Zero(t,tv.MatchedProvisioners.Available)
749+
require.Zero(t,tv.MatchedProvisioners.MostRecentlySeen.Time)
750+
}else {
751+
require.ErrorContains(t,err,tt.expectError)
752+
}
753+
})
754+
}
755+
})
644756
}
645757

646758
funcTestPatchCancelTemplateVersion(t*testing.T) {

‎go.mod‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,8 @@ require (
488488
github.com/mark3labs/mcp-gov0.32.0
489489
)
490490

491+
replacegithub.com/coder/preview =>../preview
492+
491493
require (
492494
cel.dev/exprv0.23.0// indirect
493495
cloud.google.com/gov0.120.0// indirect

‎go.sum‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -916,8 +916,6 @@ github.com/coder/pq v1.10.5-0.20250630052411-a259f96b6102 h1:ahTJlTRmTogsubgRVGO
916916
github.com/coder/pqv1.10.5-0.20250630052411-a259f96b6102/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
917917
github.com/coder/prettyv0.0.0-20230908205945-e89ba86370e0 h1:3A0ES21Ke+FxEM8CXx9n47SZOKOpgSE1bbJzlE4qPVs=
918918
github.com/coder/prettyv0.0.0-20230908205945-e89ba86370e0/go.mod h1:5UuS2Ts+nTToAMeOjNlnHFkPahrtDkmpydBen/3wgZc=
919-
github.com/coder/previewv1.0.3-0.20250701142654-c3d6e86b9393 h1:l+m2liikn8JoEv6C22QIV4qseolUfvNsyUNA6JJsD6Y=
920-
github.com/coder/previewv1.0.3-0.20250701142654-c3d6e86b9393/go.mod h1:efDWGlO/PZPrvdt5QiDhMtTUTkPxejXo9c0wmYYLLjM=
921919
github.com/coder/quartzv0.2.1 h1:QgQ2Vc1+mvzewg2uD/nj8MJ9p9gE+QhGJm+Z+NGnrSE=
922920
github.com/coder/quartzv0.2.1/go.mod h1:vsiCc+AHViMKH2CQpGIpFgdHIEQsxwm8yCscqKmzbRA=
923921
github.com/coder/retryv1.5.1 h1:iWu8YnD8YqHs3XwqrqsjoBTAVqT9ml6z9ViJ2wlMiqc=

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp