From f195a20460eb2b6f9918995cfead519e40fbb257 Mon Sep 17 00:00:00 2001 From: Anton Tolchanov <1687799+knyar@users.noreply.github.com> Date: Thu, 1 Dec 2022 14:26:01 +0000 Subject: [PATCH] Avoid serializing empty ACL fields (#39) This should avoid meaningless diffs and unnecessary empty values while managing ACL contents via Terraform. Fixes #31 --- tailscale/client.go | 68 ++++++++++++++++------------------ tailscale/client_test.go | 4 +- tailscale/time_test.go | 80 ++++++++++++++++++++++++++++++++++++++-- 3 files changed, 109 insertions(+), 43 deletions(-) diff --git a/tailscale/client.go b/tailscale/client.go index 76fbd9f..c9646a0 100644 --- a/tailscale/client.go +++ b/tailscale/client.go @@ -230,7 +230,7 @@ func (c *Client) DNSNameservers(ctx context.Context) ([]string, error) { type ( ACL struct { - ACLs []ACLEntry `json:"acls" hujson:"ACLs,omitempty"` + ACLs []ACLEntry `json:"acls,omitempty" hujson:"ACLs,omitempty"` AutoApprovers *ACLAutoApprovers `json:"autoapprovers,omitempty" hujson:"AutoApprovers,omitempty"` Groups map[string][]string `json:"groups,omitempty" hujson:"Groups,omitempty"` Hosts map[string]string `json:"hosts,omitempty" hujson:"Hosts,omitempty"` @@ -245,25 +245,25 @@ type ( } ACLAutoApprovers struct { - Routes map[string][]string `json:"routes" hujson:"Routes"` - ExitNode []string `json:"exitNode" hujson:"ExitNode"` + Routes map[string][]string `json:"routes,omitempty" hujson:"Routes,omitempty"` + ExitNode []string `json:"exitNode,omitempty" hujson:"ExitNode,omitempty"` } ACLEntry struct { - Action string `json:"action" hujson:"Action"` - Ports []string `json:"ports" hujson:"Ports"` - Users []string `json:"users" hujson:"Users"` - Source []string `json:"src" hujson:"Src"` - Destination []string `json:"dst" hujson:"Dst"` - Protocol string `json:"proto" hujson:"Proto"` + Action string `json:"action,omitempty" hujson:"Action,omitempty"` + Ports []string `json:"ports,omitempty" hujson:"Ports,omitempty"` + Users []string `json:"users,omitempty" hujson:"Users,omitempty"` + Source []string `json:"src,omitempty" hujson:"Src,omitempty"` + Destination []string `json:"dst,omitempty" hujson:"Dst,omitempty"` + Protocol string `json:"proto,omitempty" hujson:"Proto,omitempty"` } ACLTest struct { - User string `json:"user" hujson:"User"` - Allow []string `json:"allow" hujson:"Allow"` - Deny []string `json:"deny" hujson:"Deny"` - Source string `json:"src" hujson:"Src"` - Accept []string `json:"accept" hujson:"Accept"` + User string `json:"user,omitempty" hujson:"User,omitempty"` + Allow []string `json:"allow,omitempty" hujson:"Allow,omitempty"` + Deny []string `json:"deny,omitempty" hujson:"Deny,omitempty"` + Source string `json:"src,omitempty" hujson:"Src,omitempty"` + Accept []string `json:"accept,omitempty" hujson:"Accept,omitempty"` } ACLDERPMap struct { @@ -294,11 +294,11 @@ type ( } ACLSSH struct { - Action string `json:"action" hujson:"Action"` - Users []string `json:"users" hujson:"Users"` - Source []string `json:"src" hujson:"Src"` - Destination []string `json:"dst" hujson:"Dst"` - CheckPeriod Duration `json:"checkPeriod" hujson:"CheckPeriod"` + Action string `json:"action,omitempty" hujson:"Action,omitempty"` + Users []string `json:"users,omitempty" hujson:"Users,omitempty"` + Source []string `json:"src,omitempty" hujson:"Src,omitempty"` + Destination []string `json:"dst,omitempty" hujson:"Dst,omitempty"` + CheckPeriod Duration `json:"checkPeriod,omitempty" hujson:"CheckPeriod,omitempty"` } NodeAttrGrant struct { @@ -694,31 +694,25 @@ func ErrorData(err error) []APIErrorData { // The Duration type wraps a time.Duration, allowing it to be JSON marshalled as a string like "20h" rather than // a numeric value. -type Duration struct { - time.Duration -} +type Duration time.Duration -// MarshalJSON is an implementation of json.Marshal. -func (d Duration) MarshalJSON() ([]byte, error) { - return json.Marshal(d.Duration.String()) +func (d Duration) String() string { + return time.Duration(d).String() } -// UnmarshalJSON unmarshals the content of data as a time.Duration, a blank string will keep the duration at its zero value. -func (d *Duration) UnmarshalJSON(data []byte) error { - if string(data) == `""` { - return nil - } +func (d Duration) MarshalText() ([]byte, error) { + return []byte(d.String()), nil +} - var str string - if err := json.Unmarshal(data, &str); err != nil { - return err +func (d *Duration) UnmarshalText(b []byte) error { + text := string(b) + if text == "" { + text = "0s" } - - dur, err := time.ParseDuration(str) + pd, err := time.ParseDuration(text) if err != nil { return err } - - d.Duration = dur + *d = Duration(pd) return nil } diff --git a/tailscale/client_test.go b/tailscale/client_test.go index 8374653..b89c4fd 100644 --- a/tailscale/client_test.go +++ b/tailscale/client_test.go @@ -116,7 +116,7 @@ func TestACL_Unmarshal(t *testing.T) { Source: []string{"tag:logging"}, Destination: []string{"tag:prod"}, Users: []string{"root", "autogroup:nonroot"}, - CheckPeriod: tailscale.Duration{Duration: time.Hour * 20}, + CheckPeriod: tailscale.Duration(time.Hour * 20), }, }, }, @@ -195,7 +195,7 @@ func TestACL_Unmarshal(t *testing.T) { Source: []string{"tag:logging"}, Destination: []string{"tag:prod"}, Users: []string{"root", "autogroup:nonroot"}, - CheckPeriod: tailscale.Duration{Duration: time.Hour * 20}, + CheckPeriod: tailscale.Duration(time.Hour * 20), }, }, Tests: []tailscale.ACLTest{ diff --git a/tailscale/time_test.go b/tailscale/time_test.go index 6e2b5bd..9d008d5 100644 --- a/tailscale/time_test.go +++ b/tailscale/time_test.go @@ -1,6 +1,7 @@ package tailscale_test import ( + "encoding/json" "testing" "time" @@ -57,8 +58,79 @@ func TestMarshalingTimestamps(t *testing.T) { } } -func TestWrapsStdDuration(t *testing.T) { - expectedDuration := tailscale.Duration{} - newDuration := time.Duration(0) - assert.Equal(t, expectedDuration.Duration, newDuration) +func TestDurationUnmarshal(t *testing.T) { + t.Parallel() + + tt := []struct { + Name string + Content string + Expected tailscale.Duration + }{ + { + Name: "It should handle empty string as zero value", + Content: `""`, + Expected: tailscale.Duration(0), + }, + { + Name: "It should handle null as zero value", + Content: `null`, + Expected: tailscale.Duration(0), + }, + { + Name: "It should handle 0s as zero value", + Content: `"0s"`, + Expected: tailscale.Duration(0), + }, + { + Name: "It should parse duration strings", + Content: `"15s"`, + Expected: tailscale.Duration(15 * time.Second), + }, + } + + for _, tc := range tt { + t.Run(tc.Name, func(t *testing.T) { + var actual tailscale.Duration + + assert.NoError(t, json.Unmarshal([]byte(tc.Content), &actual)) + assert.Equal(t, tc.Expected, actual) + }) + } +} + +func TestDurationMarshal(t *testing.T) { + t.Parallel() + + tt := []struct { + Name string + Content any + Expected string + }{ + { + Name: "It should marshal zero duration as 0s", + Content: struct{ D tailscale.Duration }{tailscale.Duration(0)}, + Expected: `{"D":"0s"}`, + }, + { + Name: "It should not marshal zero duration if omitempty", + Content: struct { + D tailscale.Duration `json:"d,omitempty"` + }{tailscale.Duration(0)}, + Expected: `{}`, + }, + { + Name: "It should marshal duration correctly", + Content: struct{ D tailscale.Duration }{tailscale.Duration(15 * time.Second)}, + Expected: `{"D":"15s"}`, + }, + } + + for _, tc := range tt { + t.Run(tc.Name, func(t *testing.T) { + actual, err := json.Marshal(tc.Content) + + assert.NoError(t, err) + assert.Equal(t, tc.Expected, string(actual)) + }) + } }