Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(selector): select based on kork's RequestContext #1038

Merged
merged 4 commits into from
Jan 31, 2020
Merged

feat(selector): select based on kork's RequestContext #1038

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3fea9a7
feat(selector): select based on kork's RequestContext
dreynaud Jan 29, 2020
ef358e6
chore(context): RequestContext is now unused, delete it
dreynaud Jan 29, 2020
b731de1
Merge branch 'master' into clouddriver-select-properly
dreynaud Jan 29, 2020
09a98fb
fix(selector): fix typo and add log statement
dreynaud Jan 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .editorconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ insert_final_newline = true
trim_trailing_whitespace = true
indent_style = space
indent_size = 2
continuation_indent_size = 4
86 changes: 0 additions & 86 deletions gate-core/src/main/groovy/com/netflix/spinnaker/gate/security/RequestContext.java
View file
Open in desktop

This file was deleted.

21 changes: 16 additions & 5 deletions gate-web/src/main/groovy/com/netflix/spinnaker/gate/config/GateConfig.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ import com.netflix.spinnaker.gate.retrofit.Slf4jRetrofitLogger
import com.netflix.spinnaker.gate.services.EurekaLookupService
import com.netflix.spinnaker.gate.services.internal.*
import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService
import com.netflix.spinnaker.kork.web.context.AuthenticatedRequestContextProvider
import com.netflix.spinnaker.kork.web.context.RequestContextProvider
import com.netflix.spinnaker.kork.web.selector.DefaultServiceSelector
import com.netflix.spinnaker.kork.web.selector.SelectableService
import com.netflix.spinnaker.kork.web.selector.ServiceSelector
Expand Down Expand Up @@ -161,8 +163,13 @@ class GateConfig extends RedisHttpSessionConfiguration {
}

@Bean
OrcaServiceSelector orcaServiceSelector(OkHttpClient okHttpClient) {
return new OrcaServiceSelector(createClientSelector("orca", OrcaService, okHttpClient))
RequestContextProvider requestContextProvider() {
return new AuthenticatedRequestContextProvider();
}

@Bean
OrcaServiceSelector orcaServiceSelector(OkHttpClient okHttpClient, RequestContextProvider contextProvider) {
return new OrcaServiceSelector(createClientSelector("orca", OrcaService, okHttpClient), contextProvider)
}

@Bean
Expand Down Expand Up @@ -191,7 +198,9 @@ class GateConfig extends RedisHttpSessionConfiguration {
ClouddriverServiceSelector clouddriverServiceSelector(ClouddriverService defaultClouddriverService,
OkHttpClient okHttpClient,
DynamicConfigService dynamicConfigService,
DynamicRoutingConfigProperties properties) {
DynamicRoutingConfigProperties properties,
RequestContextProvider contextProvider
) {
if (serviceConfiguration.getService("clouddriver").getConfig().containsKey("dynamicEndpoints")) {
def endpoints = (Map<String, String>) serviceConfiguration.getService("clouddriver").getConfig().get("dynamicEndpoints")
// translates the following config:
Expand All @@ -217,11 +226,12 @@ class GateConfig extends RedisHttpSessionConfiguration {
selectors << new ByUserOriginSelector(service, 2, sourceApp)
}

return new ClouddriverServiceSelector(new SelectableService(selectors + defaultSelector), dynamicConfigService)
return new ClouddriverServiceSelector(
new SelectableService(selectors + defaultSelector), dynamicConfigService, contextProvider)
}

SelectableService selectableService = createClientSelector("clouddriver", ClouddriverService, okHttpClient)
return new ClouddriverServiceSelector(selectableService, dynamicConfigService)
return new ClouddriverServiceSelector(selectableService, dynamicConfigService, contextProvider)
}

//---- semi-optional components:
Expand Down Expand Up @@ -347,6 +357,7 @@ class GateConfig extends RedisHttpSessionConfiguration {

def selectorClass = it.config?.selectorClass as Class<ServiceSelector>
if (selectorClass) {
log.debug("Initializing selector class {} with baseUrl={}, priority={}, config={}", selectorClass, it.baseUrl, it.priority, it.config)
selector = selectorClass.getConstructors()[0].newInstance(
selector.service, it.priority, it.config
)
Expand Down
9 changes: 5 additions & 4 deletions gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/CapabilitiesController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,14 @@

package com.netflix.spinnaker.gate.controllers;

import com.netflix.spinnaker.gate.security.RequestContext;
import com.netflix.spinnaker.gate.services.internal.OrcaServiceSelector;
import io.swagger.annotations.ApiOperation;
import java.util.List;
import java.util.Map;
import lombok.RequiredArgsConstructor;
import org.springframework.web.bind.annotation.*;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

@RestController
@RequestMapping("/capabilities")
Expand All @@ -34,14 +35,14 @@ public class CapabilitiesController {
@ApiOperation(value = "Retrieve the list configured deployment monitors", response = List.class)
@GetMapping(value = "/deploymentMonitors")
List<Object> getDeploymentMonitors() {
return orcaService.withContext(RequestContext.get()).getDeploymentMonitors();
return orcaService.select().getDeploymentMonitors();
}

@ApiOperation(
value = "Retrieve the SpEL expression capabilities (e.g. registered functions, etc)",
response = Map.class)
@GetMapping(value = "/expressions")
Map getExpressionCapabilities() {
return orcaService.withContext(RequestContext.get()).getExpressionCapabilities();
return orcaService.select().getExpressionCapabilities();
}
}
5 changes: 1 addition & 4 deletions gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/ConcourseController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package com.netflix.spinnaker.gate.controllers;

import com.netflix.spinnaker.gate.security.RequestContext;
import com.netflix.spinnaker.gate.services.internal.IgorService;
import com.netflix.spinnaker.gate.services.internal.OrcaServiceSelector;
import io.swagger.annotations.ApiOperation;
Expand Down Expand Up @@ -81,8 +80,6 @@ void stageExecution(
@RequestParam("stageId") String stageId,
@RequestParam("job") String job,
@RequestParam("buildNumber") Integer buildNumber) {
orcaService
.withContext(RequestContext.get())
.concourseStageExecution(stageId, job, buildNumber, "");
orcaService.select().concourseStageExecution(stageId, job, buildNumber, "");
}
}
11 changes: 3 additions & 8 deletions gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/ExecutionsController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,13 @@
*/
package com.netflix.spinnaker.gate.controllers;

import com.netflix.spinnaker.gate.security.RequestContext;
import com.netflix.spinnaker.gate.services.internal.OrcaServiceSelector;
import io.swagger.annotations.ApiOperation;
import io.swagger.annotations.ApiParam;
import java.util.Collections;
import java.util.List;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.bind.annotation.*;

@RestController
public class ExecutionsController {
Expand Down Expand Up @@ -74,7 +69,7 @@ List getLatestExecutionsByConfigIds(
}

return orcaServiceSelector
.withContext(RequestContext.get())
.select()
.getSubsetOfExecutions(pipelineConfigIds, executionIds, limit, statuses, expand);
}

Expand Down Expand Up @@ -151,7 +146,7 @@ List searchForPipelineExecutionsByTrigger(
@RequestParam(value = "expand", defaultValue = "false")
boolean expand) {
return orcaServiceSelector
.withContext(RequestContext.get())
.select()
.searchForPipelineExecutionsByTrigger(
application,
triggerTypes,
Expand Down
4 changes: 2 additions & 2 deletions ...eb/src/main/groovy/com/netflix/spinnaker/gate/controllers/PipelineConfigController.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

package com.netflix.spinnaker.gate.controllers

import com.netflix.spinnaker.gate.security.RequestContext

import com.netflix.spinnaker.gate.services.commands.HystrixFactory
import com.netflix.spinnaker.gate.services.internal.Front50Service
import com.netflix.spinnaker.gate.services.internal.OrcaServiceSelector
Expand Down Expand Up @@ -67,7 +67,7 @@ class PipelineConfigController {
if (pipelineConfig == null) {
throw new NotFoundException("Pipeline config '${pipelineConfigId}' could not be found")
}
String template = orcaServiceSelector.withContext(RequestContext.get()).convertToPipelineTemplate(pipelineConfig).body.in().text
String template = orcaServiceSelector.select().convertToPipelineTemplate(pipelineConfig).body.in().text
return template
}
}
5 changes: 2 additions & 3 deletions gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/SubnetController.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ class SubnetController {

@ApiOperation(value = "Retrieve a list of subnets for a given cloud provider", response = List.class)
@RequestMapping(value = "/{cloudProvider}", method = RequestMethod.GET)
List<Map> allByCloudProvider(@PathVariable String cloudProvider,
@RequestHeader(value = "X-RateLimit-App", required = false) String sourceApp) {
clouddriverServiceSelector.select(sourceApp).getSubnets(cloudProvider)
List<Map> allByCloudProvider(@PathVariable String cloudProvider) {
clouddriverServiceSelector.select().getSubnets(cloudProvider)
}
}
2 changes: 1 addition & 1 deletion gate-web/src/main/groovy/com/netflix/spinnaker/gate/interceptors/RequestIdInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

package com.netflix.spinnaker.gate.interceptors;

import static com.netflix.spinnaker.security.AuthenticatedRequest.Header.REQUEST_ID;
import static com.netflix.spinnaker.kork.common.Header.REQUEST_ID;

import com.netflix.spinnaker.security.AuthenticatedRequest;
import javax.servlet.http.HttpServletRequest;
Expand Down
10 changes: 4 additions & 6 deletions gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/ArtifactService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,14 @@ private static HystrixCommand<Map<String, Object>> mapCommand(

public List<Map> getArtifactCredentials(String selectorKey) {
return mapListCommand(
"artifactCredentials",
clouddriverServiceSelector.select(selectorKey)::getArtifactCredentials)
"artifactCredentials", clouddriverServiceSelector.select()::getArtifactCredentials)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

selectorKey is no longer used (prob could go in a separate PR to clean those up..) (same in a few below)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes this PR was already on the big side, might do an opportunistic cleanup later

.execute();
}

public List<String> getArtifactNames(String selectorKey, String accountName, String type) {
return stringListCommand(
"artifactNames",
() ->
clouddriverServiceSelector.select(selectorKey).getArtifactNames(accountName, type))
() -> clouddriverServiceSelector.select().getArtifactNames(accountName, type))
.execute();
}

Expand All @@ -85,7 +83,7 @@ public List<String> getArtifactVersions(
"artifactVersions",
() ->
clouddriverServiceSelector
.select(selectorKey)
.select()
.getArtifactVersions(accountName, type, artifactName))
.execute();
}
Expand All @@ -96,7 +94,7 @@ public Void getArtifactContents(
"artifactContents",
() -> {
Response contentResponse =
clouddriverServiceSelector.select(selectorKey).getArtifactContent(artifact);
clouddriverServiceSelector.select().getArtifactContent(artifact);
IOUtils.copy(contentResponse.getBody().in(), outputStream);
return null;
})
Expand Down
4 changes: 2 additions & 2 deletions gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/CertificateService.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@ class CertificateService {

List<Map> getCertificates(String selectorKey) {
listCommand("certificates") {
clouddriverServiceSelector.select(selectorKey).getCertificates()
clouddriverServiceSelector.select().getCertificates()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so in places where we used to pass the selectorKey will the selection behavior change? do we care? (I assume the selector was always the app which is not in the authcontext)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the selector was the "user origin" app, which is in the auth context. You can get the same behavior as before with a config like this:

config:
  selectorClass: com.netflix.spinnaker.kork.web.selector.ByUserOriginSelector
  origin: deck

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and the clouddriverServiceSelector factory method does the translation, so you get opted into the new selector if you had the previous style of configuration:

dynamicEndpoints:
  deck: url

} execute()
}

List<Map> getCertificates(String cloudProvider, String selectorKey) {
listCommand("certificates-$cloudProvider") {
clouddriverServiceSelector.select(selectorKey).getCertificates(cloudProvider)
clouddriverServiceSelector.select().getCertificates(cloudProvider)
} execute()
}
}
4 changes: 2 additions & 2 deletions gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/CloudMetricService.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,14 @@ class CloudMetricService {

List<Map> findAll(String cloudProvider, String account, String region, Map<String, String> filters, String selectorKey) {
HystrixFactory.newListCommand(GROUP, "$GROUP:$account:$region:findAll") {
clouddriverServiceSelector.select(selectorKey).findAllCloudMetrics(cloudProvider, account, region, filters)
clouddriverServiceSelector.select().findAllCloudMetrics(cloudProvider, account, region, filters)
} execute()
}

Map getStatistics(String cloudProvider, String account, String region, String metricName,
Long startTime, Long endTime, Map<String, String> filters, String selectorKey) {
HystrixFactory.newMapCommand(GROUP, "$GROUP:$account:$region:getStatistics") {
clouddriverServiceSelector.select(selectorKey).getCloudMetricStatistics(cloudProvider, account, region, metricName, startTime, endTime, filters)
clouddriverServiceSelector.select().getCloudMetricStatistics(cloudProvider, account, region, metricName, startTime, endTime, filters)
} execute()
}
}
10 changes: 5 additions & 5 deletions gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/ClusterService.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,20 +40,20 @@ class ClusterService {

Map getClusters(String app, String selectorKey) {
HystrixFactory.newMapCommand(GROUP, "getClustersForApplication") {
clouddriverServiceSelector.select(selectorKey).getClusters(app)
clouddriverServiceSelector.select().getClusters(app)
} execute()
}

List<Map> getClustersForAccount(String app, String account, String selectorKey) {
HystrixFactory.newListCommand(GROUP, "getClustersForApplicationInAccount-${providerLookupService.providerForAccount(account)}") {
clouddriverServiceSelector.select(selectorKey).getClustersForAccount(app, account)
clouddriverServiceSelector.select().getClustersForAccount(app, account)
} execute()
}

Map getCluster(String app, String account, String clusterName, String selectorKey) {
HystrixFactory.newMapCommand(GROUP, "getCluster-${providerLookupService.providerForAccount(account)}") {
try {
clouddriverServiceSelector.select(selectorKey).getCluster(app, account, clusterName)?.getAt(0) as Map
clouddriverServiceSelector.select().getCluster(app, account, clusterName)?.getAt(0) as Map
} catch (RetrofitError e) {
if (e.response?.status == 404) {
return [:]
Expand All @@ -70,14 +70,14 @@ class ClusterService {

List<Map> getScalingActivities(String app, String account, String clusterName, String serverGroupName, String provider, String region, String selectorKey) {
HystrixFactory.newListCommand(GROUP, "getScalingActivitiesForCluster-${providerLookupService.providerForAccount(account)}") {
clouddriverServiceSelector.select(selectorKey).getScalingActivities(app, account, clusterName, provider, serverGroupName, region)
clouddriverServiceSelector.select().getScalingActivities(app, account, clusterName, provider, serverGroupName, region)
} execute()
}

Map getTargetServerGroup(String app, String account, String clusterName, String cloudProviderType, String scope, String target, Boolean onlyEnabled, Boolean validateOldest, String selectorKey) {
HystrixFactory.newMapCommand(GROUP, "getTargetServerGroup-${providerLookupService.providerForAccount(account)}") {
try {
return clouddriverServiceSelector.select(selectorKey).getTargetServerGroup(app, account, clusterName, cloudProviderType, scope, target, onlyEnabled, validateOldest)
return clouddriverServiceSelector.select().getTargetServerGroup(app, account, clusterName, cloudProviderType, scope, target, onlyEnabled, validateOldest)
} catch (RetrofitError re) {
if (re.kind == RetrofitError.Kind.HTTP && re.response?.status == 404) {
throw new ServerGroupNotFound("unable to find $target in $cloudProviderType/$account/$scope/$clusterName")
Expand Down
Loading