Skip to content

Commit

Permalink
Avoid serializing empty ACL fields (#39)
Browse files Browse the repository at this point in the history 
This should avoid meaningless diffs and unnecessary empty values while
managing ACL contents via Terraform.

Fixes #31
  • Loading branch information
@knyar
knyar authored Dec 1, 2022
1 parent b1040ba commit f195a20
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 43 deletions.
68 changes: 31 additions & 37 deletions tailscale/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
4 changes: 2 additions & 2 deletions tailscale/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
},
},
},
Expand Down Expand Up @@ -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{
Expand Down
80 changes: 76 additions & 4 deletions tailscale/time_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package tailscale_test

import (
"encoding/json"
"testing"
"time"

Expand Down Expand Up @@ -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))
})
}
}

0 comments on commit f195a20

Please sign in to comment.