Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DRAFT] Create new-idp-requirements.md #1447

Merged
merged 3 commits into from Dec 15, 2023
Merged

Conversation

pwelch
Copy link
Member

@pwelch pwelch commented Nov 2, 2023

Summary

Starting a new document to capture the requirements for adding a new IDP to Sigstore Public Deployment as proposed in #397

Release Note

  • Add New IDP Requirements documentation

Documentation

@pwelch pwelch marked this pull request as ready for review November 2, 2023 20:13
Copy link

codecov bot commented Nov 2, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a376f7b) 57.67% compared to head (f3cacbc) 57.58%.
Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1447      +/-   ##
==========================================
- Coverage   57.67%   57.58%   -0.10%     
==========================================
  Files          50       50              
  Lines        3112     3112              
==========================================
- Hits         1795     1792       -3     
- Misses       1158     1160       +2     
- Partials      159      160       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

docs/new-idp-requirements.md Show resolved Hide resolved
docs/new-idp-requirements.md Show resolved Hide resolved
docs/new-idp-requirements.md Show resolved Hide resolved
docs/new-idp-requirements.md Outdated Show resolved Hide resolved
docs/new-idp-requirements.md Outdated Show resolved Hide resolved
docs/new-idp-requirements.md Outdated Show resolved Hide resolved
docs/new-idp-requirements.md Show resolved Hide resolved
docs/new-idp-requirements.md Outdated Show resolved Hide resolved
docs/new-idp-requirements.md Outdated Show resolved Hide resolved
docs/new-idp-requirements.md Outdated Show resolved Hide resolved
@pwelch
Copy link
Member Author

pwelch commented Nov 30, 2023

@haydentherapper This one is ready for another round of reviews. I think I got everything but let me know if I missed something or if anything needs more details.

Creating a new document to capture the requirements for adding a new IDP to Sigstore Public Deployment

Signed-off-by: Paul Welch <pwelch@github.com>
Signed-off-by: Paul Welch <pwelch@github.com>
Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

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

Thank you so much for writing this! Just a few last comments, this looks great.


You should also reference the [Fulcio - ODIC.md](https://github.com/sigstore/fulcio/blob/main/docs/oidc.md) documentation for additional requirements for the type of IDP you're looking to integrate. The current two likely types of IDPs are:

- `Email` - Email-based OIDC providers use the user’s email as the subject of the certificate.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we change this to also mention service accounts? It can either be user-based authentication or machine identity for SAs

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I captured what you're looking for but let me know if not

docs/new-idp-requirements.md Outdated Show resolved Hide resolved
docs/new-idp-requirements.md Show resolved Hide resolved
docs/new-idp-requirements.md Outdated Show resolved Hide resolved
Signed-off-by: Paul Welch <pwelch@github.com>
Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

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

This looks great, thanks again for writing this up!

@haydentherapper haydentherapper enabled auto-merge (squash) December 15, 2023 18:08
@haydentherapper haydentherapper merged commit f8fbeff into sigstore:main Dec 15, 2023
13 checks passed
@pwelch pwelch deleted the patch-1 branch December 15, 2023 19:26
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.

None yet

2 participants