From a79376530e4e01224100c5321f468c142fa7804a Mon Sep 17 00:00:00 2001 From: Ismael Morato Date: Mon, 25 Mar 2024 09:50:01 +0100 Subject: [PATCH] Enable role binding (#235) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Enable role binding * Update fiaas_deploy_daemon/config.py Co-authored-by: Eloy Maillo <14107036+herodes1991@users.noreply.github.com> * Update fiaas_deploy_daemon/specs/models.py Co-authored-by: Eloy Maillo <14107036+herodes1991@users.noreply.github.com> * Change role binding deployer implementation * Ack review comments * Rename to check_if_matches * Add tests when role binding exists * Fix codestyle * Update fiaas_deploy_daemon/deployer/kubernetes/role_binding.py Co-authored-by: Øyvind Ingebrigtsen Øvergaard * Add test for clean * Fix style * Update helm/fiaas-deploy-daemon/templates/custom_rolebinding.yaml Co-authored-by: Øyvind Ingebrigtsen Øvergaard * Improve operator guide and add empty lists in the values example * Update operator_guide.md --------- Co-authored-by: Eloy Maillo <14107036+herodes1991@users.noreply.github.com> Co-authored-by: herodes1991 Co-authored-by: Øyvind Ingebrigtsen Øvergaard --- docs/operator_guide.md | 11 ++ docs/v3_spec.md | 2 + fiaas_deploy_daemon/config.py | 16 ++ fiaas_deploy_daemon/crd/types.py | 1 + .../deployer/kubernetes/__init__.py | 2 + .../deployer/kubernetes/adapter.py | 6 +- .../deployer/kubernetes/role_binding.py | 100 +++++++++++ fiaas_deploy_daemon/specs/models.py | 1 + fiaas_deploy_daemon/specs/v3/defaults.yml | 2 + fiaas_deploy_daemon/specs/v3/factory.py | 1 + .../templates/custom_rolebinding.yaml | 95 +++++++++++ helm/fiaas-deploy-daemon/templates/role.yaml | 2 + helm/fiaas-deploy-daemon/values.yaml.tpl | 2 + tests/fiaas_deploy_daemon/conftest.py | 4 +- .../deployer/kubernetes/test_adapter.py | 49 +++++- .../deployer/kubernetes/test_autoscaler.py | 2 + .../kubernetes/test_deployment_deploy.py | 2 + .../kubernetes/test_ingress_deploy.py | 6 +- .../kubernetes/test_networking_v1_ingress.py | 6 +- .../deployer/kubernetes/test_ready_check.py | 2 +- .../deployer/kubernetes/test_role_binding.py | 158 ++++++++++++++++++ .../kubernetes/test_service_deploy.py | 2 + .../deployer/test_deploy.py | 4 +- .../specs/v3/test_v3factory.py | 2 + .../test_devhose_transformer.py | 2 +- 25 files changed, 468 insertions(+), 12 deletions(-) create mode 100644 fiaas_deploy_daemon/deployer/kubernetes/role_binding.py create mode 100644 helm/fiaas-deploy-daemon/templates/custom_rolebinding.yaml create mode 100644 tests/fiaas_deploy_daemon/deployer/kubernetes/test_role_binding.py diff --git a/docs/operator_guide.md b/docs/operator_guide.md index f0483aa8..d2806f08 100644 --- a/docs/operator_guide.md +++ b/docs/operator_guide.md @@ -279,6 +279,17 @@ specified as `extensions.secrets.foosecrets.annotations` will be added to the po Using the special value 'default' for the 'type' will mean the image will be attached when an application doesn't specify any other secrets configuration. +### RoleBinding and ClusterRole Configuration (--list-of-cluster-roles --list-of-roles) + +Enables the creation of a `RoleBinding`. If provided a list-of-cluster-roles, it will generate the `RoleBinding` with the kind `ClusterRole`. If provided a list-of-roles, it will generate the `RoleBinding` with the kind `Role`. +This is useful for when you wish all applications to have read access to specific resources. Also you will need to include the roles and rolebindings in the values.yaml file, under the keys `rbac.roleBinding.roles` and `rbac.roleBinding.clusterRoles` + +##### List of ClusterRoles and Roles for Role Binding + +- `--list-of-cluster-roles=["A", "B"]`: This flag allows the creation of `RoleBinding` objects with `ClusterRole` kind for the specified cluster-level roles. + +- `--list-of-roles=["A", "B"]`: This flag allows the creation of `RoleBinding` objects with `Role` kind for the specified namespace-level roles. + #### Strongbox In AWS you have the option of using [Strongbox](https://https://github.com/schibsted/strongbox) for your secrets. If you wish to use Strongbox, the configuration option `strongbox-init-container-image` should be set to an image that can get secrets from Strongbox. This option is very similar to the previous variant, except that Strongbox gets a few more pieces of information from the application configuration. The application must specify an IAM role, and can select AWS region and a list of secret groups to get secrets from. diff --git a/docs/v3_spec.md b/docs/v3_spec.md index 721edb48..c587d77b 100644 --- a/docs/v3_spec.md +++ b/docs/v3_spec.md @@ -602,6 +602,7 @@ annotations: service_account: {} pod: {} pod_disruption_budget: {} + role_binding: {} ``` The annotations are organized under the Kubernetes resource they will be applied to. To specify custom anotations, set @@ -637,6 +638,7 @@ labels: service_account: {} pod: {} pod_disruption_budget: {} + role_binding: {} ``` The labels are organized under the Kubernetes resource they will be applied to. To specify custom labels, set diff --git a/fiaas_deploy_daemon/config.py b/fiaas_deploy_daemon/config.py index 953e6648..8cf21323 100644 --- a/fiaas_deploy_daemon/config.py +++ b/fiaas_deploy_daemon/config.py @@ -306,6 +306,22 @@ def _parse_args(self, args): "--include-status-in-app", help="Include status subresource in application CRD", default=False, action="store_true" ) + parser.add_argument( + "--list-of-roles", + help="list of roles", + default=[], + action="append", + type=str, + dest="list_of_roles", + ) + parser.add_argument( + "--list-of-cluster-roles", + help="list of clusterroles", + default=[], + action="append", + type=str, + dest="list_of_cluster_roles", + ) parser.add_argument( "--pdb-max-unavailable", help="The maximum number of pods that can be unavailable after an eviction", diff --git a/fiaas_deploy_daemon/crd/types.py b/fiaas_deploy_daemon/crd/types.py index 0ed749d5..6088a4ec 100644 --- a/fiaas_deploy_daemon/crd/types.py +++ b/fiaas_deploy_daemon/crd/types.py @@ -32,6 +32,7 @@ class AdditionalLabelsOrAnnotations(Model): pod = Field(dict) status = Field(dict) pod_disruption_budget = Field(dict) + role_binding = Field(dict) class FiaasApplicationSpec(Model): diff --git a/fiaas_deploy_daemon/deployer/kubernetes/__init__.py b/fiaas_deploy_daemon/deployer/kubernetes/__init__.py index d3dfa566..928ae261 100644 --- a/fiaas_deploy_daemon/deployer/kubernetes/__init__.py +++ b/fiaas_deploy_daemon/deployer/kubernetes/__init__.py @@ -27,6 +27,7 @@ from .service import ServiceDeployer from .service_account import ServiceAccountDeployer from .owner_references import OwnerReferences +from .role_binding import RoleBindingDeployer from .pod_disruption_budget import PodDisruptionBudgetDeployer from k8s.models.ingress import IngressTLS as V1Beta1IngressTLS from k8s.models.networking_v1_ingress import IngressTLS as StableIngressTLS @@ -45,6 +46,7 @@ def configure(self, bind): bind("ingress_tls_deployer", to_class=IngressTLSDeployer) bind("owner_references", to_class=OwnerReferences) bind("pod_disruption_budget_deployer", to_class=PodDisruptionBudgetDeployer) + bind("role_binding_deployer", to_class=RoleBindingDeployer) if self.use_networkingv1_ingress: bind("ingress_adapter", to_class=NetworkingV1IngressAdapter) diff --git a/fiaas_deploy_daemon/deployer/kubernetes/adapter.py b/fiaas_deploy_daemon/deployer/kubernetes/adapter.py index 82d8e953..c388fe9d 100644 --- a/fiaas_deploy_daemon/deployer/kubernetes/adapter.py +++ b/fiaas_deploy_daemon/deployer/kubernetes/adapter.py @@ -28,6 +28,7 @@ from .service import ServiceDeployer from .service_account import ServiceAccountDeployer from .pod_disruption_budget import PodDisruptionBudgetDeployer +from .role_binding import RoleBindingDeployer LOG = logging.getLogger(__name__) @@ -37,7 +38,8 @@ class K8s(object): def __init__( self, config, service_deployer, deployment_deployer, ingress_deployer, - autoscaler, service_account_deployer, pod_disruption_budget_deployer + autoscaler, service_account_deployer, pod_disruption_budget_deployer, + role_binding_deployer ): self._version = config.version self._enable_service_account_per_app = config.enable_service_account_per_app @@ -47,6 +49,7 @@ def __init__( self._autoscaler_deployer: AutoscalerDeployer = autoscaler self._service_account_deployer: ServiceAccountDeployer = service_account_deployer self._pod_disruption_budget_deployer: PodDisruptionBudgetDeployer = pod_disruption_budget_deployer + self._role_binding_deployer: RoleBindingDeployer = role_binding_deployer def deploy(self, app_spec: AppSpec): if _besteffort_qos_is_required(app_spec): @@ -55,6 +58,7 @@ def deploy(self, app_spec: AppSpec): labels = self._make_labels(app_spec) if self._enable_service_account_per_app is True: self._service_account_deployer.deploy(app_spec, labels) + self._role_binding_deployer.deploy(app_spec, labels) self._service_deployer.deploy(app_spec, selector, labels) self._ingress_deployer.deploy(app_spec, labels) self._deployment_deployer.deploy(app_spec, selector, labels, _besteffort_qos_is_required(app_spec)) diff --git a/fiaas_deploy_daemon/deployer/kubernetes/role_binding.py b/fiaas_deploy_daemon/deployer/kubernetes/role_binding.py new file mode 100644 index 00000000..b6f8a3f8 --- /dev/null +++ b/fiaas_deploy_daemon/deployer/kubernetes/role_binding.py @@ -0,0 +1,100 @@ +import logging + +from k8s.models.common import ObjectMeta +from k8s.models.role_binding import RoleBinding, RoleRef, Subject +from fiaas_deploy_daemon.specs.models import AppSpec +from fiaas_deploy_daemon.deployer.kubernetes.owner_references import OwnerReferences +from fiaas_deploy_daemon.tools import merge_dicts + +LOG = logging.getLogger(__name__) + + +class RoleBindingDeployer: + def __init__(self, config, owner_references): + self._owner_references: OwnerReferences = owner_references + self._list_of_roles = config.list_of_roles + self._list_of_cluster_roles = config.list_of_cluster_roles + + def deploy(self, app_spec: AppSpec, labels): + custom_annotations = {} + custom_labels = labels + custom_labels = merge_dicts(app_spec.labels.role_binding, custom_labels) + custom_annotations = merge_dicts(app_spec.annotations.role_binding, custom_annotations) + # Getting list of rolebindings with the label app=app_name + role_bindings = RoleBinding.find(name=app_spec.name, namespace=app_spec.namespace) + self._clean_not_needed_role_bindings(role_bindings) + self._update_or_create_role_bindings(app_spec, self._list_of_roles, "Role", custom_annotations, custom_labels, role_bindings) + self._update_or_create_role_bindings(app_spec, self._list_of_cluster_roles, "ClusterRole", custom_annotations, custom_labels, + role_bindings) + + def _update_or_create_role_bindings(self, app_spec: AppSpec, roles_list, role_kind, custom_annotations, custom_labels, + role_bindings): + namespace = app_spec.namespace + service_account_name = app_spec.name + for role_name in roles_list: + role_binding = self._find_role_in_role_bindings(role_kind, role_name, role_bindings) + if role_binding: + if self._owned_by_fiaas(role_binding): + LOG.info("Updating RoleBinding %s", role_binding.metadata.name) + generate = False + role_binding_name = role_binding.metadata.name + else: + LOG.info( + "Aborting the creation of a roleBinding for Application: %s with %s: %s, role is already bound by %s", + app_spec.name, + role_kind, + role_name, + role_binding.metadata.name + ) + continue + else: + role_binding = RoleBinding() + role_binding_name = f"{app_spec.name}-" + LOG.info("Creating a new rolebinding for %s, the name will be generated by K8s with the prefix %s", + app_spec.name, role_binding_name) + generate = True + + self._deploy_role_binding(app_spec, role_kind, custom_annotations, custom_labels, namespace, + service_account_name, role_name, role_binding, role_binding_name, generate) + + def _deploy_role_binding(self, app_spec, role_kind, custom_annotations, custom_labels, namespace, + service_account_name, role_name, role_binding, role_binding_name, generate): + if generate: + role_binding.metadata = ObjectMeta(generateName=role_binding_name, namespace=namespace, + labels=custom_labels, annotations=custom_annotations) + else: + role_binding.metadata = ObjectMeta(name=role_binding_name, namespace=namespace, labels=custom_labels, + annotations=custom_annotations) + + role_ref = RoleRef(kind=role_kind, apiGroup="rbac.authorization.k8s.io", name=role_name) + subject = Subject(kind="ServiceAccount", name=service_account_name, namespace=namespace) + role_binding.roleRef = role_ref + role_binding.subjects = [subject] + self._owner_references.apply(role_binding, app_spec) + role_binding.save() + + def _find_role_in_role_bindings(self, role_kind, role_name, role_bindings: list[RoleBinding]): + for role_binding in role_bindings: + if role_binding.roleRef.kind == role_kind and role_binding.roleRef.name == role_name: + return role_binding + return None + + def _clean_not_needed_role_bindings(self, role_bindings: list[RoleBinding]): + role_bindings_aux = role_bindings.copy() + for role_binding in role_bindings_aux: + if not self._should_bind(role_binding): + if self._owned_by_fiaas(role_binding): + LOG.info("Deleting RoleBinding %s", role_binding.metadata.name) + RoleBinding.delete(role_binding.metadata.name, role_binding.metadata.namespace) + role_bindings.remove(role_binding) + + def _owned_by_fiaas(self, role_binding): + return any( + ref.apiVersion == "fiaas.schibsted.io/v1" and ref.kind == "Application" + for ref in role_binding.metadata.ownerReferences + ) + + # Check if matches the role_binding with any role or clusterRole in the list_of_roles or list_of_cluster_roles + def _should_bind(self, role_binding): + return (role_binding.roleRef.kind == "Role" and role_binding.roleRef.name in self._list_of_roles) \ + or (role_binding.roleRef.kind == "ClusterRole" and role_binding.roleRef.name in self._list_of_cluster_roles) diff --git a/fiaas_deploy_daemon/specs/models.py b/fiaas_deploy_daemon/specs/models.py index 14fe5b9c..3cc5074e 100644 --- a/fiaas_deploy_daemon/specs/models.py +++ b/fiaas_deploy_daemon/specs/models.py @@ -110,6 +110,7 @@ def version(self): "pod", "status", "pod_disruption_budget", + "role_binding", ], ) diff --git a/fiaas_deploy_daemon/specs/v3/defaults.yml b/fiaas_deploy_daemon/specs/v3/defaults.yml index c851081d..d1756dc6 100644 --- a/fiaas_deploy_daemon/specs/v3/defaults.yml +++ b/fiaas_deploy_daemon/specs/v3/defaults.yml @@ -85,6 +85,7 @@ annotations: # Annotations to be set on generated objects service: {} pod: {} pod_disruption_budget: {} + role_binding: {} labels: # Labels to be set on generated objects deployment: {} horizontal_pod_autoscaler: {} @@ -93,6 +94,7 @@ labels: # Labels to be set on generated objects service: {} pod: {} pod_disruption_budget: {} + role_binding: {} secrets_in_environment: false # If a Secret of the same name exists, it is exposed to the pod as environment variables admin_access: false # What access the pod has to the k8s api server extensions: diff --git a/fiaas_deploy_daemon/specs/v3/factory.py b/fiaas_deploy_daemon/specs/v3/factory.py index b7dccf66..4a254f75 100644 --- a/fiaas_deploy_daemon/specs/v3/factory.py +++ b/fiaas_deploy_daemon/specs/v3/factory.py @@ -221,6 +221,7 @@ def _labels_annotations_spec(labels_annotations_lookup, overrides): "pod": dict(labels_annotations_lookup["pod"]), "status": {}, "pod_disruption_budget": dict(labels_annotations_lookup["pod_disruption_budget"]), + "role_binding": dict(labels_annotations_lookup["role_binding"]), } if overrides: globals = _get_value("_global", overrides) diff --git a/helm/fiaas-deploy-daemon/templates/custom_rolebinding.yaml b/helm/fiaas-deploy-daemon/templates/custom_rolebinding.yaml new file mode 100644 index 00000000..4fea4ce8 --- /dev/null +++ b/helm/fiaas-deploy-daemon/templates/custom_rolebinding.yaml @@ -0,0 +1,95 @@ +{{- if .Values.rbac.roleBinding.roles -}} +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: {{ .Values.name }}-role-grantor + labels: +{{ include "fiaas-deploy-daemon.labels" . | indent 4 }} +{{ include "fiaas-deploy-daemon.labelsOrAnnotations" .Values.rbac.roleBinding.labels | indent 4 }} +{{- if or .Values.annotations.global .Values.rbac.roleBinding.annotations }} + annotations: +{{- include "fiaas-deploy-daemon.labelsOrAnnotations" .Values.annotations.global | indent 4 }} +{{- include "fiaas-deploy-daemon.labelsOrAnnotations" .Values.rbac.roleBinding.annotations | indent 4 }} +{{- end }} +rules: + - apiGroups: + - rbac.authorization.k8s.io + resources: + - roles + verbs: + - bind + resourceNames: + {{- range $role := .Values.rbac.roleBinding.roles }} + - {{ $role }} + {{- end}} + +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: {{ .Values.name }}-rb-role-grantor + labels: +{{ include "fiaas-deploy-daemon.labels" . | indent 4 }} +{{ include "fiaas-deploy-daemon.labelsOrAnnotations" .Values.rbac.roleBinding.labels | indent 4 }} +{{- if or .Values.annotations.global .Values.rbac.roleBinding.annotations }} + annotations: +{{- include "fiaas-deploy-daemon.labelsOrAnnotations" .Values.annotations.global | indent 4 }} +{{- include "fiaas-deploy-daemon.labelsOrAnnotations" .Values.rbac.roleBinding.annotations | indent 4 }} +{{- end }} +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: {{ .Values.name }}-role-grantor +subjects: +- kind: ServiceAccount + name: {{ .Values.name }} + namespace: {{ .Release.Namespace }} +{{- end }} + +{{- if .Values.rbac.roleBinding.clusterRoles }} +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: {{ .Values.name }}-clusterrole-grantor + labels: +{{ include "fiaas-deploy-daemon.labels" . | indent 4 }} +{{ include "fiaas-deploy-daemon.labelsOrAnnotations" .Values.rbac.roleBinding.labels | indent 4 }} +{{- if or .Values.annotations.global .Values.rbac.roleBinding.annotations }} + annotations: +{{- include "fiaas-deploy-daemon.labelsOrAnnotations" .Values.annotations.global | indent 4 }} +{{- include "fiaas-deploy-daemon.labelsOrAnnotations" .Values.rbac.roleBinding.annotations | indent 4 }} +{{- end }} +rules: + - apiGroups: + - rbac.authorization.k8s.io + resources: + - clusterroles + verbs: + - bind + resourceNames: + {{- range $clusterRole := .Values.rbac.roleBinding.clusterRoles }} + - {{ $clusterRole }} + {{- end}} +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: {{ .Values.name }}-rb-clusterrole-grantor + labels: +{{ include "fiaas-deploy-daemon.labels" . | indent 4 }} +{{ include "fiaas-deploy-daemon.labelsOrAnnotations" .Values.rbac.roleBinding.labels | indent 4 }} +{{- if or .Values.annotations.global .Values.rbac.roleBinding.annotations }} + annotations: +{{- include "fiaas-deploy-daemon.labelsOrAnnotations" .Values.annotations.global | indent 4 }} +{{- include "fiaas-deploy-daemon.labelsOrAnnotations" .Values.rbac.roleBinding.annotations | indent 4 }} +{{- end }} +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: {{ .Values.name }}-clusterrole-grantor +subjects: +- kind: ServiceAccount + name: {{ .Values.name }} + namespace: {{ .Release.Namespace }} +{{- end }} diff --git a/helm/fiaas-deploy-daemon/templates/role.yaml b/helm/fiaas-deploy-daemon/templates/role.yaml index a5042e19..480dbe52 100644 --- a/helm/fiaas-deploy-daemon/templates/role.yaml +++ b/helm/fiaas-deploy-daemon/templates/role.yaml @@ -49,6 +49,7 @@ rules: - extensions - networking.k8s.io - policy + - rbac.authorization.k8s.io resources: - configmaps - deployments @@ -59,6 +60,7 @@ rules: - resourcequotas - services - serviceaccounts + - rolebindings verbs: - create - delete diff --git a/helm/fiaas-deploy-daemon/values.yaml.tpl b/helm/fiaas-deploy-daemon/values.yaml.tpl index 5ef499bd..54c6029c 100644 --- a/helm/fiaas-deploy-daemon/values.yaml.tpl +++ b/helm/fiaas-deploy-daemon/values.yaml.tpl @@ -50,6 +50,8 @@ rbac: create: true labels: {} annotations: {} + roles: [] + clusterRoles: [] clusterRole: create: true labels: {} diff --git a/tests/fiaas_deploy_daemon/conftest.py b/tests/fiaas_deploy_daemon/conftest.py index 9f7be556..df0c61a3 100644 --- a/tests/fiaas_deploy_daemon/conftest.py +++ b/tests/fiaas_deploy_daemon/conftest.py @@ -91,8 +91,8 @@ def app_spec(): teams=["foo"], tags=["bar"], deployment_id="test_app_deployment_id", - labels=LabelAndAnnotationSpec({}, {}, {}, {}, {}, {}, {}, {}), - annotations=LabelAndAnnotationSpec({}, {}, {}, {}, {}, {}, {}, {}), + labels=LabelAndAnnotationSpec({}, {}, {}, {}, {}, {}, {}, {}, {}), + annotations=LabelAndAnnotationSpec({}, {}, {}, {}, {}, {}, {}, {}, {}), ingresses=[ IngressItemSpec(host=None, pathmappings=[IngressPathMappingSpec(path="/", port=80)], annotations={}) ], diff --git a/tests/fiaas_deploy_daemon/deployer/kubernetes/test_adapter.py b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_adapter.py index dde28473..2ad42f70 100644 --- a/tests/fiaas_deploy_daemon/deployer/kubernetes/test_adapter.py +++ b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_adapter.py @@ -24,6 +24,7 @@ from fiaas_deploy_daemon.deployer.kubernetes.service import ServiceDeployer from fiaas_deploy_daemon.deployer.kubernetes.service_account import ServiceAccountDeployer from fiaas_deploy_daemon.deployer.kubernetes.pod_disruption_budget import PodDisruptionBudgetDeployer +from fiaas_deploy_daemon.deployer.kubernetes.role_binding import RoleBindingDeployer from fiaas_deploy_daemon.specs.models import ResourcesSpec, ResourceRequirementSpec FIAAS_VERSION = "1" @@ -56,6 +57,10 @@ def autoscaler_deployer(self): def pod_disruption_budget_deployer(self): return mock.create_autospec(PodDisruptionBudgetDeployer) + @pytest.fixture(autouse=True) + def role_binding_deployer(self): + return mock.create_autospec(RoleBindingDeployer) + @pytest.fixture(autouse=True) def resource_quota_list(self): with mock.patch("k8s.models.resourcequota.ResourceQuota.list") as mockk: @@ -66,7 +71,7 @@ def resource_quota_list(self): def k8s( self, service_deployer, deployment_deployer, ingress_deployer, autoscaler_deployer, service_account_deployer, - pod_disruption_budget_deployer + pod_disruption_budget_deployer, role_binding_deployer ): config = mock.create_autospec(Configuration([]), spec_set=True) config.version = FIAAS_VERSION @@ -78,6 +83,7 @@ def k8s( autoscaler_deployer, service_account_deployer, pod_disruption_budget_deployer, + role_binding_deployer ) def test_make_labels(self, k8s, app_spec): @@ -203,6 +209,7 @@ def test_pass_to_service_account( service_account_deployer, service_account_per_app_enabled, pod_disruption_budget_deployer, + role_binding_deployer ): config = mock.create_autospec(Configuration([]), spec_set=True) @@ -216,6 +223,7 @@ def test_pass_to_service_account( autoscaler_deployer, service_account_deployer, pod_disruption_budget_deployer, + role_binding_deployer ) labels = k8s._make_labels(app_spec) @@ -226,3 +234,42 @@ def test_pass_to_service_account( pytest.helpers.assert_any_call(service_account_deployer.deploy, app_spec, labels) else: service_account_deployer.deploy.assert_not_called() + + @pytest.mark.parametrize("enable_service_account_per_app", (True, False)) + def test_pass_to_role_binding( + self, + app_spec, + k8s, + service_deployer, + resource_quota_list, + deployment_deployer, + ingress_deployer, + autoscaler_deployer, + service_account_deployer, + enable_service_account_per_app, + pod_disruption_budget_deployer, + role_binding_deployer + ): + + config = mock.create_autospec(Configuration([]), spec_set=True) + config.version = FIAAS_VERSION + config.enable_service_account_per_app = enable_service_account_per_app + k8s = K8s( + config, + service_deployer, + deployment_deployer, + ingress_deployer, + autoscaler_deployer, + service_account_deployer, + pod_disruption_budget_deployer, + role_binding_deployer + ) + + labels = k8s._make_labels(app_spec) + + k8s.deploy(app_spec) + + if enable_service_account_per_app: + pytest.helpers.assert_any_call(role_binding_deployer.deploy, app_spec, labels) + else: + role_binding_deployer.deploy.assert_not_called() diff --git a/tests/fiaas_deploy_daemon/deployer/kubernetes/test_autoscaler.py b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_autoscaler.py index 4a87ea74..14d8dc83 100644 --- a/tests/fiaas_deploy_daemon/deployer/kubernetes/test_autoscaler.py +++ b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_autoscaler.py @@ -117,6 +117,7 @@ def test_new_autoscaler_with_custom_labels_and_annotations( status={}, service_account={}, pod_disruption_budget={}, + role_binding={}, ) annotations = LabelAndAnnotationSpec( deployment={}, @@ -127,6 +128,7 @@ def test_new_autoscaler_with_custom_labels_and_annotations( status={}, service_account={}, pod_disruption_budget={}, + role_binding={}, ) app_spec = app_spec._replace(labels=labels, annotations=annotations) diff --git a/tests/fiaas_deploy_daemon/deployer/kubernetes/test_deployment_deploy.py b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_deployment_deploy.py index 01ff195b..563648a5 100644 --- a/tests/fiaas_deploy_daemon/deployer/kubernetes/test_deployment_deploy.py +++ b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_deployment_deploy.py @@ -217,6 +217,7 @@ def app_spec(self, request, app_spec): pod=pod_labels, status={}, pod_disruption_budget={}, + role_binding={}, ) annotations = LabelAndAnnotationSpec( deployment=deploy_annotations, @@ -227,6 +228,7 @@ def app_spec(self, request, app_spec): pod=pod_annotations, status={}, pod_disruption_budget={}, + role_binding={}, ) if generic_toggle: diff --git a/tests/fiaas_deploy_daemon/deployer/kubernetes/test_ingress_deploy.py b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_ingress_deploy.py index c0798fec..46fd9ba3 100644 --- a/tests/fiaas_deploy_daemon/deployer/kubernetes/test_ingress_deploy.py +++ b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_ingress_deploy.py @@ -100,8 +100,8 @@ def app_spec(**kwargs): teams=["foo"], tags=["bar"], deployment_id="test_app_deployment_id", - labels=LabelAndAnnotationSpec({}, {}, {}, {}, {}, {}, {}, {}), - annotations=LabelAndAnnotationSpec({}, {}, ANNOTATIONS.copy(), {}, {}, {}, {}, {}), + labels=LabelAndAnnotationSpec({}, {}, {}, {}, {}, {}, {}, {}, {}), + annotations=LabelAndAnnotationSpec({}, {}, ANNOTATIONS.copy(), {}, {}, {}, {}, {}, {}), ingresses=[ IngressItemSpec(host=None, pathmappings=[IngressPathMappingSpec(path="/", port=80)], annotations={}) ], @@ -656,6 +656,7 @@ class IngressTestcase: pod={}, status={}, pod_disruption_budget={}, + role_binding={}, ), annotations=LabelAndAnnotationSpec( deployment={}, @@ -666,6 +667,7 @@ class IngressTestcase: pod={}, status={}, pod_disruption_budget={}, + role_binding={}, ), ), ingress( diff --git a/tests/fiaas_deploy_daemon/deployer/kubernetes/test_networking_v1_ingress.py b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_networking_v1_ingress.py index 3c8f323a..a6459d0e 100644 --- a/tests/fiaas_deploy_daemon/deployer/kubernetes/test_networking_v1_ingress.py +++ b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_networking_v1_ingress.py @@ -98,8 +98,8 @@ def app_spec(**kwargs): teams=["foo"], tags=["bar"], deployment_id="test_app_deployment_id", - labels=LabelAndAnnotationSpec({}, {}, {}, {}, {}, {}, {}, {}), - annotations=LabelAndAnnotationSpec({}, {}, ANNOTATIONS.copy(), {}, {}, {}, {}, {}), + labels=LabelAndAnnotationSpec({}, {}, {}, {}, {}, {}, {}, {}, {}), + annotations=LabelAndAnnotationSpec({}, {}, ANNOTATIONS.copy(), {}, {}, {}, {}, {}, {}), ingresses=[ IngressItemSpec(host=None, pathmappings=[IngressPathMappingSpec(path="/", port=80)], annotations={}) ], @@ -367,6 +367,7 @@ class IngressTestcase: pod={}, status={}, pod_disruption_budget={}, + role_binding={}, ), annotations=LabelAndAnnotationSpec( deployment={}, @@ -377,6 +378,7 @@ class IngressTestcase: pod={}, status={}, pod_disruption_budget={}, + role_binding={}, ), ), ingress( diff --git a/tests/fiaas_deploy_daemon/deployer/kubernetes/test_ready_check.py b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_ready_check.py index fb699734..2bdddfbf 100644 --- a/tests/fiaas_deploy_daemon/deployer/kubernetes/test_ready_check.py +++ b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_ready_check.py @@ -152,7 +152,7 @@ def test_deployment_failed( config, ): if annotations: - app_spec = app_spec._replace(annotations=LabelAndAnnotationSpec(*[annotations] * 8)) + app_spec = app_spec._replace(annotations=LabelAndAnnotationSpec(*[annotations] * 9)) self._create_response(get, requested, replicas, available, updated) diff --git a/tests/fiaas_deploy_daemon/deployer/kubernetes/test_role_binding.py b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_role_binding.py new file mode 100644 index 00000000..6ddae655 --- /dev/null +++ b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_role_binding.py @@ -0,0 +1,158 @@ +import pytest +from unittest.mock import create_autospec, patch, call +from k8s.models.common import ObjectMeta, OwnerReference +from k8s.models.role_binding import RoleBinding +from fiaas_deploy_daemon.deployer.kubernetes.role_binding import RoleBindingDeployer +from fiaas_deploy_daemon.deployer.kubernetes.owner_references import OwnerReferences +from fiaas_deploy_daemon.config import Configuration + + +class TestRoleBindingDeployer: + + @pytest.fixture + def owner_references(self): + return create_autospec(OwnerReferences, spec_set=True, instance=True) + + @pytest.fixture + def deployer(self, owner_references): + config = create_autospec(Configuration([]), spec_set=True) + config.list_of_roles = ["test-role-1", "test-role-2"] + config.list_of_cluster_roles = ["cluster-role-1", "cluster-role-2"] + return RoleBindingDeployer(config, owner_references) + + @pytest.fixture + def cr_a_role_binding(self): + role_binding = create_autospec(RoleBinding, spec_set=True, instance=True) + role_binding.roleRef.kind = "ClusterRole" + role_binding.roleRef.name = "RoleA" + return role_binding + + @pytest.fixture + def r_a_role_binding(self): + role_binding = create_autospec(RoleBinding, spec_set=True, instance=True) + role_binding.roleRef.kind = "Role" + role_binding.roleRef.name = "RoleA" + return role_binding + + @pytest.mark.parametrize( + "roles_list,role_kind", + [ + (["test-role-1", "test-role-2"], "Role"), + (["cluster-role-1", "cluster-role-2"], "ClusterRole"), + ], + ) + def test_create_bindings_with_empty_role_bindings(self, deployer, owner_references, app_spec, roles_list, role_kind): + with patch.object(RoleBinding, 'save') as mock_save: + + deployer._update_or_create_role_bindings(app_spec, roles_list, role_kind, {}, {}, []) + + mock_save.assert_has_calls([ + call(), + call() + ]) + owner_references.apply.assert_called() + + def test_no_bindings_created_when_no_lists(self, deployer, owner_references, app_spec): + roles_list = [] + cluster_roles_list = [] + role_kind = "Role" + cluster_role_kind = "ClusterRole" + with patch.object(RoleBinding, 'save') as mock_try_to_save: + + deployer._update_or_create_role_bindings(app_spec, roles_list, role_kind, {}, {}, []) + deployer._update_or_create_role_bindings(app_spec, cluster_roles_list, cluster_role_kind, {}, {}, []) + + mock_try_to_save.assert_not_called() + owner_references.apply.assert_not_called() + + def test_deploy_full_flow(self, deployer, owner_references, app_spec): + roles_list = ["test-role-1", "test-role-2"] + cluster_roles_list = ["cluster-role-1", "cluster-role-2"] + role_kind = "Role" + with patch.object(RoleBinding, 'find') as mock_find, \ + patch.object(RoleBindingDeployer, '_update_or_create_role_bindings') as mock_create_role_bindings: + + mock_find.return_value = [] + + deployer.deploy(app_spec, {}) + + mock_find.assert_called() + mock_create_role_bindings.assert_has_calls([ + call(app_spec, roles_list, role_kind, {}, {}, []), + call(app_spec, cluster_roles_list, "ClusterRole", {}, {}, []) + ]) + + def test_create_bindings_with_role_in_role_bindings_not_owned_by_fiaas(self, deployer, owner_references, app_spec, r_a_role_binding): + + deployer._update_or_create_role_bindings(app_spec, ["RoleA"], "Role", {}, {}, [r_a_role_binding]) + + r_a_role_binding.save.assert_not_called() + owner_references.apply.assert_not_called() + + def test_create_bindings_with_role_in_role_bindings_owned_by_fiaas(self, deployer, owner_references, app_spec, r_a_role_binding): + r_a_role_binding.metadata = ObjectMeta(ownerReferences=[OwnerReference(apiVersion="fiaas.schibsted.io/v1", kind="Application")]) + + deployer._update_or_create_role_bindings(app_spec, ["RoleA"], "Role", {}, {}, [r_a_role_binding]) + + r_a_role_binding.save.assert_called() + owner_references.apply.assert_called() + + def test_create_bindings_with_role_in_role_bindings_owned_by_fiaas_as_cluster_role(self, deployer, owner_references, app_spec, + cr_a_role_binding): + with patch.object(RoleBinding, 'save') as mock_save: + cr_a_role_binding.metadata = ObjectMeta(ownerReferences=[ + OwnerReference(apiVersion="fiaas.schibsted.io/v1", kind="Application")]) + + deployer._update_or_create_role_bindings(app_spec, ["RoleA"], "Role", {}, {}, [cr_a_role_binding]) + + cr_a_role_binding.save.assert_not_called() + mock_save.assert_called() + owner_references.apply.assert_called() + + def test_create_bindings_with_multiple_cases(self, deployer, owner_references, app_spec, r_a_role_binding, cr_a_role_binding): + with patch.object(RoleBinding, 'save') as mock_save: + cr_a_role_binding.metadata = ObjectMeta(ownerReferences=[ + OwnerReference(apiVersion="fiaas.schibsted.io/v1", kind="Application")]) + r_a_role_binding.metadata = ObjectMeta(ownerReferences=[ + OwnerReference(apiVersion="fiaas.schibsted.io/v1", kind="Application")]) + role_binding = create_autospec(RoleBinding, spec_set=True, instance=True) + role_binding.roleRef.kind = "Role" + role_binding.roleRef.name = "RoleC" + + deployer._update_or_create_role_bindings(app_spec, ["RoleA", "RoleB", "RoleC"], "Role", {}, {}, + [cr_a_role_binding, r_a_role_binding, role_binding]) + + cr_a_role_binding.save.assert_not_called() + r_a_role_binding.save.assert_called() + role_binding.save.assert_not_called() + mock_save.assert_called() + owner_references.apply.assert_called() + + def clean_role_bindings_should_delete_roles_not_in_role_list(self, deployer): + r_role_binding = create_autospec(RoleBinding, spec_set=True, instance=True) + r_role_binding.roleRef.kind = "Role" + r_role_binding.roleRef.name = "test-role-1" + cr_role_binding = create_autospec(RoleBinding, spec_set=True, instance=True) + cr_role_binding.roleRef.kind = "ClusterRole" + cr_role_binding.roleRef.name = "test-role-1" + r2_role_binding = create_autospec(RoleBinding, spec_set=True, instance=True) + r2_role_binding.roleRef.kind = "Role" + r2_role_binding.roleRef.name = "test-role-3" + cr2_role_binding = create_autospec(RoleBinding, spec_set=True, instance=True) + cr2_role_binding.roleRef.kind = "ClusterRole" + cr2_role_binding.roleRef.name = "test-role-3" + cr2_role_binding.metadata = ObjectMeta(ownerReferences=[OwnerReference(apiVersion="fiaas.schibsted.io/v1", kind="Application")]) + + role_bindings = [r_role_binding, cr_role_binding, r2_role_binding, cr2_role_binding] + + deployer._clean_not_needed_role_bindings(role_bindings) + + assert len(role_bindings) == 2 + assert r_role_binding in role_bindings + assert cr_role_binding in role_bindings + assert r2_role_binding not in role_bindings + assert cr2_role_binding not in role_bindings + r_role_binding.delete.assert_not_called() + cr_role_binding.delete.assert_not_called() + r2_role_binding.delete.assert_not_called() + cr2_role_binding.delete.assert_called() diff --git a/tests/fiaas_deploy_daemon/deployer/kubernetes/test_service_deploy.py b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_service_deploy.py index ee52f3cb..74914910 100644 --- a/tests/fiaas_deploy_daemon/deployer/kubernetes/test_service_deploy.py +++ b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_service_deploy.py @@ -99,6 +99,7 @@ def test_deploy_new_service_with_custom_labels_and_annotations(self, deployer, s pod={}, status={}, pod_disruption_budget={}, + role_binding={}, ) annotations = LabelAndAnnotationSpec( deployment={}, @@ -109,6 +110,7 @@ def test_deploy_new_service_with_custom_labels_and_annotations(self, deployer, s pod={}, status={}, pod_disruption_budget={}, + role_binding={}, ) app_spec_custom_labels_and_annotations = app_spec._replace(labels=labels, annotations=annotations) diff --git a/tests/fiaas_deploy_daemon/deployer/test_deploy.py b/tests/fiaas_deploy_daemon/deployer/test_deploy.py index 303c8ffa..6323b4b3 100644 --- a/tests/fiaas_deploy_daemon/deployer/test_deploy.py +++ b/tests/fiaas_deploy_daemon/deployer/test_deploy.py @@ -90,7 +90,7 @@ def test_use_adapter_to_deploy(self, app_spec, deployer, adapter): ) def test_signals_start_of_deploy(self, app_spec, lifecycle, lifecycle_subject, deployer, annotations, repository): if annotations: - app_spec = app_spec._replace(annotations=LabelAndAnnotationSpec(*[annotations] * 8)) + app_spec = app_spec._replace(annotations=LabelAndAnnotationSpec(*[annotations] * 9)) deployer._queue = [DeployerEvent("UPDATE", app_spec, lifecycle_subject)] deployer() @@ -107,7 +107,7 @@ def test_signals_failure_on_exception( self, app_spec, lifecycle, lifecycle_subject, deployer, adapter, annotations, repository ): if annotations: - app_spec = app_spec._replace(annotations=LabelAndAnnotationSpec(*[annotations] * 8)) + app_spec = app_spec._replace(annotations=LabelAndAnnotationSpec(*[annotations] * 9)) deployer._queue = [DeployerEvent("UPDATE", app_spec, lifecycle_subject)] adapter.deploy.side_effect = Exception("message") diff --git a/tests/fiaas_deploy_daemon/specs/v3/test_v3factory.py b/tests/fiaas_deploy_daemon/specs/v3/test_v3factory.py index 7c50f399..8bf21757 100644 --- a/tests/fiaas_deploy_daemon/specs/v3/test_v3factory.py +++ b/tests/fiaas_deploy_daemon/specs/v3/test_v3factory.py @@ -450,6 +450,7 @@ def test_additional_labels_and_annotations(self, load_app_config_testdata, facto pod={"pod/label": "true", "s": "override"}, status={"status/label": "true"}, pod_disruption_budget={"pod-disruption-budget/label": "true"}, + role_binding={"role-binding/label": "true"}, ) additional_annotations = AdditionalLabelsOrAnnotations( _global={"global/annotation": "true"}, @@ -460,6 +461,7 @@ def test_additional_labels_and_annotations(self, load_app_config_testdata, facto pod={"pod/annotation": "true", "z": "override"}, status={"status/annotation": "true"}, pod_disruption_budget={"pod-disruption-budget/annotation": "true"}, + role_binding={"role-binding/annotation": "true"}, ) app_spec = factory( UID, diff --git a/tests/fiaas_deploy_daemon/usage_reporting/test_devhose_transformer.py b/tests/fiaas_deploy_daemon/usage_reporting/test_devhose_transformer.py index 5634d2dc..205fd675 100644 --- a/tests/fiaas_deploy_daemon/usage_reporting/test_devhose_transformer.py +++ b/tests/fiaas_deploy_daemon/usage_reporting/test_devhose_transformer.py @@ -279,7 +279,7 @@ def transformer(self, config): ) def test_transformation(self, transformer, app_spec, statuses, timestamps, expected, annotations): if annotations: - app_spec = app_spec._replace(annotations=LabelAndAnnotationSpec(*[annotations] * 8)) + app_spec = app_spec._replace(annotations=LabelAndAnnotationSpec(*[annotations] * 9)) with mock.patch("fiaas_deploy_daemon.usage_reporting.transformer._timestamp") as timestamp: timestamp.side_effect = timestamps for status in statuses: