Skip to content

Conversation

@jbtrystram
Copy link
Member

Add SOCKS5 support to Zincati by enabling the appropriate feature flag for the reqwest dependency. Reqwest should pull in the proxy config from the environment.

Fixes coreos/fedora-coreos-tracker#2067

Add SOCKS5 support to Zincati by enabling the appropriate feature flag
for the reqwest dependency. Reqwest should pull in the proxy config
from the environment.

Fixes coreos/fedora-coreos-tracker#2067
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds SOCKS5 proxy support by enabling the socks feature for the reqwest dependency. The change is correct and addresses the linked issue. I have one suggestion to improve the maintainability of your Cargo.toml by making the dependency features more explicit, following a pattern already used for other dependencies in the file.

rand = ">=0.9, < 0.10"
regex = "1.12"
reqwest = { version = "0.12", features = ["json"] }
reqwest = { version = "0.12", features = ["json", "socks"] }

Choose a reason for hiding this comment

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

medium

To improve consistency with other dependencies in this file (like prometheus) and to make the feature set more explicit, consider disabling the default features of reqwest and specifying the required features directly. This makes it clearer which TLS backend is in use and prevents accidentally enabling new default features in future reqwest updates.

Suggested change
reqwest = { version = "0.12", features = ["json", "socks"] }
reqwest = { version = "0.12", default-features = false, features = ["json", "native-tls", "socks"] }

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.

Zincati ignores SOCKS proxy settings

1 participant