spire icon indicating copy to clipboard operation
spire copied to clipboard

Use go-spiffe v2 types

Open azdagron opened this issue 4 years ago • 4 comments

The go-spiffe v2 library introduced strong types for SPIFFE IDs, trust domains, and bundles. Using these types across the SPIRE codebase would provide uniformity and reduce opportunity for bugs and mishandling.

This work should probably be bitten off in small chunks in order to keep things scoped and auditable.

azdagron avatar Sep 08 '20 20:09 azdagron

I've tackled most of the agent side while I've been working through #1847. There are a LOT of side effects to changing these types over, particularly in the boundary between units and often more significantly, unit tests. I'd suggest this work be tackled unit-by-unit, otherwise it will be quite hard to review :)

azdagron avatar Nov 24 '20 17:11 azdagron

I'm tackling the server units ca and endpoints and probably will need to tackle some side-effects on related units.

maxlambrecht avatar Nov 25 '20 17:11 maxlambrecht

Closing #2232 Clarify type and usage by using the appropriate spiffeid types (e.g. spiffeid.TrustDomain and spiffeid.ID) in favor of tracking here.

bri365 avatar Apr 20 '21 19:04 bri365

I think most of the server code is done. We still need to go through the agent side, particularly the use of the bundleutil package.

azdagron avatar Oct 11 '22 20:10 azdagron

Hey there, I would like to contribute by continuing to resolve this issue, could it be assigned to me, please?

guilhermocc avatar Jan 11 '23 18:01 guilhermocc

Awesome! There are bound to be a lot of changes. Where possible, in order to prevent review fatigue, I suggest sending a PR for one unit at a time.

azdagron avatar Jan 12 '23 14:01 azdagron

So far, I've mapped the following agent units that need to be updated to use go-spiffe types:

  • api [✔️]
  • attestor [✔️ ]
    • node: using bundleutil.Bundle in AttestationResult struct. https://github.com/spiffe/spire/pull/3834
    • workload [✔️]
  • catalog [✔️]
  • client [:heavy_check_mark: ]: Use custom types X509SVID and JWTSVID; check if we need/can replace them with go-spiffe types.
  • common [✔️]
  • endpoints [✔️ ] https://github.com/spiffe/spire/pull/3914
    • sdsv2: Using bundleutil.Bundle type
    • sdsv3: Using bundleutil.Bundle type
    • workload: Using bundleutil.Bundle type
  • manager [✔️]
    • cache: Using bundleutil.Bundle type #4013
  • plugin [✔️ ]
  • storage [✔️]
  • svid [🎯] : Using bundleutil.Bundle type
  • workloadkey [✔️]

I will proceed updating this list as I go through remaining units.

guilhermocc avatar Feb 08 '23 14:02 guilhermocc