Skip to content

#893 - Fix deploy-k8s.sh to match project conventions #894

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yoosful wants to merge 1 commit into
base: main
Choose a base branch
from
Open

#893 - Fix deploy-k8s.sh to match project conventions #894

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions deployments/scripts/deploy-k8s.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,8 @@ sidecars:
enabled: false
oauth2Proxy:
enabled: false
podMonitor:
enabled: false
EOF

# UI values
Expand Down Expand Up @@ -509,6 +511,8 @@ services:
sidecars:
otel:
enabled: false
podMonitor:
enabled: false
EOF

log_success "Helm values files created"
Expand Down Expand Up @@ -577,13 +581,16 @@ setup_backend_operator() {

if command -v osmo &> /dev/null; then
log_info "Logging into OSMO..."
osmo login http://localhost:9000 --method=dev --username=testuser || true
osmo login http://localhost:9000 --method=dev --username=admin || true

log_info "Creating backend-operator user..."
osmo user create backend-operator --roles osmo-backend 2>/dev/null || true
Comment on lines +584 to +587

@coderabbitai coderabbitai Bot Apr 23, 2026

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.

⚠️ Potential issue | 🟠 Major

Don’t swallow bootstrap failures here.

Both the admin login and backend-operator creation ignore every error with || true. If either step fails because the port-forward is not ready, auth changed, or role resolution fails, the script continues into token generation and can still fall back to the placeholder secret path, leaving the backend operator broken again. Please fail fast on login, and only treat the specific “already exists” case as non-fatal for user creation.

🔧 Suggested hardening 
-            osmo login http://localhost:9000 --method=dev --username=admin || true
+            if ! osmo login http://localhost:9000 --method=dev --username=admin; then
+                log_error "Failed to log into OSMO; cannot create backend-operator token"
+                kill "$port_forward_pid" 2>/dev/null || true
+                return 1
+            fi
 
             log_info "Creating backend-operator user..."
-            osmo user create backend-operator --roles osmo-backend 2>/dev/null || true
+            local user_create_output=""
+            if ! user_create_output=$(osmo user create backend-operator --roles osmo-backend 2>&1); then
+                if ! grep -qi "already exists" <<<"$user_create_output"; then
+                    printf '%s\n' "$user_create_output" >&2
+                    log_error "Failed to create backend-operator user"
+                    kill "$port_forward_pid" 2>/dev/null || true
+                    return 1
+                fi
+            fi
📝 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
osmo login http://localhost:9000 --method=dev --username=admin || true
log_info "Creating backend-operator user..."
osmo user create backend-operator --roles osmo-backend 2>/dev/null || true
if ! osmo login http://localhost:9000 --method=dev --username=admin; then
log_error "Failed to log into OSMO; cannot create backend-operator token"
kill "$port_forward_pid" 2>/dev/null || true
return 1
fi
log_info "Creating backend-operator user..."
local user_create_output=""
if ! user_create_output=$(osmo user create backend-operator --roles osmo-backend 2>&1); then
if ! grep -qi "already exists" <<<"$user_create_output"; then
printf '%s\n' "$user_create_output" >&2
log_error "Failed to create backend-operator user"
kill "$port_forward_pid" 2>/dev/null || true
return 1
fi
fi
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@deployments/scripts/deploy-k8s.sh` around lines 584 - 587, Remove the
unconditional "|| true" that swallows failures from the osmo login command so
the script exits if admin login fails (refer to the osmo login
http://localhost:9000 --method=dev --username=admin invocation); for the user
creation step (log_info "Creating backend-operator user..." and osmo user create
backend-operator --roles osmo-backend) only treat the specific "already exists"
outcome as non-fatal by checking the command output/exit status and failing the
script on any other error, instead of blanket-ignoring all failures.


log_info "Generating backend operator token..."
local backend_token=$(osmo token set backend-token \
--expires-at "$BACKEND_TOKEN_EXPIRY" \
--description "Backend Operator Token" \
--service \
--user backend-operator \
--roles osmo-backend \
-t json 2>/dev/null | jq -r '.token' || echo "")

Expand Down