Skip to content

Commit 32eaeca

Browse files
heymarcelMarcos Yacob
andauthored
Remove deprecated -ttl flag from spire server cli (spiffe#5483)
* Remove deprecated -ttl flag from spire server cli This commit removes the deprecated `-ttl` flag from `spire entry create` and `spire entry update`. Docs are also updated. See discussion in spiffe#5254 Signed-off-by: Marcel Levy <[email protected]> * Remove -ttl from integration tests Signed-off-by: Marcel Levy <[email protected]> * Fix windows unit test Signed-off-by: Marcel Levy <[email protected]> --------- Signed-off-by: Marcel Levy <[email protected]> Co-authored-by: Marcos Yacob <[email protected]>
1 parent 92143cb commit 32eaeca

29 files changed

+66
-331
lines changed

cmd/spire-server/cli/entry/create.go

Lines changed: 2 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,6 @@ type createCommand struct {
4545
// Entry hint, used to disambiguate entries with the same SPIFFE ID
4646
hint string
4747

48-
// TTL for x509 and JWT SVIDs issued to this workload, unless type specific TTLs are set.
49-
// This field is deprecated in favor of the x509SVIDTTL and jwtSVIDTTL fields and will be
50-
// removed in a future release.
51-
ttl int
52-
5348
// TTL for x509 SVIDs issued to this workload
5449
x509SVIDTTL int
5550

@@ -94,9 +89,8 @@ func (c *createCommand) AppendFlags(f *flag.FlagSet) {
9489
f.StringVar(&c.entryID, "entryID", "", "A custom ID for this registration entry (optional). If not set, a new entry ID will be generated")
9590
f.StringVar(&c.parentID, "parentID", "", "The SPIFFE ID of this record's parent")
9691
f.StringVar(&c.spiffeID, "spiffeID", "", "The SPIFFE ID that this record represents")
97-
f.IntVar(&c.ttl, "ttl", 0, "The lifetime, in seconds, for SVIDs issued based on this registration entry. This flag is deprecated in favor of x509SVIDTTL and jwtSVIDTTL and will be removed in a future version")
98-
f.IntVar(&c.x509SVIDTTL, "x509SVIDTTL", 0, "The lifetime, in seconds, for x509-SVIDs issued based on this registration entry. Overrides ttl flag")
99-
f.IntVar(&c.jwtSVIDTTL, "jwtSVIDTTL", 0, "The lifetime, in seconds, for JWT-SVIDs issued based on this registration entry. Overrides ttl flag")
92+
f.IntVar(&c.x509SVIDTTL, "x509SVIDTTL", 0, "The lifetime, in seconds, for x509-SVIDs issued based on this registration entry.")
93+
f.IntVar(&c.jwtSVIDTTL, "jwtSVIDTTL", 0, "The lifetime, in seconds, for JWT-SVIDs issued based on this registration entry.")
10094
f.StringVar(&c.path, "data", "", "Path to a file containing registration JSON (optional). If set to '-', read the JSON from stdin.")
10195
f.Var(&c.selectors, "selector", "A colon-delimited type:value selector. Can be used more than once")
10296
f.Var(&c.federatesWith, "federatesWith", "SPIFFE ID of a trust domain to federate with. Can be used more than once")
@@ -158,10 +152,6 @@ func (c *createCommand) validate() (err error) {
158152
return errors.New("a SPIFFE ID is required")
159153
}
160154

161-
if c.ttl < 0 {
162-
return errors.New("a positive TTL is required")
163-
}
164-
165155
if c.x509SVIDTTL < 0 {
166156
return errors.New("a positive x509-SVID TTL is required")
167157
}
@@ -170,10 +160,6 @@ func (c *createCommand) validate() (err error) {
170160
return errors.New("a positive JWT-SVID TTL is required")
171161
}
172162

173-
if c.ttl > 0 && (c.x509SVIDTTL > 0 || c.jwtSVIDTTL > 0) {
174-
return errors.New("use x509SVIDTTL and jwtSVIDTTL flags or the deprecated ttl flag")
175-
}
176-
177163
return nil
178164
}
179165

@@ -202,18 +188,6 @@ func (c *createCommand) parseConfig() ([]*types.Entry, error) {
202188
Hint: c.hint,
203189
}
204190

205-
// c.ttl is deprecated but usable if the new c.x509Svid field is not used.
206-
// c.ttl should not be used to set the jwtSVIDTTL value because the previous
207-
// behavior was to have a hard-coded 5 minute JWT TTL no matter what the value
208-
// of ttl was set to.
209-
// validate(...) ensures that either the new fields or the deprecated field is
210-
// used, but never a mixture.
211-
//
212-
// https://github.com/spiffe/spire/issues/2700
213-
if e.X509SvidTtl == 0 {
214-
e.X509SvidTtl = int32(c.ttl)
215-
}
216-
217191
selectors := []*types.Selector{}
218192
for _, s := range c.selectors {
219193
cs, err := util.ParseSelector(s)

cmd/spire-server/cli/entry/create_test.go

Lines changed: 11 additions & 127 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func TestCreate(t *testing.T) {
5454
},
5555
}
5656

57-
fakeRespOKFromCmd2 := &entryv1.BatchCreateEntryResponse{
57+
fakeRespOKFromCmdWithoutJwtTtl := &entryv1.BatchCreateEntryResponse{
5858
Results: []*entryv1.BatchCreateEntryResponse_Result{
5959
{
6060
Entry: &types.Entry{
@@ -186,28 +186,16 @@ func TestCreate(t *testing.T) {
186186
expErrJSON: "Error: selector \"unix\" must be formatted as type:value\n",
187187
},
188188
{
189-
name: "Negative TTL",
190-
args: []string{"-selector", "unix", "-parentID", "spiffe://example.org/parent", "-spiffeID", "spiffe://example.org/workload", "-ttl", "-10"},
191-
expErrPretty: "Error: a positive TTL is required\n",
192-
expErrJSON: "Error: a positive TTL is required\n",
189+
name: "Negative X509SvidTtl",
190+
args: []string{"-selector", "unix", "-parentID", "spiffe://example.org/parent", "-spiffeID", "spiffe://example.org/workload", "-x509SVIDTTL", "-10"},
191+
expErrPretty: "Error: a positive x509-SVID TTL is required\n",
192+
expErrJSON: "Error: a positive x509-SVID TTL is required\n",
193193
},
194194
{
195-
name: "Invalid TTL and X509SvidTtl",
196-
args: []string{"-selector", "unix", "-parentID", "spiffe://example.org/parent", "-spiffeID", "spiffe://example.org/workload", "-ttl", "10", "-x509SVIDTTL", "20"},
197-
expErrPretty: "Error: use x509SVIDTTL and jwtSVIDTTL flags or the deprecated ttl flag\n",
198-
expErrJSON: "Error: use x509SVIDTTL and jwtSVIDTTL flags or the deprecated ttl flag\n",
199-
},
200-
{
201-
name: "Invalid TTL and JwtSvidTtl",
202-
args: []string{"-selector", "unix", "-parentID", "spiffe://example.org/parent", "-spiffeID", "spiffe://example.org/workload", "-ttl", "10", "-jwtSVIDTTL", "20"},
203-
expErrPretty: "Error: use x509SVIDTTL and jwtSVIDTTL flags or the deprecated ttl flag\n",
204-
expErrJSON: "Error: use x509SVIDTTL and jwtSVIDTTL flags or the deprecated ttl flag\n",
205-
},
206-
{
207-
name: "Invalid TTL and both X509SvidTtl and JwtSvidTtl",
208-
args: []string{"-selector", "unix", "-parentID", "spiffe://example.org/parent", "-spiffeID", "spiffe://example.org/workload", "-ttl", "10", "-x509SVIDTTL", "20", "-jwtSVIDTTL", "30"},
209-
expErrPretty: "Error: use x509SVIDTTL and jwtSVIDTTL flags or the deprecated ttl flag\n",
210-
expErrJSON: "Error: use x509SVIDTTL and jwtSVIDTTL flags or the deprecated ttl flag\n",
195+
name: "Negative jwtSVIDTTL",
196+
args: []string{"-selector", "unix", "-parentID", "spiffe://example.org/parent", "-spiffeID", "spiffe://example.org/workload", "-jwtSVIDTTL", "-10"},
197+
expErrPretty: "Error: a positive JWT-SVID TTL is required\n",
198+
expErrJSON: "Error: a positive JWT-SVID TTL is required\n",
211199
},
212200
{
213201
name: "Federated node entries",
@@ -346,7 +334,7 @@ StoreSvid : true
346334
"-parentID", "spiffe://example.org/parent",
347335
"-selector", "zebra:zebra:2000",
348336
"-selector", "alpha:alpha:2000",
349-
"-ttl", "60",
337+
"-x509SVIDTTL", "60",
350338
"-federatesWith", "spiffe://domaina.test",
351339
"-federatesWith", "spiffe://domainb.test",
352340
"-admin",
@@ -376,111 +364,7 @@ StoreSvid : true
376364
},
377365
},
378366
},
379-
fakeResp: fakeRespOKFromCmd2,
380-
expOutPretty: fmt.Sprintf(`Entry ID : entry-id
381-
SPIFFE ID : spiffe://example.org/workload
382-
Parent ID : spiffe://example.org/parent
383-
Revision : 0
384-
Downstream : true
385-
X509-SVID TTL : 60
386-
JWT-SVID TTL : default
387-
Expiration time : %s
388-
Selector : zebra:zebra:2000
389-
Selector : alpha:alpha:2000
390-
FederatesWith : spiffe://domaina.test
391-
FederatesWith : spiffe://domainb.test
392-
DNS name : unu1000
393-
DNS name : ung1000
394-
Admin : true
395-
StoreSvid : true
396-
397-
`, time.Unix(1552410266, 0).UTC()),
398-
expOutJSON: `{
399-
"results": [
400-
{
401-
"status": {
402-
"code": 0,
403-
"message": "OK"
404-
},
405-
"entry": {
406-
"id": "entry-id",
407-
"spiffe_id": {
408-
"trust_domain": "example.org",
409-
"path": "/workload"
410-
},
411-
"parent_id": {
412-
"trust_domain": "example.org",
413-
"path": "/parent"
414-
},
415-
"selectors": [
416-
{
417-
"type": "zebra",
418-
"value": "zebra:2000"
419-
},
420-
{
421-
"type": "alpha",
422-
"value": "alpha:2000"
423-
}
424-
],
425-
"x509_svid_ttl": 60,
426-
"federates_with": [
427-
"spiffe://domaina.test",
428-
"spiffe://domainb.test"
429-
],
430-
"hint": "",
431-
"admin": true,
432-
"created_at": "1547583197",
433-
"downstream": true,
434-
"expires_at": "1552410266",
435-
"dns_names": [
436-
"unu1000",
437-
"ung1000"
438-
],
439-
"revision_number": "0",
440-
"store_svid": true,
441-
"jwt_svid_ttl": 0
442-
}
443-
}
444-
]
445-
}`,
446-
},
447-
{
448-
name: "Create succeeds using deprecated command line arguments",
449-
args: []string{
450-
"-spiffeID", "spiffe://example.org/workload",
451-
"-parentID", "spiffe://example.org/parent",
452-
"-selector", "zebra:zebra:2000",
453-
"-selector", "alpha:alpha:2000",
454-
"-ttl", "60",
455-
"-federatesWith", "spiffe://domaina.test",
456-
"-federatesWith", "spiffe://domainb.test",
457-
"-admin",
458-
"-entryExpiry", "1552410266",
459-
"-dns", "unu1000",
460-
"-dns", "ung1000",
461-
"-downstream",
462-
"-storeSVID",
463-
},
464-
expReq: &entryv1.BatchCreateEntryRequest{
465-
Entries: []*types.Entry{
466-
{
467-
SpiffeId: &types.SPIFFEID{TrustDomain: "example.org", Path: "/workload"},
468-
ParentId: &types.SPIFFEID{TrustDomain: "example.org", Path: "/parent"},
469-
Selectors: []*types.Selector{
470-
{Type: "zebra", Value: "zebra:2000"},
471-
{Type: "alpha", Value: "alpha:2000"},
472-
},
473-
X509SvidTtl: 60,
474-
FederatesWith: []string{"spiffe://domaina.test", "spiffe://domainb.test"},
475-
Admin: true,
476-
ExpiresAt: 1552410266,
477-
DnsNames: []string{"unu1000", "ung1000"},
478-
Downstream: true,
479-
StoreSvid: true,
480-
},
481-
},
482-
},
483-
fakeResp: fakeRespOKFromCmd2,
367+
fakeResp: fakeRespOKFromCmdWithoutJwtTtl,
484368
expOutPretty: fmt.Sprintf(`Entry ID : entry-id
485369
SPIFFE ID : spiffe://example.org/workload
486370
Parent ID : spiffe://example.org/parent

cmd/spire-server/cli/entry/update.go

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,6 @@ type updateCommand struct {
4444
// Whether or not the entry is for a downstream SPIRE server
4545
downstream bool
4646

47-
// TTL for certificates issued to this workload
48-
ttl int
49-
5047
// TTL for x509 SVIDs issued to this workload
5148
x509SvidTTL int
5249

@@ -88,9 +85,8 @@ func (c *updateCommand) AppendFlags(f *flag.FlagSet) {
8885
f.StringVar(&c.entryID, "entryID", "", "The Registration Entry ID of the record to update")
8986
f.StringVar(&c.parentID, "parentID", "", "The SPIFFE ID of this record's parent")
9087
f.StringVar(&c.spiffeID, "spiffeID", "", "The SPIFFE ID that this record represents")
91-
f.IntVar(&c.ttl, "ttl", 0, "The lifetime, in seconds, for SVIDs issued based on this registration entry. This flag is deprecated in favor of x509SVIDTTL and jwtSVIDTTL and will be removed in a future version")
92-
f.IntVar(&c.x509SvidTTL, "x509SVIDTTL", 0, "The lifetime, in seconds, for x509-SVIDs issued based on this registration entry. Overrides ttl flag")
93-
f.IntVar(&c.jwtSvidTTL, "jwtSVIDTTL", 0, "The lifetime, in seconds, for JWT-SVIDs issued based on this registration entry. Overrides ttl flag")
88+
f.IntVar(&c.x509SvidTTL, "x509SVIDTTL", 0, "The lifetime, in seconds, for x509-SVIDs issued based on this registration entry.")
89+
f.IntVar(&c.jwtSvidTTL, "jwtSVIDTTL", 0, "The lifetime, in seconds, for JWT-SVIDs issued based on this registration entry.")
9490
f.StringVar(&c.path, "data", "", "Path to a file containing registration JSON (optional). If set to '-', read the JSON from stdin.")
9591
f.Var(&c.selectors, "selector", "A colon-delimited type:value selector. Can be used more than once")
9692
f.Var(&c.federatesWith, "federatesWith", "SPIFFE ID of a trust domain to federate with. Can be used more than once")
@@ -151,10 +147,6 @@ func (c *updateCommand) validate() (err error) {
151147
return errors.New("a SPIFFE ID is required")
152148
}
153149

154-
if c.ttl < 0 {
155-
return errors.New("a positive TTL is required")
156-
}
157-
158150
if c.x509SvidTTL < 0 {
159151
return errors.New("a positive x509-SVID TTL is required")
160152
}
@@ -163,10 +155,6 @@ func (c *updateCommand) validate() (err error) {
163155
return errors.New("a positive JWT-SVID TTL is required")
164156
}
165157

166-
if c.ttl > 0 && (c.x509SvidTTL > 0 || c.jwtSvidTTL > 0) {
167-
return errors.New("use x509SVIDTTL and jwtSVIDTTL flags or the deprecated ttl flag")
168-
}
169-
170158
return nil
171159
}
172160

@@ -193,18 +181,6 @@ func (c *updateCommand) parseConfig() ([]*types.Entry, error) {
193181
Hint: c.hint,
194182
}
195183

196-
// c.ttl is deprecated but usable if the new c.x509Svid field is not used.
197-
// c.ttl should not be used to set the jwtSVIDTTL value because the previous
198-
// behavior was to have a hard-coded 5 minute JWT TTL no matter what the value
199-
// of ttl was set to.
200-
// validate(...) ensures that either the new fields or the deprecated field is
201-
// used, but never a mixture.
202-
//
203-
// https://github.com/spiffe/spire/issues/2700
204-
if e.X509SvidTtl == 0 {
205-
e.X509SvidTtl = int32(c.ttl)
206-
}
207-
208184
selectors := []*types.Selector{}
209185
for _, s := range c.selectors {
210186
cs, err := util.ParseSelector(s)

0 commit comments

Comments
 (0)