marcoabreu commented on a change in pull request #19654:
URL: https://github.com/apache/incubator-mxnet/pull/19654#discussion_r541185111
##########
File path: ci/build.py
##########
@@ -117,6 +123,9 @@ def run_cmd():
image_id = _get_local_image_id(docker_tag=tag)
if not image_id:
raise FileNotFoundError('Unable to find docker image id matching with
{}'.format(tag))
+ # now that we've built the container, push it to our docker cache if
DOCKER_ECR_CACHE is defined
+ if 'DOCKER_ECR_REGISTRY' in os.environ:
+ push_docker_cache(registry, tag, image_id)
Review comment:
The issue right now though is that with this design, you are granting
regular untrusted slaves write-access to ECR. Unless you introduce another ECR
repository for restricted jobs, this could be leveraged to modify the cache
which is used by restricted jobs to inject malware into these jobs - e.g. when
generating published artifacts.
Thus I'd recommend to either create two separate ECR repositories and only
grant the appropriate Instance Profile the according permission or revert back
to having specific publish jobs.
With ccache, we've followed exactly that model. The EFS used for untrusted
slaves is a different one than used for the restricted slaves.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
