Skip to content

Commit

Permalink
feat(fiat): Suppress application details when updating permissions (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
kirangodishala committed Jun 9, 2023
1 parent 4eb895d commit 66a6483
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 0 deletions.
Expand Up @@ -16,6 +16,8 @@

package com.netflix.spinnaker.fiat.config;

import com.google.common.collect.Sets;
import java.util.Set;
import lombok.Data;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.boot.context.properties.NestedConfigurationProperty;
Expand All @@ -36,6 +38,17 @@ public class ResourceProviderConfig {
@Data
public static class ApplicationProviderConfig {
@NestedConfigurationProperty private ClouddriverConfig clouddriver = new ClouddriverConfig();

/** Controls whether the application "details" are stored with the User Permission or not */
private boolean suppressDetails;

/**
* Defines the set of keys to be excluded when suppressing application details Excluding
* chaosMonkey by default since {@link
* com.netflix.spinnaker.fiat.providers.ChaosMonkeyApplicationResourcePermissionSource} is the
* only place where these details are accessed.
*/
private Set<String> detailsExcludedFromSuppression = Sets.newHashSet("chaosMonkey");
}

@Data
Expand Down
Expand Up @@ -20,6 +20,7 @@
import static com.google.common.collect.ImmutableSet.toImmutableSet;

import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import com.google.common.collect.Streams;
import com.netflix.spinnaker.fiat.config.ResourceProviderConfig.ApplicationProviderConfig;
import com.netflix.spinnaker.fiat.model.resources.Application;
Expand All @@ -31,10 +32,13 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
import java.util.function.Predicate;
import lombok.extern.slf4j.Slf4j;

@Slf4j
public class DefaultApplicationResourceProvider extends BaseResourceProvider<Application>
implements ResourceProvider<Application> {

Expand Down Expand Up @@ -93,6 +97,12 @@ protected Set<Application> loadAll() throws ProviderException {
// Collect to a list instead of set since we're about to modify the applications
.collect(toImmutableList());

if (applicationProviderConfig.isSuppressDetails()) {
log.debug(
"Suppressing details for the fetched applications, excluding the following: {}",
applicationProviderConfig.getDetailsExcludedFromSuppression());
}

applications.forEach(
application -> {
Permissions permissions = permissionProvider.getPermissions(application);
Expand All @@ -102,6 +112,19 @@ protected Set<Application> loadAll() throws ProviderException {
executeFallbackPermissionsResolver.shouldResolve(permissions)
? executeFallbackPermissionsResolver.resolve(permissions)
: permissions);

if (applicationProviderConfig.isSuppressDetails()) {
// Suppress application details to reduce the amount of data being stored
Map<String, Object> updatedDetails = Maps.newHashMap(application.getDetails());
updatedDetails
.keySet()
.removeIf(
name ->
!applicationProviderConfig
.getDetailsExcludedFromSuppression()
.contains(name));
application.setDetails(updatedDetails);
}
});

if (allowAccessToUnknownApplications) {
Expand Down
Expand Up @@ -217,4 +217,67 @@ class DefaultApplicationProviderSpec extends Specification {
false | ["front50App1", "front50App2"]
true | ["front50App1", "front50App2", "clouddriverApp1", "clouddriverApp2"]
}

@Unroll
def "should suppress details when loading all applications"() {

setup:
def testApps = [
new Application().setName("front50App1")
.setDetails(HashMap.of("foo", "bar", "xyz", "pqr"))
.setPermissions(new Permissions.Builder().add(Authorization.READ, "role").build()),
new Application().setName("front50App2")
.setDetails(HashMap.of("foo", "bar", "xyz", "pqr"))
.setPermissions(new Permissions.Builder().add(Authorization.READ, "role").build())
]

Set<Application> expectedApps = [
new Application().setName("front50App1")
.setDetails(HashMap.of("foo", "bar", "xyz", "pqr"))
.setPermissions(new Permissions.Builder()
.add(Authorization.READ, "role")
.add(Authorization.EXECUTE, "role")
.build()),
new Application().setName("front50App2")
.setDetails(HashMap.of("foo", "bar", "xyz", "pqr"))
.setPermissions(new Permissions.Builder()
.add(Authorization.READ, "role")
.add(Authorization.EXECUTE, "role")
.build()),
] as Set

Front50Service front50Service = Mock(Front50Service) {
getAllApplications() >> testApps
}

ClouddriverService clouddriverService = Mock(ClouddriverService) {
getApplications() >> []
}

applicationProviderConfig.setSuppressDetails(suppressDetails)
applicationProviderConfig.setDetailsExcludedFromSuppression(excludeFromSupression)
provider = new DefaultApplicationResourceProvider(
front50Service,
clouddriverService,
defaultProvider,
fallbackPermissionsResolver,
true,
applicationProviderConfig,
)

when:
def result = provider.loadAll()

then:
result == expectedApps.each {
it.setDetails(expectedDetails)
}

where:
suppressDetails | excludeFromSupression | expectedDetails
false | [] as Set | [foo: "bar", xyz: "pqr"]
true | [] as Set | [:]
true | ["foo", "xyz"] as Set | [foo: "bar", xyz: "pqr"]
true | ["foo"] as Set | [foo: "bar"]
}
}

0 comments on commit 66a6483

Please sign in to comment.