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

Fix nginxCloneScriptTemplate for old shells #3650

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

IldarMinaev
Copy link

@IldarMinaev IldarMinaev commented Jan 22, 2025

dash version 0.5.10.2-5 return error for export command with spaces after = symbol:

# export qwe=nginx version: nginx/1.20.1
dash: 10: export: version:: bad variable name

Description:
Fixing bug with nginxCloneScriptTemplate on old shells.
In my case I have third-party docker image with nginx and dash Version: 0.5.10.2-5 as shell.
It return error:

# export NGINX_VERSION=$( { nginx -v ; } 2>&1 ) && echo ${NGINX_VERSION##*/} > /opt/opentelemetry-webserver/source-conf/version.txt
dash: 11: export: version:: bad variable name
# nginx -v 
nginx version: nginx/1.20.1
# export NGINX_VERSION=nginx version: nginx/1.20.1
dash: 13: export: version:: bad variable name
# 

I suggest to quote result of nginx-v command for set variable command.

Link to tracking Issue(s):

Testing:
Run instrumentation of robotshop/rs-web:latest image.

Documentation:
N/A

dash version 0.5.10.2-5 return error for export command with spaces after = symbol:
```
# export qwe=nginx version: nginx/1.20.1
dash: 10: export: version:: bad variable name
```
@IldarMinaev IldarMinaev requested a review from a team as a code owner January 22, 2025 15:39
Copy link

linux-foundation-easycla bot commented Jan 22, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@swiatekm
Copy link
Contributor

The change itself looks fine to me, but ideally I'd like to add a e2e test to avoid regressions in the future. Is there a public nginx image using dash that we could use?

@IldarMinaev
Copy link
Author

IldarMinaev commented Jan 23, 2025

@swiatekm you can use robotshop/rs-web (part of https://github.com/instana/robot-shop). But it also has a problem with old version of Nginx (nginx:1.21.6) which is not supported by auto-instrumentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nginx auto instrumentation failed to get nginx version on dash 0.5.10.2-5
2 participants