Adds new Github actions identity issuer

[nsmith5]

Summary

Implements the new issuer abstraction for github actions OIDC issuer.

Thoughts / questions for reviewers:

• I tried to centralize the pkix extensions logic in one spot so its really easy to review / extend for folks limme know if you think that was successful
• The location of the extensions is probably the wrong package. Maybe should go in x509ca? Again, limme know what ya'll think

Ticket Link

Relates to #275

Release Note

NONE

[codecov-commenter]

Codecov Report

Merging #559 (10e3917) into main (152c20d) will increase coverage by 3.96%.
The diff coverage is 90.16%.

@@            Coverage Diff             @@
##             main     #559      +/-   ##
==========================================
+ Coverage   36.84%   40.81%   +3.96%     
==========================================
  Files          19       22       +3     
  Lines        1441     1563     +122     
==========================================
+ Hits          531      638     +107     
- Misses        851      861      +10     
- Partials       59       64       +5     

Impacted Files	Coverage Δ
pkg/identity/github/principal.go	73.91% <73.91%> (ø)
pkg/identity/github/issuer.go	89.47% <89.47%> (ø)
pkg/ca/x509ca/extensions.go	100.00% <100.00%> (ø)
pkg/ca/fileca/load.go	58.62% <0.00%> (-10.35%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 152c20d...10e3917. Read the comment docs.

[haydentherapper]

Something to consider - By only adding files without refactoring the underlying code, it's hard to confirm that no business logic is changing. I definitely prefer smaller PRs though. Maybe in the PR description, pointing to the places in the code where the logic originated?
I'm a little nervous about breaking something subtly, and we don't have great test coverage besides the large API test file so it's hard to verify that nothing has changed.

[haydentherapper]

nit: I think the rest of the code base uses ', any reason to switch to `?

[nsmith5]

All the struct tags us ` I believe so you don't need to escape the ". I'm guessing you mean my ` usage for strings instead of " elsewhere? Its just a habit of mine to use them if I'm not using escape characters but I can change that to be in keeping with the rest of the code base

[haydentherapper]

I'm wondering if, once you've added all of the base structs/functions/etc, it would be better to try and make these changes piecemeal, where you change the portion of the code that's affected at the same time as making these improvements. I would greatly prefer refactors (still concise) are done in one PR, rather than just having PRs with only additions or mostly only removals.

[nsmith5]

The problem I see is that I'd have to put something like branch by abstraction in place in front of the old implementation to cut over the issuer implementations one by one. This is probably my lack of imagination but I can't think of an easy way to get that branch by abstraction bit in place. Can you think of a strategy that would be easy to implement?

I agree this is somewhat scary because we're replacing a lot of poorly tested code, but perhaps we can integration test heavily in staging to alleviate some of those fears of regression?

[nsmith5]

I can certainly point out in each PR the bits of code that will eventually be removed because of them if that helps out. I've also been trying add really thorough test coverage for everything I've written to improve our confidence in the refactor.

[haydentherapper]

With refactoring, I prefer one refactor at a time. This refactor involves moving functions, adding an abstraction layer, adding tests, moving constants, etc. These changes in this PR can be composed slightly different with more intermediate steps - it might take a few more PRs, but it'd give reviewers more confidence that no business logic is changing.

I have a good understanding of how everything will be structured at the end from your design doc, so I don't mind a few more intermediate steps. I'd just prefer avoiding anything breaking while we move logic around, and as a reviewer, I can quickly approve PRs that are moving code since that code was previously reviewed.

For example, would you consider doing the following:

• Going back to Add new Issuer and Principal abstractions #558, move the authorize function to a function in an IssuerPool, and change the callsite where authorize is called to initialize an IssuerPool and call Authenticate. Add tests at this point too. It might require some TODOs in IssuerPool, but it clearly shows logic is just moved.
• Split this PR up. For example, you could move the logic to render the x509 extensions first. Then, move the GitHub-specific logic to its own package. It might take another PR to move it into the IssuerPool abstraction layer later, but this will just be moves rather than additions and deletions.

There's a lot of helpful refactoring tips here.

[nsmith5]

🤔 Yup ok I think I can do this better