Merge pull request #371 from cfieber/boot2

merge master into boot2 branch

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

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;

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

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"

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

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<RemoteStatusChangedE
 
   private final AtomicBoolean isEnabled;
 
+  private final Gauge userRolesSyncCount;
+
   @Autowired
   public UserRolesSyncer(Optional<DiscoveryClient> discoveryClient,
+                         Registry registry,
                          LockManager lockManager,
                          PermissionsRepository permissionsRepository,
                          PermissionsResolver permissionsResolver,
@@ -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
@@ -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<String> roles) {

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,

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)

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(){
+
+  }
+}

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