Skip to content

Commit

Permalink
refactor(robot-server,api): robot server port networking fast api (#5184
Browse files Browse the repository at this point in the history 
)

* create system/wifi module and move some functions from server/endpoints/networking there.

* more refactoring

* code clean up and restructuring.

* move eap option validation from network endpoints to wifi.
rewrote test_key_lifecycle to test the key api. not the http api.

* reworked test_networking_endpoints to only test the http interaction with the key functions. Not the underlying file system. That belongs in the test_wifi module.

* test_network_status added to nmcli. this didn't belong in the server endpoint tests as it is really testing our ability to communicate with nmcli.

* remove deprecated comment

* start robot_server networking endpoints

* progress

* keys is a reserved word. use an alias.

* list keys tests pass

* networking tests pass minus the disconnect

* disconnect tests

* tests for wificonfiguration model validation

* add todo note regarding wifi disconnect

* fix lint errors

* cannot import log_control.MAX_RECORDS due to syslog import failure.

* clean up the instances of multiple response types.

* rename V1ErrorResponse to V1BasicResponse. It isn't always used for errors. Name was misleading.

* fix bad test

* fix lint error

* fix bugs that were revealed when running on PI.
Configure didn't work because we weren't testing how the nmcli.configure call was made.
get wifi/keys failed due to validation errors on pydantic model due to attribute named keys. also, someone (me) didn't port the aiohttp unit tests properly.

closes #5143
  • Loading branch information
amitlissack authored Mar 13, 2020
1 parent 30a4e97 commit 35ae8ac
Show file tree
Hide file tree
Showing 16 changed files with 1,160 additions and 441 deletions.
211 changes: 35 additions & 176 deletions api/src/opentrons/server/endpoints/networking.py
View file
Original file line number Diff line number Diff line change
@@ -1,33 +1,13 @@
import hashlib
import json
import logging
import os
import shutil
import subprocess
from typing import Dict, Any, List
from typing import Dict, Any
from aiohttp import web
from opentrons.system import nmcli
from opentrons.config import CONFIG
from opentrons.system import nmcli, wifi

log = logging.getLogger(__name__)

EAP_CONFIG_SHAPE = {
'options': [
{'name': method.qualified_name(),
'displayName': method.display_name(),
'options': [{k: v for k, v in arg.items()
if k in ['name',
'displayName',
'required',
'type']}
for arg in method.args()]}
for method in nmcli.EAP_TYPES]
}


class ConfigureArgsError(Exception):
pass


class DisconnectArgsError(Exception):
pass
Expand Down Expand Up @@ -65,95 +45,6 @@ async def list_networks(request: web.Request) -> web.Response:
return web.json_response({'list': networks}, status=200)


def _get_key_file(arg: str) -> str:
keys_dir = CONFIG['wifi_keys_dir']
available_keys = os.listdir(keys_dir)
if arg not in available_keys:
raise ConfigureArgsError('Key ID {} is not valid on the system'
.format(arg))
files_in_dir = os.listdir(os.path.join(keys_dir, arg))
if len(files_in_dir) > 1:
raise OSError(
'Key ID {} has multiple files, try deleting and reuploading'
.format(arg))
return os.path.join(keys_dir, arg, files_in_dir[0])


def _eap_check_no_extra_args(
config: Dict[str, Any], options: Any):
# options is an Any because the type annotation for EAP_CONFIG_SHAPE itself
# can’t quite express the type properly because of the inference from the
# dict annotation.
"""Check for args that are not required for this method (to aid debugging)
``config`` should be the user config.
``options`` should be the options submember for the eap method.
Before this method is called, the validity of the 'eapType' key should be
established.
"""
arg_names = [k for k in config.keys() if k != 'eapType']
valid_names = [o['name'] for o in options]
for an in arg_names:
if an not in valid_names:
raise ConfigureArgsError(
'Option {} is not valid for EAP method {}'
.format(an, config['eapType']))


