diff --git a/cmd/spire-server/cli/entry/create.go b/cmd/spire-server/cli/entry/create.go index 0ae0bc3cef4..8d3bcf884b1 100644 --- a/cmd/spire-server/cli/entry/create.go +++ b/cmd/spire-server/cli/entry/create.go @@ -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" @@ -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 @@ -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") @@ -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 @@ -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 diff --git a/cmd/spire-server/cli/entry/create_test.go b/cmd/spire-server/cli/entry/create_test.go index 1a5b8f00ae3..71b7860d33c 100644 --- 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 index 54d2168d302..4b623a03fd2 100644 --- a/cmd/spire-server/cli/entry/update.go +++ b/cmd/spire-server/cli/entry/update.go @@ -1,253 +1,253 @@ package entry import ( - "errors" - "flag" + "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" - "github.com/spiffe/spire/cmd/spire-server/util" - common_cli "github.com/spiffe/spire/pkg/common/cli" - "google.golang.org/grpc/codes" + "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" + "github.com/spiffe/spire/cmd/spire-server/util" + common_cli "github.com/spiffe/spire/pkg/common/cli" + "google.golang.org/grpc/codes" - "golang.org/x/net/context" + "golang.org/x/net/context" ) // NewUpdateCommand creates a new "update" subcommand for "entry" command. func NewUpdateCommand() cli.Command { - return newUpdateCommand(common_cli.DefaultEnv) + return newUpdateCommand(common_cli.DefaultEnv) } func newUpdateCommand(env *common_cli.Env) cli.Command { - return util.AdaptCommand(env, new(updateCommand)) + return util.AdaptCommand(env, new(updateCommand)) } type updateCommand struct { - // Path to an optional data file. If set, other - // opts will be ignored. - path string + // Path to an optional data file. If set, other + // opts will be ignored. + path string - // Registration entry id to update - entryID string + // Registration entry id to update + entryID string - // Type and value are delimited by a colon (:) - // ex. "unix:uid:1000" or "spiffe_id:spiffe://example.org/foo" - selectors StringsFlag + // Type and value are delimited by a colon (:) + // ex. "unix:uid:1000" or "spiffe_id:spiffe://example.org/foo" + selectors StringsFlag - // Workload parent spiffeID - parentID string + // Workload parent spiffeID + parentID string - // Workload spiffeID - spiffeID string + // Workload spiffeID + spiffeID string - // Whether or not the entry is for a downstream SPIRE server - downstream bool + // Whether or not the entry is for a downstream SPIRE server + downstream bool - // TTL for certificates issued to this workload - ttl int + // TTL for certificates issued to this workload + ttl int - // TTL for x509 SVIDs issued to this workload - x509SvidTtl int + // TTL for x509 SVIDs issued to this workload + x509SvidTTL int - // TTL for JWT SVIDs issued to this workload - jwtSvidTtl int + // TTL for JWT SVIDs issued to this workload + jwtSvidTTL int - // List of SPIFFE IDs of trust domains the registration entry is federated with - federatesWith StringsFlag + // List of SPIFFE IDs of trust domains the registration entry is federated with + federatesWith StringsFlag - // Whether or not the registration entry is for an "admin" workload - admin bool + // Whether or not the registration entry is for an "admin" workload + admin bool - // Expiry of entry - entryExpiry int64 + // Expiry of entry + entryExpiry int64 - // DNSNames entries for SVIDs based on this entry - dnsNames StringsFlag + // DNSNames entries for SVIDs based on this entry + dnsNames StringsFlag - // storeSVID determines if the issued SVID must be stored through an SVIDStore plugin - storeSVID bool + // storeSVID determines if the issued SVID must be stored through an SVIDStore plugin + storeSVID bool } func (*updateCommand) Name() string { - return "entry update" + return "entry update" } func (*updateCommand) Synopsis() string { - return "Updates registration entries" + return "Updates registration entries" } 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.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") - f.BoolVar(&c.admin, "admin", false, "If set, the SPIFFE ID in this entry will be granted access to the SPIRE Server's management APIs") - f.BoolVar(&c.downstream, "downstream", false, "A boolean value that, when set, indicates that the entry describes a downstream SPIRE server") - f.BoolVar(&c.storeSVID, "storeSVID", false, "A boolean value that, when set, indicates that the resulting issued SVID from this entry must be stored through an SVIDStore plugin") - f.Int64Var(&c.entryExpiry, "entryExpiry", 0, "An expiry, from epoch in seconds, for the resulting registration entry to be pruned") - f.Var(&c.dnsNames, "dns", "A DNS name that will be included in SVIDs issued based on this entry, where appropriate. Can be used more than once") + 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.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") + f.BoolVar(&c.admin, "admin", false, "If set, the SPIFFE ID in this entry will be granted access to the SPIRE Server's management APIs") + f.BoolVar(&c.downstream, "downstream", false, "A boolean value that, when set, indicates that the entry describes a downstream SPIRE server") + f.BoolVar(&c.storeSVID, "storeSVID", false, "A boolean value that, when set, indicates that the resulting issued SVID from this entry must be stored through an SVIDStore plugin") + f.Int64Var(&c.entryExpiry, "entryExpiry", 0, "An expiry, from epoch in seconds, for the resulting registration entry to be pruned") + f.Var(&c.dnsNames, "dns", "A DNS name that will be included in SVIDs issued based on this entry, where appropriate. Can be used more than once") } func (c *updateCommand) Run(ctx context.Context, env *common_cli.Env, serverClient util.ServerClient) error { - if err := c.validate(); err != nil { - return err - } - - var entries []*types.Entry - var err error - if c.path != "" { - entries, err = parseFile(c.path) - } else { - entries, err = c.parseConfig() - } - if err != nil { - return err - } - - succeeded, failed, err := updateEntries(ctx, serverClient.NewEntryClient(), entries) - if err != nil { - return err - } - - // Print entries that succeeded to be updated - for _, e := range succeeded { - printEntry(e.Entry, env.Printf) - } - - // Print entries that failed to be updated - for _, r := range failed { - env.ErrPrintf("Failed to update the following entry (code: %s, msg: %q):\n", - codes.Code(r.Status.Code), - r.Status.Message) - printEntry(r.Entry, env.ErrPrintf) - } - - if len(failed) > 0 { - return errors.New("failed to update one or more entries") - } - - return nil + if err := c.validate(); err != nil { + return err + } + + var entries []*types.Entry + var err error + if c.path != "" { + entries, err = parseFile(c.path) + } else { + entries, err = c.parseConfig() + } + if err != nil { + return err + } + + succeeded, failed, err := updateEntries(ctx, serverClient.NewEntryClient(), entries) + if err != nil { + return err + } + + // Print entries that succeeded to be updated + for _, e := range succeeded { + printEntry(e.Entry, env.Printf) + } + + // Print entries that failed to be updated + for _, r := range failed { + env.ErrPrintf("Failed to update the following entry (code: %s, msg: %q):\n", + codes.Code(r.Status.Code), + r.Status.Message) + printEntry(r.Entry, env.ErrPrintf) + } + + if len(failed) > 0 { + return errors.New("failed to update one or more entries") + } + + return nil } // validate performs basic validation, even on fields that we // have defaults defined for func (c *updateCommand) validate() (err error) { - // If a path is set, we have all we need - if c.path != "" { - return nil - } + // If a path is set, we have all we need + if c.path != "" { + return nil + } - if c.entryID == "" { - return errors.New("entry ID is required") - } + if c.entryID == "" { + return errors.New("entry ID is required") + } - if len(c.selectors) < 1 { - return errors.New("at least one selector is required") - } + if len(c.selectors) < 1 { + return errors.New("at least one selector is required") + } - if c.parentID == "" { - return errors.New("a parent ID is required") - } + if c.parentID == "" { + return errors.New("a parent ID is required") + } - if c.spiffeID == "" { - return errors.New("a SPIFFE ID is required") - } + if c.spiffeID == "" { + return errors.New("a SPIFFE ID is required") + } - if c.ttl < 0 { - return errors.New("a positive TTL 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") - } + if c.x509SvidTTL < 0 { + return errors.New("a positive x509-SVID TTL is required") + } - if c.jwtSvidTtl < 0 { - return errors.New("a positive JWT-SVID TTL is required") - } + 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 + return nil } // parseConfig builds a registration entry from the given config func (c *updateCommand) parseConfig() ([]*types.Entry, error) { - parentID, err := idStringToProto(c.parentID) - if err != nil { - return nil, err - } - spiffeID, err := idStringToProto(c.spiffeID) - if err != nil { - return nil, err - } - - e := &types.Entry{ - Id: c.entryID, - ParentId: parentID, - SpiffeId: spiffeID, - Downstream: c.downstream, - ExpiresAt: c.entryExpiry, - DnsNames: c.dnsNames, - 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 - // 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) - if err != nil { - return nil, err - } - - selectors = append(selectors, cs) - } - - e.Selectors = selectors - e.FederatesWith = c.federatesWith - e.Admin = c.admin - e.StoreSvid = c.storeSVID - return []*types.Entry{e}, nil + parentID, err := idStringToProto(c.parentID) + if err != nil { + return nil, err + } + spiffeID, err := idStringToProto(c.spiffeID) + if err != nil { + return nil, err + } + + e := &types.Entry{ + Id: c.entryID, + ParentId: parentID, + SpiffeId: spiffeID, + Downstream: c.downstream, + ExpiresAt: c.entryExpiry, + DnsNames: c.dnsNames, + 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 + // 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) + if err != nil { + return nil, err + } + + selectors = append(selectors, cs) + } + + e.Selectors = selectors + e.FederatesWith = c.federatesWith + e.Admin = c.admin + e.StoreSvid = c.storeSVID + return []*types.Entry{e}, nil } func updateEntries(ctx context.Context, c entryv1.EntryClient, entries []*types.Entry) (succeeded, failed []*entryv1.BatchUpdateEntryResponse_Result, err error) { - resp, err := c.BatchUpdateEntry(ctx, &entryv1.BatchUpdateEntryRequest{ - Entries: entries, - }) - if err != nil { - return nil, nil, err - } - - for i, r := range resp.Results { - switch r.Status.Code { - case int32(codes.OK): - succeeded = append(succeeded, r) - default: - // The Entry API does not include in the results the entries that - // failed to be updated, so we populate them from the request data. - r.Entry = entries[i] - failed = append(failed, r) - } - } - - return succeeded, failed, nil + resp, err := c.BatchUpdateEntry(ctx, &entryv1.BatchUpdateEntryRequest{ + Entries: entries, + }) + if err != nil { + return nil, nil, err + } + + for i, r := range resp.Results { + switch r.Status.Code { + case int32(codes.OK): + succeeded = append(succeeded, r) + default: + // The Entry API does not include in the results the entries that + // failed to be updated, so we populate them from the request data. + r.Entry = entries[i] + failed = append(failed, r) + } + } + + return succeeded, failed, nil } diff --git a/cmd/spire-server/cli/entry/update_test.go b/cmd/spire-server/cli/entry/update_test.go index e619b3a895d..6e2bc73f489 100644 --- 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.go b/cmd/spire-server/cli/entry/util.go index 6ff53bac8c2..7e4a322fc49 100644 --- a/cmd/spire-server/cli/entry/util.go +++ b/cmd/spire-server/cli/entry/util.go @@ -1,122 +1,122 @@ package entry import ( - "encoding/json" - "fmt" - "io" - "os" - "time" - - "github.com/spiffe/go-spiffe/v2/spiffeid" - "github.com/spiffe/spire-api-sdk/proto/spire/api/types" - "github.com/spiffe/spire/pkg/server/api" - "github.com/spiffe/spire/proto/spire/common" + "encoding/json" + "fmt" + "io" + "os" + "time" + + "github.com/spiffe/go-spiffe/v2/spiffeid" + "github.com/spiffe/spire-api-sdk/proto/spire/api/types" + "github.com/spiffe/spire/pkg/server/api" + "github.com/spiffe/spire/proto/spire/common" ) func printEntry(e *types.Entry, printf func(string, ...interface{}) error) { - _ = printf("Entry ID : %s\n", printableEntryID(e.Id)) - _ = printf("SPIFFE ID : %s\n", protoToIDString(e.SpiffeId)) - _ = printf("Parent ID : %s\n", protoToIDString(e.ParentId)) - _ = printf("Revision : %d\n", e.RevisionNumber) - - if e.Downstream { - _ = printf("Downstream : %t\n", e.Downstream) - } - - if e.X509SvidTtl == 0 { - _ = printf("X509SvidTTL : default\n") - } else { - _ = printf("X509SvidTTL : %d\n", e.X509SvidTtl) - } - - if e.JwtSvidTtl == 0 { - _ = printf("JwtSvidTTL : default\n") - } else { - _ = printf("JwtSvidTTL : %d\n", e.JwtSvidTtl) - } - - if e.ExpiresAt != 0 { - _ = printf("Expiration time : %s\n", time.Unix(e.ExpiresAt, 0).UTC()) - } - - for _, s := range e.Selectors { - _ = printf("Selector : %s:%s\n", s.Type, s.Value) - } - for _, id := range e.FederatesWith { - _ = printf("FederatesWith : %s\n", id) - } - for _, dnsName := range e.DnsNames { - _ = printf("DNS name : %s\n", dnsName) - } - - // admin is rare, so only show admin if true to keep - // from muddying the output. - if e.Admin { - _ = printf("Admin : %t\n", e.Admin) - } - - if e.StoreSvid { - _ = printf("StoreSvid : %t\n", e.StoreSvid) - } - - _ = printf("\n") + _ = printf("Entry ID : %s\n", printableEntryID(e.Id)) + _ = printf("SPIFFE ID : %s\n", protoToIDString(e.SpiffeId)) + _ = printf("Parent ID : %s\n", protoToIDString(e.ParentId)) + _ = printf("Revision : %d\n", e.RevisionNumber) + + if e.Downstream { + _ = printf("Downstream : %t\n", e.Downstream) + } + + if e.X509SvidTtl == 0 { + _ = printf("X509SvidTTL : default\n") + } else { + _ = printf("X509SvidTTL : %d\n", e.X509SvidTtl) + } + + if e.JwtSvidTtl == 0 { + _ = printf("JwtSvidTTL : default\n") + } else { + _ = printf("JwtSvidTTL : %d\n", e.JwtSvidTtl) + } + + if e.ExpiresAt != 0 { + _ = printf("Expiration time : %s\n", time.Unix(e.ExpiresAt, 0).UTC()) + } + + for _, s := range e.Selectors { + _ = printf("Selector : %s:%s\n", s.Type, s.Value) + } + for _, id := range e.FederatesWith { + _ = printf("FederatesWith : %s\n", id) + } + for _, dnsName := range e.DnsNames { + _ = printf("DNS name : %s\n", dnsName) + } + + // admin is rare, so only show admin if true to keep + // from muddying the output. + if e.Admin { + _ = printf("Admin : %t\n", e.Admin) + } + + if e.StoreSvid { + _ = printf("StoreSvid : %t\n", e.StoreSvid) + } + + _ = printf("\n") } // idStringToProto converts a SPIFFE ID from the given string to *types.SPIFFEID func idStringToProto(id string) (*types.SPIFFEID, error) { - idType, err := spiffeid.FromString(id) - if err != nil { - return nil, err - } - return &types.SPIFFEID{ - TrustDomain: idType.TrustDomain().String(), - Path: idType.Path(), - }, nil + idType, err := spiffeid.FromString(id) + if err != nil { + return nil, err + } + return &types.SPIFFEID{ + TrustDomain: idType.TrustDomain().String(), + Path: idType.Path(), + }, nil } func printableEntryID(id string) string { - if id == "" { - return "(none)" - } - return id + if id == "" { + return "(none)" + } + return id } // protoToIDString converts a SPIFFE ID from the given *types.SPIFFEID to string func protoToIDString(id *types.SPIFFEID) string { - if id == nil { - return "" - } - return fmt.Sprintf("spiffe://%s%s", id.TrustDomain, id.Path) + if id == nil { + return "" + } + return fmt.Sprintf("spiffe://%s%s", id.TrustDomain, id.Path) } // parseFile parses JSON represented RegistrationEntries // if path is "-" read JSON from STDIN func parseFile(path string) ([]*types.Entry, error) { - return parseEntryJSON(os.Stdin, path) + return parseEntryJSON(os.Stdin, path) } func parseEntryJSON(in io.Reader, path string) ([]*types.Entry, error) { - entries := &common.RegistrationEntries{} - - r := in - if path != "-" { - f, err := os.Open(path) - if err != nil { - return nil, err - } - defer f.Close() - r = f - } - - dat, err := io.ReadAll(r) - if err != nil { - return nil, err - } - - if err := json.Unmarshal(dat, &entries); err != nil { - return nil, err - } - return api.RegistrationEntriesToProto(entries.Entries) + entries := &common.RegistrationEntries{} + + r := in + if path != "-" { + f, err := os.Open(path) + if err != nil { + return nil, err + } + defer f.Close() + r = f + } + + dat, err := io.ReadAll(r) + if err != nil { + return nil, err + } + + if err := json.Unmarshal(dat, &entries); err != nil { + return nil, err + } + return api.RegistrationEntriesToProto(entries.Entries) } // StringsFlag defines a custom type for string lists. Doing @@ -125,11 +125,11 @@ type StringsFlag []string // String returns the string flag. func (s *StringsFlag) String() string { - return fmt.Sprint(*s) + return fmt.Sprint(*s) } // Set appends the string flag. func (s *StringsFlag) Set(val string) error { - *s = append(*s, val) - return nil + *s = append(*s, val) + return nil } diff --git a/cmd/spire-server/cli/entry/util_posix_test.go b/cmd/spire-server/cli/entry/util_posix_test.go index 21fb4d23cf1..c1f71b9c856 100644 --- 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/cmd/spire-server/cli/run/run.go b/cmd/spire-server/cli/run/run.go index 3148be2b43a..3c8cdf42a32 100644 --- a/cmd/spire-server/cli/run/run.go +++ b/cmd/spire-server/cli/run/run.go @@ -469,18 +469,6 @@ func NewServerConfig(c *Config, logOptions []log.Option, allowUnknownConfig bool sc.AgentTTL = ttl } - if c.Server.DefaultSVIDTTL != "" { - ttl, err := time.ParseDuration(c.Server.DefaultSVIDTTL) - if err != nil { - return nil, fmt.Errorf("could not parse default SVID ttl %q: %w", c.Server.DefaultSVIDTTL, err) - } - sc.SVIDTTL = ttl - - if sc.SVIDTTL != 0 { - logger.Warn("field default_svid_ttl is deprecated; consider using default_x509_svid_ttl and default_jwt_svid_ttl instead") - } - } - if c.Server.DefaultX509SVIDTTL != "" { ttl, err := time.ParseDuration(c.Server.DefaultX509SVIDTTL) if err != nil { @@ -488,9 +476,17 @@ func NewServerConfig(c *Config, logOptions []log.Option, allowUnknownConfig bool } sc.X509SVIDTTL = ttl - if sc.X509SVIDTTL != 0 && sc.SVIDTTL != 0 { + if sc.X509SVIDTTL != 0 && c.Server.DefaultSVIDTTL != "" { logger.Warnf("both default_x509_svid_ttl and default_svid_ttl are configured; default_x509_svid_ttl (%s) will be used for X509-SVIDs", c.Server.DefaultX509SVIDTTL) } + } else if c.Server.DefaultSVIDTTL != "" { + logger.Warn("field default_svid_ttl is deprecated; consider using default_x509_svid_ttl and default_jwt_svid_ttl instead") + + ttl, err := time.ParseDuration(c.Server.DefaultSVIDTTL) + if err != nil { + return nil, fmt.Errorf("could not parse default SVID ttl %q: %w", c.Server.DefaultSVIDTTL, err) + } + sc.X509SVIDTTL = ttl } if c.Server.DefaultJwtSVIDTTL != "" { @@ -500,7 +496,7 @@ func NewServerConfig(c *Config, logOptions []log.Option, allowUnknownConfig bool } sc.JWTSVIDTTL = ttl - if sc.JWTSVIDTTL != 0 && sc.SVIDTTL != 0 { + if sc.JWTSVIDTTL != 0 && c.Server.DefaultSVIDTTL != "" { logger.Warnf("both default_jwt_svid_ttl and default_svid_ttl are configured; default_jwt_svid_ttl (%s) will be used for JWT-SVIDs", c.Server.DefaultJwtSVIDTTL) } } @@ -515,57 +511,57 @@ func NewServerConfig(c *Config, logOptions []log.Option, allowUnknownConfig bool // If the configured TTLs can lead to surprises, then do our best to log an // accurate message and guide the user to resolution - if !hasCompatibleTTLs(sc.CATTL, sc.SVIDTTL, sc.X509SVIDTTL, sc.JWTSVIDTTL) { - msgCATTLTooSmall := fmt.Sprintf( - "One of default_svid_ttl, default_x509_svid_ttl, "+ - "or default_jwt_svid_ttl is too high for the "+ - "configured ca_ttl value. SVIDs with shorter "+ - "lifetimes may be issued. Please set all of "+ - "default_svid_ttl, default_x509_svid_ttl, or "+ - "default_jwt_svid_ttl to %v or less, or the ca_ttl "+ - "to %v or more, to guarantee the full default_svid_ttl, "+ - "default_x509_svid_ttl, or default_jwt_svid_ttl "+ - "lifetimes when CA rotations are scheduled.", - printMaxSVIDTTL(sc.CATTL), printMinCATTL(sc.SVIDTTL), - ) - msgSVIDTTLTooLargeAndCATTLTooSmall := fmt.Sprintf( - "One of default_svid_ttl, default_x509_svid_ttl, "+ - "or default_jwt_svid_ttl is too high and the ca_ttl "+ - "is too low. SVIDs with shorter lifetimes may be "+ - "issued. Please set all of default_svid_ttl, "+ - "default_x509_svid_ttl, or default_jwt_svid_ttl "+ - "to %v or less, and the ca_ttl to %v or more, to "+ - "guarantee the full default_svid_ttl, "+ - "default_x509_svid_ttl, or default_jwt_svid_ttl "+ - "lifetimes when CA rotations are scheduled.", - printDuration(ca.MaxSVIDTTL()), printMinCATTL(ca.MaxSVIDTTL()), - ) - msgSVIDTTLTooLarge := fmt.Sprintf( - "One of default_svid_ttl, default_x509_svid_ttl, "+ - "or default_jwt_svid_ttl is too high. SVIDs with "+ - "shorter lifetimes may be issued. Please set all of "+ - "default_svid_ttl, default_x509_svid_ttl, or "+ - "default_jwt_svid_ttl to %v or less to guarantee the "+ - "full default_svid_ttl, default_x509_svid_ttl, or "+ - "default_jwt_svid_ttl lifetimes when CA rotations are "+ - "scheduled.", - printMaxSVIDTTL(sc.CATTL), - ) + ttlChecks := []struct { + name string + ttl time.Duration + }{ + { + name: "default_x509_svid_ttl (or deprecated default_svid_ttl)", + ttl: sc.X509SVIDTTL, + }, + { + name: "default_jwt_svid_ttl", + ttl: sc.JWTSVIDTTL, + }, + } - switch { - case sc.SVIDTTL < ca.MaxSVIDTTL() || sc.X509SVIDTTL < ca.MaxSVIDTTL() || sc.JWTSVIDTTL < ca.MaxSVIDTTL(): - // One of the SVID TTLs is smaller than our cap, but the CA TTL - // is not large enough to accommodate it - sc.Log.Warn(msgCATTLTooSmall) - case sc.CATTL < ca.MinCATTLForSVIDTTL(ca.MaxSVIDTTL()): - // One of the SVID TTLs is larger than our cap, it needs to be - // decreased no matter what. Additionally, the CA TTL is - // too small to accommodate the maximum SVID TTL. - sc.Log.Warn(msgSVIDTTLTooLargeAndCATTLTooSmall) - default: - // One of the SVID TTLs is larger than our cap and needs to be - // decreased. - sc.Log.Warn(msgSVIDTTLTooLarge) + for _, ttlCheck := range ttlChecks { + if !hasCompatibleTTL(sc.CATTL, ttlCheck.ttl) { + var message string + + switch { + case ttlCheck.ttl < ca.MaxSVIDTTL(): + // TTL is smaller than our cap, but the CA TTL + // is not large enough to accommodate it + message = fmt.Sprintf("%s is too high for the configured "+ + "ca_ttl value. SVIDs with shorter lifetimes may "+ + "be issued. Please set %s to %v or less, or the ca_ttl "+ + "to %v or more, to guarantee the full %s lifetime "+ + "when CA rotations are scheduled.", + ttlCheck.name, ttlCheck.name, printMaxSVIDTTL(sc.CATTL), printMinCATTL(ttlCheck.ttl), ttlCheck.name, + ) + case sc.CATTL < ca.MinCATTLForSVIDTTL(ca.MaxSVIDTTL()): + // TTL is larger than our cap, it needs to be + // decreased no matter what. Additionally, the CA TTL is + // too small to accommodate the maximum SVID TTL. + message = fmt.Sprintf("%s is too high and "+ + "the ca_ttl is too low. SVIDs with shorter lifetimes "+ + "may be issued. Please set %s to %v or less, and the "+ + "ca_ttl to %v or more, to guarantee the full %s "+ + "lifetime when CA rotations are scheduled.", + ttlCheck.name, ttlCheck.name, printDuration(ca.MaxSVIDTTL()), printMinCATTL(ca.MaxSVIDTTL()), ttlCheck.name, + ) + default: + // TTL is larger than our cap and needs to be + // decreased. + message = fmt.Sprintf("%s is too high. SVIDs with shorter "+ + "lifetimes may be issued. Please set %s to %v or less "+ + "to guarantee the full %s lifetime when CA rotations "+ + "are scheduled.", + ttlCheck.name, ttlCheck.name, printMaxSVIDTTL(sc.CATTL), ttlCheck.name, + ) + } + sc.Log.Warn(message) } } @@ -840,7 +836,6 @@ func defaultConfig() *Config { CATTL: ca.DefaultCATTL.String(), LogLevel: defaultLogLevel, LogFormat: log.DefaultFormat, - DefaultSVIDTTL: ca.DefaultX509SVIDTTL.String(), DefaultX509SVIDTTL: ca.DefaultX509SVIDTTL.String(), DefaultJwtSVIDTTL: ca.DefaultJWTSVIDTTL.String(), Experimental: experimentalConfig{}, @@ -863,18 +858,12 @@ func keyTypeFromString(s string) (keymanager.KeyType, error) { } } -// hasCompatibleTTLs checks if we can guarantee the configured SVID TTLs given the -// configurd CA TTL. If we detect that a new SVIDs TTL may be cut short due to -// a scheduled CA rotation, this function will return false. -func hasCompatibleTTLs(caTTL time.Duration, svidTTLs ...time.Duration) bool { - maxSvidTtl := ca.MaxSVIDTTLForCATTL(caTTL) - for _, svidTTL := range svidTTLs { - if maxSvidTtl < svidTTL { - return false - } - } - - return true +// hasCompatibleTTL checks if we can guarantee the configured SVID TTL given the +// configurd CA TTL. If we detect that a new SVID TTL may be cut short due to +// a scheduled CA rotation, this function will return false. This method should +// be called for each SVID TTL we may use +func hasCompatibleTTL(caTTL time.Duration, svidTTL time.Duration) bool { + return svidTTL <= ca.MaxSVIDTTLForCATTL(caTTL) } // printMaxSVIDTTL calculates the display string for a sufficiently short SVID TTL diff --git a/cmd/spire-server/cli/run/run_test.go b/cmd/spire-server/cli/run/run_test.go index e5589a90647..0f78e213883 100644 --- a/cmd/spire-server/cli/run/run_test.go +++ b/cmd/spire-server/cli/run/run_test.go @@ -355,6 +355,8 @@ func TestMergeInput(t *testing.T) { msg: "default_svid_ttl should be configurable by file", fileInput: func(c *Config) { c.Server.DefaultSVIDTTL = "1h" + c.Server.DefaultX509SVIDTTL = "" + c.Server.DefaultJwtSVIDTTL = "" }, cliFlags: []string{}, test: func(t *testing.T, c *Config) { @@ -637,9 +639,11 @@ func TestNewServerConfig(t *testing.T) { msg: "default_svid_ttl is correctly parsed", input: func(c *Config) { c.Server.DefaultSVIDTTL = "1m" + c.Server.DefaultX509SVIDTTL = "" + c.Server.DefaultJwtSVIDTTL = "" }, test: func(t *testing.T, c *server.Config) { - require.Equal(t, time.Minute, c.SVIDTTL) + require.Equal(t, time.Minute, c.X509SVIDTTL) }, }, { @@ -654,10 +658,10 @@ func TestNewServerConfig(t *testing.T) { { msg: "default_jwt_svid_ttl is correctly parsed", input: func(c *Config) { - c.Server.DefaultSVIDTTL = "3m" + c.Server.DefaultJwtSVIDTTL = "3m" }, test: func(t *testing.T, c *server.Config) { - require.Equal(t, 3*time.Minute, c.SVIDTTL) + require.Equal(t, 3*time.Minute, c.JWTSVIDTTL) }, }, { @@ -665,6 +669,8 @@ func TestNewServerConfig(t *testing.T) { expectError: true, input: func(c *Config) { c.Server.DefaultSVIDTTL = "b" + c.Server.DefaultX509SVIDTTL = "" + c.Server.DefaultJwtSVIDTTL = "" }, test: func(t *testing.T, c *server.Config) { require.Nil(t, c) @@ -1440,188 +1446,234 @@ func TestLogOptions(t *testing.T) { func TestHasCompatibleTTLs(t *testing.T) { cases := []struct { - msg string - caTTL time.Duration - svidTTL time.Duration - x509SvidTtl time.Duration - jwtSvidTtl time.Duration - hasCompatibleTTLs bool + msg string + caTTL time.Duration + svidTTL time.Duration + x509SvidTTL time.Duration + jwtSvidTTL time.Duration + hasCompatibleSvidTTL bool + hasCompatibleX509SvidTTL bool + hasCompatibleJwtSvidTTL bool }{ { - msg: "All values are default values", - caTTL: 0, - svidTTL: 0, - x509SvidTtl: 0, - jwtSvidTtl: 0, - hasCompatibleTTLs: true, - }, - { - msg: "ca_ttl is large enough for all default SVID TTL", - caTTL: time.Hour * 7, - svidTTL: 0, - x509SvidTtl: 0, - jwtSvidTtl: 0, - hasCompatibleTTLs: true, - }, - { - msg: "ca_ttl is not large enough for the default SVID TTLs", - caTTL: time.Minute * 1, - svidTTL: 0, - x509SvidTtl: 0, - jwtSvidTtl: 0, - hasCompatibleTTLs: false, - }, - { - msg: "default_svid_ttl is small enough for the default CA TTL", - caTTL: 0, - svidTTL: time.Hour * 3, - x509SvidTtl: 0, - jwtSvidTtl: 0, - hasCompatibleTTLs: true, - }, - { - msg: "default_svid_ttl is not small enough for the default CA TTL", - caTTL: 0, - svidTTL: time.Hour * 24, - x509SvidTtl: 0, - jwtSvidTtl: 0, - hasCompatibleTTLs: false, - }, - { - msg: "default_svid_ttl is small enough for the configured CA TTL", - caTTL: time.Hour * 24, - svidTTL: time.Hour * 1, - x509SvidTtl: 0, - jwtSvidTtl: 0, - hasCompatibleTTLs: true, - }, - { - msg: "default_svid_ttl is not small enough for the configured CA TTL", - caTTL: time.Hour * 24, - svidTTL: time.Hour * 23, - x509SvidTtl: 0, - jwtSvidTtl: 0, - hasCompatibleTTLs: false, - }, - { - msg: "default_svid_ttl is larger than the configured CA TTL", - caTTL: time.Hour * 24, - svidTTL: time.Hour * 25, - x509SvidTtl: 0, - jwtSvidTtl: 0, - hasCompatibleTTLs: false, - }, - { - msg: "default_svid_ttl is small enough for the configured CA TTL but larger than the max", - caTTL: time.Hour * 24 * 7 * 4 * 6, // Six months - svidTTL: time.Hour * 24 * 7 * 2, // Two weeks - x509SvidTtl: 0, - jwtSvidTtl: 0, - hasCompatibleTTLs: false, - }, - { - msg: "default_x509_svid_ttl is small enough for the default CA TTL", - caTTL: 0, - svidTTL: 0, - x509SvidTtl: time.Hour * 3, - jwtSvidTtl: 0, - hasCompatibleTTLs: true, - }, - { - msg: "default_x509_svid_ttl is not small enough for the default CA TTL", - caTTL: 0, - svidTTL: 0, - x509SvidTtl: time.Hour * 24, - jwtSvidTtl: 0, - hasCompatibleTTLs: false, - }, - { - msg: "default_x509_svid_ttl is small enough for the configured CA TTL", - caTTL: time.Hour * 24, - svidTTL: 0, - x509SvidTtl: time.Hour * 1, - jwtSvidTtl: 0, - hasCompatibleTTLs: true, - }, - { - msg: "default_x509_svid_ttl is not small enough for the configured CA TTL", - caTTL: time.Hour * 24, - svidTTL: 0, - x509SvidTtl: time.Hour * 23, - jwtSvidTtl: 0, - hasCompatibleTTLs: false, - }, - { - msg: "default_x509_svid_ttl is larger than the configured CA TTL", - caTTL: time.Hour * 24, - svidTTL: 0, - x509SvidTtl: time.Hour * 25, - jwtSvidTtl: 0, - hasCompatibleTTLs: false, - }, - { - msg: "default_x509_svid_ttl is small enough for the configured CA TTL but larger than the max", - caTTL: time.Hour * 24 * 7 * 4 * 6, // Six months - svidTTL: 0, - x509SvidTtl: time.Hour * 24 * 7 * 2, // Two weeks, - jwtSvidTtl: 0, - hasCompatibleTTLs: false, - }, - { - msg: "default_jwt_svid_ttl is small enough for the default CA TTL", - caTTL: 0, - svidTTL: 0, - x509SvidTtl: 0, - jwtSvidTtl: time.Hour * 3, - hasCompatibleTTLs: true, - }, - { - msg: "default_jwt_svid_ttl is not small enough for the default CA TTL", - caTTL: 0, - svidTTL: 0, - x509SvidTtl: 0, - jwtSvidTtl: time.Hour * 24, - hasCompatibleTTLs: false, - }, - { - msg: "default_jwt_svid_ttl is small enough for the configured CA TTL", - caTTL: time.Hour * 24, - svidTTL: 0, - x509SvidTtl: 0, - jwtSvidTtl: time.Hour * 1, - hasCompatibleTTLs: true, - }, - { - msg: "default_jwt_svid_ttl is not small enough for the configured CA TTL", - caTTL: time.Hour * 24, - svidTTL: 0, - x509SvidTtl: 0, - jwtSvidTtl: time.Hour * 23, - hasCompatibleTTLs: false, - }, - { - msg: "default_jwt_svid_ttl is larger than the configured CA TTL", - caTTL: time.Hour * 24, - svidTTL: 0, - x509SvidTtl: 0, - jwtSvidTtl: time.Hour * 25, - hasCompatibleTTLs: false, - }, - { - msg: "default_jwt_svid_ttl is small enough for the configured CA TTL but larger than the max", - caTTL: time.Hour * 24 * 7 * 4 * 6, // Six months - svidTTL: 0, - x509SvidTtl: 0, - jwtSvidTtl: time.Hour * 24 * 7 * 2, // Two weeks,, - hasCompatibleTTLs: false, - }, - { - msg: "all default svid_ttls are small enough for the configured CA TTL", - caTTL: time.Hour * 24, - svidTTL: time.Hour * 1, - x509SvidTtl: time.Hour * 1, - jwtSvidTtl: time.Hour * 1, - hasCompatibleTTLs: true, + msg: "All values are default values", + caTTL: 0, + svidTTL: 0, + x509SvidTTL: 0, + jwtSvidTTL: 0, + hasCompatibleSvidTTL: true, + hasCompatibleX509SvidTTL: true, + hasCompatibleJwtSvidTTL: true, + }, + { + msg: "ca_ttl is large enough for all default SVID TTL", + caTTL: time.Hour * 7, + svidTTL: 0, + x509SvidTTL: 0, + jwtSvidTTL: 0, + hasCompatibleSvidTTL: true, + hasCompatibleX509SvidTTL: true, + hasCompatibleJwtSvidTTL: true, + }, + { + msg: "ca_ttl is not large enough for the default SVID TTL", + caTTL: time.Minute * 1, + svidTTL: 0, + x509SvidTTL: 0, + jwtSvidTTL: 0, + hasCompatibleSvidTTL: false, + hasCompatibleX509SvidTTL: false, + hasCompatibleJwtSvidTTL: false, + }, + { + msg: "default_svid_ttl is small enough for the default CA TTL", + caTTL: 0, + svidTTL: time.Hour * 3, + x509SvidTTL: 0, + jwtSvidTTL: 0, + hasCompatibleSvidTTL: true, + hasCompatibleX509SvidTTL: true, + hasCompatibleJwtSvidTTL: true, + }, + { + msg: "default_svid_ttl is not small enough for the default CA TTL", + caTTL: 0, + svidTTL: time.Hour * 24, + x509SvidTTL: 0, + jwtSvidTTL: 0, + hasCompatibleSvidTTL: false, + hasCompatibleX509SvidTTL: true, + hasCompatibleJwtSvidTTL: true, + }, + { + msg: "default_svid_ttl is small enough for the configured CA TTL", + caTTL: time.Hour * 24, + svidTTL: time.Hour * 1, + x509SvidTTL: 0, + jwtSvidTTL: 0, + hasCompatibleSvidTTL: true, + hasCompatibleX509SvidTTL: true, + hasCompatibleJwtSvidTTL: true, + }, + { + msg: "default_svid_ttl is not small enough for the configured CA TTL", + caTTL: time.Hour * 24, + svidTTL: time.Hour * 23, + x509SvidTTL: 0, + jwtSvidTTL: 0, + hasCompatibleSvidTTL: false, + hasCompatibleX509SvidTTL: true, + hasCompatibleJwtSvidTTL: true, + }, + { + msg: "default_svid_ttl is larger than the configured CA TTL", + caTTL: time.Hour * 24, + svidTTL: time.Hour * 25, + x509SvidTTL: 0, + jwtSvidTTL: 0, + hasCompatibleSvidTTL: false, + hasCompatibleX509SvidTTL: true, + hasCompatibleJwtSvidTTL: true, + }, + { + msg: "default_svid_ttl is small enough for the configured CA TTL but larger than the max", + caTTL: time.Hour * 24 * 7 * 4 * 6, // Six months + svidTTL: time.Hour * 24 * 7 * 2, // Two weeks + x509SvidTTL: 0, + jwtSvidTTL: 0, + hasCompatibleSvidTTL: false, + hasCompatibleX509SvidTTL: true, + hasCompatibleJwtSvidTTL: true, + }, + { + msg: "default_x509_svid_ttl is small enough for the default CA TTL", + caTTL: 0, + svidTTL: 0, + x509SvidTTL: time.Hour * 3, + jwtSvidTTL: 0, + hasCompatibleSvidTTL: true, + hasCompatibleX509SvidTTL: true, + hasCompatibleJwtSvidTTL: true, + }, + { + msg: "default_x509_svid_ttl is not small enough for the default CA TTL", + caTTL: 0, + svidTTL: 0, + x509SvidTTL: time.Hour * 24, + jwtSvidTTL: 0, + hasCompatibleSvidTTL: true, + hasCompatibleX509SvidTTL: false, + hasCompatibleJwtSvidTTL: true, + }, + { + msg: "default_x509_svid_ttl is small enough for the configured CA TTL", + caTTL: time.Hour * 24, + svidTTL: 0, + x509SvidTTL: time.Hour * 1, + jwtSvidTTL: 0, + hasCompatibleSvidTTL: true, + hasCompatibleX509SvidTTL: true, + hasCompatibleJwtSvidTTL: true, + }, + { + msg: "default_x509_svid_ttl is not small enough for the configured CA TTL", + caTTL: time.Hour * 24, + svidTTL: 0, + x509SvidTTL: time.Hour * 23, + jwtSvidTTL: 0, + hasCompatibleSvidTTL: true, + hasCompatibleX509SvidTTL: false, + hasCompatibleJwtSvidTTL: true, + }, + { + msg: "default_x509_svid_ttl is larger than the configured CA TTL", + caTTL: time.Hour * 24, + svidTTL: 0, + x509SvidTTL: time.Hour * 25, + jwtSvidTTL: 0, + hasCompatibleSvidTTL: true, + hasCompatibleX509SvidTTL: false, + hasCompatibleJwtSvidTTL: true, + }, + { + msg: "default_x509_svid_ttl is small enough for the configured CA TTL but larger than the max", + caTTL: time.Hour * 24 * 7 * 4 * 6, // Six months + svidTTL: 0, + x509SvidTTL: time.Hour * 24 * 7 * 2, // Two weeks, + jwtSvidTTL: 0, + hasCompatibleSvidTTL: true, + hasCompatibleX509SvidTTL: false, + hasCompatibleJwtSvidTTL: true, + }, + { + msg: "default_jwt_svid_ttl is small enough for the default CA TTL", + caTTL: 0, + svidTTL: 0, + x509SvidTTL: 0, + jwtSvidTTL: time.Hour * 3, + hasCompatibleSvidTTL: true, + hasCompatibleX509SvidTTL: true, + hasCompatibleJwtSvidTTL: true, + }, + { + msg: "default_jwt_svid_ttl is not small enough for the default CA TTL", + caTTL: 0, + svidTTL: 0, + x509SvidTTL: 0, + jwtSvidTTL: time.Hour * 24, + hasCompatibleSvidTTL: true, + hasCompatibleX509SvidTTL: true, + hasCompatibleJwtSvidTTL: false, + }, + { + msg: "default_jwt_svid_ttl is small enough for the configured CA TTL", + caTTL: time.Hour * 24, + svidTTL: 0, + x509SvidTTL: 0, + jwtSvidTTL: time.Hour * 1, + hasCompatibleSvidTTL: true, + hasCompatibleX509SvidTTL: true, + hasCompatibleJwtSvidTTL: true, + }, + { + msg: "default_jwt_svid_ttl is not small enough for the configured CA TTL", + caTTL: time.Hour * 24, + svidTTL: 0, + x509SvidTTL: 0, + jwtSvidTTL: time.Hour * 23, + hasCompatibleSvidTTL: true, + hasCompatibleX509SvidTTL: true, + hasCompatibleJwtSvidTTL: false, + }, + { + msg: "default_jwt_svid_ttl is larger than the configured CA TTL", + caTTL: time.Hour * 24, + svidTTL: 0, + x509SvidTTL: 0, + jwtSvidTTL: time.Hour * 25, + hasCompatibleSvidTTL: true, + hasCompatibleX509SvidTTL: true, + hasCompatibleJwtSvidTTL: false, + }, + { + msg: "default_jwt_svid_ttl is small enough for the configured CA TTL but larger than the max", + caTTL: time.Hour * 24 * 7 * 4 * 6, // Six months + svidTTL: 0, + x509SvidTTL: 0, + jwtSvidTTL: time.Hour * 24 * 7 * 2, // Two weeks,, + hasCompatibleSvidTTL: true, + hasCompatibleX509SvidTTL: true, + hasCompatibleJwtSvidTTL: false, + }, + { + msg: "all default svid_ttls are small enough for the configured CA TTL", + caTTL: time.Hour * 24, + svidTTL: time.Hour * 1, + x509SvidTTL: time.Hour * 1, + jwtSvidTTL: time.Hour * 1, + hasCompatibleSvidTTL: true, + hasCompatibleX509SvidTTL: true, + hasCompatibleJwtSvidTTL: true, }, } @@ -1633,15 +1685,17 @@ func TestHasCompatibleTTLs(t *testing.T) { if testCase.svidTTL == 0 { testCase.svidTTL = ca.DefaultX509SVIDTTL } - if testCase.x509SvidTtl == 0 { - testCase.x509SvidTtl = ca.DefaultX509SVIDTTL + if testCase.x509SvidTTL == 0 { + testCase.x509SvidTTL = ca.DefaultX509SVIDTTL } - if testCase.jwtSvidTtl == 0 { - testCase.jwtSvidTtl = ca.DefaultJWTSVIDTTL + if testCase.jwtSvidTTL == 0 { + testCase.jwtSvidTTL = ca.DefaultJWTSVIDTTL } t.Run(testCase.msg, func(t *testing.T) { - require.Equal(t, testCase.hasCompatibleTTLs, hasCompatibleTTLs(testCase.caTTL, testCase.svidTTL, testCase.x509SvidTtl, testCase.jwtSvidTtl)) + require.Equal(t, testCase.hasCompatibleSvidTTL, hasCompatibleTTL(testCase.caTTL, testCase.svidTTL)) + require.Equal(t, testCase.hasCompatibleX509SvidTTL, hasCompatibleTTL(testCase.caTTL, testCase.x509SvidTTL)) + require.Equal(t, testCase.hasCompatibleJwtSvidTTL, hasCompatibleTTL(testCase.caTTL, testCase.jwtSvidTTL)) }) } } diff --git a/doc/SPIRE101.md b/doc/SPIRE101.md index e39aa855fa4..79e50533e82 100644 --- a/doc/SPIRE101.md +++ b/doc/SPIRE101.md @@ -66,8 +66,6 @@ If you don't already have Docker installed, please follow these [installation in trust_domain = "example.org" data_dir = "./.data" log_level = "DEBUG" - default_x509_svid_ttl = "1h" - default_jwt_svid_ttl = "5m" ca_subject { country = ["US"] organization = ["SPIFFE"] diff --git a/pkg/agent/manager/cache/cache_test.go b/pkg/agent/manager/cache/cache_test.go index 7f13d16c308..56f4a9aead5 100644 --- a/pkg/agent/manager/cache/cache_test.go +++ b/pkg/agent/manager/cache/cache_test.go @@ -24,7 +24,6 @@ var ( bundleV3 = bundleutil.BundleFromRootCA(trustDomain1, &x509.Certificate{Raw: []byte{3}}) otherBundleV1 = bundleutil.BundleFromRootCA(trustDomain2, &x509.Certificate{Raw: []byte{4}}) otherBundleV2 = bundleutil.BundleFromRootCA(trustDomain2, &x509.Certificate{Raw: []byte{5}}) - defaultTTL = int32(600) defaultX509SvidTTL = int32(700) defaultJwtSvidTTL = int32(800) ) @@ -489,7 +488,7 @@ func TestCheckSVIDCallback(t *testing.T) { return false }) - foo := makeRegistrationEntryWithTTL("FOO", 60, 70, 80) + foo := makeRegistrationEntryWithTTL("FOO", 70, 80) // called once for FOO with no SVID callCount := 0 @@ -538,7 +537,7 @@ func TestCheckSVIDCallback(t *testing.T) { func TestGetStaleEntries(t *testing.T) { cache := newTestCache() - foo := makeRegistrationEntryWithTTL("FOO", 60, 70, 80) + foo := makeRegistrationEntryWithTTL("FOO", 70, 80) // Create entry but don't mark it stale cache.UpdateEntries(&UpdateEntries{ @@ -798,14 +797,14 @@ func makeRegistrationEntry(id string, selectors ...string) *common.RegistrationE } } -func makeRegistrationEntryWithTTL(id string, ttl int32, x509SvidTtl int32, jwtSvidTtl int32, selectors ...string) *common.RegistrationEntry { +func makeRegistrationEntryWithTTL(id string, x509SvidTTL int32, jwtSvidTTL int32, selectors ...string) *common.RegistrationEntry { return &common.RegistrationEntry{ EntryId: id, SpiffeId: "spiffe://domain.test/" + id, Selectors: makeSelectors(selectors...), DnsNames: []string{fmt.Sprintf("name-%s", id)}, - X509SvidTtl: x509SvidTtl, - JwtSvidTtl: jwtSvidTtl, + X509SvidTtl: x509SvidTTL, + JwtSvidTtl: jwtSvidTTL, } } diff --git a/pkg/agent/manager/cache/lru_cache_test.go b/pkg/agent/manager/cache/lru_cache_test.go index 681998b8ebc..520717b811c 100644 --- a/pkg/agent/manager/cache/lru_cache_test.go +++ b/pkg/agent/manager/cache/lru_cache_test.go @@ -471,7 +471,7 @@ func TestLRUCacheCheckSVIDCallback(t *testing.T) { return false }) - foo := makeRegistrationEntryWithTTL("FOO", 60, 70, 80) + foo := makeRegistrationEntryWithTTL("FOO", 70, 80) cache.UpdateEntries(&UpdateEntries{ Bundles: makeBundles(bundleV2), @@ -509,7 +509,7 @@ func TestLRUCacheCheckSVIDCallback(t *testing.T) { func TestLRUCacheGetStaleEntries(t *testing.T) { cache := newTestLRUCache() - bar := makeRegistrationEntryWithTTL("BAR", 120, 13, 140, "B") + bar := makeRegistrationEntryWithTTL("BAR", 130, 140, "B") // Create entry but don't mark it stale from checkSVID method; // it will be marked stale cause it does not have SVID cached diff --git a/pkg/common/telemetry/names.go b/pkg/common/telemetry/names.go index 3b03482099b..7504a531267 100644 --- a/pkg/common/telemetry/names.go +++ b/pkg/common/telemetry/names.go @@ -485,13 +485,13 @@ const ( // with other tags to add clarity TTL = "ttl" - // X509 TTL functionality related to a time-to-live field for X509-SVIDs; should be used + // X509 SVID TTL functionality related to a time-to-live field for X509-SVIDs; should be used // with other tags to add clarity - X509TTL = "x509_ttl" + X509SVIDTTL = "x509_svid_ttl" - // JWT TTL functionality related to a time-to-live field for JWT-SVIDs; should be used + // JWT SVID TTL functionality related to a time-to-live field for JWT-SVIDs; should be used // with other tags to add clarity - JWTTTL = "jwt_ttl" + JWTSVIDTTL = "jwt_svid_ttl" // Type tags a type Type = "type" diff --git a/pkg/server/api/entry.go b/pkg/server/api/entry.go index b6e286a43a9..e1c8bfc2927 100644 --- a/pkg/server/api/entry.go +++ b/pkg/server/api/entry.go @@ -167,14 +167,14 @@ func ProtoToRegistrationEntryWithMask(ctx context.Context, td spiffeid.TrustDoma storeSVID = e.StoreSvid } - var x509SvidTtl int32 + var x509SvidTTL int32 if mask.X509SvidTtl { - x509SvidTtl = e.X509SvidTtl + x509SvidTTL = e.X509SvidTtl } - var jwtSvidTtl int32 + var jwtSvidTTL int32 if mask.JwtSvidTtl { - jwtSvidTtl = e.JwtSvidTtl + jwtSvidTTL = e.JwtSvidTtl } return &common.RegistrationEntry{ @@ -189,7 +189,7 @@ func ProtoToRegistrationEntryWithMask(ctx context.Context, td spiffeid.TrustDoma Selectors: selectors, RevisionNumber: revisionNumber, StoreSvid: storeSVID, - X509SvidTtl: x509SvidTtl, - JwtSvidTtl: jwtSvidTtl, + X509SvidTtl: x509SvidTTL, + JwtSvidTtl: jwtSvidTTL, }, nil } diff --git a/pkg/server/api/entry/v1/service.go b/pkg/server/api/entry/v1/service.go index 7214afef008..794dc40f825 100644 --- a/pkg/server/api/entry/v1/service.go +++ b/pkg/server/api/entry/v1/service.go @@ -468,11 +468,11 @@ func fieldsFromEntryProto(ctx context.Context, proto *types.Entry, inputMask *ty } if inputMask == nil || inputMask.X509SvidTtl { - fields[telemetry.X509TTL] = proto.X509SvidTtl + fields[telemetry.X509SVIDTTL] = proto.X509SvidTtl } if inputMask == nil || inputMask.JwtSvidTtl { - fields[telemetry.JWTTTL] = proto.JwtSvidTtl + fields[telemetry.JWTSVIDTTL] = proto.JwtSvidTtl } if inputMask == nil || inputMask.FederatesWith { diff --git a/pkg/server/api/entry/v1/service_test.go b/pkg/server/api/entry/v1/service_test.go index f8837ee1dc6..94bcf59f3e7 100644 --- a/pkg/server/api/entry/v1/service_test.go +++ b/pkg/server/api/entry/v1/service_test.go @@ -1523,8 +1523,8 @@ func TestBatchCreateEntry(t *testing.T) { telemetry.Selectors: "type:value1,type:value2", telemetry.RevisionNumber: "0", telemetry.SPIFFEID: "spiffe://example.org/workload", - telemetry.X509TTL: "45", - telemetry.JWTTTL: "30", + telemetry.X509SVIDTTL: "45", + telemetry.JWTSVIDTTL: "30", telemetry.StoreSvid: "false", }, }, @@ -1550,8 +1550,8 @@ func TestBatchCreateEntry(t *testing.T) { telemetry.RevisionNumber: "0", telemetry.Selectors: "type:value", telemetry.SPIFFEID: "spiffe://example.org/malformed", - telemetry.X509TTL: "0", - telemetry.JWTTTL: "0", + telemetry.X509SVIDTTL: "0", + telemetry.JWTSVIDTTL: "0", telemetry.StoreSvid: "false", }, }, @@ -1569,8 +1569,8 @@ func TestBatchCreateEntry(t *testing.T) { telemetry.RevisionNumber: "0", telemetry.Selectors: "type:value", telemetry.SPIFFEID: "spiffe://example.org/workload2", - telemetry.X509TTL: "0", - telemetry.JWTTTL: "0", + telemetry.X509SVIDTTL: "0", + telemetry.JWTSVIDTTL: "0", telemetry.StoreSvid: "false", }, }, @@ -1662,8 +1662,8 @@ func TestBatchCreateEntry(t *testing.T) { telemetry.Selectors: "type:value1,type:value2", telemetry.RevisionNumber: "0", telemetry.SPIFFEID: "spiffe://example.org/svidstore", - telemetry.X509TTL: "0", - telemetry.JWTTTL: "0", + telemetry.X509SVIDTTL: "0", + telemetry.JWTSVIDTTL: "0", telemetry.StoreSvid: "true", }, }, @@ -1746,8 +1746,8 @@ func TestBatchCreateEntry(t *testing.T) { telemetry.RevisionNumber: "0", telemetry.Selectors: "type:value1,type:value2", telemetry.SPIFFEID: "spiffe://example.org/workload", - telemetry.X509TTL: "45", - telemetry.JWTTTL: "30", + telemetry.X509SVIDTTL: "45", + telemetry.JWTSVIDTTL: "30", telemetry.StoreSvid: "false", }, }, @@ -1783,8 +1783,8 @@ func TestBatchCreateEntry(t *testing.T) { telemetry.RevisionNumber: "0", telemetry.Selectors: "type:value1,type:value2", telemetry.SPIFFEID: "spiffe://example.org/workload", - telemetry.X509TTL: "45", - telemetry.JWTTTL: "30", + telemetry.X509SVIDTTL: "45", + telemetry.JWTSVIDTTL: "30", telemetry.StoreSvid: "false", }, }, @@ -1850,8 +1850,8 @@ func TestBatchCreateEntry(t *testing.T) { telemetry.RevisionNumber: "0", telemetry.Selectors: "type:value1", telemetry.SPIFFEID: "spiffe://example.org/bar", - telemetry.X509TTL: "45", - telemetry.JWTTTL: "30", + telemetry.X509SVIDTTL: "45", + telemetry.JWTSVIDTTL: "30", telemetry.StoreSvid: "false", }, }, @@ -1903,8 +1903,8 @@ func TestBatchCreateEntry(t *testing.T) { telemetry.Selectors: "unix:gid:1000,unix:uid:1000", telemetry.RevisionNumber: "0", telemetry.SPIFFEID: "spiffe://example.org/bar", - telemetry.X509TTL: "45", - telemetry.JWTTTL: "30", + telemetry.X509SVIDTTL: "45", + telemetry.JWTSVIDTTL: "30", telemetry.StatusCode: "AlreadyExists", telemetry.StatusMessage: "similar entry already exists", telemetry.StoreSvid: "false", @@ -1941,8 +1941,8 @@ func TestBatchCreateEntry(t *testing.T) { telemetry.Downstream: "false", telemetry.ExpiresAt: "0", telemetry.RevisionNumber: "0", - telemetry.X509TTL: "0", - telemetry.JWTTTL: "0", + telemetry.X509SVIDTTL: "0", + telemetry.JWTSVIDTTL: "0", telemetry.StoreSvid: "false", telemetry.StatusCode: "InvalidArgument", telemetry.StatusMessage: "failed to convert entry: invalid parent ID: trust domain is missing", @@ -1982,8 +1982,8 @@ func TestBatchCreateEntry(t *testing.T) { telemetry.RevisionNumber: "0", telemetry.Selectors: "type:value1,type:value2", telemetry.SPIFFEID: "spiffe://example.org/workload", - telemetry.X509TTL: "45", - telemetry.JWTTTL: "30", + telemetry.X509SVIDTTL: "45", + telemetry.JWTSVIDTTL: "30", telemetry.StoreSvid: "false", telemetry.StatusCode: "Internal", telemetry.StatusMessage: "failed to create entry: creating error", @@ -2032,8 +2032,8 @@ func TestBatchCreateEntry(t *testing.T) { telemetry.RevisionNumber: "0", telemetry.Selectors: "type:value1,type:value2", telemetry.SPIFFEID: "spiffe://example.org/workload", - telemetry.X509TTL: "45", - telemetry.JWTTTL: "30", + telemetry.X509SVIDTTL: "45", + telemetry.JWTSVIDTTL: "30", telemetry.StoreSvid: "false", telemetry.StatusCode: "Internal", telemetry.StatusMessage: "failed to convert entry: invalid SPIFFE ID: scheme is missing or invalid", @@ -2996,7 +2996,7 @@ func TestBatchUpdateEntry(t *testing.T) { }, }, { - name: "Success Update X509TTL", + name: "Success Update X509SVIDTTL", initialEntries: []*types.Entry{initialEntry}, inputMask: &types.EntryMask{ X509SvidTtl: true, @@ -3033,7 +3033,7 @@ func TestBatchUpdateEntry(t *testing.T) { telemetry.Status: "success", telemetry.Type: "audit", telemetry.RegistrationID: m[entry1SpiffeID.Path], - telemetry.X509TTL: "1000", + telemetry.X509SVIDTTL: "1000", }, }, } @@ -3259,7 +3259,7 @@ func TestBatchUpdateEntry(t *testing.T) { }, }, { - name: "Success Don't Update X509TTL", + name: "Success Don't Update X509SVIDTTL", initialEntries: []*types.Entry{initialEntry}, inputMask: &types.EntryMask{ // With this empty, the update operation should be a no-op @@ -3644,8 +3644,8 @@ func TestBatchUpdateEntry(t *testing.T) { telemetry.RevisionNumber: "0", telemetry.Selectors: "unix:uid:9999", telemetry.SPIFFEID: "spiffe://example.org/validUpdated", - telemetry.X509TTL: "400000", - telemetry.JWTTTL: "300000", + telemetry.X509SVIDTTL: "400000", + telemetry.JWTSVIDTTL: "300000", telemetry.StoreSvid: "false", }, }, @@ -3695,7 +3695,7 @@ func TestBatchUpdateEntry(t *testing.T) { telemetry.Status: "success", telemetry.Type: "audit", telemetry.RegistrationID: m[entry1SpiffeID.Path], - telemetry.X509TTL: "500000", + telemetry.X509SVIDTTL: "500000", }, }, } @@ -3775,7 +3775,7 @@ func TestBatchUpdateEntry(t *testing.T) { telemetry.Status: "success", telemetry.Type: "audit", telemetry.RegistrationID: m[entry1SpiffeID.Path], - telemetry.X509TTL: "500000", + telemetry.X509SVIDTTL: "500000", }, }, } diff --git a/pkg/server/api/svid/v1/service_test.go b/pkg/server/api/svid/v1/service_test.go index 850ae1fcca7..37778813375 100644 --- a/pkg/server/api/svid/v1/service_test.go +++ b/pkg/server/api/svid/v1/service_test.go @@ -1784,8 +1784,7 @@ func TestServiceBatchNewX509SVID(t *testing.T) { // Use entry ttl when defined ttl := test.ca.X509SVIDTTL() - switch { - case entry.X509SvidTtl != 0: + if entry.X509SvidTtl != 0 { ttl = time.Duration(entry.X509SvidTtl) * time.Second } expiresAt := now.Add(ttl) diff --git a/pkg/server/config.go b/pkg/server/config.go index e9be4be23bc..ee661c9eea1 100644 --- a/pkg/server/config.go +++ b/pkg/server/config.go @@ -59,10 +59,6 @@ type Config struct { // AgentTTL is time-to-live for agent SVIDs AgentTTL time.Duration - // SVIDTTL is default time-to-live for SVIDs - // This field is deprecated in favor of X509SVIDTTL and JWTSVIDTTL - SVIDTTL time.Duration - // X509SVIDTTL is default time-to-live for X509-SVIDs (overrides SVIDTTL) X509SVIDTTL time.Duration diff --git a/pkg/server/datastore/sqlstore/models.go b/pkg/server/datastore/sqlstore/models.go index 2aea861b1d1..17f7fdf8813 100644 --- a/pkg/server/datastore/sqlstore/models.go +++ b/pkg/server/datastore/sqlstore/models.go @@ -78,7 +78,7 @@ type RegisteredEntry struct { ParentID string `gorm:"index"` // TTL of identities derived from this entry. This field is deprecated in favor of // X509SvidTTL and JWTSvidTTL and will be removed in a future version. - // DEPRECATED: remove this in 1.6.0 + // Deprecated: remove this in 1.6.0 TTL int32 Selectors []Selector FederatesWith []Bundle `gorm:"many2many:federated_registration_entries;"` diff --git a/pkg/server/datastore/sqlstore/sqlstore.go b/pkg/server/datastore/sqlstore/sqlstore.go index 137669c62e1..1f666377bda 100644 --- a/pkg/server/datastore/sqlstore/sqlstore.go +++ b/pkg/server/datastore/sqlstore/sqlstore.go @@ -3690,9 +3690,9 @@ func modelToEntry(tx *gorm.DB, model RegisteredEntry) (*common.RegistrationEntry } // Determine appropriate X509 TTL - x509SvidTtl := model.X509SvidTTL - if x509SvidTtl <= 0 { - x509SvidTtl = model.TTL + x509SvidTTL := model.X509SvidTTL + if x509SvidTTL <= 0 { + x509SvidTTL = model.TTL } return &common.RegistrationEntry{ @@ -3700,7 +3700,7 @@ func modelToEntry(tx *gorm.DB, model RegisteredEntry) (*common.RegistrationEntry Selectors: selectors, SpiffeId: model.SpiffeID, ParentId: model.ParentID, - X509SvidTtl: x509SvidTtl, + X509SvidTtl: x509SvidTTL, FederatesWith: federatesWith, Admin: model.Admin, Downstream: model.Downstream, diff --git a/pkg/server/server.go b/pkg/server/server.go index 79a0e303afd..e700831e5bc 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -265,7 +265,8 @@ func (s *Server) loadCatalog(ctx context.Context, metrics telemetry.Metrics, ide func (s *Server) newCA(metrics telemetry.Metrics, healthChecker health.Checker) *ca.CA { return ca.NewCA(ca.Config{ Metrics: metrics, - X509SVIDTTL: s.config.SVIDTTL, + X509SVIDTTL: s.config.X509SVIDTTL, + JWTSVIDTTL: s.config.JWTSVIDTTL, JWTIssuer: s.config.JWTIssuer, TrustDomain: s.config.TrustDomain, CASubject: s.config.CASubject,