Skip to content

Conversation

@sprudel
Copy link
Contributor

@sprudel sprudel commented Sep 21, 2025

Removed incorrect and redundant helm test, which does not verify correct runtime behavior of the deployment.

Description

A helm test should verify that the deployed app responds correctly. The helm test covers basic Kubernetes functionality instead.
Additionally it pulls in busybox when not really needed to the deployment when running helm test.

What problem is being solved?

Break dependency to busybox:latest

How is it being solved?

Break dependency to busybox:latest.

Review Checklist

  • I have clicked on "allow edits by maintainers".
  • I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
  • The correct base branch is being used, if not main
  • I have added tests to validate that the change in functionality is working as expected

Summary by CodeRabbit

  • Tests
    • Removed Helm chart test for auth secret (test-auth-secret), including the test Pod and its associated Secret, to streamline testing and reduce maintenance.
  • Chores
    • Cleaned up obsolete test resources from the chart to simplify the release package and avoid shipping unnecessary test artifacts.

@sprudel sprudel requested review from a team as code owners September 21, 2025 15:37
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 21, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: rhamzeh / name: Raghd Hamzeh (f4c31d5)
  • ✅ login: sprudel / name: Hannes Herrmann (22d11b8)

@sprudel
Copy link
Contributor Author

sprudel commented Sep 22, 2025

Is there anything open which blocks this fix of being merged? The original test file was introduced here
#212

@whoisxx @jeremy-albuixech

@sprudel
Copy link
Contributor Author

sprudel commented Sep 29, 2025

Perhaps @rhamzeh

@rhamzeh
Copy link
Member

rhamzeh commented Oct 6, 2025

I don't have a strong argument for why this should stay, and I don't see anyone else making that argument. I'm OK with removing it for now.

@coderabbitai
Copy link

coderabbitai bot commented Oct 6, 2025

Walkthrough

Removed Helm test hook resources by deleting charts/openfga/templates/tests/test-auth-secret.yaml, which defined a test Pod and a Secret used to validate OPENFGA_AUTHN_PRESHARED_KEYS_SECRET via environment variables.

Changes

Cohort / File(s) Summary
Helm chart test removal
charts/openfga/templates/tests/test-auth-secret.yaml
Delete Helm test Pod and Secret hook that verified auth pre-shared keys via env var

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title concisely and accurately summarizes the primary change—the removal of an incorrect and redundant Helm test—and is clear and specific enough for reviewers.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e9fe53c and f4c31d5.

📒 Files selected for processing (1)
  • charts/openfga/templates/tests/test-auth-secret.yaml (0 hunks)
💤 Files with no reviewable changes (1)
  • charts/openfga/templates/tests/test-auth-secret.yaml

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@rhamzeh rhamzeh enabled auto-merge (squash) October 6, 2025 17:27
@rhamzeh rhamzeh merged commit c62f882 into openfga:main Oct 6, 2025
8 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