def _eap_check_option_ok(opt: Dict[str, str], config: Dict[str, Any]):
""" Check that a given EAP option is in the user config (if required)
and, if specified, is the right type.
``opt`` should be an options dict from EAP_CONFIG_SHAPE.
``config`` should be the user config dict.
Before this method is called, the validity of the eapType key should be
established.
"""
if opt['name'] not in config:
if opt['required']:
raise ConfigureArgsError(
'Required argument {} for eap method {} not present'
.format(opt['displayName'], config['eapType']))
else:
return
name = opt['name']
o_type = opt['type']
arg = config[name]
if name in config:
if o_type in ('string', 'password') and not isinstance(arg, str):
raise ConfigureArgsError('Option {} should be a str'
.format(name))
elif o_type == 'file' and not isinstance(arg, str):
raise ConfigureArgsError('Option {} must be a str'
.format(name))


def _eap_check_config(eap_config: Dict[str, Any]) -> Dict[str, Any]:
""" Check the eap specific args, and replace values where needed.
Similar to _check_configure_args but for only EAP.
"""
eap_type = eap_config.get('eapType')
for method in EAP_CONFIG_SHAPE['options']:
if method['name'] == eap_type:
options = method['options']
break
else:
raise ConfigureArgsError('EAP method {} is not valid'.format(eap_type))

_eap_check_no_extra_args(eap_config, options)

for opt in options: # type: ignore
# Ignoring most types to do with EAP_CONFIG_SHAPE because of issues
# wth type inference for dict comprehensions
_eap_check_option_ok(opt, eap_config)
if opt['type'] == 'file' and opt['name'] in eap_config:
# Special work for file: rewrite from key id to path
eap_config[opt['name']] = _get_key_file(eap_config[opt['name']])
return eap_config


