Additional review feedback

Signed-off-by: Dennis Gove <dgove1@bloomberg.net>
spiffe · Oct 24, 2022 · 66b7fe1 · 66b7fe1
1 parent a01e84f
commit 66b7fe1
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 43 deletions.
diff --git a/cmd/spire-server/cli/entry/create.go b/cmd/spire-server/cli/entry/create.go
@@ -40,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
@@ -83,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")
@@ -166,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
@@ -200,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

diff --git a/cmd/spire-server/cli/entry/create_test.go b/cmd/spire-server/cli/entry/create_test.go
@@ -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",
@@ -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",

diff --git a/cmd/spire-server/cli/entry/update.go b/cmd/spire-server/cli/entry/update.go
@@ -81,9 +81,9 @@ 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 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")
@@ -172,7 +172,7 @@ func (c *updateCommand) validate() (err error) {
 	}
 
 	if c.ttl > 0 && (c.x509SvidTTL > 0 || c.jwtSvidTTL > 0) {
-		return errors.New("use x509SvidTTL and jwtSvidTTL fields or the deprecated ttl field")
+		return errors.New("use x509SVIDTTL and jwtSVIDTTL fields or the deprecated ttl field")
 	}
 
 	return nil
@@ -201,7 +201,7 @@ func (c *updateCommand) parseConfig() ([]*types.Entry, error) {
 	}
 
 	// 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

diff --git a/cmd/spire-server/cli/entry/update_test.go b/cmd/spire-server/cli/entry/update_test.go
@@ -188,18 +188,18 @@ func TestUpdate(t *testing.T) {
 		},
 		{
 			name:   "Invalid TTL and X509SvidTtl",
-			args:   []string{"-entryID", "entry-id", "-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{"-entryID", "entry-id", "-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{"-entryID", "entry-id", "-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{"-entryID", "entry-id", "-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{"-entryID", "entry-id", "-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{"-entryID", "entry-id", "-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: "Server error",
@@ -223,8 +223,8 @@ func TestUpdate(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",
@@ -302,8 +302,8 @@ Admin            : true
 				"-parentID", "spiffe://example.org/parent",
 				"-selector", "type:key1:value",
 				"-selector", "type:key2:value",
-				"-x509SvidTTL", "60",
-				"-jwtSvidTTL", "30",
+				"-x509SVIDTTL", "60",
+				"-jwtSVIDTTL", "30",
 				"-federatesWith", "spiffe://domaina.test",
 				"-federatesWith", "spiffe://domainb.test",
 				"-entryExpiry", "1552410266",

diff --git a/cmd/spire-server/cli/entry/util_posix_test.go b/cmd/spire-server/cli/entry/util_posix_test.go
@@ -17,7 +17,7 @@ const (
     	An expiry, from epoch in seconds, for the resulting registration entry to be pruned
   -federatesWith value
     	SPIFFE ID of a trust domain to federate with. Can be used more than once
-  -jwtSvidTTL int
+  -jwtSVIDTTL int
     	The lifetime, in seconds, for JWT-SVIDs issued based on this registration entry. Overrides ttl field
   -node
     	If set, this entry will be applied to matching nodes rather than workloads
@@ -32,8 +32,8 @@ const (
   -storeSVID
     	A boolean value that, when set, indicates that the resulting issued SVID from this entry must be stored through an SVIDStore plugin
   -ttl int
-    	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
-  -x509SvidTTL int
+    	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
+  -x509SVIDTTL int
     	The lifetime, in seconds, for x509-SVIDs issued based on this registration entry. Overrides ttl field
 `
 	showUsage = `Usage of entry show:
@@ -71,7 +71,7 @@ const (
     	The Registration Entry ID of the record to update
   -federatesWith value
     	SPIFFE ID of a trust domain to federate with. Can be used more than once
-  -jwtSvidTTL int
+  -jwtSVIDTTL int
     	The lifetime, in seconds, for JWT-SVIDs issued based on this registration entry. Overrides ttl field
   -parentID string
     	The SPIFFE ID of this record's parent
@@ -84,8 +84,8 @@ const (
   -storeSVID
     	A boolean value that, when set, indicates that the resulting issued SVID from this entry must be stored through an SVIDStore plugin
   -ttl int
-    	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
-  -x509SvidTTL int
+    	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
+  -x509SVIDTTL int
     	The lifetime, in seconds, for x509-SVIDs issued based on this registration entry. Overrides ttl field
 `
 )
diff --git a/go.mod b/go.mod
@@ -55,7 +55,7 @@ require (
 	github.com/shirou/gopsutil/v3 v3.22.9
 	github.com/sirupsen/logrus v1.9.0
 	github.com/spiffe/go-spiffe/v2 v2.0.1-0.20220414143532-2ed460a8b9d3
-	github.com/spiffe/spire-api-sdk v1.2.5-0.20220608195902-84fd618158c9
+	github.com/spiffe/spire-api-sdk v1.2.5-0.20221020001527-5895a0279944
 	github.com/spiffe/spire-plugin-sdk v1.4.1-0.20220912221658-c42ab2d657f6
 	github.com/stretchr/testify v1.8.1
 	github.com/uber-go/tally/v4 v4.1.3

diff --git a/go.sum b/go.sum
@@ -912,6 +912,8 @@ github.com/spf13/viper v1.7.0/go.mod h1:8WkrPz2fc9jxqZNCJI/76HCieCp4Q8HaLFoCha5q
 github.com/spf13/viper v1.8.1/go.mod h1:o0Pch8wJ9BVSWGQMbra6iw0oQ5oktSIBaujf1rJH9Ns=
 github.com/spiffe/go-spiffe/v2 v2.0.1-0.20220414143532-2ed460a8b9d3 h1:FpqM5PfWHs4Ze36HwzMpRefrv8kkmxFgtG9Qc6hL7Dc=
 github.com/spiffe/go-spiffe/v2 v2.0.1-0.20220414143532-2ed460a8b9d3/go.mod h1:ifsAYiK9MOyuGYFUHUQ3K47dj+k/gd4IcWhlCyDJZEU=
+github.com/spiffe/spire-api-sdk v1.2.5-0.20221020001527-5895a0279944 h1:yoKYON+goNlajhkpKSfwVPB1qvmeh9MmWDyj5zc4C7o=
+github.com/spiffe/spire-api-sdk v1.2.5-0.20221020001527-5895a0279944/go.mod h1:4uuhFlN6KBWjACRP3xXwrOTNnvaLp1zJs8Lribtr4fI=
 github.com/spiffe/spire-plugin-sdk v1.4.1-0.20220912221658-c42ab2d657f6 h1:QViYo6JR+v2lTMV/w9Py1mWJEXTrLn1Hb6ZsCWSVVek=
 github.com/spiffe/spire-plugin-sdk v1.4.1-0.20220912221658-c42ab2d657f6/go.mod h1:4KW5J6abGIAyUS8IL7Fi0NOfoWR6jA5LufKPnIdm9FE=
 github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8=