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

provider/aws: Ignore errors in S3 JSON bucket policies #3331

Closed
wants to merge 1 commit into from
Closed

provider/aws: Ignore errors in S3 JSON bucket policies #3331

wants to merge 1 commit into from

Conversation

AlexanderEkdahl
Copy link
Contributor

With this PR the following works:

resource "aws_s3_bucket" "main" {
  bucket = "${var.bucket_name}"
  policy = "${template_file.policy.rendered}"

  website {
    index_document = "index.html"
  }
}

resource "template_file" "policy" {
  filename = "policy.json"

  vars {
    bucket_name = "${var.bucket_name}"
  }
}

Previously the value of policy would be "${template_file.policy.rendered}" which would result in the value "Error parsing JSON: invalid character '$' looking for beginning of value" and not the rendered template file.

It is not up to terraform to validate input anyway.

@catsby
Copy link
Contributor

catsby commented Jan 12, 2016

Hey @AlexanderEkdahl – I tried this out but wasn't getting the results I was expecting. I used this config:

output "rendered" {
  value = "${template_file.policy.rendered}"
}

resource "aws_s3_bucket" "main" {
  bucket = "testbucketctssomethign"
  policy = "${template_file.policy.rendered}"

  website {
    index_document = "index.html"
  }
}

resource "template_file" "policy" {
  template = <<POLICY
{
    "Statement": [
        {
            "Effect": "${bucket_name}",
            "Principal": "*",
            "Action": "*",
            "Resource": "*",
            "Sid":""
        }
  ],
  "Version": "2008-10-17"
}
POLICY

  vars {
    bucket_name = "Allow"
  }
}

I got this error:

Error applying plan:

2016/01/12 13:17:29 [DEBUG] waiting for all plugin processes to complete...
1 error(s) occurred:

* aws_s3_bucket.main: Error putting S3 policy: MalformedPolicy: Policy has invalid resource
    status code: 400, request id:

I'll check with @phinze to see if this is a core issue with Templates (maybe, not sure). Until then, do you see anything wrong with my template file or policy? Do you have an example config that applies cleanly?

Thanks!

@catsby catsby added the waiting-response An issue/pull request is waiting for a response from the community label Jan 12, 2016
@AlexanderEkdahl
Copy link
Contributor Author

This PR might have become irrelevant seeing how I submitted this almost 5 months ago.

Regarding your policy. Shouldn't the resource parameter be a valid ARN(http://docs.aws.amazon.com/general/latest/gr/aws-arns-and-namespaces.html)?

A working-non-working policy that I remember testing with:

{
  "Statement": [
    {
      "Sid": "AddPerm",
      "Effect": "Allow",
      "Principal": {
        "AWS": "*"
      },
      "Action": "s3:GetObject",
      "Resource": "arn:aws:s3:::${bucket_name}/*"
    }
  ],
  "Version": "2008-10-17"
}

@AlexanderEkdahl
Copy link
Contributor Author

Tested again using Terraform v0.6.7-dev (5435188+CHANGES) and this PR is still relevant and throws an error because the string "${template_file.policy.rendered}" is not valid JSON.

@radeksimko
Copy link
Member

I can confirm this is a core issue with templates. The following example:

{
  "Resources" : {
    "my-vpc": {
      "Type" : "AWS::EC2::VPC",
      "Properties" : {
        "CidrBlock" : "${cidr}",
        "Tags" : [
          {"Key": "Name", "Value": "Primary_CF_VPC"}
        ]
      }
    }
  }
}
resource "template_file" "stack" {
  template = "template.json"

  vars {
    cidr = "10.0.0.0/16"
  }
}

resource "aws_cloudformation_stack" "network" {
  name = "networking-stack"
  template_body = "${template_file.stack.rendered}"
}

results in the following plan:

+ aws_cloudformation_stack.network
    name:          "" => "networking-stack"
    outputs.#:     "" => "<computed>"
    parameters.#:  "" => "<computed>"
    policy_body:   "" => "<computed>"
    template_body: "" => "Error parsing JSON: invalid character '$' looking for beginning of value"

+ template_file.stack
    rendered:  "" => "<computed>"
    template:  "" => "template.json"
    vars.#:    "" => "1"
    vars.cidr: "" => "10.0.0.0/16"

tested using Terraform v0.6.10-dev (176a99348acf3fe5dc012e06265a77f7360e3f27)

@radeksimko
Copy link
Member

After tuning the error message I can also say the $ is coming from uninterpreted HCL, not the template itself.

+ aws_cloudformation_stack.network
    name:          "" => "networking-stack"
    outputs.#:     "" => "<computed>"
    parameters.#:  "" => "<computed>"
    policy_body:   "" => "<computed>"
    template_body: "" => "Error parsing JSON (\"${template_file.stack.rendered}\"): invalid character '$' looking for beginning of value"

+ template_file.stack
    rendered:  "" => "<computed>"
    template:  "" => "template.json"
    vars.#:    "" => "1"
    vars.cidr: "" => "10.0.0.0/16"

which leads me to a conclusion that this issue is described in #4169

@AlexanderEkdahl
Copy link
Contributor Author

It has to do with Terraform trying to normalize JSON input. Probably because AWS will do it once the resource has been created which would lead to inconsistencies if the input was not normalized.

The reason this does not happen to other fields is because the policy field uses a stateFunc which modifies the data before interpolation is done.

It is not the $ inside the template which causes errors but the $ in the policy = "${template_file.policy.rendered}". Sorry for not clarifying this earlier.

@jen20
Copy link
Contributor

jen20 commented Sep 3, 2016

Hi @AlexanderEkdahl! I believe some fixes have been applied that should fix this for S3 policies in particular - we no longer use the StateFunc to normalise JSON - this approach will be rolled out across all policies over time. Please feel free to reopen if you still see this with Terraform 0.7.3 (coming shortly) or later. Thanks for the pull request, and sorry it took so long to get a resolution here!

@jen20 jen20 closed this Sep 3, 2016
bmcustodio pushed a commit to bmcustodio/terraform that referenced this pull request Sep 26, 2017
…dpoing (hashicorp#3331)

Updated documentation to reflect "Read Single Audit Request Header" endpoint is GET-based.
@ghost
Copy link

ghost commented Apr 22, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug core waiting-response An issue/pull request is waiting for a response from the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants