Persist Public-Private keypair used by Agent Manager Service for Token Generation #278
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
📝 WalkthroughWalkthroughThe changes implement persistent JWT signing key management by introducing a Kubernetes Job to generate RSA keypairs and store them as Secrets, updating the agent-manager-service deployment to mount the Secret volume, modifying the entrypoint script to detect and use mounted keys with a fallback for local development, and adding Helm configurations to orchestrate this flow. Changes
Sequence DiagramsequenceDiagram
participant Helm as Helm Deployment
participant Job as JWT Keys Job
participant Secret as Kubernetes Secret
participant Pod as Agent Manager Pod
participant Script as Entrypoint Script
participant App as Application
Helm->>Helm: Pre-install/Pre-upgrade Hook
Helm->>Job: Create JWT Key Generation Job
Job->>Job: Install OpenSSL, kubectl, curl
Job->>Secret: Check if JWT Secret exists
alt Secret Does Not Exist
Job->>Job: Generate 4096-bit RSA keypair
Job->>Job: Create public-keys-config.json
Job->>Secret: Create Secret with keys & config
Job->>Secret: Annotate with version, key-id, timestamp
else Secret Already Exists
Job->>Job: Log and exit successfully
end
Helm->>Pod: Deploy Agent Manager with volumeMount
Pod->>Secret: Mount secret at /app/keys
Pod->>Script: Start container with entrypoint
Script->>Script: Detect keys at /app/keys
alt Keys Found
Script->>App: Pass mounted keys to application
else Keys Not Found
Script->>Script: Attempt gen_keys.sh fallback
Script->>App: Use generated or mounted keys
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@agent-manager-service/entrypoint.sh`:
- Line 24: Remove the duplicate startup echo by deleting the early echo
"Starting agent-manager-service..." (the first echo invocation) and keep the
later echo that occurs after key handling; specifically remove the initial echo
statement so the single startup message comes only from the post-key-handling
echo.
In
`@deployments/helm-charts/wso2-ai-agent-management-platform/templates/jobs/jwt-keys-generation-job.yaml`:
- Around line 83-86: The conditional check using bash-ism `[[ ... ]]` needs to
be converted to POSIX-compliant tests so the job can run under Alpine's /bin/sh;
locate the if-statement that checks for private.pem, public.pem, and
public-keys-config.json and replace the `[[`/`]]` usage with POSIX `test`/`[`
checks (e.g., use `-f` checks combined with `||`) so the script uses only
/bin/sh-compatible syntax and still exits with the same error message when any
file is missing.
- Around line 68-80: The heredoc used to write public-keys-config.json is
indented, which inserts leading spaces into the JSON and breaks it; modify the
JWT keys generation step in jwt-keys-generation-job.yaml to use an unindented
heredoc (ensure the "cat > public-keys-config.json <<EOF" and the terminating
"EOF" are at column 0) or replace the heredoc with a non-indenting alternative
(e.g., printf/echo) to write the JSON, preserving the KEY_ID and TIMESTAMP
substitutions and the publicKeyPath/public keys structure unchanged.
🧹 Nitpick comments (1)
deployments/helm-charts/wso2-ai-agent-management-platform/templates/jobs/jwt-keys-generation-job.yaml (1)
36-36: Consider pinning a specific Alpine digest for reproducibility.Using
alpine:3.21is reasonable, but for production workloads, consider pinning to a specific digest to ensure reproducible builds and avoid unexpected changes from tag updates.
| @@ -23,13 +23,18 @@ set -e | |||
|
|
|||
| echo "Starting agent-manager-service..." | |||
Contributor
There was a problem hiding this comment.
Remove duplicate startup message.
"Starting agent-manager-service..." is printed twice: once at line 24 and again at line 41. Consider removing the first occurrence since the actual startup happens after key handling.
Proposed fix
echo "Starting agent-manager-service..."
-# Check if JWT signing keys exist (mounted from Kubernetes Secret)
+# Check if JWT signing keys exist (mounted from Kubernetes Secret)Or simply remove line 24:
-echo "Starting agent-manager-service..."
-
# Check if JWT signing keys exist (mounted from Kubernetes Secret)Also applies to: 41-41
🤖 Prompt for AI Agents
In `@agent-manager-service/entrypoint.sh` at line 24, Remove the duplicate startup
echo by deleting the early echo "Starting agent-manager-service..." (the first
echo invocation) and keep the later echo that occurs after key handling;
specifically remove the initial echo statement so the single startup message
comes only from the post-key-handling echo.
Comment on lines
+68
to
+80
| cat > public-keys-config.json <<EOF | ||
| { | ||
| "keys": [ | ||
| { | ||
| "kid": "${KEY_ID}", | ||
| "algorithm": "RS256", | ||
| "publicKeyPath": "/app/keys/public.pem", | ||
| "description": "JWT signing key generated by Helm", | ||
| "createdAt": "${TIMESTAMP}" | ||
| } | ||
| ] | ||
| } | ||
| EOF |
Contributor
There was a problem hiding this comment.
Heredoc indentation will produce malformed JSON.
The heredoc content is indented with leading spaces which will be included in the generated JSON file, making it invalid. The JSON keys will have leading whitespace.
Proposed fix: Use unindented heredoc or remove leading spaces
# Create public-keys-config.json matching the format expected by gen_keys.sh
- cat > public-keys-config.json <<EOF
- {
- "keys": [
- {
- "kid": "${KEY_ID}",
- "algorithm": "RS256",
- "publicKeyPath": "/app/keys/public.pem",
- "description": "JWT signing key generated by Helm",
- "createdAt": "${TIMESTAMP}"
- }
- ]
- }
- EOF
+ cat > public-keys-config.json <<'EOF'
+{
+ "keys": [
+ {
+ "kid": "PLACEHOLDER_KEY_ID",
+ "algorithm": "RS256",
+ "publicKeyPath": "/app/keys/public.pem",
+ "description": "JWT signing key generated by Helm",
+ "createdAt": "PLACEHOLDER_TIMESTAMP"
+ }
+ ]
+}
+EOF
+ # Replace placeholders with actual values
+ sed -i "s/PLACEHOLDER_KEY_ID/${KEY_ID}/g" public-keys-config.json
+ sed -i "s/PLACEHOLDER_TIMESTAMP/${TIMESTAMP}/g" public-keys-config.jsonOr use a simpler approach with printf/echo:
# Create public-keys-config.json matching the format expected by gen_keys.sh
- cat > public-keys-config.json <<EOF
- {
- ...
- }
- EOF
+ cat > public-keys-config.json << EOF
+{
+ "keys": [
+ {
+ "kid": "${KEY_ID}",
+ "algorithm": "RS256",
+ "publicKeyPath": "/app/keys/public.pem",
+ "description": "JWT signing key generated by Helm",
+ "createdAt": "${TIMESTAMP}"
+ }
+ ]
+}
+EOFNote: The EOF marker must be at the start of the line (no leading indentation).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| cat > public-keys-config.json <<EOF | |
| { | |
| "keys": [ | |
| { | |
| "kid": "${KEY_ID}", | |
| "algorithm": "RS256", | |
| "publicKeyPath": "/app/keys/public.pem", | |
| "description": "JWT signing key generated by Helm", | |
| "createdAt": "${TIMESTAMP}" | |
| } | |
| ] | |
| } | |
| EOF | |
| cat > public-keys-config.json << EOF | |
| { | |
| "keys": [ | |
| { | |
| "kid": "${KEY_ID}", | |
| "algorithm": "RS256", | |
| "publicKeyPath": "/app/keys/public.pem", | |
| "description": "JWT signing key generated by Helm", | |
| "createdAt": "${TIMESTAMP}" | |
| } | |
| ] | |
| } | |
| EOF |
🤖 Prompt for AI Agents
In
`@deployments/helm-charts/wso2-ai-agent-management-platform/templates/jobs/jwt-keys-generation-job.yaml`
around lines 68 - 80, The heredoc used to write public-keys-config.json is
indented, which inserts leading spaces into the JSON and breaks it; modify the
JWT keys generation step in jwt-keys-generation-job.yaml to use an unindented
heredoc (ensure the "cat > public-keys-config.json <<EOF" and the terminating
"EOF" are at column 0) or replace the heredoc with a non-indenting alternative
(e.g., printf/echo) to write the JSON, preserving the KEY_ID and TIMESTAMP
substitutions and the publicKeyPath/public keys structure unchanged.
Comment on lines
+83
to
+86
| if [[ ! -f private.pem ]] || [[ ! -f public.pem ]] || [[ ! -f public-keys-config.json ]]; then | ||
| echo "ERROR: Key generation failed - missing key files" | ||
| exit 1 | ||
| fi |
Contributor
There was a problem hiding this comment.
Critical: Bash syntax [[ ]] not supported in Alpine's /bin/sh.
The script uses [[ ]] test syntax which is a bash-ism, but Alpine's default shell is ash/busybox sh, which doesn't support this syntax. This will cause the job to fail with a syntax error.
Proposed fix: Use POSIX-compliant test syntax
- if [[ ! -f private.pem ]] || [[ ! -f public.pem ]] || [[ ! -f public-keys-config.json ]]; then
+ if [ ! -f private.pem ] || [ ! -f public.pem ] || [ ! -f public-keys-config.json ]; then📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if [[ ! -f private.pem ]] || [[ ! -f public.pem ]] || [[ ! -f public-keys-config.json ]]; then | |
| echo "ERROR: Key generation failed - missing key files" | |
| exit 1 | |
| fi | |
| if [ ! -f private.pem ] || [ ! -f public.pem ] || [ ! -f public-keys-config.json ]; then | |
| echo "ERROR: Key generation failed - missing key files" | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
In
`@deployments/helm-charts/wso2-ai-agent-management-platform/templates/jobs/jwt-keys-generation-job.yaml`
around lines 83 - 86, The conditional check using bash-ism `[[ ... ]]` needs to
be converted to POSIX-compliant tests so the job can run under Alpine's /bin/sh;
locate the if-statement that checks for private.pem, public.pem, and
public-keys-config.json and replace the `[[`/`]]` usage with POSIX `test`/`[`
checks (e.g., use `-f` checks combined with `||`) so the script uses only
/bin/sh-compatible syntax and still exits with the same error message when any
file is missing.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
Fixes #275
Persist Public-Private keypair used by Agent Manager Service for Token Generation by externalizing JWT signing key management using Kubernetes Secrets with the agent-management-platform helm chart.
Goals
Approach
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit
Release Notes