Skip to content

Commit cc7335a

Browse files
ethanndicksonethanndickson
authored
fix: always use config name for new template versions (#66)
1 parent 403243a commit cc7335a

File tree

7 files changed

+115
-45
lines changed

7 files changed

+115
-45
lines changed

‎go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ go 1.22.5
44

55
require (
66
cdr.dev/slog v1.6.2-0.20240126064726-20367d4aede6
7-
github.com/coder/coder/v2 v2.14.0
7+
github.com/coder/coder/v2 v2.14.1
88
github.com/docker/docker v27.1.1+incompatible
99
github.com/docker/go-connections v0.4.0
1010
github.com/google/uuid v1.6.0

‎go.sum

+2
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ github.com/cloudflare/circl v1.3.7 h1:qlCDlTPz2n9fu58M0Nh1J/JzcFpfgkFHHX3O35r5vc
8383
github.com/cloudflare/circl v1.3.7/go.mod h1:sRTcRWXGLrKw6yIGJ+l7amYJFfAXbZG0kBSc8r4zxgA=
8484
github.com/coder/coder/v2 v2.14.0 h1:Z7iyLdnFHYh57xddSCS2IE5JDy0O6r957T4S9lt5Obo=
8585
github.com/coder/coder/v2 v2.14.0/go.mod h1:dO79BI5XlP8rrtne1JpRcVehe27bNMXdZKyn1NsWbjA=
86+
github.com/coder/coder/v2 v2.14.1 h1:tSYe7H4pNRL8hh9ynwHK7QYTOvG5hcuZJEOkn4ZPASE=
87+
github.com/coder/coder/v2 v2.14.1/go.mod h1:dO79BI5XlP8rrtne1JpRcVehe27bNMXdZKyn1NsWbjA=
8688
github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0 h1:3A0ES21Ke+FxEM8CXx9n47SZOKOpgSE1bbJzlE4qPVs=
8789
github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0/go.mod h1:5UuS2Ts+nTToAMeOjNlnHFkPahrtDkmpydBen/3wgZc=
8890
github.com/coder/serpent v0.7.0 h1:zGpD2GlF3lKIVkMjNGKbkip88qzd5r/TRcc30X/SrT0=

‎integration/integration_test.go

-1
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,6 @@ func TestIntegration(t *testing.T) {
128128
})
129129
require.NoError(t, err)
130130
require.Len(t, versions, 2)
131-
require.Equal(t, "latest", versions[0].Name)
132131
require.NotEmpty(t, versions[0].ID)
133132
require.Equal(t, templates[0].ID, *versions[0].TemplateID)
134133
require.Equal(t, templates[0].ActiveVersionID, versions[0].ID)

‎integration/template-test/main.tf

+3-8
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,10 @@ terraform {
77
}
88
}
99

10-
provider "coderd" {
11-
url = "http://localhost:3000"
12-
token = "NbRNSwdzeb-Npwlm9TIOX3bpEQIsgt2KI"
13-
}
14-
1510
resource "coderd_user" "ethan" {
16-
username = "dean"
17-
name = "Dean Coolguy"
18-
email = "deantest@coder.com"
11+
username = "ethan"
12+
name = "Ethan Coolguy"
13+
email = "test@coder.com"
1914
roles = ["owner", "template-admin"]
2015
login_type = "password"
2116
password = "SomeSecurePassword!"

‎internal/provider/group_resource.go

+7-7
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66

77
"github.com/coder/coder/v2/codersdk"
88
"github.com/google/uuid"
9+
"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
910
"github.com/hashicorp/terraform-plugin-framework/attr"
1011
"github.com/hashicorp/terraform-plugin-framework/path"
1112
"github.com/hashicorp/terraform-plugin-framework/resource"
@@ -14,6 +15,7 @@ import (
1415
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
1516
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault"
1617
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
18+
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
1719
"github.com/hashicorp/terraform-plugin-framework/types"
1820
"github.com/hashicorp/terraform-plugin-log/tflog"
1921
)
@@ -68,7 +70,10 @@ func (r *GroupResource) Schema(ctx context.Context, req resource.SchemaRequest,
6870
MarkdownDescription: "The display name of the group. Defaults to the group name.",
6971
Computed: true,
7072
Optional: true,
71-
// Defaulted in Create
73+
Validators: []validator.String{
74+
stringvalidator.LengthAtLeast(1),
75+
},
76+
Default: stringdefault.StaticString(""),
7277
},
7378
"avatar_url": schema.StringAttribute{
7479
MarkdownDescription: "The URL of the group's avatar.",
@@ -139,15 +144,10 @@ func (r *GroupResource) Create(ctx context.Context, req resource.CreateRequest,
139144

140145
orgID := data.OrganizationID.ValueUUID()
141146

142-
displayName := data.Name.ValueString()
143-
if data.DisplayName.ValueString() != "" {
144-
displayName = data.DisplayName.ValueString()
145-
}
146-
147147
tflog.Trace(ctx, "creating group")
148148
group, err := client.CreateGroup(ctx, orgID, codersdk.CreateGroupRequest{
149149
Name: data.Name.ValueString(),
150-
DisplayName: displayName,
150+
DisplayName: data.DisplayName.ValueString(),
151151
AvatarURL: data.AvatarURL.ValueString(),
152152
QuotaAllowance: int(data.QuotaAllowance.ValueInt32()),
153153
})

‎internal/provider/template_resource.go

+32-21
Original file line numberDiff line numberDiff line change
@@ -796,19 +796,24 @@ func (d *versionsPlanModifier) MarkdownDescription(context.Context) string {
796796

797797
// PlanModifyObject implements planmodifier.List.
798798
func (d *versionsPlanModifier) PlanModifyList(ctx context.Context, req planmodifier.ListRequest, resp *planmodifier.ListResponse) {
799-
var data Versions
800-
resp.Diagnostics.Append(req.PlanValue.ElementsAs(ctx, &data, false)...)
799+
var planVersions Versions
800+
resp.Diagnostics.Append(req.PlanValue.ElementsAs(ctx, &planVersions, false)...)
801+
if resp.Diagnostics.HasError() {
802+
return
803+
}
804+
var configVersions Versions
805+
resp.Diagnostics.Append(req.ConfigValue.ElementsAs(ctx, &configVersions, false)...)
801806
if resp.Diagnostics.HasError() {
802807
return
803808
}
804809

805-
for i := range data {
806-
hash, err := computeDirectoryHash(data[i].Directory.ValueString())
810+
for i := range planVersions {
811+
hash, err := computeDirectoryHash(planVersions[i].Directory.ValueString())
807812
if err != nil {
808813
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to compute directory hash: %s", err))
809814
return
810815
}
811-
data[i].DirectoryHash = types.StringValue(hash)
816+
planVersions[i].DirectoryHash = types.StringValue(hash)
812817
}
813818

814819
var lv LastVersionsByHash
@@ -828,9 +833,9 @@ func (d *versionsPlanModifier) PlanModifyList(ctx context.Context, req planmodif
828833
}
829834
}
830835

831-
data.reconcileVersionIDs(lv)
836+
planVersions.reconcileVersionIDs(lv, configVersions)
832837

833-
resp.PlanValue, resp.Diagnostics = types.ListValueFrom(ctx, req.PlanValue.ElementType(ctx), data)
838+
resp.PlanValue, resp.Diagnostics = types.ListValueFrom(ctx, req.PlanValue.ElementType(ctx), planVersions)
834839
}
835840

836841
func NewVersionsPlanModifier() planmodifier.List {
@@ -1166,22 +1171,28 @@ func (v Versions) setPrivateState(ctx context.Context, ps privateState) (diags d
11661171
return ps.SetKey(ctx, LastVersionsKey, lvBytes)
11671172
}
11681173

1169-
func (v Versions) reconcileVersionIDs(lv LastVersionsByHash) {
1170-
for i := range v {
1171-
prevList, ok := lv[v[i].DirectoryHash.ValueString()]
1174+
func (planVersions Versions) reconcileVersionIDs(lv LastVersionsByHash, configVersions Versions) {
1175+
for i := range planVersions {
1176+
prevList, ok := lv[planVersions[i].DirectoryHash.ValueString()]
11721177
// If not in state, mark as known after apply since we'll create a new version.
11731178
// Versions whose Terraform configuration has not changed will have known
11741179
// IDs at this point, so we need to set this manually.
11751180
if !ok {
1176-
v[i].ID = NewUUIDUnknown()
1181+
planVersions[i].ID = NewUUIDUnknown()
1182+
// We might have the old randomly generated name in the plan,
1183+
// so unless the user has set it to a new one, we need to set it to
1184+
// unknown so that a new one is generated
1185+
if configVersions[i].Name.IsNull() {
1186+
planVersions[i].Name = types.StringUnknown()
1187+
}
11771188
} else {
11781189
// More than one candidate, try to match by name
11791190
for j, prev := range prevList {
11801191
// If the name is the same, use the existing ID, and remove
11811192
// it from the previous version candidates
1182-
if v[i].Name.ValueString() == prev.Name {
1183-
v[i].ID = UUIDValue(prev.ID)
1184-
lv[v[i].DirectoryHash.ValueString()] = append(prevList[:j], prevList[j+1:]...)
1193+
if planVersions[i].Name.ValueString() == prev.Name {
1194+
planVersions[i].ID = UUIDValue(prev.ID)
1195+
lv[planVersions[i].DirectoryHash.ValueString()] = append(prevList[:j], prevList[j+1:]...)
11851196
break
11861197
}
11871198
}
@@ -1190,14 +1201,14 @@ func (v Versions) reconcileVersionIDs(lv LastVersionsByHash) {
11901201

11911202
// For versions whose hash was found in the private state but couldn't be
11921203
// matched, use the leftovers in the order they appear
1193-
for i := range v {
1194-
prevList := lv[v[i].DirectoryHash.ValueString()]
1195-
if len(prevList) > 0 && v[i].ID.IsUnknown() {
1196-
v[i].ID = UUIDValue(prevList[0].ID)
1197-
if v[i].Name.IsUnknown() {
1198-
v[i].Name = types.StringValue(prevList[0].Name)
1204+
for i := range planVersions {
1205+
prevList := lv[planVersions[i].DirectoryHash.ValueString()]
1206+
if len(prevList) > 0 && planVersions[i].ID.IsUnknown() {
1207+
planVersions[i].ID = UUIDValue(prevList[0].ID)
1208+
if planVersions[i].Name.IsUnknown() {
1209+
planVersions[i].Name = types.StringValue(prevList[0].Name)
11991210
}
1200-
lv[v[i].DirectoryHash.ValueString()] = prevList[1:]
1211+
lv[planVersions[i].DirectoryHash.ValueString()] = prevList[1:]
12011212
}
12021213
}
12031214
}

‎internal/provider/template_resource_test.go

+70-7
Original file line numberDiff line numberDiff line change
@@ -579,13 +579,14 @@ func TestReconcileVersionIDs(t *testing.T) {
579579
bUUID := uuid.New()
580580
cases := []struct {
581581
Name string
582-
inputVersions Versions
582+
planVersions Versions
583+
configVersions Versions
583584
inputState LastVersionsByHash
584585
expectedVersions Versions
585586
}{
586587
{
587588
Name: "IdenticalDontRename",
588-
inputVersions: []TemplateVersion{
589+
planVersions: []TemplateVersion{
589590
{
590591
Name: types.StringValue("foo"),
591592
DirectoryHash: types.StringValue("aaa"),
@@ -597,6 +598,14 @@ func TestReconcileVersionIDs(t *testing.T) {
597598
ID: NewUUIDUnknown(),
598599
},
599600
},
601+
configVersions: []TemplateVersion{
602+
{
603+
Name: types.StringValue("foo"),
604+
},
605+
{
606+
Name: types.StringValue("bar"),
607+
},
608+
},
600609
inputState: map[string][]PreviousTemplateVersion{
601610
"aaa": {
602611
{
@@ -620,7 +629,7 @@ func TestReconcileVersionIDs(t *testing.T) {
620629
},
621630
{
622631
Name: "IdenticalRenameFirst",
623-
inputVersions: []TemplateVersion{
632+
planVersions: []TemplateVersion{
624633
{
625634
Name: types.StringValue("foo"),
626635
DirectoryHash: types.StringValue("aaa"),
@@ -632,6 +641,14 @@ func TestReconcileVersionIDs(t *testing.T) {
632641
ID: NewUUIDUnknown(),
633642
},
634643
},
644+
configVersions: []TemplateVersion{
645+
{
646+
Name: types.StringValue("foo"),
647+
},
648+
{
649+
Name: types.StringValue("bar"),
650+
},
651+
},
635652
inputState: map[string][]PreviousTemplateVersion{
636653
"aaa": {
637654
{
@@ -655,7 +672,7 @@ func TestReconcileVersionIDs(t *testing.T) {
655672
},
656673
{
657674
Name: "IdenticalHashesInState",
658-
inputVersions: []TemplateVersion{
675+
planVersions: []TemplateVersion{
659676
{
660677
Name: types.StringValue("foo"),
661678
DirectoryHash: types.StringValue("aaa"),
@@ -667,6 +684,14 @@ func TestReconcileVersionIDs(t *testing.T) {
667684
ID: NewUUIDUnknown(),
668685
},
669686
},
687+
configVersions: []TemplateVersion{
688+
{
689+
Name: types.StringValue("foo"),
690+
},
691+
{
692+
Name: types.StringValue("bar"),
693+
},
694+
},
670695
inputState: map[string][]PreviousTemplateVersion{
671696
"aaa": {
672697
{
@@ -694,7 +719,7 @@ func TestReconcileVersionIDs(t *testing.T) {
694719
},
695720
{
696721
Name: "UnknownUsesStateInOrder",
697-
inputVersions: []TemplateVersion{
722+
planVersions: []TemplateVersion{
698723
{
699724
Name: types.StringValue("foo"),
700725
DirectoryHash: types.StringValue("aaa"),
@@ -706,6 +731,14 @@ func TestReconcileVersionIDs(t *testing.T) {
706731
ID: NewUUIDUnknown(),
707732
},
708733
},
734+
configVersions: []TemplateVersion{
735+
{
736+
Name: types.StringValue("foo"),
737+
},
738+
{
739+
Name: types.StringValue("bar"),
740+
},
741+
},
709742
inputState: map[string][]PreviousTemplateVersion{
710743
"aaa": {
711744
{
@@ -731,13 +764,43 @@ func TestReconcileVersionIDs(t *testing.T) {
731764
},
732765
},
733766
},
767+
{
768+
Name: "NewVersionNewRandomName",
769+
planVersions: []TemplateVersion{
770+
{
771+
Name: types.StringValue("weird_draught12"),
772+
DirectoryHash: types.StringValue("bbb"),
773+
ID: UUIDValue(aUUID),
774+
},
775+
},
776+
configVersions: []TemplateVersion{
777+
{
778+
Name: types.StringNull(),
779+
},
780+
},
781+
inputState: map[string][]PreviousTemplateVersion{
782+
"aaa": {
783+
{
784+
ID: aUUID,
785+
Name: "weird_draught12",
786+
},
787+
},
788+
},
789+
expectedVersions: []TemplateVersion{
790+
{
791+
Name: types.StringUnknown(),
792+
DirectoryHash: types.StringValue("bbb"),
793+
ID: NewUUIDUnknown(),
794+
},
795+
},
796+
},
734797
}
735798

736799
for _, c := range cases {
737800
c := c
738801
t.Run(c.Name, func(t *testing.T) {
739-
c.inputVersions.reconcileVersionIDs(c.inputState)
740-
require.Equal(t, c.expectedVersions, c.inputVersions)
802+
c.planVersions.reconcileVersionIDs(c.inputState, c.configVersions)
803+
require.Equal(t, c.expectedVersions, c.planVersions)
741804
})
742805

743806
}

0 commit comments

Comments
 (0)