8000 Make it possible to mock the current time in spiffetls dialer by vandry · Pull Request #357 · spiffe/go-spiffe · GitHub
[go: up one dir, main page]

Skip to content

Conversation

vandry
Copy link
@vandry vandry commented Sep 6, 2025

So you can do, say,

spiffetls.DialWithMode(
ctx, network, addr, mode,
spiffetls.WithDialTLSOptions(tlsconfig.WithTime(mocked))
)

This is useful in tests and is already supported by the underlying crypto/x509 library and even by the x509svid package right here but just wasn't plumbed through to the dialer.

So you can do, say,

  spiffetls.DialWithMode(
      ctx, network, addr, mode,
      spiffetls.WithDialTLSOptions(tlsconfig.WithTime(mocked))
  )

This is useful in tests and is already supported by the underlying
crypto/x509 library and even by the x509svid package right here but just
wasn't plumbed through to the dialer.
Copy link
Member
@azdagron azdagron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for opening this @vandry and sorry for the review latency. This seems like good functionality to support. I've made a suggestion on how we accomplish this to be more consistent with the API in this and other packages.


// WithTime sets the time used when verifying validity periods on X509 SVIDs.
// If not used, the current time will be used.
func WithTime(now time.Time) Option {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be consistent in the way we've passed through options in other packages, I'd recommend that we instead add a function func WithVerifyOptions(opts ...x509svid.VerifyOption) Option instead of replicating the individual options here.

@azdagron
Copy link
Member

Also, we'll need DCO signoff an all commits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0