Support (m)TLS API Socket #24601
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,7 +27,7 @@ var ( | |
| "destination" is one of the form: | ||
| [user@]hostname (will default to ssh) | ||
| ssh://[user@]hostname[:port][/path] (will obtain socket path from service, if not given.) | ||
| tcp://hostname:port (not secured without TLS enabled) | ||
| unix://path (absolute path required) | ||
| `, | ||
| RunE: add, | ||
|
|
@@ -36,6 +36,7 @@ var ( | |
| podman system connection add --identity ~/.ssh/dev_rsa testing ssh://[email protected]:2222 | ||
| podman system connection add --identity ~/.ssh/dev_rsa --port 22 production [email protected] | ||
| podman system connection add debug tcp://localhost:8080 | ||
| podman system connection add production-tls --tls-ca=ca.crt --tls-cert=tls.crt --tls-key=tls.key tcp://localhost:8080 | ||
| `, | ||
| } | ||
|
|
||
|
|
@@ -51,11 +52,14 @@ var ( | |
| dockerPath string | ||
|
|
||
| cOpts = struct { | ||
| Identity string | ||
| Port int | ||
| UDSPath string | ||
| Default bool | ||
| Farm string | ||
| TLSCertFile string | ||
| TLSKeyFile string | ||
| TLSCAFile string | ||
| }{} | ||
| ) | ||
|
|
||
|
|
@@ -74,6 +78,18 @@ func init() { | |
| flags.StringVar(&cOpts.Identity, identityFlagName, "", "path to SSH identity file") | ||
| _ = addCmd.RegisterFlagCompletionFunc(identityFlagName, completion.AutocompleteDefault) | ||
|
|
||
| tlsCertFileFlagName := "tls-cert" | ||
| flags.StringVar(&cOpts.TLSCertFile, tlsCertFileFlagName, "", "path to TLS client certificate PEM file") | ||
| _ = addCmd.RegisterFlagCompletionFunc(tlsCertFileFlagName, completion.AutocompleteDefault) | ||
|
|
||
| tlsKeyFileFlagName := "tls-key" | ||
| flags.StringVar(&cOpts.TLSKeyFile, tlsKeyFileFlagName, "", "path to TLS client certificate private key PEM file") | ||
| _ = addCmd.RegisterFlagCompletionFunc(tlsKeyFileFlagName, completion.AutocompleteDefault) | ||
|
|
||
| tlsCAFileFlagName := "tls-ca" | ||
| flags.StringVar(&cOpts.TLSCAFile, tlsCAFileFlagName, "", "path to TLS certificate Authority PEM file") | ||
| _ = addCmd.RegisterFlagCompletionFunc(tlsCAFileFlagName, completion.AutocompleteDefault) | ||
|
|
||
| socketPathFlagName := "socket-path" | ||
| flags.StringVar(&cOpts.UDSPath, socketPathFlagName, "", "path to podman socket on remote host. (default '/run/podman/podman.sock' or '/run/user/{uid}/podman/podman.sock)") | ||
| _ = addCmd.RegisterFlagCompletionFunc(socketPathFlagName, completion.AutocompleteDefault) | ||
|
|
@@ -139,14 +155,24 @@ func add(cmd *cobra.Command, args []string) error { | |
| return fmt.Errorf("invalid ssh mode") | ||
| } | ||
|
|
||
| if uri.Scheme != "tcp" { | ||
| if cmd.Flags().Changed("tls-cert") { | ||
| return fmt.Errorf("--tls-cert option not supported for %s scheme", uri.Scheme) | ||
| } | ||
| if cmd.Flags().Changed("tls-key") { | ||
| return fmt.Errorf("--tls-key option not supported for %s scheme", uri.Scheme) | ||
| } | ||
| if cmd.Flags().Changed("tls-ca") { | ||
| return fmt.Errorf("--tls-ca option not supported for %s scheme", uri.Scheme) | ||
| } | ||
| } | ||
| switch uri.Scheme { | ||
| case "ssh": | ||
| return ssh.Create(entities, sshMode) | ||
| case "unix": | ||
| if cmd.Flags().Changed("identity") { | ||
| return errors.New("--identity option not supported for unix scheme") | ||
| } | ||
| if cmd.Flags().Changed("socket-path") { | ||
| uri.Path = cmd.Flag("socket-path").Value.String() | ||
| } | ||
|
|
@@ -169,6 +195,9 @@ func add(cmd *cobra.Command, args []string) error { | |
| if cmd.Flags().Changed("identity") { | ||
| return errors.New("--identity option not supported for tcp scheme") | ||
| } | ||
| if cmd.Flags().Changed("tls-cert") != cmd.Flags().Changed("tls-key") { | ||
| return errors.New("--tls-cert and --tls-key options must be both provided if one is provided") | ||
| } | ||
| if uri.Port() == "" { | ||
| return errors.New("tcp scheme requires a port either via --port or in destination URL") | ||
| } | ||
|
|
@@ -177,8 +206,11 @@ func add(cmd *cobra.Command, args []string) error { | |
| } | ||
|
|
||
| dst := config.Destination{ | ||
| URI: uri.String(), | ||
| Identity: cOpts.Identity, | ||
| TLSCertFile: cOpts.TLSCertFile, | ||
| TLSKeyFile: cOpts.TLSKeyFile, | ||
| TLSCAFile: cOpts.TLSCAFile, | ||
| } | ||
|
|
||
| connection := args[0] | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not acceptable to me, it is certainly great to force coverage. But we run this many times on each Pr. A 4x time increase is not acceptable.
What we can consider is some split testing, we run the test on fedora rawhide, 41, 40 and debian sid so technically would could wire this up in CI ro run each case on a different distro to not add any new overhead will still getting full coverage. The transport layer should certainly not care about the distro (except underlying kernel bugs of course) so I think that may be best option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, this was a "nuclear option" in order to get tests to run with the least amount of change to the tests themselves, and to make sure that I wasn't missing anything.
One thought I had in the interim was that ginkgo allows tagging tests and sets of tests, and selecting a subset of them on the command line. Using this, it would be possible to run a full remote tests via unix sockets, and then a subset of all tests against tcp, tls, and mtls. It would then also be possible to add a separate target to run all tests in all remotes, but wouldn't be run in CI, only on-demand in development environments.
The main thing I would need for this is guidance from the podman core team on which tests they believed were critical to be tested over every possible remote, and which are "good enough" to only be tested over unix. For example, the attach endpoints would definitely need to be tested on all different remotes, as that's how I discovered the additional fixes I had to make.