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

feat: add synth & kwargs #288

Merged
merged 4 commits into from
Feb 3, 2025
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ This project adheres to [Semantic Versioning](http://semver.org/) and [Keep a Ch
### New

- add permissions boundary to seedkit CodeBuild IAM role
- add `--synth` option to `deploy_seedkit` CLI
- add `**kwargs` to `deploy_seedkit` API

### Changes

Expand Down
10 changes: 10 additions & 0 deletions aws_codeseeder/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,14 @@ def destroy() -> None:
required=False,
default=None,
)
@click.option(
"--synth/--no-synth",
type=bool,
default=False,
help="Synthesize seedkit template only. Do not deploy",
required=False,
show_default=True,
)
@click.option(
"--debug/--no-debug",
default=False,
Expand All @@ -129,6 +137,7 @@ def deploy_seedkit(
subnet_id: Tuple[str, ...],
sg_id: Tuple[str, ...],
permissions_boundary_arn: Optional[str],
synth: bool,
) -> None:
if debug:
set_log_level(level=logging.DEBUG, format=DEBUG_LOGGING_FORMAT)
Expand All @@ -144,6 +153,7 @@ def deploy_seedkit(
subnet_ids=[s for s in subnet_id],
security_group_ids=[sg for sg in sg_id],
permissions_boundary_arn=permissions_boundary_arn,
synthesize=synth,
)


Expand Down
43 changes: 24 additions & 19 deletions aws_codeseeder/_cfn_seedkit.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,16 @@
import os
import random
import string
import sys
from string import Template
from typing import Callable, List, Optional, Union
from typing import Any, Callable, Dict, List, Optional, Union

import yaml
from boto3 import Session
from cfn_flip import yaml_dumper
from cfn_tools import load_yaml

from aws_codeseeder import CLI_ROOT, LOGGER, create_output_dir
from aws_codeseeder.services import get_region, get_sts_info

FILENAME = "template.yaml"
RESOURCES_FILENAME = os.path.join(CLI_ROOT, "resources", FILENAME)
Expand All @@ -41,10 +41,13 @@ def synth(
subnet_ids: Optional[List[str]] = None,
security_group_ids: Optional[List[str]] = None,
permissions_boundary_arn: Optional[str] = None,
) -> str:
synthesize: bool = False,
**kwargs: Dict[str, Any],
) -> Optional[str]:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this function doesn't have a docstring right now, but I think it would be good to add one while we're here.

I'm skeptical on using kwargs here -- it is two extra optional parameters, but they're completely undocumented. Why not just add role_prefix and policy_prefix?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intended. kwargs is an escape hatch and would allow us to pass any other parameters from Seed-Farmer (or any other tool) in the future without breaking backward-compatibility.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For what value? To decouple seed-farmer and code-seeder versions slightly?

Copy link
Contributor Author

@kukushking kukushking Jan 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep

deploy_id = deploy_id if deploy_id else "".join(random.choice(string.ascii_lowercase) for i in range(6))
out_dir = create_output_dir(f"seedkit-{deploy_id}")
output_filename = os.path.join(out_dir, FILENAME)
kwargs = {} if kwargs is None else kwargs

