Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DPE-2131][BUGFIX] lightkube rolebindnig broken #19

Merged
merged 3 commits into from
Jun 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 29 additions & 13 deletions spark8t/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,7 @@ def create(
"resourcename": resource_name,
"namespace": namespace,
}
| extra_args
juditnovak marked this conversation as resolved.
Show resolved Hide resolved
),
).__getitem__(0)
elif resource_type == KubernetesResourceType.ROLE:
Expand All @@ -530,6 +531,7 @@ def create(
"resourcename": resource_name,
"namespace": namespace,
}
| extra_args
),
).__getitem__(0)
elif resource_type == KubernetesResourceType.ROLEBINDING:
Expand All @@ -541,6 +543,7 @@ def create(
"resourcename": resource_name,
"namespace": namespace,
}
| extra_args
),
).__getitem__(0)
elif (
Expand Down Expand Up @@ -844,10 +847,15 @@ def create(
f"create {resource_type} {resource_name}", namespace=None, output="name"
)
else:
# NOTE: removing 'username' to avoid interference with KUBECONFIG
# ERROR: more than one authentication method found for admin; found [token basicAuth], only one is allowed
# See for similar:
# https://stackoverflow.com/questions/53783871/get-error-more-than-one-authentication-method-found-for-tier-two-user-found
formatted_extra_args = " ".join(
[
f"--{k}={v}"
for k, values in extra_args.items()
if k != "username"
for v in listify(values)
]
)
Expand Down Expand Up @@ -971,17 +979,17 @@ def delete(self, account_id: str) -> str:
pass

@abstractmethod
def set_primary(self, account_id: str) -> Optional[str]:
def set_primary(self, account_id: str, namespace: Optional[str]) -> Optional[str]:
"""Set the primary account to the one related to the provided account id.

Args:
account_id: account id to be elected as new primary account
"""
pass

def get_primary(self) -> Optional[ServiceAccount]:
def get_primary(self, namespace: Optional[str] = None) -> Optional[ServiceAccount]:
"""Return the primary service account. None is there is no primary service account."""
all_accounts = self.all()
all_accounts = self.all(namespace)

if len(all_accounts) == 0:
self.logger.warning("There are no service account available.")
Expand Down Expand Up @@ -1060,15 +1068,17 @@ def _build_service_account_from_raw(self, metadata: Dict[str, Any]):
extra_confs=self._retrieve_account_configurations(name, namespace),
)

def set_primary(self, account_id: str) -> Optional[str]:
def set_primary(
self, account_id: str, namespace: Optional[str] = None
) -> Optional[str]:
"""Set the primary account to the one related to the provided account id.

Args:
account_id: account id to be elected as new primary account
"""

# Relabeling primary
primary_account = self.get_primary()
primary_account = self.get_primary(namespace)

