Fixing the lexical error Hello-world test by ramdrvcs · Pull Request #2509 · skupperproject/skupper

ramdrvcs · 2026-06-24T09:03:40Z

While I was running the E2E tests, I had the follwing error for hello-world module.

fatal: [west]: FAILED! => {"attempts": 30, "changed": true, "msg": "Task failed: Action failed: Unknown error.", "rc": 1, "return_code": 1, 
"stderr": "cannot parse the data: `lexical error: invalid character inside string.\n
{\"user\":{\"uid\":0,\"gid\":0,\"addi\n                     (right here) ------^\n`\n",
"stderr_lines": ["cannot parse the data: `lexical error: invalid character inside string.", " 
{\"user\":{\"uid\":0,\"gid\":0,\"addi", "                     (right here) ------^", "`"], 
"stdout": "", "stdout_lines": []}

Description

Fixes a critical runtime failure in the run_curl role where the task fails with an Unknown error (lexical error: invalid character inside string).

The failure occurs because ansible task definition contain end of line character in sh location. As of now it has been noticed in s390x.

Changes

Key Changes

Single Network Call (curl -s -w "\n%{http_code}" "{{ run_curl_address }}"): Reduces traffic by making exactly one request to the backend. The API response text is captured alongside the 3-digit HTTP status code, which is appended to a brand-new line at the very bottom.
Local Code Extraction (tail -n 1): Pipes the combined response string and isolates only the final line to extract the HTTP status code.
Local Body Preservation (sed "$ d"): Trims the appended status code line from the stream, safely preserving the original API message body (including any native trailing text or newlines).
Defensive String Evaluation ([ "$code" = "200" ]): Switches the conditional check to a robust string comparison. This protects the shell executor from crashing if network drops cause the status code to return empty.
Clean YAML Syntax: Formatted using the folded block scalar (>) to preserve intended script indentation and readability without breaking the Kubernetes command processor array.

Testing Triaged

Verified that payloads containing nested JSON quotes no longer trigger a lexical error.
Confirmed that the loop correctly runs exactly one HTTP request per retry iteration.
Confirmed the task successfully completed.

Summary by CodeRabbit

Bug Fixes
- Improved end-to-end curl output handling by capturing both the response body and HTTP status code from a single curl execution.
- Updated the role’s output formatting to reliably extract the status code and present results consistently as BODY: <body> on success and on failure.

coderabbitai · 2026-06-24T09:04:22Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

The run_curl role now makes one curl request, splits the combined output into body and HTTP status code, and formats the result based on whether the code is 200.

Changes

run_curl response parsing

Layer / File(s)	Summary
single curl output split `tests/e2e/collections/ansible_collections/e2e/tests/roles/run_curl/tasks/main.yml`	A single curl call appends the HTTP code to the response, the task extracts `code` and `body` from that combined output, checks for `"200"`, and prints either `BODY: ...` or `ERROR CODE: ...

Estimated code review effort: 1 (Trivial) | ~3 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is clearly related to the main change: fixing the hello-world E2E lexical parsing error.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4fabd917-9871-463f-91d8-71e5570d176b

📥 Commits

Reviewing files that changed from the base of the PR and between 6f4f2b3 and 466bb5c.

📒 Files selected for processing (1)

tests/e2e/collections/ansible_collections/e2e/tests/roles/run_curl/tasks/main.yml

hash-d

@ramdrvcs, I see the move to single curl call as a clear improvement. However, I'm not so sure about the base64 encoding. Please check my comments and let me know what you're seeing.

hash-d · 2026-07-03T16:35:24Z

+        echo "BODY_BASE64: $response";
+        exit 0;
+      else
+        echo "BODY_BASE64: $response";


Picky: only the exit 0 / exit 1 needs to be within the if/else clauses. Move the echo to before the if, as it is identical in both cases.

I have not changed the if/else block much here. I am leaving the original way as it is.

hash-d · 2026-07-03T16:38:23Z

-        fi
-      '
+      tmp_file=$(mktemp);
+      code=$(curl -s -w "%{http_code}" -o "$tmp_file" "{{ run_curl_address }}");


I like the single call to curl. That's a good improvement.

hash-d · 2026-07-03T16:57:28Z

-      '
+      tmp_file=$(mktemp);
+      code=$(curl -s -w "%{http_code}" -o "$tmp_file" "{{ run_curl_address }}");
+      response=$(base64 < "$tmp_file" | tr -d "\n");


This is the crux of the PR, but I'm not convinced it is really required. The PR description mentions nested double quotes, but I see no nesting in the provided example.

Also, the contents of the example don't seem to match what I'd expect as a response from the backend service (it contanis user information in a json, whereas I'd expect a plain message such as Hello, stranger. I am Supreme Cipher (1360c152dd3c).).

Which task is throwing that error? Is it really failing on a curl call?

It is corrected without base64

hash-d · 2026-07-03T16:58:34Z

 - name: Debug the curl command output
  ansible.builtin.debug:
-    msg: "{{ curl_result.stdout }}"
+    msg: "{{ (curl_result.stdout | regex_search('BODY_BASE64: (.*)', '\\1') | first) | b64decode }}"


This is the only place where you're decoding it from base64. Is run_curl used elsewhere? Could other tests depend / parse the actual output. If so, they'd need to do the decoding, too.

It is corrected without base64

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 97126738-8060-4333-8e5b-e36378fe907c

📥 Commits

Reviewing files that changed from the base of the PR and between e654414 and 546374f.

📒 Files selected for processing (1)

tests/e2e/collections/ansible_collections/e2e/tests/roles/run_curl/tasks/main.yml

hash-d

lgtm; I worked with Ram and the full suite still runs fine after the changes.

hash-d · 2026-07-06T17:49:52Z

    kubeconfig: "{{ kubeconfig }}"
    pod: curl
-    command: >-
+    command: >


Apparently, this is the main thing that fixes this issue, though we're not sure what the mechanism for that is

hash-d · 2026-07-06T17:50:36Z

-          exit 1;
-        fi
+      response=$(curl -s -w "\n%{http_code}"  "{{ run_curl_address }}");
+      code=$(echo "$response" | tail -n 1);


Having a single call to curl is definitely an improvement, and not using a tmpfile for that is good, too.

ramdrvcs force-pushed the main branch from 4f56ce3 to 466bb5c Compare June 24, 2026 09:16

ramdrvcs marked this pull request as ready for review June 24, 2026 09:24

ramdrvcs requested review from c-kruse, fgiorgetti and nluaces as code owners June 24, 2026 09:24

coderabbitai Bot reviewed Jun 24, 2026

View reviewed changes

Comment thread tests/e2e/collections/ansible_collections/e2e/tests/roles/run_curl/tasks/main.yml Outdated

ramdrvcs force-pushed the main branch 2 times, most recently from 4b694b7 to e654414 Compare June 24, 2026 10:21

ramdrvcs marked this pull request as draft June 24, 2026 12:39

ramdrvcs marked this pull request as ready for review June 24, 2026 12:39

hash-d requested review from granzoto and hash-d June 24, 2026 12:52

hash-d reviewed Jul 3, 2026

View reviewed changes

coderabbitai Bot reviewed Jul 6, 2026

View reviewed changes

Comment thread tests/e2e/collections/ansible_collections/e2e/tests/roles/run_curl/tasks/main.yml

ramdrvcs requested a review from hash-d July 6, 2026 16:55

Fixing the lexical error Hello-world test

8c1c775

ramdrvcs force-pushed the main branch from c8ec43c to 8c1c775 Compare July 6, 2026 17:34

hash-d approved these changes Jul 6, 2026

View reviewed changes

hash-d merged commit bb3c383 into skupperproject:main Jul 6, 2026
3 checks passed

ramdrvcs mentioned this pull request Jul 6, 2026

Fixing the lexical error Hello-world test #2522

Merged

3 tasks

Uh oh!

Conversation

ramdrvcs commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Key Changes

Testing Triaged

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hash-d left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hash-d left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ramdrvcs commented Jun 24, 2026 •

edited

Loading

coderabbitai Bot commented Jun 24, 2026 •

edited

Loading