fix(instance): avoid index out of range in server type deprecation warning#5716
fix(instance): avoid index out of range in server type deprecation warning#5716SAY-5 wants to merge 5 commits into
Conversation
…rning Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
There was a problem hiding this comment.
Pull request overview
Fixes a CLI crash in the Instance namespace when emitting the “server type deprecated” warning by ensuring the warnings slice grows correctly instead of indexing into a zero-length slice (panic seen in scw instance server get <id> for EndOfService servers).
Changes:
- Fix
warningServerTypeDeprecatedslice construction to avoidindex out of rangepanics. - Add a regression test for
warningServerTypeDeprecatedto ensure early-return paths don’t panic.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
internal/namespaces/instance/v1/helpers_types.go |
Replaces invalid warning[0] = ... assignment with append(...) to prevent a panic. |
internal/namespaces/instance/v1/helpers_types_test.go |
Adds a regression test covering the no-panic / early-return behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| instance "github.com/scaleway/scaleway-sdk-go/api/instance/v1" | ||
| "github.com/scaleway/scaleway-sdk-go/scw" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestWarningServerTypeDeprecated(t *testing.T) { | ||
| // Point the client at an unreachable endpoint so the catalog and | ||
| // compatible-types lookups fail and the function returns early. | ||
| client, err := scw.NewClient( | ||
| scw.WithAuth("SCWXXXXXXXXXXXXXXXXX", "11111111-1111-1111-1111-111111111111"), | ||
| scw.WithAPIURL("http://127.0.0.1:1"), | ||
| scw.WithDefaultZone(scw.ZoneFrPar1), | ||
| scw.WithDefaultRegion(scw.RegionFrPar), | ||
| ) | ||
| require.NoError(t, err) | ||
|
|
||
| server := &instance.Server{CommercialType: "DEV1-S", Zone: scw.ZoneFrPar1} | ||
| warnings := warningServerTypeDeprecated(context.Background(), client, server) | ||
|
|
||
| require.NotEmpty(t, warnings) | ||
| assert.Contains(t, warnings[0], "EndOfService") | ||
| } |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5716 +/- ##
==========================================
+ Coverage 50.67% 50.70% +0.03%
==========================================
Files 340 340
Lines 78101 78101
==========================================
+ Hits 39577 39605 +28
+ Misses 37028 37000 -28
Partials 1496 1496 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
|
Thanks a lot :) I think #5683 already handles this issue let us know once it is merged if it solved your problem |
warningServerTypeDeprecatedbuilds its warnings withmake([]string, 0, 2)and then assignswarning[0] = ..., which panics withindex out of range [0] with length 0because the slice has length 0. Runningscw instance server get <id>against a server whose type is deprecated hits this and crashes the CLI.Switched the first element to
appendso the slice grows correctly, which keeps the later early-return paths intact. Added a test that exercises the function with an unreachable client so it returns early without panicking.Fixes #5682.