LOGGER.debug("Reading %s", RESOURCES_FILENAME)
with open(RESOURCES_FILENAME, "r") as file:
Expand All @@ -66,21 +69,23 @@ def synth(
if permissions_boundary_arn:
input_template["Resources"]["CodeBuildRole"]["Properties"]["PermissionsBoundary"] = permissions_boundary_arn

output_template = Template(yaml.dump(input_template, Dumper=yaml_dumper.get_dumper()))
role_prefix = kwargs.get("role_prefix", "/")
policy_prefix = kwargs.get("policy_prefix", "/")

LOGGER.debug("Writing %s", output_filename)
os.makedirs(out_dir, exist_ok=True)
account_id, _, partition = get_sts_info(session=session)
with open(output_filename, "w") as file:
file.write(
output_template.safe_substitute(
seedkit_name=seedkit_name,
account_id=account_id,
region=get_region(session=session),
partition=partition,
deploy_id=deploy_id,
role_prefix="/",
)
)
output_template = Template(yaml.dump(input_template, Dumper=yaml_dumper.get_dumper()))
template = output_template.safe_substitute(
seedkit_name=seedkit_name,
deploy_id=deploy_id,
role_prefix=role_prefix,
policy_prefix=policy_prefix,
)

return output_filename
if not synthesize:
LOGGER.debug("Writing %s", output_filename)
os.makedirs(out_dir, exist_ok=True)
with open(output_filename, "w") as file:
file.write(template)
return output_filename
else:
sys.stdout.write(template)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this get mixed in with log output? Is that OK?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redirecting this into a file i.e:

codeseeder deploy seedkit test --synth > test.yaml

Doesn't mix it up with the log output and produces a valid template

return None
22 changes: 16 additions & 6 deletions aws_codeseeder/commands/_seedkit_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from typing import Callable, Dict, List, Optional, Tuple, Union
from typing import Any, Callable, Dict, List, Optional, Tuple, Union, cast

from boto3 import Session

Expand Down Expand Up @@ -55,6 +55,8 @@ def deploy_seedkit(
subnet_ids: Optional[List[str]] = None,
security_group_ids: Optional[List[str]] = None,
permissions_boundary_arn: Optional[str] = None,
synthesize: bool = False,
**kwargs: Dict[str, Any],
) -> None:
"""Deploys the seedkit resources into the environment.

Expand Down Expand Up @@ -85,6 +87,8 @@ def deploy_seedkit(
security_group_ids: Optional[List[str]]
If deploying codebuild in a VPC, a list of Security Group IDs to use
(must have vpc-id, subnets, and security_group_ids)
synthesize: bool
Synthesize seedkit template only. Do not deploy. False by default.
"""
deploy_id: Optional[str] = None
stack_exists, stack_name, stack_outputs = seedkit_deployed(seedkit_name=seedkit_name, session=session)
Expand All @@ -96,7 +100,7 @@ def deploy_seedkit(
if stack_exists:
deploy_id = stack_outputs.get("DeployId")
LOGGER.info("Seedkit found with DeployId: %s", deploy_id)
template_filename: str = _cfn_seedkit.synth(
template_filename: Optional[str] = _cfn_seedkit.synth(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make more sense to have synth return the string template instead of the path?

Right now there's a coupling that synthesize=False changes the output value. That looks like quite a refactor though, so maybe it isn't worth it. Probably just add an assertion under if not synthesize that template_filename is non-None?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I considered refactoring this but did not want to make too intrusive changes. Good call about an assertion though.

seedkit_name=seedkit_name,
deploy_id=deploy_id,
managed_policy_arns=managed_policy_arns,
Expand All @@ -106,11 +110,17 @@ def deploy_seedkit(
subnet_ids=subnet_ids,
security_group_ids=security_group_ids,
permissions_boundary_arn=permissions_boundary_arn,
synthesize=synthesize,
**kwargs,
)
cfn.deploy_template(
stack_name=stack_name, filename=template_filename, seedkit_tag=f"codeseeder-{seedkit_name}", session=session
)
LOGGER.info("Seedkit Deployed")
if not synthesize:
cfn.deploy_template(
stack_name=stack_name,
filename=cast(str, template_filename),
seedkit_tag=f"codeseeder-{seedkit_name}",
session=session,
)
LOGGER.info("Seedkit Deployed")


def destroy_seedkit(seedkit_name: str, session: Optional[Union[Callable[[], Session], Session]] = None) -> None:
Expand Down
44 changes: 22 additions & 22 deletions aws_codeseeder/resources/template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Resources:
Bucket:
Type: AWS::S3::Bucket
Properties:
BucketName: codeseeder-${seedkit_name}-${account_id}-${deploy_id}
BucketName: !Sub codeseeder-${seedkit_name}-${AWS::AccountId}-${deploy_id}
Tags:
- Key: codeseeder-seedkit-name
Value: codeseeder-${seedkit_name}
Expand Down Expand Up @@ -63,8 +63,8 @@ Resources:
Type: AWS::IAM::ManagedPolicy
Properties:
Description: Managed Policy granting access to the AWS CodeSeeder resources
ManagedPolicyName: codeseeder-${seedkit_name}-${region}-resources
Path: /
ManagedPolicyName: !Sub codeseeder-${seedkit_name}-${AWS::Region}-resources
Path: ${policy_prefix}
PolicyDocument:
Statement:
- Effect: Allow
Expand All @@ -75,38 +75,38 @@ Resources:
- codebuild:BatchPutTestCases
- codebuild:BatchPutCodeCoverages
Resource:
- arn:${partition}:codebuild:${region}:${account_id}:*
- !Sub arn:${AWS::Partition}:codebuild:${AWS::Region}:${AWS::AccountId}:*
- Effect: Allow
Action:
- codebuild:StartBuild
- codebuild:BatchGetBuilds
Resource:
- arn:${partition}:codebuild:${region}:${account_id}:project/codeseeder-${seedkit_name}
- !Sub arn:${AWS::Partition}:codebuild:${AWS::Region}:${AWS::AccountId}:project/codeseeder-${seedkit_name}
- Effect: Allow
Action:
- ssm:Get*
- ssm:Describe*
Resource:
- arn:${partition}:ssm:${region}:${account_id}:parameter/codeseeder*
- !Sub arn:${AWS::Partition}:ssm:${AWS::Region}:${AWS::AccountId}:parameter/codeseeder*
- Effect: Allow
Action:
- ssm:PutParameter
- ssm:AddTagsToResource
- ssm:DeleteParameter
- ssm:DeleteParameters
Resource:
- arn:${partition}:ssm:${region}:${account_id}:parameter/codeseeder/${seedkit_name}/*
- !Sub arn:${AWS::Partition}:ssm:${AWS::Region}:${AWS::AccountId}:parameter/codeseeder/${seedkit_name}/*
- Effect: Allow
Action:
- ssm:DescribeParameters
Resource:
- arn:${partition}:ssm:${region}:${account_id}:*
- !Sub arn:${AWS::Partition}:ssm:${AWS::Region}:${AWS::AccountId}:*
- Effect: Allow
Action:
- kms:*
Resource:
- arn:${partition}:kms:${region}:${account_id}:alias/codeseeder-${seedkit_name}*
- arn:${partition}:kms:${region}:${account_id}:key/*
- !Sub arn:${AWS::Partition}:kms:${AWS::Region}:${AWS::AccountId}:alias/codeseeder-${seedkit_name}*
- !Sub arn:${AWS::Partition}:kms:${AWS::Region}:${AWS::AccountId}:key/*
- Effect: Allow
Action:
- sts:GetServiceBearerToken
Expand All @@ -118,14 +118,14 @@ Resources:
Action:
- codecommit:*
Resource:
- arn:${partition}:codecommit:${region}:${account_id}:codeseeder-${seedkit_name}-*
- !Sub arn:${AWS::Partition}:codecommit:${AWS::Region}:${AWS::AccountId}:codeseeder-${seedkit_name}-*
- Effect: Allow
Action:
- secretsmanager:*
Resource:
- arn:${partition}:secretsmanager:${region}:${account_id}:secret:codeseeder-${seedkit_name}-*
- arn:${partition}:secretsmanager:${region}:${account_id}:secret:*-docker-credentials*
- arn:${partition}:secretsmanager:${region}:${account_id}:secret:*-mirror-credentials*
- !Sub arn:${AWS::Partition}:secretsmanager:${AWS::Region}:${AWS::AccountId}:secret:codeseeder-${seedkit_name}-*
- !Sub arn:${AWS::Partition}:secretsmanager:${AWS::Region}:${AWS::AccountId}:secret:*-docker-credentials*
- !Sub arn:${AWS::Partition}:secretsmanager:${AWS::Region}:${AWS::AccountId}:secret:*-mirror-credentials*
- Effect: Allow
Action:
- codeartifact:Create*
Expand All @@ -137,8 +137,8 @@ Resources:
- codeartifact:TagResource
- codeartifact:Associate*
Resource:
- arn:${partition}:codeartifact:${region}:${account_id}:domain/aws-codeseeder-${seedkit_name}*
- arn:${partition}:codeartifact:${region}:${account_id}:repository/aws-codeseeder-${seedkit_name}*
- !Sub arn:${AWS::Partition}:codeartifact:${AWS::Region}:${AWS::AccountId}:domain/aws-codeseeder-${seedkit_name}*
- !Sub arn:${AWS::Partition}:codeartifact:${AWS::Region}:${AWS::AccountId}:repository/aws-codeseeder-${seedkit_name}*
- Effect: Allow
Action:
- codeartifact:GetAuthorizationToken
Expand All @@ -161,8 +161,8 @@ Resources:
- s3:DeleteObjectVersion
- s3:DeleteBucket
Resource:
- arn:${partition}:s3:::codeseeder-${seedkit_name}-${account_id}-${deploy_id}/*
- arn:${partition}:s3:::codeseeder-${seedkit_name}-${account_id}-${deploy_id}
- !Sub arn:${AWS::Partition}:s3:::codeseeder-${seedkit_name}-${AWS::AccountId}-${deploy_id}/*
- !Sub arn:${AWS::Partition}:s3:::codeseeder-${seedkit_name}-${AWS::AccountId}-${deploy_id}
- Effect: Allow
Action:
- s3:GetEncryptionConfiguration
Expand All @@ -171,12 +171,12 @@ Resources:
Action:
- logs:*
Resource:
- arn:${partition}:logs:${region}:${account_id}:log-group:/aws/codebuild/codeseeder-${seedkit_name}*
- !Sub arn:${AWS::Partition}:logs:${AWS::Region}:${AWS::AccountId}:log-group:/aws/codebuild/codeseeder-${seedkit_name}*
- Effect: Allow
Action:
- cloudformation:DescribeStacks
Resource:
- arn:${partition}:cloudformation:${region}:${account_id}:stack/aws-codeseeder*
- !Sub arn:${AWS::Partition}:cloudformation:${AWS::Region}:${AWS::AccountId}:stack/aws-codeseeder*
- Effect: Allow
Action:
- ec2:DescribeDhcpOptions
Expand All @@ -194,7 +194,7 @@ Resources:
Type: AWS::IAM::Role
Properties:
Path: ${role_prefix}
RoleName: codeseeder-${seedkit_name}-${region}-codebuild
RoleName: !Sub codeseeder-${seedkit_name}-${AWS::Region}-codebuild
MaxSessionDuration: 10000
AssumeRolePolicyDocument:
Version: '2012-10-17'
Expand Down Expand Up @@ -288,7 +288,7 @@ Resources:
- Sid: Enable IAM User Permissions
Effect: Allow
Principal:
AWS: arn:${partition}:iam::${account_id}:root
AWS: !Sub arn:${AWS::Partition}:iam::${AWS::AccountId}:root
Action: kms:*
Resource: '*'
- Sid: Allow administration of the key
Expand Down
12 changes: 8 additions & 4 deletions tests/test_seedkit_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from typing import Optional

import pytest

from aws_codeseeder.commands import _seedkit_commands


Expand All @@ -21,12 +25,12 @@ def test_seedkit_not_deployed(mocker):
assert not stack_exists


def test_deploy_seedkit(mocker):
@pytest.mark.parametrize("synthesize", [None, False, True])
@pytest.mark.parametrize("kwargs", [{}, {"role_prefix": "/test1/", "policy_prefix": "/test2/"}])
def test_deploy_seedkit(mocker, synthesize: Optional[bool], kwargs):
mocker.patch("aws_codeseeder.services.cfn.does_stack_exist", return_value=(False, {}))
mocker.patch("aws_codeseeder.services.cfn.deploy_template", return_value=None)
mocker.patch("aws_codeseeder._cfn_seedkit.get_sts_info", return_value=("123456789012", "arn:aws::::", "aws"))
mocker.patch("aws_codeseeder._cfn_seedkit.get_region", return_value="us-east-1")
_seedkit_commands.deploy_seedkit("test-seedkit")
_seedkit_commands.deploy_seedkit("test-seedkit", synthesize=synthesize, **kwargs)


def test_seedkit_deployed(mocker):
Expand Down