You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "snazy (via GitHub)" <gi...@apache.org> on 2026/04/29 06:39:09 UTC

[PR] Fix `RealmContextFilterTest` relying config-overrides becoming system properties [polaris]

snazy opened a new pull request, #4313:
URL: https://github.com/apache/polaris/pull/4313

   `org.apache.polaris.core.persistence.bootstrap.RootCredentialsSet` retrieves the environmental provides values from, 1st, a system property and, if not present, from an environment variable. But `RootCredentialsSet` does not query the configuration backend.
   
   The fact that `RootCredentialsSet` could retrieve 'polaris.bootstrap.credentials' for this test via system properties was leveraging an implementation detail in Quarkus.  Configs do no longer become system properties in Quarkus/smallrye-config.
   
   The test-profile effectively ensures that this test runs "alone", so updating and restoring the relevant system property for `RootCredentialsSet` is safe.
   
   This unblocks #4297
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix `RealmContextFilterTest` relying config-overrides becoming system properties [polaris]

Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy merged PR #4313:
URL: https://github.com/apache/polaris/pull/4313


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix `RealmContextFilterTest` relying config-overrides becoming system properties [polaris]

Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #4313:
URL: https://github.com/apache/polaris/pull/4313#discussion_r3159377777


##########
runtime/service/src/test/java/org/apache/polaris/service/context/RealmContextFilterTest.java:
##########
@@ -46,9 +48,44 @@ public Map<String, String> getConfigOverrides() {
           "polaris.realm-context.header-name",
           REALM_HEADER,
           "polaris.realm-context.realms",
-          "realm1,realm2",
-          "polaris.bootstrap.credentials",
-          "realm1,client1,secret1;realm2,client2,secret2");
+          "realm1,realm2");
+    }
+
+    @Override
+    public List<TestResourceEntry> testResources() {
+      // `org.apache.polaris.core.persistence.bootstrap.RootCredentialsSet` retrieves the
+      // environmental provides values from, 1st, a system property and, if not present, from an
+      // environment variable. But `RootCredentialsSet` does not query the configuration backend.
+      //
+      // The fact that `RootCredentialsSet` could retrieve 'polaris.bootstrap.credentials' for this
+      // test via system properties was leveraging an implementation detail in Quarkus.
+      // Configs do no longer become system properties in Quarkus/smallrye-config.
+      //
+      // The test-profile effectively ensures that this test runs "alone", so updating and restoring
+      // the relevant system property for `RootCredentialsSet` is safe.
+      return List.of(new TestResourceEntry(RealmContextFilterTestResource.class));
+    }
+  }
+
+  public static class RealmContextFilterTestResource

Review Comment:
   Not sure. The goal here was to make sure that this special system-property is only used in this tests. I'm a bit cautious about splitting env-config concerns across two systems (JUnit/Pioneer vs Quarkus test profiles).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix `RealmContextFilterTest` relying config-overrides becoming system properties [polaris]

Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #4313:
URL: https://github.com/apache/polaris/pull/4313#discussion_r3159304814


##########
runtime/service/src/test/java/org/apache/polaris/service/context/RealmContextFilterTest.java:
##########
@@ -46,9 +48,44 @@ public Map<String, String> getConfigOverrides() {
           "polaris.realm-context.header-name",
           REALM_HEADER,
           "polaris.realm-context.realms",
-          "realm1,realm2",
-          "polaris.bootstrap.credentials",
-          "realm1,client1,secret1;realm2,client2,secret2");
+          "realm1,realm2");
+    }
+
+    @Override
+    public List<TestResourceEntry> testResources() {
+      // `org.apache.polaris.core.persistence.bootstrap.RootCredentialsSet` retrieves the
+      // environmental provides values from, 1st, a system property and, if not present, from an
+      // environment variable. But `RootCredentialsSet` does not query the configuration backend.
+      //
+      // The fact that `RootCredentialsSet` could retrieve 'polaris.bootstrap.credentials' for this
+      // test via system properties was leveraging an implementation detail in Quarkus.
+      // Configs do no longer become system properties in Quarkus/smallrye-config.
+      //
+      // The test-profile effectively ensures that this test runs "alone", so updating and restoring
+      // the relevant system property for `RootCredentialsSet` is safe.
+      return List.of(new TestResourceEntry(RealmContextFilterTestResource.class));
+    }
+  }
+
+  public static class RealmContextFilterTestResource

Review Comment:
   Alternatively, you could use JUnit Pioneer's `@RestoreSystemProperties` or 
   `@RestoreEnvironmentVariables` annotations. JUnit Pioneer is already used in a few tests.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix `RealmContextFilterTest` relying config-overrides becoming system properties [polaris]

Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #4313:
URL: https://github.com/apache/polaris/pull/4313#discussion_r3159311983


##########
runtime/service/src/test/java/org/apache/polaris/service/context/RealmContextFilterTest.java:
##########


Review Comment:
   Is this annotation really required?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix `RealmContextFilterTest` relying config-overrides becoming system properties [polaris]

Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #4313:
URL: https://github.com/apache/polaris/pull/4313#discussion_r3159819647


##########
runtime/service/src/test/java/org/apache/polaris/service/context/RealmContextFilterTest.java:
##########


Review Comment:
   In IJ, yea. Otherwise it complains about "unsupported HTTP headers".



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix `RealmContextFilterTest` relying config-overrides becoming system properties [polaris]

Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on PR #4313:
URL: https://github.com/apache/polaris/pull/4313#issuecomment-4346090746

   I think we could in fact produce a `RootCredentialsSet` bean from the SmallRye `Config` object. I will give this idea a try. But it doesn't invalidate the changes in this PR.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org