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

The response-transformer plugin continuously detects outdated configuration to synchronize #102

Closed
jaygorrell opened this issue Aug 23, 2018 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@jaygorrell
Copy link
Contributor

Summary

This has specifically been observed with the response-transformer plugin but may extend to others as well.

In these cases, the KongPlugin resource is created (and attached to an Ingress), resulting in the entry correctly being added to Kong with a plugin associated to the route. The issue is that by watching the ingress-controller logs, you can see that the plugin is repeatedly trying to be updated.

The larger risk here is that it may trigger things like Kong/kong#3423 this when many plugins are used.

Kong Ingress controller version

0.1.0

Kubernetes version

1.9.3

Environment

RDS Postgresql Kong Database

What happened

The KongPlugin was created and the Kong configuration was created, but the ingress-controller logs show that it thinks the two are out of sync and tries to persist the changes every 10 minutes.

Immediately after saving a new configuration I can sometimes get a "up to date" message but on the next synchronization check (10 minutes?) it's back to being "outdated".

Expected behvaior

Updates are not attempted against Kong when the KongPlugin and Kong database appear to be in sync.

Steps To Reproduce

  1. Create a KongPlugin with the following configuration
apiVersion: configuration.konghq.com/v1
kind: KongPlugin
metadata:
  name: add-security-headers
config:
  add:
    headers:
    - "X-Xss-Protection:1; mode=block"
  1. Attach this plugin to an Ingress using response-transformer.plugin.konghq.com: add-security-headers

  2. Tail the ingress-controller container logs filtered for add-security-headers

  3. Observe similar to the following logs that repeat regularly

I0823 01:27:55.081377       7 kong.go:784] configuring plugin stored in KongPlugin CRD add-security-headers
I0823 01:27:55.133977       7 kong.go:824] plugin add-security-headers configuration in kong is outdated. Updating...

Other Information

I spent a bit of time trying to further isolate this without much success. I did try specifying all components of the response-transformer plugin (remove/append/replace) with empty lists/maps with no success. I'm having trouble determining what exactly is causing it to think the values are mismatched.

@hbagdi hbagdi added the bug Something isn't working label Aug 23, 2018
@hbagdi
Copy link
Member

hbagdi commented Aug 23, 2018

@jaygorrell
I'm reading the documentation for this plugin and it seems like, it doesn't accept an array of header.

Could you please try the following?

apiVersion: configuration.konghq.com/v1
kind: KongPlugin
metadata:
  name: add-security-headers
config:
  add:
    headers: "X-Xss-Protection:1; mode=block"

@jaygorrell
Copy link
Contributor Author

I tried the string formatted values instead of a list but didn't have any luck, but I did get this worked out.

Looking at pluginDeepEqual here:

// pluginDeepEqual compares the configuration of a Plugin (CRD) against

It iterates over each k/v in the config map, making sure the KongPlugin config items are in Kong and with the same value. For most plugins this is straightforward but the transformer plugins have a nested structure that this method doesn't account for that -- so in my case it just compares the add key as a whole. Using my example above, in Kong, the structure is this:

                "add": {
                    "headers": [
                        "X-Xss-Protection:1; mode=block"
                    ],
                    "json": {}
                },
                "append": {
                    "headers": {},
                    "json": {}
                },
                "remove": {
                    "headers": {},
                    "json": {}
                },
                "replace": {
                    "headers": {},
                    "json": {}
                }

The value of the add key also contains the empty json dict, which causes the check to fail.

Changing my KongPlugin to this fixes the issue:

apiVersion: configuration.konghq.com/v1
kind: KongPlugin
metadata:
  name: $((basename))-upside-add-security-headers
config:
  add:
    headers:
      - "X-Xss-Protection:1; mode=block"
    json: {}

It's also interesting and worth noting that the empty data structure in Kong is a map, while a populated value is a list. That's a little unintuitive but unrelated to Kong Ingress -- aside from how you need to match the values.

I think there's still an issue to be addressed here in code for a better way to compare the objects but if you think this is acceptable, feel free to close the issue.

@jaygorrell
Copy link
Contributor Author

I tried rolling out this temporary fix but hit another snag. Basically the order of the headers and json keys can vary in how they get persisted in Kong, regardless which order they appear in the KongPlugin. In my above example, many of the json keys are coming back before the headers key.

This means you would need to create the KongPlugin, query Kong to see the order, then adjust the configuration order to match.

@jaygorrell jaygorrell changed the title The response-transformer plugin continuously detects outdated configuration to synchronie The response-transformer plugin continuously detects outdated configuration to synchronize Aug 24, 2018
@hbagdi
Copy link
Member

hbagdi commented Aug 24, 2018

@jaygorrell Thank you for your detailed debugging and sharing the results.

Like you already said, there are two separate issues at play here:

  • An empty JSON array ("array": []) showing up as an empty JSON object ("array": {}). This is an issue with lua-cjson, the serialization library that Kong uses, as reported in kong decode json empty array to empty object? kong#3166 and Ingress Controller can't fix that issue.

  • The other issue, which I expected we would run into sooner or later when you first opened this ticket was ordering of elements in an array. reflect.DeepEqual from Go's stdlib can compare JSONs so we should use that and fix the bug. We will open up a PR to fix it.

@hbagdi
Copy link
Member

hbagdi commented Aug 28, 2018

This issue should be fixed now with #106 merged in.
The other issue is with the plugin as noted in Kong/kong#3166.

Closing this, please re-open if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants