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

Regression: Cors Origin Comma Separated Lists Don't Work #3609

Closed
w-h37 opened this issue Jul 15, 2021 · 1 comment · Fixed by #3612
Closed

Regression: Cors Origin Comma Separated Lists Don't Work #3609

w-h37 opened this issue Jul 15, 2021 · 1 comment · Fixed by #3612
Assignees
Labels
p:low Issue is of low importance t:bug Something isn't working
Milestone

Comments

@w-h37
Copy link
Contributor

w-h37 commented Jul 15, 2021

Summary

The cors mapping resource no longer supports comma separated lists, only YAML array lists. This change came about with the AES update from 1.9 to 1.10. The following configurations written will show in detail what is and is not allowed regarding configurations with lists:

(Example of comma separated list, as seen in origins. This is not supported)

---
apiVersion: getambassador.io/v2
kind: Mapping
metadata:
  name: cors
spec:
  prefix: /cors/
  service: quote
    origins: http://foo.example,http://bar.example
    methods: POST, GET, OPTIONS
    headers: Content-Type
    credentials: true
    exposed_headers: X-Custom-Header
    max_age: "86400"

(Example of YAML array separated list, as seen in origins. This is supported)

---
apiVersion: getambassador.io/v2
kind: Mapping
metadata:
  name: cors
spec:
  prefix: /cors/
  service: quote
    origins: 
    - http://foo.example
    - http://bar.example
    methods: POST, GET, OPTIONS
    headers: Content-Type
    credentials: true
    exposed_headers: X-Custom-Header
    max_age: "86400"

It is worth noting that AMBASSADOR_LEGACY_MODE does allow the use of comma separated lists, and does serve as a workaround to this issue.

Required Feature

Cors mapping resources should include the ability to read lists of origins using a comma to separate objects

Replicator

AES 1.13.8 installed with Helm

  1. Deploy the quote service from the AES quickstart guide

  2. Create a cors mapping resource:

---
apiVersion: getambassador.io/v2
kind: Mapping
metadata:
  name: cors
spec:
  prefix: /cors/
  service: quote
    origins: http://foo.example,http://bar.example
    methods: POST, GET, OPTIONS
    headers: Content-Type
    credentials: true
    exposed_headers: X-Custom-Header
    max_age: "86400"
  1. Test the cors mapping using the following command:
    curl -kv https://{AES_LOADBALANCER_EXTERNAL_IP}/cors/ -H "Origin: http://foo.example"
    You should get a fair bit of information printed back, but specifically pay attention to the part listed below, found right above the response from the server:
< x-envoy-upstream-service-time: 0
< server: envoy

In the previous step, had the comma separated list been read correctly, you should have gotten the following response back from the curl statement:

< x-envoy-upstream-service-time: 0
< access-control-allow-origin: http://foo.example
< access-control-credentials: true
< access-control-expose-headers: X-Custom-Header
< server: envoy

If curious about testing, you can either switch to AMBASSADOR_LEGACY_MODE or AES version 1.9, and test this mapping with the curl statements, and get the above output.

Versions:

  • Ambassador: 1.13.8
  • Kubernetes environment: Kubeception
  • Version 1.17

Notes

Nothing additional to report at this time

@w-h37 w-h37 changed the title Cors Origin Comma Separated Lists Don't Work Regression: Cors Origin Comma Separated Lists Don't Work Jul 15, 2021
@Alice-Lilith Alice-Lilith added p:low Issue is of low importance t:bug Something isn't working labels Jul 15, 2021
@acookin acookin linked a pull request Jul 15, 2021 that will close this issue
4 tasks
@efunk efunk added this to the 2021 Cycle 5 milestone Jul 16, 2021
@acookin acookin self-assigned this Jul 16, 2021
@efunk
Copy link

efunk commented Jul 28, 2021

Resolved in the 1.13.10 GA release.

@efunk efunk closed this as completed Jul 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p:low Issue is of low importance t:bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants