feat: Added TLS options for git sources with self-signed certificates #2443
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2443 +/- ##
==========================================
- Coverage 70.76% 70.72% -0.05%
==========================================
Files 81 81
Lines 8110 8139 +29
==========================================
+ Hits 5739 5756 +17
- Misses 2025 2036 +11
- Partials 346 347 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Looking great! Thanks for taking this on @mattiaforc !! Also really appreciate you updating the JSON and Cue schemas!
Couple minor suggestions, but will defer to @GeorgeMac for the ✅
internal/storage/fs/git/source.go
Outdated
| } | ||
|
|
||
| // CaBundleFromFile tries to load an x.509 CA certificate from a path | ||
| func CaBundleFromFile(caPath string) ([]byte, error) { |
There was a problem hiding this comment.
doesnt look like theres any thing CA specific for this func as its really just wrapping os.ReadFile, we could probably just remove this and call os.ReadFile directly in grpc.go:
config/flipt.schema.cue
Outdated
| ca_cert_path: string | ||
| ca_cert_bytes: string |
There was a problem hiding this comment.
Could you make these two optional (CUE fields are required by default):
| ca_cert_path: string | |
| ca_cert_bytes: string | |
| ca_cert_path?: string | |
| ca_cert_bytes?: string |
config/flipt.schema.cue
Outdated
| poll_interval?: =~#duration | *"30s" | ||
| ca_cert_path: string | ||
| ca_cert_bytes: string | ||
| insecure_skip_tls?: bool | false |
There was a problem hiding this comment.
If you add a * here then CUE will default to false (when not supplied). Otherwise, this is just a type assertion and the | false bit is redundent.
| insecure_skip_tls?: bool | false | |
| insecure_skip_tls?: bool | *false |
Introduces storage.git.insecure_skip_tls, storage.git.ca_cert_path and storage.git.ca_cert_bytes config for allowing users to use git sources with self-signed certificates, either via disabling TLS (not recommended in production) or via CA bundles - that can be a path to a x.509 file or the whole certificate bytes. Refs: issue #2423
…cert_path. Added new validation error when both ca_cert_bytes and ca_cert_path are specified and small refactor
|
Hi @markphelps @GeorgeMac, thanks for the suggestions, I just pushed the changes, let me know if it's all good! |
ignore this, doesn't look like its related to your changes. digging in now |
|
@all-contributors please add @mattiaforc for code |
|
I've put up a pull request to add @mattiaforc! 🎉 |
|
Thanks again @mattiaforc !! Looks great 🎉 |
|
It was a pleasure! |
|
Will create a release later today with this in it |
…ed certificates Refs: flipt-io#2443
…ed certificates Refs: flipt-io#2443
Introduces
storage.git.insecure_skip_tls,storage.git.ca_cert_pathandstorage.git.ca_cert_bytesconfig for allowing users to use git sources with self-signed certificates, either via disabling TLS (not recommended in production) or via CA bundles - that can be a path to ax.509file or the whole certificate bytes.Refs: issue #2423