feat(http-log): add allow list for logged headers

[ItsKaleb]

For some use cases, only a set of known, allowed headers should be included in traffic logs. This updates the http-log plugin to add an optional behaviour to only include a set of allowed request/response headers in the generated traffic logs.

Summary

This PR offers an alternative default behaviour for the http-log plugin. Some use cases may require only specific headers to be logged for requests/responses. This may reduce risk of end-user injecting garbage to a header, or can restrict sensitive information from being logged from a non-standard header.

By adding the provided array of header names, this PR supplements the capabilities of the custom_fields config section. It provides an easier way to declare an allow list of logged headers, instead of needing to define a custom field rule for each header (request and response).

Additionally, by providing a default value of common headers, this allows the functionality to be on-click-opt-in using the separate enable toggle.

Checklist

• The Pull Request has tests
• There's an entry in the CHANGELOG
• There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - feat(http-log): Add docs for logged header allow list docs.konghq.com#5984

Full changelog

• Add new fields to schema
• Implement logic to enforce only logging headers that appear in the allow list
• Add unit test

Note: While an effort was made to validate this PR, due to issues experienced with running a Dev Container on a fresh clone (see issue #11422 ) I have been unable to run the test suite. I have verified that the logical implementation does work by means of testing on a Kong 2.8.4.0 environment using a duplicate of the http-log plugin. Once issue #11422 is addressed, I am happy to confirm with further testing.

[CLAassistant]

All committers have signed the CLA.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[ItsKaleb]

Hi,
My apologies if I've missed anything in raising this PR. I've tried to go through the appropriate contributing documentation.
I was hoping to have this functionality added for Kong 2.8, though with the dev needing to be forked form the master branch, I wasn't sure on how to do the back-porting. As mentioned in the PR comment, I originally tested this functionality out for 2.8.4.0. I'm happy to update the PR for the backport if anything is needed - I might just need a point in the right direction, please.

Thanks

[hanshuebner]

Hello @ItsKaleb,

thank you for your contribution! The feature seems sensible. A number of additional things is required, though, see below. Note that once the PR is complete and passes testing, it will become part of the Kong 3.5 release train. At this point, new features are not backported to 2.8.x.

Please add a database migration - Remember to add it to the init.lua and to rockspec file as well. For the migration, a test is needed. Finally, the new fields need to be added to the removed_fields.lua file to support mixed-version clusters.

[ItsKaleb]

Thanks for the feedback, @hanshuebner. I will follow up on those points.

[ItsKaleb]

Regarding the back port, can I raise a second PR targeting the release/2.8 branch?

[hanshuebner]

Regarding the back port, can I raise a second PR targeting the release/2.8 branch?

Yes, but we can't give you a release date for the next 2.8.x minor release.

[hanshuebner]

Regarding the back port, can I raise a second PR targeting the release/2.8 branch?

Yes, but we can't give you a release date for the next 2.8.x minor release.

I need to back-pedal regarding this: It is unlikely that there will be another 2.8.x release made by us. The reason for that is that we no longer have the release building infrastructure in place that was used before the 3.x release came out. It also has been our policy not to backport features to old open source releases of Kong. We do maintain 2.8.x for customers of the Enterprise Edition (EE), but feature backports in EE require upper-level management approval, so there is no automatism for this.

If you cannot upgrade to Kong 3.5, you still have the option of forking, but you will need to invest in build infrastructure so that you can build whatever deployment artifacts that you require.

From the perspective of easy building, Kong 3.x with its Bazel based build system is much improved and makes it much easier to set up a separate build chain if you cannot directly use our releases for whatever reason.

[ItsKaleb]

Yeah, I understand.

My organisation does have a platinum license for EE. We should be upgrading to 3.5 soon, but I don't have a definite timeline.

I've proven out a workaround to achieve the same functionality using the custom fields by lua config, it's just really clunky.

Up until this point, we've used a custom logging plugin that was based on an older version of the http-log plugin; we had the logged header allow-list feature as part of that. Over time though, that plugin has developed some issues and I'm trying to migrate things back to the built-in option.

Based on the info you've provided, what I'll do is continue rounding out this PR to add the feature for 3.5. I can either continue with the workaround in 2.8, or worst case scenario I already know I can duplicate the plugin from your source and add the feature we need. Then we can adapt things once we upgrade to 3.5 and the feature is released.

[ms2008]

Code looks fine to me other than what @hanshuebner mentioned. Given that this is just adding brand new fields, I think it seems like just adding removed_fields.lua would be enough.

[ItsKaleb]

Hi @hanshuebner, based on the comments from yourself and @ms2008, I've updated the removed_fields.lua
I've just added a section for 3.5.x based on the existing patterns, please let me know if I've missed something.

As for the migrations, I just wanted to check if that was still required, or what would be needed for that.
I've had a look through DEVELOPER.md and the existing migration for http-log - not sure if there'd be anything required for adding the new fields or not.
Are you able to confirm or point me in the right direction with that one?

Thanks

[hanshuebner]

@ms2008 was completely right, no migration is needed because you're only adding new fields that have defaults. Your change to removed_fields.lua looks good. I'm going to let the tests run and will merge the PR if everything passes.

[hanshuebner]

@ItsKaleb Can you address the test failures as far as they're related to your changes? Thank you!

Relevant tests fail

[hanshuebner]

@vm-001 Can you please add a CHANGELOG file?

[vm-001]

Test failed.

[vm-001]

@vm-001 Can you please add a CHANGELOG file?

Done

[hanshuebner]

@vm-001 Can you please add a CHANGELOG file?

Done

Thank you, also for your diligent review 💪

[ItsKaleb]

Thanks both for your input.

Regarding failing tests, I will be reviewing soon.

[ItsKaleb]

Just to summarise my recent commits, I have addressed some typos that were highlighted in the new variable naming, as well as resolving a few suspect issues with the added tests, and removing the default headers list. I believe things should be running smoother now.

[ItsKaleb]

Hi @hanshuebner,
I think I've managed to get my added tests sorted with this last PR, but there still seems to be a few unit tests failing. I'm not entirely sure if these are because of the changes I've made or not - they look to be related to the hybrid schema. Would you mind taking a look at the output please?

[hanshuebner]

@ItsKaleb The failing process_auto_fields test needs to be updated so that the new fields are present in the expected responses (this is in line with the purpose of the test and cannot be avoided). The asterisk in the error output tells you where the mismatch in the two structures is. If you have a local build, you should be able to run just the failing test(s) using the bin/busted script which is in the repository.

[ItsKaleb]

Thanks, will have another look with this new info.
I'm still struggling to get a local environment set up, but that's mainly to do with the fact I'm still trying to use the dev containers - I added some comments to issue #11422, just haven't had a whole lot of free time to tackle these two.

[github-actions]

This PR is marked as stale because it has been open for 14 days with no activity.

[github-actions]

Dear contributor,

We are automatically closing this pull request because it has not seen any activity for three weeks.
We're sorry that we could not merge it. If you still want to pursure your patch, please feel free to
reopen it and address any remaining issues.

Your contribution is greatly appreciated!

Please have a look
our pledge to the community
for more information.

Sincerely,
Your Kong Gateway team