Skip to content

Commit

Permalink
fix: always use config name for new template versions (#66)
Browse files Browse the repository at this point in the history
  • Loading branch information
ethanndickson authored Aug 8, 2024
1 parent 403243a commit cc7335a
Show file tree
Hide file tree
Showing 7 changed files with 115 additions and 45 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ go 1.22.5

require (
cdr.dev/slog v1.6.2-0.20240126064726-20367d4aede6
github.com/coder/coder/v2 v2.14.0
github.com/coder/coder/v2 v2.14.1
github.com/docker/docker v27.1.1+incompatible
github.com/docker/go-connections v0.4.0
github.com/google/uuid v1.6.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ github.com/cloudflare/circl v1.3.7 h1:qlCDlTPz2n9fu58M0Nh1J/JzcFpfgkFHHX3O35r5vc
github.com/cloudflare/circl v1.3.7/go.mod h1:sRTcRWXGLrKw6yIGJ+l7amYJFfAXbZG0kBSc8r4zxgA=
github.com/coder/coder/v2 v2.14.0 h1:Z7iyLdnFHYh57xddSCS2IE5JDy0O6r957T4S9lt5Obo=
github.com/coder/coder/v2 v2.14.0/go.mod h1:dO79BI5XlP8rrtne1JpRcVehe27bNMXdZKyn1NsWbjA=
github.com/coder/coder/v2 v2.14.1 h1:tSYe7H4pNRL8hh9ynwHK7QYTOvG5hcuZJEOkn4ZPASE=
github.com/coder/coder/v2 v2.14.1/go.mod h1:dO79BI5XlP8rrtne1JpRcVehe27bNMXdZKyn1NsWbjA=
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/serpent v0.7.0 h1:zGpD2GlF3lKIVkMjNGKbkip88qzd5r/TRcc30X/SrT0=
Expand Down
1 change: 0 additions & 1 deletion integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ func TestIntegration(t *testing.T) {
})
require.NoError(t, err)
require.Len(t, versions, 2)
require.Equal(t, "latest", versions[0].Name)
require.NotEmpty(t, versions[0].ID)
require.Equal(t, templates[0].ID, *versions[0].TemplateID)
require.Equal(t, templates[0].ActiveVersionID, versions[0].ID)
Expand Down
11 changes: 3 additions & 8 deletions integration/template-test/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,10 @@ terraform {
}
}

provider "coderd" {
url = "http://localhost:3000"
token = "NbRNSwdzeb-Npwlm9TIOX3bpEQIsgt2KI"
}

resource "coderd_user" "ethan" {
username = "dean"
name = "Dean Coolguy"
email = "deantest@coder.com"
username = "ethan"
name = "Ethan Coolguy"
email = "test@coder.com"
roles = ["owner", "template-admin"]
login_type = "password"
password = "SomeSecurePassword!"
Expand Down
14 changes: 7 additions & 7 deletions internal/provider/group_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/coder/coder/v2/codersdk"
"github.com/google/uuid"
"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
"github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/path"
"github.com/hashicorp/terraform-plugin-framework/resource"
Expand All @@ -14,6 +15,7 @@ import (
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-log/tflog"
)
Expand Down Expand Up @@ -68,7 +70,10 @@ func (r *GroupResource) Schema(ctx context.Context, req resource.SchemaRequest,
MarkdownDescription: "The display name of the group. Defaults to the group name.",
Computed: true,
Optional: true,
// Defaulted in Create
Validators: []validator.String{
stringvalidator.LengthAtLeast(1),
},
Default: stringdefault.StaticString(""),
},
"avatar_url": schema.StringAttribute{
MarkdownDescription: "The URL of the group's avatar.",
Expand Down Expand Up @@ -139,15 +144,10 @@ func (r *GroupResource) Create(ctx context.Context, req resource.CreateRequest,

orgID := data.OrganizationID.ValueUUID()

displayName := data.Name.ValueString()
if data.DisplayName.ValueString() != "" {
displayName = data.DisplayName.ValueString()
}

tflog.Trace(ctx, "creating group")
group, err := client.CreateGroup(ctx, orgID, codersdk.CreateGroupRequest{
Name: data.Name.ValueString(),
DisplayName: displayName,
DisplayName: data.DisplayName.ValueString(),
AvatarURL: data.AvatarURL.ValueString(),
QuotaAllowance: int(data.QuotaAllowance.ValueInt32()),
})
Expand Down
53 changes: 32 additions & 21 deletions internal/provider/template_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -796,19 +796,24 @@ func (d *versionsPlanModifier) MarkdownDescription(context.Context) string {

// PlanModifyObject implements planmodifier.List.
func (d *versionsPlanModifier) PlanModifyList(ctx context.Context, req planmodifier.ListRequest, resp *planmodifier.ListResponse) {
var data Versions
resp.Diagnostics.Append(req.PlanValue.ElementsAs(ctx, &data, false)...)
var planVersions Versions
resp.Diagnostics.Append(req.PlanValue.ElementsAs(ctx, &planVersions, false)...)
if resp.Diagnostics.HasError() {
return
}
var configVersions Versions
resp.Diagnostics.Append(req.ConfigValue.ElementsAs(ctx, &configVersions, false)...)
if resp.Diagnostics.HasError() {
return
}

for i := range data {
hash, err := computeDirectoryHash(data[i].Directory.ValueString())
for i := range planVersions {
hash, err := computeDirectoryHash(planVersions[i].Directory.ValueString())
if err != nil {
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to compute directory hash: %s", err))
return
}
data[i].DirectoryHash = types.StringValue(hash)
planVersions[i].DirectoryHash = types.StringValue(hash)
}

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

data.reconcileVersionIDs(lv)
planVersions.reconcileVersionIDs(lv, configVersions)

resp.PlanValue, resp.Diagnostics = types.ListValueFrom(ctx, req.PlanValue.ElementType(ctx), data)
resp.PlanValue, resp.Diagnostics = types.ListValueFrom(ctx, req.PlanValue.ElementType(ctx), planVersions)
}

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

func (v Versions) reconcileVersionIDs(lv LastVersionsByHash) {
for i := range v {
prevList, ok := lv[v[i].DirectoryHash.ValueString()]
func (planVersions Versions) reconcileVersionIDs(lv LastVersionsByHash, configVersions Versions) {
for i := range planVersions {
prevList, ok := lv[planVersions[i].DirectoryHash.ValueString()]
// If not in state, mark as known after apply since we'll create a new version.
// Versions whose Terraform configuration has not changed will have known
// IDs at this point, so we need to set this manually.
if !ok {
v[i].ID = NewUUIDUnknown()
planVersions[i].ID = NewUUIDUnknown()
// We might have the old randomly generated name in the plan,
// so unless the user has set it to a new one, we need to set it to
// unknown so that a new one is generated
if configVersions[i].Name.IsNull() {
planVersions[i].Name = types.StringUnknown()
}
} else {
// More than one candidate, try to match by name
for j, prev := range prevList {
// If the name is the same, use the existing ID, and remove
// it from the previous version candidates
if v[i].Name.ValueString() == prev.Name {
v[i].ID = UUIDValue(prev.ID)
lv[v[i].DirectoryHash.ValueString()] = append(prevList[:j], prevList[j+1:]...)
if planVersions[i].Name.ValueString() == prev.Name {
planVersions[i].ID = UUIDValue(prev.ID)
lv[planVersions[i].DirectoryHash.ValueString()] = append(prevList[:j], prevList[j+1:]...)
break
}
}
Expand All @@ -1190,14 +1201,14 @@ func (v Versions) reconcileVersionIDs(lv LastVersionsByHash) {

// For versions whose hash was found in the private state but couldn't be
// matched, use the leftovers in the order they appear
for i := range v {
prevList := lv[v[i].DirectoryHash.ValueString()]
if len(prevList) > 0 && v[i].ID.IsUnknown() {
v[i].ID = UUIDValue(prevList[0].ID)
if v[i].Name.IsUnknown() {
v[i].Name = types.StringValue(prevList[0].Name)
for i := range planVersions {
prevList := lv[planVersions[i].DirectoryHash.ValueString()]
if len(prevList) > 0 && planVersions[i].ID.IsUnknown() {
planVersions[i].ID = UUIDValue(prevList[0].ID)
if planVersions[i].Name.IsUnknown() {
planVersions[i].Name = types.StringValue(prevList[0].Name)
}
lv[v[i].DirectoryHash.ValueString()] = prevList[1:]
lv[planVersions[i].DirectoryHash.ValueString()] = prevList[1:]
}
}
}
77 changes: 70 additions & 7 deletions internal/provider/template_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -579,13 +579,14 @@ func TestReconcileVersionIDs(t *testing.T) {
bUUID := uuid.New()
cases := []struct {
Name string
inputVersions Versions
planVersions Versions
configVersions Versions
inputState LastVersionsByHash
expectedVersions Versions
}{
{
Name: "IdenticalDontRename",
inputVersions: []TemplateVersion{
planVersions: []TemplateVersion{
{
Name: types.StringValue("foo"),
DirectoryHash: types.StringValue("aaa"),
Expand All @@ -597,6 +598,14 @@ func TestReconcileVersionIDs(t *testing.T) {
ID: NewUUIDUnknown(),
},
},
configVersions: []TemplateVersion{
{
Name: types.StringValue("foo"),
},
{
Name: types.StringValue("bar"),
},
},
inputState: map[string][]PreviousTemplateVersion{
"aaa": {
{
Expand All @@ -620,7 +629,7 @@ func TestReconcileVersionIDs(t *testing.T) {
},
{
Name: "IdenticalRenameFirst",
inputVersions: []TemplateVersion{
planVersions: []TemplateVersion{
{
Name: types.StringValue("foo"),
DirectoryHash: types.StringValue("aaa"),
Expand All @@ -632,6 +641,14 @@ func TestReconcileVersionIDs(t *testing.T) {
ID: NewUUIDUnknown(),
},
},
configVersions: []TemplateVersion{
{
Name: types.StringValue("foo"),
},
{
Name: types.StringValue("bar"),
},
},
inputState: map[string][]PreviousTemplateVersion{
"aaa": {
{
Expand All @@ -655,7 +672,7 @@ func TestReconcileVersionIDs(t *testing.T) {
},
{
Name: "IdenticalHashesInState",
inputVersions: []TemplateVersion{
planVersions: []TemplateVersion{
{
Name: types.StringValue("foo"),
DirectoryHash: types.StringValue("aaa"),
Expand All @@ -667,6 +684,14 @@ func TestReconcileVersionIDs(t *testing.T) {
ID: NewUUIDUnknown(),
},
},
configVersions: []TemplateVersion{
{
Name: types.StringValue("foo"),
},
{
Name: types.StringValue("bar"),
},
},
inputState: map[string][]PreviousTemplateVersion{
"aaa": {
{
Expand Down Expand Up @@ -694,7 +719,7 @@ func TestReconcileVersionIDs(t *testing.T) {
},
{
Name: "UnknownUsesStateInOrder",
inputVersions: []TemplateVersion{
planVersions: []TemplateVersion{
{
Name: types.StringValue("foo"),
DirectoryHash: types.StringValue("aaa"),
Expand All @@ -706,6 +731,14 @@ func TestReconcileVersionIDs(t *testing.T) {
ID: NewUUIDUnknown(),
},
},
configVersions: []TemplateVersion{
{
Name: types.StringValue("foo"),
},
{
Name: types.StringValue("bar"),
},
},
inputState: map[string][]PreviousTemplateVersion{
"aaa": {
{
Expand All @@ -731,13 +764,43 @@ func TestReconcileVersionIDs(t *testing.T) {
},
},
},
{
Name: "NewVersionNewRandomName",
planVersions: []TemplateVersion{
{
Name: types.StringValue("weird_draught12"),
DirectoryHash: types.StringValue("bbb"),
ID: UUIDValue(aUUID),
},
},
configVersions: []TemplateVersion{
{
Name: types.StringNull(),
},
},
inputState: map[string][]PreviousTemplateVersion{
"aaa": {
{
ID: aUUID,
Name: "weird_draught12",
},
},
},
expectedVersions: []TemplateVersion{
{
Name: types.StringUnknown(),
DirectoryHash: types.StringValue("bbb"),
ID: NewUUIDUnknown(),
},
},
},
}

for _, c := range cases {
c := c
t.Run(c.Name, func(t *testing.T) {
c.inputVersions.reconcileVersionIDs(c.inputState)
require.Equal(t, c.expectedVersions, c.inputVersions)
c.planVersions.reconcileVersionIDs(c.inputState, c.configVersions)
require.Equal(t, c.expectedVersions, c.planVersions)
})

}
Expand Down

0 comments on commit cc7335a

Please sign in to comment.