GorillaStack / auto-tag

Automatically tag AWS resources on creation, for cost assignment
GNU General Public License v3.0
463 stars 159 forks source link

hard dependency on cloudformation? #23

Closed nskitch closed 6 years ago

nskitch commented 7 years ago

I did considerable work on porting the cloudformation template to terraform modules and once it's deployed I see lambda error messages failing on describeStackResource. I look and see that in the node js files there appears to be a hard dependency on cloudformation now:

https://github.com/GorillaStack/auto-tag/blob/master/src/workers/autotag_default_worker.js#L36

I'd love to get this dependency removed so I can deploy this using terraform (required by my client) and do a PR to get terraform added to this project.

Anyone have an idea of how to remedy this?

Cloudwatch logs for lambda:

message: 'User: arn:aws:sts::<account>:assumed-role/AutoTagExecutionRole/AutoTag is not authorized to perform: cloudformation:DescribeStackResource on resource: arn:aws:cloudformation:us-west-2:<account>:stack/autotag/*',```

getRoleName() { let _this = this; return new Promise((resolve, reject) => { try { let cloudFormation = new AWS.CloudFormation({ region: _this.s3Region }); cloudFormation.describeStackResource({ StackName: DEFAULT_STACK_NAME, LogicalResourceId: MASTER_ROLE_NAME }, (err, data) => { if (err) { reject(err); } else { resolve(data.StackResourceDetail.PhysicalResourceId); } }); } catch (e) { reject(e); } }); }

seren commented 7 years ago

It looks like the cloudformatino template creates an execution role and a tagging role, and the code runs as the execution role, assuming the tagging role when it needs it.

The name of the tagging role is dynamically generated by cloudformation when it's created, which is why the code has to retrieve it during runtime. If you can control the tagging role's name, then you can hard-code it in the javascript and not have to search for it.

Otherwise, if the role name is going to be randomly generated, you'd need to some way to save the role's name somewhere and update the code to retrieve it. You could also give the execution policy rights to search IAM and update the code to search the role list, but that sounds a bit hacky and like a potential security hole.

nskitch commented 7 years ago

Thank you @seren that was an accurate description and helpful.

@em0ney I'd like to remove this cloudformation dependency in the nodejs scripts, if it's alright with you. IAM Role resources support a 'RoleName' property that allows defining a custom name.

The cloudformation script can have this property added:

      "Type": "AWS::IAM::Role",
      "Properties": {
        "AssumeRolePolicyDocument": {
          "Statement": [
            {
              "Effect": "Allow",
              "Principal": {
                "AWS" : { "Fn::GetAtt" : [ "AutoTagExecutionRole", "Arn" ] }
              },
              "Action": [
                "sts:AssumeRole"
              ]
            }
          ]
        },
        "RoleName": "AutoTagMasterRole",
        "Path": "/gorillastack/autotag/master/"
      }
    },

This way the dependency in the nodejs on cloudformation can be removed. https://github.com/GorillaStack/auto-tag/blob/master/src/workers/autotag_default_worker.js#L36 or the getRole() function can just return MASTER_ROLE_NAME

thoughts?

em0ney commented 7 years ago

Hi @nskitch and @seren,

Thanks for writing in. Sorry for the tardy responses - has been busy and haven't had much time for this repository.

@seren - you are totally right about the purposes of the two roles, thanks for explaining.

@nskitch - I have a bunch of todo items for this repository, including migrating from json to yaml templates, moving some more attributes to environment variables etc. However, I am not planning to remove a dependency on cloudformation. I'd rather just like to have a separate set of instructions for installation using terraform. Interested in sharing a pull request with this? Or just a PR to show your changes for the terraform port?

nskitch commented 7 years ago

@em0ney Yes, I'd like to share a pull request for my terraform script I ported from your cloudformation template. I'll work on this.

Yes I agree in making terraform a separate installation with instructions, I was thinking cloudformation would stay and there would be an additional option for installation with terraform (my client requirement).

However, at this time, the autotag nodejs script has a dependency on the cloudformatino stack, even though I used my terraform script to install the AWS resources instead of the cloudformation template. I included the line in the first message of this thread above. My idea to get around this was to use 'rolename' in the cloudformation script so that cloudformation doesn't append all the random characters.

Actually, here's my wip for the terraform install:


resource "aws_iam_role" "AutoTagMasterRole" {
  name = "AutoTagMasterRole"
  path = "/gorillastack/autotag/master/"
  depends_on = ["aws_iam_role.AutoTagExecutionRole"]

  assume_role_policy = <<POLICY
{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Effect": "Allow",
      "Principal": {
        "AWS": "${aws_iam_role.AutoTagExecutionRole.arn}"
      },
      "Action": [
        "sts:AssumeRole"
      ]
    }
  ]
}
POLICY
}

resource "aws_lambda_function" "AutoTagLambdaFunction" {
  s3_bucket        = "${var.CodeS3Bucket}"
  s3_key           = "${var.CodeS3Path}"
  function_name    = "AutoTag"
  role             = "${aws_iam_role.AutoTagExecutionRole.arn}"
  handler          = "autotag.handler"
  runtime          = "nodejs4.3"
  description      = "Auto Tag (Open Source by GorillaStack)"
  timeout          = "30"
}

resource "aws_iam_role" "AutoTagExecutionRole" {
  name = "AutoTagExecutionRole"
  path = "/gorillastack/autotag/execution/"

  assume_role_policy = <<POLICY
{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Action": "sts:AssumeRole",
      "Principal": {
        "Service": "lambda.amazonaws.com"
      },
      "Effect": "Allow",
      "Sid": ""
    }
  ]
}
POLICY
}

resource "aws_iam_role_policy" "AutoTagMasterPolicy" {
  name  = "AutoTagMasterPolicy"
  depends_on = ["aws_iam_role.AutoTagMasterRole"]
  role = "${aws_iam_role.AutoTagMasterRole.id}"

  policy = <<POLICY
{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Effect": "Allow",
      "Action": [
        "s3:GetBucketTagging",
        "s3:PutBucketTagging",
        "ec2:CreateTags",
        "elasticloadbalancing:AddTags",
        "autoscaling:CreateOrUpdateTags",
        "rds:AddTagsToResource",
        "elasticmapreduce:AddTags",
        "datapipeline:AddTags"
      ],
      "Resource": [
        "*"
      ]
    }
  ]
}
POLICY
}

resource "aws_iam_role_policy" "AutoTagExecutionPolicy" {
  name = "AutoTagExecutionPolicy"
  role = "${aws_iam_role.AutoTagExecutionRole.id}"
  depends_on = ["aws_iam_role.AutoTagMasterRole"]

  policy = <<POLICY
{
  "Version": "2012-10-17",
  "Statement": [{
      "Effect": "Allow",
      "Action": [
        "logs:CreateLogGroup",
        "logs:CreateLogStream",
        "logs:PutLogEvents"
      ],
      "Resource": "arn:aws:logs:*:*:*"
    },
    {
      "Effect": "Allow",
      "Action": [
        "s3:GetObject",
        "s3:ListBucket"
      ],
      "Resource": [
        "*"
      ]
    },
    {
      "Effect": "Allow",
      "Action": ["sts:*"],
      "Resource": [ "${aws_iam_role.AutoTagMasterRole.arn}" ]
    }
  ]
}
POLICY
}

# role = "${aws_iam_role.user-added-lambda-iam-role.arn}"

data "aws_caller_identity" "current" { }

resource "aws_lambda_permission" "AutoTagLambdaFunction" {
  statement_id   = "AllowExecutionFromCloudWatch"
  action         = "lambda:InvokeFunction"
  function_name  = "${aws_lambda_function.AutoTagLambdaFunction.arn}"
  principal      = "s3.amazonaws.com"
  source_account = "${data.aws_caller_identity.current.account_id}"
  source_arn     = "arn:aws:s3:::${var.CloudTrailBucketName}"
}

resource "aws_s3_bucket" "CloudTrailS3Bucket" {
  bucket = "${var.CloudTrailBucketName}"
}

resource "aws_s3_bucket_policy" "CloudTrailS3Bucket" {
  bucket = "${aws_s3_bucket.CloudTrailS3Bucket.id}"
  depends_on = ["aws_s3_bucket.CloudTrailS3Bucket"]
  policy = <<POLICY
{
    "Version": "2012-10-17",
    "Statement": [{
            "Sid": "AWSCloudTrailAclCheck",
            "Effect": "Allow",
            "Principal": { "Service": "cloudtrail.amazonaws.com" },
            "Action": "s3:GetBucketAcl",
            "Resource": "arn:aws:s3:::${var.CloudTrailBucketName}"
        },
        {
            "Sid": "AWSCloudTrailWrite",
            "Effect": "Allow",
            "Principal": { "Service": "cloudtrail.amazonaws.com" },
            "Action": "s3:PutObject",
            "Resource": ["arn:aws:s3:::${var.CloudTrailBucketName}/AWSLogs/${data.aws_caller_identity.current.account_id}/*"],
            "Condition": { "StringEquals": { "s3:x-amz-acl": "bucket-owner-full-control" } }
        }]

}
POLICY
}

resource "aws_s3_bucket_notification" "bucket_notification" {
  bucket = "${aws_s3_bucket.CloudTrailS3Bucket.id}"

  lambda_function {
    lambda_function_arn = "${aws_lambda_function.AutoTagLambdaFunction.arn}"
    events              = ["s3:ObjectCreated:*"]
  }
}

resource "aws_cloudtrail" "autotag-CloudTrail" {
  name                          = "autotag-CloudTrail"
  s3_key_prefix = ""
  s3_bucket_name                = "${aws_s3_bucket.CloudTrailS3Bucket.id}"
  include_global_service_events = false
  enable_logging                = true
  include_global_service_events = true
  is_multi_region_trail         = true
}

# this will store the lamda code
resource "aws_s3_bucket" "CodeS3Bucket" {
  bucket = "${var.CodeS3Bucket}"
  acl    = "public-read"

  policy = <<POLICY
{
 "Version":"2012-10-17",
 "Statement":[
   {
     "Sid":"",
     "Effect":"Allow",
     "Principal": "*",
     "Action":["s3:GetObject"],
     "Resource":["arn:aws:s3:::${var.CodeS3Bucket}/*"]
   }
 ]
}
POLICY
}

 resource "aws_s3_bucket_object" "object" {
   depends_on = ["aws_s3_bucket.CodeS3Bucket"]
   bucket = "${var.CodeS3Bucket}"
   key    = "${var.CodeS3Path}"
   source = "../deploy/${var.CodeS3Path}"
   etag   = "${md5(file("../deploy/${var.CodeS3Path}"))}"
   acl    = "public-read"
 }
nskitch commented 7 years ago

for anyone interested, I had to update a file in the zip. (I'd provide a change to /src directory but the code does not match what's hosted on the s3 bucket)

in /src/workers/autotag_default_worker.js

{
    key: 'getRoleName',
    value: function getRoleName() {
      var _this = this;
      return new Promise(function (resolve, reject) {
        try {
          var cloudFormation = new _awsSdk2.default.CloudFormation({ region: _this.s3Region });
          cloudFormation.describeStackResource({
            StackName: DEFAULT_STACK_NAME,
            LogicalResourceId: MASTER_ROLE_NAME
          }, function (err, data) {
            if (err) {
              reject(err);
            } else {
              resolve(data.StackResourceDetail.PhysicalResourceId);
            }
          });
        } catch (e) {
          reject(e);
        }
      });
    }
  }, 

changed to:

{
    key: 'getRoleName',
    value: function getRoleName() {
      var _this = this;
      return new Promise(function (resolve, reject) {
        try {
          // Do nothing
          resolve(MASTER_ROLE_NAME);
        } catch (e) {
          reject(e);
        }
      });
    }
  },

It's a bit of a catch-22.. the only reason the author of the code performed the cloudformation stack lookup is because he used cloudformation. Cloudformation creates resources with a bunch of random characters after the name. To over come this, the cloudformation script needs to make use of the RoleName property: http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-iam-role.html#w2ab2c19c10d557c11

em0ney commented 6 years ago

Thanks for this - please submit a PR if you got this to work and we can offer a terraform version