feat(api): make server field optional with default #367
Conversation
|
@freeznet:Thanks for your contribution. For this PR, do we need to update docs? |
github-actions
bot
added
the
doc-info-missing
There was a problem hiding this comment.
Pull request overview
This PR makes the server field optional in the StreamNativeCloudConnection API with a default value of https://api.streamnative.cloud, addressing issue #364. The change simplifies configuration for users who are connecting to the default StreamNative Cloud API server.
Key Changes:
- Made the
serverfield optional with a default value in the StreamNativeCloudConnection CRD - Added a
GetServer()method to encapsulate the default value logic - Updated
api_connection.goto use the newGetServer()method - Updated copyright years across CRD files from 2025 to 2026
Reviewed changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| api/v1alpha1/streamnativecloudconnection_types.go | Added GetServer() method and made Server field optional with default value |
| pkg/streamnativecloud/api_connection.go | Updated to use GetServer() instead of direct Server field access |
| config/crd/bases/resource.streamnative.io_streamnativecloudconnections.yaml | Added default value and removed server from required fields in CRD |
| charts/pulsar-resources-operator/crds/resource.streamnative.io_streamnativecloudconnections.yaml | Mirrored CRD changes for Helm chart |
| config/crd/bases/resource.streamnative.io_*.yaml (17 files) | Updated copyright year to 2026 |
| charts/pulsar-resources-operator/crds/resource.streamnative.io_*.yaml (17 files) | Updated copyright year to 2026 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,4 +1,4 @@ | |||
| # Copyright 2025 StreamNative | |||
| # Copyright 2026 StreamNative | |||
There was a problem hiding this comment.
The copyright year has been updated to 2026, but we are currently in January 2026. This is likely premature - copyright years typically reflect when the code was created or last significantly modified. Unless this is part of a coordinated repository-wide update, this should remain as 2025.
There was a problem hiding this comment.
@freeznet we should keep the Copyright year unchanged
| // GetServer returns the server URL, using the default if not specified | ||
| func (s *StreamNativeCloudConnectionSpec) GetServer() string { | ||
| if s.Server == "" { | ||
| return DefaultStreamNativeCloudServer | ||
| } | ||
| return s.Server | ||
| } |
There was a problem hiding this comment.
The GetServer method implements default value logic that should be tested. Consider adding unit tests to verify that:
- GetServer returns the default URL when Server is empty
- GetServer returns the specified Server when it is set
This will ensure the default value functionality works correctly.
(If this PR fixes a github issue, please add
Fixes #<xyz>.)Fixes #364
(or if this PR is one task of a github issue, please add
Master Issue: #<xyz>to link to the master issue.)Master Issue: #
Motivation
Explain here the context, and why you're making that change. What is the problem you're trying to solve.
Modifications
Describe the modifications you've done.
Verifying this change
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Documentation
Check the box below.
Need to update docs?
doc-required(If you need help on updating docs, create a doc issue)
no-need-doc(Please explain why)
doc(If this PR contains doc changes)