Skip to content

Fix panic in determineMaxBodySize on empty proxy-body-size annotation - #465

Open
apvxlab wants to merge 1 commit into
mattermost:masterfrom
apvxlab:fix/maxbodysize-empty-annotation-panic
Open

Fix panic in determineMaxBodySize on empty proxy-body-size annotation#465
apvxlab wants to merge 1 commit into
mattermost:masterfrom
apvxlab:fix/maxbodysize-empty-annotation-panic

Conversation

@apvxlab

@apvxlab apvxlab commented Jul 13, 2026

Copy link
Copy Markdown

determineMaxBodySize (pkg/mattermost/helpers.go) reads the nginx.ingress.kubernetes.io/proxy-body-size ingress annotation and then indexes size[len(size)-1] without handling an empty value. Setting that annotation to "" panics (index out of range) during reconcile.

Fix: treat an empty annotation value the same as a missing one and return the default size. Adds a regression test case.

Fixed a panic in the Mattermost Operator during reconcile when the `nginx.ingress.kubernetes.io/proxy-body-size` ingress annotation is set to an empty value.

@mm-cloud-bot mm-cloud-bot added the kind/bug Categorizes issue or PR as related to a bug. label Jul 13, 2026
@mm-cloud-bot

Copy link
Copy Markdown

@apvxlab: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

I understand the commands that are listed here

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fe4c8621-b3ef-4eb5-bfc4-ae46691b4eb0

📥 Commits

Reviewing files that changed from the base of the PR and between 02488c8 and 37093a7.

📒 Files selected for processing (2)
  • pkg/mattermost/helpers.go
  • pkg/mattermost/helpers_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • pkg/mattermost/helpers_test.go
  • pkg/mattermost/helpers.go

📝 Walkthrough

Walkthrough

determineMaxBodySize now returns the default body size when the proxy body size annotation is missing or empty. A table-driven test covers the empty-value case.

Changes

Body Size Defaults

Layer / File(s) Summary
Handle empty annotations
pkg/mattermost/helpers.go, pkg/mattermost/helpers_test.go
Empty proxy-body-size annotation values now return the default size, with test coverage added for this case.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: fixing a panic caused by an empty proxy-body-size annotation.
Description check ✅ Passed The description is directly related to the code change and accurately describes the bug fix and regression test.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mm-cloud-bot mm-cloud-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed labels Jul 13, 2026
@apvxlab
apvxlab force-pushed the fix/maxbodysize-empty-annotation-panic branch from 02488c8 to f16d813 Compare July 13, 2026 17:43
determineMaxBodySize indexes size[len(size)-1] after finding the
nginx.ingress.kubernetes.io/proxy-body-size annotation, but does not
handle an empty value, so annotating the Mattermost ingress with an
empty proxy-body-size ("") panics (index out of range) during reconcile.

Treat an empty value like a missing one and return the default size.
Adds a regression test case.
@apvxlab
apvxlab force-pushed the fix/maxbodysize-empty-annotation-panic branch from f16d813 to 37093a7 Compare July 13, 2026 17:43
@mattermost-build

Copy link
Copy Markdown
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Categorizes issue or PR as related to a bug. Lifecycle/1:stale null release-note Denotes a PR that will be considered when it comes time to generate release notes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants