Feature: Support dependencies accessed over SSH in containerized builds

[Macok]

Which issue(s) does this change fix?

#1893

Why is this change necessary?

This change is needed to support git dependencies accessed over SSH, for builds executed inside docker containers (--use-container, or -u option).

How does it address the issue?

This PR adds --container-dir-mount option to SAM interface, allowing the user to mount arbitrary directories into container where the build is executed. Having this option available, the user can forward host's SSH configuration into the container by mounting $SSH_AUTH_SOCK and known_hosts files:

sam build -u --container-dir-mount $SSH_AUTH_SOCK:/ssh-agent -e SSH_AUTH_SOCK=/ssh-agent --container-dir-mount ~/.ssh/known_hosts:/etc/ssh/ssh_known_hosts

This functionality integrates easily with Github Actions. You can configure SSH on the GA worker using this plugin: webfactory/ssh-agent. Then forward worker's SSH configuration to the container as shown above.

What side effects does this change have?

None, fully backward compatible

Checklist

• [x] Add input/output type hints to new functions/methods
• [ ] Write design document (Do I need to write a design document?)
• [x] Write unit tests
• [x] Write/update functional tests
• [x] Write/update integration tests
• [x] make pr passes
• [x] make update-reproducible-reqs if dependencies were changed
• [ ] Write documentation

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[aodhan-domhnaill]

@jfuss Can we get this reviewed? It would be useful for us. Thanks!

[pkspks]

This is useful for building with git repo dependencies in requirements.txt. Can not build in a container without this.

[braaibander]

I would really like to see this functionality implemented We have private repos and varing python versions, this is a real problem for us

[aaronlelevier]

+1 running into the same issue

[qingchm]

Generally I really like how detailed your changes are, it really respects some of the conventions used in our codebase! Design-wise left some comments above! Excited to help you push this through

[Macok]

@qingchm Thanks for your review. I spent some time to resolve conflicts that arose between my branch and develop. Regarding your comments:

• Using pathvalidate library: I removed this lib and used simple regexes as you suggested.
• Choosing different name for --container-dir-mount option: Did you talk to the UX team? I'm not a native speaker so I will just take what they suggest.
• Supporting function specific mounts like Function1=/host/path:/container/path: For now I just put a TODO in app_builder.py. I think this use case would be very rare and I'd rather keep this PR small to increase chances of merging it. But if you think this is a must have I will try to find some time to work on it.

[Macok]

@qingchm, @hawflau Did you have some time to look into this?

[mndeveci]

@qingchm, @hawflau Did you have some time to look into this?

Thanks for this PR @Macok, I've updated your branch with latest from develop. I will check with @qingchm to see UX feedback.

[Macok]

Thanks for this PR @Macok, I've updated your branch with latest from develop. I will check with @qingchm to see UX feedback.

Thanks @mndeveci , let me know once there are any news!

[tniemann]

any progress here?

[moelasmar]

Thanks @Macok for your contribution. I updated your branch with the latest changes from the develop branch.

We need to ask our security team feedback about this change. We will initiate the review process, and update this PR with the updates.

[NancyMakar]

can we get updates on this please?

[jfuss]

Providing an update:

We are still working with our security team. Given re:invent and holidays, it's a tough time. This is something we are still trying to drive, but the process is just taking longer than anticipated. Unfortunately, I don't have anything more concrete to share at the moment or an ETA. We will share once we can. Thanks for understanding.

[sriram-mv]

After looking into this PR a bit more as we started on the security review. One way to go about this without having to adding new options and flags to AWS SAM CLI is to use the powerful --build-image option with sam build. A Dockerfile can be created and built with --ssh flag and the local image can then be supplied to sam build --build-image mybuildimage:latest

[sriram-mv]

Closing this PR based on the latest comment. Will look into adding an example here.

[Jonathan-Landeed]

@sriram-mv isn't --ssh for exposing ssh keys during the docker buildx build process? The keys will not be available when sam build installs the requirements into the resulting container. I suppose it might be possible by installing all requirements into the docker image and then using --cached, but that would be a pain.

Most networking or volume mounting tricks I can think of would be done when starting the container (which sam controls), not building the image.

The only thing I can think of is to bundle the ssh keys like so:

FROM public.ecr.aws/sam/build-python3.10:latest-x86_64

RUN ssh-keyscan github.com > /etc/ssh/ssh_known_hosts

COPY deploy_key /root/.ssh/deploy_key

RUN chmod 600 /root/.ssh/deploy_key && \
    chmod 700 /root/.ssh

RUN echo "    IdentityFile /root/.ssh/deploy_key" >> /etc/ssh/ssh_config

Not ideal from a security standpoint...