gophercloud / gophercloud

Gophercloud: an OpenStack SDK for Go
https://pkg.go.dev/github.com/gophercloud/gophercloud/v2
Other
893 stars 527 forks source link

Unmarshal error for LB listener "insert_headers" #1989

Open timuthy opened 4 years ago

timuthy commented 4 years ago

The following error occurs when load balancer listeners are retrieved:

json: cannot unmarshal bool into Go struct field Listener.listeners.insert_headers of type string

https://github.com/gophercloud/gophercloud/pull/1514 added support for _insertheaders and with the Go struct it tries to unmarshal the retrived listener's _insertheaders to map[string]string. In our case (Open Telekom Cloud) the OpenStack API returns bool values for the headers instead of strings:

"insert_headers": {
    "X-Forwarded-ELB-IP": false,
    "X-Forwarded-Host": true
  },
...

/cc @lingxiankong

kayrus commented 4 years ago

The same approach can be used: https://github.com/gophercloud/gophercloud/blob/c8c96f74de7f0c91297732d1a1468bc225a68137/openstack/objectstorage/v1/objects/results.go#L163..L170

kayrus commented 4 years ago

According to openstack API reference

A dictionary of optional headers to insert into the request before it is sent to the backend member. See Supported HTTP Header Insertions. Both keys and values are always specified as strings.

So this is most probably an octavia bug.

jtopjian commented 4 years ago

In our case (Open Telekom Cloud) the OpenStack API returns bool values for the headers instead of strings

This might be an issue with Open Telekom Cloud specifically. While Open Telekom Cloud is based on OpenStack, it has several modifications and isn't completely OpenStack compatible.

Gophercloud supports "vanilla" OpenStack and we test against the OpenStack projects proper. Looking through the test logs of previous commits to Gophercloud, it appears that the insert_headers field is being sent back correctly:

2020-07-06 21:07:55.107043 | ubuntu-bionic | 2020/07/06 21:07:55 [DEBUG] OpenStack Request URL: GET http://192.168.0.3/load-balancer/v2.0/lbaas/listeners/4e54779e-ad57-4597-8580-0242dd88e8f6
2020-07-06 21:07:55.107072 | ubuntu-bionic | 2020/07/06 21:07:55 [DEBUG] OpenStack request Headers:
2020-07-06 21:07:55.107088 | ubuntu-bionic | Accept: application/json
2020-07-06 21:07:55.107106 | ubuntu-bionic | User-Agent: gophercloud/2.0.0
2020-07-06 21:07:55.107120 | ubuntu-bionic | X-Auth-Token: ***
2020-07-06 21:07:55.133767 | ubuntu-bionic | 2020/07/06 21:07:55 [DEBUG] OpenStack Response Code: 200
2020-07-06 21:07:55.133848 | ubuntu-bionic | 2020/07/06 21:07:55 [DEBUG] OpenStack Response Headers:
2020-07-06 21:07:55.133870 | ubuntu-bionic | Content-Length: 933
2020-07-06 21:07:55.133890 | ubuntu-bionic | Content-Type: application/json
2020-07-06 21:07:55.133911 | ubuntu-bionic | Date: Mon, 06 Jul 2020 21:07:55 GMT
2020-07-06 21:07:55.133929 | ubuntu-bionic | Server: Apache/2.4.29 (Ubuntu)
2020-07-06 21:07:55.133962 | ubuntu-bionic | X-Openstack-Request-Id: req-e7457446-08ac-4c2b-b7c8-54a66aa91f5a
2020-07-06 21:07:55.133991 | ubuntu-bionic | 2020/07/06 21:07:55 [DEBUG] OpenStack Response Body: {
2020-07-06 21:07:55.134003 | ubuntu-bionic |   "listener": {
2020-07-06 21:07:55.134020 | ubuntu-bionic |     "admin_state_up": true,
2020-07-06 21:07:55.134037 | ubuntu-bionic |     "allowed_cidrs": null,
2020-07-06 21:07:55.134058 | ubuntu-bionic |     "client_authentication": "NONE",
2020-07-06 21:07:55.134080 | ubuntu-bionic |     "client_ca_tls_container_ref": null,
2020-07-06 21:07:55.134101 | ubuntu-bionic |     "client_crl_container_ref": null,
2020-07-06 21:07:55.134118 | ubuntu-bionic |     "connection_limit": -1,
2020-07-06 21:07:55.134141 | ubuntu-bionic |     "created_at": "2020-07-06T21:07:26",
2020-07-06 21:07:55.134164 | ubuntu-bionic |     "default_pool_id": null,
2020-07-06 21:07:55.134186 | ubuntu-bionic |     "default_tls_container_ref": null,
2020-07-06 21:07:55.134201 | ubuntu-bionic |     "description": "",
2020-07-06 21:07:55.134228 | ubuntu-bionic |     "id": "4e54779e-ad57-4597-8580-0242dd88e8f6",
2020-07-06 21:07:55.134243 | ubuntu-bionic |     "insert_headers": {
2020-07-06 21:07:55.134265 | ubuntu-bionic |       "X-Forwarded-For": "true"
2020-07-06 21:07:55.134274 | ubuntu-bionic |     },
2020-07-06 21:07:55.134288 | ubuntu-bionic |     "l7policies": [],
2020-07-06 21:07:55.134304 | ubuntu-bionic |     "loadbalancers": [
2020-07-06 21:07:55.134312 | ubuntu-bionic |       {
2020-07-06 21:07:55.134340 | ubuntu-bionic |         "id": "ccd8177b-b27c-4e67-97d4-49d364297992"
2020-07-06 21:07:55.134349 | ubuntu-bionic |       }
2020-07-06 21:07:55.134357 | ubuntu-bionic |     ],
2020-07-06 21:07:55.134369 | ubuntu-bionic |     "name": "",
2020-07-06 21:07:55.134399 | ubuntu-bionic |     "operating_status": "ONLINE",
2020-07-06 21:07:55.134446 | ubuntu-bionic |     "project_id": "2816c06d1f594fd1ba83ce2bf82d7131",
2020-07-06 21:07:55.134463 | ubuntu-bionic |     "protocol": "TCP",
2020-07-06 21:07:55.134480 | ubuntu-bionic |     "protocol_port": 76,
2020-07-06 21:07:55.134503 | ubuntu-bionic |     "provisioning_status": "ACTIVE",
2020-07-06 21:07:55.134523 | ubuntu-bionic |     "sni_container_refs": [],
2020-07-06 21:07:55.134536 | ubuntu-bionic |     "tags": [],
2020-07-06 21:07:55.134566 | ubuntu-bionic |     "tenant_id": "2816c06d1f594fd1ba83ce2bf82d7131",
2020-07-06 21:07:55.134588 | ubuntu-bionic |     "timeout_client_data": 50000,
2020-07-06 21:07:55.134610 | ubuntu-bionic |     "timeout_member_connect": 5000,
2020-07-06 21:07:55.134632 | ubuntu-bionic |     "timeout_member_data": 50000,
2020-07-06 21:07:55.134651 | ubuntu-bionic |     "timeout_tcp_inspect": 0,
2020-07-06 21:07:55.134669 | ubuntu-bionic |     "tls_ciphers": null,
2020-07-06 21:07:55.135005 | ubuntu-bionic |     "tls_versions": null,
2020-07-06 21:07:55.135033 | ubuntu-bionic |     "updated_at": "2020-07-06T21:07:55"
2020-07-06 21:07:55.135059 | ubuntu-bionic |   }
2020-07-06 21:07:55.135067 | ubuntu-bionic | }

In addition, I can confirm this same behavior happens on Vexxhost, which is a public cloud provider that uses "vanilla" OpenStack.

I could, of course, be wrong. Perhaps the behavior you're seeing is from a previous release of Octavia and it has since been fixed? If so, can someone reference a commit or bug report for the upstream Octavia project that highlights this? If this behavior is truly part of upstream Octavia, I'm happy to merge #1990 as a fix. But if this is specific to Open Telekom, the best course of action would be to notify the Open Telekom admins.

kayrus commented 4 years ago

@timuthy can you get in touch with OTC admins?

kayrus commented 4 years ago

Here is what I found in https://review.opendev.org/#/c/257901/

German Eichberger May 5, 2016 ↩ Patch Set 21: Code-Review+1

(1 comment)

small thing - like to see how others feel before +2

octavia/db/models.py Line 374: not a big fan of storing things that way; wonder if adding a boolean for each header might be better?

Kobi Samoray May 6, 2016 ↩ Patch Set 21:

(1 comment)

octavia/db/models.py Line 374: In the last midcycle we were discussing a support-any-header model. That means that we won't necessarily have a boolean here but could also have strings etc. I guess that adding another lbaas_insert_header would be the cleanest - model wise. Yet I don't think that this feature worths cluttering the DB with another table - and we never have to do any lookups by these values or anything so overall I believe that this is more than good enough.

https://review.opendev.org/#/c/257901/23/octavia/db/models.py

Adam Harwell May 24, 2016 ↩ Patch Set 23: Code-Review+2

(2 comments)

doc/source/api/octaviaapi.rst Line 522: I feel like this wording is awkward but I think it's hopefully clear enough.

octavia/db/models.py Line 383: In response to comments on patch 21, I'm normally not a fan of taking the easy way out, but I'm tempted to agree that this SHOULDN'T cause a problem. I just hope it doesn't come back to bite us later.

At first glance it looks like a database schema issue.

jtopjian commented 4 years ago

From what I can see, the insert_headers response has been string/string ever since the initial commit: https://github.com/openstack/octavia/commit/3eedc728f1db46cb1a318f844061a6e690754c62#diff-2f11a70bbdc628524dfea4a0c03033d3R51

It also appears that the valid headers that can be used in insert_headers is finite. I wonder if OTC has done some customization to this set and has inadvertently caused the result to be different?

timuthy commented 4 years ago

Thanks for your analysis. I asked OTC to check and comment this issue.