Skip to content

VELS_WEB: Discover $server_ip robust to output format changes #15

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

Open
wants to merge 1 commit into
base: autosub_devel
Choose a base branch
from

Conversation

christian-krieg
Copy link

  • The recent fix still is vulnerable to changes in the output format of 'ip
    addr', and it may fail if more than one active IP addresses are present on the
    system

  • The proposed fix asks 'ip addr' to output its data in JSON format, only
    considering 'inet' devices which are exposed to the network ('scope global').

    $ ip -json -family inet addr show scope global | python3 -c "import sys, json; print(json.dumps(json.load(sys.stdin), indent=4))"
    [
        {
            "ifindex": 2,
            "ifname": "ens18",
            "flags": [
                "BROADCAST",
                "MULTICAST",
                "UP",
                "LOWER_UP"
            ],
            "mtu": 1500,
            "qdisc": "fq_codel",
            "operstate": "UP",
            "group": "default",
            "txqlen": 1000,
            "altnames": [
                "enp0s18"
            ],
            "addr_info": [
                {
                    "family": "inet",
                    "local": "111.222.33.4",
                    "prefixlen": 23,
                    "broadcast": "111.222.55.6",
                    "scope": "global",
                    "label": "ens18",
                    "valid_life_time": 4294967295,
                    "preferred_life_time": 4294967295
                }
            ]
        }
    ]
    
  • Then, a short Python command sequence allows us to parse this JSON output to
    explicitly ask for the device's IP address

    $ ip -json -family inet addr show scope global | python3 -c "import sys, json; print(json.load(sys.stdin)[0]['addr_info'][0]['local'])"
    111.222.33.4
    

* The recent fix still is vulnerable to changes in the output format of 'ip
  addr', and it may fail if more than one active IP addresses are present on the
  system

* The proposed fix asks 'ip addr' to output its data in JSON format, only
  considering 'inet' devices which are exposed to the network ('scope global').

      $ ip -json -family inet addr show scope global | python3 -c "import sys, json; print(json.dumps(json.load(sys.stdin), indent=4))"
      [
          {
              "ifindex": 2,
              "ifname": "ens18",
              "flags": [
                  "BROADCAST",
                  "MULTICAST",
                  "UP",
                  "LOWER_UP"
              ],
              "mtu": 1500,
              "qdisc": "fq_codel",
              "operstate": "UP",
              "group": "default",
              "txqlen": 1000,
              "altnames": [
                  "enp0s18"
              ],
              "addr_info": [
                  {
                      "family": "inet",
                      "local": "111.222.33.4",
                      "prefixlen": 23,
                      "broadcast": "111.222.55.6",
                      "scope": "global",
                      "label": "ens18",
                      "valid_life_time": 4294967295,
                      "preferred_life_time": 4294967295
                  }
              ]
          }
      ]

* Then, a short Python command sequence allows us to parse this JSON output to
  explicitly ask for the device's IP address

      $ ip -json -family inet addr show scope global | python3 -c "import sys, json; print(json.load(sys.stdin)[0]['addr_info'][0]['local'])"
      111.222.33.4
@MartinMosbeck
Copy link
Member

Sorry for the very late comment.

  1. Pull request should be directed to the autosub_devel branch
  2. This feels a little overkill to me. I do not have an active instance running at the moment, but sometimes with such tools when you pass a 0.0.0.0 it just listens on all interfaces. If that works it would be easier, and admins have to anyhow make sure the machine is not publicly accessible.

@christian-krieg christian-krieg changed the base branch from master to autosub_devel December 18, 2023 10:01
@christian-krieg
Copy link
Author

Hi @MartinMosbeck

Thanks a lot for your response! Regarding your comments:

  1. Done
  2. The fix may be overkill, but the intention was to have a robust fix instead of a quick fix which may break soon again in near future. As autosub is Python3-based, the fix does not introduce additional dependencies, so we should be fine on this track.

To be honest, I did not check if there was an easier solution in general (as the one that you mention with passing 0.0.0.0). If such an easy solution exists, I would be very much in favor to use it instead of my fix.

@christian-krieg
Copy link
Author

Hi @MartinMosbeck, I just would like to check back if there is anything I can do to get this PR merged into upstream. Please just let me know!

@MartinMosbeck
Copy link
Member

As you agree that 0.0.0.0 would be the better option, I was hoping you would try it out. Anyway, I now googled it and found out 0.0.0.0 is possible (answer from the maintainer). As far as I know @fipsi7 is the new VELS responsible person at the ICT, he also has the needed rights.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants