Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: introduce aws-ecs byovpc sandbox #20

Merged
merged 1 commit into from
Jan 20, 2024
Merged

Conversation

jonmorehouse
Copy link
Contributor

This PR introduces a built-in sandbox for aws-ecs-byovpc. This sandbox uses inputs to wire in existing vpcs, and creates a fargate cluster and the necessary IAM roles for our a.) runner installer b.) the static runner and c.) the odr.

Notably, the runner installer is in Nuon's infrastructure and is the role that we will assume when doing the installation. The static runner only creates/deletes the ODRs, and hte ODR is the thing that needs the most permissions, as it manages the components.

@jonmorehouse jonmorehouse requested a review from a team as a code owner January 11, 2024 13:42
Copy link
Contributor

@jordan-acosta jordan-acosta left a comment

Choose a reason for hiding this comment

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

I found a few issues, but it looks like it's mostly stuff left over from copy/pasting the EKS sandbox.

- s3:PutObject
Resource: '*'

# function that sends the ARN of the SandboxBootstrapRole to the source AWS account,
Copy link
Contributor

Choose a reason for hiding this comment

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

All this stuff is still in here? I should have cleaned this up ages ago.

- route53:TagResource
- s3:GetObject
- s3:ListBucket
- s3:PutObject
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't require any ECS permissions?

echo "ensuring AWS is setup"
aws sts get-caller-identity > /dev/null

echo "looking for ENIs which were orphaned by vpc-cni plugin"
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought this was just for EKS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jordan-acosta leaving this cleanup in here, for now so I can monitor from a known good, and then if we see on deprovision that no ENIs are leaking, we can go ahead and remove this.

region = "us-east-2"
# assume_role_arn = "arn:aws:iam::949309607565:role/nuon-demo-install-access"
assume_role_arn = "" # This needs to be set for use from our services but should not be when running locally.
install_role_arn = "arn:aws:iam::618886478608:role/install-k8s-admin-stage"
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like some EKS stuff got left here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, cleaning up. Thanks!

type = string
description = "The Kubernetes version to use for the EKS cluster."
default = ""
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume there are other config values that would be relevant to ECS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's not a lot re: ECS, you can parameterize some things in Fargate, such as the capacity providers but we probably can wait on that.

@jonmorehouse jonmorehouse merged commit b2c2bfa into main Jan 20, 2024
2 checks passed
@jonmorehouse jonmorehouse deleted the jm/aws-ecs-byo-vpc branch January 20, 2024 00:01
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants