kubernetes-ingress-controller icon indicating copy to clipboard operation
kubernetes-ingress-controller copied to clipboard

When Kong rejects an mtls-auth CA certificate, link the error message to the plugin

Open mflendrich opened this issue 4 years ago • 1 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Problem Statement

When KIC (dbless) tries to sync an expired CA certificate for mtls, Kong rejects the update with HTTP 400 and the following error

time="2021-07-13T07:14:28Z" level=error msg="failed to update kong configuration: posting new config to /config: 400 Bad Request {\"fields\":{\"ca_certificates\":[{\"@entity\":[\"certificate expired, \\\"Not After\\\" time is in the past\"]},{\"@entity\":[\"certificate expired, \\\"Not After\\\" time is in the past\"]}],\"plugins\":[null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,{\"consumer\":{\"id\":\"missing primary key\"}}]},\"name\":\"invalid declarative configuration\",\"code\":14,\"message\":\"declarative config is invalid: {ca_certificates={{[\\\"@entity\\\"]={\\\"certificate expired, \\\\\\\"Not After\\\\\\\" time is in the past\\\"}},{[\\\"@entity\\\"]={\\\"certificate expired, \\\\\\\"Not After\\\\\\\" time is in the past\\\"}}},plugins={[18]={consumer={id=\\\"missing primary key\\\"}}}}\"}" component=controller
time="2021-07-13T07:14:28Z" level=error msg="failed to sync: posting new config to /config: 400 Bad Request {\"fields\":{\"ca_certificates\":[{\"@entity\":[\"certificate expired, \\\"Not After\\\" time is in the past\"]},{\"@entity\":[\"certificate expired, \\\"Not After\\\" time is in the past\"]}],\"plugins\":[null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,{\"consumer\":{\"id\":\"missing primary key\"}}]},\"name\":\"invalid declarative configuration\",\"code\":14,\"message\":\"declarative config is invalid: {ca_certificates={{[\\\"@entity\\\"]={\\\"certificate expired, \\\\\\\"Not After\\\\\\\" time is in the past\\\"}},{[\\\"@entity\\\"]={\\\"certificate expired, \\\\\\\"Not After\\\\\\\" time is in the past\\\"}}},plugins={[18]={consumer={id=\\\"missing primary key\\\"}}}}\"}" component=sync-queue

Proposed Solution

  • Interpret Kong sync errors so as to link them back to the resources that caused trouble
    • The way I'm thinking is the following:
      • parser, when it creates resources in KongState stores a backreference to the k8s objects that resulted in the KongState object being created
      • when KIC tries to sync using either deck or /config, it gets feedback as to which specific resource in KongState failed to sync
      • ~reporting flavor 1: when a KongState resource is known to have failed to sync, KIC prints a log line saying something like KongPlugin namespace/name failed to sync: <why>~
      • reporting flavor 2: KIC sets the status field of the resource in question to indicate an error

Additional information

Suggested approach: "flavor 2" as written up above.

Acceptance Criteria

  • [ ] KongPlugin has a status field that indicates that the attached mtls-auth certificate has been rejected.

mflendrich avatar Oct 04 '21 14:10 mflendrich

IMHO, reporting flavor 2 might be a better solution for me since we can directly know that the KongPlugin resource is in a wrong status when getting the resource. I would implement in flavor 2.

randmonkey avatar May 17 '22 07:05 randmonkey

I have prepared https://github.com/Kong/kubernetes-ingress-controller/pull/3063 as a first step to resolving this issue. It's ensuring CA cert validity in the parser, making it possible to skip the object's synchronization and log an error message with the affected plugins list (as was proposed in flavor 1 in the issue).

czeslavo avatar Oct 18 '22 06:10 czeslavo

Given the CA certificate issues have been covered with warning events attached to the affected objects in https://github.com/Kong/kubernetes-ingress-controller/pull/3114, I think we can close this issue as done. I've changed the AC to not mention status updates, but events only for now.

Status updates are going to be added separately, probably not in the v2.8.0 release.

czeslavo avatar Nov 08 '22 18:11 czeslavo