Skip to content

ext_authz: respect keep_empty_value in HeaderValueOption#45103

Open
404SkillNotFound wants to merge 14 commits into
envoyproxy:mainfrom
404SkillNotFound:fix/ext-authz-keep-empty-value
Open

ext_authz: respect keep_empty_value in HeaderValueOption#45103
404SkillNotFound wants to merge 14 commits into
envoyproxy:mainfrom
404SkillNotFound:fix/ext-authz-keep-empty-value

Conversation

@404SkillNotFound
Copy link
Copy Markdown

@404SkillNotFound 404SkillNotFound commented May 17, 2026

Commit Message: ext_authz: respect keep_empty_value in HeaderValueOption

Additional Description:
ext_authz was ignoring the keep_empty_value field in HeaderValueOption
and blindly adding all headers from auth responses, including empty-valued
ones. Added a check in copyHeaderFieldIntoResponse and copyOkResponseMutations
to skip empty-valued headers when keep_empty_value is false (default).

Risk Level: low
Testing: CI
Docs Changes: none
Release Notes: added changelog entry for ext_authz keep_empty_value fix

I used AI to help navigate the codebase for this change. I own the change.

Fixes #45003

Signed-off-by: 404SkillNotFound <shivang.upadhyay1@gmail.com>
@repokitteh-read-only
Copy link
Copy Markdown

Hi @404SkillNotFound, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #45103 was opened by 404SkillNotFound.

see: more, trace.

Copy link
Copy Markdown
Member

@wbpcode wbpcode left a comment

Choose a reason for hiding this comment

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

thanks for the contribution. This requires a test case to cover the new code.

Signed-off-by: 404SkillNotFound <shivang.upadhyay1@gmail.com>
Signed-off-by: 404SkillNotFound <shivang.upadhyay1@gmail.com>
@404SkillNotFound
Copy link
Copy Markdown
Author

Added test cases covering both branches, when keep_empty_value is false (header dropped) and when it's true (header kept). Let me know if anything else is needed!

@paul-r-gall
Copy link
Copy Markdown
Contributor

So even though this is a bugfix, it's also a behavior change, and needs to be protected by a runtime flag please.

Signed-off-by: 404SkillNotFound <shivang.upadhyay1@gmail.com>
@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/runtime-guard-changes: FYI only for changes made to (source/common/runtime/runtime_features.cc).

🐱

Caused by: #45103 was synchronize by 404SkillNotFound.

see: more, trace.

@404SkillNotFound
Copy link
Copy Markdown
Author

Done, guarded behind a runtime flag.

Copy link
Copy Markdown
Member

@wbpcode wbpcode left a comment

Choose a reason for hiding this comment

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

LGTM overall with only single comment.

Comment on lines +242 to +243
// TODO(404SkillNotFound): Flip to true to enforce keep_empty_value in ext_authz by default.
FALSE_RUNTIME_GUARD(envoy_reloadable_features_ext_authz_respect_keep_empty_value);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The change is pretty safe and only affects the empty header value. I inclined you can use the normal runtime guard rather than the false runtime guard. Thanks.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done, switched to RUNTIME_GUARD. Also added a changelog entry.

Signed-off-by: 404SkillNotFound <shivang.upadhyay1@gmail.com>
Signed-off-by: 404SkillNotFound <shivang.upadhyay1@gmail.com>
Signed-off-by: 404SkillNotFound <shivang.upadhyay1@gmail.com>
Signed-off-by: Shivang Upadhyay <shivang.upadhyay1@gmail.com>
@404SkillNotFound 404SkillNotFound temporarily deployed to external-contributors May 19, 2026 10:18 — with GitHub Actions Inactive
@404SkillNotFound 404SkillNotFound requested a review from wbpcode May 19, 2026 10:31
wbpcode
wbpcode previously approved these changes May 19, 2026
Signed-off-by: 404SkillNotFound <shivang.upadhyay1@gmail.com>
…04SkillNotFound/envoy into fix/ext-authz-keep-empty-value

Signed-off-by: 404SkillNotFound <shivang.upadhyay1@gmail.com>
Comment thread changelogs/current.yaml Outdated
HMAC secret validity.

- area: ext_authz
change: |
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

needs to use new changelog layout - see #45095

/wait

Copy link
Copy Markdown
Author

@404SkillNotFound 404SkillNotFound May 20, 2026

Choose a reason for hiding this comment

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

done

…ntry layout

Signed-off-by: 404SkillNotFound <shivang.upadhyay1@gmail.com>
Signed-off-by: 404SkillNotFound <shivang.upadhyay1@gmail.com>
Signed-off-by: 404SkillNotFound <shivang.upadhyay1@gmail.com>
@404SkillNotFound 404SkillNotFound temporarily deployed to external-contributors May 21, 2026 04:37 — with GitHub Actions Inactive
…on timeouts

Signed-off-by: 404SkillNotFound <shivang.upadhyay1@gmail.com>
@404SkillNotFound 404SkillNotFound requested a deployment to external-contributors May 21, 2026 06:15 — with GitHub Actions Waiting
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

Successfully merging this pull request may close these issues.

ext_authz does not respect keep_empty_value configuration in HeaderValueOption

5 participants