PMR: Remove access to stale Profiles with create_or_update_profiles()

CONTRIBUTING.md

@@ -23,6 +23,41 @@ tox -e integration   # integration tests
 tox                  # runs 'fmt', 'lint', 'static', and 'unit' environments
 ```
 
+## Integration Tests
+
+When running/developing integration tests there will be times when the tests fail. During teardown
+of the integration tests the following things happen:
+1. The Juju model that the Profiles Controller charm was deployed gets deleted
+2. The Profile CRs created during tests will still be there
+
+Deleting then a Profile will result in the Profile to never be deleted. This is because:
+1. The created Profile has a finalizer
+2. The Profile Controller that should be responsible for removing the finaliser is not deployed
+3. K8s waits indefinitely until the finaliser is removed, to remove the Profile object
+
+While developing tests locally there are two approaches to go about this:
+
+#### Have Profiles Controller always deployed
+
+In this case you'll need to ensure the Profiles controller is always running. This could be achieved by:
+1. installing the `kubeflow-profiles` charm in a separate model
+2. commenting out the `await deploy_profiles_controller` command
+
+The above will be the fastest way to run the integration tests a lot of times, as you both save time
+from re-deploying the Profiles Controller as well as the Profiles can now be deleted as expected with
+`kubectl`.
+
+You can install the Profiles Controller with the following command:
+```bash
+juju deploy kubeflow-profiles --channel 1.9/stable --trust
+```
+
+#### Remove finaliser from Profile
+
+If you don't want to mess with your cluster and just get the Profile deleted, then you can
+remove the `metadata.finalizers` field in the Profile and K8s will remove the Profile, and garbage
+collect the created namespace.
+
 ## Build the charm
 
 Build the charm in this git repository using:

charmcraft.yaml

@@ -24,6 +24,10 @@ parts:
     # https://github.com/canonical/charmcraft/issues/1722
     build-snaps:
       - rustup
+    build-packages:
+      - pkg-config
+      - libffi-dev
+      - libssl-dev
     override-build: |
       rustup default stable
       craftctl default

pyproject.toml

@@ -4,7 +4,9 @@ package-mode = false
 
 [tool.poetry.dependencies]
 python = "^3.12"
+charmed-kubeflow-chisme = "^0.4.5"
 jsonschema = "^4.23.0"
+lightkube = "^0.15.7"
 ops = "^2.17.0"
 pydantic = "^2.10.3"
 
@@ -84,7 +86,7 @@ optional = true
 [tool.poetry.group.lint.dependencies]
 ruff = "^0.8.0"
 codespell = "^2.3.0"
-pyright = "1.1.386"
+pyright = "^1.1.386"
 black = "^24.10.0"
 pytest = "^8.3.3"
 pytest-operator = ">0.30"

requirements.txt

@@ -1,12 +1,61 @@
 annotated-types==0.7.0 ; python_version >= "3.12" and python_version < "4.0"
-attrs==24.2.0 ; python_version >= "3.12" and python_version < "4.0"
+anyio==4.8.0 ; python_version >= "3.12" and python_version < "4.0"
+attrs==24.3.0 ; python_version >= "3.12" and python_version < "4.0"
+bcrypt==4.2.1 ; python_version >= "3.12" and python_version < "4.0"
+cachetools==5.5.0 ; python_version >= "3.12" and python_version < "4.0"
+certifi==2024.12.14 ; python_version >= "3.12" and python_version < "4.0"
+cffi==1.17.1 ; python_version >= "3.12" and python_version < "4.0"
+charmed-kubeflow-chisme==0.4.6 ; python_version >= "3.12" and python_version < "4.0"
+charset-normalizer==3.4.1 ; python_version >= "3.12" and python_version < "4.0"
+cryptography==44.0.0 ; python_version >= "3.12" and python_version < "4.0"
+deepdiff==6.2.1 ; python_version >= "3.12" and python_version < "4.0"
+google-auth==2.37.0 ; python_version >= "3.12" and python_version < "4.0"
+h11==0.14.0 ; python_version >= "3.12" and python_version < "4.0"
+httpcore==1.0.7 ; python_version >= "3.12" and python_version < "4.0"
+httpx==0.27.2 ; python_version >= "3.12" and python_version < "4.0"
+hvac==2.3.0 ; python_version >= "3.12" and python_version < "4.0"
+idna==3.10 ; python_version >= "3.12" and python_version < "4.0"
+jinja2==3.1.5 ; python_version >= "3.12" and python_version < "4.0"
 jsonschema-specifications==2024.10.1 ; python_version >= "3.12" and python_version < "4.0"
 jsonschema==4.23.0 ; python_version >= "3.12" and python_version < "4.0"
+juju==3.6.1.0 ; python_version >= "3.12" and python_version < "4.0"
+kubernetes==30.1.0 ; python_version >= "3.12" and python_version < "4.0"
+lightkube-models==1.32.0.8 ; python_version >= "3.12" and python_version < "4.0"
+lightkube==0.15.8 ; python_version >= "3.12" and python_version < "4.0"
+macaroonbakery==1.3.4 ; python_version >= "3.12" and python_version < "4.0"
+markupsafe==3.0.2 ; python_version >= "3.12" and python_version < "4.0"
+mypy-extensions==1.0.0 ; python_version >= "3.12" and python_version < "4.0"
+oauthlib==3.2.2 ; python_version >= "3.12" and python_version < "4.0"
 ops==2.17.1 ; python_version >= "3.12" and python_version < "4.0"
-pydantic-core==2.27.1 ; python_version >= "3.12" and python_version < "4.0"
-pydantic==2.10.3 ; python_version >= "3.12" and python_version < "4.0"
+ordered-set==4.1.0 ; python_version >= "3.12" and python_version < "4.0"
+packaging==24.2 ; python_version >= "3.12" and python_version < "4.0"
+paramiko==3.5.0 ; python_version >= "3.12" and python_version < "4.0"
+protobuf==5.29.3 ; python_version >= "3.12" and python_version < "4.0"
+pyasn1-modules==0.4.1 ; python_version >= "3.12" and python_version < "4.0"
+pyasn1==0.6.1 ; python_version >= "3.12" and python_version < "4.0"
+pycparser==2.22 ; python_version >= "3.12" and python_version < "4.0"
+pydantic-core==2.27.2 ; python_version >= "3.12" and python_version < "4.0"
+pydantic==2.10.5 ; python_version >= "3.12" and python_version < "4.0"
+pymacaroons==0.13.0 ; python_version >= "3.12" and python_version < "4.0"
+pynacl==1.5.0 ; python_version >= "3.12" and python_version < "4.0"
+pyrfc3339==1.1 ; python_version >= "3.12" and python_version < "4.0"
+python-dateutil==2.9.0.post0 ; python_version >= "3.12" and python_version < "4.0"
+pytz==2024.2 ; python_version >= "3.12" and python_version < "4.0"
 pyyaml==6.0.2 ; python_version >= "3.12" and python_version < "4.0"
 referencing==0.35.1 ; python_version >= "3.12" and python_version < "4.0"
+requests-oauthlib==2.0.0 ; python_version >= "3.12" and python_version < "4.0"
+requests==2.32.3 ; python_version >= "3.12" and python_version < "4.0"
 rpds-py==0.22.3 ; python_version >= "3.12" and python_version < "4.0"
+rsa==4.9 ; python_version >= "3.12" and python_version < "4"
+ruamel-yaml-clib==0.2.12 ; platform_python_implementation == "CPython" and python_version < "3.13" and python_version >= "3.12"
+ruamel-yaml==0.18.10 ; python_version >= "3.12" and python_version < "4.0"
+serialized-data-interface==0.6.0 ; python_version >= "3.12" and python_version < "4.0"
+six==1.17.0 ; python_version >= "3.12" and python_version < "4.0"
+sniffio==1.3.1 ; python_version >= "3.12" and python_version < "4.0"
+tenacity==9.0.0 ; python_version >= "3.12" and python_version < "4.0"
+toposort==1.10 ; python_version >= "3.12" and python_version < "4.0"
 typing-extensions==4.12.2 ; python_version >= "3.12" and python_version < "4.0"
+typing-inspect==0.9.0 ; python_version >= "3.12" and python_version < "4.0"
+urllib3==2.3.0 ; python_version >= "3.12" and python_version < "4.0"
 websocket-client==1.8.0 ; python_version >= "3.12" and python_version < "4.0"
+websockets==14.1 ; python_version >= "3.12" and python_version < "4.0"

src/profiles_management/create_or_update.py

@@ -0,0 +1,82 @@
+"""Module responsible for creating and updating Profiles based on a PMR.
+
+This module includes both
+1. the high level functions for updating/creating Profiles based on a PM
+2. helpers that will handle the different phases of the above logic
+
+The main function exposed is the create_or_update_profile(pmr), which
+will run an update on the whole cluster's Profiles anad Contributors
+based on a PMR.
+"""
+
+import logging
+from typing import Dict
+
+from charmed_kubeflow_chisme.lightkube.batch import delete_many
+from lightkube import Client
+from lightkube.generic_resource import GenericGlobalResource
+
+from profiles_management.helpers.k8s import get_name
+from profiles_management.helpers.kfam import (
+    list_contributor_authorization_policies,
+    list_contributor_rolebindings,
+)
+from profiles_management.helpers.profiles import list_profiles
+from profiles_management.pmr.classes import ProfilesManagementRepresentation
+
+log = logging.getLogger(__name__)
+client = Client(field_manager="profiles-automator-lightkube")
+
+
+def remove_access_to_stale_profile(profile: GenericGlobalResource):
+    """Remove access to all users from a Profile.
+
+    This is achieved by removing all KFAM RoleBindings and
+    AuthorizationPolicies in the namespace. The RoleBinding / AuthorizationPolicy
+    for the Profile owner will not be touched.
+
+    Args:
+        profile: The lightkube Profile object from which all contributors should be removed.
+    """
+    profile_namespace = get_name(profile)
+
+    log.info("Deleting all KFAM RoleBindings")
+    contributor_rbs = list_contributor_rolebindings(client, profile_namespace)
+    delete_many(client, contributor_rbs, logger=log)
+    log.info("Deleted all KFAM RoleBindings")
+
+    log.info("Deleting all KFAM AuthorizationPolicies")
+    existing_aps = list_contributor_authorization_policies(client)
+    delete_many(client, existing_aps, logger=log)
+    log.info("Deleted all KFAM AuthorizationPolicies")
+
+
+def create_or_update_profiles(pmr: ProfilesManagementRepresentation):
+    """Update the cluster to ensure Profiles and contributors are updated accordingly.
+
+    The function ensures that:
+    1. A Profile is created, if defined in the PMR
+    2. AuthorizationPolicies / RoleBindings are created for a user, if
+       a user is defined to be a contributor to a Profile in the PMR
+    3. AuthorizationPolicies / RoleBindings of a user are removed, if
+       a user isn’t defined to be a contributor to a Profile in the PMR
+    4. If a Profile, in the cluster, is not defined in the PMR:
+       a. It will not be automatically removed, to avoid data loss
+       b. All RoleBindings and AuthorizationPolicies will be removed in that
+          Profile, so that no user will have further access to it
+
+    Args:
+        pmr: The ProfilesManagementRepresentation expressing what Profiles and contributors
+             should exist in the cluster.
+    """
+    log.info("Fetching all Profiles in the cluster")
+
+    existing_profiles: Dict[str, GenericGlobalResource] = {}
+    for profile in list_profiles(client):
+        existing_profiles[get_name(profile)] = profile
+
+    log.info("Removing access to all stale Profiles")
+    for profile_name, existing_profile in existing_profiles.items():
+        if not pmr.has_profile(profile_name):
+            logging.info("Profile %s not in PMR. Will remove access.", profile_name)
+            remove_access_to_stale_profile(existing_profile)

src/profiles_management/helpers/k8s.py

@@ -0,0 +1,24 @@
+"""Generic helpers for manipulating K8s objects, via lightkube."""
+
+from lightkube.generic_resource import GenericGlobalResource, GenericNamespacedResource
+
+
+def get_name(res: GenericNamespacedResource | GenericGlobalResource) -> str:
+    """Return the name from generic lightkube resource.
+
+    Args:
+        res: The resource to get it's name from metadata.name
+
+    Raises:
+        ValueError: if the object doesn't have metadata or metadata.name
+
+    Returns:
+        The name of the object from its metadata.
+    """
+    if not res.metadata:
+        raise ValueError("Couldn't detect name, object has no metadata: %s" % res)
+
+    if not res.metadata.name:
+        raise ValueError("Couldn't detect name, object has no name field: %s" % res)
+
+    return res.metadata.name

src/profiles_management/helpers/kfam.py

@@ -0,0 +1,92 @@
+"""Utility module for manipulating KFAM resources."""
+
+import logging
+from typing import List
+
+from lightkube import Client
+from lightkube.generic_resource import GenericNamespacedResource, create_namespaced_resource
+from lightkube.resources.rbac_authorization_v1 import RoleBinding
+
+log = logging.getLogger(__name__)
+
+AuthorizationPolicy = create_namespaced_resource(
+    group="security.istio.io",
+    version="v1beta1",
+    kind="AuthorizationPolicy",
+    plural="authorizationpolicies",
+)
+
+
+def has_kfam_annotations(resource: GenericNamespacedResource | RoleBinding) -> bool:
+    """Check if resource has "user" and "role" KFAM annotations."""
+    if resource.metadata and resource.metadata.annotations:
+        return "role" in resource.metadata.annotations and "user" in resource.metadata.annotations
+
+    return False
+
+
+def resource_is_for_profile_owner(resource: GenericNamespacedResource | RoleBinding) -> bool:
+    """Check if the resource is for the Profile owner."""
+    if resource.metadata:
+        return (
+            resource.metadata.name == "ns-owner-access-istio"
+            or resource.metadata.name == "namespaceAdmin"
+        )
+
+    return False
+
+
+def list_contributor_rolebindings(client: Client, namespace="") -> List[RoleBinding]:
+    """Return a list of KFAM RoleBindings.
+
+    Only RoleBindings which have "role" and "user" annotations will be returned.
+    The RoleBinding for the Profile owner, with name namespaceAdmin, will not be
+    returned."
+
+    Args:
+        client: The lightkube client to use
+        namespace: The namespace to list contributors from. For all namespaces
+                   you can pass an empty string "".
+
+    Returns:
+        A list of RoleBindings that are used from KFAM for contributors.
+    """
+    role_bindings = client.list(RoleBinding, namespace=namespace)
+
+    # We exclude the RB created by the Profile Controller for the
+    # owner of the Profile
+    # https://github.com/kubeflow/kubeflow/issues/6576
+    return [
+        rb
+        for rb in role_bindings
+        if has_kfam_annotations(rb) and not resource_is_for_profile_owner(rb)
+    ]
+
+
+def list_contributor_authorization_policies(
+    client: Client, namespace=""
+) -> List[GenericNamespacedResource]:
+    """Return a list of KFAM AuthorizationPolicies.
+
+    Only AuthorizationPolicies which have "role" and "user" annotations will be returned.
+    The AuthoriationPolicy for the Profile admin, with name ns-owner-access-istio, will not be
+    returned."
+
+    Args:
+        client: The lightkube client to use
+        namespace: The namespace to list contributors from. For all namespaces
+                   you can use "" value.
+
+    Returns:
+        A list of AuthorizationPolicies that are used from KFAM for contributors.
+    """
+    authorization_policies = client.list(AuthorizationPolicy, namespace=namespace)
+
+    # We exclude the AP created by the Profile Controller for the
+    # owner of the Profile
+    # https://github.com/kubeflow/kubeflow/issues/6576
+    return [
+        ap
+        for ap in authorization_policies
+        if has_kfam_annotations(ap) and not resource_is_for_profile_owner(ap)
+    ]

src/profiles_management/helpers/profiles.py

@@ -0,0 +1,22 @@
+"""Utility module for manipulating Profiles."""
+
+from typing import Iterator
+
+from lightkube import Client
+from lightkube.generic_resource import GenericGlobalResource, create_global_resource
+
+Profile = create_global_resource(
+    group="kubeflow.org", version="v1", kind="Profile", plural="profiles"
+)
+
+
+def list_profiles(client: Client) -> Iterator[GenericGlobalResource]:
+    """Return all Profile CRs in the cluster.
+
+    Args:
+        client: The lightkube client to use
+
+    Returns:
+        Iterator of Profiles in the cluster.
+    """
+    return client.list(Profile)

tests/integration/profiles_management/conftest.py

@@ -0,0 +1,36 @@
+"""Fixtures for all integration tests."""
+
+import logging
+
+import lightkube
+import pytest
+from pytest_operator.plugin import OpsTest
+
+PROFILES_CHARM = "kubeflow-profiles"
+PROFILES_CHANNEL = "1.9/stable"
+PROFILES_TRUST = True
+
+log = logging.getLogger(__name__)
+
+
+@pytest.fixture(scope="session")
+def lightkube_client():
+    """Fixture to create a Lightkube client."""
+    log.info("Initializing lightkube client.")
+    return lightkube.Client(field_manager="profile-automator-tests")
+
+
+# All tests will need to modify Profiles in some way and resources
+# inside their namespace
+@pytest.fixture(scope="module")
+async def deploy_profiles_controller(ops_test: OpsTest):
+    """Deploy the Profiles Controller charm."""
+    if not ops_test.model:
+        pytest.fail("ops_test has a None model", pytrace=False)
+
+    log.info("Deploying the Profiles Controller charm.")
+    await ops_test.model.deploy(PROFILES_CHARM, channel=PROFILES_CHANNEL, trust=PROFILES_TRUST)
+
+    log.info("Waiting for the Profile Controller charm to become active.")
+    await ops_test.model.wait_for_idle(status="active", timeout=60 * 20)
+    log.info("Profile Controller charm is active.")