def _deduce_security(kwargs) -> nmcli.SECURITY_TYPES:
""" Make sure that the security_type is known, or throw. """
# Security should be one of our valid strings
Expand All @@ -164,7 +55,7 @@ def _deduce_security(kwargs) -> nmcli.SECURITY_TYPES:
}
if not kwargs.get('securityType'):
if kwargs.get('psk') and kwargs.get('eapConfig'):
raise ConfigureArgsError(
raise wifi.ConfigureArgsError(
'Cannot deduce security type: psk and eap both passed')
elif kwargs.get('psk'):
kwargs['securityType'] = 'wpa-psk'
Expand All @@ -175,8 +66,9 @@ def _deduce_security(kwargs) -> nmcli.SECURITY_TYPES:
try:
return sec_translation[kwargs['securityType']]
except KeyError:
raise ConfigureArgsError('securityType must be one of {}'
.format(','.join(sec_translation.keys())))
raise wifi.ConfigureArgsError(
'securityType must be one of {}'.format(
','.join(sec_translation.keys())))


def _check_configure_args(configure_args: Dict[str, Any]) -> Dict[str, Any]:
Expand All @@ -188,30 +80,30 @@ def _check_configure_args(configure_args: Dict[str, Any]) -> Dict[str, Any]:
# SSID must always be present
if not configure_args.get('ssid')\
or not isinstance(configure_args['ssid'], str):
raise ConfigureArgsError("SSID must be specified")
raise wifi.ConfigureArgsError("SSID must be specified")
# If specified, hidden must be a bool
if not configure_args.get('hidden'):
configure_args['hidden'] = False
elif not isinstance(configure_args['hidden'], bool):
raise ConfigureArgsError('If specified, hidden must be a bool')
raise wifi.ConfigureArgsError('If specified, hidden must be a bool')

configure_args['securityType'] = _deduce_security(configure_args)

# If we have wpa2-personal, we need a psk
if configure_args['securityType'] == nmcli.SECURITY_TYPES.WPA_PSK:
if not configure_args.get('psk'):
raise ConfigureArgsError(
raise wifi.ConfigureArgsError(
'If securityType is wpa-psk, psk must be specified')
return configure_args

# If we have wpa2-enterprise, we need eap config, and we need to check
# it
if configure_args['securityType'] == nmcli.SECURITY_TYPES.WPA_EAP:
if not configure_args.get('eapConfig'):
raise ConfigureArgsError(
raise wifi.ConfigureArgsError(
'If securityType is wpa-eap, eapConfig must be specified')
configure_args['eapConfig']\
= _eap_check_config(configure_args['eapConfig'])
= wifi.eap_check_config(configure_args['eapConfig'])
return configure_args

# If we’re still here we have no security and we’re done
Expand Down Expand Up @@ -255,7 +147,7 @@ async def configure(request: web.Request) -> web.Response:

try:
configure_kwargs = _check_configure_args(body)
except ConfigureArgsError as e:
except wifi.ConfigureArgsError as e:
return web.json_response({'message': str(e)}, status=400)

try:
Expand Down Expand Up @@ -418,7 +310,6 @@ async def add_key(request: web.Request) -> web.Response:
}
```
"""
keys_dir = CONFIG['wifi_keys_dir']
if not request.can_read_body:
return web.json_response({'message': "Must upload key file"},
status=400)
Expand All @@ -427,34 +318,19 @@ async def add_key(request: web.Request) -> web.Response:
if not keyfile:
return web.json_response(
{'message': "No key 'key' in request"}, status=400)
hasher = hashlib.sha256()
key_contents = keyfile.file.read()
hasher.update(key_contents)
key_hash = hasher.hexdigest()
if key_hash in os.listdir(keys_dir):
files = os.listdir(os.path.join(keys_dir, key_hash))
if files:
return web.json_response(
{'message': 'Key file already present',
'uri': '/wifi/keys/{}'.format(key_hash),
'id': key_hash,
'name': files[0]},
status=200)
else:
log.warning(
"Key directory with nothing in it: {}"
.format(key_hash))
os.rmdir(os.path.join(keys_dir, key_hash))
key_hash_path = os.path.join(keys_dir, key_hash)
os.mkdir(key_hash_path)
with open(os.path.join(key_hash_path,
os.path.basename(keyfile.filename)), 'wb') as f:
f.write(key_contents)
return web.json_response(
{'uri': '/wifi/keys/{}'.format(key_hash),
'id': key_hash,
'name': os.path.basename(keyfile.filename)},
status=201)

add_key_result = wifi.add_key(keyfile.filename, keyfile.file.read())

response_body = {
'uri': '/wifi/keys/{}'.format(add_key_result.key.directory),
'id': add_key_result.key.directory,
'name': os.path.basename(add_key_result.key.file)
}
if add_key_result.created:
return web.json_response(response_body, status=201)
else:
response_body['message'] = 'Key file already present'
return web.json_response(response_body, status=200)


async def list_keys(request: web.Request) -> web.Response:
Expand All @@ -475,21 +351,11 @@ async def list_keys(request: web.Request) -> web.Response:
}
```
"""
keys_dir = CONFIG['wifi_keys_dir']
keys: List[Dict[str, str]] = []
# TODO(mc, 2018-10-24): add last modified info to keys for sort purposes
for path in os.listdir(keys_dir):
full_path = os.path.join(keys_dir, path)
if os.path.isdir(full_path):
in_path = os.listdir(full_path)
if len(in_path) > 1:
log.warning("Garbage in key dir for key {}".format(path))
keys.append(
{'uri': '/wifi/keys/{}'.format(path),
'id': path,
'name': os.path.basename(in_path[0])})
else:
log.warning("Garbage in wifi keys dir: {}".format(full_path))
keys = [
{'uri': '/wifi/keys/{}'.format(key.directory),
'id': key.directory,
'name': os.path.basename(key.file)} for key in wifi.list_keys()
]
return web.json_response({'keys': keys}, status=200)


Expand All @@ -503,19 +369,13 @@ async def remove_key(request: web.Request) -> web.Response:
{message: 'Removed key keyfile.pem'}
```
"""
keys_dir = CONFIG['wifi_keys_dir']
available_keys = os.listdir(keys_dir)
requested_hash = request.match_info['key_uuid']
if requested_hash not in available_keys:
deleted_file = wifi.remove_key(requested_hash)
if not deleted_file:
return web.json_response(
{'message': 'No such key file {}'
.format(requested_hash)},
status=404)
key_path = os.path.join(keys_dir, requested_hash)
name = os.listdir(key_path)[0]
shutil.rmtree(key_path)
{'message': f"No such key file {requested_hash}"}, status=404)
return web.json_response(
{'message': 'Key file {} deleted'.format(name)},
{'message': f'Key file {deleted_file} deleted'},
status=200)


Expand Down Expand Up @@ -582,5 +442,4 @@ async def eap_options(request: web.Request) -> web.Response:
}
```
"""

return web.json_response(EAP_CONFIG_SHAPE, status=200)
return web.json_response(wifi.EAP_CONFIG_SHAPE, status=200)
Loading

0 comments on commit 35ae8ac

Please sign in to comment.