Skip to content
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

On Podman, detect if application is listening on the loopback interface, and either error out or not depending on --ignore-localhost #6620

Conversation

rm3l
Copy link
Member

@rm3l rm3l commented Feb 23, 2023

What type of PR is this:
/kind feature
/area dev
/area odo-on-podman

What does this PR do / why we need it:
The way odo currently does port-forwarding on Podman is via a HostPort in the relevant container in the pod spec. But as reported in #6510 and containers/podman#17353, Podman won't forward traffic to the container if the container port is bound to the container loopback interface.
As discussed in #6510 (comment) (and similar to how things work on DevSpaces), this PR tries to detect if ports that need to be forwarded are bound to the container loopback interface. If this is the case:

  • by default, odo dev on Podman will error out with an error message indicating the issue, along with a recommendation to either change the application to listen on 0.0.0.0, or to run odo dev with --forward-localhost (PR to follow up soon).
  • if odo dev is run with --ignore-localhost, this message will be displayed as a warning, but this won't prevent odo dev from running. However, any request sent out to the local port forwarded to the container on its loopback interface might not work with Podman.

I'll create a subsequent PR that adds a new --forward-localhost flag, the goal of which is to make port-forwarding work in such cases via a side container. It'll be inspired by what had been done in #6589

Which issue(s) this PR fixes:
This relates to #6510

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

This behavior should not affect the default cluster mode.

To test the changes on Podman, either use a project with an application listening on some port on localhost, or leverage some existing stacks like Node.JS which have debuggers listening on localhost.

$ mkdir /tmp/nodejs && cd /tmp/nodejs
$ odo init --name debug-nodejs --devfile nodejs --starter nodejs-starter
  • Without --ignore-localhost, odo dev on Podman should start but error out after detecting that the port is bound to the container loopback interface:
$ ODO_EXPERIMENTAL_MODE=t odo dev --platform=podman --debug

...
↪ Running on podman in Dev mode
 ✓  Deploying pod [3s]
 ✓  Building your application in container (command: install) [2s]
 •  Executing the application (command: debug)  ...
 ✗  Detected that the following port(s) can be reached only via the container loopback interface: debug (5858).
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 any of the following options:
- --ignore-localhost: no error will be returned by odo, but port-forwarding on those ports might not work on Podman.
- --forward-localhost: odo will inject a dedicated side container to redirect traffic to such ports.
Cleaning up resources
 ✗  Finished executing the application (command: debug) [16s]
 ✗  cannot make port forwarding work with ports bound to the loopback interface only

NOTE: only ports that are expected to be forwarded are checked. In the example above, if I run ODO_EXPERIMENTAL_MODE=t odo dev --platform=podman (without --debug), odo dev should start and work normally.

  • With --ignore-localhost, odo dev on Podman should start but display a warning message after detecting that the port is bound to the container loopback interface:
$ ODO_EXPERIMENTAL_MODE=t odo dev --platform=podman --debug --ignore-localhost

...
↪ Running on podman in Dev mode
 ✓  Deploying pod [3s]
 ✓  Building your application in container (command: install) [4s]
 •  Executing the application (command: debug)  ...
 ⚠  Detected that the following port(s) can be reached only via the container loopback interface: debug (5858).
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 '--forward-localhost' to make port-forwarding work with such ports.
 -  Forwarding from 127.0.0.1:20001 -> 3000
 -  Forwarding from 127.0.0.1:20002 -> 5858

↪ Dev mode
 Status:
 Watching for changes in the current directory /tmp/test/nodejs

 Keyboard Commands:
[Ctrl+c] - Exit and delete resources from podman
     [p] - Manually apply local changes to the application on podman

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Feb 23, 2023
@openshift-ci
Copy link

openshift-ci bot commented Feb 23, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@netlify
Copy link

netlify bot commented Feb 23, 2023

Deploy Preview for odo-docusaurus-preview canceled.

Name Link
🔨 Latest commit 17e0292
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/63ff564590710200075621c6

@odo-robot
Copy link

odo-robot bot commented Feb 23, 2023

OpenShift Unauthenticated Tests on commit 3b65124 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 23, 2023

NoCluster Tests on commit 3b65124 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 23, 2023

Unit Tests on commit 3b65124 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 23, 2023

Validate Tests on commit 3b65124 finished successfully.
View logs: TXT HTML

@rm3l rm3l force-pushed the 6510-make-odo-dev-on-podman-fail-if-app-is-listening-on-localhost branch from fc47c36 to 47a6943 Compare February 23, 2023 17:16
@odo-robot
Copy link

odo-robot bot commented Feb 23, 2023

Kubernetes Tests on commit ed5723e finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 23, 2023

Windows Tests (OCP) on commit 3b65124 finished with errors.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 23, 2023

OpenShift Tests on commit 3b65124 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 23, 2023

Kubernetes Docs Tests on commit 463c15d finished successfully.
View logs: TXT HTML

