Skip to content

fix(docs): correct Jenkins external-CI example payload and token request - #809

Open
kaviththiranga wants to merge 5 commits into
openchoreo:mainfrom
kaviththiranga:fix/external-ci-jenkins-example
Open

fix(docs): correct Jenkins external-CI example payload and token request#809
kaviththiranga wants to merge 5 commits into
openchoreo:mainfrom
kaviththiranga:fix/external-ci-jenkins-example

Conversation

@kaviththiranga

@kaviththiranga kaviththiranga commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

1. Wrong Workload payload (Step 3, Jenkins)

The example posts:

{"containers":{"main":{"image":"..."}}}

The Workload schema requires metadata.name plus spec.owner{projectName, componentName} and spec.container.image. Posting the documented body returns:

{"code":"INTERNAL_ERROR","error":"Internal server error"}
HTTP 500

The GitHub Actions example immediately below it on the same page already uses the
correct shape — only the Jenkins block was wrong.

2. No 409 handling (Step 3, Jenkins)

The example uses curl -sf with a bare POST and no fallback. The first build
creates the Workload; every subsequent build gets HTTP 409, curl -sf exits
non-zero, and the stage fails. Since Jenkins jobs re-run constantly this is the
first thing a real user hits. The GitHub Actions example already handles it; this
change brings the Jenkins one in line.

3. Token request uses HTTP Basic (Step 1)

The verification snippet passes credentials with curl -u. Backend Service
applications are registered with token_endpoint_auth_method: client_secret_post,
so that request fails:

{"error":"unauthorized_client",
 "error_description":"Client is not allowed to use the specified token endpoint authentication method"}
HTTP 400

Changed to form parameters (--data-urlencode) and added a note explaining why.
A set +x guard was also added to the Jenkins example: Jenkins masks injected
credentials in the build log but not the bearer token derived from them.

4. Wrong jenkins.io/job-full-name annotation format (Step 4)

The guide told users to set the annotation to a URL path
(/job/my-org/job/my-service). The Backstage plugin expects the job full
name
(my-org/my-service) and inserts the /job/ segments itself, so the
documented value resolves to a folder literally named job and the Jenkins tab
404s. Fixed here and in the developer guide's CI overview.

5. New: occ CLI alternative for the registration step

Added a short "Using the OpenChoreo CLI instead of curl" subsection after the
Jenkins example. occ login --client-credentials + occ workload create +
occ apply performs the same registration with no jq, no 409 fallback, and no
set +x (the CLI reads credentials from env vars and never prints the token).
The subsection also covers the things to watch: pin the CLI version to the
OpenChoreo release, point HOME at the per-build workspace because occ login
stores the client secret in ~/.openchoreo/config, and keep endpoints in the
workload.yaml descriptor since occ apply replaces the whole spec.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Documentation
    • Added jq prerequisites and Jenkins installation guidance.
    • Expanded Jenkins deployment instructions to create or update workloads and report status and failures.
    • Added an occ CLI-based Jenkins deployment alternative.
    • Updated OAuth token request examples for URL-encoded form parameters.
    • Updated GitHub Actions token requests and troubleshooting commands.
    • Clarified that Jenkins annotations and setup use the job’s full name, with standalone and foldered examples.

Walkthrough

The external CI documentation updates token requests, Jenkins prerequisites, Workload deployment methods, error handling, and Jenkins job naming. It also updates the external CI setup guide and troubleshooting commands.

Changes

External CI workflow

Layer / File(s) Summary
ThunderID token authentication
docs/platform-engineer-guide/workflows/external-ci.mdx
Token requests use URL-encoded OAuth parameters. GitHub Actions and troubleshooting examples use the same format.
Jenkins Workload deployment
docs/platform-engineer-guide/workflows/external-ci.mdx
The guide adds curl and jq prerequisites. Jenkins examples create or update Workload CRs with curl or the occ CLI and report deployment failures.
Jenkins job naming
docs/developer-guide/workflows/ci/overview.md, docs/platform-engineer-guide/workflows/external-ci.mdx
The documentation uses Jenkins job full names instead of /job/... paths for configuration and annotations.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Jenkins
  participant ThunderID
  participant WorkloadAPI
  Jenkins->>ThunderID: Request token with URL-encoded credentials
  ThunderID-->>Jenkins: Return access token
  Jenkins->>WorkloadAPI: Create or apply Workload CR
  WorkloadAPI-->>Jenkins: Return success, conflict, or error details
  Jenkins->>WorkloadAPI: Update Workload CR after conflict
  WorkloadAPI-->>Jenkins: Return deployment response
Loading

Possibly related PRs

Suggested reviewers: chalindukodikara

🚥 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.
Title check ✅ Passed The title clearly identifies the Jenkins external-CI payload and token-request fixes covered by the pull request.
Description check ✅ Passed The description clearly explains the fixes and their verification, but it omits the template headings, related issues, and checklist.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

