diff --git a/.pylintrc b/.pylintrc new file mode 100644 index 0000000..3d762a3 --- /dev/null +++ b/.pylintrc @@ -0,0 +1,101 @@ +[MASTER] +ignore = ,input +persistent = yes + +[MESSAGES CONTROL] +disable = + missing-docstring, + fixme, + duplicate-code, + no-member, + parse-error, + bad-continuation, + too-few-public-methods, + global-statement, + cyclic-import, + locally-disabled, + file-ignored, + no-else-return, + unnecessary-lambda, + wrong-import-position, + logging-format-interpolation, + bare-except, + too-many-public-methods + +[REPORTS] +output-format = text +files-output = no +reports = no + +[FORMAT] +max-line-length = 120 +max-statement-lines = 75 +single-line-if-stmt = no +no-space-check = trailing-comma,dict-separator +max-module-lines = 1000 +indent-string = ' ' +string-quote=single-avoid-escape +triple-quote=single +docstring-quote=double + +[MISCELLANEOUS] +notes = FIXME,XXX,TODO + +[SIMILARITIES] +min-similarity-lines = 4 +ignore-comments = yes +ignore-docstrings = yes +ignore-imports = no + +[BASIC] +# Regular expression which should only match correct module names +module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$ + +# Regular expression which should only match correct module level names +const-rgx=(([A-Za-z_][A-Za-z1-9_]*)|(__.*__))$ + +# Regular expression which should only match correct class names +class-rgx=[A-Z_][a-zA-Z0-9_]+$ + +# Regular expression which should only match correct function names +function-rgx=[a-z_][a-z0-9_]{2,35}$ + +# Regular expression which should only match correct method names +method-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression which should only match correct instance attribute names +attr-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression which should only match correct argument names +argument-rgx=[a-z_][a-z0-9_]{0,30}$ + +# Regular expression which should only match correct variable names +variable-rgx=[a-z_][a-z0-9_]{0,30}$ + +# Regular expression which should only match correct list comprehension / +# generator expression variable names +inlinevar-rgx=[A-Za-z_][A-Za-z0-9_]*$ + +# Good variable names which should always be accepted, separated by a comma +good-names=logger,id,ID + +# Bad variable names which should always be refused, separated by a comma +bad-names=foo,bar,baz,toto,tutu,tata + +# List of builtins function names that should not be used, separated by a comma +bad-functions=apply,input + +[DESIGN] +max-args = 10 +ignored-argument-names = _.* +max-locals = 20 +max-returns = 6 +max-branches = 15 +max-statements = 55 +max-parents = 7 +max-attributes = 10 +min-public-methods = 2 +max-public-methods = 20 + +[EXCEPTIONS] +overgeneral-exceptions = Exception diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 0000000..e74c499 --- /dev/null +++ b/.travis.yml @@ -0,0 +1,8 @@ +language: python +python: + - "3.6" + +install: + - "pip install -r requirements.txt" +script: + - "pylint proxstar" diff --git a/proxstar/__init__.py b/proxstar/__init__.py index 359c8a4..9e1af15 100644 --- a/proxstar/__init__.py +++ b/proxstar/__init__.py @@ -1,11 +1,11 @@ import os import json import time -import psutil import atexit import logging -import psycopg2 import subprocess +import psutil +import psycopg2 import rq_dashboard from rq import Queue from redis import Redis @@ -13,13 +13,15 @@ from rq_scheduler import Scheduler from sqlalchemy import create_engine from sqlalchemy.orm import sessionmaker from flask import Flask, render_template, request, redirect, session, abort -from proxstar.db import * -from proxstar.vnc import * +from proxstar.db import (Base, datetime, get_pool_cache, renew_vm_expire, set_user_usage_limits, get_template, +get_templates, get_allowed_users, add_ignored_pool, delete_ignored_pool, add_allowed_user, delete_allowed_user, +get_template_disk, set_template_info, url_for) +from proxstar.vnc import (send_stop_ssh_tunnel, stop_ssh_tunnel, add_vnc_target, start_ssh_tunnel, get_vnc_targets, +delete_vnc_target, stop_websockify) from proxstar.auth import get_auth from proxstar.util import gen_password -from proxstar.starrs import * -from proxstar.ldapdb import * -from proxstar.proxmox import * +from proxstar.starrs import check_hostname, renew_ip +from proxstar.proxmox import connect_proxmox, get_isos, get_pools, get_ignored_pools logging.basicConfig( format='%(asctime)s %(levelname)s %(message)s', level=logging.INFO) @@ -37,8 +39,8 @@ app.config.from_pyfile(config) app.config["GIT_REVISION"] = subprocess.check_output( ['git', 'rev-parse', '--short', 'HEAD']).decode('utf-8').rstrip() -with open('proxmox_ssh_key', 'w') as key: - key.write(app.config['PROXMOX_SSH_KEY']) +with open('proxmox_ssh_key', 'w') as ssh_key_file: + ssh_key_file.write(app.config['PROXMOX_SSH_KEY']) ssh_tunnels = [] @@ -60,7 +62,8 @@ starrs = psycopg2.connect( from proxstar.vm import VM from proxstar.user import User -from proxstar.tasks import generate_pool_cache_task, process_expiring_vms_task, cleanup_vnc_task, delete_vm_task, create_vm_task, setup_template_task +from proxstar.tasks import (generate_pool_cache_task, process_expiring_vms_task, cleanup_vnc_task, +delete_vm_task, create_vm_task, setup_template_task) if 'generate_pool_cache' not in scheduler: logging.info('adding generate pool cache task to scheduler') @@ -87,7 +90,7 @@ if 'cleanup_vnc' not in scheduler: def add_rq_dashboard_auth(blueprint): @blueprint.before_request @auth.oidc_auth - def rq_dashboard_auth(*args, **kwargs): + def rq_dashboard_auth(*args, **kwargs): #pylint: disable=unused-argument,unused-variable if 'rtp' not in session['userinfo']['groups']: abort(403) @@ -100,13 +103,13 @@ app.register_blueprint(rq_dashboard_blueprint, url_prefix="/rq") @app.errorhandler(404) def not_found(e): user = User(session['userinfo']['preferred_username']) - return render_template('404.html', user=user), 404 + return render_template('404.html', user=user, e=e), 404 @app.errorhandler(403) def forbidden(e): user = User(session['userinfo']['preferred_username']) - return render_template('403.html', user=user), 403 + return render_template('403.html', user=user, e=e), 403 @app.route("/") @@ -115,7 +118,7 @@ def forbidden(e): def list_vms(user_view=None): user = User(session['userinfo']['preferred_username']) rtp_view = False - proxmox = connect_proxmox() + connect_proxmox() if user_view and not user.rtp: abort(403) elif user_view and user.rtp: @@ -155,8 +158,8 @@ def list_vms(user_view=None): @auth.oidc_auth def isos(): proxmox = connect_proxmox() - isos = get_isos(proxmox, app.config['PROXMOX_ISO_STORAGE']) - return json.dumps({"isos": isos}) + stored_isos = get_isos(proxmox, app.config['PROXMOX_ISO_STORAGE']) + return json.dumps({"isos": stored_isos}) @app.route("/hostname/") @@ -175,7 +178,7 @@ def hostname(name): @auth.oidc_auth def vm_details(vmid): user = User(session['userinfo']['preferred_username']) - proxmox = connect_proxmox() + connect_proxmox() if user.rtp or int(vmid) in user.allowed_vms: vm = VM(vmid) usage_check = user.check_usage(vm.cpu, vm.mem, 0) @@ -187,19 +190,19 @@ def vm_details(vmid): limits=user.limits, usage_check=usage_check) else: - abort(403) + return abort(403) @app.route("/vm//power/", methods=['POST']) @auth.oidc_auth def vm_power(vmid, action): user = User(session['userinfo']['preferred_username']) - proxmox = connect_proxmox() + connect_proxmox() if user.rtp or int(vmid) in user.allowed_vms: vm = VM(vmid) if action == 'start': - config = vm.config - usage_check = user.check_usage(config['cores'], config['memory'], + vmconfig = vm.config + usage_check = user.check_usage(vmconfig['cores'], vmconfig['memory'], 0) if usage_check: return usage_check @@ -235,7 +238,7 @@ def vm_console_stop(vmid): @auth.oidc_auth def vm_console(vmid): user = User(session['userinfo']['preferred_username']) - proxmox = connect_proxmox() + connect_proxmox() if user.rtp or int(vmid) in user.allowed_vms: vm = VM(vmid) stop_ssh_tunnel(vm.id, ssh_tunnels) @@ -255,7 +258,7 @@ def vm_console(vmid): @auth.oidc_auth def vm_cpu(vmid, cores): user = User(session['userinfo']['preferred_username']) - proxmox = connect_proxmox() + connect_proxmox() if user.rtp or int(vmid) in user.allowed_vms: vm = VM(vmid) cur_cores = vm.cpu @@ -276,7 +279,7 @@ def vm_cpu(vmid, cores): @auth.oidc_auth def vm_mem(vmid, mem): user = User(session['userinfo']['preferred_username']) - proxmox = connect_proxmox() + connect_proxmox() if user.rtp or int(vmid) in user.allowed_vms: vm = VM(vmid) cur_mem = vm.mem // 1024 @@ -297,10 +300,9 @@ def vm_mem(vmid, mem): @auth.oidc_auth def vm_disk(vmid, disk, size): user = User(session['userinfo']['preferred_username']) - proxmox = connect_proxmox() + connect_proxmox() if user.rtp or int(vmid) in user.allowed_vms: vm = VM(vmid) - cur_cores = vm.cpu usage_check = user.check_usage(0, 0, size) if usage_check: return usage_check @@ -314,7 +316,7 @@ def vm_disk(vmid, disk, size): @auth.oidc_auth def vm_renew(vmid): user = User(session['userinfo']['preferred_username']) - proxmox = connect_proxmox() + connect_proxmox() if user.rtp or int(vmid) in user.allowed_vms: vm = VM(vmid) renew_vm_expire(db, vmid, app.config['VM_EXPIRE_MONTHS']) @@ -330,7 +332,7 @@ def vm_renew(vmid): @auth.oidc_auth def iso_eject(vmid): user = User(session['userinfo']['preferred_username']) - proxmox = connect_proxmox() + connect_proxmox() if user.rtp or int(vmid) in user.allowed_vms: vm = VM(vmid) vm.eject_iso() @@ -343,7 +345,7 @@ def iso_eject(vmid): @auth.oidc_auth def iso_mount(vmid, iso): user = User(session['userinfo']['preferred_username']) - proxmox = connect_proxmox() + connect_proxmox() if user.rtp or int(vmid) in user.allowed_vms: iso = "{}:iso/{}".format(app.config['PROXMOX_ISO_STORAGE'], iso) vm = VM(vmid) @@ -357,7 +359,7 @@ def iso_mount(vmid, iso): @auth.oidc_auth def delete(vmid): user = User(session['userinfo']['preferred_username']) - proxmox = connect_proxmox() + connect_proxmox() if user.rtp or int(vmid) in user.allowed_vms: send_stop_ssh_tunnel(vmid) # Submit the delete VM task to RQ @@ -369,9 +371,9 @@ def delete(vmid): @app.route("/vm//boot_order", methods=['POST']) @auth.oidc_auth -def boot_order(vmid): +def get_boot_order(vmid): user = User(session['userinfo']['preferred_username']) - proxmox = connect_proxmox() + connect_proxmox() if user.rtp or int(vmid) in user.allowed_vms: boot_order = [] for key in sorted(request.form): @@ -390,7 +392,7 @@ def create(): proxmox = connect_proxmox() if user.active or user.rtp: if request.method == 'GET': - isos = get_isos(proxmox, app.config['PROXMOX_ISO_STORAGE']) + stored_isos = get_isos(proxmox, app.config['PROXMOX_ISO_STORAGE']) pools = get_pools(proxmox, db) templates = get_templates(db) return render_template( @@ -399,7 +401,7 @@ def create(): usage=user.usage, limits=user.limits, percents=user.usage_percent, - isos=isos, + isos=stored_isos, pools=pools, templates=templates) elif request.method == 'POST': @@ -449,6 +451,7 @@ def create(): timeout=600) return '', 200 return '', 200 + return None else: return '', 403 @@ -470,7 +473,7 @@ def set_limits(user): @auth.oidc_auth def delete_user(user): if 'rtp' in session['userinfo']['groups']: - proxmox = connect_proxmox() + connect_proxmox() User(user).delete() return '', 200 else: @@ -483,16 +486,16 @@ def settings(): user = User(session['userinfo']['preferred_username']) if user.rtp: templates = get_templates(db) - ignored_pools = get_ignored_pools(db) - allowed_users = get_allowed_users(db) + db_ignored_pools = get_ignored_pools(db) + db_allowed_users = get_allowed_users(db) return render_template( 'settings.html', user=user, templates=templates, - ignored_pools=ignored_pools, - allowed_users=allowed_users) + ignored_pools=db_ignored_pools, + allowed_users=db_allowed_users) else: - abort(403) + return abort(403) @app.route("/pool//ignore", methods=['POST', 'DELETE']) diff --git a/proxstar/auth.py b/proxstar/auth.py index 7f0e693..242f07e 100644 --- a/proxstar/auth.py +++ b/proxstar/auth.py @@ -8,5 +8,4 @@ def get_auth(app): app, issuer=app.config['OIDC_ISSUER'], client_registration_info=app.config['OIDC_CLIENT_CONFIG']) - auth return auth diff --git a/proxstar/db.py b/proxstar/db.py index 3c5a89d..588ee21 100644 --- a/proxstar/db.py +++ b/proxstar/db.py @@ -3,8 +3,8 @@ import datetime from dateutil.relativedelta import relativedelta from sqlalchemy import exists -from proxstar.ldapdb import * -from proxstar.models import (Allowed_Users, Base, Ignored_Pools, Pool_Cache, +from proxstar.ldapdb import is_rtp +from proxstar.models import (Allowed_Users, Ignored_Pools, Pool_Cache, Template, Usage_Limit, VM_Expiration) diff --git a/proxstar/ldapdb.py b/proxstar/ldapdb.py index a7c5fcd..2ddc31a 100644 --- a/proxstar/ldapdb.py +++ b/proxstar/ldapdb.py @@ -34,7 +34,7 @@ def is_current_student(user): def is_user(user): ldap = connect_ldap() try: - rtp_group = ldap.get_member(user, uid=True) + ldap.get_member(user, uid=True) return True except: return False diff --git a/proxstar/proxmox.py b/proxstar/proxmox.py index 89025c9..a9b76be 100644 --- a/proxstar/proxmox.py +++ b/proxstar/proxmox.py @@ -2,7 +2,7 @@ from flask import current_app as app from proxmoxer import ProxmoxAPI from proxstar import logging -from proxstar.db import get_ignored_pools, get_user_usage_limits +from proxstar.db import get_ignored_pools from proxstar.ldapdb import is_user @@ -14,7 +14,7 @@ def connect_proxmox(): user=app.config['PROXMOX_USER'], password=app.config['PROXMOX_PASS'], verify_ssl=False) - version = proxmox.version.get() + proxmox.version.get() return proxmox except: if app.config['PROXMOX_HOSTS'].index(host) == ( @@ -33,7 +33,7 @@ def connect_proxmox_ssh(): private_key_file='proxmox_ssh_key', password=app.config['PROXMOX_SSH_KEY_PASS'], backend='ssh_paramiko') - version = proxmox.version.get() + proxmox.version.get() return proxmox except: if app.config['PROXMOX_HOSTS'].index(host) == ( @@ -58,6 +58,7 @@ def get_vm_node(proxmox, vmid): for vm in proxmox.cluster.resources.get(type='vm'): if vm['vmid'] == int(vmid): return vm['node'] + return None def get_isos(proxmox, storage): diff --git a/proxstar/starrs.py b/proxstar/starrs.py index fa0d0a7..faa1664 100644 --- a/proxstar/starrs.py +++ b/proxstar/starrs.py @@ -72,7 +72,7 @@ def check_hostname(starrs, hostname): if c.fetchall(): available = False c.execute("COMMIT") - except (psycopg2.InternalError): + except psycopg2.InternalError: valid = False available = False finally: diff --git a/proxstar/tasks.py b/proxstar/tasks.py index 85d7a7c..8615bde 100644 --- a/proxstar/tasks.py +++ b/proxstar/tasks.py @@ -2,7 +2,6 @@ import logging import os import time -import paramiko import psycopg2 import requests from flask import Flask @@ -10,12 +9,11 @@ from rq import get_current_job from sqlalchemy import create_engine from sqlalchemy.orm import sessionmaker -from proxstar.db import * -from proxstar.mail import * +from proxstar.db import Base, get_vm_expire, delete_vm_expire, datetime, store_pool_cache, get_template +from proxstar.mail import send_vm_expire_email, send_rtp_vm_delete_email from proxstar.proxmox import connect_proxmox, get_pools -from proxstar.starrs import * +from proxstar.starrs import get_next_ip, register_starrs, delete_starrs from proxstar.user import User, get_vms_for_rtp -from proxstar.util import * from proxstar.vm import VM, clone_vm, create_vm from proxstar.vnc import send_stop_ssh_tunnel @@ -36,8 +34,8 @@ app.config.from_pyfile(config) def connect_db(): engine = create_engine(app.config['SQLALCHEMY_DATABASE_URI']) Base.metadata.bind = engine - DBSession = sessionmaker(bind=engine) - db = DBSession() + dbsession = sessionmaker(bind=engine) + db = dbsession() return db @@ -114,7 +112,7 @@ def process_expiring_vms_task(): with app.app_context(): proxmox = connect_proxmox() db = connect_db() - starrs = connect_starrs() + connect_starrs() pools = get_pools(proxmox, db) expired_vms = [] for pool in pools: @@ -125,7 +123,6 @@ def process_expiring_vms_task(): vm = VM(vm['vmid']) days = (vm.expire - datetime.date.today()).days if days in [10, 7, 3, 1, 0, -1, -2, -3, -4, -5, -6]: - name = vm.name expiring_vms.append([vm.id, vm.name, days]) if days <= 0: expired_vms.append([vm.id, vm.name, days]) @@ -158,7 +155,7 @@ def setup_template_task(template_id, name, user, ssh_key, cores, memory): db = connect_db() logging.info("[{}] Retrieving template info for template {}.".format( name, template_id)) - template = get_template(db, template_id) + get_template(db, template_id) logging.info("[{}] Cloning template {}.".format(name, template_id)) set_job_status(job, 'cloning template') vmid = clone_vm(proxmox, template_id, name, user) diff --git a/proxstar/user.py b/proxstar/user.py index a707dd8..483e96f 100644 --- a/proxstar/user.py +++ b/proxstar/user.py @@ -2,13 +2,13 @@ from proxmoxer.core import ResourceException from rq.registry import StartedJobRegistry from proxstar import db, q, redis_conn -from proxstar.db import * -from proxstar.proxmox import * -from proxstar.util import * +from proxstar.db import get_allowed_users, is_active, is_current_student, is_rtp, is_user, get_user_usage_limits +from proxstar.proxmox import connect_proxmox, get_pools +from proxstar.util import lazy_property from proxstar.vm import VM -class User(object): +class User(): def __init__(self, username): self.name = username self.active = is_active(self.name) or is_current_student( @@ -96,11 +96,12 @@ class User(object): def check_usage(self, cpu, mem, disk): if int(self.usage['cpu']) + int(cpu) > int(self.limits['cpu']): return 'exceeds_cpu_limit' - elif int(self.usage['mem']) + (int(mem) / 1024) > int( - self.limits['mem']): + elif int(self.usage['mem']) + (int(mem) / 1024) > int(self.limits['mem']): return 'exceeds_memory_limit' elif int(self.usage['disk']) + int(disk) > int(self.limits['disk']): return 'exceeds_disk_limit' + else: + return None def delete(self): proxmox = connect_proxmox() @@ -114,9 +115,9 @@ class User(object): self.name)).delete() -def get_vms_for_rtp(proxmox, db): +def get_vms_for_rtp(proxmox, database): pools = [] - for pool in get_pools(proxmox, db): + for pool in get_pools(proxmox, database): user = User(pool) pool_dict = dict() pool_dict['user'] = user.name diff --git a/proxstar/util.py b/proxstar/util.py index 598614a..2ef474b 100644 --- a/proxstar/util.py +++ b/proxstar/util.py @@ -1,5 +1,4 @@ import random -import string def gen_password( diff --git a/proxstar/vm.py b/proxstar/vm.py index 1504758..7b76a0c 100644 --- a/proxstar/vm.py +++ b/proxstar/vm.py @@ -1,5 +1,4 @@ import json -import time import urllib from flask import current_app as app @@ -12,7 +11,7 @@ from proxstar.starrs import get_ip_for_mac from proxstar.util import lazy_property -class VM(object): +class VM(): def __init__(self, vmid): self.id = vmid @@ -52,6 +51,7 @@ class VM(object): for vm in proxmox.cluster.resources.get(type='vm'): if vm['vmid'] == int(self.id): return vm['node'] + return None @retry(wait=wait_fixed(2), stop=stop_after_attempt(5)) def delete(self): @@ -127,8 +127,8 @@ class VM(object): } raw_boot_order = self.config.get('boot', 'cdn') boot_order = [] - for i in range(0, len(raw_boot_order)): - boot_order.append(boot_order_lookup[raw_boot_order[i]]) + for order in raw_boot_order: + boot_order.append(boot_order_lookup[order]) return boot_order @lazy_property @@ -145,14 +145,14 @@ class VM(object): 'Network': 'n' } raw_boot_order = '' - for i in range(0, len(boot_order)): - raw_boot_order += boot_order_lookup[boot_order[i]] + for order in boot_order: + raw_boot_order += boot_order_lookup[order] proxmox.nodes(self.node).qemu(self.id).config.put(boot=raw_boot_order) @lazy_property def interfaces(self): interfaces = [] - for key, val in self.config.items(): + for key, _ in self.config.items(): if 'net' in key: mac = self.config[key].split(',') valid_int_types = ['virtio', 'e1000', 'rtl8139', 'vmxnet3'] diff --git a/proxstar/vnc.py b/proxstar/vnc.py index 2579a7b..d133f16 100644 --- a/proxstar/vnc.py +++ b/proxstar/vnc.py @@ -7,7 +7,7 @@ from flask import current_app as app from sshtunnel import SSHTunnelForwarder from proxstar import logging -from proxstar.util import * +from proxstar.util import gen_password def stop_websockify(): diff --git a/requirements.txt b/requirements.txt index 7807885..677962f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -17,3 +17,5 @@ sqlalchemy==1.2.12 sshtunnel==0.1.4 tenacity==5.0.2 websockify==0.8.0 +pylint==2.3.1 +pylint-quotes==0.2.1