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

upgrades fiat to boot2 and new BOM dependency management #388

Merged
merged 35 commits into from
Apr 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
e0289ae
refactor(boot2): Mostly working... one transient test failure
robzienert Mar 11, 2019
a4f053b
chore(boot2): Bump to spin-deps rc.17, add spring properties migrator
robzienert Mar 11, 2019
c888310
chore(boot2): spin-dep rc.18
robzienert Mar 11, 2019
ccc04a3
fix(boot2): spin-dep rc.21; fixed failing test
robzienert Mar 12, 2019
72e6321
fix(boot2): Fix transient test?
robzienert Mar 12, 2019
30cb454
chore(boot2): Why cant I get this transient failure to happen on my l…
robzienert Mar 13, 2019
13d0078
fix(boot2): Im sure this will not solve it
robzienert Mar 13, 2019
d481c73
chore(boot2): Cleanup of some speculative test output
robzienert Mar 13, 2019
35738da
chore(boot2): Disable parallel gradle execution
robzienert Mar 14, 2019
3a8a981
refactor(boot2): Mostly working... one transient test failure
robzienert Mar 11, 2019
99268cc
chore(boot2): Bump to spin-deps rc.17, add spring properties migrator
robzienert Mar 11, 2019
bdebc70
chore(boot2): spin-dep rc.18
robzienert Mar 11, 2019
32b74ca
fix(boot2): spin-dep rc.21; fixed failing test
robzienert Mar 12, 2019
eb19e22
chore(boot2): Why cant I get this transient failure to happen on my l…
robzienert Mar 13, 2019
b256f87
chore(boot2): Cleanup of some speculative test output
robzienert Mar 13, 2019
6c09171
Add back JSON module. Fiat now boots up.
dibyom Mar 25, 2019
8c4679f
Fix tests
dibyom Mar 26, 2019
af66546
Fix case
dibyom Mar 26, 2019
d3fafaf
chore(deps): latest spinnaker-dependencies
cfieber Apr 12, 2019
1a04e08
Merge remote-tracking branch 'upstream/master' into boot2
cfieber Apr 12, 2019
03d8217
supply property placeholder for igor
cfieber Apr 12, 2019
61cdc5c
Merge pull request #371 from cfieber/boot2
cfieber Apr 12, 2019
972c3f3
update to kork bom (#376)
cfieber Apr 17, 2019
69350e0
Merge branch 'master' into boot2
cfieber Apr 19, 2019
4e85f1b
fix(dependencies): dependency cleanup (#377)
cfieber Apr 19, 2019
3f8fa98
Merge remote-tracking branch 'upstream/master' into boot2
cfieber Apr 22, 2019
bdbae49
chore(dependencies): kork 4.1.0-rc.5-springBoot2
cfieber Apr 22, 2019
cfa2357
Merge remote-tracking branch 'upstream/master' into boot2
cfieber Apr 22, 2019
df017c7
chore(build): spinnaker-gradle-project 6.0.0
cfieber Apr 22, 2019
fb79831
Merge remote-tracking branch 'upstream/master' into boot2
cfieber Apr 24, 2019
fa1f056
chore(dependencies): kork 4.1.0-rc.11+springBoot2
cfieber Apr 24, 2019
d26254a
chore(build): remove useLastTag from non release travis buildscripts
cfieber Apr 24, 2019
57207a6
chore(dependencies): kork 4.1.0-rc.13+springBoot2
cfieber Apr 24, 2019
7dd8ebe
chore(dependencies): update to kork 5.0.0
cfieber Apr 24, 2019
708c142
chore(build): remove unnecessary gradle config for jvm version
cfieber Apr 24, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 26 additions & 37 deletions build.gradle
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
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"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these all are actually runtime dependencies. With BOM based dependency opinions we won't run into problems on version alignment so it is safe to make these implementation rather than compileOnly

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"
}
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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this disables defaults, our auth filter chain for fiat is mostly about having the FiatAuthenticationFilter in there and falling back to anonymous

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);
}
}
}
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
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
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
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