Signed-off-by: Kavith Lokuhewage <kaviththiranga@gmail.com>
@kaviththiranga
kaviththiranga force-pushed the fix/external-ci-jenkins-example branch from 098e32b to b1f1ed7 Compare August 4, 2026 23:53

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/platform-engineer-guide/workflows/external-ci.mdx`:
- Around line 176-181: Update the PUT fallback in the external CI workflow
around the curl request to capture both its HTTP status and response body
instead of relying on curl -sf. Apply the same explicit non-2xx validation used
by the POST request, preserving the response body for error reporting and
ensuring all non-2xx PUT responses are reported.
- Around line 72-74: Update both OAuth token request examples in
docs/platform-engineer-guide/workflows/external-ci.mdx (lines 72-74 and 151-153)
to use curl’s --data-urlencode for every form field, including grant_type,
client_id, and client_secret, instead of raw -d values.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 28b6fd83-3f66-4285-aed2-231c1c6eda70

📥 Commits

Reviewing files that changed from the base of the PR and between bca8952 and 098e32b.

⛔ Files ignored due to path filters (1)
  • versioned_docs/version-v1.2.x/platform-engineer-guide/workflows/external-ci.mdx is excluded by !versioned_docs/**
📒 Files selected for processing (1)
  • docs/platform-engineer-guide/workflows/external-ci.mdx

Comment thread docs/platform-engineer-guide/workflows/external-ci.mdx Outdated
Comment thread docs/platform-engineer-guide/workflows/external-ci.mdx Outdated
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

- url-encode OAuth form values
- check the PUT fallback status, not just curl -sf
- note the jq prerequisite and that jenkins/jenkins:lts lacks it
- set +x so the bearer token is not echoed into the build log

Signed-off-by: Kavith Lokuhewage <kaviththiranga@gmail.com>
@kaviththiranga
kaviththiranga force-pushed the fix/external-ci-jenkins-example branch from 3117d78 to 06d9285 Compare August 5, 2026 02:49
The documented /job/my-org/job/my-service is a URL path; the plugin expects a
job full name and inserts the /job/ segments itself.

Signed-off-by: Kavith Lokuhewage <kaviththiranga@gmail.com>
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

Signed-off-by: Kavith Lokuhewage <kaviththiranga@gmail.com>
@kaviththiranga
kaviththiranga force-pushed the fix/external-ci-jenkins-example branch from e10287e to a0d2cd1 Compare August 5, 2026 04:57

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/platform-engineer-guide/workflows/external-ci.mdx (1)

171-175: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Fail when token acquisition fails.

The pipeline returns the jq status, not the curl status. If curl fails before it returns JSON, jq can succeed with no input and assign an empty TOKEN. The pipeline then reports a Workload API error instead of the token request failure.

Capture the token response before parsing it. Require a non-empty .access_token.

Proposed fix
-                        TOKEN=$(curl -sf -X POST "${THUNDER_URL}/oauth2/token" \
+                        if ! TOKEN_RESPONSE=$(curl -sf -X POST "${THUNDER_URL}/oauth2/token" \
                           --data-urlencode "grant_type=client_credentials" \
                           --data-urlencode "client_id=${CLIENT_ID}" \
-                          --data-urlencode "client_secret=${CLIENT_SECRET}" \
-                          | jq -r '.access_token')
+                          --data-urlencode "client_secret=${CLIENT_SECRET}"); then
+                          echo "Token request failed"
+                          exit 1
+                        fi
+
+                        if ! TOKEN=$(printf '%s' "${TOKEN_RESPONSE}" | jq -er '.access_token'); then
+                          echo "Token response has no access token"
+                          exit 1
+                        fi
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/platform-engineer-guide/workflows/external-ci.mdx` around lines 171 -
175, Update the token acquisition block for TOKEN to capture the curl response
separately before passing it to jq, so curl failures propagate instead of being
masked by the pipeline. Parse the captured response and validate that
.access_token is non-empty, failing the workflow immediately when the request
fails or no token is returned.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@docs/platform-engineer-guide/workflows/external-ci.mdx`:
- Around line 171-175: Update the token acquisition block for TOKEN to capture
the curl response separately before passing it to jq, so curl failures propagate
instead of being masked by the pipeline. Parse the captured response and
validate that .access_token is non-empty, failing the workflow immediately when
the request fails or no token is returned.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2127650e-5494-41ac-8d6d-322353fbfa40

📥 Commits

Reviewing files that changed from the base of the PR and between fffbd8d and cd7359a.

⛔ Files ignored due to path filters (1)
  • versioned_docs/version-v1.2.x/platform-engineer-guide/workflows/external-ci.mdx is excluded by !versioned_docs/**
📒 Files selected for processing (1)
  • docs/platform-engineer-guide/workflows/external-ci.mdx

Signed-off-by: Kavith Lokuhewage <kaviththiranga@gmail.com>
@kaviththiranga
kaviththiranga force-pushed the fix/external-ci-jenkins-example branch from cd7359a to 41a9e52 Compare August 5, 2026 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant