Enable persistent sessions by default

Run CI with the feature disabled to test also the old settings Closes keycloak#32265 Signed-off-by: Michal Hajas <mhajas@redhat.com>
miquelsi · Aug 21, 2024 · f5b2775 · f5b2775
1 parent 607ab01
commit f5b2775
Show file tree

Hide file tree

Showing 9 changed files with 57 additions and 72 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -322,16 +322,12 @@ jobs:
         with:
           job-id: jdk-integration-tests-${{ matrix.os }}-${{ matrix.dist }}-${{ matrix.version }}
 
-  persistent-sessions-tests:
-    name: Persistent Sessions IT
+  volatile-sessions-tests:
+    name: Volatile Sessions IT
     needs: [build, conditional]
     if: needs.conditional.outputs.ci-store == 'true'
     runs-on: ubuntu-latest
     timeout-minutes: 150
-    strategy:
-      matrix:
-        variant: [ "pus-ec", "pus-rc" ]
-      fail-fast: false
     steps:
       - uses: actions/checkout@v4
 
@@ -341,22 +337,9 @@ jobs:
 
       - name: Run base tests
         run: |
-          TESTS=`testsuite/integration-arquillian/tests/base/testsuites/suite.sh persistent-sessions`
+          TESTS=`testsuite/integration-arquillian/tests/base/testsuites/suite.sh volatile-sessions`
           echo "Tests: $TESTS"
-          case "${{ matrix.variant }}" in
-            pus-ec)
-              VARIANT="-Dauth.server.feature=persistent-user-sessions"
-              ;;
-            pus-rc)
-              VARIANT="-Pinfinispan-server -Dauth.server.feature=persistent-user-sessions,multi-site,remote-cache"
-              ;;
-            *)
-              echo "Unknown Matrix element"
-              exit 1
-              ;;
-          esac
-          echo "Variant: $VARIANT"
-          ./mvnw test ${{ env.SUREFIRE_RETRY }} -Pauth-server-quarkus "-Dwebdriver.chrome.driver=$CHROMEWEBDRIVER/chromedriver" $VARIANT -Dtest=$TESTS -pl testsuite/integration-arquillian/tests/base 2>&1 | misc/log/trimmer.sh
+          ./mvnw test ${{ env.SUREFIRE_RETRY }} -Pauth-server-quarkus "-Dwebdriver.chrome.driver=$CHROMEWEBDRIVER/chromedriver" -Dauth.server.feature.disable=persistent-user-sessions -Dtest=$TESTS -pl testsuite/integration-arquillian/tests/base 2>&1 | misc/log/trimmer.sh
 
       - name: Upload JVM Heapdumps
         if: always()
@@ -403,7 +386,7 @@ jobs:
         run: |
           TESTS=`testsuite/integration-arquillian/tests/base/testsuites/suite.sh remote-cache`
           echo "Tests: $TESTS"
-          ./mvnw test ${{ env.SUREFIRE_RETRY }} -Pauth-server-quarkus -Pinfinispan-server -Dauth.server.feature=${{ matrix.variant }} -Dtest=$TESTS -pl testsuite/integration-arquillian/tests/base 2>&1 | misc/log/trimmer.sh
+          ./mvnw test ${{ env.SUREFIRE_RETRY }} -Pauth-server-quarkus -Pinfinispan-server -Dauth.server.feature=${{ matrix.variant }} -Dauth.server.feature.disable=persistent-user-sessions -Dtest=$TESTS -pl testsuite/integration-arquillian/tests/base 2>&1 | misc/log/trimmer.sh
 
       - name: Upload JVM Heapdumps
         if: always()
@@ -973,7 +956,7 @@ jobs:
       - quarkus-integration-tests
       - jdk-integration-tests
       - store-integration-tests
-      - persistent-sessions-tests
+      - volatile-sessions-tests
       - store-model-tests
       - clustering-integration-tests
       - fips-unit-tests

