Skip to content

Commit

Permalink
fix(kubernetes): Use consistent casing for built-in kinds (#3989)
Browse files Browse the repository at this point in the history
* fix(kubernetes): Revert consistent casing for kind

The prior commit to always serialize kinds as lowercase
is still something we should do longer-term to reduce the
fragility of code that is depending on particular casings
(ex: replicaSet).

That being said, it turns out there are more places depending
on the current casing than I initially thought. In particular,
the logic to substitute parts of a kubernetes manifest (ex:
to replace a config map with the fully-qualified config map
with a version) depends on the kind being camelCased.

As we're approaching 1.16, I think that this is a more invasive
change that I'd like to do right now, and would also like to
decouple that change from the more important work to refactor
Kubernetes kinds.  So this commit rolls back the change that
always converts kinds to lower-case; the following commit
fixes the issue with the refactor a different (and less
invasive way).

* fix(kubernetes): Use consistent casing for built-in kinds

A lot of code depends on case-sensitive comparisons between
kinds; the recent refactor removed the global kind registry
so when Spinnaker encounters ReplicaSet it is leaving it cased
as ReplicaSet instead of (as before) converting it to replicaSet.

This change builds a small collection of the built-in kinds and
handles that conversion to Spinnaker-canonical casing when we're
reading kinds. This is only done for built-in kinds as these are
the only ones where we have built-in logic that depends on the case.

A later PR (after 1.16) will re-implement always converting to lower
case and will fix places where we're incorrectly doing case-sensitive
comparisons.
  • Loading branch information
ezimanyi authored and plumpy committed Aug 28, 2019
1 parent deb7d43 commit 9526c10
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ private KubernetesKind(@Nonnull String name, @Nullable KubernetesApiGroup apiGro
private static KubernetesKind createWithAlias(
@Nonnull String name, @Nullable String alias, @Nullable KubernetesApiGroup apiGroup) {
KubernetesKind kind = new KubernetesKind(name, apiGroup);
aliasMap.put(kind, kind);
if (alias != null) {
aliasMap.put(new KubernetesKind(alias, apiGroup), kind);
}
Expand Down Expand Up @@ -147,8 +148,8 @@ public static KubernetesKind fromString(@Nonnull String qualifiedKind) {
@Override
public String toString() {
if (apiGroup.isNativeGroup()) {
return lcName;
return name;
}
return lcName + "." + apiGroup.toString();
return name + "." + apiGroup.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class KeysSpec extends Specification {

where:
kind | apiVersion | account | namespace | name || key
KubernetesKind.REPLICA_SET | KubernetesApiVersion.EXTENSIONS_V1BETA1 | "ac" | "namespace" | "v1-v000" || "kubernetes.v2:infrastructure:replicaset:ac:namespace:v1-v000"
KubernetesKind.REPLICA_SET | KubernetesApiVersion.EXTENSIONS_V1BETA1 | "ac" | "namespace" | "v1-v000" || "kubernetes.v2:infrastructure:replicaSet:ac:namespace:v1-v000"
KubernetesKind.SERVICE | KubernetesApiVersion.V1 | "ac" | "namespace" | "v1" || "kubernetes.v2:infrastructure:service:ac:namespace:v1"
KubernetesKind.DEPLOYMENT | KubernetesApiVersion.APPS_V1BETA1 | "ac" | "namespace" | "v1" || "kubernetes.v2:infrastructure:deployment:ac:namespace:v1"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class KubernetesCacheDataSpec extends Specification {
def application = cacheData.stream().filter({cd -> cd.id == APPLICATION_KEY.toString()}).findFirst().get()
// Ensure that the default "name" key was added to the logical key
application.attributes.get("name") == "app"
def applicationRelationships = application.relationships.get("replicaset") as Collection<String>
def applicationRelationships = application.relationships.get("replicaSet") as Collection<String>
applicationRelationships.size() == 1
applicationRelationships.contains(REPLICA_SET_KEY.toString())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,23 @@ class KubernetesKindSpec extends Specification {
"networkpolicy" | KubernetesKind.NETWORK_POLICY
}

@Unroll
void "kinds are serialized using the Spinnaker-canonical form"() {
when:
def kind = KubernetesKind.fromString(name)

then:
kind.toString().equals("replicaSet")

where:
name << [
"replicaSet",
"replicaset",
"ReplicaSet",
"REPLICASET",
]
}

@Unroll
void "kinds from core API groups are returned if any core API group is input"() {
when:
Expand Down Expand Up @@ -118,8 +135,8 @@ class KubernetesKindSpec extends Specification {

where:
name | expectedString
"replicaSet" | "replicaset"
"replicaSet.apps" | "replicaset"
"replicaSet" | "replicaSet"
"replicaSet.apps" | "replicaSet"
"deployment.extensions" | "deployment"
}

Expand Down

0 comments on commit 9526c10

Please sign in to comment.