@rm3l rm3l force-pushed the 6510-make-odo-dev-on-podman-fail-if-app-is-listening-on-localhost branch from 47a6943 to 965d5f5 Compare February 27, 2023 16:17
@rm3l rm3l changed the title [WIP] On Podman, detect and fail if port is bound to the loopback interface [WIP] On Podman, detect if application is listening on the loopback interface, and either error out by default or don't if --ignore-localhost is passed Feb 27, 2023
@rm3l rm3l closed this Feb 27, 2023
@rm3l rm3l reopened this Feb 27, 2023
@rm3l rm3l changed the title [WIP] On Podman, detect if application is listening on the loopback interface, and either error out by default or don't if --ignore-localhost is passed [WIP] On Podman, detect if application is listening on the loopback interface, and either error out or not depending on --ignore-localhost Feb 27, 2023
@rm3l rm3l closed this Feb 27, 2023
@rm3l rm3l reopened this Feb 27, 2023
@rm3l rm3l closed this Feb 27, 2023
@rm3l rm3l reopened this Feb 27, 2023
rm3l added 5 commits February 28, 2023 14:38
Specifically, this will be useful in Podman to detect
applications that are bound to the loopback interface
…und 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 redhat-developer#6510 (comment)
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.
- 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.
@rm3l rm3l force-pushed the 6510-make-odo-dev-on-podman-fail-if-app-is-listening-on-localhost branch from af76f69 to d60d931 Compare February 28, 2023 13:44
@rm3l rm3l added kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation area/dev Issues or PRs related to `odo dev` area/odo-on-podman Issues or PRs related to running odo against Podman labels Feb 28, 2023
@rm3l rm3l changed the title [WIP] On Podman, detect if application is listening on the loopback interface, and either error out or not depending on --ignore-localhost On Podman, detect if application is listening on the loopback interface, and either error out or not depending on --ignore-localhost Feb 28, 2023
@rm3l rm3l marked this pull request as ready for review February 28, 2023 14:25
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Feb 28, 2023
@rm3l rm3l added this to the v3.8.0 🚀 milestone Feb 28, 2023
@openshift-ci openshift-ci bot requested review from feloy and rnapoles-rh February 28, 2023 14:26
@rm3l rm3l requested review from kadel and valaparthvi and removed request for rnapoles-rh February 28, 2023 14:26
Copy link
Contributor

@feloy feloy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Mar 1, 2023
@rm3l
Copy link
Member Author

rm3l commented Mar 1, 2023

  Expected
      <*url.Error | 0xc0007d3170>: {
          Op: "Post",
          URL: "http://127.0.0.1:65530/api/newuser",
          Err: <*errors.errorString | 0xc00007c130>{s: "EOF"},
      }
  to be nil
  In [It] at: C:/Users/Administrator.ANSIBLE-TEST-VS/3329/tests/e2escenarios/e2e_test.go:306

  There were additional failures detected after the initial failure.  Here's a summary - for full details run Ginkgo in verbose mode:
    [FAILED] in [AfterEach] at C:/Users/Administrator.ANSIBLE-TEST-VS/3329/tests/helper/helper_filesystem.go:48
------------------------------


Summarizing 1 Failure:
  [FAIL] E2E Test starting with non-empty Directory add Binding [It] should verify developer workflow of using binding as env in innerloop
  C:/Users/Administrator.ANSIBLE-TEST-VS/3329/tests/e2escenarios/e2e_test.go:306

Flaky E2E test - reported in #6582

/override windows-integration-test/Windows-test

@openshift-ci
Copy link

openshift-ci bot commented Mar 1, 2023

@rm3l: Overrode contexts on behalf of rm3l: windows-integration-test/Windows-test

In response to this:

 Expected
     <*url.Error | 0xc0007d3170>: {
         Op: "Post",
         URL: "http://127.0.0.1:65530/api/newuser",
         Err: <*errors.errorString | 0xc00007c130>{s: "EOF"},
     }
 to be nil
 In [It] at: C:/Users/Administrator.ANSIBLE-TEST-VS/3329/tests/e2escenarios/e2e_test.go:306

 There were additional failures detected after the initial failure.  Here's a summary - for full details run Ginkgo in verbose mode:
   [FAILED] in [AfterEach] at C:/Users/Administrator.ANSIBLE-TEST-VS/3329/tests/helper/helper_filesystem.go:48
------------------------------


Summarizing 1 Failure:
 [FAIL] E2E Test starting with non-empty Directory add Binding [It] should verify developer workflow of using binding as env in innerloop
 C:/Users/Administrator.ANSIBLE-TEST-VS/3329/tests/e2escenarios/e2e_test.go:306

Flaky E2E test - reported in #6582

/override windows-integration-test/Windows-test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@rm3l rm3l closed this Mar 1, 2023
@rm3l rm3l reopened this Mar 1, 2023
@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 1, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@openshift-merge-robot openshift-merge-robot merged commit 3adf8e5 into redhat-developer:main Mar 1, 2023
@rm3l rm3l deleted the 6510-make-odo-dev-on-podman-fail-if-app-is-listening-on-localhost branch March 1, 2023 16:18
anandrkskd pushed a commit to anandrkskd/odo that referenced this pull request Mar 7, 2023
…ce, and either error out or not depending on `--ignore-localhost` (redhat-developer#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 redhat-developer#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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dev Issues or PRs related to `odo dev` area/odo-on-podman Issues or PRs related to running odo against Podman kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants