Skip to content

Conversation

@kickster97
Copy link
Member

@kickster97 kickster97 commented Nov 24, 2025

WHAT is this pull request doing?

The OpenAPI schema listed simple algorithm names (Bcrypt, SHA256, etc.) as enum values for hashing_algorithm, but the API returns normalized values like lavinmq_bcrypt and rabbit_password_hashing_sha256. This causes Terraform to detect configuration drift when the inserted value didn't match the retrieved value.

Updated users.yaml to document the normalized values the API actually returns and explain the flexible input format it accepts. The API still accepts simplified algorithm names (case-insensitive)

HOW can this pull request be tested?

Look at http api users endpoints.

@kickster97 kickster97 requested a review from a team as a code owner November 24, 2025 09:27
@claude
Copy link

claude bot commented Nov 24, 2025

Claude finished @kickster97's task —— View job


Reviewing PR for issues

  • Read the changed file to understand the modifications
  • Analyze the changes for potential issues
  • Check if Carl's comment about other algorithms reveals additional problems

Review Analysis

No issues found.

The PR correctly addresses the schema inconsistency. The code analysis confirms:

✅ API behavior is correctly documented:

  • API accepts input like "Bcrypt", "SHA256", etc. (case-insensitive, suffix matching)
  • API returns normalized values: lavinmq_bcrypt, rabbit_password_hashing_sha256, rabbit_password_hashing_sha512, rabbit_password_hashing_md5
  • Schema now properly documents these normalized return values

✅ All algorithms consistently handled:
In response to Carl's comment about prefixes - all algorithms ARE consistently prefixed in the API responses:

  • bcrypt → lavinmq_bcrypt (src/lavinmq/auth/password.cr:18)
  • sha256 → rabbit_password_hashing_sha256 (src/lavinmq/auth/password.cr:108)
  • sha512 → rabbit_password_hashing_sha512 (src/lavinmq/auth/password.cr:130)
  • md5 → rabbit_password_hashing_md5 (src/lavinmq/auth/password.cr:152)

The schema update correctly documents all normalized values, fixing the Terraform drift issue.

@carlhoerberg
Copy link
Member

isn't it the same for the others? prefix with either lavinmq_ or rabbitmq_?

@kickster97 kickster97 merged commit c96fef5 into main Nov 27, 2025
17 of 18 checks passed
@kickster97 kickster97 deleted the docs_hash_pass_bcrypt branch November 27, 2025 07:12
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.

4 participants