From 4cef98ad6ca437accc28039876bd4dc61fe0690f Mon Sep 17 00:00:00 2001 From: Phoevos Kalemkeris Date: Thu, 21 Sep 2023 11:15:50 +0100 Subject: [PATCH] fix: Add Persistence Agent ClusterRole and Binding (#324) Apply auth manifests (KF 1.8) for the KFP Persistence Agent, including the ClusterRole, ClusterRoleBinding, and ServiceAccount that allow the workload to get, list, and watch workflows/scheduledworkflows, as well as get namespaces. Signed-off-by: Phoevos Kalemkeris --- charms/kfp-persistence/requirements-unit.txt | 4 +- charms/kfp-persistence/requirements.in | 1 + charms/kfp-persistence/requirements.txt | 4 +- charms/kfp-persistence/src/charm.py | 24 +++++++- .../src/templates/auth_manifests.yaml.j2 | 58 +++++++++++++++++++ .../tests/unit/test_operator.py | 20 ++++--- 6 files changed, 101 insertions(+), 10 deletions(-) create mode 100644 charms/kfp-persistence/src/templates/auth_manifests.yaml.j2 diff --git a/charms/kfp-persistence/requirements-unit.txt b/charms/kfp-persistence/requirements-unit.txt index 66368a48..73ba63c0 100644 --- a/charms/kfp-persistence/requirements-unit.txt +++ b/charms/kfp-persistence/requirements-unit.txt @@ -45,7 +45,9 @@ jinja2==3.1.2 jsonschema==4.17.3 # via serialized-data-interface lightkube==0.14.0 - # via charmed-kubeflow-chisme + # via + # -r requirements.in + # charmed-kubeflow-chisme lightkube-models==1.27.1.4 # via lightkube markupsafe==2.1.3 diff --git a/charms/kfp-persistence/requirements.in b/charms/kfp-persistence/requirements.in index 9569fd46..6a4175d6 100644 --- a/charms/kfp-persistence/requirements.in +++ b/charms/kfp-persistence/requirements.in @@ -1,3 +1,4 @@ charmed-kubeflow-chisme +lightkube ops serialized-data-interface diff --git a/charms/kfp-persistence/requirements.txt b/charms/kfp-persistence/requirements.txt index 3bfd9266..668e277c 100644 --- a/charms/kfp-persistence/requirements.txt +++ b/charms/kfp-persistence/requirements.txt @@ -39,7 +39,9 @@ jinja2==3.1.2 jsonschema==4.17.3 # via serialized-data-interface lightkube==0.14.0 - # via charmed-kubeflow-chisme + # via + # -r requirements.in + # charmed-kubeflow-chisme lightkube-models==1.27.1.4 # via lightkube markupsafe==2.1.3 diff --git a/charms/kfp-persistence/src/charm.py b/charms/kfp-persistence/src/charm.py index 46a910df..a72b898f 100755 --- a/charms/kfp-persistence/src/charm.py +++ b/charms/kfp-persistence/src/charm.py @@ -9,11 +9,16 @@ import logging +import lightkube from charmed_kubeflow_chisme.components.charm_reconciler import CharmReconciler +from charmed_kubeflow_chisme.components.kubernetes_component import KubernetesComponent from charmed_kubeflow_chisme.components.leadership_gate_component import LeadershipGateComponent from charmed_kubeflow_chisme.components.serialised_data_interface_components import ( SdiRelationDataReceiverComponent, ) +from charmed_kubeflow_chisme.kubernetes import create_charm_default_labels +from lightkube.resources.core_v1 import ServiceAccount +from lightkube.resources.rbac_authorization_v1 import ClusterRole, ClusterRoleBinding from ops import CharmBase, main from components.pebble_components import ( @@ -23,6 +28,8 @@ log = logging.getLogger() +K8S_RESOURCE_FILES = ["src/templates/auth_manifests.yaml.j2"] + class KfpPersistenceOperator(CharmBase): """Charm for the data persistence application of Kubeflow Pipelines.""" @@ -46,6 +53,21 @@ def __init__(self, *args, **kwargs): depends_on=[self.leadership_gate], ) + self.kubernetes_resources = self.charm_reconciler.add( + component=KubernetesComponent( + charm=self, + name="kubernetes:auth", + resource_templates=K8S_RESOURCE_FILES, + krh_resource_types={ClusterRole, ClusterRoleBinding, ServiceAccount}, + krh_labels=create_charm_default_labels( + self.app.name, self.model.name, scope="auth" + ), + context_callable=lambda: {"app_name": self.app.name, "namespace": self.model.name}, + lightkube_client=lightkube.Client(), + ), + depends_on=[self.leadership_gate], + ) + self.persistenceagent_container = self.charm_reconciler.add( component=PersistenceAgentPebbleService( charm=self, @@ -67,7 +89,7 @@ def __init__(self, *args, **kwargs): ], ), ), - depends_on=[self.leadership_gate, self.kfp_api_relation], + depends_on=[self.leadership_gate, self.kfp_api_relation, self.kubernetes_resources], ) self.charm_reconciler.install_default_event_handlers() diff --git a/charms/kfp-persistence/src/templates/auth_manifests.yaml.j2 b/charms/kfp-persistence/src/templates/auth_manifests.yaml.j2 new file mode 100644 index 00000000..1a19c933 --- /dev/null +++ b/charms/kfp-persistence/src/templates/auth_manifests.yaml.j2 @@ -0,0 +1,58 @@ +# Source manifests/apps/pipeline/upstream/base/installs/multi-user/persistence-agent/ +# These manifest files have been modified to suit the needs of the charm; the app label, metadata name, +# and namespace fields will be rendered with information from the application and the model. +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + labels: + app: {{ app_name }} + name: {{ app_name }}-role +rules: +- apiGroups: + - argoproj.io + resources: + - workflows + verbs: + - get + - list + - watch +- apiGroups: + - kubeflow.org + resources: + - scheduledworkflows + verbs: + - get + - list + - watch +- apiGroups: + - pipelines.kubeflow.org + resources: + - scheduledworkflows + - workflows + verbs: + - report +- apiGroups: + - '' + resources: + - namespaces + verbs: + - get +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: {{ app_name }}-binding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: {{ app_name }}-role +subjects: +- kind: ServiceAccount + name: {{ app_name }}-sa + namespace: {{ namespace }} +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: {{ app_name }}-sa + namespace: {{ namespace }} diff --git a/charms/kfp-persistence/tests/unit/test_operator.py b/charms/kfp-persistence/tests/unit/test_operator.py index 1052ce3c..00adacff 100644 --- a/charms/kfp-persistence/tests/unit/test_operator.py +++ b/charms/kfp-persistence/tests/unit/test_operator.py @@ -21,9 +21,15 @@ def harness(): return harness -def test_not_leader( - harness, # noqa F811 -): +@pytest.fixture() +def mocked_lightkube_client(mocker): + """Mocks the Lightkube Client in charm.py, returning a mock instead.""" + mocked_lightkube_client = MagicMock() + mocker.patch("charm.lightkube.Client", return_value=mocked_lightkube_client) + yield mocked_lightkube_client + + +def test_not_leader(harness, mocked_lightkube_client): """Test not a leader scenario.""" harness.begin_with_initial_hooks() assert harness.charm.model.unit.status == WaitingStatus( @@ -35,7 +41,7 @@ def test_not_leader( OTHER_APP_NAME = "kfp-api-provider" -def test_kfp_api_relation_with_data(harness): +def test_kfp_api_relation_with_data(harness, mocked_lightkube_client): """Test that if Leadership is Active, the kfp-api relation operates as expected.""" # Arrange harness.begin() @@ -51,7 +57,7 @@ def test_kfp_api_relation_with_data(harness): assert isinstance(harness.charm.kfp_api_relation.status, ActiveStatus) -def test_kfp_api_relation_without_data(harness): +def test_kfp_api_relation_without_data(harness, mocked_lightkube_client): """Test that the kfp-api relation goes Blocked if no data is available.""" # Arrange harness.begin() @@ -67,7 +73,7 @@ def test_kfp_api_relation_without_data(harness): assert isinstance(harness.charm.kfp_api_relation.status, BlockedStatus) -def test_kfp_api_relation_without_relation(harness): +def test_kfp_api_relation_without_relation(harness, mocked_lightkube_client): """Test that the kfp-api relation goes Blocked if no relation is established.""" # Arrange harness.begin() @@ -83,7 +89,7 @@ def test_kfp_api_relation_without_relation(harness): assert isinstance(harness.charm.kfp_api_relation.status, BlockedStatus) -def test_pebble_services_running(harness): +def test_pebble_services_running(harness, mocked_lightkube_client): """Test that if the Kubernetes Component is Active, the pebble services successfully start.""" # Arrange harness.begin()