Skip to content

Commit

Permalink
On Podman, detect if application is listening on the loopback interfa…
Browse files Browse the repository at this point in the history
…ce, and either error out or not depending on `--ignore-localhost` (#6620)

* Add functions allowing to detect ports opened in a given container

Specifically, this will be useful in Podman to detect
applications that are bound to the loopback interface

* Make `odo dev` fail on Podman if we detect that the application is bound to the loopback interface (on any ports supposed to be forwarded)

Next step will be to provide an option for end-users
to override this behavior, by either:
- ignoring this error (--ignore-localhost);
- or explicitly adding a redirect via a side container (--forward-localhost)

More context in #6510 (comment)

* Add '--ignore-localhost' flag to 'odo dev' on Podman

Currently, `odo dev` on Podman will error out
if it detects that the application is listening on the container loopback interface.
Instead of erroring out, this flag allows users to ignore such failure; a warning will be displayed anyway if
the application is listening on the container loopback interface, but odo will not error out.
Ports will be marked as forwarded, but Podman might fail to redirect traffic to the application
if it is bound to this loopback interface.

* Add test cases

* Fix existing integration tests by passing --ignore-localhost on Podman

- odo describe component
- odo dev --debug

Some projects used there are listening to the loopback interface,
so they won't work on Podman unless --ignore-localhost is passed.

Next, we'll pass --forward-localhost when it is implemented,
so we can have a fully working project with port-forwarding.

* Extract logic for handling loopback ports in a separate method

Requested in review
  • Loading branch information
rm3l authored Mar 1, 2023
1 parent 463c15d commit 3adf8e5
Show file tree
Hide file tree
Showing 14 changed files with 1,619 additions and 22 deletions.
3 changes: 3 additions & 0 deletions pkg/dev/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ type StartOptions struct {
RandomPorts bool
// if WatchFiles is set, files changes will trigger a new sync to the container
WatchFiles bool
// IgnoreLocalhost indicates whether to proceed with port-forwarding regardless of any container ports being bound to the container loopback interface.
// Applicable to Podman only.
IgnoreLocalhost bool
// Variables to override in the Devfile
Variables map[string]string
}
Expand Down
16 changes: 9 additions & 7 deletions pkg/dev/podmandev/podmandev.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ func (o *DevClient) Start(
DevfileRunCmd: options.RunCommand,
Variables: options.Variables,
RandomPorts: options.RandomPorts,
IgnoreLocalhost: options.IgnoreLocalhost,
WatchFiles: options.WatchFiles,
WatchCluster: false,
Out: out,
Expand Down Expand Up @@ -188,13 +189,14 @@ func (o *DevClient) watchHandler(ctx context.Context, pushParams adapters.PushPa
printWarningsOnDevfileChanges(ctx, watchParams)

startOptions := dev.StartOptions{
IgnorePaths: watchParams.FileIgnores,
Debug: watchParams.Debug,
BuildCommand: watchParams.DevfileBuildCmd,
RunCommand: watchParams.DevfileRunCmd,
RandomPorts: watchParams.RandomPorts,
WatchFiles: watchParams.WatchFiles,
Variables: watchParams.Variables,
IgnorePaths: watchParams.FileIgnores,
Debug: watchParams.Debug,
BuildCommand: watchParams.DevfileBuildCmd,
RunCommand: watchParams.DevfileRunCmd,
RandomPorts: watchParams.RandomPorts,
IgnoreLocalhost: watchParams.IgnoreLocalhost,
WatchFiles: watchParams.WatchFiles,
Variables: watchParams.Variables,
}
return o.reconcile(ctx, watchParams.Out, watchParams.ErrOut, startOptions, componentStatus)
}
Expand Down
53 changes: 53 additions & 0 deletions pkg/dev/podmandev/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package podmandev

import (
"context"
"errors"
"fmt"
"io"
"path/filepath"
Expand All @@ -18,6 +19,7 @@ import (
"github.com/redhat-developer/odo/pkg/libdevfile"
"github.com/redhat-developer/odo/pkg/log"
odocontext "github.com/redhat-developer/odo/pkg/odo/context"
"github.com/redhat-developer/odo/pkg/port"
"github.com/redhat-developer/odo/pkg/watch"

corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -113,6 +115,14 @@ func (o *DevClient) reconcile(
componentStatus.RunExecuted = true
}

// By default, Podman will not forward to container applications listening on the loopback interface.
// So we are trying to detect such cases and act accordingly.
// See https://github.com/redhat-developer/odo/issues/6510#issuecomment-1439986558
err = o.handleLoopbackPorts(options, pod, fwPorts)
if err != nil {
return err
}

for _, fwPort := range fwPorts {
s := fmt.Sprintf("Forwarding from %s:%d -> %d", fwPort.LocalAddress, fwPort.LocalPort, fwPort.ContainerPort)
fmt.Fprintf(out, " - %s", log.SboldColor(color.FgGreen, s))
Expand Down Expand Up @@ -194,3 +204,46 @@ func (o *DevClient) deployPod(ctx context.Context, options dev.StartOptions) (*c
spinner.End(true)
return pod, fwPorts, nil
}

// handleLoopbackPorts tries to detect if any of the ports to forward (in fwPorts) is actually bound to the loopback interface within the specified pod.
// If that is the case, it will either return an error if options.IgnoreLocalhost is false, or no error otherwise.
//
// Note that this method should be called after the process representing the application (run or debug command) is actually started in the pod.
func (o *DevClient) handleLoopbackPorts(options dev.StartOptions, pod *corev1.Pod, fwPorts []api.ForwardedPort) error {
if len(pod.Spec.Containers) == 0 {
return nil
}

loopbackPorts, err := port.DetectRemotePortsBoundOnLoopback(o.execClient, pod.Name, pod.Spec.Containers[0].Name, fwPorts)
if err != nil {
return fmt.Errorf("unable to detect container ports bound on the loopback interface: %w", err)
}

if len(loopbackPorts) == 0 {
return nil
}

klog.V(5).Infof("detected %d ports bound on the loopback interface in the pod: %v", len(loopbackPorts), loopbackPorts)
list := make([]string, 0, len(loopbackPorts))
for _, p := range loopbackPorts {
list = append(list, fmt.Sprintf("%s (%d)", p.PortName, p.ContainerPort))
}
msg := fmt.Sprintf(`Detected that the following port(s) can be reached only via the container loopback interface: %s.
Port forwarding on Podman currently does not work with applications listening on the loopback interface.
Either change the application to make those port(s) reachable on all interfaces (0.0.0.0), or rerun 'odo dev' with `, strings.Join(list, ", "))
if options.IgnoreLocalhost {
msg += "'--forward-localhost' to make port-forwarding work with such ports."
} else {
msg += `any of the following options:
- --ignore-localhost: no error will be returned by odo, but forwarding to those ports might not work on Podman.
- --forward-localhost: odo will inject a dedicated side container to redirect traffic to such ports.`
}
if !options.IgnoreLocalhost {
log.Errorf(msg)
return errors.New("cannot make port forwarding work with applications listening only on the loopback interface")
}
// No error, but only a warning if using --ignore-localhost
log.Warningf(msg)

return nil
}
35 changes: 23 additions & 12 deletions pkg/odo/cli/dev/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package dev

import (
"context"
"errors"
"fmt"
"io"
"path/filepath"
Expand Down Expand Up @@ -50,11 +51,12 @@ type DevOptions struct {
cancel context.CancelFunc

// Flags
noWatchFlag bool
randomPortsFlag bool
debugFlag bool
buildCommandFlag string
runCommandFlag string
noWatchFlag bool
randomPortsFlag bool
debugFlag bool
buildCommandFlag string
runCommandFlag string
ignoreLocalhostFlag bool
}

var _ genericclioptions.Runnable = (*DevOptions)(nil)
Expand Down Expand Up @@ -105,6 +107,9 @@ func (o *DevOptions) Validate(ctx context.Context) error {
platform := fcontext.GetPlatform(ctx, commonflags.PlatformCluster)
switch platform {
case commonflags.PlatformCluster:
if o.ignoreLocalhostFlag {
return errors.New("--ignore-localhost cannot be used when running in cluster mode")
}
if o.clientset.KubernetesClient == nil {
return kclient.NewNoConnectionError()
}
Expand Down Expand Up @@ -180,13 +185,14 @@ func (o *DevOptions) Run(ctx context.Context) (err error) {
o.out,
o.errOut,
dev.StartOptions{
IgnorePaths: o.ignorePaths,
Debug: o.debugFlag,
BuildCommand: o.buildCommandFlag,
RunCommand: o.runCommandFlag,
RandomPorts: o.randomPortsFlag,
WatchFiles: !o.noWatchFlag,
Variables: variables,
IgnorePaths: o.ignorePaths,
Debug: o.debugFlag,
BuildCommand: o.buildCommandFlag,
RunCommand: o.runCommandFlag,
RandomPorts: o.randomPortsFlag,
WatchFiles: !o.noWatchFlag,
IgnoreLocalhost: o.ignoreLocalhostFlag,
Variables: variables,
},
)
}
Expand Down Expand Up @@ -226,6 +232,11 @@ It forwards endpoints with any exposure values ('public', 'internal' or 'none')
"Alternative build command. The default one will be used if this flag is not set.")
devCmd.Flags().StringVar(&o.runCommandFlag, "run-command", "",
"Alternative run command to execute. The default one will be used if this flag is not set.")
devCmd.Flags().BoolVar(&o.ignoreLocalhostFlag, "ignore-localhost", false,
"Whether to ignore errors related to port-forwarding apps listening on the container loopback interface. Applicable only if platform is podman.")
// TODO Unhide when moving Podman out of the experimental mode : https://github.com/redhat-developer/odo/issues/6592
_ = devCmd.Flags().MarkHidden("ignore-localhost")

clientset.Add(devCmd,
clientset.BINDING,
clientset.DEV,
Expand Down
Loading

0 comments on commit 3adf8e5

Please sign in to comment.