Skip to content
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.

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

[aws-lambda-python] Allow the use of CodeArtifact #10298

Closed
mrpackethead opened this issue Sep 10, 2020 · 12 comments · Fixed by #18082
Closed

[aws-lambda-python] Allow the use of CodeArtifact #10298

mrpackethead opened this issue Sep 10, 2020 · 12 comments · Fixed by #18082
Labels
@aws-cdk/aws-lambda-python effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Comments

@mrpackethead
Copy link

mrpackethead commented Sep 10, 2020

Allow the use of CodeArtifact, so that private modules can be loaded from a private repository

Use Case

Many librarys are private, which get used to build lambdas

Proposed Solution

pass parameters for the code-artifact repo to the container.

This is a 🚀 Feature Request

@mrpackethead mrpackethead added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Sep 10, 2020
@PierreKiwi
Copy link

Thanks @mrpackethead (I suggest you change from code-artifact to CodeArtifact ;)).

The issue #9942 seems to be related as well.

To reiterate what I have shared on Slack, I have solved the problem with Function by first configuring pip using the login command and then I mounted a volume to the Docker container to make my pip.confg available during the build.

my_lambda = aws_lambda.Function(
    self,
    "MyLambdaFunction",
    code=aws_lambda.Code.from_asset(
        "path/to/lambda",
        asset_hash_type=core.AssetHashType.SOURCE,
        bundling=core.BundlingOptions(
            image=aws_lambda.Runtime.PYTHON_3_8.bundling_docker_image,
            command=[
                "bash",
                "-c",
                " && ".join(
                    [
                        "pip install -r requirements.txt -t /asset-output",
                        "cp -au . /asset-output",
                    ]
                ),
            ],
            user="root:root",
            volumes=[
                core.DockerVolume(
                    container_path="/root/.config/pip/pip.conf",
                    host_path=f"{Path.home()}/.config/pip/pip.conf",
                ),
            ],
        ),
    ),
)

@mrpackethead
Copy link
Author

mrpackethead commented Sep 11, 2020

maybe this just needs a way to provide a pip config ( it could sit next to the requirements.txt ) to the bundling container.

@SomayaB SomayaB changed the title aws-lambda-python [aws-lambda-python] Allow the use of CodeArtifact Sep 11, 2020
@eladb eladb added the effort/medium Medium work item – several days of effort label Sep 16, 2020
@eladb
Copy link
Contributor

eladb commented Sep 16, 2020

@adamelmore thoughts?

@eladb eladb added the p2 label Sep 16, 2020
@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Sep 16, 2020
@adamdottv
Copy link
Contributor

@adamelmore thoughts?

@eladb Makes sense to me! I'll wait until #9582 lands as it makes sweeping changes to PythonFunction. Then I should have some bandwidth to take this on.

@lucasam
Copy link

lucasam commented Dec 2, 2020

👍 Just add one more customer interested on this feature

@eikeon
Copy link

eikeon commented Jan 16, 2021

@adamelmore thoughts?

@eladb Makes sense to me! I'll wait until #9582 lands as it makes sweeping changes to PythonFunction. Then I should have some bandwidth to take this on.

I believe #9582 has landed. Looking forward to this functionality. TY, all for the work on the CDK

@mrpackethead
Copy link
Author

adding to that, I've changed jobs, but the requirment has'tn chagned... @PierreKiwi 's work ( above ) is helpful but it woudl be nice to do this more tiderly.

@mrpackethead
Copy link
Author

Related to this ticket is

#11296

I posted a workaround detailing how to use your own ECR

@sumpfork
Copy link

To note, we hacked around the lack of direct support a little differently: we rewrite the requirements file from a template at deployment time and insert the correct codeartifact pypi index via --index-url <codeartifact_url> at the top before using it during lambda packaging in docker.

@eladb eladb removed their assignment Feb 25, 2021
@setu4993
Copy link
Contributor

setu4993 commented Jun 26, 2021

I started off with #15306 to really address this, but only later realized it wouldn't be enough to add support for buildArgs to the construct, and that it'll also need an updated build image.

One potential alternative is to allow specifying a custom build image specifically for the Python Lambdas, and pass in CodeArtifact auth using build args. That requires being able to set args in the build image. This type of a setup would also allow using the various packaging tools like poetry. However, it does mean the build image would be duplicated in the local project, separate from the one here (but derived from it).

More concretely, what I'm suggesting is:

a) A build Dockerfile like:

# The correct AWS SAM build image based on the runtime of the function will be
# passed as build arg. The default allows to do `docker build .` when testing.
ARG IMAGE=public.ecr.aws/sam/build-python3.7
FROM $IMAGE

# <-- additional args begin
ARG POETRY_HTTP_BASIC_CODEARTIFACT_USERNAME
ARG POETRY_HTTP_BASIC_CODEARTIFACT_PASSWORD
# additional args end -->

# Ensure rsync is installed
RUN yum -q list installed rsync &>/dev/null || yum install -y rsync

# Upgrade pip (required by cryptography v3.4 and above, which is a dependency of poetry)
RUN pip install --upgrade pip

# Install pipenv and poetry so we can create a requirements.txt if we detect pipfile or poetry.lock respectively
RUN pip install pipenv poetry

# Install the dependencies in a cacheable layer
WORKDIR /var/dependencies
COPY Pipfile* pyproject* poetry* requirements.tx[t] ./
RUN [ -f 'Pipfile' ] && pipenv lock -r >requirements.txt; \
    [ -f 'poetry.lock' ] && poetry export --with-credentials --format requirements.txt --output requirements.txt; \
    [ -f 'requirements.txt' ] && pip install -r requirements.txt -t .;

CMD [ "python" ]

b) Then, the auth could be passed in from local env vars or by running the subprocess command inline into the PythonFunction as:

new PythonFunction(this, 'MyFunction', {
  entry: '/path/to/my/function', // required
  index: 'my_index.py', // optional, defaults to 'index.py'
  handler: 'my_exported_func', // optional, defaults to 'handler'
  buildDockerfile: "/path/to/local/build/Dockerfile",
  buildArgs: {
        POETRY_HTTP_BASIC_CODEARTIFACT_USERNAME: process.env.POETRY_HTTP_BASIC_CODEARTIFACT_USERNAME!,
        POETRY_HTTP_BASIC_CODEARTIFACT_PASSWORD: process.env.POETRY_HTTP_BASIC_CODEARTIFACT_PASSWORD!
  }
});

This would also make it more inline with the NodeJsFunction, which allows passing in a build image that is different from the default one.

This also allows flexibility in how args are passed in and interpreted without tying it to a single packaging solution given there's various possible with Python.

I'm happy to take a stab at implementing this to build on what I started in #15306.

@setu4993
Copy link
Contributor

I created a draft PR here to start an implementation: #15324

I simplified my changes suggested above to reuse cdk.DockerImage instead. So, the function call would be similar to:

new PythonFunction(this, 'MyFunction', {
  entry: '/path/to/my/function', // required
  index: 'my_index.py', // optional, defaults to 'index.py'
  handler: 'my_exported_func', // optional, defaults to 'handler'
  buildImage: cdk.DockerImage.fromBuild("/path/to/file", {
    file: "Dockerfile.build",
    buildArgs: {
        POETRY_HTTP_BASIC_CODEARTIFACT_USERNAME: process.env.POETRY_HTTP_BASIC_CODEARTIFACT_USERNAME!,
        POETRY_HTTP_BASIC_CODEARTIFACT_PASSWORD: process.env.POETRY_HTTP_BASIC_CODEARTIFACT_PASSWORD!
    }
  }
});

@mergify mergify bot closed this as completed in #18082 Dec 30, 2021
mergify bot pushed a commit that referenced this issue Dec 30, 2021
…mage (#18082)

This refactors the bundling process to match the NodeJs and Go Lambda functions and allows providing a custom bundling docker image.

Changes:
- refactor bundling to use `cdk.BundlingOptions`
- Use updated `Bundling` class
- Update tests to use updated `Bundling` class


Fixes #10298, #12949, #15391, #16234, #15306

BREAKING CHANGE: `assetHashType` and `assetHash` properties moved to new `bundling` property.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022
…mage (aws#18082)

This refactors the bundling process to match the NodeJs and Go Lambda functions and allows providing a custom bundling docker image.

Changes:
- refactor bundling to use `cdk.BundlingOptions`
- Use updated `Bundling` class
- Update tests to use updated `Bundling` class


Fixes aws#10298, aws#12949, aws#15391, aws#16234, aws#15306

BREAKING CHANGE: `assetHashType` and `assetHash` properties moved to new `bundling` property.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-lambda-python effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
9 participants