Skip to content

Commit

Permalink
Review feedback
Browse files Browse the repository at this point in the history 
Signed-off-by: Dennis Gove <dgove1@bloomberg.net>
  • Loading branch information
@dennisgove
dennisgove committed Oct 24, 2022
1 parent 13c26e6 commit cafa109
Show file tree
Hide file tree
Showing 20 changed files with 690 additions and 653 deletions.
27 changes: 14 additions & 13 deletions cmd/spire-server/cli/entry/create.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package entry
import (
"errors"
"flag"

"github.com/mitchellh/cli"
entryv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/entry/v1"
"github.com/spiffe/spire-api-sdk/proto/spire/api/types"
Expand Down Expand Up @@ -39,15 +40,15 @@ type createCommand struct {
spiffeID 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
// 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
x509SvidTTL int

// TTL for JWT SVIDs issued to this workload
jwtSvidTtl int
jwtSvidTTL int

// List of SPIFFE IDs of trust domains the registration entry is federated with
federatesWith StringsFlag
Expand Down Expand Up @@ -82,9 +83,9 @@ func (*createCommand) Synopsis() string {
func (c *createCommand) AppendFlags(f *flag.FlagSet) {
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 field 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 field")
f.IntVar(&c.jwtSvidTtl, "jwtSvidTtl", 0, "The lifetime, in seconds, for JWT-SVIDs issued based on this registration entry. Overrides ttl field")
f.IntVar(&c.ttl, "ttl", 0, "The lifetime, in seconds, for SVIDs issued based on this registration entry. This field 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 field")
f.IntVar(&c.jwtSvidTTL, "jwtSvidTTL", 0, "The lifetime, in seconds, for JWT-SVIDs issued based on this registration entry. Overrides ttl field")
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 @@ -165,16 +166,16 @@ func (c *createCommand) validate() (err error) {
return errors.New("a positive TTL is required")
}

if c.x509SvidTtl < 0 {
if c.x509SvidTTL < 0 {
return errors.New("a positive x509-SVID TTL is required")
}

if c.jwtSvidTtl < 0 {
if c.jwtSvidTTL < 0 {
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 fields or the deprecated ttl field")
if c.ttl > 0 && (c.x509SvidTTL > 0 || c.jwtSvidTTL > 0) {
return errors.New("use x509SvidTTL and jwtSvidTTL fields or the deprecated ttl field")
}

return nil
Expand All @@ -199,12 +200,12 @@ func (c *createCommand) parseConfig() ([]*types.Entry, error) {
ExpiresAt: c.entryExpiry,
DnsNames: c.dnsNames,
StoreSvid: c.storeSVID,
X509SvidTtl: int32(c.x509SvidTtl),
JwtSvidTtl: int32(c.jwtSvidTtl),
X509SvidTtl: int32(c.x509SvidTTL),
JwtSvidTtl: int32(c.jwtSvidTTL),
}

// 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
// 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
Expand Down
16 changes: 8 additions & 8 deletions cmd/spire-server/cli/entry/create_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,18 +180,18 @@ func TestCreate(t *testing.T) {
},
{
name: "Invalid TTL and X509SvidTtl",
args: []string{"-selector", "unix", "-parentID", "spiffe://example.org/parent", "-spiffeID", "spiffe://example.org/workload", "-ttl", "10", "-x509SvidTtl", "20"},
expErr: "Error: use x509SvidTtl and jwtSvidTtl fields or the deprecated ttl field\n",
args: []string{"-selector", "unix", "-parentID", "spiffe://example.org/parent", "-spiffeID", "spiffe://example.org/workload", "-ttl", "10", "-x509SvidTTL", "20"},
expErr: "Error: use x509SvidTTL and jwtSvidTTL fields or the deprecated ttl field\n",
},
{
name: "Invalid TTL and JwtSvidTtl",
args: []string{"-selector", "unix", "-parentID", "spiffe://example.org/parent", "-spiffeID", "spiffe://example.org/workload", "-ttl", "10", "-jwtSvidTtl", "20"},
expErr: "Error: use x509SvidTtl and jwtSvidTtl fields or the deprecated ttl field\n",
args: []string{"-selector", "unix", "-parentID", "spiffe://example.org/parent", "-spiffeID", "spiffe://example.org/workload", "-ttl", "10", "-jwtSvidTTL", "20"},
expErr: "Error: use x509SvidTTL and jwtSvidTTL fields or the deprecated ttl field\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"},
expErr: "Error: use x509SvidTtl and jwtSvidTtl fields or the deprecated ttl field\n",
args: []string{"-selector", "unix", "-parentID", "spiffe://example.org/parent", "-spiffeID", "spiffe://example.org/workload", "-ttl", "10", "-x509SvidTTL", "20", "-jwtSvidTTL", "30"},
expErr: "Error: use x509SvidTTL and jwtSvidTTL fields or the deprecated ttl field\n",
},
{
name: "Federated node entries",
Expand All @@ -218,8 +218,8 @@ func TestCreate(t *testing.T) {
"-parentID", "spiffe://example.org/parent",
"-selector", "zebra:zebra:2000",
"-selector", "alpha:alpha:2000",
"-x509SvidTtl", "60",
"-jwtSvidTtl", "30",
"-x509SvidTTL", "60",
"-jwtSvidTTL", "30",
"-federatesWith", "spiffe://domaina.test",
"-federatesWith", "spiffe://domainb.test",
"-admin",
Expand Down
Loading

0 comments on commit cafa109

Please sign in to comment.