From 4b742042eedfbe734eb5f92398529fd15fa3663f Mon Sep 17 00:00:00 2001 From: max furman Date: Fri, 18 Jan 2019 10:39:12 -0800 Subject: [PATCH 1/4] make Duration wrapper publicly accessible --- authority/config.go | 6 +++--- authority/provisioner.go | 6 +++--- authority/types.go | 13 +++++++------ 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/authority/config.go b/authority/config.go index a6a78523..3bc8e810 100644 --- a/authority/config.go +++ b/authority/config.go @@ -26,9 +26,9 @@ var ( } defaultDisableRenewal = false globalProvisionerClaims = ProvisionerClaims{ - MinTLSDur: &duration{5 * time.Minute}, - MaxTLSDur: &duration{24 * time.Hour}, - DefaultTLSDur: &duration{24 * time.Hour}, + MinTLSDur: &Duration{5 * time.Minute}, + MaxTLSDur: &Duration{24 * time.Hour}, + DefaultTLSDur: &Duration{24 * time.Hour}, DisableRenewal: &defaultDisableRenewal, } ) diff --git a/authority/provisioner.go b/authority/provisioner.go index 53372f11..6dd1b1ac 100644 --- a/authority/provisioner.go +++ b/authority/provisioner.go @@ -12,9 +12,9 @@ import ( // ProvisionerClaims so that individual provisioners can override global claims. type ProvisionerClaims struct { globalClaims *ProvisionerClaims - MinTLSDur *duration `json:"minTLSCertDuration,omitempty"` - MaxTLSDur *duration `json:"maxTLSCertDuration,omitempty"` - DefaultTLSDur *duration `json:"defaultTLSCertDuration,omitempty"` + MinTLSDur *Duration `json:"minTLSCertDuration,omitempty"` + MaxTLSDur *Duration `json:"maxTLSCertDuration,omitempty"` + DefaultTLSDur *Duration `json:"defaultTLSCertDuration,omitempty"` DisableRenewal *bool `json:"disableRenewal,omitempty"` } diff --git a/authority/types.go b/authority/types.go index d9120f59..0d003eac 100644 --- a/authority/types.go +++ b/authority/types.go @@ -7,25 +7,26 @@ import ( "github.com/pkg/errors" ) -type duration struct { +// Duration is a wrapper around Time.Duration to aid with marshal/unmarshal. +type Duration struct { time.Duration } -// MarshalJSON parses a duration string and sets it to the duration. +// MarshalJSON parses a Duration string and sets it to the duration. // // A duration string is a possibly signed sequence of decimal numbers, each with // optional fraction and a unit suffix, such as "300ms", "-1.5h" or "2h45m". // Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". -func (d *duration) MarshalJSON() ([]byte, error) { +func (d *Duration) MarshalJSON() ([]byte, error) { return json.Marshal(d.String()) } -// UnmarshalJSON parses a duration string and sets it to the duration. +// UnmarshalJSON parses a Duration string and sets it to the duration. // -// A duration string is a possibly signed sequence of decimal numbers, each with +// A Duration string is a possibly signed sequence of decimal numbers, each with // optional fraction and a unit suffix, such as "300ms", "-1.5h" or "2h45m". // Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". -func (d *duration) UnmarshalJSON(data []byte) (err error) { +func (d *Duration) UnmarshalJSON(data []byte) (err error) { var s string if err = json.Unmarshal(data, &s); err != nil { return errors.Wrapf(err, "error unmarshalling %s", data) From 0615f7eb11bf9a1e39caded936a1f54104a84369 Mon Sep 17 00:00:00 2001 From: max furman Date: Fri, 18 Jan 2019 12:08:18 -0800 Subject: [PATCH 2/4] don't wrap time.Duration --- authority/config.go | 9 ++++-- authority/provisioner.go | 18 ++++++------ authority/types.go | 27 ++++++++++------- authority/types_test.go | 62 +++++++++++++++++++++++++++++++++++++++- 4 files changed, 92 insertions(+), 24 deletions(-) diff --git a/authority/config.go b/authority/config.go index 3bc8e810..16a10a77 100644 --- a/authority/config.go +++ b/authority/config.go @@ -25,10 +25,13 @@ var ( Renegotiation: false, } defaultDisableRenewal = false + minTLSDur = 5 * time.Minute + maxTLSDur = 24 * time.Hour + defaultTLSDur = 24 * time.Hour globalProvisionerClaims = ProvisionerClaims{ - MinTLSDur: &Duration{5 * time.Minute}, - MaxTLSDur: &Duration{24 * time.Hour}, - DefaultTLSDur: &Duration{24 * time.Hour}, + MinTLSDur: (*duration)(&minTLSDur), + MaxTLSDur: (*duration)(&maxTLSDur), + DefaultTLSDur: (*duration)(&defaultTLSDur), DisableRenewal: &defaultDisableRenewal, } ) diff --git a/authority/provisioner.go b/authority/provisioner.go index 6dd1b1ac..e3dc7d1a 100644 --- a/authority/provisioner.go +++ b/authority/provisioner.go @@ -12,9 +12,9 @@ import ( // ProvisionerClaims so that individual provisioners can override global claims. type ProvisionerClaims struct { globalClaims *ProvisionerClaims - MinTLSDur *Duration `json:"minTLSCertDuration,omitempty"` - MaxTLSDur *Duration `json:"maxTLSCertDuration,omitempty"` - DefaultTLSDur *Duration `json:"defaultTLSCertDuration,omitempty"` + MinTLSDur *duration `json:"minTLSCertDuration,omitempty"` + MaxTLSDur *duration `json:"maxTLSCertDuration,omitempty"` + DefaultTLSDur *duration `json:"defaultTLSCertDuration,omitempty"` DisableRenewal *bool `json:"disableRenewal,omitempty"` } @@ -32,30 +32,30 @@ func (pc *ProvisionerClaims) Init(global *ProvisionerClaims) (*ProvisionerClaims // provisioner. If the default is not set within the provisioner, then the global // default from the authority configuration will be used. func (pc *ProvisionerClaims) DefaultTLSCertDuration() time.Duration { - if pc.DefaultTLSDur == nil || pc.DefaultTLSDur.Duration == 0 { + if pc.DefaultTLSDur == nil || *pc.DefaultTLSDur == 0 { return pc.globalClaims.DefaultTLSCertDuration() } - return pc.DefaultTLSDur.Duration + return time.Duration(*pc.DefaultTLSDur) } // MinTLSCertDuration returns the minimum TLS cert duration for the provisioner. // If the minimum is not set within the provisioner, then the global // minimum from the authority configuration will be used. func (pc *ProvisionerClaims) MinTLSCertDuration() time.Duration { - if pc.MinTLSDur == nil || pc.MinTLSDur.Duration == 0 { + if pc.MinTLSDur == nil || *pc.MinTLSDur == 0 { return pc.globalClaims.MinTLSCertDuration() } - return pc.MinTLSDur.Duration + return time.Duration(*pc.MinTLSDur) } // MaxTLSCertDuration returns the maximum TLS cert duration for the provisioner. // If the maximum is not set within the provisioner, then the global // maximum from the authority configuration will be used. func (pc *ProvisionerClaims) MaxTLSCertDuration() time.Duration { - if pc.MaxTLSDur == nil || pc.MaxTLSDur.Duration == 0 { + if pc.MaxTLSDur == nil || *pc.MaxTLSDur == 0 { return pc.globalClaims.MaxTLSCertDuration() } - return pc.MaxTLSDur.Duration + return time.Duration(*pc.MaxTLSDur) } // IsDisableRenewal returns if the renewal flow is disabled for the diff --git a/authority/types.go b/authority/types.go index 0d003eac..a0c7661a 100644 --- a/authority/types.go +++ b/authority/types.go @@ -8,32 +8,37 @@ import ( ) // Duration is a wrapper around Time.Duration to aid with marshal/unmarshal. -type Duration struct { - time.Duration -} +type duration time.Duration -// MarshalJSON parses a Duration string and sets it to the duration. +// MarshalJSON parses a duration string and sets it to the duration. // // A duration string is a possibly signed sequence of decimal numbers, each with // optional fraction and a unit suffix, such as "300ms", "-1.5h" or "2h45m". // Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". -func (d *Duration) MarshalJSON() ([]byte, error) { - return json.Marshal(d.String()) +func (d *duration) MarshalJSON() ([]byte, error) { + return json.Marshal((*time.Duration)(d).String()) } -// UnmarshalJSON parses a Duration string and sets it to the duration. +// UnmarshalJSON parses a duration string and sets it to the duration. // -// A Duration string is a possibly signed sequence of decimal numbers, each with +// A duration string is a possibly signed sequence of decimal numbers, each with // optional fraction and a unit suffix, such as "300ms", "-1.5h" or "2h45m". // Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". -func (d *Duration) UnmarshalJSON(data []byte) (err error) { - var s string +func (d *duration) UnmarshalJSON(data []byte) (err error) { + var ( + s string + _d time.Duration + ) + if d == nil { + return errors.New("duration cannot be nil") + } if err = json.Unmarshal(data, &s); err != nil { return errors.Wrapf(err, "error unmarshalling %s", data) } - if d.Duration, err = time.ParseDuration(s); err != nil { + if _d, err = time.ParseDuration(s); err != nil { return errors.Wrapf(err, "error parsing %s as duration", s) } + *d = duration(_d) return } diff --git a/authority/types_test.go b/authority/types_test.go index 36877dcc..6050409c 100644 --- a/authority/types_test.go +++ b/authority/types_test.go @@ -3,6 +3,7 @@ package authority import ( "reflect" "testing" + "time" ) func Test_multiString_First(t *testing.T) { @@ -71,7 +72,6 @@ func Test_multiString_MarshalJSON(t *testing.T) { } func Test_multiString_UnmarshalJSON(t *testing.T) { - type args struct { data []byte } @@ -101,3 +101,63 @@ func Test_multiString_UnmarshalJSON(t *testing.T) { }) } } + +func durPtr(_d time.Duration) *duration { + d := new(duration) + *d = duration(_d) + return d +} + +func Test_duration_UnmarshalJSON(t *testing.T) { + type args struct { + data []byte + } + tests := []struct { + name string + d *duration + args args + want *duration + wantErr bool + }{ + {"empty", new(duration), args{[]byte{}}, new(duration), true}, + {"bad type", new(duration), args{[]byte(`15`)}, new(duration), true}, + {"empty string", new(duration), args{[]byte(`""`)}, new(duration), true}, + {"non duration", new(duration), args{[]byte(`"15"`)}, new(duration), true}, + {"duration", new(duration), args{[]byte(`"15m30s"`)}, durPtr(15*time.Minute + 30*time.Second), false}, + {"nil", nil, args{nil}, nil, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := tt.d.UnmarshalJSON(tt.args.data); (err != nil) != tt.wantErr { + t.Errorf("multiString.UnmarshalJSON() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(tt.d, tt.want) { + t.Errorf("multiString.UnmarshalJSON() = %v, want %v", tt.d, tt.want) + } + }) + } +} + +func Test_duration_MarshalJSON(t *testing.T) { + tests := []struct { + name string + d *duration + want []byte + wantErr bool + }{ + {"string", durPtr(15*time.Minute + 30*time.Second), []byte(`"15m30s"`), false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := tt.d.MarshalJSON() + if (err != nil) != tt.wantErr { + t.Errorf("duration.MarshalJSON() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("duration.MarshalJSON() = %v, want %v", got, tt.want) + } + }) + } +} From 6dc89f46d8d66c9e80055be1ce909cfe74409a82 Mon Sep 17 00:00:00 2001 From: max furman Date: Sun, 20 Jan 2019 21:33:14 -0800 Subject: [PATCH 3/4] make Duration public --- authority/config.go | 6 +++--- authority/provisioner.go | 18 +++++++++--------- authority/types.go | 12 +++++++----- authority/types_test.go | 34 ++++++++++++++-------------------- 4 files changed, 33 insertions(+), 37 deletions(-) diff --git a/authority/config.go b/authority/config.go index 16a10a77..029d5ebe 100644 --- a/authority/config.go +++ b/authority/config.go @@ -29,9 +29,9 @@ var ( maxTLSDur = 24 * time.Hour defaultTLSDur = 24 * time.Hour globalProvisionerClaims = ProvisionerClaims{ - MinTLSDur: (*duration)(&minTLSDur), - MaxTLSDur: (*duration)(&maxTLSDur), - DefaultTLSDur: (*duration)(&defaultTLSDur), + MinTLSDur: &Duration{5 * time.Minute}, + MaxTLSDur: &Duration{24 * time.Hour}, + DefaultTLSDur: &Duration{24 * time.Hour}, DisableRenewal: &defaultDisableRenewal, } ) diff --git a/authority/provisioner.go b/authority/provisioner.go index e3dc7d1a..6dd1b1ac 100644 --- a/authority/provisioner.go +++ b/authority/provisioner.go @@ -12,9 +12,9 @@ import ( // ProvisionerClaims so that individual provisioners can override global claims. type ProvisionerClaims struct { globalClaims *ProvisionerClaims - MinTLSDur *duration `json:"minTLSCertDuration,omitempty"` - MaxTLSDur *duration `json:"maxTLSCertDuration,omitempty"` - DefaultTLSDur *duration `json:"defaultTLSCertDuration,omitempty"` + MinTLSDur *Duration `json:"minTLSCertDuration,omitempty"` + MaxTLSDur *Duration `json:"maxTLSCertDuration,omitempty"` + DefaultTLSDur *Duration `json:"defaultTLSCertDuration,omitempty"` DisableRenewal *bool `json:"disableRenewal,omitempty"` } @@ -32,30 +32,30 @@ func (pc *ProvisionerClaims) Init(global *ProvisionerClaims) (*ProvisionerClaims // provisioner. If the default is not set within the provisioner, then the global // default from the authority configuration will be used. func (pc *ProvisionerClaims) DefaultTLSCertDuration() time.Duration { - if pc.DefaultTLSDur == nil || *pc.DefaultTLSDur == 0 { + if pc.DefaultTLSDur == nil || pc.DefaultTLSDur.Duration == 0 { return pc.globalClaims.DefaultTLSCertDuration() } - return time.Duration(*pc.DefaultTLSDur) + return pc.DefaultTLSDur.Duration } // MinTLSCertDuration returns the minimum TLS cert duration for the provisioner. // If the minimum is not set within the provisioner, then the global // minimum from the authority configuration will be used. func (pc *ProvisionerClaims) MinTLSCertDuration() time.Duration { - if pc.MinTLSDur == nil || *pc.MinTLSDur == 0 { + if pc.MinTLSDur == nil || pc.MinTLSDur.Duration == 0 { return pc.globalClaims.MinTLSCertDuration() } - return time.Duration(*pc.MinTLSDur) + return pc.MinTLSDur.Duration } // MaxTLSCertDuration returns the maximum TLS cert duration for the provisioner. // If the maximum is not set within the provisioner, then the global // maximum from the authority configuration will be used. func (pc *ProvisionerClaims) MaxTLSCertDuration() time.Duration { - if pc.MaxTLSDur == nil || *pc.MaxTLSDur == 0 { + if pc.MaxTLSDur == nil || pc.MaxTLSDur.Duration == 0 { return pc.globalClaims.MaxTLSCertDuration() } - return time.Duration(*pc.MaxTLSDur) + return pc.MaxTLSDur.Duration } // IsDisableRenewal returns if the renewal flow is disabled for the diff --git a/authority/types.go b/authority/types.go index a0c7661a..f0a781d5 100644 --- a/authority/types.go +++ b/authority/types.go @@ -8,15 +8,17 @@ import ( ) // Duration is a wrapper around Time.Duration to aid with marshal/unmarshal. -type duration time.Duration +type Duration struct { + time.Duration +} // MarshalJSON parses a duration string and sets it to the duration. // // A duration string is a possibly signed sequence of decimal numbers, each with // optional fraction and a unit suffix, such as "300ms", "-1.5h" or "2h45m". // Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". -func (d *duration) MarshalJSON() ([]byte, error) { - return json.Marshal((*time.Duration)(d).String()) +func (d *Duration) MarshalJSON() ([]byte, error) { + return json.Marshal(d.Duration.String()) } // UnmarshalJSON parses a duration string and sets it to the duration. @@ -24,7 +26,7 @@ func (d *duration) MarshalJSON() ([]byte, error) { // A duration string is a possibly signed sequence of decimal numbers, each with // optional fraction and a unit suffix, such as "300ms", "-1.5h" or "2h45m". // Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". -func (d *duration) UnmarshalJSON(data []byte) (err error) { +func (d *Duration) UnmarshalJSON(data []byte) (err error) { var ( s string _d time.Duration @@ -38,7 +40,7 @@ func (d *duration) UnmarshalJSON(data []byte) (err error) { if _d, err = time.ParseDuration(s); err != nil { return errors.Wrapf(err, "error parsing %s as duration", s) } - *d = duration(_d) + d.Duration = _d return } diff --git a/authority/types_test.go b/authority/types_test.go index 6050409c..c49c368f 100644 --- a/authority/types_test.go +++ b/authority/types_test.go @@ -102,38 +102,32 @@ func Test_multiString_UnmarshalJSON(t *testing.T) { } } -func durPtr(_d time.Duration) *duration { - d := new(duration) - *d = duration(_d) - return d -} - -func Test_duration_UnmarshalJSON(t *testing.T) { +func TestDuration_UnmarshalJSON(t *testing.T) { type args struct { data []byte } tests := []struct { name string - d *duration + d *Duration args args - want *duration + want *Duration wantErr bool }{ - {"empty", new(duration), args{[]byte{}}, new(duration), true}, - {"bad type", new(duration), args{[]byte(`15`)}, new(duration), true}, - {"empty string", new(duration), args{[]byte(`""`)}, new(duration), true}, - {"non duration", new(duration), args{[]byte(`"15"`)}, new(duration), true}, - {"duration", new(duration), args{[]byte(`"15m30s"`)}, durPtr(15*time.Minute + 30*time.Second), false}, + {"empty", new(Duration), args{[]byte{}}, new(Duration), true}, + {"bad type", new(Duration), args{[]byte(`15`)}, new(Duration), true}, + {"empty string", new(Duration), args{[]byte(`""`)}, new(Duration), true}, + {"non duration", new(Duration), args{[]byte(`"15"`)}, new(Duration), true}, + {"duration", new(Duration), args{[]byte(`"15m30s"`)}, &Duration{15*time.Minute + 30*time.Second}, false}, {"nil", nil, args{nil}, nil, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { if err := tt.d.UnmarshalJSON(tt.args.data); (err != nil) != tt.wantErr { - t.Errorf("multiString.UnmarshalJSON() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("Duration.UnmarshalJSON() error = %v, wantErr %v", err, tt.wantErr) return } if !reflect.DeepEqual(tt.d, tt.want) { - t.Errorf("multiString.UnmarshalJSON() = %v, want %v", tt.d, tt.want) + t.Errorf("Duration.UnmarshalJSON() = %v, want %v", tt.d, tt.want) } }) } @@ -142,21 +136,21 @@ func Test_duration_UnmarshalJSON(t *testing.T) { func Test_duration_MarshalJSON(t *testing.T) { tests := []struct { name string - d *duration + d *Duration want []byte wantErr bool }{ - {"string", durPtr(15*time.Minute + 30*time.Second), []byte(`"15m30s"`), false}, + {"string", &Duration{15*time.Minute + 30*time.Second}, []byte(`"15m30s"`), false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got, err := tt.d.MarshalJSON() if (err != nil) != tt.wantErr { - t.Errorf("duration.MarshalJSON() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("Duration.MarshalJSON() error = %v, wantErr %v", err, tt.wantErr) return } if !reflect.DeepEqual(got, tt.want) { - t.Errorf("duration.MarshalJSON() = %v, want %v", got, tt.want) + t.Errorf("Duration.MarshalJSON() = %v, want %v", got, tt.want) } }) } From 2c72ada610dd78d62deec0bf4cd1f3ad7864a57f Mon Sep 17 00:00:00 2001 From: max furman Date: Sun, 20 Jan 2019 21:37:12 -0800 Subject: [PATCH 4/4] remove dead code --- authority/config.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/authority/config.go b/authority/config.go index 029d5ebe..3bc8e810 100644 --- a/authority/config.go +++ b/authority/config.go @@ -25,9 +25,6 @@ var ( Renegotiation: false, } defaultDisableRenewal = false - minTLSDur = 5 * time.Minute - maxTLSDur = 24 * time.Hour - defaultTLSDur = 24 * time.Hour globalProvisionerClaims = ProvisionerClaims{ MinTLSDur: &Duration{5 * time.Minute}, MaxTLSDur: &Duration{24 * time.Hour},