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

Run time configuration consumed from VertxWebProcessor.kubernetes() #3677

Closed
dmlloyd opened this issue Aug 23, 2019 · 8 comments · Fixed by #3715
Closed

Run time configuration consumed from VertxWebProcessor.kubernetes() #3677

dmlloyd opened this issue Aug 23, 2019 · 8 comments · Fixed by #3715
Labels
kind/bug Something isn't working
Milestone

Comments

@dmlloyd
Copy link
Member

dmlloyd commented Aug 23, 2019

Describe the bug
A build step (VertxWebProcessor#kubernetes) is consuming a run time configuration object during build time to determine the HTTP listen port. This is not allowed because run time values cannot be resolved (#3516, #3637) or accessed during build time. The run time configuration objects can only correctly be used to pass as proxies to recorders for run time actions (though in this case, it is quite hard to detect invalid accesses to config fields because of the decision to opt for field injection instead of using e.g. a proxy interface for configuration values, so I might still look into directly injecting the config roots into the recorder proxies and drop support for any injection of run time config objects into build steps).

Additional context
Blocks #3516/#3637.

@dmlloyd dmlloyd added the kind/bug Something isn't working label Aug 23, 2019
@dmlloyd
Copy link
Member Author

dmlloyd commented Aug 23, 2019

/cc @stuartwdouglas

@geoand
Copy link
Contributor

geoand commented Aug 26, 2019

The KubernetesPortBuildItem is used to populate the Kubernetes resources and is only ever used at build time - meaning that changing the port value at runtime wouldn't affect the Kubernetes code in any way, since none of that code even exists at runtime.

If we can't consume the runtime config value at build time, what is the alternative?
We need to be able to use whatever value is configured in application.properties (or fallback to whatever the HttpConfiguration.port default is), so should we code this logic inside the processor instead of relying on the regular Quarkus config handling?

@stuartwdouglas
Copy link
Member

stuartwdouglas commented Aug 26, 2019 via email

@geoand
Copy link
Contributor

geoand commented Aug 26, 2019

PR incoming - I agree that the case is somewhat odd and it's the only one I of this sort I know of in the current codebase

geoand added a commit to geoand/quarkus that referenced this issue Aug 26, 2019
The Kubernetes resource generation only occurs at build so it suffices
to read the http port value from MP Config

Fixes: quarkusio#3677
@dmlloyd
Copy link
Member Author

dmlloyd commented Aug 26, 2019

The port value might not be present at build time, so reading it from the config only works if the user happened to supply it to the build. Maybe there should just be a separate configuration item for the Kubernetes port?

@geoand
Copy link
Contributor

geoand commented Aug 26, 2019

If the port is not supplied, wouldn't we just use the default http port?

@dmlloyd
Copy link
Member Author

dmlloyd commented Aug 26, 2019

Run time config is read & used at run time, so the port number could change from run to run.

@geoand
Copy link
Contributor

geoand commented Aug 26, 2019

That is true, but the kubernetes resources can't be changed in any case. It's just a file that the users can choose (or not) to use when the they build the application.

stuartwdouglas added a commit to stuartwdouglas/quarkus that referenced this issue Aug 27, 2019
The Kubernetes resource generation only occurs at build time so it suffices
to read the http port value from MP Config

Fixes: quarkusio#3677
@gsmet gsmet added this to the 0.22.0 milestone Aug 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
4 participants