feat(kiali): Update Kiali url#402
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: josunect The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @josunect. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughOpenShift Kiali setup now repairs the Istio validating webhook, validates Bookinfo through Kiali, discovers the Kiali Route endpoint, and writes an OpenShift-specific MCP configuration overlay. ChangesOpenShift Kiali setup
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant setup-kiali-openshift
participant Kubernetes
participant Kiali
participant MCPConfig
setup-kiali-openshift->>Kubernetes: repair Istio validating webhook
setup-kiali-openshift->>Kubernetes: install Bookinfo
setup-kiali-openshift->>Kiali: validate Bookinfo health
setup-kiali-openshift->>Kiali: discover Route and probe API paths
Kiali-->>setup-kiali-openshift: return working endpoint
setup-kiali-openshift->>MCPConfig: write OpenShift Kiali overlay
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
build/openshift/kiali.mk (1)
199-201: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCloned Service carries over source annotations/labels, not just the deleted fields.
The
jq del(...)only stripsresourceVersion,uid,creationTimestamp,managedFields,ownerReferences,status, and the ClusterIP fields. Any othermetadata.annotations(e.g. Helm'smeta.helm.sh/release-name/release-namespace) and all ofmetadata.labelsfromistiod-default-v*are copied verbatim onto the newistiodService. Since.spec.selector(untouched by this cleanup) is what actually determines which pods the alias targets,metadata.labelsisn't needed for correctness — but carrying stale Helm-release annotations onto a manually created "istiod" Service can confuse future reconciliation/ownership checks by the Sail/Istio operator against a resource it doesn't actually manage.♻️ Suggested cleanup
- jq 'del(.metadata.resourceVersion,.metadata.uid,.metadata.creationTimestamp,.metadata.managedFields,.metadata.ownerReferences,.status,.spec.clusterIP,.spec.clusterIPs) | .metadata.name="istiod" | .metadata.annotations["mcp.local/alias-for"]=$$rev' --arg rev "$$rev_svc" | \ + jq 'del(.metadata.resourceVersion,.metadata.uid,.metadata.creationTimestamp,.metadata.managedFields,.metadata.ownerReferences,.metadata.labels,.metadata.annotations,.status,.spec.clusterIP,.spec.clusterIPs) | .metadata.name="istiod" | .metadata.annotations={"mcp.local/alias-for":$$rev}' --arg rev "$$rev_svc" | \🤖 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 `@build/openshift/kiali.mk` around lines 199 - 201, Update the jq cleanup in the Service clone pipeline to remove the source metadata.annotations and metadata.labels before setting the new name and alias annotation. Preserve the existing field deletions, selector, and apply behavior, while retaining only the newly assigned mcp.local/alias-for annotation.
🤖 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 `@build/openshift/kiali.mk`:
- Line 153: Update the probe commands around kiali_token and the curl
invocations to avoid placing the bearer token in command-line arguments. Write
the Authorization header to a short-lived, restrictive-permission temporary
file, pass it to curl through a supported configuration or header-file
mechanism, and remove the file after all probes complete, including failure
paths.
- Around line 158-171: Make the base URL selection in the probe logic explicit
by replacing the two independent case statements with an ordered conditional
that documents and enforces the intended priority between base_kiali and
base_root when both probes succeed. Preserve the existing fallback to base_root
when neither probe responds successfully, and keep the HTTP status matching
behavior unchanged.
---
Nitpick comments:
In `@build/openshift/kiali.mk`:
- Around line 199-201: Update the jq cleanup in the Service clone pipeline to
remove the source metadata.annotations and metadata.labels before setting the
new name and alias annotation. Preserve the existing field deletions, selector,
and apply behavior, while retaining only the newly assigned mcp.local/alias-for
annotation.
🪄 Autofix (Beta)
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: Enterprise
Run ID: 54d790fd-6b00-40a8-bde1-b1de463697d5
📒 Files selected for processing (1)
build/openshift/kiali.mk
Updates OpenShift Kiali local setup so MCP evals can talk to the cluster Route without changing the tracked config.
write-kiali-mcp-config-openshift
fix-istiod-validating-webhook
setup-kiali-openshift
Summary by CodeRabbit