if primary_account is not None:
self.kube_interface.remove_label(
Expand Down Expand Up @@ -1110,13 +1120,17 @@ def create(self, service_account: ServiceAccount) -> str:
Args:
service_account: ServiceAccount to be stored in the registry
"""
rolename = service_account.name + "-role"
rolebindingname = service_account.name + "-role-binding"
username = service_account.name
serviceaccount = service_account.id

rolename = username + "-role"
rolebindingname = username + "-role-binding"

self.kube_interface.create(
KubernetesResourceType.SERVICEACCOUNT,
service_account.name,
username,
namespace=service_account.namespace,
**{"username": username},
)
self.kube_interface.create(
KubernetesResourceType.ROLE,
Expand All @@ -1131,7 +1145,9 @@ def create(self, service_account: ServiceAccount) -> str:
KubernetesResourceType.ROLEBINDING,
rolebindingname,
namespace=service_account.namespace,
**{"role": rolename, "serviceaccount": service_account.id},
role=rolename,
serviceaccount=serviceaccount,
username=username,
)

self.kube_interface.set_label(
Expand All @@ -1154,12 +1170,12 @@ def create(self, service_account: ServiceAccount) -> str:
)

if service_account.primary is True:
self.set_primary(service_account.id)
self.set_primary(serviceaccount, service_account.namespace)

if len(service_account.extra_confs) > 0:
self.set_configurations(service_account.id, service_account.extra_confs)
self.set_configurations(serviceaccount, service_account.extra_confs)

return service_account.id
return serviceaccount

def _create_account_configuration(self, service_account: ServiceAccount):
secret_name = self._get_secret_name(service_account.name)
Expand Down Expand Up @@ -1317,7 +1333,7 @@ def delete(self, account_id: str) -> str:
"""
return self.cache.pop(account_id).id

def set_primary(self, account_id: str) -> str:
def set_primary(self, account_id: str, namespace: Optional[str] = None) -> str:
"""Set the primary account to the one related to the provided account id.

Args:
Expand Down
61 changes: 44 additions & 17 deletions tests/integration/test_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,7 @@
"kubeinterface_name, kuberegistry_name",
[
("kubeinterface", "kube_registry"),
pytest.param(
"lightkubeinterface",
"lightkube_registry",
marks=pytest.mark.xfail(
reason="https://warthogs.atlassian.net/browse/DPE-2131"
),
),
("lightkubeinterface", "lightkube_registry"),
juditnovak marked this conversation as resolved.
Show resolved Hide resolved
],
)
@pytest.mark.parametrize(
Expand Down Expand Up @@ -57,13 +51,7 @@ def test_registry_io(kubeinterface_name, kuberegistry_name, namespace, user, req
"kubeinterface_name, kuberegistry_name",
[
("kubeinterface", "kube_registry"),
pytest.param(
"lightkubeinterface",
"lightkube_registry",
marks=pytest.mark.xfail(
reason="https://warthogs.atlassian.net/browse/DPE-2131"
),
),
("lightkubeinterface", "lightkube_registry"),
],
)
@pytest.mark.parametrize(
Expand Down Expand Up @@ -95,11 +83,50 @@ def test_registry_change_primary_account(
registry.create(sa1)
registry.create(sa2)

assert registry.get_primary().id == sa1.id
assert registry.get_primary(namespace).id == sa1.id

registry.set_primary(sa2.id)
registry.set_primary(sa2.id, namespace)

assert registry.get_primary().id == sa2.id
assert registry.get_primary(namespace).id == sa2.id


@pytest.mark.xfail(
reason="[BUG]: No namespace means ALL for KubeInterface, 'default' for LightKube..."
)
@pytest.mark.parametrize(
"kubeinterface_name, kuberegistry_name",
[
("kubeinterface", "kube_registry"),
("lightkubeinterface", "lightkube_registry"),
],
)
def test_registry_all(kubeinterface_name, kuberegistry_name, request):
kubeinterface = request.getfixturevalue(kubeinterface_name)
registry = request.getfixturevalue(kuberegistry_name)

kubeinterface.create(resource_type="namespace", resource_name="namespace-1")
kubeinterface.create(resource_type="namespace", resource_name="namespace-2")

sa1 = ServiceAccount(
"username-1",
"namespace-1",
kubeinterface.api_server,
primary=True,
extra_confs=PropertyFile({"k1": "v1"}),
)
sa2 = ServiceAccount(
"username-2",
"namespace-2",
kubeinterface.api_server,
primary=False,
extra_confs=PropertyFile({"k2": "v2"}),
)
registry.create(sa1)
registry.create(sa2)

assert len(registry.all("namespace-1")) == 1
assert len(registry.all("namespace-2")) == 1
assert len(registry.all()) == 2
juditnovak marked this conversation as resolved.
Show resolved Hide resolved


@pytest.mark.usefixtures("integration_test")
Expand Down
7 changes: 6 additions & 1 deletion tests/unittest/test_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -1448,6 +1448,7 @@ def test_k8s_registry_create(self, mock_kube_interface):
KubernetesResourceType.SERVICEACCOUNT,
name3,
namespace=namespace3,
username=name3,
)

mock_kube_interface.create.assert_any_call(
Expand All @@ -1464,7 +1465,11 @@ def test_k8s_registry_create(self, mock_kube_interface):
KubernetesResourceType.ROLEBINDING,
f"{name3}-role-binding",
namespace=namespace3,
**{"role": f"{name3}-role", "serviceaccount": sa3_obj.id},
**{
"role": f"{name3}-role",
"serviceaccount": sa3_obj.id,
"username": name3,
},
)

juditnovak marked this conversation as resolved.
Show resolved Hide resolved
mock_kube_interface.set_label.assert_any_call(
Expand Down
Loading