Skip to content

Commit ba0cbb3

Browse files
defelmnqdefelmnq
committed
improved testing and validation
1 parent 760b905 commit ba0cbb3

File tree

4 files changed

+183
-72
lines changed

4 files changed

+183
-72
lines changed

‎examples/data-sources/coder_resources_monitoring/data-source.tf

-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ data "coder_workspace" "dev" {}
77
resource "coder_agent" "main" {
88
arch = data.coder_provisioner.dev.arch
99
os = data.coder_provisioner.dev.os
10-
dir = "/workspace"
1110
resources_monitoring {
1211
memory {
1312
enabled = true

‎provider/agent.go

+74-15
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@ package provider
33
import (
44
"context"
55
"fmt"
6+
"path/filepath"
67
"reflect"
78
"strings"
89

910
"github.com/google/uuid"
11+
"github.com/hashicorp/go-cty/cty"
1012
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
1113
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
1214
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
@@ -264,13 +266,15 @@ func agentResource() *schema.Resource {
264266
Description: "The resources monitoring configuration for this agent.",
265267
ForceNew: true,
266268
Optional: true,
269+
MaxItems: 1,
267270
Elem: &schema.Resource{
268271
Schema: map[string]*schema.Schema{
269272
"memory": {
270273
Type: schema.TypeSet,
271274
Description: "The memory monitoring configuration for this agent.",
272275
ForceNew: true,
273276
Optional: true,
277+
MaxItems: 1,
274278
Elem: &schema.Resource{
275279
Schema: map[string]*schema.Schema{
276280
"enabled": {
@@ -284,6 +288,12 @@ func agentResource() *schema.Resource {
284288
Description: "The memory usage threshold in percentage at which to trigger an alert. Value should be between 0 and 100.",
285289
ForceNew: true,
286290
Required: true,
291+
ValidateDiagFunc: func(i interface{}, s cty.Path) diag.Diagnostics {
292+
if i.(int) < 0 || i.(int) > 100 {
293+
return diag.Errorf("volume threshold must be between 0 and 100")
294+
}
295+
return nil
296+
},
287297
},
288298
},
289299
},
@@ -300,6 +310,17 @@ func agentResource() *schema.Resource {
300310
Description: "The path of the volume to monitor.",
301311
ForceNew: true,
302312
Required: true,
313+
ValidateDiagFunc: func(i interface{}, s cty.Path) diag.Diagnostics {
314+
if i.(string) == "" {
315+
return diag.Errorf("volume path must not be empty")
316+
}
317+
318+
if !filepath.IsAbs(i.(string)) {
319+
return diag.Errorf("volume path must be an absolute path")
320+
}
321+
322+
return nil
323+
},
303324
},
304325
"enabled": {
305326
Type: schema.TypeBool,
@@ -312,6 +333,12 @@ func agentResource() *schema.Resource {
312333
Description: "The volume usage threshold in percentage at which to trigger an alert. Value should be between 0 and 100.",
313334
ForceNew: true,
314335
Required: true,
336+
ValidateDiagFunc: func(i interface{}, s cty.Path) diag.Diagnostics {
337+
if i.(int) < 0 || i.(int) > 100 {
338+
return diag.Errorf("volume threshold must be between 0 and 100")
339+
}
340+
return nil
341+
},
315342
},
316343
},
317344
},
@@ -321,29 +348,61 @@ func agentResource() *schema.Resource {
321348
},
322349
},
323350
CustomizeDiff: func(ctx context.Context, rd *schema.ResourceDiff, i any) error {
324-
if !rd.HasChange("metadata") {
325-
return nil
351+
if rd.HasChange("metadata") {
352+
keys := map[string]bool{}
353+
metadata, ok := rd.Get("metadata").([]any)
354+
if !ok {
355+
return xerrors.Errorf("unexpected type %T for metadata, expected []any", rd.Get("metadata"))
356+
}
357+
for _, t := range metadata {
358+
obj, ok := t.(map[string]any)
359+
if !ok {
360+
return xerrors.Errorf("unexpected type %T for metadata, expected map[string]any", t)
361+
}
362+
key, ok := obj["key"].(string)
363+
if !ok {
364+
return xerrors.Errorf("unexpected type %T for metadata key, expected string", obj["key"])
365+
}
366+
if keys[key] {
367+
return xerrors.Errorf("duplicate agent metadata key %q", key)
368+
}
369+
keys[key] = true
370+
}
326371
}
327372

328-
keys := map[string]bool{}
329-
metadata, ok := rd.Get("metadata").([]any)
330-
if !ok {
331-
return xerrors.Errorf("unexpected type %T for metadata, expected []any", rd.Get("metadata"))
332-
}
333-
for _, t := range metadata {
334-
obj, ok := t.(map[string]any)
373+
if rd.HasChange("resources_monitoring") {
374+
monitors, ok := rd.Get("resources_monitoring").(*schema.Set)
335375
if !ok {
336-
return xerrors.Errorf("unexpected type %T for metadata, expected map[string]any", t)
376+
return xerrors.Errorf("unexpected type %T for resources_monitoring.0.volume, expected []any", rd.Get("resources_monitoring.0.volume"))
337377
}
338-
key, ok := obj["key"].(string)
378+
379+
monitor := monitors.List()[0].(map[string]any)
380+
381+
volumes, ok := monitor["volume"].(*schema.Set)
339382
if !ok {
340-
return xerrors.Errorf("unexpected type %T for metadata key, expected string", obj["key"])
383+
return xerrors.Errorf("unexpected type %T for resources_monitoring.0.volume, expected []any", monitor["volume"])
341384
}
342-
if keys[key] {
343-
return xerrors.Errorf("duplicate agent metadata key %q", key)
385+
386+
paths := map[string]bool{}
387+
for _, volume := range volumes.List() {
388+
obj, ok := volume.(map[string]any)
389+
if !ok {
390+
return xerrors.Errorf("unexpected type %T for volume, expected map[string]any", volume)
391+
}
392+
393+
// print path for debug purpose
394+
395+
path, ok := obj["path"].(string)
396+
if !ok {
397+
return xerrors.Errorf("unexpected type %T for volume path, expected string", obj["path"])
398+
}
399+
if paths[path] {
400+
return xerrors.Errorf("duplicate volume path %q", path)
401+
}
402+
paths[path] = true
344403
}
345-
keys[key] = true
346404
}
405+
347406
return nil
348407
},
349408
}

‎provider/agent_test.go

+108-56
Original file line numberDiff line numberDiff line change
@@ -213,11 +213,13 @@ func TestAgent_Metadata(t *testing.T) {
213213

214214
func TestAgent_ResourcesMonitoring(t *testing.T) {
215215
t.Parallel()
216-
resource.Test(t, resource.TestCase{
217-
ProviderFactories: coderFactory(),
218-
IsUnitTest: true,
219-
Steps: []resource.TestStep{{
220-
Config: `
216+
217+
t.Run("OK", func(t *testing.T) {
218+
resource.Test(t, resource.TestCase{
219+
ProviderFactories: coderFactory(),
220+
IsUnitTest: true,
221+
Steps: []resource.TestStep{{
222+
Config: `
221223
provider "coder" {
222224
url = "https://example.com"
223225
}
@@ -241,65 +243,115 @@ func TestAgent_ResourcesMonitoring(t *testing.T) {
241243
}
242244
}
243245
}`,
244-
Check: func(state *terraform.State) error {
245-
require.Len(t, state.Modules, 1)
246-
require.Len(t, state.Modules[0].Resources, 1)
247-
248-
resource := state.Modules[0].Resources["coder_agent.dev"]
249-
require.NotNil(t, resource)
246+
Check: func(state *terraform.State) error {
247+
require.Len(t, state.Modules, 1)
248+
require.Len(t, state.Modules[0].Resources, 1)
250249

251-
t.Logf("resource: %v", resource.Primary.Attributes)
250+
resource := state.Modules[0].Resources["coder_agent.dev"]
251+
require.NotNil(t, resource)
252252

253-
attr := resource.Primary.Attributes
254-
require.Equal(t, "1", attr["resources_monitoring.#"])
255-
require.Equal(t, "1", attr["resources_monitoring.0.memory.#"])
256-
require.Equal(t, "2", attr["resources_monitoring.0.volume.#"])
257-
require.Equal(t, "80", attr["resources_monitoring.0.memory.0.threshold"])
258-
require.Equal(t, "/volume1", attr["resources_monitoring.0.volume.0.path"])
259-
require.Equal(t, "100", attr["resources_monitoring.0.volume.1.threshold"])
260-
require.Equal(t, "/volume2", attr["resources_monitoring.0.volume.1.path"])
261-
return nil
262-
},
263-
}},
253+
attr := resource.Primary.Attributes
254+
require.Equal(t, "1", attr["resources_monitoring.#"])
255+
require.Equal(t, "1", attr["resources_monitoring.0.memory.#"])
256+
require.Equal(t, "2", attr["resources_monitoring.0.volume.#"])
257+
require.Equal(t, "80", attr["resources_monitoring.0.memory.0.threshold"])
258+
require.Equal(t, "/volume1", attr["resources_monitoring.0.volume.0.path"])
259+
require.Equal(t, "100", attr["resources_monitoring.0.volume.1.threshold"])
260+
require.Equal(t, "/volume2", attr["resources_monitoring.0.volume.1.path"])
261+
return nil
262+
},
263+
}},
264+
})
264265
})
265-
}
266266

267-
func TestAgent_ResourcesMonitoring_OnlyMemory(t *testing.T) {
268-
t.Parallel()
269-
resource.Test(t, resource.TestCase{
270-
ProviderFactories: coderFactory(),
271-
IsUnitTest: true,
272-
Steps: []resource.TestStep{{
273-
Config: `
274-
provider "coder" {
275-
url = "https://example.com"
276-
}
277-
resource "coder_agent" "dev" {
278-
os = "linux"
279-
arch = "amd64"
280-
resources_monitoring {
281-
memory {
282-
enabled = true
283-
threshold = 80
284-
}
267+
t.Run("OnlyMemory", func(t *testing.T) {
268+
resource.Test(t, resource.TestCase{
269+
ProviderFactories: coderFactory(),
270+
IsUnitTest: true,
271+
Steps: []resource.TestStep{{
272+
Config: `
273+
provider "coder" {
274+
url = "https://example.com"
285275
}
286-
}`,
287-
Check: func(state *terraform.State) error {
288-
require.Len(t, state.Modules, 1)
289-
require.Len(t, state.Modules[0].Resources, 1)
276+
resource "coder_agent" "dev" {
277+
os = "linux"
278+
arch = "amd64"
279+
resources_monitoring {
280+
memory {
281+
enabled = true
282+
threshold = 80
283+
}
284+
}
285+
}`,
286+
Check: func(state *terraform.State) error {
287+
require.Len(t, state.Modules, 1)
288+
require.Len(t, state.Modules[0].Resources, 1)
290289

291-
resource := state.Modules[0].Resources["coder_agent.dev"]
292-
require.NotNil(t, resource)
290+
resource := state.Modules[0].Resources["coder_agent.dev"]
291+
require.NotNil(t, resource)
293292

294-
t.Logf("resource: %v", resource.Primary.Attributes)
293+
attr := resource.Primary.Attributes
294+
require.Equal(t, "1", attr["resources_monitoring.#"])
295+
require.Equal(t, "1", attr["resources_monitoring.0.memory.#"])
296+
require.Equal(t, "80", attr["resources_monitoring.0.memory.0.threshold"])
297+
return nil
298+
},
299+
}},
300+
})
301+
})
295302

296-
attr := resource.Primary.Attributes
297-
require.Equal(t, "1", attr["resources_monitoring.#"])
298-
require.Equal(t, "1", attr["resources_monitoring.0.memory.#"])
299-
require.Equal(t, "80", attr["resources_monitoring.0.memory.0.threshold"])
300-
return nil
301-
},
302-
}},
303+
t.Run("InvalidThreshold", func(t *testing.T) {
304+
resource.Test(t, resource.TestCase{
305+
ProviderFactories: coderFactory(),
306+
IsUnitTest: true,
307+
Steps: []resource.TestStep{{
308+
Config: `
309+
provider "coder" {
310+
url = "https://example.com"
311+
}
312+
resource "coder_agent" "dev" {
313+
os = "linux"
314+
arch = "amd64"
315+
resources_monitoring {
316+
memory {
317+
enabled = true
318+
threshold = 101
319+
}
320+
}
321+
}`,
322+
ExpectError: regexp.MustCompile("threshold must be between 0 and 100"),
323+
}},
324+
})
325+
})
326+
327+
t.Run("DuplicatePaths", func(t *testing.T) {
328+
resource.Test(t, resource.TestCase{
329+
ProviderFactories: coderFactory(),
330+
IsUnitTest: true,
331+
Steps: []resource.TestStep{{
332+
Config: `
333+
provider "coder" {
334+
url = "https://example.com"
335+
}
336+
resource "coder_agent" "dev" {
337+
os = "linux"
338+
arch = "amd64"
339+
resources_monitoring {
340+
volume {
341+
path = "/volume1"
342+
enabled = true
343+
threshold = 80
344+
}
345+
volume {
346+
path = "/volume1"
347+
enabled = true
348+
threshold = 100
349+
}
350+
}
351+
}`,
352+
ExpectError: regexp.MustCompile("duplicate volume path"),
353+
}},
354+
})
303355
})
304356
}
305357

‎provider/examples_test.go

+1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ func TestExamples(t *testing.T) {
1515
for _, testDir := range []string{
1616
"coder_parameter",
1717
"coder_workspace_tags",
18+
"coder_resources_monitoring",
1819
} {
1920
t.Run(testDir, func(t *testing.T) {
2021
testDir := testDir

0 commit comments

Comments
 (0)