Skip to content

Commit

Permalink
feat(search): provide a way to turn off cats search provider and kube…
Browse files Browse the repository at this point in the history
…rnetes search provider (#5595)

* feat(search): provide a way to turn off cats search provider

* test(core): make sure that CatsSearchProviderTest works with spring 2.3

as mentioned here: https://github.com/spring-projects/spring-boot/wiki/Spring-Boot-2.3-Release-Notes#applicationcontextrunner-disables-bean-overriding-by-default,
tests should add: withAllowBeanDefinitionOverriding for backwards consistency

* feat(search): provide a way to turn off kubernetes search provider

* fix(search): ensure that search result==null is handled properly

    if any of the search providers return items.results as null, then we see:
    Ambiguous method overloading for method java.util.ArrayList#addAll.
     Cannot resolve which method to invoke for [null] due to overlapping prototypes between:
       [interface java.util.Collection]
       [interface java.lang.Iterable]
       [interface java.util.Iterator]
    Therefore, let's default to an empty [] in such cases.
  • Loading branch information
apoorvmahajan committed May 16, 2022
1 parent 9d3d51e commit 4096e61
Show file tree
Hide file tree
Showing 9 changed files with 208 additions and 1 deletion.
1 change: 1 addition & 0 deletions clouddriver-core/clouddriver-core.gradle
Expand Up @@ -61,5 +61,6 @@ dependencies {
testImplementation "org.junit.jupiter:junit-jupiter-engine"
testImplementation "org.junit.jupiter:junit-jupiter-params"
testImplementation "org.mockito:mockito-core"
testImplementation "org.springframework.boot:spring-boot-test"
testImplementation "org.springframework.boot:spring-boot-starter-test"
}
Expand Up @@ -103,6 +103,7 @@ class CacheConfig {
}

@Bean
@ConditionalOnProperty(value = "caching.search.enabled", matchIfMissing = true)
SearchProvider catsSearchProvider(CatsInMemorySearchProperties catsInMemorySearchProperties,
Cache cacheView,
List<SearchableProvider> providers,
Expand Down
Expand Up @@ -43,6 +43,7 @@ private static SearchResultSet empty(String query, Integer pageNumger, Integer p
.pageNumber(pageNumger)
.pageSize(pageSize)
.query(query)
.results(List.of())
.build();
}
}
@@ -0,0 +1,77 @@
/*
* Copyright 2021 Salesforce.com, 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.cache;

import static org.assertj.core.api.Assertions.assertThat;

import com.netflix.spectator.api.NoopRegistry;
import com.netflix.spectator.api.Registry;
import com.netflix.spinnaker.cats.cache.Cache;
import com.netflix.spinnaker.cats.mem.InMemoryCache;
import org.junit.jupiter.api.Test;
import org.springframework.boot.context.annotation.UserConfigurations;
import org.springframework.boot.test.context.runner.ApplicationContextRunner;
import org.springframework.context.annotation.Bean;

public class CatsSearchProviderTest {
private final ApplicationContextRunner runner =
new ApplicationContextRunner()
.withPropertyValues("caching.write-enabled=false", "redis.enabled:false")
.withConfiguration(UserConfigurations.of(CacheConfig.class, TestConfiguration.class))
.withAllowBeanDefinitionOverriding(true);

@Test
void testCatsSearchProviderBeanIsPresentByDefault() {
runner.run(ctx -> assertThat(ctx).hasSingleBean(CatsSearchProvider.class));
}

@Test
void testCatsSearchProviderBeanIsPresentWhenConfiguredInSuchAWay() {
runner
.withPropertyValues("caching.search.enabled=true")
.run(ctx -> assertThat(ctx).hasSingleBean(CatsSearchProvider.class));
}

@Test
void testCatsSearchProviderBeanIsNotPresentWhenConfiguredInSuchAWay() {
runner
.withPropertyValues("caching.search.enabled=false")
.run(ctx -> assertThat(ctx).doesNotHaveBean(CatsSearchProvider.class));
}

/**
* test class that supplies the minimum set of beans needed to autowire the CatsSearchProvider
* bean and other required beans in the CacheConfig class
*/
static class TestConfiguration {
@Bean
CatsInMemorySearchProperties catsInMemorySearchProperties() {
return new CatsInMemorySearchProperties();
}

@Bean
Cache cache() {
return new InMemoryCache();
}

@Bean
Registry registry() {
return new NoopRegistry();
}
}
}
1 change: 1 addition & 0 deletions clouddriver-kubernetes/clouddriver-kubernetes.gradle
Expand Up @@ -100,6 +100,7 @@ dependencies {
testImplementation "org.spockframework:spock-core"
testImplementation "org.spockframework:spock-spring"
testImplementation "org.springframework:spring-test"
testImplementation "org.springframework.boot:spring-boot-test"


integrationImplementation project(":clouddriver-web")
Expand Down
Expand Up @@ -48,9 +48,11 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.stereotype.Component;

@Component
@ConditionalOnProperty(value = "kubernetes.search.enabled", matchIfMissing = true)
public class KubernetesSearchProvider implements SearchProvider {
private static final Logger log = LoggerFactory.getLogger(KubernetesSearchProvider.class);
private final KubernetesCacheUtils cacheUtils;
Expand Down
@@ -0,0 +1,78 @@
/*
* Copyright 2021 Salesforce.com, 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.caching.view.provider;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.netflix.spinnaker.clouddriver.kubernetes.description.KubernetesSpinnakerKindMap;
import java.util.List;
import org.junit.jupiter.api.Test;
import org.springframework.boot.context.annotation.UserConfigurations;
import org.springframework.boot.test.context.runner.ApplicationContextRunner;
import org.springframework.context.annotation.Bean;

public class KubernetesSearchProviderTest {
private final ApplicationContextRunner runner =
new ApplicationContextRunner()
.withConfiguration(
UserConfigurations.of(KubernetesSearchProvider.class, TestConfiguration.class));

@Test
void testKubernetesSearchProviderBeanIsPresentByDefault() {
runner.run(ctx -> assertThat(ctx).hasSingleBean(KubernetesSearchProvider.class));
}

@Test
void testKubernetesSearchProviderBeanIsPresentWhenConfiguredInSuchAWay() {
runner
.withPropertyValues("kubernetes.search.enabled=true")
.run(ctx -> assertThat(ctx).hasSingleBean(KubernetesSearchProvider.class));
}

@Test
void testKubernetesSearchProviderBeanIsNotPresentWhenConfiguredInSuchAWay() {
runner
.withPropertyValues("kubernetes.search.enabled=false")
.run(ctx -> assertThat(ctx).doesNotHaveBean(KubernetesSearchProvider.class));
}

/** test class that supplies beans needed to autowire the KubernetesSearchProvider bean */
static class TestConfiguration {
@Bean
ObjectMapper getObjectMapper() {
return new ObjectMapper();
}

@Bean
KubernetesCacheUtils getKubernetesCacheUtils() {
return mock(KubernetesCacheUtils.class);
}

@Bean
KubernetesSpinnakerKindMap kubernetesSpinnakerKindMap() {
return new KubernetesSpinnakerKindMap(List.of());
}

@Bean
KubernetesAccountResolver kubernetesAccountResolver() {
return mock(KubernetesAccountResolver.class);
}
}
}
Expand Up @@ -87,7 +87,17 @@ class SearchController {
} else {

int total = results.inject(0) { acc, item -> acc + item.totalMatches }
List<Map<String, String>> allResults = results.inject([]) { acc, item -> acc.addAll(item.results); acc }
List<Map<String, String>> allResults = results.inject([]) { acc, item ->
// if any of the search providers return items.results as null, then we see
// Ambiguous method overloading for method java.util.ArrayList#addAll.
// Cannot resolve which method to invoke for [null] due to overlapping prototypes between:
// [interface java.util.Collection]
// [interface java.lang.Iterable]
// [interface java.util.Iterator]
// Therefore, let's default to an empty [] in such cases.
acc.addAll(item.results?: [])
acc
}

//TODO-cfieber: this is a temporary workaround to https://github.com/spinnaker/deck/issues/128
[new SearchResultSet(
Expand Down
Expand Up @@ -16,6 +16,10 @@

package com.netflix.spinnaker.clouddriver.controllers

import com.netflix.spinnaker.clouddriver.core.services.Front50Service
import com.netflix.spinnaker.clouddriver.search.ApplicationSearchProvider
import com.netflix.spinnaker.clouddriver.search.NoopSearchProvider
import com.netflix.spinnaker.clouddriver.search.ProjectSearchProvider
import com.netflix.spinnaker.clouddriver.search.SearchProvider
import com.netflix.spinnaker.clouddriver.search.SearchResultSet
import spock.lang.Specification
Expand Down Expand Up @@ -133,6 +137,38 @@ class SearchControllerSpec extends Specification {
searchResultSets == [resultSetA]
}

def 'search on type for which there is no search provider'() {
setup:
Front50Service front50Service = Mock(Front50Service)
ApplicationSearchProvider applicationSearchProvider = new ApplicationSearchProvider(front50Service)
ProjectSearchProvider projectSearchProvider = new ProjectSearchProvider(front50Service)
// none of the 3 search providers support the search type
searchController = new SearchController(searchProviders: [applicationSearchProvider, projectSearchProvider, new NoopSearchProvider()])
def filters = [
q: "aBC",
type: ['serverGroups'],
page: 1,
pageSize: 10
]

when:
List searchResultSets = getSearchResults(filters)

then:
1 * request.getParameterNames() >> enumeration

searchResultSets == [
new SearchResultSet(
totalMatches: 0,
pageNumber: 1,
pageSize: 10,
platform: 'aws',
query: 'aBC',
results: []
)
]
}

def "if only one search provider, don't aggregate into an aws result"() {
SearchResultSet rsA = Stub(SearchResultSet)
searchController = new SearchController(searchProviders: [searchProviderA])
Expand Down

0 comments on commit 4096e61

Please sign in to comment.