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

Env vars are not read anymore starting with 4.39.0 #2992

Open
venatir opened this issue Nov 8, 2024 · 7 comments
Open

Env vars are not read anymore starting with 4.39.0 #2992

venatir opened this issue Nov 8, 2024 · 7 comments
Labels
needs more info An issue that may be a bug or useful feature, but requires more information

Comments

@venatir
Copy link

venatir commented Nov 8, 2024

How to replicate:

Here's a sample file to test

input:
  type: stdin
pipeline:
  processors:
    - log:
        level: info
        message: "DB_CONNECTION_URL: ${DB_CONNECTION_URL}"
output:
  type: stdout
  1. set the test env var and running this in 4.38.0. - works
  2. set the test env var and running this in 4.39.0. - complains about missing env var.

This has been tested in the 4.38.0 and 4.39.0 tags for docker - redpandadata/connect in the dockerhub repository

@mihaitodor
Copy link
Collaborator

Hi @venatir 👋 I tried reproducing it on my end but no luck. Seems to work OK. How are you running that Docker container? Can you please share the full command line?

@mihaitodor mihaitodor added the needs more info An issue that may be a bug or useful feature, but requires more information label Nov 9, 2024
@venatir
Copy link
Author

venatir commented Nov 10, 2024

@mihaitodor Here's a way to replicate. 4.38.0 works. 4.39.0 doesn't

ver="4.39.0";
tmp_conf_file=`mktemp`
cat << EOF >$tmp_conf_file
input:
  type: stdin
pipeline:
  processors:
    - log:
        level: info
        message: "TEST: \${TEST}"
output:
  type: stdout
EOF

docker run -it --rm -e TEST="test" -v $tmp_conf_file:/my_conf.yaml redpandadata/connect:$ver -c /my_conf.yaml

@choult
Copy link

choult commented Nov 12, 2024

Confirmed recreated on my side when upgrading from benthos/4.6 to redpanda connect 4.39

Example codebase/docker-compose setup here:

https://github.com/choult/benthos-pres/blob/rp-connect-2992/docker-compose.yml

Switching to 4.38 for all containers works as expected

@mihaitodor
Copy link
Collaborator

Small update on this issue: The now deprecated flag -c is indeed ignoring environment variables and we'll have a fix out ASAP. In the meantime, if you wish to use v4.39.0, you should be able to do rpk connect run ... or redpanda-connect run ....

@peczenyj
Copy link
Contributor

$ docker run -it --rm -e TEST="test" -v ./foo:/c redpandadata/connect:4.39.0 run /c/config.yaml
INFO Running main config from specified file       @service=redpanda-connect benthos_version=v4.39.0 path=/c/config.yaml
INFO Listening for HTTP requests at: http://0.0.0.0:4195  @service=redpanda-connect
INFO Input type stdin is now active                @service=redpanda-connect label="" path=root.input
INFO Launching a Redpanda Connect instance, use CTRL+C to close  @service=redpanda-connect
INFO Output type stdout is now active              @service=redpanda-connect label="" path=root.output
sqdsqd
INFO TEST: test                                    @service=redpanda-connect custom_source=true label="" path=root.pipeline.processors.0
sqdsqd
^CINFO Received SIGINT, the service is closing       @service=redpanda-connect

it seems a problem when you don't use the command run

BTW I can't get the -c flag to work on any version

@venatir
Copy link
Author

venatir commented Dec 4, 2024

Thanks @mihaitodor. I'll use run.
It's my fault for not following the deprecation announcements, but I don't know anyone who does this for every product.
I wonder if giving a warning that -c is deprecated can help? cc: @mihaitodor

@peczenyj , the -c flag, while deprecated still works. See my example above #2992 (comment)

@mihaitodor
Copy link
Collaborator

I wonder if giving a warning that -c is deprecated can help? cc: @mihaitodor

It really wasn't meant to break and the intent is to keep it around until at least another major version bump given how many people used it before run was introduced. Eventually, it will be covered in one of the migration guides.

I think @peczenyj got tripped up by the template docs which had a bug that was fixed here. If there's some other corner case, please let us know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs more info An issue that may be a bug or useful feature, but requires more information
Projects
None yet
Development

No branches or pull requests

4 participants