Skip to content

Commit

Permalink
fix(kubernetes): Allow namespaces that don't exist in validation (#3748)
Browse files Browse the repository at this point in the history
In 1.14, we tried to improve the failure mode for users with
misconfigured accounts by using the actual live list of namespaces
while validating accounts.

This fails when users are creating a namespace and deploying something
to the namespace as part of the same operation; in that case the live
list does not know about the namespace (but will by the time the
deployment completes).

This reverts #3639 (and adds a comment and test
to make sure this subtle bug is not re-introduced).
  • Loading branch information
ezimanyi authored and maggieneterval committed Jun 5, 2019
1 parent 10f5a6f commit ab80b0f
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public class KubernetesV2Credentials implements KubernetesCredentials {
private final Clock clock;
private final KubectlJobExecutor jobExecutor;

@Getter private final String accountName;
private final String accountName;
@Getter private final List<String> namespaces;
@Getter private final List<String> omitNamespaces;
private final List<KubernetesKind> kinds;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,13 +139,17 @@ private boolean validateKind(KubernetesKind kind, KubernetesV2Credentials creden
}

protected boolean validateNamespace(String namespace, KubernetesV2Credentials credentials) {
final List<String> configuredNamespaces = credentials.getDeclaredNamespaces();
if (!configuredNamespaces.contains(namespace)) {
reject(
String.format(
"Account %s is not configured to deploy to namespace %s",
credentials.getAccountName(), namespace),
namespace);
final List<String> configuredNamespaces = credentials.getNamespaces();
if (configuredNamespaces != null
&& !configuredNamespaces.isEmpty()
&& !configuredNamespaces.contains(namespace)) {
reject("wrongNamespace", namespace);
return false;
}

final List<String> omitNamespaces = credentials.getOmitNamespaces();
if (omitNamespaces != null && omitNamespaces.contains(namespace)) {
reject("omittedNamespace", namespace);
return false;
}
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,13 @@ import spock.lang.Specification
import spock.lang.Unroll

class KubernetesValidationUtilSpec extends Specification {
def namespaces = ["test-namespace"]

@Unroll
void "wiring of kind/namespace validation"() {
given:
Errors errors = Mock(Errors)
String kubernetesAccount = "testAccount"
def namespaces = ["test-namespace"]
def omitNamespaces = ["omit-namespace"]
def kind = KubernetesKind.DEPLOYMENT
AccountCredentials accountCredentials = Mock(AccountCredentials)
KubernetesV2Credentials credentials = Mock(KubernetesV2Credentials)
Expand All @@ -48,7 +47,8 @@ class KubernetesValidationUtilSpec extends Specification {
then:
accountCredentialsProvider.getCredentials(kubernetesAccount) >> accountCredentials
accountCredentials.getCredentials() >> credentials
credentials.getDeclaredNamespaces() >> namespaces
credentials.getOmitNamespaces() >> omitNamespaces
credentials.namespaces >> namespaces
manifest.getNamespace() >> testNamespace
manifest.getKind() >> kind
credentials.isValidKind(kind) >> true
Expand All @@ -74,15 +74,23 @@ class KubernetesValidationUtilSpec extends Specification {
def judgement = kubernetesValidationUtil.validateNamespace(testNamespace, credentials)

then:
credentials.getDeclaredNamespaces() >> namespaces
credentials.getOmitNamespaces() >> omitNamespaces
credentials.namespaces >> namespaces
judgement == allowedNamespace

where:
namespaces | omitNamespaces | testNamespace || allowedNamespace
["test-namespace"] | ["omit-namespace"] | "test-namespace" || true
null | ["omit-namespace"] | "test-namespace" || true
["test-namespace"] | null | "test-namespace" || true
["test-namespace"] | ["omit-namespace"] | "omit-namespace" || false
null | ["omit-namespace"] | "omit-namespace" || false
["test-namespace"] | ["omit-namespace"] | "unknown-namespace" || false
[] | [] | "unknown-namespace" || false
null | null | "unknown-namespace" || true
// When namespaces is not specified (and we rely on dynamic discovery) we need to treat an unknown namespace as
// allowed. This is because we might be adding the namespace as part of the same deploy operation, so can't rely
// on looking in the namespace cache for the unknown namespace.
[] | [] | "unknown-namespace" || true
[] | ["omit-namespace"] | "unknown-namespace" || true
}
}

0 comments on commit ab80b0f

Please sign in to comment.