Skip to content
New issue

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

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

Already on GitHub? # to your account

Support TLS Server/Client certificates read from a file, and refreshe… #149

Merged
merged 1 commit into from
Nov 3, 2022

Conversation

joeljeske
Copy link
Contributor

@joeljeske joeljeske commented Oct 28, 2022

…d on an interval.

Incompatible Changes:

This change moves the existing {client,server}_certificate and {client,server}_private_key into {client,server}_key_pair.inline.certificate and {client,server}_key_pair.inline.private_key

This change adds an alternate strategy for specifying certificates/private keys by using {client,server}_key_pair.files.certificate_path and {client,server}_key_pair.files.private_key_path which should reference PEM files on disk. A refresh_interval must also be specified to dictate the interval at which the files should be read and used in place of the existing certificate/key if such files have been changed.

In kubernetes for example, this allows a side-car to rotate certificate based on user's needs.

pkg/proto/configuration/tls/tls.proto Outdated Show resolved Hide resolved
pkg/proto/configuration/tls/tls.proto Outdated Show resolved Hide resolved
@joeljeske joeljeske force-pushed the tls-rotate branch 4 times, most recently from c9e9456 to 5f49aeb Compare October 28, 2022 20:11
@joeljeske joeljeske force-pushed the tls-rotate branch 2 times, most recently from 9b7928e to a732c0e Compare October 28, 2022 20:25
@joeljeske joeljeske marked this pull request as ready for review October 29, 2022 11:01
pkg/util/tls_certificate.go Outdated Show resolved Hide resolved
pkg/util/tls.go Outdated Show resolved Hide resolved
Copy link
Member

@EdSchouten EdSchouten left a comment

Choose a reason for hiding this comment

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

Looking slick, Joel!

pkg/util/tls.go Outdated Show resolved Hide resolved
pkg/util/tls.go Outdated Show resolved Hide resolved
pkg/util/tls_certificate.go Outdated Show resolved Hide resolved
@@ -207,6 +267,14 @@ func TestTLSConfigFromClientConfiguration(t *testing.T) {
}

func TestTLSConfigFromServerConfiguration(t *testing.T) {
tempDir := t.TempDir()
exampleCertFile := tempDir + "/example-cert.pem"
Copy link
Member

Choose a reason for hiding this comment

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

filepath.Join() ?

Copy link
Member

Choose a reason for hiding this comment

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

Whoops! It looks like we can also use filepath.Join() here.

@joeljeske
Copy link
Contributor Author

Thanks Ed! Very helpful tips, I appreciate it. Changes are made! :)

Copy link
Member

@EdSchouten EdSchouten left a comment

Choose a reason for hiding this comment

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

One small change requested, and then we're good with merging this.

@@ -207,6 +267,14 @@ func TestTLSConfigFromClientConfiguration(t *testing.T) {
}

func TestTLSConfigFromServerConfiguration(t *testing.T) {
tempDir := t.TempDir()
exampleCertFile := tempDir + "/example-cert.pem"
Copy link
Member

Choose a reason for hiding this comment

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

Whoops! It looks like we can also use filepath.Join() here.

…d on an interval.

Incompatible Changes:

This change moves the existing `{client,server}_certificate` and `{client,server}_private_key` into `{client,server}_key_pair_data.certificate`  and `{client,server}_key_pair_data.private_key`

This change adds an alternate strategy for specifying certificates/private keys by using `{client,server}_key_pair_files.certificate` and `{client,server}_key_pair_files.private_key`  which should reference PEM files on disk. A refresh_interval must also be specified to dictate the interval at which the files should be read and used in place of the existing certificate/key if such files have been changed.

In kubernetes for example, this allows a side-car to rotate certificate based on user's needs.
@joeljeske
Copy link
Contributor Author

Ah! So sorry, fixed now :)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants