Merged
Conversation
Contributor
There was a problem hiding this comment.
3 issues found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/CHANGELOG.md">
<violation number="1" location="docs/CHANGELOG.md:23">
P1: Custom agent: **Changelog Review Agent**
Changelog entry is missing the required user-centric before/after explanation and reference link(s), so it doesn’t follow the mandated changelog structure.</violation>
</file>
<file name="internal/controller/operator/factory/vmauth/vmusers_config.go">
<violation number="1" location="internal/controller/operator/factory/vmauth/vmusers_config.go:193">
P2: JWT branch always returns false, so secret updates (e.g., changed `name`) are dropped and never persisted.</violation>
</file>
<file name="api/operator/v1beta1/vmuser_types.go">
<violation number="1" location="api/operator/v1beta1/vmuser_types.go:339">
P1: Validate JWT config requires either `skipVerify=true` or at least one public key/public key ref. Right now `spec.jwt: {}` passes validation and can produce an empty `jwt` section.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
f801044 to
4c14d2c
Compare
4c14d2c to
eafe415
Compare
eafe415 to
b7dfa57
Compare
vrutkovs
approved these changes
Mar 17, 2026
| return fmt.Errorf("exactly one spec.jwt.{skipVerify,publicKeys|publicKeyRefs,oidc} JWT verification mechanism is expected, got: [%s]", strings.Join(jwtVerification, ",")) | ||
| } | ||
| } | ||
| if len(authMechanisms) > 1 { |
Collaborator
There was a problem hiding this comment.
We didn't have a check for this before, but what if its zero? Is it treated as unauthenticated user?
Contributor
Author
There was a problem hiding this comment.
this check was added as with this PR more auth mechanisms are supported
if there are more than 1 auth mechanism set vmauth fails during configuration validation. if there're no auth mechanisms VMUser is treated as basic auth user
Signed-off-by: Vadim Rutkovsky <vadim@vrutkovs.eu>
c1f46a9 to
45b87cc
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
related issue #591
related issue VictoriaMetrics/VictoriaMetrics#9439
Summary by cubic
Adds JWT auth to
VMUserforvmauth, with OIDC issuer support and claim-based routing. Verification supports inline public keys,Secretrefs (merged at build), or skip-verify for testing, with validation enforcing exactly one mechanism.VMUserSpec.jwt:skipVerify,publicKeys,publicKeyRefs,matchClaims, andoidc.issuer.username,bearerToken, orjwt; and exactly one ofjwt.{skipVerify | publicKeys/publicKeyRefs | oidc};oidc.issueris required whenoidcis set.jwt.skip_verify,jwt.public_keys,jwt.match_claims, andjwt.oidc.issuer;publicKeyRefsare resolved intopublicKeys.Written for commit 45b87cc. Summary will update on new commits.