Skip to content

Commit

Permalink
Remove deprecated -ttl flag from spire server cli (#5483)
Browse files Browse the repository at this point in the history 
* 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 #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]>
  • Loading branch information
@heymarcel @MarcosDY
heymarcel and MarcosDY authored Sep 12, 2024
1 parent 92143cb commit 32eaeca
Show file tree
Hide file tree
Showing 29 changed files with 66 additions and 331 deletions.
30 changes: 2 additions & 28 deletions cmd/spire-server/cli/entry/create.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,6 @@ type createCommand struct {
// Entry hint, used to disambiguate entries with the same SPIFFE ID
hint string

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

// TTL for x509 SVIDs issued to this workload
x509SVIDTTL int

Expand Down Expand Up @@ -94,9 +89,8 @@ func (c *createCommand) AppendFlags(f *flag.FlagSet) {
f.StringVar(&c.entryID, "entryID", "", "A custom ID for this registration entry (optional). If not set, a new entry ID will be generated")
f.StringVar(&c.parentID, "parentID", "", "The SPIFFE ID of this record's parent")
f.StringVar(&c.spiffeID, "spiffeID", "", "The SPIFFE ID that this record represents")
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")
f.IntVar(&c.x509SVIDTTL, "x509SVIDTTL", 0, "The lifetime, in seconds, for x509-SVIDs issued based on this registration entry. Overrides ttl flag")
f.IntVar(&c.jwtSVIDTTL, "jwtSVIDTTL", 0, "The lifetime, in seconds, for JWT-SVIDs issued based on this registration entry. Overrides ttl flag")
f.IntVar(&c.x509SVIDTTL, "x509SVIDTTL", 0, "The lifetime, in seconds, for x509-SVIDs issued based on this registration entry.")
f.IntVar(&c.jwtSVIDTTL, "jwtSVIDTTL", 0, "The lifetime, in seconds, for JWT-SVIDs issued based on this registration entry.")
f.StringVar(&c.path, "data", "", "Path to a file containing registration JSON (optional). If set to '-', read the JSON from stdin.")
f.Var(&c.selectors, "selector", "A colon-delimited type:value selector. Can be used more than once")
f.Var(&c.federatesWith, "federatesWith", "SPIFFE ID of a trust domain to federate with. Can be used more than once")
Expand Down Expand Up @@ -158,10 +152,6 @@ func (c *createCommand) validate() (err error) {
return errors.New("a SPIFFE ID is required")
}

if c.ttl < 0 {
return errors.New("a positive TTL is required")
}

if c.x509SVIDTTL < 0 {
return errors.New("a positive x509-SVID TTL is required")
}
Expand All @@ -170,10 +160,6 @@ func (c *createCommand) validate() (err error) {
return errors.New("a positive JWT-SVID TTL is required")
}

if c.ttl > 0 && (c.x509SVIDTTL > 0 || c.jwtSVIDTTL > 0) {
return errors.New("use x509SVIDTTL and jwtSVIDTTL flags or the deprecated ttl flag")
}

return nil
}

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

// c.ttl is deprecated but usable if the new c.x509Svid field is not used.
// c.ttl should not be used to set the jwtSVIDTTL value because the previous
// behavior was to have a hard-coded 5 minute JWT TTL no matter what the value
// of ttl was set to.
// validate(...) ensures that either the new fields or the deprecated field is
// used, but never a mixture.
//
// https://github.com/spiffe/spire/issues/2700
if e.X509SvidTtl == 0 {
e.X509SvidTtl = int32(c.ttl)
}

selectors := []*types.Selector{}
for _, s := range c.selectors {
cs, err := util.ParseSelector(s)
Expand Down
138 changes: 11 additions & 127 deletions cmd/spire-server/cli/entry/create_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func TestCreate(t *testing.T) {
},
}

fakeRespOKFromCmd2 := &entryv1.BatchCreateEntryResponse{
fakeRespOKFromCmdWithoutJwtTtl := &entryv1.BatchCreateEntryResponse{
Results: []*entryv1.BatchCreateEntryResponse_Result{
{
Entry: &types.Entry{
Expand Down Expand Up @@ -186,28 +186,16 @@ func TestCreate(t *testing.T) {
expErrJSON: "Error: selector \"unix\" must be formatted as type:value\n",
},
{
name: "Negative TTL",
args: []string{"-selector", "unix", "-parentID", "spiffe://example.org/parent", "-spiffeID", "spiffe://example.org/workload", "-ttl", "-10"},
expErrPretty: "Error: a positive TTL is required\n",
expErrJSON: "Error: a positive TTL is required\n",
name: "Negative X509SvidTtl",
args: []string{"-selector", "unix", "-parentID", "spiffe://example.org/parent", "-spiffeID", "spiffe://example.org/workload", "-x509SVIDTTL", "-10"},
expErrPretty: "Error: a positive x509-SVID TTL is required\n",
expErrJSON: "Error: a positive x509-SVID TTL is required\n",
},
{
name: "Invalid TTL and X509SvidTtl",
args: []string{"-selector", "unix", "-parentID", "spiffe://example.org/parent", "-spiffeID", "spiffe://example.org/workload", "-ttl", "10", "-x509SVIDTTL", "20"},
expErrPretty: "Error: use x509SVIDTTL and jwtSVIDTTL flags or the deprecated ttl flag\n",
expErrJSON: "Error: use x509SVIDTTL and jwtSVIDTTL flags or the deprecated ttl flag\n",
},
{
name: "Invalid TTL and JwtSvidTtl",
args: []string{"-selector", "unix", "-parentID", "spiffe://example.org/parent", "-spiffeID", "spiffe://example.org/workload", "-ttl", "10", "-jwtSVIDTTL", "20"},
expErrPretty: "Error: use x509SVIDTTL and jwtSVIDTTL flags or the deprecated ttl flag\n",
expErrJSON: "Error: use x509SVIDTTL and jwtSVIDTTL flags or the deprecated ttl flag\n",
},
{
name: "Invalid TTL and both X509SvidTtl and JwtSvidTtl",
args: []string{"-selector", "unix", "-parentID", "spiffe://example.org/parent", "-spiffeID", "spiffe://example.org/workload", "-ttl", "10", "-x509SVIDTTL", "20", "-jwtSVIDTTL", "30"},
expErrPretty: "Error: use x509SVIDTTL and jwtSVIDTTL flags or the deprecated ttl flag\n",
expErrJSON: "Error: use x509SVIDTTL and jwtSVIDTTL flags or the deprecated ttl flag\n",
name: "Negative jwtSVIDTTL",
args: []string{"-selector", "unix", "-parentID", "spiffe://example.org/parent", "-spiffeID", "spiffe://example.org/workload", "-jwtSVIDTTL", "-10"},
expErrPretty: "Error: a positive JWT-SVID TTL is required\n",
expErrJSON: "Error: a positive JWT-SVID TTL is required\n",
},
{
name: "Federated node entries",
Expand Down Expand Up @@ -346,7 +334,7 @@ StoreSvid : true
"-parentID", "spiffe://example.org/parent",
"-selector", "zebra:zebra:2000",
"-selector", "alpha:alpha:2000",
"-ttl", "60",
"-x509SVIDTTL", "60",
"-federatesWith", "spiffe://domaina.test",
"-federatesWith", "spiffe://domainb.test",
"-admin",
Expand Down Expand Up @@ -376,111 +364,7 @@ StoreSvid : true
},
},
},
fakeResp: fakeRespOKFromCmd2,
expOutPretty: fmt.Sprintf(`Entry ID : entry-id
SPIFFE ID : spiffe://example.org/workload
Parent ID : spiffe://example.org/parent
Revision : 0
Downstream : true
X509-SVID TTL : 60
JWT-SVID TTL : default
Expiration time : %s
Selector : zebra:zebra:2000
Selector : alpha:alpha:2000
FederatesWith : spiffe://domaina.test
FederatesWith : spiffe://domainb.test
DNS name : unu1000
DNS name : ung1000
Admin : true
StoreSvid : true
`, time.Unix(1552410266, 0).UTC()),
expOutJSON: `{
"results": [
{
"status": {
"code": 0,
"message": "OK"
},
"entry": {
"id": "entry-id",
"spiffe_id": {
"trust_domain": "example.org",
"path": "/workload"
},
"parent_id": {
"trust_domain": "example.org",
"path": "/parent"
},
"selectors": [
{
"type": "zebra",
"value": "zebra:2000"
},
{
"type": "alpha",
"value": "alpha:2000"
}
],
"x509_svid_ttl": 60,
"federates_with": [
"spiffe://domaina.test",
"spiffe://domainb.test"
],
"hint": "",
"admin": true,
"created_at": "1547583197",
"downstream": true,
"expires_at": "1552410266",
"dns_names": [
"unu1000",
"ung1000"
],
"revision_number": "0",
"store_svid": true,
"jwt_svid_ttl": 0
}
}
]
}`,
},
{
name: "Create succeeds using deprecated command line arguments",
args: []string{
"-spiffeID", "spiffe://example.org/workload",
"-parentID", "spiffe://example.org/parent",
"-selector", "zebra:zebra:2000",
"-selector", "alpha:alpha:2000",
"-ttl", "60",
"-federatesWith", "spiffe://domaina.test",
"-federatesWith", "spiffe://domainb.test",
"-admin",
"-entryExpiry", "1552410266",
"-dns", "unu1000",
"-dns", "ung1000",
"-downstream",
"-storeSVID",
},
expReq: &entryv1.BatchCreateEntryRequest{
Entries: []*types.Entry{
{
SpiffeId: &types.SPIFFEID{TrustDomain: "example.org", Path: "/workload"},
ParentId: &types.SPIFFEID{TrustDomain: "example.org", Path: "/parent"},
Selectors: []*types.Selector{
{Type: "zebra", Value: "zebra:2000"},
{Type: "alpha", Value: "alpha:2000"},
},
X509SvidTtl: 60,
FederatesWith: []string{"spiffe://domaina.test", "spiffe://domainb.test"},
Admin: true,
ExpiresAt: 1552410266,
DnsNames: []string{"unu1000", "ung1000"},
Downstream: true,
StoreSvid: true,
},
},
},
fakeResp: fakeRespOKFromCmd2,
fakeResp: fakeRespOKFromCmdWithoutJwtTtl,
expOutPretty: fmt.Sprintf(`Entry ID : entry-id
SPIFFE ID : spiffe://example.org/workload
Parent ID : spiffe://example.org/parent
Expand Down
28 changes: 2 additions & 26 deletions cmd/spire-server/cli/entry/update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,6 @@ type updateCommand struct {
// Whether or not the entry is for a downstream SPIRE server
downstream bool

// TTL for certificates issued to this workload
ttl int

// TTL for x509 SVIDs issued to this workload
x509SvidTTL int

Expand Down Expand Up @@ -88,9 +85,8 @@ func (c *updateCommand) AppendFlags(f *flag.FlagSet) {
f.StringVar(&c.entryID, "entryID", "", "The Registration Entry ID of the record to update")
f.StringVar(&c.parentID, "parentID", "", "The SPIFFE ID of this record's parent")
f.StringVar(&c.spiffeID, "spiffeID", "", "The SPIFFE ID that this record represents")
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")
f.IntVar(&c.x509SvidTTL, "x509SVIDTTL", 0, "The lifetime, in seconds, for x509-SVIDs issued based on this registration entry. Overrides ttl flag")
f.IntVar(&c.jwtSvidTTL, "jwtSVIDTTL", 0, "The lifetime, in seconds, for JWT-SVIDs issued based on this registration entry. Overrides ttl flag")
f.IntVar(&c.x509SvidTTL, "x509SVIDTTL", 0, "The lifetime, in seconds, for x509-SVIDs issued based on this registration entry.")
f.IntVar(&c.jwtSvidTTL, "jwtSVIDTTL", 0, "The lifetime, in seconds, for JWT-SVIDs issued based on this registration entry.")
f.StringVar(&c.path, "data", "", "Path to a file containing registration JSON (optional). If set to '-', read the JSON from stdin.")
f.Var(&c.selectors, "selector", "A colon-delimited type:value selector. Can be used more than once")
f.Var(&c.federatesWith, "federatesWith", "SPIFFE ID of a trust domain to federate with. Can be used more than once")
Expand Down Expand Up @@ -151,10 +147,6 @@ func (c *updateCommand) validate() (err error) {
return errors.New("a SPIFFE ID is required")
}

if c.ttl < 0 {
return errors.New("a positive TTL is required")
}

if c.x509SvidTTL < 0 {
return errors.New("a positive x509-SVID TTL is required")
}
Expand All @@ -163,10 +155,6 @@ func (c *updateCommand) validate() (err error) {
return errors.New("a positive JWT-SVID TTL is required")
}

if c.ttl > 0 && (c.x509SvidTTL > 0 || c.jwtSvidTTL > 0) {
return errors.New("use x509SVIDTTL and jwtSVIDTTL flags or the deprecated ttl flag")
}

return nil
}

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

// c.ttl is deprecated but usable if the new c.x509Svid field is not used.
// c.ttl should not be used to set the jwtSVIDTTL value because the previous
// behavior was to have a hard-coded 5 minute JWT TTL no matter what the value
// of ttl was set to.
// validate(...) ensures that either the new fields or the deprecated field is
// used, but never a mixture.
//
// https://github.com/spiffe/spire/issues/2700
if e.X509SvidTtl == 0 {
e.X509SvidTtl = int32(c.ttl)
}

selectors := []*types.Selector{}
for _, s := range c.selectors {
cs, err := util.ParseSelector(s)
Expand Down
Loading

0 comments on commit 32eaeca

Please sign in to comment.