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: fixed bug where proxy was ignored #609

Merged
merged 6 commits into from
Sep 26, 2023

Conversation

robbert229
Copy link
Contributor

This PR does two main things,

http.Transport proxy

In areas where we manually construct an http.Transport unless we explicitly provide a Proxy field of http.ProxyFromEnvironment, we are explicitly configuring the http package to ignore the proxy environment variables. This means that otf ignores all proxy settings.

oidc.NewProvider Timeout

Changes the oidc_authenticator to ensure that it throws an error after waiting for more than ten seconds to construct the oidc provider. This "fix" is intended to keep the tool silently waiting when the proxy settings are ommitted (or other network issues are blocking in the NewProvider func)

@robbert229 robbert229 force-pushed the bugfix/handle-proxy branch 2 times, most recently from 29a3a62 to da28f34 Compare September 26, 2023 06:56
@leg100
Copy link
Owner

leg100 commented Sep 26, 2023

@robbert229 Thanks for this.

I've pushed a further change. I think the better approach is to wrap the standard library's http.DefaultTransport. The default not only respects proxy settings, but it looks like it has some default timeouts too, which may provide the OIDC provider timeout you desire. Please test this, I may well be wrong (there's lots of different types of timeouts involved).

@robbert229
Copy link
Contributor Author

robbert229 commented Sep 26, 2023

@leg100 Thanks, and good idea with wrapping http.DefaultTransport.

I am running the updated fix you provided, and it looks like everything is working quite nicely.

@leg100 leg100 merged commit c1ee8d8 into leg100:master Sep 26, 2023
5 checks passed
leg100 added a commit that referenced this pull request Oct 20, 2023
🤖 I have created a release *beep* *boop*
---


## [0.1.14](v0.1.13...v0.1.14)
(2023-10-19)


### Features

* github app: [#617](#617)
* always use latest terraform version
([#616](#616))
([83469ca](83469ca)),
closes [#608](#608)


### Bug Fixes

* error 'schema: converter not found for integration.manifest'
([e53ebf2](e53ebf2))
* fixed bug where proxy was ignored
([#609](#609))
([c1ee8d8](c1ee8d8))
* prevent modules with no published versions from crashing otf
([#611](#611))
([84aa299](84aa299))
* skip reporting runs created via API
([#622](#622))
([5d4527b](5d4527b)),
closes [#618](#618)


### Miscellaneous

* add note re cloud block to allow CLI apply
([4f03544](4f03544))
* remove unused exchange code response
([4a966cd](4a966cd))
* upgrade vulnerable markdown go mod
([781e0f6](781e0f6))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Louis Garman <75728+leg100@users.noreply.github.com>
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.

2 participants