Skip to content

Commit

Permalink
refactor(aws/cloudformation): remove unnecessary hierarchy (#3673)
Browse files Browse the repository at this point in the history
This hierarchy was initially added to support different kinds of infra
as a service templates, but it became useless as the interface is a 1-1
match with its only implementation (Amazon). Just remove this for the
sake of clarity and ease reading.
  • Loading branch information
Xavi León authored and gardleopard committed May 31, 2019
1 parent 2be08a3 commit d86cb9d
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 110 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

package com.netflix.spinnaker.clouddriver.aws.controllers;

import com.netflix.spinnaker.clouddriver.aws.model.CloudFormationStack;
import com.netflix.spinnaker.clouddriver.aws.model.AmazonCloudFormationStack;
import com.netflix.spinnaker.clouddriver.aws.provider.view.AmazonCloudFormationProvider;
import com.netflix.spinnaker.kork.web.exceptions.NotFoundException;
import java.util.List;
Expand All @@ -43,15 +43,15 @@ public CloudFormationController(AmazonCloudFormationProvider cloudFormationProvi
}

@RequestMapping(method = RequestMethod.GET)
List<CloudFormationStack> list(
List<AmazonCloudFormationStack> list(
@RequestParam String accountName,
@RequestParam(required = false, defaultValue = "*") String region) {
log.debug("Cloud formation list stacks for account {}", accountName);
return cloudFormationProvider.list(accountName, region);
}

@RequestMapping(method = RequestMethod.GET, value = "/**")
CloudFormationStack get(HttpServletRequest request) {
AmazonCloudFormationStack get(HttpServletRequest request) {
String pattern = (String) request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE);
String stackId =
new AntPathMatcher().extractPathWithinPattern(pattern, request.getRequestURI());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import java.util.Map;

@JsonInclude(JsonInclude.Include.NON_EMPTY)
public class AmazonCloudFormationStack implements CloudFormationStack {
public class AmazonCloudFormationStack {
final String type = "aws";
private String stackId;
private Map<String, String> tags;
Expand All @@ -33,52 +33,42 @@ public class AmazonCloudFormationStack implements CloudFormationStack {
private String accountId;
private Date creationTime;

@Override
public String getStackId() {
return stackId;
}

@Override
public Map<String, String> getTags() {
return tags;
}

@Override
public Map<String, String> getOutputs() {
return outputs;
}

@Override
public String getStackName() {
return stackName;
}

@Override
public String getRegion() {
return region;
}

@Override
public String getAccountName() {
return accountName;
}

@Override
public String getAccountId() {
return accountId;
}

@Override
public String getStackStatus() {
return stackStatus;
}

@Override
public String getStackStatusReason() {
return stackStatusReason;
}

@Override
public Date getCreationTime() {
return creationTime;
}
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
import com.netflix.spinnaker.cats.cache.RelationshipCacheFilter;
import com.netflix.spinnaker.clouddriver.aws.cache.Keys;
import com.netflix.spinnaker.clouddriver.aws.model.AmazonCloudFormationStack;
import com.netflix.spinnaker.clouddriver.aws.model.CloudFormationProvider;
import com.netflix.spinnaker.clouddriver.aws.model.CloudFormationStack;
import java.util.Collection;
import java.util.List;
import java.util.Optional;
Expand All @@ -36,7 +34,7 @@

@Slf4j
@Component
public class AmazonCloudFormationProvider implements CloudFormationProvider<CloudFormationStack> {
public class AmazonCloudFormationProvider {

private final Cache cacheView;
private final ObjectMapper objectMapper;
Expand All @@ -48,20 +46,19 @@ public AmazonCloudFormationProvider(
this.objectMapper = objectMapper;
}

public List<CloudFormationStack> list(String accountName, String region) {
public List<AmazonCloudFormationStack> list(String accountName, String region) {
String filter = Keys.getCloudFormationKey("*", region, accountName);
log.debug("List all stacks with filter {}", filter);
return loadResults(cacheView.filterIdentifiers(STACKS.getNs(), filter));
}

@Override
public Optional<CloudFormationStack> get(String stackId) {
public Optional<AmazonCloudFormationStack> get(String stackId) {
String filter = Keys.getCloudFormationKey(stackId, "*", "*");
log.debug("Get stack with filter {}", filter);
return loadResults(cacheView.filterIdentifiers(STACKS.getNs(), filter)).stream().findFirst();
}

List<CloudFormationStack> loadResults(Collection<String> identifiers) {
List<AmazonCloudFormationStack> loadResults(Collection<String> identifiers) {
return cacheView.getAll(STACKS.getNs(), identifiers, RelationshipCacheFilter.none()).stream()
.map(
data -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@
*/
package com.netflix.spinnaker.clouddriver.aws.controllers

import com.fasterxml.jackson.annotation.JsonInclude
import com.netflix.spinnaker.clouddriver.aws.model.CloudFormationStack
import com.netflix.spinnaker.clouddriver.aws.model.AmazonCloudFormationStack
import com.netflix.spinnaker.clouddriver.aws.provider.view.AmazonCloudFormationProvider
import groovy.transform.Immutable
import org.springframework.test.web.servlet.MockMvc
import com.netflix.spinnaker.kork.web.exceptions.NotFoundException
import org.springframework.test.web.servlet.setup.MockMvcBuilders
Expand All @@ -43,7 +41,7 @@ class CloudFormationControllerSpec extends Specification {
def "request a list of stacks returns all the stacks for a given account (any region)"() {
given:
def accountName = 'aws-account-name'
cloudFormationProvider.list(accountName, '*') >> [ new CloudFormationStackTest(accountName: accountName) ]
cloudFormationProvider.list(accountName, '*') >> [ new AmazonCloudFormationStack(accountName: accountName) ]
when:
def results = mockMvc.perform(get("/aws/cloudFormation/stacks?accountName=$accountName"))
Expand All @@ -57,7 +55,7 @@ class CloudFormationControllerSpec extends Specification {
given:
def accountName = 'aws-account-name'
def region = 'region'
cloudFormationProvider.list(accountName, region) >> [ new CloudFormationStackTest(accountName: accountName, region: region) ]
cloudFormationProvider.list(accountName, region) >> [ new AmazonCloudFormationStack(accountName: accountName, region: region) ]
when:
def results = mockMvc.perform(get("/aws/cloudFormation/stacks?accountName=$accountName&region=$region"))
Expand All @@ -71,7 +69,7 @@ class CloudFormationControllerSpec extends Specification {
def "requesting a single stack by stackId"() {
given:
def stackId = "arn:cloudformation:stack/name"
cloudFormationProvider.get(stackId) >> Optional.of(new CloudFormationStackTest(stackId: stackId))
cloudFormationProvider.get(stackId) >> Optional.of(new AmazonCloudFormationStack(stackId: stackId))
when:
def results = mockMvc.perform(get("/aws/cloudFormation/stacks/$stackId"))
Expand All @@ -93,18 +91,4 @@ class CloudFormationControllerSpec extends Specification {
thrown(Exception) //loosened because we removed the dependency on spring data rest
}
@Immutable
@JsonInclude(JsonInclude.Include.NON_EMPTY)
class CloudFormationStackTest implements CloudFormationStack {
final String stackId
final Map<String, String> tags
final Map<String, String> outputs
final String stackName
final String region
final String stackStatus
final String stackStatusReason
final String accountName
final String accountId
final Date creationTime
}
}

0 comments on commit d86cb9d

Please sign in to comment.