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

API Server permissions fix + set base compute capacity KANBAN-558 #51

Merged
merged 2 commits into from
May 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions .github/workflows/PR-validation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,6 @@ jobs:
uses: actions/setup-python@v5
with:
python-version: "3.12"
- name: CDK resource assertions (unit tests)
run: |
make run-unit-tests
- name: Python typing test
run: |
make run-python-type-check
Expand All @@ -258,7 +255,7 @@ jobs:
role-session-name: gh-actions-${{github.run_id}}.${{github.run_number}}.${{github.run_attempt}}-cdk-test
aws-region: us-east-1
- name: Validate production CDK stack code
run: cdk diff PaviPipelineCdkStack
run: make validate
api-code-checks:
name: API code checks
runs-on: ubuntu-22.04
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/main-build-and-deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ jobs:
- name: CDK validations (resource assertions and cdk diff)
run: make validate
- name: cdk deploy
run: cdk deploy PaviPipelineCdkStack --require-approval never
run: make deploy ADD_CDK_ARGS="--require-approval never"
api-deploy-image-repo:
name: Deploy/update container image repository stack for API
needs: [on-merge-and-deploy]
Expand Down
22 changes: 6 additions & 16 deletions api/aws_infra/cdk_classes/application_stack.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from aws_cdk import (
aws_elasticbeanstalk as eb,
aws_iam as iam,
aws_s3 as s3,
Stack,
Tags as cdk_tags
)
Expand Down Expand Up @@ -72,19 +71,10 @@ def __init__(self, scope: Construct, construct_id: str,
self, id='eb-service-role',
role_name='aws-elasticbeanstalk-service-role')

# Define permissions to read pipeline results
pipeline_nextflow_bucket = s3.Bucket.from_bucket_name(
self, id='pipeline-s3-bucket',
bucket_name='agr-pavi-pipeline-nextflow')

s3_bucket_access_statements = [
iam.PolicyStatement(
sid="S3BucketReadAll",
effect=iam.Effect.ALLOW,
actions=['s3:ListBucket*', 's3:Get*'],
resources=[pipeline_nextflow_bucket.bucket_arn, pipeline_nextflow_bucket.bucket_arn + '/*'])]

s3_pipeline_bucket_policy_doc = iam.PolicyDocument(statements=s3_bucket_access_statements)
# Assign permissions to execute the pipeline
pipeline_execution_policy = iam.ManagedPolicy.from_managed_policy_name(
self, 'pavi-pipeline-execution-policy',
managed_policy_name='agr-pavi-pipeline-nf-aws-execution-policy')

# Define role and instance profile
eb_ec2_role = iam.Role(
Expand All @@ -94,8 +84,8 @@ def __init__(self, scope: Construct, construct_id: str,
managed_policies=[
iam.ManagedPolicy.from_aws_managed_policy_name('AWSElasticBeanstalkWebTier'),
iam.ManagedPolicy.from_aws_managed_policy_name('CloudWatchAgentServerPolicy'),
iam.ManagedPolicy.from_managed_policy_name(self, "iam-ecr-read-policy", "ReadOnlyAccessECR")],
inline_policies={'read-pipeline-results': s3_pipeline_bucket_policy_doc})
iam.ManagedPolicy.from_managed_policy_name(self, "iam-ecr-read-policy", "ReadOnlyAccessECR"),
pipeline_execution_policy])

cdk_tags.of(eb_ec2_role).add("Product", "PAVI") # type: ignore
cdk_tags.of(eb_ec2_role).add("Managed_by", "PAVI") # type: ignore
Expand Down
11 changes: 10 additions & 1 deletion pipeline/aws_infra/Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
.PHONY: check-venv-active check-node

SUPPORTED_NODE := ^v18\.

PAVI_COMPUTE_MIN_VCPU ?= 2
ADD_CDK_ARGS ?=

check-venv-active:
ifeq ($(VIRTUAL_ENV),)
@echo 'No active python virtual environment found.'\
Expand Down Expand Up @@ -41,7 +45,12 @@ run-python-style-check: python-dependencies python-test-dependencies

validate: check-node run-unit-tests
# Validate production stack code
cdk diff PaviPipelineCdkStack
export PAVI_COMPUTE_MIN_VCPU=${PAVI_COMPUTE_MIN_VCPU} && \
cdk diff PaviPipelineCdkStack

validate-dev: check-venv-active validate
@:

deploy:
export PAVI_COMPUTE_MIN_VCPU=${PAVI_COMPUTE_MIN_VCPU} && \
cdk deploy PaviPipelineCdkStack ${ADD_CDK_ARGS}
7 changes: 6 additions & 1 deletion pipeline/aws_infra/cdk_classes/aws_batch.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,12 @@
Tags as cdk_tags
)

from os import getenv
from typing import Optional

min_compute_vcpu: int = int(getenv('PAVI_COMPUTE_MIN_VCPU', 0))
'''Minimum vCPU capacity to maintain in the compute environment when inactive.'''


class PaviExecutionEnvironment:

Expand Down Expand Up @@ -95,7 +99,8 @@ def __init__(self, scope: Stack, env_suffix: str, shared_work_dir_bucket: Option
instance_role=instance_role, # type: ignore
spot=False,
terminate_on_update=False, update_timeout=Duration.minutes(10),
update_to_latest_image_version=True)
update_to_latest_image_version=True,
minv_cpus=min_compute_vcpu)

self.compute_environment.apply_removal_policy(RemovalPolicy.DESTROY)

Expand Down
3 changes: 2 additions & 1 deletion pipeline/aws_infra/tests/unit/test_cdk_infra_stack.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ def test_pipeline_nf_s3_bucket() -> None:


# The Nextflow AWS execution policy is assigned to the GH-actions role for PAVI, in addition to PAVI-managed resources.
# Changing the name of this policy might require the gh-actions role to be updated to include the new role.
# Changing the name of this policy might require the gh-actions role to be updated to include the new role, along
# with changing all references to the role in other PAVI CDK stacks.
def test_pipeline_nf_s3_bucket_policy() -> None:
template.has_resource(type=ResourceType.of('AWS::IAM::ManagedPolicy').compliance_resource_type, props={
"Properties": {
Expand Down