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

Microprofile Config doesn't read config profiled properties when injected into servlet.Filter class in native mode #23807

Closed
pbezpalko opened this issue Feb 18, 2022 · 11 comments · Fixed by #26996
Assignees
Labels
area/config kind/bug Something isn't working
Milestone

Comments

@pbezpalko
Copy link

pbezpalko commented Feb 18, 2022

Describe the bug

It seems to quarkus-undertow (processing Filter classes) doesn't work properly with profiled Quarkus configuration under native image.

Injected org.eclipse.microprofile.config.Config instance in class implementing javax.servlet.Filter doesn't contain values defined in active profile. But this happens only in native mode. Under standard JVM everything works fine and all variables (core + profiled) are available.

Filter class:

@WebFilter(filterName = "BasicAuthFilter", urlPatterns = "/urlpattern/*")
public class BasicAuthFilter implements Filter {
...
 @javax.inject.Inject 
  org.eclipse.microprofile.config.Config config;

    @Override
    public void doFilter(final ServletRequest servletRequest, final ServletResponse servletResponse,
            final FilterChain filterChain) throws IOException, ServletException {
          .....
            Optional<String> env = config.getOptionalValue("stage.environment");
          .....
    }

Configuration file:

quarkus:
  package:
    type: uber-jar
  log:
    level: INFO
'%NONE-CI':
  stage:
    environment: CI

Run under JVM:
java -jar ... -Dquarkus.profile=NONE-CI....

Run under Native:
./application -Dquarkus.profile=NONE-CI

Expected behavior

For both, native and standard jvm, execution of config.getOptionalValue("stage.environment") returns the same value if the same profile will be used.

Actual behavior

config.getOptionalValue("stage.environment") works invalid when application is started under native mode and returns empty Optional object.

config.getOptionalValue("stage.environment") works properly when application is started under standard JVM mode and returns value "CI" - which is perfectly OK.

My working Workaround:

  • Create some class with @ApplicationScoped (e.g. Helper)
  • Put @Inject Config into this class
  • Add method getConfigValue as proxy to Config instance
  • Inject Helper class in my Filter class
  • And use simple Helper.getConfig
// request scope is also fine
@javax.enterprise.context.ApplicationScoped
public class Helper {

   // injection by field works as well
   private Config config;

    @Inject
    public void setConfig(final Config config) {
        this.config = config;
    }

   public Optional<String> getConfigValue(String key) {
         config.getOptionalValue(key);
   }
}


public .... implements javax.servlet.Filter {

   @Inject 
   Helper  helper;

   // then somewhere  in code....
     helper.getConfigValue("stage.environment");
  // returns always valid value - whether in JVM or native mode

}

How to Reproduce?

No response

Output of uname -a or ver

Microsoft Windows [Version 10.0.19042.1466] / Native run under Docker image based on registry.access.redhat.com/ubi8/ubi-minimal:8.4

Output of java -version

OpenJDK Runtime Environment Corretto-11.0.7.10.1 (build 11.0.7+10-LTS)

GraalVM version (if different from Java)

quarkus-mandrel:21.3-java11

Quarkus version or git rev

2.5.0.Final

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T20:33:14+02:00) Java version: 11.0.7, vendor: Amazon.com Inc., runtime: C:\Programs\jdk-11-amazoncorretto Default locale: en_US, platform encoding: Cp1252 OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"

Additional information

Application compile & build:

mvn clean package 
   -Pnative 
   -Dquarkus.package.type=native 
   -Dquarkus.native.container-build=true 
   -Dquarkus.native.builder-image=quay.io/quarkus/ubi-quarkus-mandrel:21.3-java11

I verified this also in servlet class (extends javax.servlet.http.HttpServlet). In this case all works properly and variables are read correctly both from directly injected Config instance and indirectly injected via Helper (application scoped) class.

@pbezpalko pbezpalko added the kind/bug Something isn't working label Feb 18, 2022
@quarkus-bot quarkus-bot bot added area/mandrel env/windows Impacts Windows machines labels Feb 18, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Feb 18, 2022

/cc @Karm, @galderz, @zakkak

@galderz
Copy link
Member

galderz commented Mar 4, 2022

Hmmmm, I'm no expert in this area, but I wonder if quarkus.profile=NONE-CI should be passed at build time for this to work? I'm unsure how this would be done.

@gsmet @geoand This is not really about Mandrel, but about Quarkus resolution of configuration property names. When/where do those happen?

@geoand
Copy link
Contributor

geoand commented Mar 4, 2022

Hmmmm, I'm no expert in this area, but I wonder if quarkus.profile=NONE-CI should be passed at build time for this to work? I'm unsure how this would be done.

Shouldn't be necessary.

@gsmet @geoand This is not really about Mandrel, but about Quarkus resolution of configuration property names. When/where do those happen?

It's complicated, but this should work.

@geoand geoand added area/config and removed area/mandrel env/windows Impacts Windows machines labels Mar 4, 2022
@geoand
Copy link
Contributor

geoand commented Mar 4, 2022

@pbezpalko any chance you could put together a sample application that exhibits the problematic behavior?

@pbezpalko
Copy link
Author

Hi @geoand - yes, I will try to prepare the simplest application demonstrating all the problems found. I hope to be able to do this later this week.

@radcortez radcortez added the triage/needs-reproducer We are waiting for a reproducer. label Mar 8, 2022
@pbezpalko
Copy link
Author

Hi @geoand - I prepare simple demo for this bug - can be found here:
https://github.com/pbezpalko/quarkus-issues-23807

There is README.md with detailed information how to build and run. The bug is quite visible :)

@geoand geoand removed the triage/needs-reproducer We are waiting for a reproducer. label Mar 15, 2022
@geoand
Copy link
Contributor

geoand commented Mar 15, 2022

Looks like one for @radcortez

@radcortez
Copy link
Member

I'll have a look.

@radcortez radcortez self-assigned this Mar 15, 2022
@radcortez
Copy link
Member

This is a very interesting case. Let me see if I can explain what is happening properly:

The filters are initialized during the native image build, so the config instance that gets injected is also one that is created during static initialization, which uses the profile set during the native image build (if omitted is prod).

In the other case with the helper, the filter is also initialized during the native image build time and the injection point gets a lazy reference that is only created when required (when the app is called for the first time). In this case, the config instance you get is one that is created during runtime which uses the profile set by the user in the command line.

One way to fix this is to wrap your Config instance in Instance<Config> config.

But this raises some additional questions:

  • We are not locking the runtime config profile for the native image, so we are allowing to have different config profiles for static and runtime. We should probably disallow this because it generates inconsistent behavior but I'm sure users are relying on this right now. We may have to at least log a warning.
  • We had a similar discussion before about the static config injection (with REST Filters). We may have to detect this cases and log a warning as well, since this generates some confusion.

@geoand @mkouba any thoughts?

@geoand
Copy link
Contributor

geoand commented Mar 16, 2022

Logging a warning sounds like a good idea to me

@mkouba
Copy link
Contributor

mkouba commented Mar 18, 2022

Well, it's a known issue but I have no idea how to detect these cases...

One way to fix this is to wrap your Config instance in Instance config.

Yes, a general workaround is to fetch the config lazily at runtime. You can do the same with @ConfigProperty Instance<TYPE> as well; in this case you can leverage the @io.quarkus.arc.WithCaching annotation to avoid property lookup for every Instance.get().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/config kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants