diff --git a/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/caching/agent/KubernetesCoreCachingAgent.java b/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/caching/agent/KubernetesCoreCachingAgent.java index e2ae22b7990..f811fbb1fd0 100644 --- a/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/caching/agent/KubernetesCoreCachingAgent.java +++ b/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/caching/agent/KubernetesCoreCachingAgent.java @@ -70,11 +70,9 @@ public Collection getProvidedDataTypes() { @Override protected List primaryKinds() { - synchronized (KubernetesKind.getValues()) { - return KubernetesKind.getValues().stream() - .filter(credentials::isValidKind) - .filter(k -> !k.isDynamic()) - .collect(Collectors.toList()); - } + return KubernetesKind.getRegisteredKinds().stream() + .filter(credentials::isValidKind) + .filter(k -> !k.isDynamic()) + .collect(Collectors.toList()); } } diff --git a/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/description/manifest/KubernetesKind.java b/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/description/manifest/KubernetesKind.java index d7844b94756..34de646690f 100644 --- a/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/description/manifest/KubernetesKind.java +++ b/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/description/manifest/KubernetesKind.java @@ -19,12 +19,8 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonValue; -import java.util.ArrayList; -import java.util.Collections; import java.util.List; -import java.util.Objects; import java.util.Optional; -import java.util.function.Predicate; import java.util.stream.Collectors; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -37,9 +33,7 @@ @EqualsAndHashCode(onlyExplicitlyIncluded = true) @Slf4j public final class KubernetesKind { - @Getter - private static final List values = - Collections.synchronizedList(new ArrayList<>()); + private static final KubernetesKindRegistry kindRegistry = new KubernetesKindRegistry(); public static final KubernetesKind API_SERVICE = createAndRegisterKind( @@ -126,12 +120,12 @@ public final class KubernetesKind { public static final KubernetesKind NONE = createAndRegisterKind("none", KubernetesApiGroup.NONE, null, true, false); - @Nonnull private final String name; + @Getter @Nonnull private final String name; @EqualsAndHashCode.Include @Nonnull private final String lcName; - @Nonnull private final KubernetesApiGroup apiGroup; + @Getter @Nonnull private final KubernetesApiGroup apiGroup; @EqualsAndHashCode.Include @Nullable private final KubernetesApiGroup customApiGroup; - @Nullable private final String alias; + @Getter @Nullable private final String alias; @Getter private final boolean isNamespaced; // generally reserved for workloads, can be read as "does this belong to a spinnaker cluster?" private final boolean hasClusterRelationship; @@ -146,11 +140,9 @@ private static KubernetesKind createAndRegisterKind( @Nullable String alias, boolean isNamespaced, boolean hasClusterRelationship) { - KubernetesKind kind = + return kindRegistry.registerKind( new KubernetesKind( - name, apiGroup, alias, isNamespaced, hasClusterRelationship, false, true); - values.add(kind); - return kind; + name, apiGroup, alias, isNamespaced, hasClusterRelationship, false, true)); } private KubernetesKind( @@ -211,9 +203,11 @@ public static KubernetesKind fromString(@Nonnull final String name) { return fromString(scopedKind.kindName, scopedKind.apiGroup); } + @Nonnull public static KubernetesKind fromString( @Nonnull final String name, @Nullable final KubernetesApiGroup apiGroup) { - return getRegisteredKind(name, apiGroup) + return kindRegistry + .getRegisteredKind(name, apiGroup) .orElseGet( () -> new KubernetesKind( @@ -226,70 +220,30 @@ public static KubernetesKind fromString( false)); } - private static Optional getRegisteredKind( - @Nonnull final String name, @Nullable final KubernetesApiGroup apiGroup) { - if (StringUtils.isEmpty(name)) { - return Optional.of(KubernetesKind.NONE); - } - - if (name.equalsIgnoreCase(KubernetesKind.NONE.toString())) { - throw new IllegalArgumentException("The 'NONE' kind cannot be read."); - } - - Predicate groupMatches = - kind -> { - // Exact match - if (Objects.equals(kind.apiGroup, apiGroup)) { - return true; - } - - // If we have not specified an API group, default to finding a native kind that matches - if (apiGroup == null || apiGroup.isNativeGroup()) { - return kind.apiGroup.isNativeGroup(); - } - - return false; - }; - - // separate from the above chain to avoid concurrent modification of the values list - return values.stream() - .filter( - v -> - v.name.equalsIgnoreCase(name) - || (v.alias != null && v.alias.equalsIgnoreCase(name))) - .filter(groupMatches) - .findAny(); - } - @Nonnull public static KubernetesKind getOrRegisterKind( @Nonnull final String name, final boolean registered, final boolean namespaced, @Nullable final KubernetesApiGroup apiGroup) { - synchronized (values) { - Optional kindOptional = getRegisteredKind(name, apiGroup); - // separate from the above chain to avoid concurrent modification of the values list - return kindOptional.orElseGet( - () -> { - log.info( - "Dynamically registering {}, (namespaced: {}, registered: {})", - name, - namespaced, - registered); - KubernetesKind kind = - new KubernetesKind( - name, - Optional.ofNullable(apiGroup).orElse(KubernetesApiGroup.NONE), - null, - namespaced, - false, - true, - registered); - values.add(kind); - return kind; - }); - } + return kindRegistry.getOrRegisterKind( + name, + apiGroup, + () -> { + log.info( + "Dynamically registering {}, (namespaced: {}, registered: {})", + name, + namespaced, + registered); + return new KubernetesKind( + name, + Optional.ofNullable(apiGroup).orElse(KubernetesApiGroup.NONE), + null, + namespaced, + false, + true, + registered); + }); } @Nonnull @@ -299,10 +253,16 @@ public static KubernetesKind getOrRegisterKind( return getOrRegisterKind(scopedKind.kindName, true, isNamespaced, scopedKind.apiGroup); } - public static List getOrRegisterKinds(List names) { + @Nonnull + public static List getOrRegisterKinds(@Nonnull List names) { return names.stream().map(k -> getOrRegisterKind(k, true)).collect(Collectors.toList()); } + @Nonnull + public static List getRegisteredKinds() { + return kindRegistry.getRegisteredKinds(); + } + @RequiredArgsConstructor private static class ScopedKind { public final String kindName; diff --git a/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/description/manifest/KubernetesKindRegistry.java b/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/description/manifest/KubernetesKindRegistry.java new file mode 100644 index 00000000000..ade10c699b4 --- /dev/null +++ b/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/description/manifest/KubernetesKindRegistry.java @@ -0,0 +1,103 @@ +/* + * Copyright 2019 Google, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License") + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.netflix.spinnaker.clouddriver.kubernetes.v2.description.manifest; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.function.Predicate; +import java.util.function.Supplier; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import org.apache.commons.lang3.StringUtils; + +public class KubernetesKindRegistry { + private final List values = Collections.synchronizedList(new ArrayList<>()); + + /** Registers a given {@link KubernetesKind} into the registry and returns the kind */ + @Nonnull + public KubernetesKind registerKind(@Nonnull KubernetesKind kind) { + values.add(kind); + return kind; + } + + /** + * Searches the registry for a {@link KubernetesKind} with the supplied name and apiGroup. If a + * kind is found, it is returned. If no kind is found, the provided {@link + * Supplier} is invoked and the resulting kind is registered. + * + *

This method is guaranteed to atomically check and register the kind. + */ + @Nonnull + public synchronized KubernetesKind getOrRegisterKind( + @Nonnull final String name, + @Nullable final KubernetesApiGroup apiGroup, + @Nonnull Supplier supplier) { + return getRegisteredKind(name, apiGroup).orElseGet(() -> registerKind(supplier.get())); + } + + /** + * Searches the registry for a {@link KubernetesKind} with the supplied name and apiGroup. Returns + * an {@link Optional} containing the kind, or an empty {@link Optional} if no + * kind is found. + * + *

Kinds whose API groups are different but are both is a native API groups (see {@link + * KubernetesApiGroup#isNativeGroup()}) are considered to match. + */ + @Nonnull + public Optional getRegisteredKind( + @Nonnull final String name, @Nullable final KubernetesApiGroup apiGroup) { + if (StringUtils.isEmpty(name)) { + return Optional.of(KubernetesKind.NONE); + } + + if (name.equalsIgnoreCase(KubernetesKind.NONE.toString())) { + throw new IllegalArgumentException("The 'NONE' kind cannot be read."); + } + + Predicate groupMatches = + kind -> { + // Exact match + if (Objects.equals(kind.getApiGroup(), apiGroup)) { + return true; + } + + // If we have not specified an API group, default to finding a native kind that matches + if (apiGroup == null || apiGroup.isNativeGroup()) { + return kind.getApiGroup().isNativeGroup(); + } + + return false; + }; + + return values.stream() + .filter( + v -> + v.getName().equalsIgnoreCase(name) + || (v.getAlias() != null && v.getAlias().equalsIgnoreCase(name))) + .filter(groupMatches) + .findAny(); + } + + /** Returns a list of all registered kinds */ + @Nonnull + public List getRegisteredKinds() { + return values; + } +} diff --git a/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/security/KubernetesV2Credentials.java b/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/security/KubernetesV2Credentials.java index 010cfa34aba..4a3874bf3cb 100644 --- a/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/security/KubernetesV2Credentials.java +++ b/clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/security/KubernetesV2Credentials.java @@ -334,7 +334,7 @@ private void determineOmitKinds() { // otherwise, checking all namespaces for all kinds is too expensive in large clusters (imagine // a cluster with 100s of namespaces). String checkNamespace = namespaces.get(0); - List allKinds = KubernetesKind.getValues(); + List allKinds = KubernetesKind.getRegisteredKinds(); log.info( "Checking permissions on configured kinds for account {}... {}", accountName, allKinds); diff --git a/clouddriver-kubernetes/src/test/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/description/KubernetesKindRegistrySpec.groovy b/clouddriver-kubernetes/src/test/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/description/KubernetesKindRegistrySpec.groovy new file mode 100644 index 00000000000..923e37fd778 --- /dev/null +++ b/clouddriver-kubernetes/src/test/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/description/KubernetesKindRegistrySpec.groovy @@ -0,0 +1,132 @@ +/* + * Copyright 2019 Google, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License") + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.netflix.spinnaker.clouddriver.kubernetes.v2.description + +import com.netflix.spinnaker.clouddriver.kubernetes.v2.description.manifest.KubernetesApiGroup +import com.netflix.spinnaker.clouddriver.kubernetes.v2.description.manifest.KubernetesKind +import com.netflix.spinnaker.clouddriver.kubernetes.v2.description.manifest.KubernetesKindRegistry +import spock.lang.Specification +import spock.lang.Subject +import spock.lang.Unroll + +class KubernetesKindRegistrySpec extends Specification { + static final KubernetesApiGroup CUSTOM_API_GROUP = KubernetesApiGroup.fromString("test") + static final KubernetesKind CUSTOM_KIND = KubernetesKind.fromString("customKind", CUSTOM_API_GROUP) + + @Unroll + void "kinds from core API groups are returned if any core API group is input"() { + given: + @Subject KubernetesKindRegistry kindRegistry = new KubernetesKindRegistry() + kindRegistry.registerKind(KubernetesKind.REPLICA_SET) + + when: + def kind = kindRegistry.getRegisteredKind(name, apiGroup) + + then: + result == kind.orElse(null) + + where: + name | apiGroup | result + "replicaSet" | null | KubernetesKind.REPLICA_SET + "replicaSet" | KubernetesApiGroup.APPS | KubernetesKind.REPLICA_SET + "replicaSet" | KubernetesApiGroup.EXTENSIONS | KubernetesKind.REPLICA_SET + "replicaSet" | CUSTOM_API_GROUP | null + "rs" | null | KubernetesKind.REPLICA_SET + "rs" | KubernetesApiGroup.APPS | KubernetesKind.REPLICA_SET + "replicaSet" | KubernetesApiGroup.EXTENSIONS | KubernetesKind.REPLICA_SET + "replicaSet" | CUSTOM_API_GROUP | null + + } + + @Unroll + void "getRegisteredKind returns kinds that have been registered"() { + given: + @Subject KubernetesKindRegistry kindRegistry = new KubernetesKindRegistry() + KubernetesKind result + + when: + result = kindRegistry.getRegisteredKind("customKind", CUSTOM_API_GROUP).orElse(null) + + then: + result == null + + when: + kindRegistry.registerKind(CUSTOM_KIND) + result = kindRegistry.getRegisteredKind("customKind", CUSTOM_API_GROUP).orElse(null) + + then: + result == CUSTOM_KIND + } + + @Unroll + void "getOrRegisterKind registers kinds that are not in the registry"() { + given: + @Subject KubernetesKindRegistry kindRegistry = new KubernetesKindRegistry() + KubernetesKind result + + when: + result = kindRegistry.getOrRegisterKind("customKind", CUSTOM_API_GROUP, {CUSTOM_KIND}) + + then: + result == CUSTOM_KIND + + when: + result = kindRegistry.getRegisteredKind("customKind", CUSTOM_API_GROUP).orElse(null) + + then: + result == CUSTOM_KIND + } + + @Unroll + void "getOrRegisterKind does not call the supplier for a kind that is already registered"() { + given: + @Subject KubernetesKindRegistry kindRegistry = new KubernetesKindRegistry() + kindRegistry.registerKind(CUSTOM_KIND) + + when: + def result = kindRegistry.getOrRegisterKind("customKind", CUSTOM_API_GROUP, { + throw new Exception("Should not have called supplier") + }) + + then: + result == CUSTOM_KIND + noExceptionThrown() + } + + @Unroll + void "getRegisteredKinds returns all kinds that are registered"() { + given: + @Subject KubernetesKindRegistry kindRegistry = new KubernetesKindRegistry() + Collection kinds + + when: + kinds = kindRegistry.getRegisteredKinds() + + then: + kinds.isEmpty() + + when: + kindRegistry.registerKind(KubernetesKind.REPLICA_SET) + kindRegistry.registerKind(CUSTOM_KIND) + kinds = kindRegistry.getRegisteredKinds() + + then: + kinds.size() == 2 + kinds.contains(KubernetesKind.REPLICA_SET) + kinds.contains(CUSTOM_KIND) + } +}