Add JWT handling to spiffe package

[jjcollinge]

Signed-off-by: Jonathan Collinge [email protected]

Description

This PR extends the spiffecontext package to support both X.509 SVID and JWT SVID sources. This will enable usages of this package as we add support for JWT Spiffe identity in the Dapr runtime.

The expectation is that Sentry will be updated so SignCertificateResponse will contain a JWT in addition to the existing x.509 certificate. Thus the RequestSVIDFn will be able to return both a JWT and x.509.

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #[issue number]

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

• Code compiles correctly
• Created/updated tests

[jjcollinge]

tests passing in CI on linux and locally on darwin... maybe a timing issue? Can someone re-run?

[JoshVanL]

whoop

[jjcollinge]

The tests is failing in CI on darwin but passing locally and in linux_arm, is this not a race condition in the existing tests?

--- FAIL: Test_Run (0.05s)
    --- FAIL: Test_Run/should_renew_certificate_when_it_has_expired (0.01s)
        spiffe_test.go:184: 
            	Error Trace:	/Users/runner/work/kit/kit/crypto/spiffe/spiffe_test.go:184
            	Error:      	readyCh should not be closed
            	Test:       	Test_Run/should_renew_certificate_when_it_has_expired

ctx, cancel := context.WithCancel(context.Background())
errCh := make(chan error)
go func() {
         // This will fetch the identity and then close the channel if error or success
	errCh <- s.Run(ctx)
}()

select {
// if the Run is succeeded or failed by the time we run this then the channel will be closed?
case <-s.readyCh:
	assert.Fail(t, "readyCh should not be closed")
default:
}

[JoshVanL]

Few more things from me, but is looking good.

[JoshVanL]

@jjcollinge linter failing

  crypto/spiffe/spiffe.go:262:10  revive  indent-error-flow: if block ends with a return statement, so drop this else and outdent its block

[mikeee]

@jjcollinge Looks like the deprecation comments state the method signature as FromX509 but are defined as X509From, assuming the latter is more correct and is what we're running with or is this going to be refactored at a later date?

[jjcollinge]

@jjcollinge Looks like the deprecation comments state the method signature as FromX509 but are defined as X509From, assuming the latter is more correct and is what we're running with or is this going to be refactored at a later date?

Good spot, that's an oversight on my part after a rename. Here's the fix #125