spring-cloud-dataflow icon indicating copy to clipboard operation
spring-cloud-dataflow copied to clipboard

Unable to configure https dataflow version 2.9.1

Open slobodanl opened this issue 3 years ago • 4 comments

Description: Customization in docker-compose file gets ignored.

Release versions: 2.9.1 / 2.8.2 Custom apps:

Steps to reproduce: Add a property in the environment section, e.g. - server.port=9595

Additional context: The dataflow server instead of starting with 9595 will bind the default 9393 port. Noticed the bug since we have 2.7.0 dataflow version in production with enable https configuration all that gets ignored in new version 2.9.1. I tested more and for some reason, if i disable the waiting part for the MySQL container the expected configuration will kick in e.g. 9595 port started with https.

slobodanl avatar Dec 06 '21 11:12 slobodanl

@slobodanl not sure I completely understand the issue. But in general to set a property in the docker-compose environment section you should use env. variables not property names. E.g. could you try to use - SERVER_PORT=9595 instead?

tzolov avatar Dec 06 '21 16:12 tzolov

I just tried it and looks like stuff we do in an entrypoint somewhat causes format server.port not passed to process. If you use format SERVER_PORT in environment section then it works.

jvalkeal avatar Dec 06 '21 17:12 jvalkeal

@tzolov yes it works as you propose but it's a bit of a workaround of the problem don't you agree, especially in light of documentation examples how to customize your local environment customize-dataflow , maybe changing the port was oversimplification but when it comes to configuring https we will need to add 6 additional variables as proposed not to mention enabling oauth2. @jvalkeal thank you for your time confirming this.

slobodanl avatar Dec 07 '21 08:12 slobodanl

I don't see anything in the link customize-dataflow that indicates there is support for passing in environment variables of the form lower.case. Can you point us to that?

Also, how do you propose that lower.case format would simplify configuration of https/oauth because it is still a flat listing of variables. Using UPPER_CASE_ENV_VAR format seems like a reasonable path.

markpollack avatar May 17 '22 15:05 markpollack

@corneil Can you verify that with the latest work done on revamping the docker compose experience works well with respect to two things.

  1. I see that the SSL configuration docker-compose-ssl.yml is not discussed in the ref docs. It seems like this was not part of the revamping of the docker compose experience.
  2. What is the recommended way to configure things. It looks to me like the UPPER_CASE_ENV_VAR in the environment section is the way to go. Whatever the outcome, we can add some info to the docs.

markpollack avatar Sep 22 '22 12:09 markpollack

@corneil Can you verify that with the latest work done on revamping the docker compose experience works well with respect to two things.

  1. I see that the SSL configuration docker-compose-ssl.yml is not discussed in the ref docs. It seems like this was not part of the revamping of the docker compose experience.
  2. What is the recommended way to configure things. It looks to me like the UPPER_CASE_ENV_VAR in the environment section is the way to go. Whatever the outcome, we can add some info to the docs.

The change made just separated the docker-compose files so there are no assumptions about broker or database. Users will always have to use more than one file to start the containers. docker-compose-ssl.yml only adds environmental variables to provide SSL. So it can just be added to the list of docker-compose files used when starting.

With docker-compose it is the easiest to add required environmental variables. There are a few cases where all uppercase doesn't evaluate properly. I think they are fairly obvious when you come across them. I assumed it was some mistake and broke stuff. Reverting fixed it. @jvalkeal has the inside track on those cases.

corneil avatar Sep 22 '22 12:09 corneil

ok, i'll keep this issue open as a tweak to our current docs to explicitly cover these two areas. I'll rename the issue and put in in the next milestone/rc release for 2.10.

markpollack avatar Sep 22 '22 14:09 markpollack

The docker compose usage has been documented.

corneil avatar Dec 12 '22 08:12 corneil