Skip to content

added proxy support for build#2402

Merged
htuch merged 3 commits intoenvoyproxy:masterfrom
sudeeptoroy:proxy_support_build
Jan 23, 2018
Merged

added proxy support for build#2402
htuch merged 3 commits intoenvoyproxy:masterfrom
sudeeptoroy:proxy_support_build

Conversation

@sudeeptoroy
Copy link
Copy Markdown
Contributor

title: added support to build in a proxy env

Description:

it adds http_proxy env variables which can be used while building the image
user is expected to set http_proxy and https_proxy in the env before running the build command.

Risk Level: Low | Medium | High
Low

Testing:
tested in a proxy env

Signed-off-by: Sudeepto Roy <sudeepto.roy@gmail.com>
mkdir -p "${ENVOY_DOCKER_BUILD_DIR}"
# Since we specify an explicit hash, docker-run will pull from the remote repo if missing.
docker run --rm -t -i -u "${USER}":"${USER_GROUP}" -v "${ENVOY_DOCKER_BUILD_DIR}":/build \
docker run --rm -t -i -e http_proxy=${http_proxy} -e https_proxy=${https_proxy} -e HTTP_PROXY=${http_proxy} -e HTTPS_PROXY=${https_proxy} \
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for this fix. Why both upper and lower case proxy vars?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why set the lowercase then?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

git uses the lower case one

https://git-scm.com/docs/git-config#git-config-httpproxy

So one for git and another for bazel http/https files if defined.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One other though; can't we do this in Bazel? I.e. always use upper case, and only when we are invoking native git do the environment variable remap? This might be more Bazel plumbing than its worth.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We need to in inject the environment variable, the question is which one..
I tested once with lower case and once with upper case, both works.. we can get rid of one of them.. lower case is more famous so we can just keep that.. what do u suggest?

Signed-off-by: Sudeepto Roy <sudeepto.roy@gmail.com>
```bash
IMAGE_NAME=envoyproxy/envoy-build-centos ./ci/run_envoy_docker.sh './ci/do_ci.sh bazel.dev'
```
In case your setup is behind a proxy, set `http_proxy` and `https_proxy` to the proxy servers before invoking the build.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The example to add is:

IMAGE_NAME=envoyproxy/envoy-build-centos ./ci/run_envoy_docker.sh 'http_proxy=foo.com:8080 https_proxy=foo.com:4430 ./ci/do_ci.sh bazel.dev'

mkdir -p "${ENVOY_DOCKER_BUILD_DIR}"
# Since we specify an explicit hash, docker-run will pull from the remote repo if missing.
docker run --rm -t -i -u "${USER}":"${USER_GROUP}" -v "${ENVOY_DOCKER_BUILD_DIR}":/build \
docker run --rm -t -i -e http_proxy=${http_proxy} -e https_proxy=${https_proxy} \
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Did you determine that all caps is no longer needed by Bazel?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, I see, you say it works. Fine, SGTM.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

generally http_proxy and https_proxy would already be exported for such build environment. I added an example as you suggested.

Signed-off-by: Sudeepto Roy <sudeepto.roy@gmail.com>
@sudeeptoroy
Copy link
Copy Markdown
Contributor Author

sudeeptoroy commented Jan 22, 2018

@htuch
I don't have code to commit, not sure how do I start the review again.

this is my comments for the example you suggested.

IMAGE_NAME=envoyproxy/envoy-build-centos ./ci/run_envoy_docker.sh 'http_proxy=foo.com:8080 https_proxy=foo.com:4430 ./ci/do_ci.sh bazel.dev'
<<<<

I tested and found that what you have suggested fails.
env http_proxy and https_proxy needs to be set before run_envoy_docker.sh is run, so i had the change the example and setup env before the script is called.

Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Thanks!

@htuch htuch merged commit 5900423 into envoyproxy:master Jan 23, 2018
@sudeeptoroy sudeeptoroy deleted the proxy_support_build branch January 26, 2018 07:18
jpsim pushed a commit that referenced this pull request Nov 28, 2022
This upgrades `rules_foreign_cc` to a version including
bazel-contrib/rules_foreign_cc#938, which fixes
a build failure when the requested Apple SDK from `--xcode_version` does
not match the system's default Xcode's SDKs.

Example output:
```
rules_foreign_cc: Build failed!
rules_foreign_cc: Keeping temp build directory and dependencies directory for debug.
rules_foreign_cc: Please note that the directories inside a sandbox are still cleaned unless you specify --sandbox_debug Bazel command line flag.
rules_foreign_cc: Printing build logs:
_____ BEGIN BUILD LOGS _____
xcrun: error: SDK "macosx12.1" cannot be located
xcrun: error: SDK "macosx12.1" cannot be located
xcrun: error: unable to lookup item 'Path' in SDK 'macosx12.1'
+ XCODE_VERSION_OVERRIDE=13.2.1.13C100
+ APPLE_SDK_VERSION_OVERRIDE=12.1
+ APPLE_SDK_PLATFORM=MacOSX
```

This fixes a hermeticity problem in the build and is a prerequisite for
upgradting the macOS RE cluster to macOS 12, which in turn is a
requirement for upgrading to Xcode 13.4.

Progress on envoyproxy/envoy-mobile#2100

Signed-off-by: Yannic Bonenberger <yannic@engflow.com>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit that referenced this pull request Nov 29, 2022
This upgrades `rules_foreign_cc` to a version including
bazel-contrib/rules_foreign_cc#938, which fixes
a build failure when the requested Apple SDK from `--xcode_version` does
not match the system's default Xcode's SDKs.

Example output:
```
rules_foreign_cc: Build failed!
rules_foreign_cc: Keeping temp build directory and dependencies directory for debug.
rules_foreign_cc: Please note that the directories inside a sandbox are still cleaned unless you specify --sandbox_debug Bazel command line flag.
rules_foreign_cc: Printing build logs:
_____ BEGIN BUILD LOGS _____
xcrun: error: SDK "macosx12.1" cannot be located
xcrun: error: SDK "macosx12.1" cannot be located
xcrun: error: unable to lookup item 'Path' in SDK 'macosx12.1'
+ XCODE_VERSION_OVERRIDE=13.2.1.13C100
+ APPLE_SDK_VERSION_OVERRIDE=12.1
+ APPLE_SDK_PLATFORM=MacOSX
```

This fixes a hermeticity problem in the build and is a prerequisite for
upgradting the macOS RE cluster to macOS 12, which in turn is a
requirement for upgrading to Xcode 13.4.

Progress on envoyproxy/envoy-mobile#2100

Signed-off-by: Yannic Bonenberger <yannic@engflow.com>
Signed-off-by: JP Simard <jp@jpsim.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