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

fix(proxy-cache): changes age param #12812

Merged
merged 2 commits into from
May 28, 2024
Merged

fix(proxy-cache): changes age param #12812

merged 2 commits into from
May 28, 2024

Conversation

joelact
Copy link
Contributor

@joelact joelact commented Mar 29, 2024

Summary

The age parameter on the schema was in lower case which caused the header Age to appear only when the kong debug option was enabled. This commit changes the schema parameter from age to Age.

Checklist

  • The Pull Request has tests
  • A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
  • There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Issue reference

Fix #12787

@CLAassistant
Copy link

CLAassistant commented Mar 29, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added plugins/proxy-cache schema-change-noteworthy cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee labels Mar 29, 2024
@team-eng-enablement team-eng-enablement added the author/community PRs from the open-source community (not Kong Inc) label Mar 29, 2024
@joelact joelact force-pushed the master branch 4 times, most recently from 1a869ca to b77c205 Compare April 4, 2024 12:29
@jschmid1
Copy link
Contributor

jschmid1 commented Apr 8, 2024

Thanks for your contribution @joelact.
While a headers are case-insensitive according to the RFC it's a contention to have them Title-cased.

Your change includes a schema change, which requires additional steps to ensure backwards compatibility with older Kong versions. If you choose to follow-up on those, I'm happy to assist you in doing so.
Alternatively, we'll pick up the PR and continue from here. Up to you @joelact

@jschmid1 jschmid1 self-requested a review April 8, 2024 12:41
@joelact
Copy link
Contributor Author

joelact commented Apr 8, 2024

Hey @jschmid1. Thanks for taking a look at this.
I want to continue working on this PR so can you tell me the next steps to get this going?

@jschmid1
Copy link
Contributor

Sure @joelact, Your change qualifies as a "rename" of a field. To ensure compatibility to older versions when running Kong in hybrid-mode you have to teach the Kong how to manipulate the plugin config so that older versions still understand it.

You can take a look at the compatibility checkers here -> https://github.com/Kong/kong/blob/master/kong/clustering/compat/checkers.lua

and for testing check this file -> https://github.com/Kong/kong/blob/master/spec/02-integration/09-hybrid_mode/09-config-compat_spec.lua

@joelact
Copy link
Contributor Author

joelact commented Apr 30, 2024

Hey @jschmid1! Sorry for the delay in completing the mentioned actions. When possible can you review this again?

Copy link
Contributor

@jschmid1 jschmid1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late reponse @joelact. There is one last thing we need to address. Other than that we're good to go. Thanks for all the effort, really appreciated 👍🏼

@joelact joelact force-pushed the master branch 2 times, most recently from cb10ad9 to bad9bd2 Compare May 24, 2024 08:54
@jschmid1 jschmid1 self-requested a review May 27, 2024 08:02
@jschmid1
Copy link
Contributor

Looks good, waiting for the CI to turn green. 👍🏼

@jschmid1 jschmid1 requested review from a team May 27, 2024 08:03
@jschmid1
Copy link
Contributor

From https://github.com/Kong/kong/actions/runs/9221338444/job/25446686155?pr=12812

File (kong/plugins/proxy-cache/clustering/compat/response_headers_translation.lua) not found in rockspec (kong-3.8.0-0.rockspec)

You need to add this file to the rockspec.

@joelact joelact force-pushed the master branch 3 times, most recently from c6334f2 to 5bf95fc Compare May 27, 2024 14:30
@joelact
Copy link
Contributor Author

joelact commented May 27, 2024

@jschmid1 added file to rockspec.

The age parameter on the schema was in lower case which caused
the header Age to appear only when the kong debug option was enabled.
This commit changes the schema parameter from age to Age.

Fix Kong#12787
@jschmid1 jschmid1 dismissed their stale review May 28, 2024 08:32

resolved

@jschmid1 jschmid1 merged commit 4707465 into Kong:master May 28, 2024
25 checks passed
@github-actions github-actions bot added the incomplete-cherry-pick A cherry-pick was incomplete and needs manual intervention label May 28, 2024
@jschmid1
Copy link
Contributor

Thanks for your contribution, much appreciated @joelact

@kikito kikito removed the incomplete-cherry-pick A cherry-pick was incomplete and needs manual intervention label Jun 4, 2024
@Kong Kong deleted a comment from team-gateway-bot Jun 4, 2024
jschmid1 added a commit that referenced this pull request Jul 17, 2024
jschmid1 added a commit that referenced this pull request Jul 23, 2024
jschmid1 added a commit that referenced this pull request Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author/community PRs from the open-source community (not Kong Inc) cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee core/clustering plugins/proxy-cache schema-change-noteworthy size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug in proxy-cache plugin not returning age header
6 participants