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: aws_api_gateway_method_response headers? #6092

Closed
ijin opened this issue Apr 8, 2016 · 15 comments
Closed

provider/aws: aws_api_gateway_method_response headers? #6092

ijin opened this issue Apr 8, 2016 · 15 comments

Comments

@ijin
Copy link
Contributor

ijin commented Apr 8, 2016

Are setting response headers implemented yet?

I need to set headers like Access-Control-Allow-Origin to my responses in API GW.

@stack72
Copy link
Contributor

stack72 commented Apr 8, 2016

Hi @ijin

This hasn't been implemented yet. I just found the following in the code:

func resourceAwsApiGatewayIntegrationResponseCreate(d *schema.ResourceData, meta interface{}) error {
    conn := meta.(*AWSClient).apigateway

    templates := make(map[string]string)
    for k, v := range d.Get("response_templates").(map[string]interface{}) {
        templates[k] = v.(string)
    }

    _, err := conn.PutIntegrationResponse(&apigateway.PutIntegrationResponseInput{
        HttpMethod:        aws.String(d.Get("http_method").(string)),
        ResourceId:        aws.String(d.Get("resource_id").(string)),
        RestApiId:         aws.String(d.Get("rest_api_id").(string)),
        StatusCode:        aws.String(d.Get("status_code").(string)),
        ResponseTemplates: aws.StringMap(templates),
        // TODO implement once [GH-2143](https://github.com/hashicorp/terraform/issues/2143) has been implemented
        ResponseParameters: nil,
    })
    if err != nil {
        return fmt.Errorf("Error creating API Gateway Integration Response: %s", err)
    }

    d.SetId(fmt.Sprintf("agir-%s-%s-%s-%s", d.Get("rest_api_id").(string), d.Get("resource_id").(string), d.Get("http_method").(string), d.Get("status_code").(string)))
    log.Printf("[DEBUG] API Gateway Integration Response ID: %s", d.Id())

    return nil
}

Notice the comment there above RequestParameters

Thanks

Paul

@ijin
Copy link
Contributor Author

ijin commented Apr 9, 2016

oh, thanks. looks like it's going to take a while..

#2143

@joshrtay
Copy link
Contributor

is there any reason this can't be implemented before fixing #2143? i assume this would work with JSON syntax and it's critical for us.

@joshrtay
Copy link
Contributor

joshrtay commented Apr 15, 2016

Another options here is to expose a different API:

It looks like AWS sdk go expects the following for response paramaters:

{
   "integration.response.body.type/Content-Type": true
}

While aws sdk node expects:

{
  "Content-Type": "integration.response.body.type"
}

If terraform exposed an api that looked more like nodes, would we be in the clear?

resource "aws_api_gateway_integration_response" "test" {
  rest_api_id = "${aws_api_gateway_rest_api.test.id}"
  resource_id = "${aws_api_gateway_resource.test.id}"
  http_method = "${aws_api_gateway_method.test.http_method}"
  status_code = "${aws_api_gateway_method_response.error.status_code}"

  response_paramaters = {
    "Content-Type" = "integration.response.body.type"
  }

  response_templates = {
    "application/json" = ""
    "application/xml" = "#set($inputRoot = $input.path('$'))\n{ }"
  }
}

@joshrtay
Copy link
Contributor

Welp forget that. Just realized I mixed-up the types for integration response and method response parameters.

@joshrtay
Copy link
Contributor

I think the observation stands though. We can change up the terraform api so that there are no dots in keys.

method response response parameters could look like:

response_parameters = {
  "Content-Type" = true
}

and integration response could look like:

response_paramaters = {
    "Content-Type" = "integration.response.body.type"
  }

Then the provider can change the keys to be of the form integration.response.header.{name}

@joshrtay
Copy link
Contributor

joshrtay commented Apr 15, 2016

Then, assuming I have the prefix right, the code becomes:

func resourceAwsApiGatewayIntegrationResponseCreate(d *schema.ResourceData, meta interface{}) error {
    conn := meta.(*AWSClient).apigateway

    templates := make(map[string]string)
    for k, v := range d.Get("response_templates").(map[string]interface{}) {
        templates[k] = v.(string)
    }

    paramaters := make(map[string]string)
    for k, v := range d.Get("response_paramaters").(map[string]interface{}) {
        paramaters["integration.response.header." + k] = v.(string)
    }

    _, err := conn.PutIntegrationResponse(&apigateway.PutIntegrationResponseInput{
        HttpMethod:        aws.String(d.Get("http_method").(string)),
        ResourceId:        aws.String(d.Get("resource_id").(string)),
        RestApiId:         aws.String(d.Get("rest_api_id").(string)),
        StatusCode:        aws.String(d.Get("status_code").(string)),
        ResponseTemplates: aws.StringMap(templates),
        ResponseParameters: aws.StringMap(paramaters),
    })
    if err != nil {
        return fmt.Errorf("Error creating API Gateway Integration Response: %s", err)
    }

    d.SetId(fmt.Sprintf("agir-%s-%s-%s-%s", d.Get("rest_api_id").(string), d.Get("resource_id").(string), d.Get("http_method").(string), d.Get("status_code").(string)))
    log.Printf("[DEBUG] API Gateway Integration Response ID: %s", d.Id())

    return nil
}

@joshrtay
Copy link
Contributor

@radeksimko would love to get your thoughts on changing the api to avoid #2143

@radeksimko
Copy link
Member

@joshrtay While I totally understand the need to get this implemented with or without #2143 I'm not entirely sure about this approach.

If we're really going to implement this without #2143 I believe there should be an easy way to migrate users to the correct DSL once #2143 is fixed.

I was thinking along the lines of raw JSON, e.g.

response_paramaters_in_json = <<PARAMS
{
   "integration.response.body.type/Content-Type": true
}
PARAMS

it may not look as nice and natural, but it respects the API/SDK and provides a fairly clean migration path to a state when #2143 is fixed - e.g. we can mark response_paramaters_in_json deprecated and introduce response_paramaters, or we might even keep both there for a while and have common logic processing these as the only difference would be json-marshalled string or unmarshalled object.

Also the field name suggests what is expected from the user (_in_json).

What do you think?

@joshrtay
Copy link
Contributor

That works for me.

When using JSON syntax I assume you'd still have to use the response_parameters_in_json field name and it would look something like?

"response_parameters_in_json": "{ \"integration.response.body.type/Content-Type\": true }"

@radeksimko
Copy link
Member

If you (for any reason) didn't want to use the HEREDOC syntax I presented in the example above, then yes, it would look exactly like that - including the backslashes.

We could also use normalizeJson helper we use elsewhere to ensure that no diffs/updates are generated for whitespace/newline changes.

@joshrtay
Copy link
Contributor

Ok got it. I'd love to get this in master by the end of the week. If it would speed things up, I'd be happy to submit a pull request. Though I have zero go experience.

@joshrtay
Copy link
Contributor

@radeksimko Took a stab at it. #6344

@radeksimko
Copy link
Member

#6344 merged, it will be available in the next release 0.6.16 as response_parameters_in_json for both aws_api_gateway_integration_response and aws_api_gateway_method_response.

It may be reimplemented in the future once as discussed above, but I'm closing this issue for now.

@ghost
Copy link

ghost commented Apr 26, 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 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants