From 31bf61218039286d2c4a58e2757eb0be09e50e5d Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 29 Jul 2024 11:12:08 +0000 Subject: [PATCH] fix: support non-enterprise template resources --- docs/resources/group.md | 2 +- docs/resources/template.md | 56 +++---- internal/provider/group_data_source.go | 6 +- internal/provider/group_resource.go | 2 +- internal/provider/organization_data_source.go | 6 +- internal/provider/template_resource.go | 137 +++++++++--------- internal/provider/template_resource_test.go | 82 ++++++++--- 7 files changed, 167 insertions(+), 124 deletions(-) diff --git a/docs/resources/group.md b/docs/resources/group.md index 1972265..92feeef 100644 --- a/docs/resources/group.md +++ b/docs/resources/group.md @@ -23,7 +23,7 @@ A group on the Coder deployment. If you want to have a group resource with unman - `avatar_url` (String) The URL of the group's avatar. - `display_name` (String) The display name of the group. Defaults to the group name. -- `members` (Set of String) Members of the group, by ID. If null, members will not be added or removed. +- `members` (Set of String) Members of the group, by ID. If null, members will not be added or removed by Terraform. - `organization_id` (String) The organization ID that the group belongs to. Defaults to the provider default organization ID. - `quota_allowance` (Number) The number of quota credits to allocate to each user in the group. diff --git a/docs/resources/template.md b/docs/resources/template.md index 924cd44..da9a590 100644 --- a/docs/resources/template.md +++ b/docs/resources/template.md @@ -17,12 +17,12 @@ A Coder template ### Required -- `acl` (Attributes) Access control list for the template. (see [below for nested schema](#nestedatt--acl)) - `name` (String) The name of the template. - `versions` (Attributes List) (see [below for nested schema](#nestedatt--versions)) ### Optional +- `acl` (Attributes) Access control list for the template. Requires an enterprise Coder deployment. If null, ACL policies will not be added or removed by Terraform. (see [below for nested schema](#nestedatt--acl)) - `allow_user_auto_start` (Boolean) - `allow_user_auto_stop` (Boolean) - `description` (String) A description of the template. @@ -34,33 +34,6 @@ A Coder template - `id` (String) The ID of the template. - -### Nested Schema for `acl` - -Required: - -- `groups` (Attributes Set) (see [below for nested schema](#nestedatt--acl--groups)) -- `users` (Attributes Set) (see [below for nested schema](#nestedatt--acl--users)) - - -### Nested Schema for `acl.groups` - -Required: - -- `id` (String) -- `role` (String) - - - -### Nested Schema for `acl.users` - -Required: - -- `id` (String) -- `role` (String) - - - ### Nested Schema for `versions` @@ -97,3 +70,30 @@ Required: - `name` (String) - `value` (String) + + + + +### Nested Schema for `acl` + +Required: + +- `groups` (Attributes Set) (see [below for nested schema](#nestedatt--acl--groups)) +- `users` (Attributes Set) (see [below for nested schema](#nestedatt--acl--users)) + + +### Nested Schema for `acl.groups` + +Required: + +- `id` (String) +- `role` (String) + + + +### Nested Schema for `acl.users` + +Required: + +- `id` (String) +- `role` (String) diff --git a/internal/provider/group_data_source.go b/internal/provider/group_data_source.go index 4906b9a..7d5551c 100644 --- a/internal/provider/group_data_source.go +++ b/internal/provider/group_data_source.go @@ -174,8 +174,10 @@ func (d *GroupDataSource) Read(ctx context.Context, req datasource.ReadRequest, data.OrganizationID = UUIDValue(d.data.DefaultOrganizationID) } - var group codersdk.Group - var err error + var ( + group codersdk.Group + err error + ) if !data.ID.IsNull() { groupID := data.ID.ValueUUID() group, err = client.Group(ctx, groupID) diff --git a/internal/provider/group_resource.go b/internal/provider/group_resource.go index 46bc6c9..f396108 100644 --- a/internal/provider/group_resource.go +++ b/internal/provider/group_resource.go @@ -93,7 +93,7 @@ func (r *GroupResource) Schema(ctx context.Context, req resource.SchemaRequest, }, }, "members": schema.SetAttribute{ - MarkdownDescription: "Members of the group, by ID. If null, members will not be added or removed.", + MarkdownDescription: "Members of the group, by ID. If null, members will not be added or removed by Terraform.", ElementType: UUIDType, Optional: true, }, diff --git a/internal/provider/organization_data_source.go b/internal/provider/organization_data_source.go index 772e43c..8471adc 100644 --- a/internal/provider/organization_data_source.go +++ b/internal/provider/organization_data_source.go @@ -115,8 +115,10 @@ func (d *OrganizationDataSource) Read(ctx context.Context, req datasource.ReadRe client := d.data.Client - var org codersdk.Organization - var err error + var ( + org codersdk.Organization + err error + ) if !data.ID.IsNull() { // By ID orgID := data.ID.ValueUUID() org, err = client.Organization(ctx, orgID) diff --git a/internal/provider/template_resource.go b/internal/provider/template_resource.go index 4b62fe2..0439634 100644 --- a/internal/provider/template_resource.go +++ b/internal/provider/template_resource.go @@ -12,6 +12,7 @@ import ( "github.com/google/uuid" "github.com/hashicorp/terraform-plugin-framework-validators/listvalidator" "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" "github.com/hashicorp/terraform-plugin-framework/resource/schema" @@ -22,6 +23,7 @@ import ( "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-framework/types/basetypes" "github.com/hashicorp/terraform-plugin-log/tflog" ) @@ -51,11 +53,11 @@ type TemplateResourceModel struct { AllowUserAutoStart types.Bool `tfsdk:"allow_user_auto_start"` AllowUserAutoStop types.Bool `tfsdk:"allow_user_auto_stop"` - ACL *ACL `tfsdk:"acl"` - Versions Versions `tfsdk:"versions"` + ACL types.Object `tfsdk:"acl"` + Versions Versions `tfsdk:"versions"` } -// EqualTemplateMetadata returns true if two templates have identical metadata & ACL. +// EqualTemplateMetadata returns true if two templates have identical metadata (excluding ACL). func (m TemplateResourceModel) EqualTemplateMetadata(other TemplateResourceModel) bool { return m.Name.Equal(other.Name) && m.DisplayName.Equal(other.DisplayName) && @@ -63,8 +65,7 @@ func (m TemplateResourceModel) EqualTemplateMetadata(other TemplateResourceModel m.OrganizationID.Equal(other.OrganizationID) && m.Icon.Equal(other.Icon) && m.AllowUserAutoStart.Equal(other.AllowUserAutoStart) && - m.AllowUserAutoStop.Equal(other.AllowUserAutoStop) && - m.ACL.Equal(other.ACL) + m.AllowUserAutoStop.Equal(other.AllowUserAutoStop) } type TemplateVersion struct { @@ -110,38 +111,10 @@ type ACL struct { GroupPermissions []Permission `tfsdk:"groups"` } -func (a *ACL) Equal(other *ACL) bool { - if len(a.UserPermissions) != len(other.UserPermissions) { - return false - } - if len(a.GroupPermissions) != len(other.GroupPermissions) { - return false - } - for _, e1 := range a.UserPermissions { - found := false - for _, e2 := range other.UserPermissions { - if e1.Equal(&e2) { - found = true - break - } - } - if !found { - return false - } - } - for _, e1 := range a.GroupPermissions { - found := false - for _, e2 := range other.GroupPermissions { - if e1.Equal(&e2) { - found = true - break - } - } - if !found { - return false - } - } - return true +// aclTypeAttr is the type schema for an instance of `ACL`. +var aclTypeAttr = map[string]attr.Type{ + "users": permissionTypeAttr, + "groups": permissionTypeAttr, } type Permission struct { @@ -151,12 +124,8 @@ type Permission struct { Role types.String `tfsdk:"role"` } -func (p *Permission) Equal(other *Permission) bool { - return p.ID.Equal(other.ID) && p.Role.Equal(other.Role) -} - -// permissionsAttribute is the attribute schema for an instance of `[]Permission`. -var permissionsAttribute = schema.SetNestedAttribute{ +// permissionAttribute is the attribute schema for an instance of `[]Permission`. +var permissionAttribute = schema.SetNestedAttribute{ Required: true, NestedObject: schema.NestedAttributeObject{ Attributes: map[string]schema.Attribute{ @@ -165,14 +134,19 @@ var permissionsAttribute = schema.SetNestedAttribute{ }, "role": schema.StringAttribute{ Required: true, - Validators: []validator.String{ - stringvalidator.OneOf("admin", "use", ""), - }, }, }, }, } +// permissionTypeAttr is the type schema for an instance of `[]Permission`. +var permissionTypeAttr = basetypes.SetType{ElemType: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "id": basetypes.StringType{}, + "role": basetypes.StringType{}, + }, +}} + func (r *TemplateResource) Metadata(ctx context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) { resp.TypeName = req.ProviderTypeName + "_template" } @@ -234,11 +208,11 @@ func (r *TemplateResource) Schema(ctx context.Context, req resource.SchemaReques Default: booldefault.StaticBool(true), }, "acl": schema.SingleNestedAttribute{ - MarkdownDescription: "Access control list for the template.", - Required: true, + MarkdownDescription: "Access control list for the template. Requires an enterprise Coder deployment. If null, ACL policies will not be added or removed by Terraform.", + Optional: true, Attributes: map[string]schema.Attribute{ - "users": permissionsAttribute, - "groups": permissionsAttribute, + "users": permissionAttribute, + "groups": permissionAttribute, }, }, "versions": schema.ListNestedAttribute{ @@ -371,13 +345,22 @@ func (r *TemplateResource) Create(ctx context.Context, req resource.CreateReques "id": templateResp.ID, }) - tflog.Trace(ctx, "updating template ACL") - err = client.UpdateTemplateACL(ctx, templateResp.ID, convertACLToRequest(data.ACL)) - if err != nil { - resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to update template ACL: %s", err)) - return + if !data.ACL.IsNull() { + tflog.Trace(ctx, "updating template ACL") + var acl ACL + resp.Diagnostics.Append( + data.ACL.As(ctx, &acl, basetypes.ObjectAsOptions{})..., + ) + if resp.Diagnostics.HasError() { + return + } + err = client.UpdateTemplateACL(ctx, templateResp.ID, convertACLToRequest(acl)) + if err != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to create template ACL: %s", err)) + return + } + tflog.Trace(ctx, "successfully updated template ACL") } - tflog.Trace(ctx, "successfully updated template ACL") } if version.Active.ValueBool() { tflog.Trace(ctx, "marking template version as active", map[string]any{ @@ -430,12 +413,22 @@ func (r *TemplateResource) Read(ctx context.Context, req resource.ReadRequest, r data.AllowUserAutoStart = types.BoolValue(template.AllowUserAutostart) data.AllowUserAutoStop = types.BoolValue(template.AllowUserAutostop) - acl, err := client.TemplateACL(ctx, templateID) - if err != nil { - resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to get template ACL: %s", err)) - return + if !data.ACL.IsNull() { + tflog.Trace(ctx, "reading template ACL") + acl, err := client.TemplateACL(ctx, templateID) + if err != nil { + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to get template ACL: %s", err)) + return + } + tfACL := convertResponseToACL(acl) + aclObj, diag := types.ObjectValueFrom(ctx, aclTypeAttr, tfACL) + diag.Append(diag...) + if diag.HasError() { + return + } + data.ACL = aclObj + tflog.Trace(ctx, "read template ACL") } - data.ACL = convertResponseToACL(acl) for idx, version := range data.Versions { versionID := version.ID.ValueUUID() @@ -500,11 +493,20 @@ func (r *TemplateResource) Update(ctx context.Context, req resource.UpdateReques DisableEveryoneGroupAccess: true, }) if err != nil { - resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to update template: %s", err)) + resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to update template metadata: %s", err)) return } tflog.Trace(ctx, "successfully updated template metadata") - err = client.UpdateTemplateACL(ctx, templateID, convertACLToRequest(planState.ACL)) + } + + // If there's a change, and we're still managing ACL + if !planState.ACL.Equal(curState.ACL) && !planState.ACL.IsNull() { + var acl ACL + resp.Diagnostics.Append(planState.ACL.As(ctx, &acl, basetypes.ObjectAsOptions{})...) + if resp.Diagnostics.HasError() { + return + } + err := client.UpdateTemplateACL(ctx, templateID, convertACLToRequest(acl)) if err != nil { resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to update template ACL: %s", err)) return @@ -784,10 +786,7 @@ func newVersion(ctx context.Context, client *codersdk.Client, req newVersionRequ return &versionResp, nil } -func convertACLToRequest(permissions *ACL) codersdk.UpdateTemplateACL { - if permissions == nil { - return codersdk.UpdateTemplateACL{} - } +func convertACLToRequest(permissions ACL) codersdk.UpdateTemplateACL { var userPerms = make(map[string]codersdk.TemplateRole) for _, perm := range permissions.UserPermissions { userPerms[perm.ID.ValueString()] = codersdk.TemplateRole(perm.Role.ValueString()) @@ -802,7 +801,7 @@ func convertACLToRequest(permissions *ACL) codersdk.UpdateTemplateACL { } } -func convertResponseToACL(acl codersdk.TemplateACL) *ACL { +func convertResponseToACL(acl codersdk.TemplateACL) ACL { userPerms := make([]Permission, 0, len(acl.Users)) for _, user := range acl.Users { userPerms = append(userPerms, Permission{ @@ -817,7 +816,7 @@ func convertResponseToACL(acl codersdk.TemplateACL) *ACL { Role: types.StringValue(string(group.Role)), }) } - return &ACL{ + return ACL{ UserPermissions: userPerms, GroupPermissions: groupPerms, } diff --git a/internal/provider/template_resource_test.go b/internal/provider/template_resource_test.go index 29307b1..6608913 100644 --- a/internal/provider/template_resource_test.go +++ b/internal/provider/template_resource_test.go @@ -41,10 +41,12 @@ func TestAccTemplateResource(t *testing.T) { }, }, }, - GroupACL: []testAccTemplateKeyValueConfig{ - { - Key: PtrTo(firstUser.OrganizationIDs[0].String()), - Value: PtrTo("use"), + ACL: testAccTemplateACLConfig{ + GroupACL: []testAccTemplateKeyValueConfig{ + { + Key: PtrTo(firstUser.OrganizationIDs[0].String()), + Value: PtrTo("use"), + }, }, }, } @@ -54,7 +56,7 @@ func TestAccTemplateResource(t *testing.T) { cfg2.Name = PtrTo("example-template-new") cfg2.Versions[0].Directory = PtrTo("../../integration/template-test/example-template-2/") cfg2.Versions[0].Name = PtrTo("new") - cfg2.UserACL = []testAccTemplateKeyValueConfig{ + cfg2.ACL.UserACL = []testAccTemplateKeyValueConfig{ { Key: PtrTo(firstUser.ID.String()), Value: PtrTo("admin"), @@ -87,8 +89,12 @@ func TestAccTemplateResource(t *testing.T) { cfg6 := cfg4 cfg6.Versions = slices.Clone(cfg6.Versions[1:]) + cfg7 := cfg6 + cfg7.ACL.null = true + resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, + IsUnitTest: true, ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, Steps: []resource.TestStep{ { @@ -113,7 +119,7 @@ func TestAccTemplateResource(t *testing.T) { ImportState: true, ImportStateVerify: true, // In the real world, `versions` needs to be added to the configuration after importing - ImportStateVerifyIgnore: []string{"versions"}, + ImportStateVerifyIgnore: []string{"versions", "acl"}, }, // Update existing version & metadata { @@ -180,6 +186,13 @@ func TestAccTemplateResource(t *testing.T) { }), ), }, + // Unmanaged ACL + { + Config: cfg7.String(t), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckNoResourceAttr("coderd_template.test", "acl"), + ), + }, }, }) } @@ -193,25 +206,21 @@ type testAccTemplateResourceConfig struct { Description *string OrganizationID *string Versions []testAccTemplateVersionConfig - GroupACL []testAccTemplateKeyValueConfig - UserACL []testAccTemplateKeyValueConfig + ACL testAccTemplateACLConfig } -func (c testAccTemplateResourceConfig) String(t *testing.T) string { - t.Helper() - tpl := ` -provider coderd { - url = "{{.URL}}" - token = "{{.Token}}" +type testAccTemplateACLConfig struct { + null bool + GroupACL []testAccTemplateKeyValueConfig + UserACL []testAccTemplateKeyValueConfig } -resource "coderd_template" "test" { - name = {{orNull .Name}} - display_name = {{orNull .DisplayName}} - description = {{orNull .Description}} - organization_id = {{orNull .OrganizationID}} - - acl = { +func (c testAccTemplateACLConfig) String(t *testing.T) string { + if c.null == true { + return "null" + } + t.Helper() + tpl := `{ groups = [ {{- range .GroupACL}} { @@ -229,6 +238,37 @@ resource "coderd_template" "test" { {{- end}} ] } + ` + + funcMap := template.FuncMap{ + "orNull": PrintOrNull, + } + + buf := strings.Builder{} + tmpl, err := template.New("test").Funcs(funcMap).Parse(tpl) + require.NoError(t, err) + + err = tmpl.Execute(&buf, c) + require.NoError(t, err) + + return buf.String() +} + +func (c testAccTemplateResourceConfig) String(t *testing.T) string { + t.Helper() + tpl := ` +provider coderd { + url = "{{.URL}}" + token = "{{.Token}}" +} + +resource "coderd_template" "test" { + name = {{orNull .Name}} + display_name = {{orNull .DisplayName}} + description = {{orNull .Description}} + organization_id = {{orNull .OrganizationID}} + + acl = ` + c.ACL.String(t) + ` versions = [ {{- range .Versions }}