-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor podman system connection #6938
Merged
openshift-merge-robot
merged 1 commit into
containers:master
from
jwhonce:wip/n-connection
Jul 24, 2020
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 |
|---|---|---|
| @@ -1,209 +1,34 @@ | ||
| package system | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "fmt" | ||
| "net" | ||
| "net/url" | ||
| "os" | ||
| "os/user" | ||
| "regexp" | ||
|
|
||
| "github.com/containers/common/pkg/config" | ||
| "github.com/containers/libpod/v2/cmd/podman/registry" | ||
| "github.com/containers/libpod/v2/libpod/define" | ||
| "github.com/containers/libpod/v2/cmd/podman/validate" | ||
| "github.com/containers/libpod/v2/pkg/domain/entities" | ||
| "github.com/containers/libpod/v2/pkg/terminal" | ||
| "github.com/pkg/errors" | ||
| "github.com/sirupsen/logrus" | ||
| "github.com/spf13/cobra" | ||
| "golang.org/x/crypto/ssh" | ||
| "golang.org/x/crypto/ssh/agent" | ||
| ) | ||
|
|
||
| const schemaPattern = "^[A-Za-z][A-Za-z0-9+.-]*:" | ||
|
|
||
| var ( | ||
| // Skip creating engines since this command will obtain connection information to engine | ||
| // Skip creating engines since this command will obtain connection information to said engines | ||
| noOp = func(cmd *cobra.Command, args []string) error { | ||
| return nil | ||
| } | ||
| connectionCmd = &cobra.Command{ | ||
| Use: "connection [flags] DESTINATION", | ||
| Args: cobra.ExactArgs(1), | ||
| Long: `Store ssh destination information in podman configuration. | ||
| "destination" is of the form [user@]hostname or | ||
| an URI of the form ssh://[user@]hostname[:port] | ||
| `, | ||
| Short: "Record remote ssh destination", | ||
| PersistentPreRunE: noOp, | ||
| PersistentPostRunE: noOp, | ||
| TraverseChildren: false, | ||
| RunE: connection, | ||
| Example: `podman system connection server.fubar.com | ||
| podman system connection --identity ~/.ssh/dev_rsa ssh://[email protected]:2222 | ||
| podman system connection --identity ~/.ssh/dev_rsa --port 22 [email protected]`, | ||
| } | ||
|
|
||
| cOpts = struct { | ||
| Identity string | ||
| Port int | ||
| UDSPath string | ||
| }{} | ||
| ConnectionCmd = &cobra.Command{ | ||
| Use: "connection", | ||
| Short: "Manage remote ssh destinations", | ||
| Long: `Manage ssh destination information in podman configuration`, | ||
| DisableFlagsInUseLine: true, | ||
| PersistentPreRunE: noOp, | ||
| RunE: validate.SubCommandExists, | ||
| PersistentPostRunE: noOp, | ||
| TraverseChildren: false, | ||
| } | ||
| ) | ||
|
|
||
| func init() { | ||
| registry.Commands = append(registry.Commands, registry.CliCommand{ | ||
| Mode: []entities.EngineMode{entities.ABIMode, entities.TunnelMode}, | ||
| Command: connectionCmd, | ||
| Command: ConnectionCmd, | ||
| Parent: systemCmd, | ||
| }) | ||
|
|
||
| flags := connectionCmd.Flags() | ||
| flags.IntVarP(&cOpts.Port, "port", "p", 22, "SSH port number for destination") | ||
| flags.StringVar(&cOpts.Identity, "identity", "", "path to SSH identity file") | ||
| flags.StringVar(&cOpts.UDSPath, "socket-path", "", "path to podman socket on remote host. (default '/run/podman/podman.sock' or '/run/user/{uid}/podman/podman.sock)") | ||
| } | ||
|
|
||
| func connection(cmd *cobra.Command, args []string) error { | ||
| // Default to ssh: schema if none given | ||
| dest := []byte(args[0]) | ||
| if match, err := regexp.Match(schemaPattern, dest); err != nil { | ||
| return errors.Wrapf(err, "internal regex error %q", schemaPattern) | ||
| } else if !match { | ||
| dest = append([]byte("ssh://"), dest...) | ||
| } | ||
|
|
||
| uri, err := url.Parse(string(dest)) | ||
| if err != nil { | ||
| return errors.Wrapf(err, "failed to parse %q", string(dest)) | ||
| } | ||
|
|
||
| if uri.User.Username() == "" { | ||
| if uri.User, err = getUserInfo(uri); err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| if cmd.Flag("socket-path").Changed { | ||
| uri.Path = cmd.Flag("socket-path").Value.String() | ||
| } | ||
|
|
||
| if cmd.Flag("port").Changed { | ||
| uri.Host = net.JoinHostPort(uri.Hostname(), cmd.Flag("port").Value.String()) | ||
| } | ||
|
|
||
| if uri.Port() == "" { | ||
| uri.Host = net.JoinHostPort(uri.Hostname(), cmd.Flag("port").DefValue) | ||
| } | ||
|
|
||
| if uri.Path == "" { | ||
| if uri.Path, err = getUDS(cmd, uri); err != nil { | ||
| return errors.Wrapf(err, "failed to connect to %q", uri.String()) | ||
| } | ||
| } | ||
|
|
||
| custom, err := config.ReadCustomConfig() | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if cmd.Flag("identity").Changed { | ||
| custom.Engine.RemoteIdentity = cOpts.Identity | ||
| } | ||
|
|
||
| custom.Engine.RemoteURI = uri.String() | ||
| return custom.Write() | ||
| } | ||
|
|
||
| func getUserInfo(uri *url.URL) (*url.Userinfo, error) { | ||
| var ( | ||
| usr *user.User | ||
| err error | ||
| ) | ||
| if u, found := os.LookupEnv("_CONTAINERS_ROOTLESS_UID"); found { | ||
| usr, err = user.LookupId(u) | ||
| if err != nil { | ||
| return nil, errors.Wrapf(err, "failed to find user %q", u) | ||
| } | ||
| } else { | ||
| usr, err = user.Current() | ||
| if err != nil { | ||
| return nil, errors.Wrapf(err, "failed to obtain current user") | ||
| } | ||
| } | ||
|
|
||
| pw, set := uri.User.Password() | ||
| if set { | ||
| return url.UserPassword(usr.Username, pw), nil | ||
| } | ||
| return url.User(usr.Username), nil | ||
| } | ||
|
|
||
| func getUDS(cmd *cobra.Command, uri *url.URL) (string, error) { | ||
| var authMethods []ssh.AuthMethod | ||
| passwd, set := uri.User.Password() | ||
| if set { | ||
| authMethods = append(authMethods, ssh.Password(passwd)) | ||
| } | ||
|
|
||
| ident := cmd.Flag("identity") | ||
| if ident.Changed { | ||
| auth, err := terminal.PublicKey(ident.Value.String(), []byte(passwd)) | ||
| if err != nil { | ||
| return "", errors.Wrapf(err, "Failed to read identity %q", ident.Value.String()) | ||
| } | ||
| authMethods = append(authMethods, auth) | ||
| } | ||
|
|
||
| if sock, found := os.LookupEnv("SSH_AUTH_SOCK"); found { | ||
| logrus.Debugf("Found SSH_AUTH_SOCK %q, ssh-agent signer enabled", sock) | ||
|
|
||
| c, err := net.Dial("unix", sock) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| a := agent.NewClient(c) | ||
| authMethods = append(authMethods, ssh.PublicKeysCallback(a.Signers)) | ||
| } | ||
|
|
||
| config := &ssh.ClientConfig{ | ||
| User: uri.User.Username(), | ||
| Auth: authMethods, | ||
| HostKeyCallback: ssh.InsecureIgnoreHostKey(), | ||
| } | ||
| dial, err := ssh.Dial("tcp", uri.Host, config) | ||
| if err != nil { | ||
| return "", errors.Wrapf(err, "failed to connect to %q", uri.Host) | ||
| } | ||
| defer dial.Close() | ||
|
|
||
| session, err := dial.NewSession() | ||
| if err != nil { | ||
| return "", errors.Wrapf(err, "failed to create new ssh session on %q", uri.Host) | ||
| } | ||
| defer session.Close() | ||
|
|
||
| // Override podman binary for testing etc | ||
| podman := "podman" | ||
| if v, found := os.LookupEnv("PODMAN_BINARY"); found { | ||
| podman = v | ||
| } | ||
| run := podman + " info --format=json" | ||
|
|
||
| var buffer bytes.Buffer | ||
| session.Stdout = &buffer | ||
| if err := session.Run(run); err != nil { | ||
| return "", errors.Wrapf(err, "failed to run %q", run) | ||
| } | ||
|
|
||
| var info define.Info | ||
| if err := json.Unmarshal(buffer.Bytes(), &info); err != nil { | ||
| return "", errors.Wrapf(err, "failed to parse 'podman info' results") | ||
| } | ||
|
|
||
| if info.Host.RemoteSocket == nil || len(info.Host.RemoteSocket.Path) == 0 { | ||
| return "", fmt.Errorf("remote podman %q failed to report its UDS socket", uri.Host) | ||
| } | ||
| return info.Host.RemoteSocket.Path, nil | ||
| } |
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.
I'm feeling pretty squicked out by the interchange of URI and URL. I think that will lead to maintenance headaches. From my (limited) understanding, I believe URL (l as in Lima) is the correct term. Would you consider replacing the code instances of
uri(i as India) tourlfor consistency?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.
Yes let's stick with URL
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.
@rhatdan @edsantiago
uri was chosen for a reason. url is a golang package name. There is a lot of code that uses the url package to process the "uri" variable. We would be forced to alias the package name or make up longer names which I deemed to be a large maintenance headache. I attempt to use uri for internals and URL for user facing so everyone is happy.
As a url is a uri this naming simplifies the coding and is correct naming.
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.
Would calling it
pathbe a good compromise? It's less specific as to what exact type of path it is, and only one character longer.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.
@mheon Path is an element of the url/uri, so that would be confusing to see path.Path.
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.
How about we call everything URI and drop URL, if we want to still support the --url for backwards compatibility just make it an alias.