Skip to content

Commit df07972

Browse files
committed
Use native time.Time type for timestamp flags
Replace opaque string types with proper time.Time for the recreate-vms-created-before and vms-created-before flags. This provides type safety, validation at parse time, and clearer error messages for invalid RFC 3339 timestamps. - Add TimeArg type with UnmarshalFlag for parsing RFC 3339 timestamps - Update DeployOpts and RecreateOpts to use TimeArg - Update director UpdateOpts and RecreateOpts to use time.Time - Add comprehensive tests for TimeArg parsing and formatting
1 parent 67f5095 commit df07972

File tree

10 files changed

+126
-25
lines changed

10 files changed

+126
-25
lines changed

cmd/deploy.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ func (c DeployCmd) Run(opts DeployOpts) error {
104104
updateOpts := boshdir.UpdateOpts{
105105
RecreatePersistentDisks: opts.RecreatePersistentDisks,
106106
Recreate: opts.Recreate,
107-
RecreateVMsCreatedBefore: opts.RecreateVMsCreatedBefore,
107+
RecreateVMsCreatedBefore: opts.RecreateVMsCreatedBefore.Time,
108108
Fix: opts.Fix,
109109
SkipDrain: opts.SkipDrain,
110110
DryRun: opts.DryRun,

cmd/deploy_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package cmd_test
22

33
import (
44
"errors"
5+
"time"
56

67
"github.com/cppforlife/go-patch/patch"
78
. "github.com/onsi/ginkgo/v2"
@@ -89,7 +90,7 @@ var _ = Describe("DeployCmd", func() {
8990

9091
It("deploys manifest allowing to recreate VMs created before a timestamp", func() {
9192
deployOpts.Recreate = true
92-
deployOpts.RecreateVMsCreatedBefore = "2026-01-01T00:00:00Z"
93+
deployOpts.RecreateVMsCreatedBefore = opts.TimeArg{Time: time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)}
9394

9495
err := act()
9596
Expect(err).ToNot(HaveOccurred())
@@ -100,7 +101,7 @@ var _ = Describe("DeployCmd", func() {
100101
Expect(bytes).To(Equal([]byte("name: dep\n")))
101102
Expect(updateOpts).To(Equal(boshdir.UpdateOpts{
102103
Recreate: true,
103-
RecreateVMsCreatedBefore: "2026-01-01T00:00:00Z",
104+
RecreateVMsCreatedBefore: time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC),
104105
}))
105106
})
106107

cmd/opts/opts.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,7 @@ type DeployOpts struct {
505505

506506
Recreate bool `long:"recreate" description:"Recreate all VMs in deployment"`
507507
RecreatePersistentDisks bool `long:"recreate-persistent-disks" description:"Recreate all persistent disks in deployment"`
508-
RecreateVMsCreatedBefore string `long:"recreate-vms-created-before" description:"Only recreate VMs created before the given RFC 3339 timestamp (requires --recreate)"`
508+
RecreateVMsCreatedBefore TimeArg `long:"recreate-vms-created-before" description:"Only recreate VMs created before the given RFC 3339 timestamp (requires --recreate)"`
509509
Fix bool `long:"fix" description:"Recreate an instance with an unresponsive agent instead of erroring"`
510510
FixReleases bool `long:"fix-releases" description:"Reupload releases in manifest and replace corrupt or missing jobs/packages"`
511511
SkipDrain []boshdir.SkipDrain `long:"skip-drain" value-name:"[INSTANCE-GROUP[/INSTANCE-ID]]" description:"Skip running drain and pre-stop scripts for specific instance groups" optional:"true" optional-value:"*"`
@@ -942,9 +942,9 @@ type RestartOpts struct {
942942
type RecreateOpts struct {
943943
Args AllOrInstanceGroupOrInstanceSlugArgs `positional-args:"true"`
944944

945-
SkipDrain bool `long:"skip-drain" description:"Skip running drain and pre-stop scripts"`
946-
Fix bool `long:"fix" description:"Recreate an instance with an unresponsive agent instead of erroring"`
947-
VMsCreatedBefore string `long:"vms-created-before" description:"Only recreate VMs created before the given RFC 3339 timestamp"`
945+
SkipDrain bool `long:"skip-drain" description:"Skip running drain and pre-stop scripts"`
946+
Fix bool `long:"fix" description:"Recreate an instance with an unresponsive agent instead of erroring"`
947+
VMsCreatedBefore TimeArg `long:"vms-created-before" description:"Only recreate VMs created before the given RFC 3339 timestamp"`
948948

949949
Canaries string `long:"canaries" description:"Override manifest values for canaries"`
950950
MaxInFlight string `long:"max-in-flight" description:"Override manifest values for max_in_flight"`

cmd/opts/time_arg.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
package opts
2+
3+
import (
4+
"time"
5+
6+
bosherr "github.com/cloudfoundry/bosh-utils/errors"
7+
)
8+
9+
type TimeArg struct {
10+
time.Time
11+
}
12+
13+
func (a *TimeArg) UnmarshalFlag(data string) error {
14+
t, err := time.Parse(time.RFC3339, data)
15+
if err != nil {
16+
return bosherr.Errorf("Invalid RFC 3339 timestamp '%s': %s", data, err)
17+
}
18+
a.Time = t
19+
return nil
20+
}
21+
22+
func (a TimeArg) IsSet() bool {
23+
return !a.Time.IsZero()
24+
}
25+
26+
func (a TimeArg) AsString() string {
27+
if a.IsSet() {
28+
return a.Time.Format(time.RFC3339)
29+
}
30+
return ""
31+
}

cmd/opts/time_arg_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package opts_test
2+
3+
import (
4+
"time"
5+
6+
. "github.com/onsi/ginkgo/v2"
7+
. "github.com/onsi/gomega"
8+
9+
. "github.com/cloudfoundry/bosh-cli/v7/cmd/opts"
10+
)
11+
12+
var _ = Describe("TimeArg", func() {
13+
Describe("UnmarshalFlag", func() {
14+
It("parses valid RFC 3339 timestamps", func() {
15+
var arg TimeArg
16+
err := arg.UnmarshalFlag("2026-01-01T00:00:00Z")
17+
Expect(err).ToNot(HaveOccurred())
18+
Expect(arg.Time).To(Equal(time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)))
19+
})
20+
21+
It("parses RFC 3339 timestamps with timezone offset", func() {
22+
var arg TimeArg
23+
err := arg.UnmarshalFlag("2026-06-15T14:30:00-07:00")
24+
Expect(err).ToNot(HaveOccurred())
25+
Expect(arg.Time.UTC()).To(Equal(time.Date(2026, 6, 15, 21, 30, 0, 0, time.UTC)))
26+
})
27+
28+
It("returns error for invalid timestamps", func() {
29+
var arg TimeArg
30+
err := arg.UnmarshalFlag("not-a-timestamp")
31+
Expect(err).To(HaveOccurred())
32+
Expect(err.Error()).To(ContainSubstring("Invalid RFC 3339 timestamp"))
33+
})
34+
35+
It("returns error for non-RFC3339 date formats", func() {
36+
var arg TimeArg
37+
err := arg.UnmarshalFlag("2026-01-01")
38+
Expect(err).To(HaveOccurred())
39+
Expect(err.Error()).To(ContainSubstring("Invalid RFC 3339 timestamp"))
40+
})
41+
})
42+
43+
Describe("IsSet", func() {
44+
It("returns false for zero time", func() {
45+
var arg TimeArg
46+
Expect(arg.IsSet()).To(BeFalse())
47+
})
48+
49+
It("returns true for non-zero time", func() {
50+
arg := TimeArg{Time: time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)}
51+
Expect(arg.IsSet()).To(BeTrue())
52+
})
53+
})
54+
55+
Describe("AsString", func() {
56+
It("returns empty string for zero time", func() {
57+
var arg TimeArg
58+
Expect(arg.AsString()).To(Equal(""))
59+
})
60+
61+
It("returns RFC 3339 formatted string for non-zero time", func() {
62+
arg := TimeArg{Time: time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)}
63+
Expect(arg.AsString()).To(Equal("2026-01-01T00:00:00Z"))
64+
})
65+
})
66+
})

cmd/recreate.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func newRecreateOpts(opts RecreateOpts) (boshdir.RecreateOpts, error) {
3939
Canaries: opts.Canaries,
4040
MaxInFlight: opts.MaxInFlight,
4141
Converge: true,
42-
VMsCreatedBefore: opts.VMsCreatedBefore,
42+
VMsCreatedBefore: opts.VMsCreatedBefore.Time,
4343
}
4444
return recreateOpts, nil
4545
}

cmd/recreate_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package cmd_test
22

33
import (
44
"errors"
5+
"time"
56

67
. "github.com/onsi/ginkgo/v2"
78
. "github.com/onsi/gomega"
@@ -116,15 +117,15 @@ var _ = Describe("RecreateCmd", func() {
116117
})
117118

118119
It("can set vms_created_before", func() {
119-
recreateOpts.VMsCreatedBefore = "2026-01-01T00:00:00Z"
120+
recreateOpts.VMsCreatedBefore = opts.TimeArg{Time: time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)}
120121

121122
err := act()
122123
Expect(err).ToNot(HaveOccurred())
123124

124125
Expect(deployment.RecreateCallCount()).To(Equal(1))
125126

126127
_, recreateOpts := deployment.RecreateArgsForCall(0)
127-
Expect(recreateOpts.VMsCreatedBefore).To(Equal("2026-01-01T00:00:00Z"))
128+
Expect(recreateOpts.VMsCreatedBefore).To(Equal(time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)))
128129
})
129130

130131
It("does not recreate if confirmation is rejected", func() {

director/deployment.go

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"net/http"
77
"net/url"
88
"strings"
9+
"time"
910

1011
bosherr "github.com/cloudfoundry/bosh-utils/errors"
1112
)
@@ -138,7 +139,7 @@ func (d DeploymentImpl) Start(slug AllOrInstanceGroupOrInstanceSlug, opts StartO
138139
if !opts.Converge {
139140
return d.nonConvergingJobAction("start", slug, false, false, false)
140141
}
141-
return d.changeJobState("started", slug, false, false, false, false, opts.Canaries, opts.MaxInFlight, "")
142+
return d.changeJobState("started", slug, false, false, false, false, opts.Canaries, opts.MaxInFlight, time.Time{})
142143
}
143144

144145
func (d DeploymentImpl) Stop(slug AllOrInstanceGroupOrInstanceSlug, opts StopOpts) error {
@@ -150,15 +151,15 @@ func (d DeploymentImpl) Stop(slug AllOrInstanceGroupOrInstanceSlug, opts StopOpt
150151
if opts.Hard {
151152
state = "detached"
152153
}
153-
return d.changeJobState(state, slug, opts.SkipDrain, opts.Force, false, false, opts.Canaries, opts.MaxInFlight, "")
154+
return d.changeJobState(state, slug, opts.SkipDrain, opts.Force, false, false, opts.Canaries, opts.MaxInFlight, time.Time{})
154155
}
155156

156157
func (d DeploymentImpl) Restart(slug AllOrInstanceGroupOrInstanceSlug, opts RestartOpts) error {
157158
if !opts.Converge {
158159
return d.nonConvergingJobAction("restart", slug, opts.SkipDrain, false, false)
159160
}
160161

161-
return d.changeJobState("restart", slug, opts.SkipDrain, opts.Force, false, false, opts.Canaries, opts.MaxInFlight, "")
162+
return d.changeJobState("restart", slug, opts.SkipDrain, opts.Force, false, false, opts.Canaries, opts.MaxInFlight, time.Time{})
162163
}
163164

164165
func (d DeploymentImpl) Recreate(slug AllOrInstanceGroupOrInstanceSlug, opts RecreateOpts) error {
@@ -173,7 +174,7 @@ func (d DeploymentImpl) nonConvergingJobAction(action string, slug AllOrInstance
173174
return d.client.NonConvergingJobAction(action, d.name, slug.Name(), slug.IndexOrID(), skipDrain, hard, ignoreUnresponsiveAgent)
174175
}
175176

176-
func (d DeploymentImpl) changeJobState(state string, slug AllOrInstanceGroupOrInstanceSlug, skipDrain bool, force bool, fix bool, dryRun bool, canaries string, maxInFlight string, vmsCreatedBefore string) error {
177+
func (d DeploymentImpl) changeJobState(state string, slug AllOrInstanceGroupOrInstanceSlug, skipDrain bool, force bool, fix bool, dryRun bool, canaries string, maxInFlight string, vmsCreatedBefore time.Time) error {
177178
return d.client.ChangeJobState(
178179
state, d.name, slug.Name(), slug.IndexOrID(), skipDrain, force, fix, dryRun, canaries, maxInFlight, vmsCreatedBefore)
179180
}
@@ -387,7 +388,7 @@ func (c Client) NonConvergingJobAction(action string, deployment string, instanc
387388
return nil
388389
}
389390

390-
func (c Client) ChangeJobState(state, deploymentName, job, indexOrID string, skipDrain bool, force bool, fix bool, dryRun bool, canaries string, maxInFlight string, vmsCreatedBefore string) error {
391+
func (c Client) ChangeJobState(state, deploymentName, job, indexOrID string, skipDrain bool, force bool, fix bool, dryRun bool, canaries string, maxInFlight string, vmsCreatedBefore time.Time) error {
391392
if len(state) == 0 {
392393
return bosherr.Error("Expected non-empty job state")
393394
}
@@ -426,8 +427,8 @@ func (c Client) ChangeJobState(state, deploymentName, job, indexOrID string, ski
426427
query.Add("max_in_flight", maxInFlight)
427428
}
428429

429-
if vmsCreatedBefore != "" {
430-
query.Add("recreate_vm_created_before", vmsCreatedBefore)
430+
if !vmsCreatedBefore.IsZero() {
431+
query.Add("recreate_vm_created_before", vmsCreatedBefore.Format(time.RFC3339))
431432
}
432433

433434
path := fmt.Sprintf("/deployments/%s/jobs", deploymentName)
@@ -529,8 +530,8 @@ func (c Client) UpdateDeployment(manifest []byte, opts UpdateOpts) error {
529530
query.Add("recreate_persistent_disks", "true")
530531
}
531532

532-
if opts.RecreateVMsCreatedBefore != "" {
533-
query.Add("recreate_vm_created_before", opts.RecreateVMsCreatedBefore)
533+
if !opts.RecreateVMsCreatedBefore.IsZero() {
534+
query.Add("recreate_vm_created_before", opts.RecreateVMsCreatedBefore.Format(time.RFC3339))
534535
}
535536

536537
if opts.Fix {

director/deployment_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"net/http"
66
"net/url"
77
"strings"
8+
"time"
89

910
semver "github.com/cppforlife/go-semi-semantic/version"
1011
. "github.com/onsi/ginkgo/v2"
@@ -552,10 +553,10 @@ var _ = Describe("Deployment", func() {
552553
})
553554

554555
It("changes state with vms_created_before filter", func() {
555-
timestamp := "2026-01-01T00:00:00Z"
556+
timestamp := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)
556557
recreateOpts.VMsCreatedBefore = timestamp
557558

558-
query := fmt.Sprintf("state=%s&recreate_vm_created_before=%s", state, url.QueryEscape(timestamp))
559+
query := fmt.Sprintf("state=%s&recreate_vm_created_before=%s", state, url.QueryEscape(timestamp.Format(time.RFC3339)))
559560

560561
ConfigureTaskResult(
561562
ghttp.CombineHandlers(
@@ -989,10 +990,10 @@ var _ = Describe("Deployment", func() {
989990
})
990991

991992
It("succeeds updating deployment with recreate_vm_created_before flag", func() {
992-
timestamp := "2026-01-01T00:00:00Z"
993+
timestamp := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)
993994
ConfigureTaskResult(
994995
ghttp.CombineHandlers(
995-
ghttp.VerifyRequest("POST", "/deployments", "recreate=true&recreate_vm_created_before="+url.QueryEscape(timestamp)),
996+
ghttp.VerifyRequest("POST", "/deployments", "recreate=true&recreate_vm_created_before="+url.QueryEscape(timestamp.Format(time.RFC3339))),
996997
ghttp.VerifyBasicAuth("username", "password"),
997998
ghttp.VerifyHeader(http.Header{
998999
"Content-Type": []string{"text/yaml"},

director/interfaces.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,13 +215,13 @@ type RecreateOpts struct {
215215
SkipDrain bool
216216
DryRun bool
217217
Converge bool
218-
VMsCreatedBefore string
218+
VMsCreatedBefore time.Time
219219
}
220220

221221
type UpdateOpts struct {
222222
Recreate bool
223223
RecreatePersistentDisks bool
224-
RecreateVMsCreatedBefore string
224+
RecreateVMsCreatedBefore time.Time
225225
Fix bool
226226
SkipDrain SkipDrains
227227
Canaries string

0 commit comments

Comments
 (0)