Skip to content

Update comment for credentialName#786

Merged
wenchenglu merged 2 commits intoistio:release-1.1from
JimmyCYJ:release-1.1
Feb 6, 2019
Merged

Update comment for credentialName#786
wenchenglu merged 2 commits intoistio:release-1.1from
JimmyCYJ:release-1.1

Conversation

@JimmyCYJ
Copy link
Copy Markdown
Member

@JimmyCYJ JimmyCYJ commented Feb 6, 2019

Update comment for credentialName.
When customer specifies credentialName (e.g. "foo.credential"), and use MUTUAL TLS.
Gateway workloads will send SDS requests with resource name "foo.credential" to retrieve serverCertificate and privateKey, and send SDS requests with resource name "foo.credential-cacert" to retrieve CaCertificates. In Kubernetes, customer needs to create a Kubernetes secret named "foo.credential", which contains server certificate, the private key, and the CA certificate.
If customer specifies credentialName (e.g. "foo.credential"), and use SIMPLE TLS. Gateway workload will only send SDS requests with resource name "foo.credential" to retrieve serverCertificate and privateKey. And the Kubernetes secret named "foo.credential" only contains server certificate and private key.

@JimmyCYJ JimmyCYJ requested a review from rshriram February 6, 2019 19:57
@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Feb 6, 2019
@istio-testing
Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: JimmyCYJ
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: rshriram

If they are not already assigned, you can assign the PR to them by writing /assign @rshriram in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@JimmyCYJ JimmyCYJ removed request for geeknoid and hklai February 6, 2019 19:57
// using the file system paths specified above. If using mutual TLS,
// gateway workloads will retrieve the CaCertificates using
// credentialName-cacert. The semantics of the name are platform dependent.
// In Kubernetes, the default Istio supplied credentail server expects the
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

typo: credential

// credentialName-cacert. The semantics of the name are platform dependent.
// In Kubernetes, the default Istio supplied credentail server expects the
// credentialName to match the name of the Kubernetes secret that holds the
// server certificate, the private key, and the CA certificate
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

and if using mTLS, "credentialName-cacert" to match the name of the Kubernetes secret that holds the CA certificate.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If using mTLS, customer only creates one k8s secret with credentialName (e.g. "foo.credential"), Pilot creates two SDS configs for gateway listener, one is named "foo.credential", and is used to fetch key and cert. The other is named "foo.credential-cacert", and is used to fetch certificate validation context that has CA cert. So I think the comment is okay.

Copy link
Copy Markdown
Member

@rshriram rshriram left a comment

Choose a reason for hiding this comment

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

please fix typo and address minor comment

@wenchenglu wenchenglu merged commit 1a129f0 into istio:release-1.1 Feb 6, 2019
@JimmyCYJ JimmyCYJ deleted the release-1.1 branch February 6, 2019 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants