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

Unable to Update protocol to http from https for service already registered in Kong #481

Open
Vikash08Mishra opened this issue Nov 20, 2024 · 0 comments

Comments

@Vikash08Mishra
Copy link

Vikash08Mishra commented Nov 20, 2024

Go Kong Version

v0.60.0

Description

Service is registered with protocol: https and tls_verify: false. To change the protocol to "http" I retrieved existing service entity and updated protocol to "http" and made client.Services.Update API call and noticed below error
2 schema violations (failed conditional validation given value of field 'protocol'; tls_verify: value must be null)

Root Cause

  • Kong service schema definition has constraint w.r.t protocol and tls_verify value, reference: https://github.com/Kong/kong/blob/master/kong/db/schema/entities/services.lua#L61,L64 , when protocol is http then tls_verify must be null.

  • Important Kong admin API behavior explicitly needs null value when updated protocol value in API is http and tls_verify existing value is true | false. Reason is it's a PATCH call and hence makes sense!! Please refer attachment where 3 API calls are made:

Service-Update-Patch-Calls

  - First call simply shows that service has https protocol and tls_verify is false.
  - second call provides http but not tls_verify and error is returned.
   - third call provides http protocol and tls_verify is empty which works!!
  • Root cause is this line which omits tls_verify field during json serialization when tls_verify is null - we cannot skip null, it needs to be passed because existing value of tls_verify is true|false for https scheme and explicit value is required for PATCH which is what used in service Update API call in client.

Suggested Fix

removing omit for json serialization seems to fix the current issue. However, for a different PATCH request when this field is not intended to be updated, omitting can reset value to nil unintentionally. That gives 2 scenarios:

  1. PATCH request is for service with http protocol: value sent would be null even though not set but that works as that's what http need.
  2. PATCH request is for service with https | tls protocol : as per conditional check request will fail - means it would be mandatory to set tls_verify explicitly for non- https | tls PATCH request.

We may need to consider skip omitting tls_verify explicitly for a given PATCH request if field was was set (even with nil value). A potential solution could be to have a Update method with skip omitting field list which can be used by Json serializer and keep those fields. this approach helps with varying use case where different fields may have different value constraint (empty vs null vs something else), so let caller provide what they do not want to skip in PATCH.
another approach could be to accept a custom serializer which can serialize as per client need, this may increase implementation complexity for the users though.

Steps to reproduce issue

  1. register a service in Kong with protocol https and tls_verify true | false.
  2. Using Go kong client, retrieve service and update protocol to http. you can try explicitly setting tls_verify null.
  3. Make service update call. Error is encountered.
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

1 participant