Skip to content

fix: clarify SPIRE warning message to avoid confusion - #505

Merged
Alan-Cha merged 1 commit into
mainfrom
fix/clarify-spire-warning-message
Aug 11, 2026
Merged

fix: clarify SPIRE warning message to avoid confusion#505
Alan-Cha merged 1 commit into
mainfrom
fix/clarify-spire-warning-message

Conversation

@Alan-Cha

Copy link
Copy Markdown
Member

No description provided.

The existing warning 'SPIRE not detected for mTLS-enabled workload' is
confusing because:

1. SPIRE may be running (via Helm) but not detected by this check
2. JWT-SVID authentication works fine despite this warning
3. The check only looks for socket volumes in the pod spec

This commit clarifies that:
- The check is specific to X.509-SVID based mTLS
- JWT-SVID authentication is unaffected
- The message explains what's actually being checked

Fixes rossoctl/rossoctl#2361

Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
@Alan-Cha
Alan-Cha requested a review from a team as a code owner August 11, 2026 01:59
@Alan-Cha Alan-Cha added this to the Release v0.8.0 milestone Aug 11, 2026

@rubambiza rubambiza 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.

LGTM from a quick human review. AI-assisted review will come next with an approval and any nits/suggestions it may have from its broader context. I'm also using this to test the migrated github-pr-review skill in Claude Code 😉

@rubambiza rubambiza 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.

Clear improvement — the reworded log line ("SPIRE socket volumes not detected (X.509-SVID mTLS unavailable)") is more precise than the previous "SPIRE not detected," and the added comment correctly clarifies that this check is X.509-SVID-specific and that JWT-SVID SPIFFE auth still works when SPIRE is installed. Message/comment-only; no logic change.

  • nit: the PR description is empty — a short ## Summary of the wording change would help reviewers and the changelog.

Reviewed by clawgenti using the github-pr-review skill

@Alan-Cha
Alan-Cha merged commit 3d7a0fb into main Aug 11, 2026
17 checks passed
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.

2 participants