Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

[v1.x] For ECR, ensure we sanitize region input from environment variable #19882

Merged
merged 2 commits into from
Feb 11, 2021
Merged

[v1.x] For ECR, ensure we sanitize region input from environment variable #19882

merged 2 commits into from
Feb 11, 2021

Conversation

josephevans
Copy link
Contributor

Description

  • Sanitize the region input that we extract from an environment variable to prevent command injection.
  • Set a default value for recently introduced cache_intermediate.
  • If using ECR, don't throw an exception if dockerhub environment variables are not set
  • Track ECR login using a global variable, so we only have to retrieve login credentials once

@mxnet-bot
Copy link

Hey @josephevans , Thanks for submitting the PR
All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands:

  • To trigger all jobs: @mxnet-bot run ci [all]
  • To trigger specific jobs: @mxnet-bot run ci [job1, job2]

CI supported jobs: [website, unix-cpu, centos-gpu, sanity, windows-gpu, edge, miscellaneous, windows-cpu, unix-gpu, centos-cpu, clang]

Note:
Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin.
All CI tests must pass before the PR can be merged.

@lanking520 lanking520 added the pr-awaiting-testing PR is reviewed and waiting CI build and test label Feb 11, 2021
@lanking520 lanking520 added pr-awaiting-merge Review and CI is complete. Ready to Merge and removed pr-awaiting-testing PR is reviewed and waiting CI build and test labels Feb 11, 2021
@sandeep-krishnamurthy sandeep-krishnamurthy merged commit 0138c29 into apache:v1.x Feb 11, 2021
@josephevans josephevans deleted the ecr_v1.x branch February 11, 2021 17:50
josephevans added a commit to josephevans/mxnet that referenced this pull request Feb 24, 2021
@josephevans
[v1.x] For ECR, ensure we sanitize region input from environment vari…
e81d2b9 
…able (apache#19882)

* Set default for cache_intermediate.

* Make sure we sanitize region extracted from registry, since we pass it to os.system.

Co-authored-by: Joe Evans <[email protected]>
@josephevans josephevans mentioned this pull request Feb 24, 2021
Closed
Zha0q1 added a commit that referenced this pull request Mar 2, 2021
@Zha0q1 @josephevans @ptrendx
[V1.8.x] Attemp to fix cd for v1.8.x (#19947)
a03c509 
* [v1.x] Migrate to use ECR as docker cache instead of dockerhub (#19654)

* [v1.x] Update CI build scripts to install python 3.6 from deadsnakes repo (#19788)

* Install python3.6 from deadsnakes repo, since 3.5 is EOL'd and get-pip.py no longer works with 3.5.

* Set symlink for python3 to point to newly installed 3.6 version.

* Setting symlink or using update-alternatives causes add-apt-repository to fail, so instead just set alias in environment to call the correct python version.

* Setup symlinks in /usr/local/bin, since it comes first in the path.

* Don't use absolute path for python3 executable, just use python3 from path.

Co-authored-by: Joe Evans <[email protected]>

* Disable unix-gpu-cu110 pipeline for v1.x build since we now build with cuda 11.0 in windows pipelines. (#19828)

Co-authored-by: Joe Evans <[email protected]>

* [v1.x] For ECR, ensure we sanitize region input from environment variable (#19882)

* Set default for cache_intermediate.

* Make sure we sanitize region extracted from registry, since we pass it to os.system.

Co-authored-by: Joe Evans <[email protected]>

* [v1.x] Address CI failures with docker timeouts (v2) (#19890)

* Add random sleep only, since retry attempts are already implemented.

* Reduce random sleep to 2-10 sec.

Co-authored-by: Joe Evans <[email protected]>

* [v1.x] CI fixes to make more stable and upgradable (#19895)

* Test moving pipelines from p3 to g4.

* Remove fallback codecov command - the existing (first) command works and the second always fails a few times before finally succeeding (and also doesn't support the -P parameter, which causes an error.)

* Stop using docker python client, since it still doesn't support latest nvidia 'gpus' attribute. Switch to using subprocess calls using list parameter (to avoid shell injections).

See docker/docker-py#2395

* Remove old files.

* Fix comment

* Set default environment variables

* Fix GPU syntax.

* Use subprocess.run and redirect output to stdout, don't run docker in interactive mode.

* Check if codecov works without providing parameters now.

* Send docker stderr to sys.stderr

* Support both nvidia-docker configurations, first try '--gpus all', and if that fails, then try '--runtime nvidia'.

Co-authored-by: Joe Evans <[email protected]>

* fix cd

* fix cudnn version for cu10.2 buiuld

* WAR the dataloader issue with forked processes holding stale references (#19924)

* skip some tests

* fix ski[

* [v.1x] Attempt to fix v1.x cd by installing new cuda compt package (#19959)

* update cude compt for cd

* Update Dockerfile.build.ubuntu_gpu_cu102

* Update Dockerfile.build.ubuntu_gpu_cu102

* Update Dockerfile.build.ubuntu_gpu_cu110

* Update runtime_functions.sh

* Update Dockerfile.build.ubuntu_gpu_cu110

* Update Dockerfile.build.ubuntu_gpu_cu102

* update command

Co-authored-by: Joe Evans <[email protected]>
Co-authored-by: Joe Evans <[email protected]>
Co-authored-by: Joe Evans <[email protected]>
Co-authored-by: Przemyslaw Tredak <[email protected]>
mseth10 pushed a commit to mseth10/incubator-mxnet that referenced this pull request Mar 15, 2021
@Zha0q1 @josephevans
@ptrendx @mseth10
[V1.8.x] Attemp to fix cd for v1.8.x (apache#19947)
e203f03 
* [v1.x] Migrate to use ECR as docker cache instead of dockerhub (apache#19654)

* [v1.x] Update CI build scripts to install python 3.6 from deadsnakes repo (apache#19788)

* Install python3.6 from deadsnakes repo, since 3.5 is EOL'd and get-pip.py no longer works with 3.5.

* Set symlink for python3 to point to newly installed 3.6 version.

* Setting symlink or using update-alternatives causes add-apt-repository to fail, so instead just set alias in environment to call the correct python version.

* Setup symlinks in /usr/local/bin, since it comes first in the path.

* Don't use absolute path for python3 executable, just use python3 from path.

Co-authored-by: Joe Evans <[email protected]>

* Disable unix-gpu-cu110 pipeline for v1.x build since we now build with cuda 11.0 in windows pipelines. (apache#19828)

Co-authored-by: Joe Evans <[email protected]>

* [v1.x] For ECR, ensure we sanitize region input from environment variable (apache#19882)

* Set default for cache_intermediate.

* Make sure we sanitize region extracted from registry, since we pass it to os.system.

Co-authored-by: Joe Evans <[email protected]>

* [v1.x] Address CI failures with docker timeouts (v2) (apache#19890)

* Add random sleep only, since retry attempts are already implemented.

* Reduce random sleep to 2-10 sec.

Co-authored-by: Joe Evans <[email protected]>

* [v1.x] CI fixes to make more stable and upgradable (apache#19895)

* Test moving pipelines from p3 to g4.

* Remove fallback codecov command - the existing (first) command works and the second always fails a few times before finally succeeding (and also doesn't support the -P parameter, which causes an error.)

* Stop using docker python client, since it still doesn't support latest nvidia 'gpus' attribute. Switch to using subprocess calls using list parameter (to avoid shell injections).

See docker/docker-py#2395

* Remove old files.

* Fix comment

* Set default environment variables

* Fix GPU syntax.

* Use subprocess.run and redirect output to stdout, don't run docker in interactive mode.

* Check if codecov works without providing parameters now.

* Send docker stderr to sys.stderr

* Support both nvidia-docker configurations, first try '--gpus all', and if that fails, then try '--runtime nvidia'.

Co-authored-by: Joe Evans <[email protected]>

* fix cd

* fix cudnn version for cu10.2 buiuld

* WAR the dataloader issue with forked processes holding stale references (apache#19924)

* skip some tests

* fix ski[

* [v.1x] Attempt to fix v1.x cd by installing new cuda compt package (apache#19959)

* update cude compt for cd

* Update Dockerfile.build.ubuntu_gpu_cu102

* Update Dockerfile.build.ubuntu_gpu_cu102

* Update Dockerfile.build.ubuntu_gpu_cu110

* Update runtime_functions.sh

* Update Dockerfile.build.ubuntu_gpu_cu110

* Update Dockerfile.build.ubuntu_gpu_cu102

* update command

Co-authored-by: Joe Evans <[email protected]>
Co-authored-by: Joe Evans <[email protected]>
Co-authored-by: Joe Evans <[email protected]>
Co-authored-by: Przemyslaw Tredak <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@marcoabreu marcoabreu marcoabreu approved these changes

@aaronmarkham aaronmarkham Awaiting requested review from aaronmarkham aaronmarkham is a code owner

Assignees
No one assigned
Labels
pr-awaiting-merge Review and CI is complete. Ready to Merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@josephevans @mxnet-bot @marcoabreu @sandeep-krishnamurthy @lanking520