-
Notifications
You must be signed in to change notification settings - Fork 775
Do not set Kibana server.name #8930
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
base: main
Are you sure you want to change the base?
Conversation
Update unit tests Signed-off-by: Michael Montgomery <[email protected]>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR removes the automatic setting of the server.name configuration in Kibana deployments to allow Kibana to default to using the pod name instead. This improves the usefulness of stack monitoring when multiple Kibana instances are deployed, as each instance will have a unique identifier based on its pod name rather than all sharing the same Kibana resource name.
Key Changes
- Removed automatic setting of
server.namein the base Kibana configuration - Updated test expectations to reflect the absence of
server.namein generated configurations
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/controller/kibana/config_settings.go | Removed the ServerName: kb.Name entry from the base settings map, allowing Kibana to use its default server name (pod name) |
| pkg/controller/kibana/config_settings_test.go | Updated test fixtures defaultConfig and defaultConfig8 to remove the expected name: "testkb" line from server configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
|
Change looks good to me code wise. But I am trying to assess any issues this might cause for existing users. Monitoring History Continuity: kbn-name header: |
pebrc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Let's make sure to include this in the breaking changes documentation for the next release where we
- explain our motivation (official Kibana documentation)
- mention the workaround for uses that want to restore the original (incorrect) behaviour.
re: #8929
From my local testing, this defaults properly to the pod name, which makes stack monitoring a bit more useful in the case of multiple Kibana instances.