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

Add response headers documentation incorrect #4155

Closed
cyrus-mc opened this issue Mar 7, 2022 · 7 comments
Closed

Add response headers documentation incorrect #4155

cyrus-mc opened this issue Mar 7, 2022 · 7 comments

Comments

@cyrus-mc
Copy link

cyrus-mc commented Mar 7, 2022

According to documentation add_response_headers accepts string, bool or object as shown in the example Mapping:

apiVersion: getambassador.io/v3alpha1
kind:  Mapping
metadata:
  name:  quote-backend
spec:
  prefix: /backend/
  add_response_headers:
    x-test-proto: "%PROTOCOL%"
    x-test-ip: "%DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT%"
    x-test-static: This is a test header
    x-test-object:
      append: False
      value: this is from object header config
  service: quote

When using anything other than an object you receive error message:

error: error validating "mapping.yaml": error validating data: ValidationError(Mapping.spec.add_response_headers.x-test-proto): invalid type for io.getambassador.v3alpha1.Mapping.spec.add_response_headers: got "string", expected "map"; if you choose to ignore these errors, turn validation off with --validate=false

Looking at the CRD definitions (if my understanding is correct):

          spec:
            description: MappingSpec defines the desired state of Mapping
            properties:
              add_linkerd_headers:
                type: boolean
              add_request_headers:
                type: object
                x-kubernetes-preserve-unknown-fields: true
              add_response_headers:
                type: object
                x-kubernetes-preserve-unknown-fields: true

It looks like it only accepts an object.

Steps to reproduce the behavior:

  1. Create mapping example as shown in the documentation

This error is present on the latest version 2.2.2.

@tenshinhigashi
Copy link
Contributor

I tested this and can confirm the behavior of validation errors when trying to use string and bool.

@cindymullins-dw
Copy link
Contributor

From engineering: the v2 add_response_headers allowed string, bool, and objects. In v3alpha1, only objects are allowed. We'll raise a docs request to fix the reference there, and close this issue.

@so-amuk
Copy link

so-amuk commented Mar 25, 2022

I need to configure the HTTP Strict Transport Security(HSTS) response header. So in order to add response headers I am following this document:
https://www.getambassador.io/docs/emissary/latest/topics/using/headers/add_response_headers/

apiVersion: getambassador.io/v3alpha1
kind: Mapping
metadata:
  name: node-service-mapping
  namespace: teamnamespace
spec:
  hostname: "example.com"
  prefix: /node-service
  service: node-service
  timeout_ms: 120000
  add_response_headers:
    Strict-Transport-Security: max-age=15724800

After applying this, I am getting following error. Can someone let me know how to resolve this and what is the correct way of adding response_headers?

error validating "": error validating data: ValidationError(Mapping.spec.add_response_headers.Strict-Transport-Security): invalid type for io.getambassador.v3alpha1.Mapping.spec.add_response_headers: got "string", expected "map"

Any solution how to resolve it ?

@so-amuk
Copy link

so-amuk commented Mar 26, 2022

Following solution worked for me

add_response_headers:
  Strict-Transport-Security:
    value: max-age=15724800

thanks @cindymullins-dw

@cindymullins-dw
Copy link
Contributor

Thanks for reporting @so-amuk!

@psalaberria002
Copy link
Contributor

This is still broken. @so-amuk 's solution works, but that's just an alternative to defining the headers directly as strings. Even the docs mention both options are supported. The example from the docs isn't working.

Is this a bug in the schema generation?

add_response_headers:
                additionalProperties:
                  properties:
                    append:
                      type: boolean
                    v2Representation:
                      enum:
                      - ""
                      - string
                      - "null"
                      type: string
                    value:
                      type: string
                  type: object
                type: object

@ppeble
Copy link
Contributor

ppeble commented Feb 17, 2023

This has bitten my team as well just today with the latest emissary. I don't think this is actually solved. @cindymullins-dw

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

No branches or pull requests

6 participants