Skip to content

Commit

Permalink
upgrades fiat to boot2 and new BOM dependency management (#388)
Browse files Browse the repository at this point in the history 
* refactor(boot2): Mostly working... one transient test failure

* chore(boot2): Bump to spin-deps rc.17, add spring properties migrator

* chore(boot2): spin-dep rc.18

* fix(boot2): spin-dep rc.21; fixed failing test

* fix(boot2): Fix transient test?

* chore(boot2): Why cant I get this transient failure to happen on my laptop

* fix(boot2): Im sure this will not solve it

* chore(boot2): Cleanup of some speculative test output

* chore(boot2): Disable parallel gradle execution

* refactor(boot2): Mostly working... one transient test failure

* chore(boot2): Bump to spin-deps rc.17, add spring properties migrator

* chore(boot2): spin-dep rc.18

* fix(boot2): spin-dep rc.21; fixed failing test

* chore(boot2): Why cant I get this transient failure to happen on my laptop

* chore(boot2): Cleanup of some speculative test output

* Add back JSON module. Fiat now boots up.

* Fix tests

* Fix case

* chore(deps): latest spinnaker-dependencies

* supply property placeholder for igor

* update to kork bom (#376)

* gradle 5.x

* somebody set up us the BOM

* fix(dependencies): dependency cleanup (#377)

* fix(dependencies): dependency cleanup

* removes lots of cargo-culted dependency declarations
* trims dependencies in fiat-core and fiat-api to minimize the
  impact consuming projects
* guava caches -> caffeine

* fix(api): cleanup FiatAuthenticationConfig WebSecurityConfigurerAdapter

* chore(dependencies): kork 4.1.0-rc.5-springBoot2

* chore(build): spinnaker-gradle-project 6.0.0

* chore(dependencies): kork 4.1.0-rc.11+springBoot2

* chore(build): remove useLastTag from non release travis buildscripts

* chore(dependencies): kork 4.1.0-rc.13+springBoot2

* chore(dependencies): update to kork 5.0.0

* chore(build): remove unnecessary gradle config for jvm version
  • Loading branch information
@cfieber
cfieber authored Apr 24, 2019
1 parent d912d40 commit bb8a3b6
Show file tree
Hide file tree
Showing 53 changed files with 316 additions and 419 deletions.
63 changes: 26 additions & 37 deletions build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,48 +15,30 @@
*/

buildscript {
ext {
korkVersion = "5.0.0"
}
repositories {
jcenter()
maven { url "https://spinnaker.bintray.com/gradle" }
maven { url "https://plugins.gradle.org/m2/" }
}
dependencies {
classpath 'com.netflix.spinnaker.gradle:spinnaker-dev-plugin:5.2.2'
classpath 'com.netflix.spinnaker.gradle:spinnaker-dev-plugin:6.0.0'
if (Boolean.valueOf(enablePublishing)) {
classpath 'com.netflix.spinnaker.gradle:spinnaker-gradle-project:6.0.0'
}
}
}

allprojects {
group = "com.netflix.spinnaker.fiat"
apply plugin: 'spinnaker.base-project'
apply plugin: 'java'
apply plugin: 'groovy'

ext {
spinnakerDependenciesVersion = '1.40.0'
if (project.hasProperty('spinnakerDependenciesVersion')) {
spinnakerDependenciesVersion = project.property('spinnakerDependenciesVersion')
}
}

def checkLocalVersions = [spinnakerDependenciesVersion: spinnakerDependenciesVersion]
if (ext.has('versions')) {
def extVers = ext.get('versions')
if (extVers instanceof Map) {
checkLocalVersions.putAll(extVers)
}
}

def localVersions = checkLocalVersions.findAll { it.value.endsWith('-SNAPSHOT') }
if (localVersions) {
logger.info("Enabling mavenLocal repo for $localVersions")
repositories {
mavenLocal()
}
}

spinnaker {
dependenciesVersion = spinnakerDependenciesVersion
if (Boolean.valueOf(enablePublishing)) {
apply plugin: 'spinnaker.project'
}
apply plugin: 'java-library'
apply plugin: 'groovy'

test {
testLogging {
Expand All @@ -78,16 +60,23 @@ allprojects {

subprojects { project ->
dependencies {
spinnaker.group('test')
implementation platform("com.netflix.spinnaker.kork:kork-bom:$korkVersion")
compileOnly "org.projectlombok:lombok"
annotationProcessor platform("com.netflix.spinnaker.kork:kork-bom:$korkVersion")
annotationProcessor "org.projectlombok:lombok"
testAnnotationProcessor platform("com.netflix.spinnaker.kork:kork-bom:$korkVersion")
testAnnotationProcessor "org.projectlombok:lombok"

testCompile spinnaker.dependency('groovy')
}
implementation "org.springframework.boot:spring-boot-properties-migrator"

//c&p this because NetflixOss reverts it to 1.7 and ends up getting applied last..
project.plugins.withType(JavaBasePlugin) {
JavaPluginConvention convention = project.convention.getPlugin(JavaPluginConvention)
convention.sourceCompatibility = JavaVersion.VERSION_1_8
convention.targetCompatibility = JavaVersion.VERSION_1_8
testImplementation "org.springframework.boot:spring-boot-starter-test"
testImplementation "org.spockframework:spock-core"
testImplementation "org.spockframework:spock-spring"
testImplementation "org.springframework:spring-test"
testImplementation "org.hamcrest:hamcrest-core"
testRuntimeOnly "cglib:cglib-nodep"
testRuntimeOnly "org.objenesis:objenesis"
testImplementation "org.codehaus.groovy:groovy-all"
}
}

Expand Down
41 changes: 15 additions & 26 deletions fiat-api/fiat-api.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,33 +15,22 @@
*/

dependencies {
compile project(":fiat-core")
implementation project(":fiat-core")

compileOnly spinnaker.dependency("bootWeb")
compileOnly spinnaker.dependency("frigga")
compileOnly spinnaker.dependency("korkSecurity")
compileOnly spinnaker.dependency("korkWeb")
compileOnly spinnaker.dependency("kork")
compileOnly spinnaker.dependency("lombok")
annotationProcessor spinnaker.dependency("lombok")
compileOnly spinnaker.dependency("okHttp")
compileOnly spinnaker.dependency("okHttpUrlconnection")
compileOnly spinnaker.dependency("okHttpApache")
compileOnly spinnaker.dependency("retrofit")
compileOnly spinnaker.dependency("retrofitJackson")
compileOnly spinnaker.dependency("spectatorApi")
implementation "org.springframework:spring-aop"
implementation "org.springframework:spring-web"
implementation "org.springframework.security:spring-security-core"
implementation "org.springframework.security:spring-security-web"
implementation "com.netflix.spectator:spectator-api"
implementation "com.netflix.spinnaker.kork:kork-core"
implementation "com.netflix.spinnaker.kork:kork-security"
implementation "com.netflix.spinnaker.kork:kork-web"
implementation "com.squareup.retrofit:retrofit"
implementation "com.squareup.retrofit:converter-jackson"

compile spinnaker.dependency("guava")
compile spinnaker.dependency("springSecurityConfig")
compile spinnaker.dependency("springSecurityCore")
compile spinnaker.dependency("springSecurityWeb")
compileOnly "javax.servlet:javax.servlet-api"

testCompile spinnaker.dependency("retrofit")
testCompile spinnaker.dependency("okHttp")
testCompile spinnaker.dependency("slf4jApi")
testCompile spinnaker.dependency("frigga")
testCompile spinnaker.dependency("korkSecurity")
testCompile spinnaker.dependency("kork")
testCompile spinnaker.dependency("bootAutoConfigure")
testCompile spinnaker.dependency("spectatorApi")
implementation "com.github.ben-manes.caffeine:caffeine"

testImplementation "org.slf4j:slf4j-api"
}
42 changes: 8 additions & 34 deletions fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatAuthenticationConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,13 @@

import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.netflix.spectator.api.Registry;
import com.netflix.spinnaker.config.OkHttpClientConfiguration;
import com.netflix.spinnaker.okhttp.SpinnakerRequestInterceptor;
import com.netflix.spinnaker.retrofit.Slf4jRetrofitLogger;
import com.squareup.okhttp.OkHttpClient;
import lombok.Setter;
import lombok.extern.slf4j.Slf4j;
import lombok.val;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
Expand All @@ -37,6 +35,7 @@
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
import org.springframework.security.web.authentication.AnonymousAuthenticationFilter;
import org.springframework.security.web.context.SecurityContextPersistenceFilter;
import retrofit.Endpoints;
import retrofit.RestAdapter;
Expand All @@ -59,8 +58,7 @@ public class FiatAuthenticationConfig {

@Bean
@ConditionalOnMissingBean(FiatService.class) // Allows for override
public FiatService fiatService(Registry registry,
FiatClientConfigurationProperties fiatConfigurationProperties,
public FiatService fiatService(FiatClientConfigurationProperties fiatConfigurationProperties,
SpinnakerRequestInterceptor interceptor,
OkHttpClientConfiguration okHttpClientConfiguration) {
// New role providers break deserialization if this is not enabled.
Expand Down Expand Up @@ -98,40 +96,16 @@ private class FiatWebSecurityConfigurerAdapter extends WebSecurityConfigurerAdap
private final FiatStatus fiatStatus;

private FiatWebSecurityConfigurerAdapter(FiatStatus fiatStatus) {
super(true);
this.fiatStatus = fiatStatus;
}

@Override
protected void configure(HttpSecurity http) throws Exception {
/*
* Having `FiatAuthenticationFilter` prior to `SecurityContextPersistenceFilter` results in the
* `SecurityContextHolder` being overridden with a null value.
*
* The null value then causes the `AnonymousAuthenticationFilter` to inject an "anonymousUser" which when
* passed over the wire to fiat is promptly rejected.
*
* This behavior is triggered when `management.security.enabled` is `false`.
*/
http
.csrf().disable()
.addFilterAfter(new FiatAuthenticationFilter(fiatStatus), SecurityContextPersistenceFilter.class);
}
}

private static class Slf4jRetrofitLogger implements RestAdapter.Log {
private final Logger logger;

Slf4jRetrofitLogger(Class type) {
this(LoggerFactory.getLogger(type));
}

Slf4jRetrofitLogger(Logger logger) {
this.logger = logger;
}

@Override
public void log(String message) {
logger.info(message);
http.servletApi().and()
.exceptionHandling().and()
.anonymous().and()
.addFilterBefore(new FiatAuthenticationFilter(fiatStatus), AnonymousAuthenticationFilter.class);
}
}
}
5 changes: 2 additions & 3 deletions ...pi/src/main/java/com/netflix/spinnaker/fiat/shared/FiatClientConfigurationProperties.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,15 @@ public RetryConfiguration getRetry() {
}

@Data
class PermissionsCache {
public static class PermissionsCache {
private Integer maxEntries = 1000;

private Integer expiresAfterWriteSeconds = 20;
}

@Data
class RetryConfiguration {
public static class RetryConfiguration {
private DynamicConfigService dynamicConfigService;

private long maxBackoffMillis = 10000;
private long initialBackoffMillis = 500;
private double retryMultiplier = 1.5;
Expand Down
68 changes: 36 additions & 32 deletions fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,8 @@

package com.netflix.spinnaker.fiat.shared;

import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
import com.google.common.util.concurrent.UncheckedExecutionException;
import com.github.benmanes.caffeine.cache.Cache;
import com.github.benmanes.caffeine.cache.Caffeine;
import com.netflix.spectator.api.Id;
import com.netflix.spectator.api.Registry;
import com.netflix.spinnaker.fiat.model.Authorization;
Expand All @@ -46,7 +45,6 @@
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
Expand Down Expand Up @@ -130,7 +128,7 @@ private static RetryHandler buildRetryHandler(FiatClientConfigurationProperties
this.fiatStatus = fiatStatus;
this.retryHandler = retryHandler;

this.permissionsCache = CacheBuilder
this.permissionsCache = Caffeine
.newBuilder()
.maximumSize(configProps.getCache().getMaxEntries())
.expireAfterWrite(configProps.getCache().getExpiresAfterWriteSeconds(), TimeUnit.SECONDS)
Expand Down Expand Up @@ -205,35 +203,41 @@ public UserPermission.View getPermission(String username) {
AtomicReference<Throwable> exception = new AtomicReference<>();

try {
view = permissionsCache.get(username, () -> {
view = permissionsCache.get(username, (loadUserName) -> {
cacheHit.set(false);
return AuthenticatedRequest.propagate(() -> {
try {
return retryHandler.retry("getUserPermission for " + username, () -> fiatService.getUserPermission(username));
} catch (Exception e) {
if (!fiatStatus.isLegacyFallbackEnabled()) {
throw e;
try {
return AuthenticatedRequest.propagate(() -> {
try {
return retryHandler.retry("getUserPermission for " + loadUserName, () -> fiatService.getUserPermission(loadUserName));
} catch (Exception e) {
if (!fiatStatus.isLegacyFallbackEnabled()) {
throw e;
}

legacyFallback.set(true);
successfulLookup.set(false);
exception.set(e);

// this fallback permission will be temporarily cached in the permissions cache
return new UserPermission.View(
new UserPermission()
.setId(AuthenticatedRequest.getSpinnakerUser().orElse("anonymous"))
.setAccounts(
Arrays
.stream(AuthenticatedRequest.getSpinnakerAccounts().orElse("").split(","))
.map(a -> new Account().setName(a))
.collect(Collectors.toSet())
)
).setLegacyFallback(true).setAllowAccessToUnknownApplications(true);
}

legacyFallback.set(true);
successfulLookup.set(false);
exception.set(e);

// this fallback permission will be temporarily cached in the permissions cache
return new UserPermission.View(
new UserPermission()
.setId(AuthenticatedRequest.getSpinnakerUser().orElse("anonymous"))
.setAccounts(
Arrays
.stream(AuthenticatedRequest.getSpinnakerAccounts().orElse("").split(","))
.map(a -> new Account().setName(a))
.collect(Collectors.toSet())
)
).setLegacyFallback(true).setAllowAccessToUnknownApplications(true);
}
}).call();
}).call();
} catch (RuntimeException re) {
throw re;
} catch (Exception e) {
throw new RuntimeException(e);
}
});
} catch (ExecutionException | UncheckedExecutionException e) {
} catch (Exception e) {
successfulLookup.set(false);
exception.set(e.getCause() != null ? e.getCause() : e);
}
Expand Down Expand Up @@ -352,7 +356,7 @@ public boolean isAdmin(Authentication authentication) {
return permission != null && permission.isAdmin();
}

public class AuthorizationFailure {
public static class AuthorizationFailure {
private final Authorization authorization;
private final ResourceType resourceType;
private final String resourceName;
Expand Down
2 changes: 1 addition & 1 deletion fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
package com.netflix.spinnaker.fiat.shared;

import com.netflix.spinnaker.fiat.model.UserPermission;
import com.squareup.okhttp.Response;
import retrofit.client.Response;
import retrofit.http.Body;
import retrofit.http.DELETE;
import retrofit.http.GET;
Expand Down
5 changes: 3 additions & 2 deletions fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatStatus.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.netflix.spinnaker.fiat.shared;

import com.netflix.spectator.api.Registry;
import com.netflix.spectator.api.patterns.PolledMeter;
import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -46,8 +47,8 @@ public FiatStatus(Registry registry,
this.enabled = new AtomicBoolean(fiatClientConfigurationProperties.isEnabled());
this.legacyFallbackEnabled = new AtomicBoolean(fiatClientConfigurationProperties.isLegacyFallback());

registry.gauge("fiat.enabled", enabled, value -> enabled.get() ? 1 : 0);
registry.gauge("fiat.legacyFallback.enabled", legacyFallbackEnabled, value -> legacyFallbackEnabled.get() ? 1 : 0);
PolledMeter.using(registry).withName("fiat.enabled").monitorValue(enabled, value -> enabled.get() ? 1 : 0);
PolledMeter.using(registry).withName("fiat.legacyFallback.enabled").monitorValue(legacyFallbackEnabled, value -> legacyFallbackEnabled.get() ? 1 : 0);
}

public boolean isEnabled() {
Expand Down
Loading

0 comments on commit bb8a3b6

Please sign in to comment.