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

Launch EC2 instance with CloudFormation #69

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

khashf
Copy link
Contributor

@khashf khashf commented Apr 12, 2018

AWS CloudFormation to create an ec2 instance that has read permission to the existing s3 instance(s)

Small notes:
I accidentally branched out from a non-master branch and so pushed 2 unrelated files. The 2nd commit of this branch/PR fixes this issue. (Sorry Mike)

Upon:
Suggested by @DingoEatingFuzz in a discussion on PR #48
Requested by @MikeTheCanuck at assignment #61

khashf and others added 7 commits April 4, 2018 01:25
The /data volume needs to be preserved if we happen to terminate the databases VM, at least until we have a better backup strategy
Since the improvement/fix script has been merged yet, and I just accidentally branch out from a non-master branch, again :(
Action:
- "s3:GetObject"
- "s3:ListBucket"
Resource: "*"
Copy link
Contributor

Choose a reason for hiding this comment

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

The existing policy is slightly different and limited to only one bucket:

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Action": "s3:*",
            "Resource": [
                "arn:aws:s3:::hacko-data-archive/*"
            ]
        }
    ]
}

Copy link
Contributor Author

@khashf khashf Apr 13, 2018

Choose a reason for hiding this comment

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

Thanks, @DingoEatingFuzz . Yes, I was aware of this. I wasn't sure which bucket to select so I left it a wildcard for easier testing. Patches incoming in a few mins

Copy link
Contributor Author

@khashf khashf Apr 13, 2018

Choose a reason for hiding this comment

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

@DingoEatingFuzz : Updated.
Is it ok if I limit the allowed actions to the 2 following ones?

Action: 
   - "s3:GetObject"
   - "s3:ListBucket"

Copy link
Contributor

Choose a reason for hiding this comment

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

Potentially. Depends on whether or not the box will also be writing back to the bucket. I defer to @MikeTheCanuck

Copy link
Contributor

Choose a reason for hiding this comment

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

For now let's just leave it as read-only, but be prepared to generate a similar policy for whatever strategy we happen to use for backups. I'm conflicted on whether backups would be appropriate to write to the hacko-data-archive bucket, otherwise no reason I can think of to open it for writes.

@MikeTheCanuck
Copy link
Contributor

On Thursday I believe we discussed moving this template (and any others like it) under a parent folder called cloudformation. Otherwise I'm assuming this will work, though we don't have a decent place to test these things.

Next question is, how much of create-ec2-machine-database.sh does this replace? It appears to me that if we were to use this template on a completely new VPC, and then run create-ec2-machine-database.sh in that VPC, we'd end up with two machines.

Is it possible, or even practical, to have this template run first, then run a modified/reduced version of the script, to get the same result as we get with the script today?

@khashf
Copy link
Contributor Author

khashf commented Apr 20, 2018

@MikeTheCanuck These CloudFormation templates can replace create-ec2-machine-database.sh completely. Thus, there is no need to modify the sh version script to get the same result - it's already the same. Further, anything the create-ec2-machine-database.sh or related scripts can accomplish, these CloudFormation templates can also accomplish the same (and in an even more concise and maintainable way)

Copy link
Contributor

@MikeTheCanuck MikeTheCanuck left a comment

Choose a reason for hiding this comment

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

  1. There's a lot of hard-coded parameters in here that could well inherit from what we have defined in hackoregon-aws-infrastructure, including
  • AvailabilityZone
  • SubnetId
  • SecurityGroupId
  • DBInstance > ImageId
  • DBInstance > AvailabilityZone
  • DBInstance > SubnetId
  1. the InstanceType is now out of date - that's now a t2.large

  2. I'm pretty sure we don't want to use create-stack to instantiate this as part of the larger ECS infrastructure - I imagine there's some kind of update-stack that would work better for us.

  3. I am about 70% sure that many of the hard-coded values buried in the Resources > DBInstance section would be better off as items in the Parameters section, that are then just pulled in by !Ref in the Resources > DBInstance section.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants