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: add http env to workers execution #184

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

ahitacat
Copy link
Contributor

@ahitacat ahitacat commented Nov 9, 2023

yggdrasil adds the proxy as an env variable when starts the worker.

Resolves: CCT-112

Copy link
Contributor

@jirihnidek jirihnidek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall it looks OK. I have only small requests.

HACKING.md Outdated Show resolved Hide resolved
cmd/yggd/main.go Show resolved Hide resolved
@ahitacat ahitacat force-pushed the add-env-worker branch 2 times, most recently from 53a6d87 to 5ffb207 Compare November 14, 2023 11:57
cmd/yggd/main.go Outdated Show resolved Hide resolved
cmd/yggd/main.go Show resolved Hide resolved
@ahitacat
Copy link
Contributor Author

Changelog

  • Remove HTTP_PROXY as it can be use in CGI environments to execute scripts, that could cause some malicious problems.
  • Added lowercase env variables options, no_proxy and https_proxy.

HACKING.md Outdated
Send a data message to `echo worker`
```
export CONSUMER_ID=$(openssl x509 -in cert.pem -subject -nocert | cut -f3 -d" ")
echo "{\"type\":\"data\",\"message_id\":\"$(uuidgen | tr -d '\n')\", \"response_to\":\" \",\"version\":1,\"sent\":\"$(date --iso-8601=seconds --utc | tr -d '\n')\",\"directive\":\"echo\",\"metadata\":\"{}\",\"content\":{\"hello world\"}}" | pub -broker tcp://test.mosquitto.org:1883 -topic yggdrasil/${CONSUMER_ID}/data/in
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In yggdrasil-0.2, there no other mention of pub. The example "ping" above uses mosquitto_pub. I think we should stick with consistency in the documentation, even though this is the HACKING/CONTRIBUTING guide.

Suggested change
echo "{\"type\":\"data\",\"message_id\":\"$(uuidgen | tr -d '\n')\", \"response_to\":\" \",\"version\":1,\"sent\":\"$(date --iso-8601=seconds --utc | tr -d '\n')\",\"directive\":\"echo\",\"metadata\":\"{}\",\"content\":{\"hello world\"}}" | pub -broker tcp://test.mosquitto.org:1883 -topic yggdrasil/${CONSUMER_ID}/data/in
mosquitto_pub --host 127.0.0.1 --port 1883 --topic yggdrasil/${CONSUMER_ID}/data/in --message "{\"type\":\"data\",\"message_id\":\"$(uuidgen | tr -d '\n')\", \"response_to\":\" \",\"version\":1,\"sent\":\"$(date --iso-8601=seconds --utc | tr -d '\n')\",\"directive\":\"echo\",\"metadata\":\"{}\",\"content\":{\"hello world\"}}"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops sorry! Changing it.

This commit adds:
yggdrasil proxy related env variable when starts the worker.
How to send data messages to echo workers added in HACKING file.

Signed-off-by: Alba Hita Catala <ahitacat@redhat.com>
Copy link
Collaborator

@subpop subpop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@jirihnidek jirihnidek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jirihnidek jirihnidek merged commit 463df06 into RedHatInsights:yggdrasil-0.2 Nov 21, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-yggdrasil-0.2 bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants