fix(instances): Preserve env variables values verbatim by nurof3n · Pull Request #395 · unikraft-cloud/cli

nurof3n · 2026-07-03T12:54:02Z

Closes TOOL-1125

Copilot

Pull request overview

This PR updates instance-related commands to correctly accept environment variables in KEY=VALUE form even when values contain commas, by preventing Kong’s default comma-splitting and handling parsing explicitly before shortcut-flag processing.

Changes:

Set sep:"none" for -e/--env on instance create/edit to avoid Kong splitting values on commas.
Pre-process Env into --set runtime.env=... entries before applying generic shortcut flags (create/edit/run).
Add helpers to split and encode env assignments as JSON map fragments for consistent patch parsing.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
internal/cmd/run.go	Pre-processes `--env` into `--set` entries before generic shortcut handling to support `KEY=VALUE` parsing.
internal/cmd/instances.go	Disables Kong comma-splitting for `--env`, adds env parsing helpers, and applies the same pre-processing for create/edit.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

craciunoiuc

Some comments my side 🥺

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

craciunoiuc · 2026-07-07T13:08:52Z

@nurof3n tests failing 😳

Also please close copilot comments which no longer apply

Looks much leaner now 🫦

Signed-off-by: Alex-Andrei Cioc <[email protected]>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

jedevc · 2026-07-07T17:00:57Z

+			// Each input string is a single key=value entry. Split on the
+			// first "=" only; the value is then parsed by parseReflect
+			// according to the map's element type. For string element types
+			// (e.g. map[string]string) the value is taken verbatim, so it may
+			// contain commas, "=", or any other characters. For non-string
+			// element types (e.g. slices, structs), parseReflect may still
+			// interpret commas per the element type's own parsing rules.
+			// Multiple entries are supplied via repeated --set flags (multiple
+			// input strings), not via commas within a single string.
+			//
+			// Skip empty/whitespace-only inputs so a bare "--set field=" can
+			// be used without creating a spurious empty-key entry, and trim
+			// the key only (values are kept verbatim).


YESSSS, this is what I meant! Perfect 🎉 Incredible find 🎉

I will say though... this might have an impact on other things. Take a quick look and see where else it impacts. Also, note there's above logic for structs where we have similar logic actually!

We could also make the previous range behavior optional, similar to kong, and obey a sep tag on things. If it's none then we skip it, otherwise we split on the value.

I tried to do that, let me know if it's too complicated again :x

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Signed-off-by: Alex-Andrei Cioc <[email protected]>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

+		candidate := strings.TrimSpace(input[i+1:])
+		if candidate == "" || strings.HasPrefix(candidate, ",") || looksLikeStructField(candidate, s) {
+			items = append(items, strings.TrimSpace(input[start:i]))
+			start = i + 1
+		}


+	t.Run("struct", func(t *testing.T) {
+		got, err := Parse[testStruct]([]string{"name=hello,value=42"})
 		require.NoError(t, err)
-		assert.Equal(t, map[string]string{"a": "1", "b": "2"}, got)
+		assert.Equal(t, testStruct{Name: "hello", Value: 42}, got)
 	})


jedevc · 2026-07-08T16:49:06Z

-				}
-				mapp.SetMapIndex(key, val)
+
+			// Each input string is a single key=value entry. Split on the


Shouldn't this section also use sep? Right now, it's only being used for slices+structs, not maps.

jedevc · 2026-07-08T16:50:53Z

+	return strings.Split(input, sep)
+}
+
+func splitStructValues(input string, s reflect.Value, sep string) []string {


🤔 Hm, wait why aren't we just using splitStructValues here?

Also, again, I don't love methods like looksLikeStructField - this means it's going to magically try and work out how to parse something, which is unexpected and will make the user sad.

nurof3n requested a review from jedevc July 3, 2026 12:54

nurof3n force-pushed the nurof3n/fix/instances-env-sep branch from 2e442b8 to 89f1bc0 Compare July 6, 2026 13:05

craciunoiuc requested a review from Copilot July 6, 2026 18:08

Copilot started reviewing on behalf of craciunoiuc July 6, 2026 18:08 View session

Copilot AI reviewed Jul 6, 2026

View reviewed changes

Comment thread internal/cmd/instances.go Outdated

craciunoiuc requested changes Jul 6, 2026

View reviewed changes

Comment thread internal/cmd/instances.go

Comment thread internal/cmd/instances.go Outdated

Comment thread internal/cmd/instances.go Outdated

jedevc reviewed Jul 7, 2026

View reviewed changes

Comment thread internal/cmd/instances.go Outdated

nurof3n force-pushed the nurof3n/fix/instances-env-sep branch from 89f1bc0 to 5311264 Compare July 7, 2026 12:48

nurof3n requested review from Copilot, craciunoiuc and jedevc July 7, 2026 12:48

Copilot started reviewing on behalf of nurof3n July 7, 2026 12:49 View session

Copilot AI reviewed Jul 7, 2026

View reviewed changes

Comment thread internal/cmd/instances.go Outdated

Comment thread internal/resource/value/parse_test.go Outdated

Comment thread internal/resource/value/parse.go Outdated

nurof3n force-pushed the nurof3n/fix/instances-env-sep branch from 5311264 to a8d856f Compare July 7, 2026 13:00

nurof3n requested a review from Copilot July 7, 2026 13:00

Copilot started reviewing on behalf of nurof3n July 7, 2026 13:00 View session

Copilot AI reviewed Jul 7, 2026

View reviewed changes

Comment thread internal/resource/value/parse.go

Comment thread internal/resource/value/parse_test.go Outdated

Comment thread internal/cmd/instances.go

nurof3n changed the title ~~fix(instances): Handle env variables with key=value, syntax~~ fix(instances): Preserve env variables verbatim Jul 7, 2026

nurof3n changed the title ~~fix(instances): Preserve env variables verbatim~~ fix(instances): Preserve env variables values verbatim Jul 7, 2026

fix(instances): Preserve env variables values verbatim

937a9a1

Signed-off-by: Alex-Andrei Cioc <[email protected]>

nurof3n force-pushed the nurof3n/fix/instances-env-sep branch from a8d856f to 937a9a1 Compare July 7, 2026 13:26

nurof3n requested a review from Copilot July 7, 2026 13:27

Copilot started reviewing on behalf of nurof3n July 7, 2026 13:28 View session

Copilot AI reviewed Jul 7, 2026

View reviewed changes

Comment thread internal/resource/value/parse.go Outdated

craciunoiuc reviewed Jul 7, 2026

View reviewed changes

Comment thread internal/resource/value/parse.go Outdated

craciunoiuc reviewed Jul 7, 2026

View reviewed changes

Comment thread internal/resource/value/parse_test.go Outdated

jedevc reviewed Jul 7, 2026

View reviewed changes

nurof3n requested review from craciunoiuc and jedevc July 8, 2026 11:11

nurof3n requested a review from Copilot July 8, 2026 11:11

Copilot started reviewing on behalf of nurof3n July 8, 2026 11:11 View session

Copilot AI reviewed Jul 8, 2026

View reviewed changes

Comment thread internal/resource/value/parse.go

fix(parse): Take separator into account

4dc1888

Signed-off-by: Alex-Andrei Cioc <[email protected]>

nurof3n force-pushed the nurof3n/fix/instances-env-sep branch from 469fb03 to 4dc1888 Compare July 8, 2026 11:25

nurof3n requested a review from Copilot July 8, 2026 11:26

Copilot started reviewing on behalf of nurof3n July 8, 2026 11:26 View session

Copilot AI reviewed Jul 8, 2026

View reviewed changes

jedevc reviewed Jul 8, 2026

View reviewed changes

Uh oh!

Conversation

nurof3n commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

craciunoiuc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

craciunoiuc commented Jul 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jedevc Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nurof3n Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

jedevc Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

jedevc Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nurof3n commented Jul 3, 2026 •

edited

Loading

jedevc Jul 7, 2026 •

edited

Loading