From d81f428b5b35c112cea5f25e2e17ff964b5e7bd1 Mon Sep 17 00:00:00 2001 From: Quan Pham Date: Fri, 26 Jan 2024 11:50:54 -0500 Subject: [PATCH 1/4] Copied over Account Manager before intergration --- .../acct_mgt/__init__.py | 0 src/coldfront_plugin_cloud/acct_mgt/app.py | 242 +++++++++ .../acct_mgt/defaults.py | 5 + .../acct_mgt/exceptions.py | 32 ++ .../acct_mgt/moc_openshift.py | 468 ++++++++++++++++++ src/coldfront_plugin_cloud/acct_mgt/wsgi.py | 13 + 6 files changed, 760 insertions(+) create mode 100644 src/coldfront_plugin_cloud/acct_mgt/__init__.py create mode 100644 src/coldfront_plugin_cloud/acct_mgt/app.py create mode 100644 src/coldfront_plugin_cloud/acct_mgt/defaults.py create mode 100644 src/coldfront_plugin_cloud/acct_mgt/exceptions.py create mode 100644 src/coldfront_plugin_cloud/acct_mgt/moc_openshift.py create mode 100644 src/coldfront_plugin_cloud/acct_mgt/wsgi.py diff --git a/src/coldfront_plugin_cloud/acct_mgt/__init__.py b/src/coldfront_plugin_cloud/acct_mgt/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/coldfront_plugin_cloud/acct_mgt/app.py b/src/coldfront_plugin_cloud/acct_mgt/app.py new file mode 100644 index 00000000..83e6e0f5 --- /dev/null +++ b/src/coldfront_plugin_cloud/acct_mgt/app.py @@ -0,0 +1,242 @@ +"""Flask application for MOC openshift account management microservice""" + +import os + +from flask import Flask, make_response, request +from flask_httpauth import HTTPBasicAuth + +import kubernetes.config +import kubernetes.client +import kubernetes.dynamic.exceptions as kexc +from openshift.dynamic import DynamicClient + +from . import defaults +from . import moc_openshift +from . import exceptions + +ENVPREFIX = "ACCT_MGT_" + + +def env_config(): + """Get configuration values from environment variables. + + Look up all environment variables that start with ENVPREFIX (by default + "ACCT_MGT_"), strip the prefix, and store them in a dictionary. Return the + dictionary to the caller. + """ + + return { + k[len(ENVPREFIX) :]: v for k, v in os.environ.items() if k.startswith(ENVPREFIX) + } + + +def get_dynamic_client(logger): + try: + k8s_client = kubernetes.config.new_client_from_config() + logger.info("using kubeconfig credentials") + except kubernetes.config.config_exception.ConfigException: + kubernetes.config.load_incluster_config() + k8s_client = kubernetes.client.ApiClient() + logger.info("using in-cluster credentials") + return DynamicClient(k8s_client) + + +def get_openshift(client, logger, config): + return moc_openshift.MocOpenShift4x(client, logger, config) + + +# pylint: disable=too-many-statements,too-many-locals,redefined-outer-name +def create_app(**config): + APP = Flask(__name__) + AUTH = HTTPBasicAuth() + + APP.config.from_object(defaults) + APP.config.from_mapping(config) + + # Allow unit tests to explicitly disable environment configuration + if not APP.config.get("DISABLE_ENV_CONFIG", False): + APP.config.from_mapping(env_config()) + + dyn_client = get_dynamic_client(APP.logger) + shift = get_openshift(dyn_client, APP.logger, APP.config) + + @AUTH.verify_password + def verify_password(username, password): + """Validates a username and password.""" + + return ( + APP.config.get("AUTH_DISABLED", "false").lower() + in ("true", "t", "yes", "1") + ) or ( + username == APP.config["ADMIN_USERNAME"] + and password == APP.config["ADMIN_PASSWORD"] + ) + + @APP.errorhandler(exceptions.ApiException) + def handle_acct_mgt_errors(error): + msg = error.message if error.visible else "Internal Server Error" + APP.logger.error(msg) + return make_response({"msg": msg}, error.status_code) + + @APP.errorhandler(kexc.DynamicApiError) + def handle_openshift_api_errors(error): + msg = f"Unexpected response from OpenShift API: {error.summary()}" + APP.logger.error(msg) + return make_response({"msg": msg}, 400) + + @APP.errorhandler(ValueError) + def handle_value_errors(error): + msg = f"Invalid value in input: {error}" + APP.logger.error(msg) + return make_response({"msg": msg}, 400) + + @APP.route( + "/users//projects//roles/", methods=["GET"] + ) + @AUTH.login_required + def get_moc_rolebindings(project_name, user_name, role): + # role can be one of admin, edit, view + if shift.user_rolebinding_exists(user_name, project_name, role): + return {"msg": f"user role exists ({project_name},{user_name},{role})"} + + return make_response( + {"msg": f"user role does not exist ({project_name},{user_name},{role})"}, + 404, + ) + + @APP.route( + "/users//projects//roles/", methods=["PUT"] + ) + @AUTH.login_required + def create_moc_rolebindings(project_name, user_name, role): + # role can be one of admin, edit, view + return shift.add_user_to_role(project_name, user_name, role) + + @APP.route( + "/users//projects//roles/", methods=["DELETE"] + ) + @AUTH.login_required + def delete_moc_rolebindings(project_name, user_name, role): + # role can be one of admin, edit, view + return shift.remove_user_from_role(project_name, user_name, role) + + @APP.route("/projects/", methods=["GET"]) + @AUTH.login_required + def get_moc_project(project_name): + if shift.project_exists(project_name): + return make_response( + {"msg": f"project exists ({project_name})"}, + ) + return make_response({"msg": f"project does not exist ({project_name})"}, 400) + + @APP.route("/projects/", methods=["PUT"]) + @APP.route("/projects//owner/", methods=["PUT"]) + @AUTH.login_required + def create_moc_project(project_name, user_name=None): + # first check the project_name is a valid openshift project name + suggested_project_name = shift.cnvt_project_name(project_name) + if project_name != suggested_project_name: + raise exceptions.BadRequest( + "project name must match regex '[a-z0-9]([-a-z0-9]*[a-z0-9])?'." + f" Suggested name: {suggested_project_name}." + ) + + if shift.project_exists(project_name): + raise exceptions.Conflict("project already exists.") + + payload = request.get_json(silent=True) or {} + display_name = payload.pop("displayName", project_name) + annotations = payload.pop("annotations", {}) + labels = payload.pop("labels", {}) + + shift.create_project( + project_name, + display_name, + user_name, + annotations=annotations, + labels=labels, + ) + return {"msg": f"project created ({project_name})"} + + @APP.route("/projects/", methods=["DELETE"]) + @AUTH.login_required + def delete_moc_project(project_name): + if shift.project_exists(project_name): + shift.delete_project(project_name) + + return make_response({"msg": f"project deleted ({project_name})"}) + + @APP.route("/users/", methods=["GET"]) + @AUTH.login_required + def get_moc_user(user_name): + if shift.user_exists(user_name): + return make_response({"msg": f"user ({user_name}) exists"}) + return make_response({"msg": f"user ({user_name}) does not exist"}, 404) + + @APP.route("/users/", methods=["PUT"]) + @AUTH.login_required + def create_moc_user(user_name): + # these three values should be added to generalize this function + # full_name - the full name of the user as it is really convenient + # id_provider - this is in the yaml configuration for this project - needed in the past + + full_name = user_name + id_user = user_name # until we support different user names see above. + + created = False + + if not shift.user_exists(user_name): + created = True + shift.create_user(user_name, full_name) + + if not shift.identity_exists(id_user): + created = True + shift.create_identity(id_user) + + if not shift.useridentitymapping_exists(user_name, id_user): + created = True + shift.create_useridentitymapping(user_name, id_user) + + if created: + return make_response({"msg": f"user created ({user_name})"}) + return make_response({"msg": f"user already exists ({user_name})"}, 400) + + @APP.route("/users/", methods=["DELETE"]) + @AUTH.login_required + def delete_moc_user(user_name): + if shift.user_exists(user_name): + shift.delete_user(user_name) + + if shift.identity_exists(user_name): + shift.delete_identity(user_name) + + return make_response({"msg": f"user deleted ({user_name})"}) + + @APP.route("/projects//quota", methods=["GET"]) + @AUTH.login_required + def get_quota(project): + return shift.get_moc_quota(project) + + @APP.route("/projects//quota", methods=["PUT", "POST"]) + @AUTH.login_required + def put_quota(project): + moc_quota = request.get_json(force=True) + return shift.update_moc_quota(project, moc_quota, patch=False) + + @APP.route("/projects//quota", methods=["PATCH"]) + @AUTH.login_required + def patch_quota(project): + moc_quota = request.get_json(force=True) + return shift.update_moc_quota(project, moc_quota, patch=True) + + @APP.route("/projects//quota", methods=["DELETE"]) + @AUTH.login_required + def delete_quota(project): + return shift.delete_moc_quota(project) + + @APP.route("/projects//users", methods=["GET"]) + @AUTH.login_required + def get_users_in_project(project): + return shift.get_users_in_project(project) + + return APP diff --git a/src/coldfront_plugin_cloud/acct_mgt/defaults.py b/src/coldfront_plugin_cloud/acct_mgt/defaults.py new file mode 100644 index 00000000..07e63714 --- /dev/null +++ b/src/coldfront_plugin_cloud/acct_mgt/defaults.py @@ -0,0 +1,5 @@ +"""Default values for Flask app configuration""" + +ADMIN_USERNAME = "admin" +QUOTA_DEF_FILE = "quotas.json" +LIMIT_DEF_FILE = "limits.json" diff --git a/src/coldfront_plugin_cloud/acct_mgt/exceptions.py b/src/coldfront_plugin_cloud/acct_mgt/exceptions.py new file mode 100644 index 00000000..923b516a --- /dev/null +++ b/src/coldfront_plugin_cloud/acct_mgt/exceptions.py @@ -0,0 +1,32 @@ +"""Defined exceptions used by acct-mgt.""" + + +class ApiException(Exception): + """Base exception class for errors. + + All exceptions subclassing ApiException will be caught from Flask's + error handler and return the appropriate status code and message. + The visible parameter controls whether the error message is visible + to the end user. + """ + + status_code = 500 + visible = True + default_message = "Internal Server Error." + + def __init__(self, message=None): + self.message = message or self.default_message + + +class BadRequest(ApiException): + """Exception class for invalid requests.""" + + status_code = 400 + default_message = "Invalid Request." + + +class Conflict(BadRequest): + """Exception class for requests that create already existing resources.""" + + status_code = 409 + default_message = "Resource already exists." diff --git a/src/coldfront_plugin_cloud/acct_mgt/moc_openshift.py b/src/coldfront_plugin_cloud/acct_mgt/moc_openshift.py new file mode 100644 index 00000000..3c9917af --- /dev/null +++ b/src/coldfront_plugin_cloud/acct_mgt/moc_openshift.py @@ -0,0 +1,468 @@ +"""API wrapper for interacting with OpenShift authorization""" +import json +import re +import sys +import time + +import kubernetes.dynamic.exceptions as kexc + +OPENSHIFT_ROLES = ["admin", "edit", "view"] + +API_PROJECT = "project.openshift.io/v1" +API_USER = "user.openshift.io/v1" +API_RBAC = "rbac.authorization.k8s.io/v1" +API_CORE = "v1" +IGNORED_ATTRIBUTES = [ + "resourceVersion", + "creationTimestamp", + "uid", +] + + +def clean_openshift_metadata(obj): + if "metadata" in obj: + for attr in IGNORED_ATTRIBUTES: + if attr in obj["metadata"]: + del obj["metadata"][attr] + + return obj + + +# pylint: disable=too-many-public-methods +class MocOpenShift4x: + """API implementation for OpenShift 4.x""" + + @staticmethod + def split_quota_name(moc_quota_name): + name_array = moc_quota_name.split(":") + return name_array[0] or "Project", name_array[1] + + @staticmethod + def cnvt_project_name(project_name): + suggested_project_name = re.sub("^[^A-Za-z0-9]+", "", project_name.lower()) + suggested_project_name = re.sub("[^A-Za-z0-9]+$", "", suggested_project_name) + suggested_project_name = re.sub("[^A-Za-z0-9-]+", "-", suggested_project_name) + return suggested_project_name + + @staticmethod + def user_in_rolebinding(user_name, rolebinding): + return [ + subject + for subject in rolebinding["subjects"] + if subject["kind"] == "User" and subject["name"] == user_name + ] + + @staticmethod + def validate_role(role): + if role not in OPENSHIFT_ROLES: + raise ValueError( + f"Invalid role, {role} is not one of {', '.join(OPENSHIFT_ROLES)}" + ) + + def __init__(self, client, logger, config): + self.client = client + self.logger = logger + self.id_provider = config["IDENTITY_PROVIDER"] + self.quotafile = config["QUOTA_DEF_FILE"] + self.limitfile = config["LIMIT_DEF_FILE"] + self.apis = {} + + if not self.limitfile: + self.logger.error("No default limit file provided.") + sys.exit(1) + + def get_resource_api(self, api_version: str, kind: str): + """Either return the cached resource api from self.apis, or fetch a + new one, store it in self.apis, and return it.""" + k = f"{api_version}:{kind}" + api = self.apis.setdefault( + k, self.client.resources.get(api_version=api_version, kind=kind) + ) + return api + + def useridentitymapping_exists(self, user_name, id_user): + try: + user = self.get_user(user_name) + except kexc.NotFoundError: + return False + + return any( + identity == self.qualified_id_user(id_user) + for identity in user.get("identities", []) + ) + + def user_rolebinding_exists(self, user_name, project_name, role): + self.validate_role(role) + + try: + result = self.get_rolebindings(project_name, role) + except kexc.NotFoundError: + return False + + return any( + (subject["kind"] == "User" and subject["name"] == user_name) + for subject in result["subjects"] + ) + + def add_user_to_role(self, project_name, user_name, role): + self.validate_role(role) + + try: + rolebinding = self.get_rolebindings(project_name, role) + + if not self.user_in_rolebinding(user_name, rolebinding): + rolebinding["subjects"].append({"kind": "User", "name": user_name}) + self.update_rolebindings(project_name, rolebinding) + except kexc.NotFoundError: + rolebinding = self.create_rolebindings(project_name, user_name, role) + + return { + "msg": f"added user {user_name} to role {role} in {project_name}", + } + + def remove_user_from_role(self, project_name, user_name, role): + self.validate_role(role) + + try: + rolebinding = self.get_rolebindings(project_name, role) + + for subject in self.user_in_rolebinding(user_name, rolebinding): + rolebinding["subjects"].remove(subject) + + self.update_rolebindings(project_name, rolebinding) + except kexc.NotFoundError: + pass + + return { + "msg": f"removed user {user_name} from role {role} in {project_name}", + } + + def update_moc_quota(self, project_name, new_quota, patch=False): + """This will update resourcequota objects in a project and create new + ones based on the new_quota specification""" + quota_def = self.get_quota_definitions() + + if patch: + existing_quota = self.get_moc_quota_from_resourcequotas(project_name) + for quota, value in existing_quota.items(): + quota_def.setdefault(quota, {})["value"] = value + + for quota, value in new_quota["Quota"].items(): + quota_def[quota]["value"] = value + self.logger.info( + f"New Quota for project {project_name}: {json.dumps(new_quota, indent=2)}" + ) + + self.delete_moc_quota(project_name) + self.create_shift_quotas(project_name, quota_def) + + return {"msg": "MOC quotas updated"} + + def get_quota_definitions(self): + self.logger.info("reading quotas from %s", self.quotafile) + with open(self.quotafile, "r") as file: + quota = json.load(file) + for k in quota: + quota[k]["value"] = None + + return quota + + def get_limit_definitions(self): + with open(self.limitfile, "r") as file: + return json.load(file) + + def get_project(self, project_name): + api = self.get_resource_api(API_PROJECT, "Project") + return clean_openshift_metadata(api.get(name=project_name).to_dict()) + + def project_exists(self, project_name): + try: + self.get_project(project_name) + except kexc.NotFoundError: + return False + return True + + # pylint: disable-msg=too-many-arguments + def create_project( + self, project_name, display_name, user_name, annotations=None, labels=None + ): + if annotations is None: + annotations = {} + else: + annotations = dict(annotations) + + api = self.get_resource_api(API_PROJECT, "Project") + + annotations.update( + { + "openshift.io/display-name": display_name, + "openshift.io/requester": user_name, + } + ) + + _nerc_project_label = { + "nerc.mghpcc.org/project": "true", + } + + if labels is None: + labels = _nerc_project_label + else: + labels = dict(labels) + labels.update(_nerc_project_label) + + payload = { + "metadata": { + "name": project_name, + "annotations": annotations, + "labels": labels, + }, + } + res = api.create(body=payload).to_dict() + self.create_limits(project_name) + return res + + # pylint: enable-msg=too-many-arguments + + def delete_project(self, project_name): + api = self.get_resource_api(API_PROJECT, "Project") + return api.delete(name=project_name).to_dict() + + def get_user(self, user_name): + api = self.get_resource_api(API_USER, "User") + return clean_openshift_metadata(api.get(name=user_name).to_dict()) + + def user_exists(self, user_name): + try: + self.get_user(user_name) + except kexc.NotFoundError: + return False + return True + + def create_user(self, user_name, full_name): + api = self.get_resource_api(API_USER, "User") + payload = { + "metadata": {"name": user_name}, + "fullName": full_name, + } + return api.create(body=payload).to_dict() + + def delete_user(self, user_name): + api = self.get_resource_api(API_USER, "User") + return api.delete(name=user_name).to_dict() + + def qualified_id_user(self, id_user): + return f"{self.id_provider}:{id_user}" + + def get_identity(self, id_user): + api = self.get_resource_api(API_USER, "Identity") + return clean_openshift_metadata( + api.get(name=self.qualified_id_user(id_user)).to_dict() + ) + + def identity_exists(self, id_user): + try: + self.get_identity(id_user) + except kexc.NotFoundError: + return False + return True + + def create_identity(self, id_user): + api = self.get_resource_api(API_USER, "Identity") + + payload = { + "providerName": self.id_provider, + "providerUserName": id_user, + } + return api.create(body=payload).to_dict() + + def delete_identity(self, id_user): + api = self.get_resource_api(API_USER, "Identity") + return api.delete(name=self.qualified_id_user(id_user)).to_dict() + + def create_useridentitymapping(self, user_name, id_user): + api = self.get_resource_api(API_USER, "UserIdentityMapping") + payload = { + "user": {"name": user_name}, + "identity": {"name": self.qualified_id_user(id_user)}, + } + return api.create(body=payload).to_dict() + + # member functions to associate roles for users on projects + def get_rolebindings(self, project_name, role): + api = self.get_resource_api(API_RBAC, "RoleBinding") + res = clean_openshift_metadata( + api.get(namespace=project_name, name=role).to_dict() + ) + + # Ensure that rbd["subjects"] is a list (it can be None if the + # rolebinding object had no subjects). + if not res.get("subjects"): + res["subjects"] = [] + + return res + + def list_rolebindings(self, project_name): + api = self.get_resource_api(API_RBAC, "RoleBinding") + try: + res = clean_openshift_metadata(api.get(namespace=project_name).to_dict()) + except kexc.NotFoundError: + return [] + + return res["items"] + + def create_rolebindings(self, project_name, user_name, role): + api = self.get_resource_api(API_RBAC, "RoleBinding") + payload = { + "metadata": {"name": role, "namespace": project_name}, + "subjects": [{"name": user_name, "kind": "User"}], + "roleRef": {"name": role, "kind": "ClusterRole"}, + } + return api.create(body=payload, namespace=project_name).to_dict() + + def update_rolebindings(self, project_name, rolebinding): + api = self.get_resource_api(API_RBAC, "RoleBinding") + return api.patch(body=rolebinding, namespace=project_name).to_dict() + + def get_moc_quota(self, project_name): + quota_from_project = self.get_moc_quota_from_resourcequotas(project_name) + + quota = {} + for quota_name, quota_value in quota_from_project.items(): + if quota_value: + quota[quota_name] = quota_value + + quota_object = { + "Version": "0.9", + "Kind": "MocQuota", + "ProjectName": project_name, + "Quota": quota, + } + return quota_object + + def wait_for_quota_to_settle(self, project_name, resource_quota): + """Wait for quota on resourcequotas to settle. + + When creating a new resourcequota that sets a quota on resourcequota objects, we need to + wait for OpenShift to calculate the quota usage before we attempt to create any new + resourcequota objects. + """ + + if "resourcequotas" in resource_quota["spec"]["hard"]: + self.logger.info("waiting for resourcequota quota") + + api = self.get_resource_api(API_CORE, "ResourceQuota") + while True: + resp = clean_openshift_metadata( + api.get( + namespace=project_name, name=resource_quota["metadata"]["name"] + ).to_dict() + ) + if "resourcequotas" in resp["status"].get("used", {}): + break + time.sleep(0.1) + + def create_shift_quotas(self, project_name, quota_spec): + quota_def = {} + # separate the quota_spec by quota_scope + for mangled_quota_name in quota_spec: + (scope, quota_name) = self.split_quota_name(mangled_quota_name) + quota_def.setdefault(scope, {}) + quota_def[scope][quota_name] = quota_spec[mangled_quota_name] + + for scope, quota_item in quota_def.items(): + resource_quota = { + "metadata": {"name": f"{project_name.lower()}-{scope.lower()}"}, + "spec": {"hard": {}}, + } + + if scope != "Project": + resource_quota["spec"]["scopes"] = [scope] + + resource_quota["spec"]["hard"] = { + quota_name: quota_item[quota_name]["value"] + for quota_name in quota_item + if quota_item[quota_name]["value"] is not None + } + + if resource_quota["spec"]["hard"]: + api = self.get_resource_api(API_CORE, "ResourceQuota") + res = api.create(namespace=project_name, body=resource_quota).to_dict() + self.wait_for_quota_to_settle(project_name, res) + + return {"msg": f"All quotas for {project_name} successfully created"} + + def get_resourcequotas(self, project_name): + """Returns a list of all of the resourcequota objects""" + # Raise a NotFound error if the project doesn't exist + self.get_project(project_name) + + api = self.get_resource_api(API_CORE, "ResourceQuota") + res = clean_openshift_metadata(api.get(namespace=project_name).to_dict()) + + return res["items"] + + def delete_resourcequota(self, project_name, resourcequota_name): + """In an openshift namespace {project_name) delete a specified resourcequota""" + api = self.get_resource_api(API_CORE, "ResourceQuota") + return api.delete(namespace=project_name, name=resourcequota_name).to_dict() + + def delete_moc_quota(self, project_name): + """deletes all resourcequotas from an openshift project""" + resourcequotas = self.get_resourcequotas(project_name) + for resourcequota in resourcequotas: + self.delete_resourcequota(project_name, resourcequota["metadata"]["name"]) + + return {"msg": f"All quotas for {project_name} successfully deleted"} + + def get_moc_quota_from_resourcequotas(self, project_name): + """This returns a dictionary suitable for merging in with the + specification from Adjutant/ColdFront""" + resourcequotas = self.get_resourcequotas(project_name) + moc_quota = {} + for rq in resourcequotas: + name, spec = rq["metadata"]["name"], rq["spec"] + self.logger.info(f"processing resourcequota: {project_name}:{name}") + scope_list = spec.get("scopes", [""]) + for quota_name, quota_value in spec.get("hard", {}).items(): + for scope_item in scope_list: + moc_quota_name = f"{scope_item}:{quota_name}" + moc_quota.setdefault(moc_quota_name, quota_value) + return moc_quota + + def create_limits(self, project_name, limits=None): + """ + project_name: project_name in which to create LimitRange + limits: dictionary of limits to create, or None for default + """ + api = self.get_resource_api(API_CORE, "LimitRange") + + payload = { + "metadata": {"name": f"{project_name.lower()}-limits"}, + "spec": {"limits": limits or self.get_limit_definitions()}, + } + return api.create(body=payload, namespace=project_name).to_dict() + + def get_users_in_project(self, project_name): + """ + Returns a list of users that have a role in a given project/namespace + """ + # Raise a NotFound error if the project doesn't exist + self.get_project(project_name) + + users = set() + role_binding_list = [] + + for role in OPENSHIFT_ROLES: + try: + role_binding_list.append(self.get_rolebindings(project_name, role)) + except kexc.NotFoundError: + continue + + for role_binding in role_binding_list: + users.update( + subject["name"] + for subject in role_binding["subjects"] + if subject["kind"] == "User" + ) + + return list(users) diff --git a/src/coldfront_plugin_cloud/acct_mgt/wsgi.py b/src/coldfront_plugin_cloud/acct_mgt/wsgi.py new file mode 100644 index 00000000..1125b462 --- /dev/null +++ b/src/coldfront_plugin_cloud/acct_mgt/wsgi.py @@ -0,0 +1,13 @@ +"""WSGI wrapper for tooling that expects a top-level application object""" + +import logging + +from . import app + +APP = app.create_app() + +if __name__ == "__main__": + APP.run() +else: + APP.logger = logging.getLogger("gunicorn.error") + APP.logger.setLevel(logging.INFO) From 357815dc3148244c025eb1c68d50cba974239338 Mon Sep 17 00:00:00 2001 From: Quan Pham Date: Tue, 16 Jan 2024 14:09:37 -0500 Subject: [PATCH 2/4] Integrated account-manager into openshift allocator object, allowing the plugin to communicate with any Openshift cluster --- .../test-py39-functional-microshift.yaml | 3 +- ci/microshift.sh | 13 +- ci/run_functional_tests_openshift.sh | 9 +- requirements.txt | 4 + src/coldfront_plugin_cloud/acct_mgt/app.py | 242 --------------- .../acct_mgt/defaults.py | 5 - .../acct_mgt/exceptions.py | 32 -- .../acct_mgt/moc_openshift.py | 40 ++- src/coldfront_plugin_cloud/acct_mgt/wsgi.py | 13 - src/coldfront_plugin_cloud/attributes.py | 2 + .../commands/add_openshift_resource.py | 15 +- src/coldfront_plugin_cloud/openshift.py | 291 +++++++++++------- src/coldfront_plugin_cloud/tests/base.py | 5 +- .../functional/openshift/test_allocation.py | 3 +- .../unit/test_calculate_quota_unit_hours.py | 7 + test-requirements.txt | 2 + 16 files changed, 246 insertions(+), 440 deletions(-) delete mode 100644 src/coldfront_plugin_cloud/acct_mgt/app.py delete mode 100644 src/coldfront_plugin_cloud/acct_mgt/defaults.py delete mode 100644 src/coldfront_plugin_cloud/acct_mgt/exceptions.py delete mode 100644 src/coldfront_plugin_cloud/acct_mgt/wsgi.py diff --git a/.github/workflows/test-py39-functional-microshift.yaml b/.github/workflows/test-py39-functional-microshift.yaml index e3470d86..f004b2ea 100644 --- a/.github/workflows/test-py39-functional-microshift.yaml +++ b/.github/workflows/test-py39-functional-microshift.yaml @@ -9,7 +9,6 @@ on: env: PYTHONWARNINGS: ignore KUBECONFIG: ${{ github.workspace }}/kubeconfig - ACCT_MGT_VERSION: "6012025c247ab25fb2cab3be9ad06080e28713ee" jobs: build: @@ -35,7 +34,7 @@ jobs: run: | bash ./ci/setup-oc-client.sh - - name: Install Microshift + - name: Install Microshift and add service account run: | ./ci/microshift.sh diff --git a/ci/microshift.sh b/ci/microshift.sh index 660ad5bf..2dcc7f99 100755 --- a/ci/microshift.sh +++ b/ci/microshift.sh @@ -44,14 +44,5 @@ while ! oc get route -A; do done echo "::endgroup::" -# Install OpenShift Account Management -git clone "${ACCT_MGT_REPOSITORY}" "$test_dir/openshift-acct-mgt" -git -C "$test_dir/openshift-acct-mgt" config advice.detachedHead false -git -C "$test_dir/openshift-acct-mgt" checkout "$ACCT_MGT_VERSION" - -echo "::group::Deploy openshift-acct-mgt" -oc apply -k "$test_dir/openshift-acct-mgt/k8s/overlays/crc" -oc wait -n onboarding --for=condition=available --timeout=800s deployment/onboarding -echo "::endgroup::" - -sleep 60 +oc create sa coldfront +oc adm policy add-cluster-role-to-user cluster-admin system:serviceaccount:default:coldfront diff --git a/ci/run_functional_tests_openshift.sh b/ci/run_functional_tests_openshift.sh index d10b2a21..10f6e8ae 100755 --- a/ci/run_functional_tests_openshift.sh +++ b/ci/run_functional_tests_openshift.sh @@ -3,16 +3,17 @@ # Tests expect the resource to be name Devstack set -xe -export OPENSHIFT_MICROSHIFT_USERNAME="admin" -export OPENSHIFT_MICROSHIFT_PASSWORD="pass" - if [[ ! "${CI}" == "true" ]]; then source /tmp/coldfront_venv/bin/activate fi export DJANGO_SETTINGS_MODULE="local_settings" export FUNCTIONAL_TESTS="True" -export OS_AUTH_URL="https://onboarding-onboarding.cluster.local" + +export OS_AUTH_URL="https://onboarding-onboarding.cluster.local:6443" +export ACCT_MGT_IDENTITY_PROVIDER=developer #TODO: Replace this with resource attribute instead + +export OPENSHIFT_MICROSHIFT_TOKEN="$(oc create token coldfront)" coverage run --source="." -m django test coldfront_plugin_cloud.tests.functional.openshift coverage report diff --git a/requirements.txt b/requirements.txt index 2a3ce5af..6057e43d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,3 +6,7 @@ python-memcached==1.59 python-novaclient python-neutronclient python-swiftclient +python-dotenv +gunicorn +kubernetes +openshift diff --git a/src/coldfront_plugin_cloud/acct_mgt/app.py b/src/coldfront_plugin_cloud/acct_mgt/app.py deleted file mode 100644 index 83e6e0f5..00000000 --- a/src/coldfront_plugin_cloud/acct_mgt/app.py +++ /dev/null @@ -1,242 +0,0 @@ -"""Flask application for MOC openshift account management microservice""" - -import os - -from flask import Flask, make_response, request -from flask_httpauth import HTTPBasicAuth - -import kubernetes.config -import kubernetes.client -import kubernetes.dynamic.exceptions as kexc -from openshift.dynamic import DynamicClient - -from . import defaults -from . import moc_openshift -from . import exceptions - -ENVPREFIX = "ACCT_MGT_" - - -def env_config(): - """Get configuration values from environment variables. - - Look up all environment variables that start with ENVPREFIX (by default - "ACCT_MGT_"), strip the prefix, and store them in a dictionary. Return the - dictionary to the caller. - """ - - return { - k[len(ENVPREFIX) :]: v for k, v in os.environ.items() if k.startswith(ENVPREFIX) - } - - -def get_dynamic_client(logger): - try: - k8s_client = kubernetes.config.new_client_from_config() - logger.info("using kubeconfig credentials") - except kubernetes.config.config_exception.ConfigException: - kubernetes.config.load_incluster_config() - k8s_client = kubernetes.client.ApiClient() - logger.info("using in-cluster credentials") - return DynamicClient(k8s_client) - - -def get_openshift(client, logger, config): - return moc_openshift.MocOpenShift4x(client, logger, config) - - -# pylint: disable=too-many-statements,too-many-locals,redefined-outer-name -def create_app(**config): - APP = Flask(__name__) - AUTH = HTTPBasicAuth() - - APP.config.from_object(defaults) - APP.config.from_mapping(config) - - # Allow unit tests to explicitly disable environment configuration - if not APP.config.get("DISABLE_ENV_CONFIG", False): - APP.config.from_mapping(env_config()) - - dyn_client = get_dynamic_client(APP.logger) - shift = get_openshift(dyn_client, APP.logger, APP.config) - - @AUTH.verify_password - def verify_password(username, password): - """Validates a username and password.""" - - return ( - APP.config.get("AUTH_DISABLED", "false").lower() - in ("true", "t", "yes", "1") - ) or ( - username == APP.config["ADMIN_USERNAME"] - and password == APP.config["ADMIN_PASSWORD"] - ) - - @APP.errorhandler(exceptions.ApiException) - def handle_acct_mgt_errors(error): - msg = error.message if error.visible else "Internal Server Error" - APP.logger.error(msg) - return make_response({"msg": msg}, error.status_code) - - @APP.errorhandler(kexc.DynamicApiError) - def handle_openshift_api_errors(error): - msg = f"Unexpected response from OpenShift API: {error.summary()}" - APP.logger.error(msg) - return make_response({"msg": msg}, 400) - - @APP.errorhandler(ValueError) - def handle_value_errors(error): - msg = f"Invalid value in input: {error}" - APP.logger.error(msg) - return make_response({"msg": msg}, 400) - - @APP.route( - "/users//projects//roles/", methods=["GET"] - ) - @AUTH.login_required - def get_moc_rolebindings(project_name, user_name, role): - # role can be one of admin, edit, view - if shift.user_rolebinding_exists(user_name, project_name, role): - return {"msg": f"user role exists ({project_name},{user_name},{role})"} - - return make_response( - {"msg": f"user role does not exist ({project_name},{user_name},{role})"}, - 404, - ) - - @APP.route( - "/users//projects//roles/", methods=["PUT"] - ) - @AUTH.login_required - def create_moc_rolebindings(project_name, user_name, role): - # role can be one of admin, edit, view - return shift.add_user_to_role(project_name, user_name, role) - - @APP.route( - "/users//projects//roles/", methods=["DELETE"] - ) - @AUTH.login_required - def delete_moc_rolebindings(project_name, user_name, role): - # role can be one of admin, edit, view - return shift.remove_user_from_role(project_name, user_name, role) - - @APP.route("/projects/", methods=["GET"]) - @AUTH.login_required - def get_moc_project(project_name): - if shift.project_exists(project_name): - return make_response( - {"msg": f"project exists ({project_name})"}, - ) - return make_response({"msg": f"project does not exist ({project_name})"}, 400) - - @APP.route("/projects/", methods=["PUT"]) - @APP.route("/projects//owner/", methods=["PUT"]) - @AUTH.login_required - def create_moc_project(project_name, user_name=None): - # first check the project_name is a valid openshift project name - suggested_project_name = shift.cnvt_project_name(project_name) - if project_name != suggested_project_name: - raise exceptions.BadRequest( - "project name must match regex '[a-z0-9]([-a-z0-9]*[a-z0-9])?'." - f" Suggested name: {suggested_project_name}." - ) - - if shift.project_exists(project_name): - raise exceptions.Conflict("project already exists.") - - payload = request.get_json(silent=True) or {} - display_name = payload.pop("displayName", project_name) - annotations = payload.pop("annotations", {}) - labels = payload.pop("labels", {}) - - shift.create_project( - project_name, - display_name, - user_name, - annotations=annotations, - labels=labels, - ) - return {"msg": f"project created ({project_name})"} - - @APP.route("/projects/", methods=["DELETE"]) - @AUTH.login_required - def delete_moc_project(project_name): - if shift.project_exists(project_name): - shift.delete_project(project_name) - - return make_response({"msg": f"project deleted ({project_name})"}) - - @APP.route("/users/", methods=["GET"]) - @AUTH.login_required - def get_moc_user(user_name): - if shift.user_exists(user_name): - return make_response({"msg": f"user ({user_name}) exists"}) - return make_response({"msg": f"user ({user_name}) does not exist"}, 404) - - @APP.route("/users/", methods=["PUT"]) - @AUTH.login_required - def create_moc_user(user_name): - # these three values should be added to generalize this function - # full_name - the full name of the user as it is really convenient - # id_provider - this is in the yaml configuration for this project - needed in the past - - full_name = user_name - id_user = user_name # until we support different user names see above. - - created = False - - if not shift.user_exists(user_name): - created = True - shift.create_user(user_name, full_name) - - if not shift.identity_exists(id_user): - created = True - shift.create_identity(id_user) - - if not shift.useridentitymapping_exists(user_name, id_user): - created = True - shift.create_useridentitymapping(user_name, id_user) - - if created: - return make_response({"msg": f"user created ({user_name})"}) - return make_response({"msg": f"user already exists ({user_name})"}, 400) - - @APP.route("/users/", methods=["DELETE"]) - @AUTH.login_required - def delete_moc_user(user_name): - if shift.user_exists(user_name): - shift.delete_user(user_name) - - if shift.identity_exists(user_name): - shift.delete_identity(user_name) - - return make_response({"msg": f"user deleted ({user_name})"}) - - @APP.route("/projects//quota", methods=["GET"]) - @AUTH.login_required - def get_quota(project): - return shift.get_moc_quota(project) - - @APP.route("/projects//quota", methods=["PUT", "POST"]) - @AUTH.login_required - def put_quota(project): - moc_quota = request.get_json(force=True) - return shift.update_moc_quota(project, moc_quota, patch=False) - - @APP.route("/projects//quota", methods=["PATCH"]) - @AUTH.login_required - def patch_quota(project): - moc_quota = request.get_json(force=True) - return shift.update_moc_quota(project, moc_quota, patch=True) - - @APP.route("/projects//quota", methods=["DELETE"]) - @AUTH.login_required - def delete_quota(project): - return shift.delete_moc_quota(project) - - @APP.route("/projects//users", methods=["GET"]) - @AUTH.login_required - def get_users_in_project(project): - return shift.get_users_in_project(project) - - return APP diff --git a/src/coldfront_plugin_cloud/acct_mgt/defaults.py b/src/coldfront_plugin_cloud/acct_mgt/defaults.py deleted file mode 100644 index 07e63714..00000000 --- a/src/coldfront_plugin_cloud/acct_mgt/defaults.py +++ /dev/null @@ -1,5 +0,0 @@ -"""Default values for Flask app configuration""" - -ADMIN_USERNAME = "admin" -QUOTA_DEF_FILE = "quotas.json" -LIMIT_DEF_FILE = "limits.json" diff --git a/src/coldfront_plugin_cloud/acct_mgt/exceptions.py b/src/coldfront_plugin_cloud/acct_mgt/exceptions.py deleted file mode 100644 index 923b516a..00000000 --- a/src/coldfront_plugin_cloud/acct_mgt/exceptions.py +++ /dev/null @@ -1,32 +0,0 @@ -"""Defined exceptions used by acct-mgt.""" - - -class ApiException(Exception): - """Base exception class for errors. - - All exceptions subclassing ApiException will be caught from Flask's - error handler and return the appropriate status code and message. - The visible parameter controls whether the error message is visible - to the end user. - """ - - status_code = 500 - visible = True - default_message = "Internal Server Error." - - def __init__(self, message=None): - self.message = message or self.default_message - - -class BadRequest(ApiException): - """Exception class for invalid requests.""" - - status_code = 400 - default_message = "Invalid Request." - - -class Conflict(BadRequest): - """Exception class for requests that create already existing resources.""" - - status_code = 409 - default_message = "Resource already exists." diff --git a/src/coldfront_plugin_cloud/acct_mgt/moc_openshift.py b/src/coldfront_plugin_cloud/acct_mgt/moc_openshift.py index 3c9917af..a26a7d13 100644 --- a/src/coldfront_plugin_cloud/acct_mgt/moc_openshift.py +++ b/src/coldfront_plugin_cloud/acct_mgt/moc_openshift.py @@ -3,9 +3,12 @@ import re import sys import time +import logging import kubernetes.dynamic.exceptions as kexc +logger = logging.getLogger(__name__) + OPENSHIFT_ROLES = ["admin", "edit", "view"] API_PROJECT = "project.openshift.io/v1" @@ -59,18 +62,14 @@ def validate_role(role): f"Invalid role, {role} is not one of {', '.join(OPENSHIFT_ROLES)}" ) - def __init__(self, client, logger, config): + def __init__(self, client, logger, identity_name, quotas, limits): self.client = client self.logger = logger - self.id_provider = config["IDENTITY_PROVIDER"] - self.quotafile = config["QUOTA_DEF_FILE"] - self.limitfile = config["LIMIT_DEF_FILE"] + self.id_provider = identity_name + self.quotas = quotas + self.limits = limits self.apis = {} - if not self.limitfile: - self.logger.error("No default limit file provided.") - sys.exit(1) - def get_resource_api(self, api_version: str, kind: str): """Either return the cached resource api from self.apis, or fetch a new one, store it in self.apis, and return it.""" @@ -115,10 +114,9 @@ def add_user_to_role(self, project_name, user_name, role): self.update_rolebindings(project_name, rolebinding) except kexc.NotFoundError: rolebinding = self.create_rolebindings(project_name, user_name, role) + + self.logger.info(f"added user {user_name} to role {role} in {project_name}") - return { - "msg": f"added user {user_name} to role {role} in {project_name}", - } def remove_user_from_role(self, project_name, user_name, role): self.validate_role(role) @@ -133,9 +131,9 @@ def remove_user_from_role(self, project_name, user_name, role): except kexc.NotFoundError: pass - return { - "msg": f"removed user {user_name} from role {role} in {project_name}", - } + self.logger.info(f"removed user {user_name} from role {role} in {project_name}") + + def update_moc_quota(self, project_name, new_quota, patch=False): """This will update resourcequota objects in a project and create new @@ -156,20 +154,18 @@ def update_moc_quota(self, project_name, new_quota, patch=False): self.delete_moc_quota(project_name) self.create_shift_quotas(project_name, quota_def) - return {"msg": "MOC quotas updated"} + self.logger.info("MOC quotas updated") def get_quota_definitions(self): - self.logger.info("reading quotas from %s", self.quotafile) - with open(self.quotafile, "r") as file: - quota = json.load(file) + + quota = self.quotas for k in quota: quota[k]["value"] = None return quota def get_limit_definitions(self): - with open(self.limitfile, "r") as file: - return json.load(file) + return self.limits def get_project(self, project_name): api = self.get_resource_api(API_PROJECT, "Project") @@ -389,7 +385,7 @@ def create_shift_quotas(self, project_name, quota_spec): res = api.create(namespace=project_name, body=resource_quota).to_dict() self.wait_for_quota_to_settle(project_name, res) - return {"msg": f"All quotas for {project_name} successfully created"} + self.logger.info(f"All quotas for {project_name} successfully created") def get_resourcequotas(self, project_name): """Returns a list of all of the resourcequota objects""" @@ -412,7 +408,7 @@ def delete_moc_quota(self, project_name): for resourcequota in resourcequotas: self.delete_resourcequota(project_name, resourcequota["metadata"]["name"]) - return {"msg": f"All quotas for {project_name} successfully deleted"} + self.logger.info(f"All quotas for {project_name} successfully deleted") def get_moc_quota_from_resourcequotas(self, project_name): """This returns a dictionary suitable for merging in with the diff --git a/src/coldfront_plugin_cloud/acct_mgt/wsgi.py b/src/coldfront_plugin_cloud/acct_mgt/wsgi.py deleted file mode 100644 index 1125b462..00000000 --- a/src/coldfront_plugin_cloud/acct_mgt/wsgi.py +++ /dev/null @@ -1,13 +0,0 @@ -"""WSGI wrapper for tooling that expects a top-level application object""" - -import logging - -from . import app - -APP = app.create_app() - -if __name__ == "__main__": - APP.run() -else: - APP.logger = logging.getLogger("gunicorn.error") - APP.logger.setLevel(logging.INFO) diff --git a/src/coldfront_plugin_cloud/attributes.py b/src/coldfront_plugin_cloud/attributes.py index c2044f1d..212841a1 100644 --- a/src/coldfront_plugin_cloud/attributes.py +++ b/src/coldfront_plugin_cloud/attributes.py @@ -19,6 +19,7 @@ class CloudAllocationAttribute: RESOURCE_AUTH_URL = 'Identity Endpoint URL' +RESOURCE_IDENTITY_NAME = 'Identity Name' RESOURCE_ROLE = 'Role for User in Project' RESOURCE_FEDERATION_PROTOCOL = 'OpenStack Federation Protocol' @@ -32,6 +33,7 @@ class CloudAllocationAttribute: RESOURCE_ATTRIBUTES = [ CloudResourceAttribute(name=RESOURCE_AUTH_URL), + CloudResourceAttribute(name=RESOURCE_IDENTITY_NAME), CloudResourceAttribute(name=RESOURCE_FEDERATION_PROTOCOL), CloudResourceAttribute(name=RESOURCE_IDP), CloudResourceAttribute(name=RESOURCE_PROJECT_DOMAIN), diff --git a/src/coldfront_plugin_cloud/management/commands/add_openshift_resource.py b/src/coldfront_plugin_cloud/management/commands/add_openshift_resource.py index 6e96bcb9..6306d8a4 100644 --- a/src/coldfront_plugin_cloud/management/commands/add_openshift_resource.py +++ b/src/coldfront_plugin_cloud/management/commands/add_openshift_resource.py @@ -15,8 +15,10 @@ class Command(BaseCommand): def add_arguments(self, parser): parser.add_argument('--name', type=str, required=True, help='Name of OpenShift resource') - parser.add_argument('--auth-url', type=str, required=True, - help='URL of the openshift-acct-mgt endpoint') + parser.add_argument('--cluster-url', type=str, required=True, + help='URL of the Openshift cluster') + parser.add_argument('--identity-name', type=str, required=True, + help='Name of the cluster\'s identity provider') parser.add_argument('--role', type=str, default='edit', help='Role for user when added to project (default: edit)') @@ -35,7 +37,7 @@ def handle(self, *args, **options): resource_attribute_type=ResourceAttributeType.objects.get( name=attributes.RESOURCE_AUTH_URL), resource=openshift, - value=options['auth_url'] + value=options['cluster_url'] ) ResourceAttribute.objects.get_or_create( resource_attribute_type=ResourceAttributeType.objects.get( @@ -43,3 +45,10 @@ def handle(self, *args, **options): resource=openshift, value=options['role'] ) + ResourceAttribute.objects.get_or_create( + resource_attribute_type=ResourceAttributeType.objects.get( + name=attributes.RESOURCE_IDENTITY_NAME), + resource=openshift, + value=options['identity_name'] + ) + diff --git a/src/coldfront_plugin_cloud/openshift.py b/src/coldfront_plugin_cloud/openshift.py index d20e3818..fa4ad1f5 100644 --- a/src/coldfront_plugin_cloud/openshift.py +++ b/src/coldfront_plugin_cloud/openshift.py @@ -1,18 +1,67 @@ -import functools -import json import logging import os -import requests -from requests.auth import HTTPBasicAuth -import time -from simplejson.errors import JSONDecodeError +import kubernetes.config +import kubernetes.client +import kubernetes.dynamic.exceptions as kexc +from openshift.dynamic import DynamicClient +from coldfront.core.allocation import models as allocation_models +from coldfront.core.resource import models as resource_models + +from coldfront_plugin_cloud.acct_mgt import moc_openshift from coldfront_plugin_cloud import attributes, base, utils +QUOTA_OPENSHIFT = { + ":requests.cpu": {"base": 2, "coefficient": 0}, + ":requests.memory": {"base": 2, "coefficient": 0}, + ":limits.cpu": {"base": 2, "coefficient": 0}, + ":limits.memory": {"base": 2, "coefficient": 0}, + ":requests.storage": {"base": 2, "coefficient": 0, "units": "Gi"}, + ":limits.storage": {"base": 2, "coefficient": 0, "units": "Gi"}, + ":requests.ephemeral-storage": {"base": 2, "coefficient": 8, "units": "Gi"}, + ":requests.nvidia.com/gpu": {"base": 0, "coefficient": 0}, + ":limits.ephemeral-storage": {"base": 2, "coefficient": 8, "units": "Gi"}, + ":persistentvolumeclaims": {"base": 2, "coefficient": 0}, + ":replicationcontrollers": {"base": 2, "coefficient": 0}, + ":resourcequotas": {"base": 5, "coefficient": 0}, + ":services": {"base": 4, "coefficient": 0}, + ":services.loadbalancers": {"base": 2, "coefficient": 0}, + ":services.nodeports": {"base": 2, "coefficient": 0}, + ":secrets": {"base": 4, "coefficient": 0}, + ":configmaps": {"base": 4, "coefficient": 0}, + ":openshift.io/imagestreams": {"base": 2, "coefficient": 0}, + "BestEffort:pods": {"base": 2, "coefficient": 2}, + "NotBestEffort:pods": {"base": 2, "coefficient": 2}, + "NotBestEffort:requests.memory": {"base": 2, "coefficient": 4, "units": "Gi"}, + "NotBestEffort:limits.memory": {"base": 2, "coefficient": 4, "units": "Gi"}, + "NotBestEffort:requests.cpu": {"base": 2, "coefficient": 2}, + "NotBestEffort:limits.cpu": {"base": 2, "coefficient": 2}, + "Terminating:pods": {"base": 2, "coefficient": 2}, + "Terminating:requests.memory": {"base": 2, "coefficient": 4, "units": "Gi"}, + "Terminating:limits.memory": {"base": 2, "coefficient": 4, "units": "Gi"}, + "Terminating:requests.cpu": {"base": 2, "coefficient": 2}, + "Terminating:limits.cpu": {"base": 2, "coefficient": 2}, + "NotTerminating:pods": {"base": 2, "coefficient": 2}, + "NotTerminating:requests.memory": {"base": 2, "coefficient": 4, "units": "Gi"}, + "NotTerminating:limits.memory": {"base": 2, "coefficient": 4, "units": "Gi"}, + "NotTerminating:requests.cpu": {"base": 2, "coefficient": 2}, + "NotTerminating:limits.cpu": {"base": 2, "coefficient": 2}, +} + +LIMITS_OPENSHIFT = [ + { + "type": "Container", + "default": {"cpu": "2", "memory": "1024Mi", "nvidia.com/gpu": "0"}, + "defaultRequest": {"cpu": "1", "memory": "512Mi", "nvidia.com/gpu": "0"}, + } +] + QUOTA_KEY_MAPPING = { attributes.QUOTA_LIMITS_CPU: lambda x: {":limits.cpu": f"{x * 1000}m"}, attributes.QUOTA_LIMITS_MEMORY: lambda x: {":limits.memory": f"{x}Mi"}, - attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB: lambda x: {":limits.ephemeral-storage": f"{x}Gi"}, + attributes.QUOTA_LIMITS_EPHEMERAL_STORAGE_GB: lambda x: { + ":limits.ephemeral-storage": f"{x}Gi" + }, attributes.QUOTA_REQUESTS_STORAGE: lambda x: {":requests.storage": f"{x}Gi"}, attributes.QUOTA_REQUESTS_GPU: lambda x: {":requests.nvidia.com/gpu": f"{x}"}, attributes.QUOTA_PVC: lambda x: {":persistentvolumeclaims": f"{x}"}, @@ -33,153 +82,189 @@ class Conflict(ApiException): class OpenShiftResourceAllocator(base.ResourceAllocator): - - resource_type = 'openshift' + resource_type = "openshift" project_name_max_length = 63 - @functools.cached_property - def session(self): + def __init__( + self, + resource: resource_models.Resource, + allocation: allocation_models.Allocation, + ): + super().__init__(resource, allocation) + + # Load Endpoint URL and Auth token for new k8 client var_name = utils.env_safe_name(self.resource.name) - username = os.getenv(f'OPENSHIFT_{var_name}_USERNAME') - password = os.getenv(f'OPENSHIFT_{var_name}_PASSWORD') - - session = requests.session() - if username and password: - session.auth = HTTPBasicAuth(username, password) - - functional_tests = os.environ.get('FUNCTIONAL_TESTS', '').lower() - verify = os.getenv(f'OPENSHIFT_{var_name}_VERIFY', '').lower() - if functional_tests == 'true' or verify == 'false': - session.verify = False - - return session - - @staticmethod - def check_response(response: requests.Response): - if 200 <= response.status_code < 300: - try: - return response.json() - except JSONDecodeError: - # https://github.com/CCI-MOC/openshift-acct-mgt/issues/54 - return response.text - if response.status_code == 404: - raise NotFound(f"{response.status_code}: {response.text}") - elif 'does not exist' in response.text or 'not found' in response.text: - raise NotFound(f"{response.status_code}: {response.text}") - elif 'already exists' in response.text: - raise Conflict(f"{response.status_code}: {response.text}") + openshift_token = os.getenv(f"OPENSHIFT_{var_name}_TOKEN") + openshift_url = resource.get_attribute(attributes.RESOURCE_AUTH_URL) + identity_name = resource.get_attribute(attributes.RESOURCE_IDENTITY_NAME) + + functional_tests = os.environ.get("FUNCTIONAL_TESTS", "").lower() + verify = os.getenv(f"OPENSHIFT_{var_name}_VERIFY", "").lower() + + k8_config = kubernetes.client.Configuration() + k8_config.api_key["authorization"] = openshift_token + k8_config.api_key_prefix["authorization"] = "Bearer" + k8_config.host = openshift_url + + if functional_tests == "true" or verify == "false": + self.logger = logging.getLogger() + logger = logging.getLogger() + k8_config.verify_ssl = False else: - raise ApiException(f"{response.status_code}: {response.text}") + self.logger = logging.getLogger("django") + logger = logging.getLogger("django") + k8_config.verify_ssl = True + + k8s_client = kubernetes.client.ApiClient(configuration=k8_config) + + self.client = moc_openshift.MocOpenShift4x( + DynamicClient(k8s_client), logger, identity_name, QUOTA_OPENSHIFT, LIMITS_OPENSHIFT + ) def create_project(self, suggested_project_name): - sanitized_project_name = utils.get_sanitized_project_name(suggested_project_name) + sanitized_project_name = utils.get_sanitized_project_name( + suggested_project_name + ) project_id = utils.get_unique_project_name( - sanitized_project_name, - max_length=self.project_name_max_length) + sanitized_project_name, max_length=self.project_name_max_length + ) project_name = project_id self._create_project(project_name, project_id) return self.Project(project_name, project_id) def set_quota(self, project_id): - url = f"{self.auth_url}/projects/{project_id}/quota" payload = dict() for key, func in QUOTA_KEY_MAPPING.items(): if (x := self.allocation.get_attribute(key)) is not None: payload.update(func(x)) - r = self.session.put(url, data=json.dumps({'Quota': payload})) - self.check_response(r) + + return self.client.update_moc_quota(project_id, {"Quota": payload}, patch=False) def get_quota(self, project_id): - url = f"{self.auth_url}/projects/{project_id}/quota" - r = self.session.get(url) - return self.check_response(r) + return self.client.get_moc_quota(project_id) def create_project_defaults(self, project_id): pass def disable_project(self, project_id): - url = f"{self.auth_url}/projects/{project_id}" - r = self.session.delete(url) - self.check_response(r) + if self.client.project_exists(project_id): + self.client.delete_project(project_id) def reactivate_project(self, project_id): project_name = self.allocation.get_attribute(attributes.ALLOCATION_PROJECT_NAME) - try: - self._create_project(project_name, project_id) - except Conflict: - # This is a reactivation of an already active project - # most likely for a quota update - pass + self._create_project(project_name, project_id) def get_federated_user(self, username): - url = f"{self.auth_url}/users/{username}" - try: - r = self.session.get(url) - self.check_response(r) - return {'username': username} - except NotFound: - pass + if self.client.user_exists(username) and self.client.identity_exists( + username) and self.client.useridentitymapping_exists(username, username): + return {"username": username} + + self.logger.info("404: " + f"user ({username}) does not exist") def create_federated_user(self, unique_id): - url = f"{self.auth_url}/users/{unique_id}" try: - r = self.session.put(url) - self.check_response(r) - except Conflict: + full_name = unique_id + id_user = unique_id # until we support different user names see above. + + created = False + + if not self.client.user_exists(unique_id): + created = True + self.client.create_user(unique_id, full_name) + + if not self.client.identity_exists(id_user): + created = True + self.client.create_identity(id_user) + + if not self.client.useridentitymapping_exists(unique_id, id_user): + created = True + self.client.create_useridentitymapping(unique_id, id_user) + + if created: + self.logger.info(f"msg: user created ({unique_id})") + return + + except Exception: pass def assign_role_on_user(self, username, project_id): # /users//projects//roles/ - url = (f"{self.auth_url}/users/{username}/projects/{project_id}" - f"/roles/{self.member_role_name}") try: - r = self.session.put(url) - self.check_response(r) + return self.client.add_user_to_role( + project_id, username, self.member_role_name + ) except Conflict: pass def remove_role_from_user(self, username, project_id): # /users//projects//roles/ - url = (f"{self.auth_url}/users/{username}/projects/{project_id}" - f"/roles/{self.member_role_name}") - r = self.session.delete(url) - self.check_response(r) + + return self.client.remove_user_from_role( + project_id, username, self.member_role_name + ) def _create_project(self, project_name, project_id): - url = f"{self.auth_url}/projects/{project_id}" - headers = {"Content-type": "application/json"} - annotations = {"cf_project_id": str(self.allocation.project_id), - "cf_pi": self.allocation.project.pi.username} - labels = { - 'opendatahub.io/dashboard': "true", - 'modelmesh-enabled': "true", + suggested_project_name = self.client.cnvt_project_name(project_name) + if project_name != suggested_project_name: + self.logger.info("400: " + + "project name must match regex '[a-z0-9]([-a-z0-9]*[a-z0-9])?'." + + f" Suggested name: {suggested_project_name}." + ) + return + + if self.client.project_exists(project_name): + self.logger.info("409: project already exists.") + return + + display_name = project_name + annotations = { + "cf_project_id": str(self.allocation.project_id), + "cf_pi": self.allocation.project.pi.username, } - - payload = {"displayName": project_name, - "annotations": annotations, - "labels": labels} - r = self.session.put(url, data=json.dumps(payload), headers=headers) - self.check_response(r) + labels = {"opendatahub.io/dashboard": "true"} + user_name = None + + self.client.create_project( + project_name, + display_name, + user_name, + annotations=annotations, + labels=labels, + ) + self.logger.info(f"msg: project created ({project_name})") def _get_role(self, username, project_id): # /users//projects//roles/ - url = (f"{self.auth_url}/users/{username}/projects/{project_id}" - f"/roles/{self.member_role_name}") - r = self.session.get(url) - return self.check_response(r) + + if self.client.user_rolebinding_exists( + username, project_id, self.member_role_name + ): + self.logger.info( + f"msg: user role exists ({project_id},{username},{self.member_role_name})" + ) + return + + raise NotFound( + "404: " + + f"user role does not exist ({project_id},{username},{self.member_role_name})" + ) def _get_project(self, project_id): - url = f"{self.auth_url}/projects/{project_id}" - r = self.session.get(url) - return self.check_response(r) + if self.client.project_exists(project_id): + self.logger.info(f"msg: project exists ({project_id})") + return + + raise NotFound("404: " + f"project does not exist ({project_id})") def _delete_user(self, username): - url = f"{self.auth_url}/users/{username}" - r = self.session.delete(url) - return self.check_response(r) + if self.client.user_exists(username): + self.client.delete_user(username) + + if self.client.identity_exists(username): + self.client.delete_identity(username) + + return {"msg": f"user deleted ({username})"} def get_users(self, project_id): - url = f"{self.auth_url}/projects/{project_id}/users" - r = self.session.get(url) - return set(self.check_response(r)) + return set(self.client.get_users_in_project(project_id)) diff --git a/src/coldfront_plugin_cloud/tests/base.py b/src/coldfront_plugin_cloud/tests/base.py index 6d62f227..1084621d 100644 --- a/src/coldfront_plugin_cloud/tests/base.py +++ b/src/coldfront_plugin_cloud/tests/base.py @@ -60,13 +60,14 @@ def new_resource(name=None, auth_url=None) -> Resource: return Resource.objects.get(name=resource_name) @staticmethod - def new_openshift_resource(name=None, auth_url=None) -> Resource: + def new_openshift_resource(name=None, auth_url=None, identity_name=None) -> Resource: resource_name = name or uuid.uuid4().hex call_command( 'add_openshift_resource', name=resource_name, - auth_url=auth_url or 'https://onboarding-onboarding.cluster.local', + cluster_url=auth_url or 'https://onboarding-onboarding.cluster.local', + identity_name=identity_name ) return Resource.objects.get(name=resource_name) diff --git a/src/coldfront_plugin_cloud/tests/functional/openshift/test_allocation.py b/src/coldfront_plugin_cloud/tests/functional/openshift/test_allocation.py index 483d224c..74cfc5f6 100644 --- a/src/coldfront_plugin_cloud/tests/functional/openshift/test_allocation.py +++ b/src/coldfront_plugin_cloud/tests/functional/openshift/test_allocation.py @@ -15,7 +15,8 @@ def setUp(self) -> None: super().setUp() self.resource = self.new_openshift_resource( name='Microshift', - auth_url=os.getenv('OS_AUTH_URL') + auth_url=os.getenv('OS_AUTH_URL'), + identity_name="developer" ) def test_new_allocation(self): diff --git a/src/coldfront_plugin_cloud/tests/unit/test_calculate_quota_unit_hours.py b/src/coldfront_plugin_cloud/tests/unit/test_calculate_quota_unit_hours.py index 972921ec..1667224c 100644 --- a/src/coldfront_plugin_cloud/tests/unit/test_calculate_quota_unit_hours.py +++ b/src/coldfront_plugin_cloud/tests/unit/test_calculate_quota_unit_hours.py @@ -15,6 +15,7 @@ def test_new_allocation_quota(self): self.resource = self.new_openshift_resource( name="", auth_url="", + identity_name="", ) with freezegun.freeze_time("2020-03-15 00:01:00"): @@ -44,6 +45,7 @@ def test_new_allocation_quota_expired(self): self.resource = self.new_openshift_resource( name="", auth_url="", + identity_name="", ) user = self.new_user() project = self.new_project(pi=user) @@ -73,6 +75,7 @@ def test_new_allocation_quota_denied(self): self.resource = self.new_openshift_resource( name="", auth_url="", + identity_name="", ) user = self.new_user() project = self.new_project(pi=user) @@ -101,6 +104,7 @@ def test_new_allocation_quota_last_revoked(self): self.resource = self.new_openshift_resource( name="", auth_url="", + identity_name="", ) user = self.new_user() project = self.new_project(pi=user) @@ -140,6 +144,7 @@ def test_new_allocation_quota_new(self): self.resource = self.new_openshift_resource( name="", auth_url="", + identity_name="", ) user = self.new_user() project = self.new_project(pi=user) @@ -159,6 +164,7 @@ def test_new_allocation_quota_never_approved(self): self.resource = self.new_openshift_resource( name="", auth_url="", + identity_name="", ) user = self.new_user() project = self.new_project(pi=user) @@ -181,6 +187,7 @@ def test_new_allocation_quota_change_request(self): self.resource = self.new_openshift_resource( name="", auth_url="", + identity_name="", ) user = self.new_user() project = self.new_project(pi=user) diff --git a/test-requirements.txt b/test-requirements.txt index f17fbf0e..456589c6 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -7,3 +7,5 @@ python-memcached==1.59 python-novaclient python-neutronclient python-swiftclient +kubernetes +openshift From 23ca13b611194cc96a636010b7fb0202854acabb Mon Sep 17 00:00:00 2001 From: Quan Pham Date: Tue, 12 Mar 2024 10:52:34 -0400 Subject: [PATCH 3/4] Copied over acct_mgt unit tests for Openshift API Wrapper with no changes --- .../tests/unit/moc_openshift/__init__.py | 0 .../tests/unit/moc_openshift/conftest.py | 24 ++ .../unit/moc_openshift/test_moc_openshift.py | 70 +++++ .../test_moc_openshift_clean_metadata.py | 32 +++ .../test_moc_openshift_identity.py | 78 ++++++ .../test_moc_openshift_project.py | 107 ++++++++ .../moc_openshift/test_moc_openshift_quota.py | 255 ++++++++++++++++++ .../moc_openshift/test_moc_openshift_rbac.py | 134 +++++++++ .../moc_openshift/test_moc_openshift_user.py | 50 ++++ 9 files changed, 750 insertions(+) create mode 100644 src/coldfront_plugin_cloud/tests/unit/moc_openshift/__init__.py create mode 100644 src/coldfront_plugin_cloud/tests/unit/moc_openshift/conftest.py create mode 100644 src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift.py create mode 100644 src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_clean_metadata.py create mode 100644 src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_identity.py create mode 100644 src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_project.py create mode 100644 src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_quota.py create mode 100644 src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_rbac.py create mode 100644 src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_user.py diff --git a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/__init__.py b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/conftest.py b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/conftest.py new file mode 100644 index 00000000..e10439e4 --- /dev/null +++ b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/conftest.py @@ -0,0 +1,24 @@ +# pylint: disable=missing-module-docstring,wrong-import-position,redefined-outer-name + +from unittest import mock + +import logging +import pytest + +from acct_mgt.moc_openshift import MocOpenShift4x + + +@pytest.fixture +def config(): + return { + "IDENTITY_PROVIDER": "fake-id-provider", + "QUOTA_DEF_FILE": "fake-quota-file", + "LIMIT_DEF_FILE": "fake-limit-file", + } + + +@pytest.fixture() +def moc(config): + fake_client = mock.Mock(spec=["resources"]) + fake_logger = mock.Mock(spec=logging.Logger) + return MocOpenShift4x(fake_client, fake_logger, config) diff --git a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift.py b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift.py new file mode 100644 index 00000000..69240363 --- /dev/null +++ b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift.py @@ -0,0 +1,70 @@ +# pylint: disable=missing-module-docstring +from unittest import mock + +import pytest + +from acct_mgt.moc_openshift import MocOpenShift4x + + +def test_moc_openshift(moc): + assert moc.id_provider == "fake-id-provider" + assert moc.quotafile == "fake-quota-file" + assert moc.limitfile == "fake-limit-file" + + +def test_moc_openshift_no_limit(): + with pytest.raises(SystemExit): + MocOpenShift4x( + mock.Mock(), + mock.Mock(), + { + "IDENTITY_PROVIDER": "fake", + "QUOTA_DEF_FILE": "fake", + "LIMIT_DEF_FILE": None, + }, + ) + + +@pytest.mark.xfail(reason="bug") +def test_moc_openshift_no_quota(): + with pytest.raises(SystemExit): + MocOpenShift4x( + mock.Mock(), + mock.Mock(), + { + "IDENTITY_PROVIDER": "fake", + "QUOTA_DEF_FILE": None, + "LIMIT_DEF_FILE": "fake", + }, + ) + + +def test_split_quota_name(moc): + assert moc.split_quota_name(":fake") == ("Project", "fake") + assert moc.split_quota_name("scope:fake") == ("scope", "fake") + + +@pytest.mark.parametrize( + "orig,expected", + [ + ("fake", "fake"), + (" fake fake ", "fake-fake"), + ("This Is My Project!", "this-is-my-project"), + ("Ñaño 1234", "a-o-1234"), + ], +) +def test_cnvt_project_name(moc, orig, expected): + assert moc.cnvt_project_name(orig) == expected + + +def test_get_resource_api_cached(moc): + moc.apis = {"fake-apiversion:fake-kind": "fake-resource-api"} + res = moc.get_resource_api("fake-apiversion", "fake-kind") + assert res == "fake-resource-api" + + +def test_get_resource_api_new(moc): + moc.client.resources.get.return_value = "fake-resource-api" + res = moc.get_resource_api("fake-apiversion", "fake-kind") + assert res == "fake-resource-api" + assert "fake-apiversion:fake-kind" in moc.apis diff --git a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_clean_metadata.py b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_clean_metadata.py new file mode 100644 index 00000000..393d1256 --- /dev/null +++ b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_clean_metadata.py @@ -0,0 +1,32 @@ +# pylint: disable=missing-module-docstring +import pytest + +import acct_mgt.moc_openshift + + +@pytest.mark.parametrize( + "data,expected", + [ + ({}, {}), + ({"spec": "test"}, {"spec": "test"}), + ({"metadata": {}, "spec": "something"}, {"metadata": {}, "spec": "something"}), + ( + {"metadata": {"resourceVersion": "1"}, "spec": "something"}, + {"metadata": {}, "spec": "something"}, + ), + ( + { + "metadata": { + "resourceVersion": "1", + "uid": "", + "creationTimestamp": "", + "name": "test", + }, + "spec": "something", + }, + {"metadata": {"name": "test"}, "spec": "something"}, + ), + ], +) +def test_clean_openshift_metadata(data, expected): + assert acct_mgt.moc_openshift.clean_openshift_metadata(data) == expected diff --git a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_identity.py b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_identity.py new file mode 100644 index 00000000..0803984d --- /dev/null +++ b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_identity.py @@ -0,0 +1,78 @@ +# pylint: disable=missing-module-docstring +from unittest import mock + +import kubernetes.dynamic.exceptions as kexc + + +def test_qualified_id_user(moc): + assert moc.qualified_id_user("foo") == "fake-id-provider:foo" + + +def test_get_identity(moc): + fake_identity = mock.Mock(spec=["to_dict"]) + fake_identity.to_dict.return_value = {"id": "fake-id"} + moc.client.resources.get.return_value.get.return_value = fake_identity + res = moc.get_identity("fake-id") + assert res == {"id": "fake-id"} + + +def test_identity_exists(moc): + fake_identity = mock.Mock(spec=["to_dict"]) + fake_identity.to_dict.return_value = {"id": "fake-id"} + moc.client.resources.get.return_value.get.return_value = fake_identity + assert moc.identity_exists("fake-id") + + +def test_identity_exists_not(moc): + moc.client.resources.get.return_value.get.side_effect = kexc.NotFoundError( + mock.Mock() + ) + assert not moc.identity_exists("fake-id") + + +def test_create_identity(moc): + fake_identity = mock.Mock(spec=["to_dict"]) + fake_identity.to_dict.return_value = {"id": "fake-id"} + moc.client.resources.get.return_value.create.return_value = fake_identity + res = moc.create_identity("fake-id") + assert res["id"] == "fake-id" + moc.client.resources.get.return_value.create.assert_called_with( + body={"providerName": "fake-id-provider", "providerUserName": "fake-id"} + ) + + +def test_delete_identity(moc): + fake_identity = mock.Mock(spec=["to_dict"]) + fake_identity.to_dict.return_value = {} + moc.client.resources.get.return_value.delete.return_value = fake_identity + res = moc.delete_identity("fake-id") + assert res == {} + moc.client.resources.get.return_value.delete.assert_called_with( + name="fake-id-provider:fake-id" + ) + + +@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_user") +def test_useridentitymapping_exists(fake_get_user, moc): + fake_get_user.return_value = {"identities": ["fake-id-provider:fake-id"]} + assert moc.useridentitymapping_exists("fake-user", "fake-id") + + +@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_user") +def test_useridentitymapping_exists_not(fake_get_user, moc): + fake_get_user.side_effect = kexc.NotFoundError(mock.Mock()) + assert not moc.useridentitymapping_exists("fake-user", "fake-id") + + +def test_createuseridentitymapping(moc): + fake_idm = mock.Mock(spec=["to_dict"]) + fake_idm.to_dict.return_value = {} + moc.client.resources.get.return_value.create.return_value = fake_idm + res = moc.create_useridentitymapping("fake-user", "fake-id") + assert res == {} + moc.client.resources.get.return_value.create.assert_called_with( + body={ + "user": {"name": "fake-user"}, + "identity": {"name": "fake-id-provider:fake-id"}, + } + ) diff --git a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_project.py b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_project.py new file mode 100644 index 00000000..8758ae7b --- /dev/null +++ b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_project.py @@ -0,0 +1,107 @@ +# pylint: disable=missing-module-docstring +from unittest import mock + +import pytest + +import kubernetes.dynamic.exceptions as kexc + + +def test_get_project(moc): + fake_project = mock.Mock(spec=["to_dict"]) + fake_project.to_dict.return_value = {"project": "fake-project"} + moc.client.resources.get.return_value.get.return_value = fake_project + res = moc.get_project("fake-project") + assert res == {"project": "fake-project"} + + +def test_project_exists(moc): + fake_project = mock.Mock() + fake_project.to_dict.return_value = {} + moc.client.resources.get.return_value.get.return_value = fake_project + assert moc.project_exists("fake-project") + + +def test_project_exists_not(moc): + moc.client.resources.get.return_value.get.side_effect = kexc.NotFoundError( + mock.Mock() + ) + assert not moc.project_exists("fake-project") + + +@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.create_limits", mock.Mock()) +def test_create_project(moc): + moc.create_project("fake-project", "Fake Project", "fake-user") + moc.client.resources.get.return_value.create.assert_called_with( + body={ + "metadata": { + "name": "fake-project", + "annotations": { + "openshift.io/display-name": "Fake Project", + "openshift.io/requester": "fake-user", + }, + "labels": {"nerc.mghpcc.org/project": "true"}, + } + } + ) + + +def test_delete_project(moc): + moc.delete_project("fake-project") + moc.client.resources.get.return_value.delete.assert_called_with(name="fake-project") + + +def test_get_users_in_project_with_nonexistent_project(moc): + moc.client.resources.get.return_value.get.side_effect = kexc.NotFoundError( + mock.Mock() + ) + + with pytest.raises(kexc.NotFoundError): + moc.get_users_in_project("nonexistent_project") + + +def test_get_users_in_project_with_no_rolebindings(moc): + dummy_project = {"name": "project1"} + moc.get_project = mock.Mock(return_value=dummy_project) + + moc.get_rolebindings = mock.Mock(side_effect=kexc.NotFoundError(mock.Mock())) + + users = moc.get_users_in_project("project1") + + assert users == [] + + +def test_get_users_in_project_with_project_with_one_rolebinding(moc): + dummy_project = {"name": "project1"} + moc.get_project = mock.Mock(return_value=dummy_project) + + # pylint: disable=unused-argument + def get_rolebindings_side_effect(project_name, role): + if role == "view": + return {"role": "view", "subjects": [{"kind": "User", "name": "viewer"}]} + raise kexc.NotFoundError(mock.Mock()) + + moc.get_rolebindings = mock.Mock(side_effect=get_rolebindings_side_effect) + + users = moc.get_users_in_project("project1") + + assert users == ["viewer"] + + +def test_get_users_in_project_with_multiple_rolebindings(moc): + dummy_project = {"name": "project1"} + moc.get_project = mock.Mock(return_value=dummy_project) + + # pylint: disable=unused-argument + def get_rolebindings_side_effect(project_name, role): + if role in ["admin", "view", "edit"]: + return { + "role": role, + "subjects": [{"kind": "User", "name": f"{role}-user"}], + } + raise ValueError(role) + + moc.get_rolebindings = mock.Mock(side_effect=get_rolebindings_side_effect) + + users = moc.get_users_in_project("project1") + + assert set(users) == set(["view-user", "admin-user", "edit-user"]) diff --git a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_quota.py b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_quota.py new file mode 100644 index 00000000..4baec58e --- /dev/null +++ b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_quota.py @@ -0,0 +1,255 @@ +# pylint: disable=missing-module-docstring +from unittest import mock + +import json +import pytest + + +@pytest.mark.xfail(reason="raises FileNotFoundError") +def test_get_quota_definitions_missing(moc): + """What happens if the quota file is missing?""" + with mock.patch("builtins.open", mock.mock_open()) as fake_open: + fake_open.side_effect = FileNotFoundError() + res = moc.get_quota_definitions() + assert res == {} + + +@pytest.mark.xfail(reason="raises JSONDecodeError") +def test_get_quota_definitions_empty(moc): + """What happens if the quota file exists but is empty?""" + with mock.patch("builtins.open", mock.mock_open(read_data="")): + res = moc.get_quota_definitions() + assert res == {} + + +@pytest.mark.xfail(reason="raises TypeError") +def test_get_quota_definitions_invalid(moc): + """What happens if the quota file exists but contains invalid data?""" + with mock.patch("builtins.open", mock.mock_open(read_data='{"foo": "bar"}')): + res = moc.get_quota_definitions() + assert res == {} + + +def test_get_quota_definitions_valid(moc): + """What happens if a valid quota file exists?""" + quotadefs = { + ":configmaps": {"base": 2, "coefficient": 0}, + } + with mock.patch("builtins.open", mock.mock_open(read_data=json.dumps(quotadefs))): + res = moc.get_quota_definitions() + quotadefs[":configmaps"]["value"] = None + assert res == quotadefs + + +def test_split_quota_name(moc): + assert moc.split_quota_name(":foo") == ("Project", "foo") + assert moc.split_quota_name("scope:foo") == ("scope", "foo") + + +@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_project", mock.Mock()) +def test_get_resourcequotas(moc): + fake_quota = mock.Mock(spec=["to_dict"]) + fake_quota.to_dict.return_value = {"items": []} + moc.client.resources.get.return_value.get.return_value = fake_quota + res = moc.get_resourcequotas("fake-project") + moc.client.resources.get.return_value.get.assert_called() + assert res == [] + + +def test_delete_quota(moc): + fake_quota = mock.Mock(spec=["to_dict"]) + fake_quota.to_dict.return_value = {} + moc.client.resources.get.return_value.delete.return_value = fake_quota + res = moc.delete_resourcequota("test-project", "test-quota") + moc.client.resources.get.return_value.delete.assert_called() + assert res == {} + + +@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_resourcequotas") +def test_delete_moc_quota(fake_get_resourcequotas, moc): + fake_get_resourcequotas.return_value = [{"metadata": {"name": "fake-quota"}}] + moc.delete_moc_quota("test-project") + moc.client.resources.get.return_value.delete.assert_any_call( + namespace="test-project", name="fake-quota" + ) + + +@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_resourcequotas") +def test_get_moc_quota_from_resourcequotas(fake_get_resourcequotas, moc): + fake_get_resourcequotas.return_value = [ + { + "metadata": {"name": "fake-quota"}, + "spec": {"hard": {"cpu": "1"}}, + }, + { + "metadata": {"name": "fake-quota"}, + "spec": {"hard": {"memory": "1"}, "scopes": ["BestEffort"]}, + }, + ] + + res = moc.get_moc_quota_from_resourcequotas("test_project") + assert res == {":cpu": "1", "BestEffort:memory": "1"} + + +@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.wait_for_quota_to_settle") +def test_create_shift_quotas(fake_wait_quota, moc): + quotadefs = { + ":configmaps": {"value": "1"}, + ":cpu": {"value": "1"}, + ":resourcequotas": {"value": "1"}, + } + + moc.client.resources.get.return_value.create.return_value = mock.Mock() + + moc.create_shift_quotas("fake-project", quotadefs) + + moc.client.resources.get.return_value.create.assert_called_with( + namespace="fake-project", + body={ + "metadata": {"name": "fake-project-project"}, + "spec": {"hard": {"configmaps": "1", "cpu": "1", "resourcequotas": "1"}}, + }, + ) + + fake_wait_quota.assert_called() + + +def test_wait_for_quota_to_settle(moc): + fake_quota = mock.Mock(spec=["to_dict"]) + fake_quota.to_dict.return_value = { + "metadata": {"name": "fake-quota"}, + "spec": {"hard": {"resourcequotas": "1"}}, + "status": {"used": {"resourcequotas": "1"}}, + } + moc.client.resources.get.return_value.get.return_value = fake_quota + + moc.wait_for_quota_to_settle("fake-project", fake_quota.to_dict()) + + moc.client.resources.get.return_value.get.assert_called_with( + namespace="fake-project", + name="fake-quota", + ) + + +@mock.patch( + "acct_mgt.moc_openshift.MocOpenShift4x.get_moc_quota_from_resourcequotas", + mock.Mock(), +) +@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.delete_moc_quota", mock.Mock()) +@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.create_shift_quotas") +def test_update_moc_quota( + fake_create_quotas, + moc, +): + quotadefs = { + ":configmaps": {}, + ":cpu": {}, + } + + new_quota = { + "Quota": { + ":cpu": "1000", + } + } + + with mock.patch("builtins.open", mock.mock_open(read_data=json.dumps(quotadefs))): + moc.update_moc_quota("fake-project", new_quota) + fake_create_quotas.assert_called_with( + "fake-project", + {":configmaps": {"value": None}, ":cpu": {"value": "1000"}}, + ) + + +@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.delete_moc_quota", mock.Mock()) +@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_resourcequotas") +@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.create_shift_quotas") +def test_update_moc_quota_patch( + fake_create_quotas, + fake_get_resourcequotas, + moc, +): + fake_quota = { + "metadata": {"name": "fake-quota"}, + "spec": { + "hard": {"services": "2"}, + }, + } + + quotadefs = { + ":configmaps": {}, + ":cpu": {}, + } + + new_quota = { + "Quota": { + ":cpu": "1000", + } + } + + fake_get_resourcequotas.return_value = [fake_quota] + + with mock.patch("builtins.open", mock.mock_open(read_data=json.dumps(quotadefs))): + moc.update_moc_quota("fake-project", new_quota, patch=True) + fake_create_quotas.assert_called_with( + "fake-project", + { + ":services": {"value": "2"}, + ":configmaps": {"value": None}, + ":cpu": {"value": "1000"}, + }, + ) + + +@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_moc_quota_from_resourcequotas") +def test_get_moc_quota(fake_get_quota, moc): + fake_get_quota.return_value = { + ":services": {"value": "2"}, + ":configmaps": {"value": None}, + ":cpu": {"value": "1000"}, + } + res = moc.get_moc_quota("fake-project") + assert res == { + "Version": "0.9", + "Kind": "MocQuota", + "ProjectName": "fake-project", + "Quota": { + ":services": {"value": "2"}, + ":configmaps": {"value": None}, + ":cpu": {"value": "1000"}, + }, + } + + +def test_get_limit_definitions_valid(moc): + with mock.patch("builtins.open", mock.mock_open(read_data="{}")): + res = moc.get_limit_definitions() + assert res == {} + + +def test_create_limits(moc): + limitdefs = '[{"type": "Container", "default": {"cpu": "200m", "memory": "512Mi"}}]' + fake_limit = mock.Mock(spec=["to_dict"]) + fake_limit.to_dict.return_value = "fake-limit" + moc.client.resources.get.return_value.create.return_value = fake_limit + with mock.patch("builtins.open", mock.mock_open(read_data=limitdefs)): + res = moc.create_limits("fake-project") + assert res == "fake-limit" + moc.client.resources.get.return_value.create.assert_called_with( + namespace="fake-project", + body={ + "metadata": {"name": "fake-project-limits"}, + "spec": {"limits": json.loads(limitdefs)}, + }, + ) + + +def test_create_limits_custom(moc): + with mock.patch("builtins.open", mock.mock_open()): + moc.create_limits("fake-project", limits="fake-limits") + moc.client.resources.get.return_value.create.assert_called_with( + namespace="fake-project", + body={ + "metadata": {"name": "fake-project-limits"}, + "spec": {"limits": "fake-limits"}, + }, + ) diff --git a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_rbac.py b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_rbac.py new file mode 100644 index 00000000..0a4891aa --- /dev/null +++ b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_rbac.py @@ -0,0 +1,134 @@ +# pylint: disable=missing-module-docstring +from unittest import mock + +import pytest + +import kubernetes.dynamic.exceptions as kexc + + +def test_user_rolebinding_exists_invalid_role(moc): + with pytest.raises(ValueError): + moc.user_rolebinding_exists("fake-user", "fake-project", "invalid-role") + + +@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_rolebindings") +def test_user_rolebinding_exists_not(fake_get_rb, moc): + fake_get_rb.side_effect = kexc.NotFoundError(mock.Mock()) + assert not moc.user_rolebinding_exists("fake-user", "fake-project", "admin") + + +@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_rolebindings") +def test_user_rolebinding_exists(fake_get_rb, moc): + fake_get_rb.return_value = { + "subjects": [ + { + "kind": "User", + "name": "fake-user", + } + ], + } + + assert moc.user_rolebinding_exists("fake-user", "fake-project", "admin") + + +def test_add_user_to_role_invalid_role(moc): + with pytest.raises(ValueError): + moc.add_user_to_role("fake-project", "fake-user", "invalid-role") + + +@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_rolebindings") +@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.update_rolebindings") +def test_add_user_to_role(fake_update_rb, fake_get_rb, moc): + fake_get_rb.return_value = { + "subjects": [], + } + + moc.add_user_to_role("fake-project", "fake-user", "admin") + fake_update_rb.assert_called_with( + "fake-project", {"subjects": [{"kind": "User", "name": "fake-user"}]} + ) + + +@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_rolebindings") +@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.create_rolebindings") +def test_add_user_to_role_not_exists(fake_create_rb, fake_get_rb, moc): + fake_get_rb.side_effect = kexc.NotFoundError(mock.Mock()) + + moc.add_user_to_role("fake-project", "fake-user", "admin") + fake_create_rb.assert_called_with("fake-project", "fake-user", "admin") + + +def test_remove_user_from_role_invalid_role(moc): + with pytest.raises(ValueError): + moc.remove_user_from_role("fake-project", "fake-user", "invalid-role") + + +@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_rolebindings") +def test_remove_user_from_role(fake_get_rb, moc): + fake_get_rb.return_value = { + "subjects": [{"kind": "User", "name": "fake-user"}], + } + moc.remove_user_from_role("fake-project", "fake-user", "admin") + moc.client.resources.get.return_value.patch.assert_called_with( + body={"subjects": []}, namespace="fake-project" + ) + + +def test_remove_user_from_role_not_exists(moc): + moc.client.resources.get.return_value.get.side_effect = kexc.NotFoundError( + mock.Mock() + ) + moc.remove_user_from_role("fake-project", "fake-user", "admin") + moc.client.resources.get.return_value.patch.assert_not_called() + + +def test_get_rolebindings(moc): + fake_rb = mock.Mock(spec=["to_dict"]) + fake_rb.to_dict.return_value = {"subjects": []} + moc.client.resources.get.return_value.get.return_value = fake_rb + res = moc.get_rolebindings("fake-project", "admin") + assert res == fake_rb.to_dict() + + +def test_get_rolebindings_no_subjects(moc): + fake_rb = mock.Mock(spec=["to_dict"]) + fake_rb.to_dict.return_value = {} + moc.client.resources.get.return_value.get.return_value = fake_rb + res = moc.get_rolebindings("fake-project", "admin") + assert res == {"subjects": []} + + +def test_list_rolebindings(moc): + fake_rb = mock.Mock(spec=["to_dict"]) + fake_rb.to_dict.return_value = { + "items": ["rb1", "rb2"], + } + moc.client.resources.get.return_value.get.return_value = fake_rb + + res = moc.list_rolebindings("fake-project") + assert res == ["rb1", "rb2"] + + +def test_list_rolebindings_not_exists(moc): + moc.client.resources.get.return_value.get.side_effect = kexc.NotFoundError( + mock.Mock() + ) + + res = moc.list_rolebindings("fake-project") + assert res == [] + + +def test_create_rolebindings(moc): + fake_rb = mock.Mock(spec=["to_dict"]) + fake_rb.to_dict.return_value = {} + moc.client.resources.get.return_value.create.return_value = fake_rb + res = moc.create_rolebindings("fake-project", "fake-user", "admin") + assert res == {} + moc.client.resources.get.return_value.create.assert_called_with( + namespace="fake-project", + body={ + "metadata": {"name": "admin", "namespace": "fake-project"}, + "subjects": [{"name": "fake-user", "kind": "User"}], + "roleRef": {"name": "admin", "kind": "ClusterRole"}, + }, + ) diff --git a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_user.py b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_user.py new file mode 100644 index 00000000..4291adb5 --- /dev/null +++ b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_user.py @@ -0,0 +1,50 @@ +# pylint: disable=missing-module-docstring +from unittest import mock + +import kubernetes.dynamic.exceptions as kexc + + +def test_get_user(moc): + fake_user = mock.Mock(spec=["to_dict"]) + fake_user.to_dict.return_value = {"user": "fake_user"} + moc.client.resources.get.return_value.get.return_value = fake_user + res = moc.get_user("fake_user_name") + assert res == {"user": "fake_user"} + + +def test_user_exists(moc): + fake_user = mock.Mock(spec=["to_dict"]) + fake_user.to_dict.return_value = {"user": "fake_user"} + moc.client.resources.get.return_value.get.return_value = fake_user + res = moc.user_exists("fake_user_name") + assert res + + +def test_user_exists_not(moc): + moc.client.resources.get.return_value.get.side_effect = kexc.NotFoundError( + mock.Mock() + ) + res = moc.user_exists("fake_user_name") + assert not res + + +def test_create_user(moc): + fake_user = mock.Mock(spec=["to_dict"]) + fake_user.to_dict.return_value = {"user": "fake_user_name"} + moc.client.resources.get.return_value.create.return_value = fake_user + res = moc.create_user("fake_user_name", "Fake User") + assert res["user"] == "fake_user_name" + moc.client.resources.get.return_value.create.assert_called_with( + body={"metadata": {"name": "fake_user_name"}, "fullName": "Fake User"} + ) + + +def test_delete_user(moc): + fake_user = mock.Mock(spec=["to_dict"]) + fake_user.to_dict.return_value = {} + moc.client.resources.get.return_value.delete.return_value = fake_user + res = moc.delete_user("fake_user_name") + assert res == {} + moc.client.resources.get.return_value.delete.assert_called_with( + name="fake_user_name" + ) From 054961121085c8409398521a8ae631954ae8dbde Mon Sep 17 00:00:00 2001 From: Quan Pham Date: Tue, 12 Mar 2024 10:49:43 -0400 Subject: [PATCH 4/4] Adapted acct-mgt unit tests for Openshift API Wrapper --- .github/workflows/test-py39-unit.yaml | 1 + ci/devstack-test-oidc.py | 26 ---- ci/run_functional_tests_openstack.sh | 4 - ci/run_unit_tests.sh | 4 +- .../acct_mgt/moc_openshift.py | 4 + .../tests/unit/moc_openshift/conftest.py | 17 ++- .../unit/moc_openshift/test_moc_openshift.py | 32 +++-- .../test_moc_openshift_clean_metadata.py | 4 +- .../test_moc_openshift_identity.py | 4 +- .../test_moc_openshift_project.py | 2 +- .../moc_openshift/test_moc_openshift_quota.py | 115 ++++++++---------- .../moc_openshift/test_moc_openshift_rbac.py | 14 +-- test-requirements.txt | 3 + 13 files changed, 106 insertions(+), 124 deletions(-) delete mode 100644 ci/devstack-test-oidc.py diff --git a/.github/workflows/test-py39-unit.yaml b/.github/workflows/test-py39-unit.yaml index a4b74990..4b605297 100644 --- a/.github/workflows/test-py39-unit.yaml +++ b/.github/workflows/test-py39-unit.yaml @@ -25,3 +25,4 @@ jobs: - name: Run functional tests run: | ./ci/run_unit_tests.sh + pytest ./src/coldfront_plugin_cloud/tests/unit/moc_openshift/ -v --cov=acct_mgt --cov-report=term diff --git a/ci/devstack-test-oidc.py b/ci/devstack-test-oidc.py deleted file mode 100644 index ad92d3ea..00000000 --- a/ci/devstack-test-oidc.py +++ /dev/null @@ -1,26 +0,0 @@ -import os -import sys - -from keystoneauth1 import identity -from keystoneauth1 import session - -host_ip = os.getenv('HOST_IP', 'localhost') -auth = identity.v3.oidc.OidcPassword( - f'http://{host_ip}/identity/v3', - identity_provider='sso', - protocol='openid', - client_id='devstack', - client_secret='nomoresecret', - access_token_endpoint=f'https://{host_ip}:8443/realms/master/protocol/openid-connect/token', - discovery_endpoint=f'https://{host_ip}:8443/realms/master/.well-known/openid-configuration', - username='admin', - password='nomoresecret', - project_name='federated_project', - project_domain_name='federated_domain', -) -s = session.Session(auth) - -if s.get_token(): - print('Authentication successful!') -else: - sys.exit('OpenID Authentication failed') diff --git a/ci/run_functional_tests_openstack.sh b/ci/run_functional_tests_openstack.sh index 5c6b8a6d..903c578f 100755 --- a/ci/run_functional_tests_openstack.sh +++ b/ci/run_functional_tests_openstack.sh @@ -22,10 +22,6 @@ fi export DJANGO_SETTINGS_MODULE="local_settings" export FUNCTIONAL_TESTS="True" export OS_AUTH_URL="http://$HOST_IP/identity" -export KEYCLOAK_URL="http://$HOST_IP:8080" -export KEYCLOAK_USER="admin" -export KEYCLOAK_PASS="nomoresecret" -export KEYCLOAK_REALM="master" coverage run --source="." -m django test coldfront_plugin_cloud.tests.functional.openstack coverage report diff --git a/ci/run_unit_tests.sh b/ci/run_unit_tests.sh index c9b3e6a9..3377c8d2 100755 --- a/ci/run_unit_tests.sh +++ b/ci/run_unit_tests.sh @@ -6,5 +6,7 @@ fi export DJANGO_SETTINGS_MODULE="local_settings" -coverage run --source="." -m django test coldfront_plugin_cloud.tests.unit +coverage run --source="." -a -m django test coldfront_plugin_cloud.tests.unit.test_attribute_migration +coverage run --source="." -a -m django test coldfront_plugin_cloud.tests.unit.test_calculate_quota_unit_hours +coverage run --source="." -a -m django test coldfront_plugin_cloud.tests.unit.test_utils coverage report diff --git a/src/coldfront_plugin_cloud/acct_mgt/moc_openshift.py b/src/coldfront_plugin_cloud/acct_mgt/moc_openshift.py index a26a7d13..b2870d42 100644 --- a/src/coldfront_plugin_cloud/acct_mgt/moc_openshift.py +++ b/src/coldfront_plugin_cloud/acct_mgt/moc_openshift.py @@ -70,6 +70,10 @@ def __init__(self, client, logger, identity_name, quotas, limits): self.limits = limits self.apis = {} + if not self.limits: + self.logger.error("No default limit file provided.") + sys.exit(1) + def get_resource_api(self, api_version: str, kind: str): """Either return the cached resource api from self.apis, or fetch a new one, store it in self.apis, and return it.""" diff --git a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/conftest.py b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/conftest.py index e10439e4..d4deb89f 100644 --- a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/conftest.py +++ b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/conftest.py @@ -5,15 +5,20 @@ import logging import pytest -from acct_mgt.moc_openshift import MocOpenShift4x - +from coldfront_plugin_cloud.acct_mgt.moc_openshift import MocOpenShift4x @pytest.fixture def config(): return { - "IDENTITY_PROVIDER": "fake-id-provider", - "QUOTA_DEF_FILE": "fake-quota-file", - "LIMIT_DEF_FILE": "fake-limit-file", + "identity_name": "fake-id-provider", + "quotas": { + ":requests.fake1": {"base": 2, "coefficient": 0}, + ":requests.fake2": {"base": 2, "coefficient": 0} + }, + "limits": { + "type": "FakeContainer", + "default": {"cpu": "2", "memory": "1024Mi", "nvidia.com/gpu": "0"} + }, } @@ -21,4 +26,4 @@ def config(): def moc(config): fake_client = mock.Mock(spec=["resources"]) fake_logger = mock.Mock(spec=logging.Logger) - return MocOpenShift4x(fake_client, fake_logger, config) + return MocOpenShift4x(fake_client, fake_logger, **config) diff --git a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift.py b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift.py index 69240363..22d1d0b0 100644 --- a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift.py +++ b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift.py @@ -3,13 +3,19 @@ import pytest -from acct_mgt.moc_openshift import MocOpenShift4x +from coldfront_plugin_cloud.acct_mgt.moc_openshift import MocOpenShift4x def test_moc_openshift(moc): assert moc.id_provider == "fake-id-provider" - assert moc.quotafile == "fake-quota-file" - assert moc.limitfile == "fake-limit-file" + assert moc.quotas == { + ":requests.fake1": {"base": 2, "coefficient": 0}, + ":requests.fake2": {"base": 2, "coefficient": 0} + } + assert moc.limits == { + "type": "FakeContainer", + "default": {"cpu": "2", "memory": "1024Mi", "nvidia.com/gpu": "0"} + } def test_moc_openshift_no_limit(): @@ -17,11 +23,11 @@ def test_moc_openshift_no_limit(): MocOpenShift4x( mock.Mock(), mock.Mock(), - { - "IDENTITY_PROVIDER": "fake", - "QUOTA_DEF_FILE": "fake", - "LIMIT_DEF_FILE": None, - }, + **{ + "identity_name": "fake-id-provider", + "quotas": "fake-quota-file", + "limits": None, + } ) @@ -31,11 +37,11 @@ def test_moc_openshift_no_quota(): MocOpenShift4x( mock.Mock(), mock.Mock(), - { - "IDENTITY_PROVIDER": "fake", - "QUOTA_DEF_FILE": None, - "LIMIT_DEF_FILE": "fake", - }, + **{ + "identity_name": "fake-id-provider", + "quotas": None, + "limits": {"fake-limits": 1}, + } ) diff --git a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_clean_metadata.py b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_clean_metadata.py index 393d1256..bed0c6a6 100644 --- a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_clean_metadata.py +++ b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_clean_metadata.py @@ -1,7 +1,7 @@ # pylint: disable=missing-module-docstring import pytest -import acct_mgt.moc_openshift +import coldfront_plugin_cloud.acct_mgt.moc_openshift as moc_openshift @pytest.mark.parametrize( @@ -29,4 +29,4 @@ ], ) def test_clean_openshift_metadata(data, expected): - assert acct_mgt.moc_openshift.clean_openshift_metadata(data) == expected + assert moc_openshift.clean_openshift_metadata(data) == expected diff --git a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_identity.py b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_identity.py index 0803984d..c4f1196f 100644 --- a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_identity.py +++ b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_identity.py @@ -52,13 +52,13 @@ def test_delete_identity(moc): ) -@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_user") +@mock.patch("coldfront_plugin_cloud.acct_mgt.moc_openshift.MocOpenShift4x.get_user") def test_useridentitymapping_exists(fake_get_user, moc): fake_get_user.return_value = {"identities": ["fake-id-provider:fake-id"]} assert moc.useridentitymapping_exists("fake-user", "fake-id") -@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_user") +@mock.patch("coldfront_plugin_cloud.acct_mgt.moc_openshift.MocOpenShift4x.get_user") def test_useridentitymapping_exists_not(fake_get_user, moc): fake_get_user.side_effect = kexc.NotFoundError(mock.Mock()) assert not moc.useridentitymapping_exists("fake-user", "fake-id") diff --git a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_project.py b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_project.py index 8758ae7b..91357b80 100644 --- a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_project.py +++ b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_project.py @@ -28,7 +28,7 @@ def test_project_exists_not(moc): assert not moc.project_exists("fake-project") -@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.create_limits", mock.Mock()) +@mock.patch("coldfront_plugin_cloud.acct_mgt.moc_openshift.MocOpenShift4x.create_limits", mock.Mock()) def test_create_project(moc): moc.create_project("fake-project", "Fake Project", "fake-user") moc.client.resources.get.return_value.create.assert_called_with( diff --git a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_quota.py b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_quota.py index 4baec58e..a8d82de6 100644 --- a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_quota.py +++ b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_quota.py @@ -5,40 +5,31 @@ import pytest -@pytest.mark.xfail(reason="raises FileNotFoundError") -def test_get_quota_definitions_missing(moc): - """What happens if the quota file is missing?""" - with mock.patch("builtins.open", mock.mock_open()) as fake_open: - fake_open.side_effect = FileNotFoundError() - res = moc.get_quota_definitions() - assert res == {} - - @pytest.mark.xfail(reason="raises JSONDecodeError") def test_get_quota_definitions_empty(moc): - """What happens if the quota file exists but is empty?""" - with mock.patch("builtins.open", mock.mock_open(read_data="")): - res = moc.get_quota_definitions() - assert res == {} + """What happens if the quota is None?""" + moc.quotas = None + res = moc.get_quota_definitions() + assert res == {} @pytest.mark.xfail(reason="raises TypeError") def test_get_quota_definitions_invalid(moc): - """What happens if the quota file exists but contains invalid data?""" - with mock.patch("builtins.open", mock.mock_open(read_data='{"foo": "bar"}')): - res = moc.get_quota_definitions() - assert res == {} + """What happens if the quota exists but contains invalid data?""" + moc.quotas = {"foo": "bar"} + res = moc.get_quota_definitions() + assert res == {} def test_get_quota_definitions_valid(moc): - """What happens if a valid quota file exists?""" + """What happens if a valid quota exists?""" quotadefs = { ":configmaps": {"base": 2, "coefficient": 0}, } - with mock.patch("builtins.open", mock.mock_open(read_data=json.dumps(quotadefs))): - res = moc.get_quota_definitions() - quotadefs[":configmaps"]["value"] = None - assert res == quotadefs + moc.quotas = quotadefs + res = moc.get_quota_definitions() + quotadefs[":configmaps"]["value"] = None + assert res == quotadefs def test_split_quota_name(moc): @@ -46,7 +37,7 @@ def test_split_quota_name(moc): assert moc.split_quota_name("scope:foo") == ("scope", "foo") -@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_project", mock.Mock()) +@mock.patch("coldfront_plugin_cloud.acct_mgt.moc_openshift.MocOpenShift4x.get_project", mock.Mock()) def test_get_resourcequotas(moc): fake_quota = mock.Mock(spec=["to_dict"]) fake_quota.to_dict.return_value = {"items": []} @@ -65,7 +56,7 @@ def test_delete_quota(moc): assert res == {} -@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_resourcequotas") +@mock.patch("coldfront_plugin_cloud.acct_mgt.moc_openshift.MocOpenShift4x.get_resourcequotas") def test_delete_moc_quota(fake_get_resourcequotas, moc): fake_get_resourcequotas.return_value = [{"metadata": {"name": "fake-quota"}}] moc.delete_moc_quota("test-project") @@ -74,7 +65,7 @@ def test_delete_moc_quota(fake_get_resourcequotas, moc): ) -@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_resourcequotas") +@mock.patch("coldfront_plugin_cloud.acct_mgt.moc_openshift.MocOpenShift4x.get_resourcequotas") def test_get_moc_quota_from_resourcequotas(fake_get_resourcequotas, moc): fake_get_resourcequotas.return_value = [ { @@ -91,7 +82,7 @@ def test_get_moc_quota_from_resourcequotas(fake_get_resourcequotas, moc): assert res == {":cpu": "1", "BestEffort:memory": "1"} -@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.wait_for_quota_to_settle") +@mock.patch("coldfront_plugin_cloud.acct_mgt.moc_openshift.MocOpenShift4x.wait_for_quota_to_settle") def test_create_shift_quotas(fake_wait_quota, moc): quotadefs = { ":configmaps": {"value": "1"}, @@ -132,11 +123,11 @@ def test_wait_for_quota_to_settle(moc): @mock.patch( - "acct_mgt.moc_openshift.MocOpenShift4x.get_moc_quota_from_resourcequotas", + "coldfront_plugin_cloud.acct_mgt.moc_openshift.MocOpenShift4x.get_moc_quota_from_resourcequotas", mock.Mock(), ) -@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.delete_moc_quota", mock.Mock()) -@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.create_shift_quotas") +@mock.patch("coldfront_plugin_cloud.acct_mgt.moc_openshift.MocOpenShift4x.delete_moc_quota", mock.Mock()) +@mock.patch("coldfront_plugin_cloud.acct_mgt.moc_openshift.MocOpenShift4x.create_shift_quotas") def test_update_moc_quota( fake_create_quotas, moc, @@ -152,17 +143,17 @@ def test_update_moc_quota( } } - with mock.patch("builtins.open", mock.mock_open(read_data=json.dumps(quotadefs))): - moc.update_moc_quota("fake-project", new_quota) - fake_create_quotas.assert_called_with( - "fake-project", - {":configmaps": {"value": None}, ":cpu": {"value": "1000"}}, - ) + moc.quotas = quotadefs + moc.update_moc_quota("fake-project", new_quota) + fake_create_quotas.assert_called_with( + "fake-project", + {":configmaps": {"value": None}, ":cpu": {"value": "1000"}}, + ) -@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.delete_moc_quota", mock.Mock()) -@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_resourcequotas") -@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.create_shift_quotas") +@mock.patch("coldfront_plugin_cloud.acct_mgt.moc_openshift.MocOpenShift4x.delete_moc_quota", mock.Mock()) +@mock.patch("coldfront_plugin_cloud.acct_mgt.moc_openshift.MocOpenShift4x.get_resourcequotas") +@mock.patch("coldfront_plugin_cloud.acct_mgt.moc_openshift.MocOpenShift4x.create_shift_quotas") def test_update_moc_quota_patch( fake_create_quotas, fake_get_resourcequotas, @@ -188,19 +179,19 @@ def test_update_moc_quota_patch( fake_get_resourcequotas.return_value = [fake_quota] - with mock.patch("builtins.open", mock.mock_open(read_data=json.dumps(quotadefs))): - moc.update_moc_quota("fake-project", new_quota, patch=True) - fake_create_quotas.assert_called_with( - "fake-project", - { - ":services": {"value": "2"}, - ":configmaps": {"value": None}, - ":cpu": {"value": "1000"}, - }, - ) + moc.quotas = quotadefs + moc.update_moc_quota("fake-project", new_quota, patch=True) + fake_create_quotas.assert_called_with( + "fake-project", + { + ":services": {"value": "2"}, + ":configmaps": {"value": None}, + ":cpu": {"value": "1000"}, + }, + ) -@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_moc_quota_from_resourcequotas") +@mock.patch("coldfront_plugin_cloud.acct_mgt.moc_openshift.MocOpenShift4x.get_moc_quota_from_resourcequotas") def test_get_moc_quota(fake_get_quota, moc): fake_get_quota.return_value = { ":services": {"value": "2"}, @@ -221,9 +212,9 @@ def test_get_moc_quota(fake_get_quota, moc): def test_get_limit_definitions_valid(moc): - with mock.patch("builtins.open", mock.mock_open(read_data="{}")): - res = moc.get_limit_definitions() - assert res == {} + moc.limits = {} + res = moc.get_limit_definitions() + assert res == {} def test_create_limits(moc): @@ -231,16 +222,16 @@ def test_create_limits(moc): fake_limit = mock.Mock(spec=["to_dict"]) fake_limit.to_dict.return_value = "fake-limit" moc.client.resources.get.return_value.create.return_value = fake_limit - with mock.patch("builtins.open", mock.mock_open(read_data=limitdefs)): - res = moc.create_limits("fake-project") - assert res == "fake-limit" - moc.client.resources.get.return_value.create.assert_called_with( - namespace="fake-project", - body={ - "metadata": {"name": "fake-project-limits"}, - "spec": {"limits": json.loads(limitdefs)}, - }, - ) + moc.limits = json.loads(limitdefs) + res = moc.create_limits("fake-project") + assert res == "fake-limit" + moc.client.resources.get.return_value.create.assert_called_with( + namespace="fake-project", + body={ + "metadata": {"name": "fake-project-limits"}, + "spec": {"limits": json.loads(limitdefs)}, + }, + ) def test_create_limits_custom(moc): diff --git a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_rbac.py b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_rbac.py index 0a4891aa..925686b5 100644 --- a/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_rbac.py +++ b/src/coldfront_plugin_cloud/tests/unit/moc_openshift/test_moc_openshift_rbac.py @@ -11,13 +11,13 @@ def test_user_rolebinding_exists_invalid_role(moc): moc.user_rolebinding_exists("fake-user", "fake-project", "invalid-role") -@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_rolebindings") +@mock.patch("coldfront_plugin_cloud.acct_mgt.moc_openshift.MocOpenShift4x.get_rolebindings") def test_user_rolebinding_exists_not(fake_get_rb, moc): fake_get_rb.side_effect = kexc.NotFoundError(mock.Mock()) assert not moc.user_rolebinding_exists("fake-user", "fake-project", "admin") -@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_rolebindings") +@mock.patch("coldfront_plugin_cloud.acct_mgt.moc_openshift.MocOpenShift4x.get_rolebindings") def test_user_rolebinding_exists(fake_get_rb, moc): fake_get_rb.return_value = { "subjects": [ @@ -36,8 +36,8 @@ def test_add_user_to_role_invalid_role(moc): moc.add_user_to_role("fake-project", "fake-user", "invalid-role") -@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_rolebindings") -@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.update_rolebindings") +@mock.patch("coldfront_plugin_cloud.acct_mgt.moc_openshift.MocOpenShift4x.get_rolebindings") +@mock.patch("coldfront_plugin_cloud.acct_mgt.moc_openshift.MocOpenShift4x.update_rolebindings") def test_add_user_to_role(fake_update_rb, fake_get_rb, moc): fake_get_rb.return_value = { "subjects": [], @@ -49,8 +49,8 @@ def test_add_user_to_role(fake_update_rb, fake_get_rb, moc): ) -@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_rolebindings") -@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.create_rolebindings") +@mock.patch("coldfront_plugin_cloud.acct_mgt.moc_openshift.MocOpenShift4x.get_rolebindings") +@mock.patch("coldfront_plugin_cloud.acct_mgt.moc_openshift.MocOpenShift4x.create_rolebindings") def test_add_user_to_role_not_exists(fake_create_rb, fake_get_rb, moc): fake_get_rb.side_effect = kexc.NotFoundError(mock.Mock()) @@ -63,7 +63,7 @@ def test_remove_user_from_role_invalid_role(moc): moc.remove_user_from_role("fake-project", "fake-user", "invalid-role") -@mock.patch("acct_mgt.moc_openshift.MocOpenShift4x.get_rolebindings") +@mock.patch("coldfront_plugin_cloud.acct_mgt.moc_openshift.MocOpenShift4x.get_rolebindings") def test_remove_user_from_role(fake_get_rb, moc): fake_get_rb.return_value = { "subjects": [{"kind": "User", "name": "fake-user"}], diff --git a/test-requirements.txt b/test-requirements.txt index 456589c6..0a9ed8fe 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -9,3 +9,6 @@ python-neutronclient python-swiftclient kubernetes openshift +pytest +pytest-check +pytest-coverage