diff --git a/common/src/main/java/org/keycloak/common/Profile.java b/common/src/main/java/org/keycloak/common/Profile.java
@@ -111,7 +111,7 @@ public enum Feature {
         HOSTNAME_V1("Hostname Options V1", Type.DEPRECATED, 1),
         HOSTNAME_V2("Hostname Options V2", Type.DEFAULT, 2),
 
-        PERSISTENT_USER_SESSIONS("Persistent online user sessions across restarts and upgrades", Type.PREVIEW),
+        PERSISTENT_USER_SESSIONS("Persistent online user sessions across restarts and upgrades", Type.DEFAULT),
 
         OID4VC_VCI("Support for the OID4VCI protocol as part of OID4VC.", Type.EXPERIMENTAL),
 

diff --git a/...java/org/keycloak/testsuite/arquillian/containers/AbstractQuarkusDeployableContainer.java b/...java/org/keycloak/testsuite/arquillian/containers/AbstractQuarkusDeployableContainer.java
@@ -236,34 +236,43 @@ private static void prepareCommandsForRebuilding(List<String> commands) {
     }
 
     protected void addFeaturesOption(List<String> commands) {
-        String defaultFeatures = configuration.getDefaultFeatures();
+        String enabledFeatures = configuration.getEnabledFeatures();
+        String disabledFeatures = configuration.getDisabledFeatures();
 
-        if (StringUtil.isBlank(defaultFeatures)) {
+        if (StringUtil.isBlank(enabledFeatures) && StringUtil.isBlank(disabledFeatures)) {
             return;
         }
 
         if (commands.stream().anyMatch(List.of("import", "export")::contains)) {
             return;
         }
 
-        StringBuilder featuresOption = new StringBuilder("--features=").append(defaultFeatures);
+        if (!StringUtil.isBlank(enabledFeatures)) {
+            appendOrAddCommand(commands, "--features=", enabledFeatures);
+        }
+
+        if (!StringUtil.isBlank(disabledFeatures)) {
+            appendOrAddCommand(commands, "--features-disabled=", disabledFeatures);
+        }
+
+        // enabling or disabling features requires rebuilding the image
+        prepareCommandsForRebuilding(commands);
+    }
+
+    private void appendOrAddCommand(List<String> commands, String command, String addition) {
         Iterator<String> iterator = commands.iterator();
 
         while (iterator.hasNext()) {
-            String command = iterator.next();
+            String existingCommand = iterator.next();
 
-            if (command.startsWith("--features")) {
-                featuresOption = new StringBuilder(command);
-                featuresOption.append(",").append(defaultFeatures);
+            if (existingCommand.startsWith(command)) {
                 iterator.remove();
-                break;
+                commands.add(existingCommand + "," + addition);
+                return;
             }
         }
 
-        // enabling or disabling features requires rebuilding the image
-        prepareCommandsForRebuilding(commands);
-
-        commands.add(featuresOption.toString());
+        commands.add(command + addition);
     }
 
     protected List<String> configureArgs(List<String> commands) {
@@ -425,7 +434,7 @@ private void addFipsOptions(List<String> commands) {
     }
 
     private Collection<String> getDefaultFeatures() {
-        var features = configuration.getDefaultFeatures();
+        var features = configuration.getEnabledFeatures();
         if (features == null || features.isBlank()) {
             return List.of();
         }

diff --git a/.../main/java/org/keycloak/testsuite/arquillian/containers/KeycloakQuarkusConfiguration.java b/.../main/java/org/keycloak/testsuite/arquillian/containers/KeycloakQuarkusConfiguration.java
@@ -48,7 +48,8 @@ public class KeycloakQuarkusConfiguration implements ContainerConfiguration {
 
     private FipsMode fipsMode = FipsMode.valueOfOption(System.getProperty("auth.server.fips.mode"));
 
-    private String defaultFeatures;
+    private String enabledFeatures;
+    private String disabledFeatures;
 
     @Override
     public void validate() throws ConfigurationException {
@@ -241,11 +242,19 @@ public void setFipsMode(FipsMode fipsMode) {
         this.fipsMode = fipsMode;
     }
 
-    public void setDefaultFeatures(String defaultFeatures) {
-        this.defaultFeatures = defaultFeatures;
+    public void setEnabledFeatures(String enabledFeatures) {
+        this.enabledFeatures = enabledFeatures;
     }
 
-    public String getDefaultFeatures() {
-        return defaultFeatures;
+    public String getEnabledFeatures() {
+        return enabledFeatures;
+    }
+
+    public String getDisabledFeatures() {
+        return disabledFeatures;
+    }
+
+    public void setDisabledFeatures(String disabledFeatures) {
+        this.disabledFeatures = disabledFeatures;
     }
 }
diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/arquillian.xml b/testsuite/integration-arquillian/tests/base/src/test/resources/arquillian.xml
@@ -636,7 +636,8 @@
             <property name="javaOpts">-Xms512m -Xmx512m -XX:MetaspaceSize=96M -XX:MaxMetaspaceSize=512m
                 -Djava.net.preferIPv4Stack=true -Dauth.server.db.host=some
             </property>
-            <property name="defaultFeatures">${auth.server.feature}</property>
+            <property name="enabledFeatures">${auth.server.feature}</property>
+            <property name="disabledFeatures">${auth.server.feature.disable}</property>
         </configuration>
     </container>
 
@@ -647,7 +648,8 @@
                 org.keycloak.testsuite.arquillian.containers.KeycloakQuarkusEmbeddedDeployableContainer
             </property>
             <property name="bindHttpPortOffset">${auth.server.port.offset}</property>
-            <property name="defaultFeatures">${auth.server.feature}</property>
+            <property name="enabledFeatures">${auth.server.feature}</property>
+            <property name="disabledFeatures">${auth.server.feature.disable}</property>
         </configuration>
     </container>
 

diff --git a/...base/testsuites/persistent-sessions-suite → ...s/base/testsuites/volatile-sessions-suite b/...base/testsuites/persistent-sessions-suite → ...s/base/testsuites/volatile-sessions-suite
diff --git a/testsuite/integration-arquillian/tests/pom.xml b/testsuite/integration-arquillian/tests/pom.xml
@@ -96,6 +96,7 @@
 
         <auth.server.profile/>
         <auth.server.feature/>
+        <auth.server.feature.disable/>
 
         <auth.server.host2>${auth.server.host}</auth.server.host2> <!-- for broker and JS adapter tests; defaults to auth.server.host -->
         <app.server.host>localhost</app.server.host>
@@ -454,6 +455,7 @@
 
                             <auth.server.profile>${auth.server.profile}</auth.server.profile>
                             <auth.server.feature>${auth.server.feature}</auth.server.feature>
+                            <auth.server.feature.disable>${auth.server.feature.disable}</auth.server.feature.disable>
 
                             <auth.server.host2>${auth.server.host2}</auth.server.host2> <!-- for broker tests -->
 

diff --git a/testsuite/model/pom.xml b/testsuite/model/pom.xml
@@ -224,32 +224,12 @@
         </profile>
 
         <profile>
-            <id>jpa+infinispan+persistentsessions</id>
+            <id>jpa+infinispan+volatilesessions</id>
             <properties>
-                <keycloak.model.parameters>Infinispan,Jpa,PersistentUserSessions</keycloak.model.parameters>
+                <keycloak.model.parameters>Infinispan,Jpa,VolatileUserSessions</keycloak.model.parameters>
             </properties>
         </profile>
 
-        <profile>
-            <id>jpa+cross-dc-infinispan+persistentsessions</id>
-            <properties>
-                <keycloak.model.parameters>CrossDCInfinispan,Jpa,PersistentUserSessions</keycloak.model.parameters>
-            </properties>
-            <build>
-                <plugins>
-                    <plugin>
-                        <groupId>org.apache.maven.plugins</groupId>
-                        <artifactId>maven-surefire-plugin</artifactId>
-                        <configuration>
-                            <systemPropertyVariables>
-                                <keycloak.profile.feature.multi_site>enabled</keycloak.profile.feature.multi_site>
-                            </systemPropertyVariables>
-                        </configuration>
-                    </plugin>
-                </plugins>
-            </build>
-        </profile>
-
         <profile>
             <id>jpa+infinispan+client-storage</id>
             <properties>

diff --git a/...el/parameters/PersistentUserSessions.java → ...odel/parameters/VolatileUserSessions.java b/...el/parameters/PersistentUserSessions.java → ...odel/parameters/VolatileUserSessions.java
@@ -1,13 +1,13 @@
 /*
  * Copyright 2020 Red Hat, Inc. and/or its affiliates
  * and other contributors as indicated by the @author tags.
- * 
+ *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  * http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -23,9 +23,9 @@
 
 import java.util.Collections;
 
-public class PersistentUserSessions extends KeycloakModelParameters {
+public class VolatileUserSessions extends KeycloakModelParameters {
 
-    public PersistentUserSessions() {
+    public VolatileUserSessions() {
         super(Collections.emptySet(), Collections.emptySet());
     }
 
@@ -35,6 +35,6 @@ public void updateConfig(Config cf) {
     }
 
     public static void updateConfigForJpa(Config cf) {
-        System.getProperties().put(PropertiesProfileConfigResolver.getPropertyKey(Profile.Feature.PERSISTENT_USER_SESSIONS), "enabled");
+        System.getProperties().put(PropertiesProfileConfigResolver.getPropertyKey(Profile.Feature.PERSISTENT_USER_SESSIONS), "disabled");
     }
 }