Skip to content

add combine TLS certs feature#7609

Merged
deepthi merged 1 commit intovitessio:masterfrom
eselyavka:eseliavka/7608
Mar 10, 2021
Merged

add combine TLS certs feature#7609
deepthi merged 1 commit intovitessio:masterfrom
eselyavka:eseliavka/7608

Conversation

@eselyavka
Copy link
Contributor

@eselyavka eselyavka commented Mar 4, 2021

Description

Certificate chain (or Chain of Trust) is made up of a list of certificates that start from a server's certificate and terminate with the root certificate. If your server's certificate is to be trusted, its signature has to be traceable back to its root CA. However in case of Vitess we cannot see the full list of certificates. The change is to pass the full list of certs.

Related Issue(s)

#7608

Checklist

  • Should this PR be backported?
  • Tests were added or are not required
  • Documentation was added or is not required

Deployment Notes

Tested in my local env.

Impacted Areas in Vitess

Components that this PR will affect:

  • Query Serving
  • VReplication
  • Cluster Management
  • Build/CI
  • VTAdmin

Copy link
Collaborator

@deepthi deepthi left a comment

Choose a reason for hiding this comment

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

I have some comments, but we'll need a more thorough review from one of the others.

@deepthi deepthi requested review from aquarapid and deepthi March 4, 2021 20:25
@eselyavka eselyavka force-pushed the eseliavka/7608 branch 2 times, most recently from 0037680 to 136465b Compare March 4, 2021 22:55
@harshit-gangal
Copy link
Member

While we review your PR, could you please remove the issue id MVP-53256 from the commit message. This looks unrelated to Vitess Github Issue.

@eselyavka eselyavka changed the title [MVP-53256]: add combine TLS certs feature add combine TLS certs feature Mar 5, 2021
@eselyavka
Copy link
Contributor Author

eselyavka commented Mar 5, 2021

While we review your PR, could you please remove the issue id MVP-53256 from the commit message. This looks unrelated to Vitess Github Issue.
@harshit-gangal Done, squash commits with new message + updated header.

Copy link
Member

@harshit-gangal harshit-gangal left a comment

Choose a reason for hiding this comment

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

LGTM

@deepthi
Copy link
Collaborator

deepthi commented Mar 9, 2021

Can you resolve the merge conflicts? Please choose the require.NoError version of the check instead of t.Fatalf.

Signed-off-by: Evgenii Seliavka <eseliavka@twitter.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants