Skip to content

Commit

Permalink
fix(kubernetes): Fix handling of kinds not available on the server (#…
Browse files Browse the repository at this point in the history
…4010)

kubectl auth can-i returns 'yes' for kinds that you have permission
for, even if they don't exist on the cluster. The change to defer
kind checking removed filtering on the result of kubectl api-resources
(mistakenly thinking this filtering was only for performance).

This means that clouddriver is trying to cache unreadable kinds if
they are unreadable because they don't exist. Fix this by reverting
to the old behavior of just trying to call 'list' on a resource to
determine if it's readable. (This is probably not any more expensive
than doing an api-resources call every time we check if a kind is
readable and is simpler.  It also reproduces exactly what the caching
agent is doing so should avoid edge cases like the bug here.)
  • Loading branch information
ezimanyi committed Sep 6, 2019
1 parent 0c95915 commit b61fbfd
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -646,23 +646,20 @@ private boolean canReadKind(KubernetesKind kind) {
return true;
}
log.info("Checking if {} is readable in account '{}'...", kind, accountName);
boolean allowed;
if (kindRegistry.getRegisteredKind(kind).isNamespaced()) {
allowed =
jobExecutor.authCanINamespaced(
KubernetesV2Credentials.this, getCheckNamespace(), kind.getName(), "list");
} else {
allowed = jobExecutor.authCanI(KubernetesV2Credentials.this, kind.getName(), "list");
}

if (!allowed) {
try {
if (kindRegistry.getRegisteredKind(kind).isNamespaced()) {
list(kind, checkNamespace.get());
} else {
list(kind, null);
}
return true;
} catch (Exception e) {
log.info(
"Kind {} will not be cached in account '{}' because it cannot be listed.",
kind,
accountName);
return false;
}

return allowed;
}

private boolean checkMetricsReadable() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,11 @@ class KubernetesV2CredentialsSpec extends Specification {
checkPermissionsOnStartup: true,
)
)
kubectlJobExecutor.authCanINamespaced(_, _, "deployment", _) >> {
return false
kubectlJobExecutor.list(_ as KubernetesV2Credentials, [KubernetesKind.DEPLOYMENT], NAMESPACE, _ as KubernetesSelectorList) >> {
throw new KubectlJobExecutor.KubectlException("Error", new Exception())
}
kubectlJobExecutor.authCanINamespaced(_, _, "replicaSet", _) >> {
return true
kubectlJobExecutor.list(_ as KubernetesV2Credentials, [KubernetesKind.REPLICA_SET], NAMESPACE, _ as KubernetesSelectorList) >> {
return Collections.emptyList()
}

expect:
Expand Down

0 comments on commit b61fbfd

Please sign in to comment.