feat(cli): support inline CA certificates in gRPC connections and config#2721
Conversation
Store CA certificate content (base64-encoded) in config instead of file paths, enabling portable configurations across environments. The gRPC connection layer now accepts CA content directly via WithCAContent option. Signed-off-by: Javier Rodriguez <javier@chainloop.dev>
javirln
changed the title
Add unit tests for CA certificate loading functionality including file path detection, PEM content loading, base64-encoded content, and option functions. Tests verify backward compatibility with file paths and new inline content support. Signed-off-by: Javier Rodriguez <javier@chainloop.dev>
Signed-off-by: Javier Rodriguez <javier@chainloop.dev>
Yes, it is. I've added tests for that. It will attempt to load the path, and if successful, store it as base64. Otherwise, It will try to load the base64 directly. |
I don't mean the storing, but the consumption of an already stored path, just to confirm, is that covered? |
Add tests verifying that new clients can consume old configurations with stored file paths, and that the file path detection correctly routes to the legacy loading method. Addresses PR feedback on backward compatibility. Signed-off-by: Javier Rodriguez <javier@chainloop.dev>
Yes, it's backwards compatible with CAs as paths stored in the configuration from all the tests I could ran. |
Signed-off-by: Javier Rodriguez <javier@chainloop.dev>
| if caFilePath := viper.GetString(confOptions.controlplaneCA.viperKey); caFilePath != "" { | ||
| opts = append(opts, grpcconn.WithCAFile(caFilePath)) | ||
| if caValue := viper.GetString(confOptions.controlplaneCA.viperKey); caValue != "" { | ||
| // Check if the value is a file path, if it is we read the content and encode it to base64, if not we assume it's the content already |
There was a problem hiding this comment.
Should it use different keys instead of trying to do a os.Stat? Not sure about the security consequences of this.
There was a problem hiding this comment.
If we use different keys, which would take precedence then?
There was a problem hiding this comment.
As far as I remember os.Stat is safe to use because it does not open the file descriptor. It calls directly the underlying OS to read just the file attributes
Signed-off-by: Javier Rodriguez <javier@chainloop.dev>
migmartri
left a comment
migmartri
left a comment
There was a problem hiding this comment.
Just double check what would happen with the ee CLI, if it will keep working
| }, | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| // Process CA flags - read file contents and encode to base64 if needed | ||
| if err := processCAFlag(confOptions.controlplaneCA); err != nil { |
There was a problem hiding this comment.
btw, how's is this change gonna affect the platform CLI?
There was a problem hiding this comment.
As we do in other commands being inherited from OSS, we will extend the config save on EE so it handles Platforms' CAs. The changes are in draft already there :)
3 participants
Store CA certificate content (base64-encoded) in config instead of file paths, enabling portable configurations across environments. The gRPC connection layer now accepts CA content directly via WithCAContent option.
PFM-4423