feat: adds 3-level certificate chain generation script #165
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@yogeshbdeshpande @setrofim @thomas-fossati Please take a look when get a chance. |
setrofim
reviewed
Feb 11, 2025
Akhilesh-max
force-pushed
the
include-certs
branch
from
5be2f7a to
2d5c5c3
Compare
|
@thomas-fossati @setrofim Addressed all the comments above. PTAL. |
Akhilesh-max
pushed a commit
to Akhilesh-max/corim
that referenced
this pull request
Feb 11, 2025
Signed-off-by: Akhilesh Kr. Yadav <akhileshkr.yadav@Akhileshs-MacBook-Air.local>
Akhilesh-max
commented
Feb 11, 2025
| -C Do not clean up intermediate artifacts (e.g., CSRs). | ||
|
|
||
| Note: Regenerating the certificate chain is an exceptional action and should | ||
| only be done when necessary (e.g., when certificates expire). |
There was a problem hiding this comment.
@thomas-fossati
wrt https://github.com/veraison/corim/pull/165/files#r1951084868
Is it the way we wanted it?
There was a problem hiding this comment.
You should also update the help message in the Makefile. And maybe use "regenerate" rather than "generate" as the certs are already generated, so normally there would be no need to generate them again.
Akhilesh-max
pushed a commit
to Akhilesh-max/corim
that referenced
this pull request
Feb 11, 2025
Signed-off-by: Akhilesh Kr. Yadav <akhileshkr.yadav@Akhileshs-MacBook-Air.local>
Makefile
Outdated
| @@ -58,3 +63,4 @@ help: | |||
| @echo " * presubmit: check you are ready to push your local branch to remote" | |||
| @echo " * help: print this menu" | |||
| @echo " * licenses: check licenses of dependent packages" | |||
| @echo " * certs: generate the certificate chain" | |||
There was a problem hiding this comment.
Suggested change
| @echo " * certs: generate the certificate chain" | |
| @echo " * test-certs: generate the certificate chain" |
Akhilesh-max
pushed a commit
to Akhilesh-max/corim
that referenced
this pull request
Feb 11, 2025
Signed-off-by: Akhilesh Kr. Yadav <akhileshkr.yadav@Akhileshs-MacBook-Air.local> Addressing comments on veraison#165 Signed-off-by: Akhilesh Kr. Yadav <akhileshkr.yadav@Akhileshs-MacBook-Air.local> Addressing comments on veraison#165 Signed-off-by: Akhilesh Kr. Yadav <akhileshkr.yadav@Akhileshs-MacBook-Air.local> Addressing comments on veraison#165 Signed-off-by: Akhilesh Kr. Yadav <akhileshkr.yadav@Akhileshs-MacBook-Air.local>
Akhilesh-max
force-pushed
the
include-certs
branch
from
d21af02 to
751e20d
Compare
thomas-fossati
approved these changes
Feb 11, 2025
|
@setrofim PTAL. Thanks! |
Makefile
Outdated
| @@ -58,3 +63,4 @@ help: | |||
| @echo " * presubmit: check you are ready to push your local branch to remote" | |||
| @echo " * help: print this menu" | |||
| @echo " * licenses: check licenses of dependent packages" | |||
| @echo " * test-certs: generate the certificate chain" | |||
There was a problem hiding this comment.
is the rendering on line 66 off, please check!
Also for re-generate, is it going to be the same command, then please clarify in the message
scripts/gen-certs.sh
Outdated
| create | ||
| Create the root, intermediate, and end-entity certificates. | ||
|
|
||
| clean |
There was a problem hiding this comment.
It is not clear the scope of clean to me?
Is it all certs, then please specify clearly..
There was a problem hiding this comment.
It should have been clean_certs_artifacts. Updated accordingly.
Akhilesh-max
pushed a commit
to Akhilesh-max/corim
that referenced
this pull request
Feb 12, 2025
Signed-off-by: Akhilesh Kr. Yadav <akhileshkr.yadav@Akhileshs-MacBook-Air.local> Addressing comments on veraison#165 Signed-off-by: Akhilesh Kr. Yadav <akhileshkr.yadav@Akhileshs-MacBook-Air.local> Addressing comments on veraison#165 Signed-off-by: Akhilesh Kr. Yadav <akhileshkr.yadav@Akhileshs-MacBook-Air.local> Addressing comments on veraison#165 Signed-off-by: Akhilesh Kr. Yadav <akhileshkr.yadav@Akhileshs-MacBook-Air.local> Addressing comments on veraison#165 Signed-off-by: Akhilesh Kr. Yadav <akhileshkr.yadav@Akhileshs-MacBook-Air.local>
Akhilesh-max
force-pushed
the
include-certs
branch
from
751e20d to
2a38dda
Compare
Signed-off-by: Akhilesh Kr. Yadav <akhileshkr.yadav@Akhileshs-MacBook-Air.local> Addressing comments on veraison#165 Signed-off-by: Akhilesh Kr. Yadav <akhileshkr.yadav@Akhileshs-MacBook-Air.local> Addressing comments on veraison#165 Signed-off-by: Akhilesh Kr. Yadav <akhileshkr.yadav@Akhileshs-MacBook-Air.local> Addressing comments on veraison#165 Signed-off-by: Akhilesh Kr. Yadav <akhileshkr.yadav@Akhileshs-MacBook-Air.local> Addressing comments on veraison#165 Signed-off-by: Akhilesh Kr. Yadav <akhileshkr.yadav@Akhileshs-MacBook-Air.local> Addressing comments on veraison#165 Signed-off-by: Akhilesh Kr. Yadav <akhileshkr.yadav@Akhileshs-MacBook-Air.local>
Akhilesh-max
force-pushed
the
include-certs
branch
from
2a38dda to
c454eae
Compare
|
@yogeshbdeshpande @setrofim Updated as per the reviews. |
yogeshbdeshpande
approved these changes
Feb 12, 2025
setrofim
approved these changes
Feb 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes -- #164