Skip to content

fix(instance): index out of range (warningServerTypeDeprecated)#5683

Merged
remyleone merged 2 commits into
scaleway:mainfrom
frivoire:patch-2
Jun 19, 2026
Merged

fix(instance): index out of range (warningServerTypeDeprecated)#5683
remyleone merged 2 commits into
scaleway:mainfrom
frivoire:patch-2

Conversation

@frivoire

@frivoire frivoire commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Fixes #5682

Given my (almost-zero) golang level, this PR is "vibe-coded".
But I reviewed it and Copilot's analysis looks right to me :

The bug is a classic Go slice mistake in warningServerTypeDeprecated in helpers_types.go:69-70:
make([]string, 0, 2) creates a slice with length 0 and capacity 2. Writing to warning[0] requires the length to be at least 1 — this panics for any server with EndOfService: true.

(my instance indeed has "end_of_service": true, in the API json)

And the details of the fix:

The PR changes it to use a slice literal instead. This is Go's composite literal syntax. It does three things in one line:

  1. Creates a slice
  2. Initializes it with the values you provide inside { }
  3. Automatically sets both length and capacity to match the number of elements

So []string{ value1 } creates a slice with length=1, capacity≥1, already containing that first element.

Can someone validate this (or at least let the CI run on it) ? 🙏
Thanks !

@github-actions github-actions Bot added the instance Instance issues, bugs and feature requests label Jun 11, 2026
@frivoire frivoire marked this pull request as ready for review June 11, 2026 19:03
@frivoire frivoire requested a review from a team as a code owner June 11, 2026 19:03
@frivoire frivoire changed the title fix(instance): fix index out of range panic in warningServerTypeDepre… fix(instance): fix index out of range panic (warningServerTypeDeprecated) Jun 11, 2026
@frivoire frivoire changed the title fix(instance): fix index out of range panic (warningServerTypeDeprecated) fix(instance): index out of range (warningServerTypeDeprecated) Jun 11, 2026
@codecov-commenter

codecov-commenter commented Jun 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.66%. Comparing base (113811f) to head (a5565e6).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
internal/namespaces/instance/v1/helpers_types.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5683   +/-   ##
=======================================
  Coverage   50.66%   50.66%           
=======================================
  Files         340      340           
  Lines       78134    78134           
=======================================
  Hits        39583    39583           
  Misses      37055    37055           
  Partials     1496     1496           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pypaut

pypaut commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Hello @frivoire, thank you for your pull request! I allowed the CI to run and there is a issue with the linting.

@frivoire

frivoire commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Hello Geoffrey and thanks for CI 👍

there is a issue with the linting.

About the issue: I'm not sure to understand the issue... why is this an issue ?
Is it about performance ? From what I see here: For most programs usage of prealloc will be a premature optimization., so I'm doubting a bit.
Furthermore, today's code is using prealloc (IIUC), so the CI is pushing us towards a syntax that is bug-prone (cf the issue that I'm trying to fix) for potentially a non-existing perf question, so ... is it really the right tradeoff (for a CLI tool) ?

@pypaut

pypaut commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

I understand your point of vue. Though this kind of optimisation seems overkill yet, the code abides by it for now and enforcing it didn't cost much so far.

Could you try replacing the warning[0] with append for example?

@remyleone

Copy link
Copy Markdown
Member

Could you provide us a snippet of code that reproduce the issue you observed? I would like to include a test to ensure that it is fixed for good

@frivoire

frivoire commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

@pypaut :

Could you try replacing the warning[0] with append for example?

Oki, I've rewritten the branch with this idea => the CI should be happy :)
And I've tested it locally (by building the cli + executing again the exact broken command) => no more issue

@frivoire

Copy link
Copy Markdown
Contributor Author

Coucou @remyleone 👋

Could you provide us a snippet of code that reproduce the issue you observed? I would like to include a test to ensure that it is fixed for good

I gave the following command in the related issue:

scw instance server get bd7de168-a479-4793-aa99-ddc7609fe315

And I'm guessing that it's because my instance has "end_of_service": true in the API json (it's a START1-XS)

@remyleone remyleone enabled auto-merge June 18, 2026 14:03
@remyleone remyleone disabled auto-merge June 18, 2026 14:03
@remyleone remyleone added this pull request to the merge queue Jun 19, 2026
Merged via the queue into scaleway:main with commit fd39a52 Jun 19, 2026
122 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

instance Instance issues, bugs and feature requests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

runtime error: index out of range (warningServerTypeDeprecated)

4 participants