Skip to content

Split root_ca in CSR API #3480

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+154 −15
Open

Split root_ca in CSR API #3480

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 77 additions & 14 deletions security/v1alpha1/ca.pb.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

37 changes: 36 additions & 1 deletion security/v1alpha1/ca.pb.html
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions security/v1alpha1/ca.proto
View file
Original file line number Diff line number Diff line change
@@ -46,6 +46,15 @@ message IstioCertificateResponse {
// PEM-encoded certificate chain.
// The leaf cert is the first element, and the root cert is the last element.
repeated string cert_chain = 1;
// Root certificates. This field is newer, and therefor is optional with the following semantics:
// * if root_cert is specified, cert_chain contains [leaf, intermediate1, intermediate2] (intermediates are optional).
// root_cert contains [root1, root2].
// * if root_cert is not specified, cert_chain contains [leaf, intermediate1, intermediate2, root1+root2] concatenated into one entry.
// Note that the individual cert_chain is only signed by a single root. The roots provided here is the full bundle of trusted roots.
Roots root_cert = 2;
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if having a more significant addition which introduces a new structured type containing a full replacement for cert_chanin would be better/cleaner than making a small change like this. Don't make them mutually exclusive and then we could have an interop mode where cert_chain and name_tbd are both populated with the ~same information.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to that, let's introduce a real replacement struct, potentially with arrays for intermediates and roots and a separate leaf variable

Copy link
Contributor

Choose a reason for hiding this comment

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

I could get behind that

}
message Roots {
repeated string ca_cert = 1;
}

// Service for managing certificates issued by the CA.
21 changes: 21 additions & 0 deletions security/v1alpha1/ca_deepcopy.gen.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions security/v1alpha1/ca_json.gen.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.