diff --git a/build.gradle b/build.gradle index 72497bed0..5bfcd050b 100644 --- a/build.gradle +++ b/build.gradle @@ -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') } diff --git a/fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluator.java b/fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluator.java index 11aa27b7e..3c9435535 100644 --- a/fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluator.java +++ b/fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluator.java @@ -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; diff --git a/fiat-api/src/test/groovy/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluatorSpec.groovy b/fiat-api/src/test/groovy/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluatorSpec.groovy index c9231971a..b62c23935 100644 --- a/fiat-api/src/test/groovy/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluatorSpec.groovy +++ b/fiat-api/src/test/groovy/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluatorSpec.groovy @@ -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 diff --git a/fiat-roles/fiat-roles.gradle b/fiat-roles/fiat-roles.gradle index 070fb3a9a..27247ea7f 100644 --- a/fiat-roles/fiat-roles.gradle +++ b/fiat-roles/fiat-roles.gradle @@ -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" diff --git a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/config/RedisConfig.java b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/config/RedisConfig.java index 8957a73c3..acdfa403b 100644 --- a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/config/RedisConfig.java +++ b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/config/RedisConfig.java @@ -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(); - } - }; - } } diff --git a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/roles/UserRolesSyncer.java b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/roles/UserRolesSyncer.java index f4f82278c..da87540de 100644 --- a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/roles/UserRolesSyncer.java +++ b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/roles/UserRolesSyncer.java @@ -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; @@ -71,8 +73,11 @@ public class UserRolesSyncer implements ApplicationListener discoveryClient, + Registry registry, LockManager lockManager, PermissionsRepository permissionsRepository, PermissionsResolver permissionsResolver, @@ -99,6 +104,8 @@ public UserRolesSyncer(Optional discoveryClient, // default to enabled iff discovery is not available !discoveryClient.isPresent() ); + + this.userRolesSyncCount = registry.gauge("fiat.userRoles.syncCount"); } @Override @@ -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; } @@ -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 roles) { diff --git a/fiat-roles/src/test/groovy/com/netflix/spinnaker/fiat/roles/UserRolesSyncerSpec.groovy b/fiat-roles/src/test/groovy/com/netflix/spinnaker/fiat/roles/UserRolesSyncerSpec.groovy index 725991afd..f0cd688b1 100644 --- a/fiat-roles/src/test/groovy/com/netflix/spinnaker/fiat/roles/UserRolesSyncerSpec.groovy +++ b/fiat-roles/src/test/groovy/com/netflix/spinnaker/fiat/roles/UserRolesSyncerSpec.groovy @@ -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 @@ -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 @@ -133,6 +138,7 @@ class UserRolesSyncerSpec extends Specification { @Subject def syncer = new UserRolesSyncer( Optional.ofNullable(null), + registry, lockManager, repo, permissionsResolver, @@ -205,6 +211,7 @@ class UserRolesSyncerSpec extends Specification { def lockManager = Mock(LockManager) def userRolesSyncer = new UserRolesSyncer( Optional.ofNullable(discoveryClient), + registry, lockManager, null, null, diff --git a/fiat-web/src/main/java/com/netflix/spinnaker/fiat/config/ResourcesConfig.java b/fiat-web/src/main/java/com/netflix/spinnaker/fiat/config/ResourcesConfig.java index 9cda01756..e1ed84e82 100644 --- a/fiat-web/src/main/java/com/netflix/spinnaker/fiat/config/ResourcesConfig.java +++ b/fiat-web/src/main/java/com/netflix/spinnaker/fiat/config/ResourcesConfig.java @@ -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) diff --git a/fiat-web/src/test/groovy/com/netflix/spinnaker/fiat/MainSpec.java b/fiat-web/src/test/groovy/com/netflix/spinnaker/fiat/MainSpec.java new file mode 100644 index 000000000..b03a42ff7 --- /dev/null +++ b/fiat-web/src/test/groovy/com/netflix/spinnaker/fiat/MainSpec.java @@ -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(){ + + } +} \ No newline at end of file diff --git a/fiat-web/src/test/resources/fiat-test.yml b/fiat-web/src/test/resources/fiat-test.yml new file mode 100644 index 000000000..db1170b9a --- /dev/null +++ b/fiat-web/src/test/resources/fiat-test.yml @@ -0,0 +1,8 @@ +services: + clouddriver: + baseUrl: 'http://localhost:7003' + front50: + baseUrl: 'http://localhost:8080' + igor: + enabled: false + baseUrl: null