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.

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

feat(fiat): Suppress application details when updating permissions #1060

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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"]
}
}