Skip to content

bug fix: add missing proxies to the http request for the GitHub Installer #12900

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.

Sign up for GitHub

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

Jump to bottom
Merged
alexrashed merged 3 commits into from
Jul 25, 2025
Merged

bug fix: add missing proxies to the http request for the GitHub Installer #12900

alexrashed merged 3 commits into from
Jul 25, 2025

Conversation

@plafond
Copy link
Contributor

@plafond plafond commented Jul 24, 2025 •
edited
Loading

Ensure requests done with the GitHub Installer can resolve when an HTTP Proxy is used and configured via OUTBOUND_HTTP_PROXY / OUTBOUND_HTTPS_PROXY

Identified and tested with LocalStack pro by trying to install the EKS module via LPM.

Added 2nd / similar change (setting proxy on outbound request) - the impact in java.py appears more limited/non-breaking and impacts a check to help identify latest/specific jdk version to use.

Confirmed and validated 2nd change via LPM module install of non-default java version

@plafond
bug fix: add missing proxies to the http request for the GitHub insta…
9fafb7b 
…ller to ensure requests can resolve when an HTTP Proxy is used and configured via OUTBOUND_HTTP_PROXY / OUTBOUND_HTTPS_PROXY
localstack-bot
localstack-bot reviewed Jul 24, 2025
View reviewed changes
Copy link
Contributor

@localstack-bot localstack-bot left a comment

Choose a reason for hiding this comment

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

Welcome to LocalStack! Thanks for raising your first Pull Request and landing in your contributions. Our team will reach out with any reviews or feedbacks that we have shortly. We recommend joining our Slack Community and share your PR on the #community channel to share your contributions with us. Please make sure you are following our contributing guidelines and our Code of Conduct.

@localstack-bot
Copy link
Contributor

localstack-bot commented Jul 24, 2025 •
edited
Loading

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@github-actions
Copy link

github-actions bot commented Jul 24, 2025 •
edited
Loading

Test Results - Preflight, Unit

21 983 tests  ±0   20 249 ✅ ±0   6m 14s ⏱️ -11s
     1 suites ±0    1 734 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 7224188. ± Comparison against base commit 82db418.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Jul 24, 2025 •
edited
Loading

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 20s ⏱️ +5s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 7224188. ± Comparison against base commit 82db418.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Jul 24, 2025 •
edited
Loading

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   2h 20m 31s ⏱️
5 297 tests 4 367 ✅ 930 💤 0 ❌
5 303 runs  4 367 ✅ 936 💤 0 ❌

Results for commit 7224188.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Jul 24, 2025 •
edited
Loading

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 45m 47s ⏱️ + 3m 22s
4 938 tests +1  4 161 ✅ +1  777 💤 ±0  0 ❌ ±0 
4 940 runs  +1  4 161 ✅ +1  779 💤 ±0  0 ❌ ±0 

Results for commit 7224188. ± Comparison against base commit 82db418.

This pull request removes 1 and adds 2 tests. Note that renamed tests count towards both. 
tests.aws.services.s3.test_s3.TestS3 ‑ test_s3_object_expiry

tests.aws.services.cloudformation.resources.test_apigateway ‑ test_apigateway_deployment_canary_settings
tests.aws.services.s3.test_s3.TestS3 ‑ test_s3_object_expires

♻️ This comment has been updated with latest results.

@alexrashed alexrashed added the semver: patch Non-breaking changes which can be included in patch releases label Jul 24, 2025
@alexrashed alexrashed added this to the 4.7 milestone Jul 24, 2025
@alexrashed alexrashed self-assigned this Jul 24, 2025
@plafond
Copy link
Contributor Author

plafond commented Jul 24, 2025

I have read the CLA Document and I hereby sign the CLA

localstack-bot added a commit that referenced this pull request Jul 24, 2025
@localstack-bot
@plafond has signed the CLA in #12900
0521722
@plafond plafond added the review: merge when ready Signals to the reviewer that a PR can be merged if accepted label Jul 24, 2025
@plafond plafond marked this pull request as ready for review July 24, 2025 17:19
@plafond plafond requested a review from alexrashed as a code owner July 24, 2025 17:19
plafond added 2 commits July 24, 2025 14:02
@plafond
bug fix: similar/related to setting proxy on outbound request - this …
ce7e5af 
…appears to only impact check to look for which specific jdk version to use
@plafond
fix linting/formatting
7224188
alexrashed
alexrashed approved these changes Jul 25, 2025
View reviewed changes
Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Good catch! Thanks a lot for jumping on this! Unfortunately, this piece is pretty hard to properly test in an automated way. Let's just get this in the way it is, and let's try to enhance the test coverage in upcoming iterations! 💯

@alexrashed alexrashed merged commit 1775743 into main Jul 25, 2025
39 checks passed
@alexrashed alexrashed deleted the bug_proxy_github_installer branch July 25, 2025 05:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@localstack-bot localstack-bot localstack-bot left review comments

@alexrashed alexrashed alexrashed approved these changes

Assignees

@alexrashed alexrashed

Labels

review: merge when ready Signals to the reviewer that a PR can be merged if accepted semver: patch Non-breaking changes which can be included in patch releases

Projects

None yet

Milestone

4.7

Development

Successfully merging this pull request may close these issues.

3 participants

@plafond @localstack-bot @alexrashed