Skip to content

Commit

Permalink
Merge branch 'master' into feat/excludeDisabledPipelines
Browse files Browse the repository at this point in the history
  • Loading branch information
christosarvanitis committed Dec 30, 2024
2 parents edd381b + 268236a commit 6dbf835
Show file tree
Hide file tree
Showing 15 changed files with 91 additions and 57 deletions.
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ plugins {
id 'io.spinnaker.project' version "$spinnakerGradleVersion" apply false
id "org.jetbrains.kotlin.jvm" version "$kotlinVersion" apply false
id "org.jetbrains.kotlin.plugin.allopen" version "$kotlinVersion" apply false

}

allprojects {
Expand Down
1 change: 1 addition & 0 deletions front50-core/front50-core.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,6 @@ dependencies {
// in front50-test to front50-common, but front50-test seems like a better place.
testImplementation project(":front50-test")
testImplementation "io.spinnaker.kork:kork-sql-test"
testImplementation "com.squareup.retrofit2:retrofit-mock"

}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.netflix.spinnaker.front50.model.application.ApplicationDAO;
import com.netflix.spinnaker.front50.model.application.ApplicationPermissionDAO;
import com.netflix.spinnaker.kork.exceptions.SystemException;
import com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall;
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException;
import com.netflix.spinnaker.kork.web.exceptions.NotFoundException;
import java.util.AbstractMap.SimpleEntry;
Expand Down Expand Up @@ -182,7 +183,7 @@ private void syncUsers(Permission newPermission, Permission oldPermission) {

if (fiatConfigurationProperties.getRoleSync().isEnabled()) {
try {
fiatService.get().sync(new ArrayList<>(roles));
Retrofit2SyncCall.execute(fiatService.get().sync(new ArrayList<>(roles)));
} catch (SpinnakerServerException e) {
log.warn("Error syncing users", e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.netflix.spinnaker.front50.config.annotations.ConditionalOnAnyProviderExceptRedisIsEnabled;
import com.netflix.spinnaker.front50.model.serviceaccount.ServiceAccount;
import com.netflix.spinnaker.front50.model.serviceaccount.ServiceAccountDAO;
import com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall;
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException;
import com.netflix.spinnaker.kork.web.exceptions.NotFoundException;
import java.util.Collection;
Expand Down Expand Up @@ -86,7 +87,8 @@ public void deleteServiceAccounts(Collection<ServiceAccount> serviceAccountsToDe
sa -> {
try {
serviceAccountDAO.delete(sa.getId());
fiatService.ifPresent(service -> service.logoutUser(sa.getId()));
fiatService.ifPresent(
service -> Retrofit2SyncCall.execute(service.logoutUser(sa.getId())));
} catch (Exception e) {
log.warn("Could not delete service account user {}", sa.getId(), e);
}
Expand Down Expand Up @@ -129,7 +131,7 @@ private void syncUsers(Collection<ServiceAccount> serviceAccounts) {
.flatMap(Collection::stream)
.distinct()
.collect(Collectors.toList());
fiatService.get().sync(rolesToSync);
Retrofit2SyncCall.execute(fiatService.get().sync(rolesToSync));
log.debug("Synced users with roles: {}", rolesToSync);
// Invalidate the current user's permissions in the local cache
Authentication auth = SecurityContextHolder.getContext().getAuthentication();
Expand All @@ -149,7 +151,10 @@ private void syncServiceAccount(ServiceAccount serviceAccount) {
return;
}
try {
fiatService.get().syncServiceAccount(serviceAccount.getId(), serviceAccount.getMemberOf());
Retrofit2SyncCall.execute(
fiatService
.get()
.syncServiceAccount(serviceAccount.getId(), serviceAccount.getMemberOf()));
log.debug(
"Synced service account {} with roles: {}",
serviceAccount.getId(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import com.netflix.spinnaker.front50.config.FiatConfigurationProperties
import com.netflix.spinnaker.front50.model.application.Application
import com.netflix.spinnaker.front50.model.application.ApplicationDAO
import com.netflix.spinnaker.front50.model.application.ApplicationPermissionDAO
import retrofit2.mock.Calls
import spock.lang.Specification
import spock.lang.Unroll

Expand All @@ -44,7 +45,7 @@ class ApplicationPermissionsServiceSpec extends Specification {
subject.createApplicationPermission(permission)

then:
1 * fiatService.sync(expectedSyncedRoles)
1 * fiatService.sync(expectedSyncedRoles) >> Calls.response(null)

where:
permission | expectedSyncedRoles
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import com.netflix.spinnaker.front50.model.serviceaccount.ServiceAccountDAO
import org.springframework.security.core.Authentication
import org.springframework.security.core.context.SecurityContext
import org.springframework.security.core.context.SecurityContextHolder
import retrofit2.mock.Calls
import spock.lang.Specification
import spock.lang.Subject

Expand Down Expand Up @@ -67,13 +68,14 @@ class ServiceAccountsServiceSpec extends Specification {
}
SecurityContextHolder.setContext(securityContext)
fiatConfigurationProperties.isDisableRoleSyncWhenSavingServiceAccounts() >> false

when:
serviceAccountDAO.create(serviceAccount.id, serviceAccount) >> serviceAccount
service.createServiceAccount(serviceAccount)

then:
1 * fiatPermissionsEvaluator.invalidatePermission(_)
1 * fiatService.sync(["test-role"])
1 * fiatService.sync(["test-role"]) >> Calls.response(null)
}

def "deleting multiple service account should call sync once"() {
Expand All @@ -92,13 +94,14 @@ class ServiceAccountsServiceSpec extends Specification {
]
)]
fiatConfigurationProperties.isDisableRoleSyncWhenSavingServiceAccounts() >> false

when:
service.deleteServiceAccounts(serviceAccounts)

then:
1 * serviceAccountDAO.delete("test-svc-acct-1")
1 * serviceAccountDAO.delete("test-svc-acct-2")
1 * fiatService.sync(['test-role-1', 'test-role-2'])
1 * fiatService.sync(['test-role-1', 'test-role-2']) >> Calls.response(null)
}

def "unknown managed service accounts should not throw exception"() {
Expand All @@ -118,6 +121,8 @@ class ServiceAccountsServiceSpec extends Specification {
1 * serviceAccountDAO.findById(test1ServiceAccount.id) >> test1ServiceAccount
1 * serviceAccountDAO.findById(test2ServiceAccount.id) >> { throw new NotFoundException(test2ServiceAccount.id) }
1 * serviceAccountDAO.delete(test1ServiceAccount.id)
1 * fiatService.logoutUser(_) >> Calls.response(null)
1 * fiatService.sync(_) >> Calls.response(1L)
0 * serviceAccountDAO.delete(test2ServiceAccount.id)
}

Expand All @@ -144,7 +149,7 @@ class ServiceAccountsServiceSpec extends Specification {

then:
1 * fiatPermissionsEvaluator.invalidatePermission(_)
1 * fiatService.syncServiceAccount("test-svc-acct", ["test-role"])
1 * fiatService.syncServiceAccount("test-svc-acct", ["test-role"]) >> Calls.response(1L)
0 * fiatService.sync(["test-role"])
}
}
3 changes: 2 additions & 1 deletion front50-web/front50-web.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,11 @@ dependencies {
implementation "io.spinnaker.fiat:fiat-core:$fiatVersion"
implementation "io.spinnaker.kork:kork-artifacts"
implementation "io.spinnaker.kork:kork-config"
implementation "io.spinnaker.kork:kork-retrofit"
implementation "io.spinnaker.kork:kork-web"
implementation "io.spinnaker.kork:kork-exceptions"
implementation "com.squareup.retrofit:converter-jackson"
implementation "io.swagger:swagger-annotations"
implementation "io.swagger.core.v3:swagger-annotations"
implementation "commons-codec:commons-codec"
implementation "javax.validation:validation-api"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@

import com.netflix.spinnaker.front50.model.AdminOperations;
import com.netflix.spinnaker.front50.model.ObjectType;
import io.swagger.annotations.Api;
import io.swagger.annotations.ApiOperation;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.tags.Tag;
import java.util.Collection;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.web.bind.annotation.RequestBody;
Expand All @@ -29,7 +29,7 @@

@RestController
@RequestMapping("/admin")
@Api(value = "admin", description = "Various administrative operations")
@Tag(name = "admin", description = "Various administrative operations")
public class AdminController {

private final Collection<AdminOperations> adminOperations;
Expand All @@ -39,7 +39,7 @@ public AdminController(Collection<AdminOperations> adminOperations) {
this.adminOperations = adminOperations;
}

@ApiOperation(value = "", notes = "Recover a previously deleted object")
@Operation(summary = "", description = "Recover a previously deleted object")
@RequestMapping(value = "/recover", method = RequestMethod.POST)
void recover(@RequestBody AdminOperations.Recover operation) {
adminOperations.forEach(o -> o.recover(operation));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import com.netflix.spinnaker.front50.model.delivery.Delivery;
import com.netflix.spinnaker.front50.model.delivery.DeliveryRepository;
import com.netflix.spinnaker.kork.web.exceptions.NotFoundException;
import io.swagger.annotations.ApiOperation;
import io.swagger.v3.oas.annotations.Operation;
import java.util.Collection;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression;
Expand All @@ -25,35 +25,35 @@ public DeliveryController(DeliveryRepository deliveryRepository) {
}

@PostFilter("hasPermission(filterObject.application, 'APPLICATION', 'READ')")
@ApiOperation(value = "", notes = "Get all delivery configs")
@Operation(summary = "", description = "Get all delivery configs")
@RequestMapping(method = RequestMethod.GET, value = "/deliveries")
Collection<Delivery> getAllConfigs() {
return deliveryRepository.getAllConfigs();
}

@PreAuthorize("hasPermission(#application, 'APPLICATION', 'READ')")
@ApiOperation(value = "", notes = "Get the delivery configs for an application")
@Operation(summary = "", description = "Get the delivery configs for an application")
@RequestMapping(method = RequestMethod.GET, value = "/applications/{application}/deliveries")
Collection<Delivery> getConfigByAppName(@PathVariable String application) {
return deliveryRepository.getConfigsByApplication(application);
}

@PostAuthorize("hasPermission(returnObject.application, 'APPLICATION', 'READ')")
@ApiOperation(value = "", notes = "Get a delivery config by id")
@Operation(summary = "", description = "Get a delivery config by id")
@RequestMapping(method = RequestMethod.GET, value = "deliveries/{id}")
Delivery getConfigById(@PathVariable String id) {
return deliveryRepository.findById(id);
}

@PreAuthorize("hasPermission(#config.application, 'APPLICATION', 'WRITE')")
@ApiOperation(value = "", notes = "Create a delivery config")
@Operation(summary = "", description = "Create a delivery config")
@RequestMapping(method = RequestMethod.POST, value = "/deliveries")
Delivery createConfig(@RequestBody Delivery config) {
return deliveryRepository.upsertConfig(config);
}

@PreAuthorize("hasPermission(#config.application, 'APPLICATION', 'WRITE')")
@ApiOperation(value = "", notes = "Update a delivery config")
@Operation(summary = "", description = "Update a delivery config")
@RequestMapping(method = RequestMethod.PUT, value = "/deliveries/{id}")
Delivery upsertConfig(@PathVariable String id, @RequestBody Delivery config) {
if (!id.equals(config.getId())) {
Expand All @@ -71,7 +71,7 @@ Delivery upsertConfig(@PathVariable String id, @RequestBody Delivery config) {
}

@PreAuthorize("hasPermission(#application, 'APPLICATION', 'WRITE')")
@ApiOperation(value = "", notes = "Delete a delivery config")
@Operation(summary = "", description = "Delete a delivery config")
@RequestMapping(
method = RequestMethod.DELETE,
value = "/applications/{application}/deliveries/{id}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import com.netflix.spinnaker.front50.ApplicationPermissionsService;
import com.netflix.spinnaker.front50.model.application.Application;
import io.swagger.annotations.ApiOperation;
import io.swagger.v3.oas.annotations.Operation;
import java.util.Set;
import org.springframework.web.bind.annotation.*;

Expand All @@ -16,7 +16,7 @@ public PermissionsController(ApplicationPermissionsService permissionsService) {
this.permissionsService = permissionsService;
}

@ApiOperation(value = "", notes = "Get all application permissions. Internal use only.")
@Operation(summary = "", description = "Get all application permissions. Internal use only.")
@RequestMapping(method = RequestMethod.GET, value = "/applications")
public Set<Application.Permission> getAllApplicationPermissions() {
return permissionsService.getAllApplicationPermissions();
Expand All @@ -27,7 +27,7 @@ public Application.Permission getApplicationPermission(@PathVariable String appN
return permissionsService.getApplicationPermission(appName);
}

@ApiOperation(value = "", notes = "Create an application permission.")
@Operation(summary = "", description = "Create an application permission.")
@RequestMapping(method = RequestMethod.POST, value = "/applications")
public Application.Permission createApplicationPermission(
@RequestBody Application.Permission newPermission) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,16 +203,25 @@ public List<Pipeline> listByApplication(
@PathVariable(value = "application") String application,
@RequestParam(value = "pipelineNameFilter", required = false) String pipelineNameFilter,
@RequestParam(required = false, value = "refresh", defaultValue = "true") boolean refresh,
@RequestParam(required = false, value = "enabledPipelines", defaultValue = "false")
Boolean enabledPipelines) {
@RequestParam(required = false, value = "pipelineStateFilter", defaultValue = "all")
String pipelineStateFilter) {
List<Pipeline> pipelines =
new ArrayList<>(
pipelineDAO.getPipelinesByApplication(application, pipelineNameFilter, refresh));

if (!enabledPipelines) {
if (pipelineStateFilter.isEmpty() || pipelineStateFilter.equalsIgnoreCase("all")) {
return sortPipelines(pipelines);
}

Boolean enabledPipelines;
if (pipelineStateFilter.equalsIgnoreCase("enabled")) {
enabledPipelines = true;
} else if (pipelineStateFilter.equalsIgnoreCase("disabled")) {
enabledPipelines = false;
} else {
enabledPipelines = null;
}

Predicate<Pipeline> pipelinePredicate =
pipeline -> {
// pipeline.getDisabled may be null, so check that before comparing. If
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@
import com.netflix.spinnaker.front50.model.application.ApplicationDAO;
import com.netflix.spinnaker.front50.model.application.ApplicationPermissionDAO;
import com.netflix.spinnaker.front50.model.application.ApplicationService;
import com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall;
import com.netflix.spinnaker.kork.web.exceptions.NotFoundException;
import io.swagger.annotations.Api;
import io.swagger.annotations.ApiOperation;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.tags.Tag;
import java.util.*;
import java.util.stream.Collectors;
import javax.servlet.http.HttpServletResponse;
Expand All @@ -30,7 +31,7 @@

@RestController
@RequestMapping("/v2/applications")
@Api(value = "application", description = "Application API")
@Tag(name = "application", description = "Application API")
public class ApplicationsController {

private static final Logger log = LoggerFactory.getLogger(ApplicationsController.class);
Expand Down Expand Up @@ -62,9 +63,9 @@ public ApplicationsController(

@PreAuthorize("#restricted ? @fiatPermissionEvaluator.storeWholePermission() : true")
@PostFilter("#restricted ? hasPermission(filterObject.name, 'APPLICATION', 'READ') : true")
@ApiOperation(
value = "",
notes =
@Operation(
summary = "",
description =
"Fetch all applications.\n\nSupports filtering by one or more attributes:\n- [email protected]\n- [email protected]&name=flex")
@RequestMapping(method = RequestMethod.GET)
public List<Application> applications(
Expand Down Expand Up @@ -113,7 +114,7 @@ public List<Application> applications(
}

@PreAuthorize("@fiatPermissionEvaluator.canCreate('APPLICATION', #app)")
@ApiOperation(value = "", notes = "Create an application")
@Operation(summary = "", description = "Create an application")
@RequestMapping(method = RequestMethod.POST)
public Application create(@RequestBody final Application app) {
if (applicationService.findByName(app.getName()) != null) {
Expand All @@ -125,7 +126,7 @@ public Application create(@RequestBody final Application app) {
&& fiatConfigurationProperties.getRoleSync().isEnabled()
&& fiatService.isPresent()) {
try {
fiatService.get().sync();
Retrofit2SyncCall.execute(fiatService.get().sync());
} catch (Exception e) {
log.warn("failed to trigger fiat permission sync", e);
}
Expand All @@ -135,15 +136,15 @@ public Application create(@RequestBody final Application app) {
}

@PreAuthorize("hasPermission(#applicationName, 'APPLICATION', 'WRITE')")
@ApiOperation(value = "", notes = "Delete an application")
@Operation(summary = "", description = "Delete an application")
@RequestMapping(method = RequestMethod.DELETE, value = "/{applicationName:.+}")
public void delete(@PathVariable String applicationName, HttpServletResponse response) {
applicationService.delete(applicationName);
response.setStatus(HttpStatus.NO_CONTENT.value());
}

@PreAuthorize("hasPermission(#app.name, 'APPLICATION', 'WRITE')")
@ApiOperation(value = "", notes = "Update an existing application by merging the attributes")
@Operation(summary = "", description = "Update an existing application by merging the attributes")
@RequestMapping(method = RequestMethod.PATCH, value = "/{applicationName:.+}")
public Application update(
@PathVariable final String applicationName, @RequestBody final Application app) {
Expand All @@ -158,7 +159,9 @@ public Application update(
}

@PreAuthorize("hasPermission(#app.name, 'APPLICATION', 'WRITE')")
@ApiOperation(value = "", notes = "Update an existing application by replacing all attributes")
@Operation(
summary = "",
description = "Update an existing application by replacing all attributes")
@RequestMapping(method = RequestMethod.PUT, value = "/{applicationName:.+}")
public Application replace(
@PathVariable final String applicationName, @RequestBody final Application app) {
Expand All @@ -173,7 +176,7 @@ public Application replace(
}

@PostAuthorize("hasPermission(#applicationName, 'APPLICATION', 'READ')")
@ApiOperation(value = "", notes = "Fetch a single application by name")
@Operation(summary = "", description = "Fetch a single application by name")
@RequestMapping(method = RequestMethod.GET, value = "/{applicationName:.+}")
public Application get(@PathVariable final String applicationName) {
Application app = applicationDAO.findByName(applicationName.toUpperCase());
Expand Down
Loading

0 comments on commit 6dbf835

Please sign in to comment.