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

merge master into boot2 branch #371

Merged
merged 11 commits into from
Apr 12, 2019
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ allprojects {
apply plugin: 'io.spring.dependency-management'

ext {
spinnakerDependenciesVersion = '2.0.0-rc.21-springBoot2'
spinnakerDependenciesVersion = '2.0.0-rc.24-springBoot2'
if (project.hasProperty('spinnakerDependenciesVersion')) {
spinnakerDependenciesVersion = project.property('spinnakerDependenciesVersion')
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,15 @@ public boolean isAdmin() {
return true; // TODO(ttomsu): Chosen by fair dice roll. Guaranteed to be random.
}


public boolean isAdmin(Authentication authentication) {
if (!fiatStatus.isEnabled()) {
return true;
}
UserPermission.View permission = getPermission(getUsername(authentication));
return permission != null && permission.isAdmin();
}

public class AuthorizationFailure {
private final Authorization authorization;
private final ResourceType resourceType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,23 @@ class FiatPermissionEvaluatorSpec extends Specification {
resourceType << ResourceType.values()*.toString()
}

def "should support isAdmin check for a user"() {
given:
1 * fiatService.getUserPermission("testUser") >> {
return new UserPermission.View()
.setApplications(Collections.emptySet())
.setAdmin(isAdmin)
}

expect:
evaluator.isAdmin(authentication) == expectedIsAdmin

where:
isAdmin || expectedIsAdmin
false || false
true || true
}

private static FiatClientConfigurationProperties buildConfigurationProperties() {
FiatClientConfigurationProperties configurationProperties = new FiatClientConfigurationProperties();
configurationProperties.enabled = true
Expand Down
1 change: 1 addition & 0 deletions fiat-roles/fiat-roles.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ dependencies {
compile spinnaker.dependency("korkDynomite")
compile spinnaker.dependency("korkHystrix")
compile spinnaker.dependency("korkJedis")
compile "redis.clients:jedis:2.9.3"

testCompile spinnaker.dependency("korkJedisTest")
testCompile "org.apache.commons:commons-collections4:4.1"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,40 +70,4 @@ private static JedisPool createPool(GenericObjectPoolConfig redisPoolConfig,

return new JedisPool(redisPoolConfig, host, port, timeout, password, database, null);
}

@Bean
public HealthIndicator redisHealth(final JedisPool jedisPool) {
return new HealthIndicator() {
@Override
public Health health() {
Health.Builder health;
try (Jedis jedis = jedisPool.getResource()) {
if ("PONG".equals(jedis.ping())) {
health = Health.up();
} else {
health = Health.down();
}
} catch (Exception ex) {
health = Health.down(ex);
}

try {
Field f = FieldUtils.getField(JedisPool.class, "internalPool", true /*forceAccess*/);
Object o = FieldUtils.readField(f, jedisPool, true /*forceAccess*/);
if (o instanceof GenericObjectPool) {
GenericObjectPool internal = (GenericObjectPool) o;
health.withDetail("maxIdle", internal.getMaxIdle());
health.withDetail("minIdle", internal.getMinIdle());
health.withDetail("numActive", internal.getNumActive());
health.withDetail("numIdle", internal.getNumIdle());
health.withDetail("numWaiters", internal.getNumWaiters());
}
} catch (IllegalAccessException iae) {
log.debug("Can't access jedis' internal pool", iae);
}

return health.build();
}
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

import com.netflix.appinfo.InstanceInfo;
import com.netflix.discovery.DiscoveryClient;
import com.netflix.spectator.api.Gauge;
import com.netflix.spectator.api.Registry;
import com.netflix.spinnaker.fiat.config.ResourceProvidersHealthIndicator;
import com.netflix.spinnaker.fiat.config.UnrestrictedResourceConfig;
import com.netflix.spinnaker.fiat.model.UserPermission;
Expand Down Expand Up @@ -71,8 +73,11 @@ public class UserRolesSyncer implements ApplicationListener<RemoteStatusChangedE

private final AtomicBoolean isEnabled;

private final Gauge userRolesSyncCount;

@Autowired
public UserRolesSyncer(Optional<DiscoveryClient> discoveryClient,
Registry registry,
LockManager lockManager,
PermissionsRepository permissionsRepository,
PermissionsResolver permissionsResolver,
Expand All @@ -99,6 +104,8 @@ public UserRolesSyncer(Optional<DiscoveryClient> discoveryClient,
// default to enabled iff discovery is not available
!discoveryClient.isPresent()
);

this.userRolesSyncCount = registry.gauge("fiat.userRoles.syncCount");
}

@Override
Expand All @@ -109,6 +116,7 @@ public void onApplicationEvent(RemoteStatusChangedEvent event) {
@Scheduled(fixedDelay = 30000L)
public void schedule() {
if (syncDelayMs < 0 || !isEnabled.get()) {
log.warn("User roles syncing is disabled (syncDelayMs: {}, isEnabled: {})", syncDelayMs, isEnabled.get());
return;
}

Expand All @@ -118,7 +126,14 @@ public void schedule() {
.withSuccessInterval(Duration.ofMillis(syncDelayMs))
.withFailureInterval(Duration.ofMillis(syncFailureDelayMs));

lockManager.acquireLock(lockOptions, () -> this.syncAndReturn(new ArrayList<>()));
lockManager.acquireLock(lockOptions, () -> {
try {
userRolesSyncCount.set(this.syncAndReturn(new ArrayList<>()));
} catch (Exception e) {
log.error("User roles synchronization failed", e);
userRolesSyncCount.set(-1);
}
});
}

public long syncAndReturn(List<String> roles) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import com.fasterxml.jackson.annotation.JsonInclude
import com.fasterxml.jackson.databind.ObjectMapper
import com.netflix.appinfo.InstanceInfo.InstanceStatus
import com.netflix.discovery.DiscoveryClient
import com.netflix.spectator.api.NoopRegistry
import com.netflix.spectator.api.Registry
import com.netflix.spinnaker.fiat.config.ResourceProvidersHealthIndicator
import com.netflix.spinnaker.fiat.config.UnrestrictedResourceConfig
import com.netflix.spinnaker.fiat.model.UserPermission
Expand Down Expand Up @@ -47,6 +49,9 @@ class UserRolesSyncerSpec extends Specification {

private static final String UNRESTRICTED = UnrestrictedResourceConfig.UNRESTRICTED_USERNAME;

@Shared
Registry registry = new NoopRegistry()

@Shared
@AutoCleanup("destroy")
EmbeddedRedis embeddedRedis
Expand Down Expand Up @@ -133,6 +138,7 @@ class UserRolesSyncerSpec extends Specification {
@Subject
def syncer = new UserRolesSyncer(
Optional.ofNullable(null),
registry,
lockManager,
repo,
permissionsResolver,
Expand Down Expand Up @@ -205,6 +211,7 @@ class UserRolesSyncerSpec extends Specification {
def lockManager = Mock(LockManager)
def userRolesSyncer = new UserRolesSyncer(
Optional.ofNullable(discoveryClient),
registry,
lockManager,
null,
null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ ClouddriverService clouddriverService(ClouddriverApi clouddriverApi) {

@Bean
@ConditionalOnProperty("services.igor.enabled")
IgorApi igorApi() {
IgorApi igorApi(@Value("${services.igor.baseUrl}") String igorEndpoint) {
return new RestAdapter.Builder()
.setEndpoint(Endpoints.newFixedEndpoint(igorEndpoint))
.setClient(okClient)
Expand Down
25 changes: 25 additions & 0 deletions fiat-web/src/test/groovy/com/netflix/spinnaker/fiat/MainSpec.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package com.netflix.spinnaker.fiat;

import com.netflix.hystrix.strategy.HystrixPlugins;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.context.TestPropertySource;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;

@RunWith(SpringJUnit4ClassRunner.class)
@SpringBootTest(classes = {Main.class})
@TestPropertySource(properties = {"spring.config.location=classpath:fiat-test.yml"})
public class MainSpec {

@Before
public void setUp() throws Exception {
HystrixPlugins.reset();
}

@Test
public void startupTest(){

}
}
8 changes: 8 additions & 0 deletions fiat-web/src/test/resources/fiat-test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
services:
clouddriver:
baseUrl: 'http://localhost:7003'
front50:
baseUrl: 'http://localhost:8080'
igor:
enabled: false
baseUrl: null