Remove unwanted Thunder references from tests folder#2499
Merged
brionmario merged 1 commit intoasgardeo:mainfrom Apr 29, 2026
Merged
Remove unwanted Thunder references from tests folder#2499brionmario merged 1 commit intoasgardeo:mainfrom
tests folder#2499brionmario merged 1 commit intoasgardeo:mainfrom
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (30)
📝 WalkthroughWalkthroughThis PR performs a systematic rebranding refactor, replacing "Thunder" product naming and environment variable conventions (THUNDER_) with generic "server" terminology and SERVER_ variables across configuration files, CI/CD workflows, test suites, utilities, and documentation. Directory names, email addresses, and utility class names are also updated accordingly. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 0/1 reviews remaining, refill in 16 minutes and 2 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
backend/cmd/server/bootstrap/01-default-resources.ps1 (1)
1-1: Optional: Consider adding BOM encoding for non-ASCII content.Static analysis flagged that this file contains non-ASCII characters (emojis like 🏛️ and 👨💻) but lacks a BOM. While this is not a functional issue—PowerShell 7+ (already required at line 28) handles UTF-8 without BOM correctly—adding a BOM would satisfy the analyzer and ensure maximum compatibility.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/cmd/server/bootstrap/01-default-resources.ps1` at line 1, This file contains non-ASCII emojis but lacks a UTF-8 BOM; save or rewrite the script so it begins with a UTF-8 BOM (EF BB BF) before the existing shebang "#!/usr/bin/env pwsh" to satisfy the static analyzer and improve compatibility—open the file in your editor or CI text tool and re-save as "UTF-8 with BOM" (ensuring the shebang and script content, including emojis like 🏛️ and 👨💻, remain unchanged).install/helm/values.yaml (1)
496-496: Consider updating other example secret names for consistency.The change to
server-setup-secretsaligns well with the PR's goal of removing Thunder-specific naming. However, other example secret names in this file still use thethunder-*prefix:
- Line 79:
secretName: thunder-runtime-secrets(indeployment.secretEnvexample)- Line 519:
secretName: thunder-secrets(insetup.extraVolumesexample)For consistency across documentation, consider updating these to generic names like
server-runtime-secretsandserver-secrets.📝 Suggested updates for consistency
Line 79:
-# secretName: thunder-runtime-secrets +# secretName: server-runtime-secretsLine 519:
-# secretName: thunder-secrets +# secretName: server-secrets🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@install/helm/values.yaml` at line 496, Update the remaining example secret names to match the new generic naming: change the `secretName` in the `deployment.secretEnv` example (currently `thunder-runtime-secrets`) to `server-runtime-secrets`, and change the `secretName` in the `setup.extraVolumes` example (currently `thunder-secrets`) to `server-secrets`, so they are consistent with the already-updated `server-setup-secrets`.samples/api/postman/authentication/authentication_demo.json (1)
801-801: Parameterize issuer instead of hardcoding localhost.At Line 801 and the repeated update payloads,
issueris fixed tohttps://localhost:8090. If{{baseUrl}}changes, issuer and runtime endpoints can drift.♻️ Suggested change pattern
- "issuer": "https://localhost:8090", + "issuer": "{{baseUrl}}",Apply this to all repeated app payload blocks in this file (top-level token/assertion issuer and nested token issuer fields).
Also applies to: 1753-1753, 1941-1941, 2118-2118, 2363-2363, 2608-2608, 2982-2982, 3245-3245
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@samples/api/postman/authentication/authentication_demo.json` at line 801, Replace the hardcoded "https://localhost:8090" in the app payloads with a template variable (e.g. "{{baseUrl}}" or "{{issuer}}") so issuers stay in sync with environment changes; specifically update the assertion.issuer and the nested inboundAuthConfig[].config.token.accessToken.issuer (and any other token.idToken issuer fields) across all payload blocks mentioned (lines referencing assertion issuer and token issuer) to use the template variable and apply the same change to each repeated block in the file.tests/integration/testutils/test_utils.go (1)
94-95: Clean up small comment typos for readabilityThere are a few wording slips in changed comments (e.g., “running the server”, “the the”, “residual the server”). Non-blocking, but worth polishing.
🧹 Suggested wording cleanup
-// GetServerPID returns the PID of the running the server process, or 0 if not started. +// GetServerPID returns the PID of the running server process, or 0 if not started. -// pidFilePath returns the path to the well-known PID file that always reflects the -// PID of the currently running the server process, even after subprocess restarts. +// pidFilePath returns the path to the well-known PID file that always reflects the +// PID of the currently running server process, even after subprocess restarts. -// writePidFile writes the given PID to the the server PID file. +// writePidFile writes the given PID to the server PID file. -// Kill any residual the server process that may have been started by a test +// Kill any residual server process that may have been started by a testAlso applies to: 599-600, 609-610, 762-763
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/testutils/test_utils.go` around lines 94 - 95, Fix small typos and awkward phrasing in the comments around GetServerPID: change “running the the server” to “running the server”, reword “residual the server” to “residual server” (or “leftover server process”), and polish sentences for clarity (e.g., “GetServerPID returns the PID of the running the server process, or 0 if not started.” → “GetServerPID returns the PID of the running server process, or 0 if it is not started.”). Apply the same wording fixes to the other nearby comment blocks that mirror this text (the subsequent comment groups corresponding to the same test utilities)..github/actions/release-notification/action.yml (1)
41-75: Make container URL configurable to match product-level branding changes.Line 41 is now product-driven, but Line 75 still hardcodes the Thunder container URL. This can drift from the displayed product name.
♻️ Suggested diff
inputs: webhook: description: 'Google Chat webhook URL' required: true product-name: description: 'Product name to display in the notification' required: true + container-url: + description: 'Container URL to open from the notification' + required: false + default: 'https://github.com/asgardeo/thunder/pkgs/container/thunder' @@ env: GOOGLE_CHAT_WEBHOOK: ${{ inputs.webhook }} RELEASE_NAME: ${{ inputs.release-name }} RELEASE_TAG: ${{ inputs.release-tag }} RELEASE_URL: ${{ inputs.release-url }} + CONTAINER_URL: ${{ inputs.container-url }} @@ "openLink": { - "url": "https://github.com/asgardeo/thunder/pkgs/container/thunder" + "url": "$CONTAINER_URL" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/release-notification/action.yml around lines 41 - 75, The "VIEW CONTAINER" button currently hardcodes "https://github.com/asgardeo/thunder/pkgs/container/thunder"; make the container URL configurable by adding a new action input (e.g., inputs.container-url) with a sensible default and replace the hardcoded URL in the "VIEW CONTAINER" button's openLink (the block containing "text": "VIEW CONTAINER") to use the new input (e.g., ${{ inputs.container-url }} or $CONTAINER_URL) so the container link follows the product-name branding..github/workflows/windows-build-validation.yml (1)
330-330: Align secure stderr redirection with expected error log path.At Line 330, stderr is merged into
server-secure.log, but Line 439 expectsserver-secure-err.log. The error log check will always be empty.Proposed fix
- ./thunder.exe > server-secure.log 2>&1 & + ./thunder.exe > server-secure.log 2> server-secure-err.log &Also applies to: 437-440
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/windows-build-validation.yml at line 330, The secure server launch currently merges stderr into the main secure log, but the test expects a separate error file; modify the thunder.exe startup invocation so stdout continues to go to server-secure.log while stderr is redirected to server-secure-err.log (i.e., split stderr out instead of merging it), and apply the same change for the other occurrences around the secure server startup checks so the subsequent error-file existence checks will find content.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/integration/application/consent_api_test.go`:
- Around line 110-114: The test assertion message passed to ts.Require().NoError
when calling testutils.RestartServer() contains a typo ("failed to restart y
with consent-enabled config"); update that message to use "server" instead of
"y" so it reads something like "failed to restart server with consent-enabled
config" to improve clarity for ts.Require().NoError in the test failure output.
---
Nitpick comments:
In @.github/actions/release-notification/action.yml:
- Around line 41-75: The "VIEW CONTAINER" button currently hardcodes
"https://github.com/asgardeo/thunder/pkgs/container/thunder"; make the container
URL configurable by adding a new action input (e.g., inputs.container-url) with
a sensible default and replace the hardcoded URL in the "VIEW CONTAINER"
button's openLink (the block containing "text": "VIEW CONTAINER") to use the new
input (e.g., ${{ inputs.container-url }} or $CONTAINER_URL) so the container
link follows the product-name branding.
In @.github/workflows/windows-build-validation.yml:
- Line 330: The secure server launch currently merges stderr into the main
secure log, but the test expects a separate error file; modify the thunder.exe
startup invocation so stdout continues to go to server-secure.log while stderr
is redirected to server-secure-err.log (i.e., split stderr out instead of
merging it), and apply the same change for the other occurrences around the
secure server startup checks so the subsequent error-file existence checks will
find content.
In `@backend/cmd/server/bootstrap/01-default-resources.ps1`:
- Line 1: This file contains non-ASCII emojis but lacks a UTF-8 BOM; save or
rewrite the script so it begins with a UTF-8 BOM (EF BB BF) before the existing
shebang "#!/usr/bin/env pwsh" to satisfy the static analyzer and improve
compatibility—open the file in your editor or CI text tool and re-save as "UTF-8
with BOM" (ensuring the shebang and script content, including emojis like 🏛️
and 👨💻, remain unchanged).
In `@install/helm/values.yaml`:
- Line 496: Update the remaining example secret names to match the new generic
naming: change the `secretName` in the `deployment.secretEnv` example (currently
`thunder-runtime-secrets`) to `server-runtime-secrets`, and change the
`secretName` in the `setup.extraVolumes` example (currently `thunder-secrets`)
to `server-secrets`, so they are consistent with the already-updated
`server-setup-secrets`.
In `@samples/api/postman/authentication/authentication_demo.json`:
- Line 801: Replace the hardcoded "https://localhost:8090" in the app payloads
with a template variable (e.g. "{{baseUrl}}" or "{{issuer}}") so issuers stay in
sync with environment changes; specifically update the assertion.issuer and the
nested inboundAuthConfig[].config.token.accessToken.issuer (and any other
token.idToken issuer fields) across all payload blocks mentioned (lines
referencing assertion issuer and token issuer) to use the template variable and
apply the same change to each repeated block in the file.
In `@tests/integration/testutils/test_utils.go`:
- Around line 94-95: Fix small typos and awkward phrasing in the comments around
GetServerPID: change “running the the server” to “running the server”, reword
“residual the server” to “residual server” (or “leftover server process”), and
polish sentences for clarity (e.g., “GetServerPID returns the PID of the running
the server process, or 0 if not started.” → “GetServerPID returns the PID of the
running server process, or 0 if it is not started.”). Apply the same wording
fixes to the other nearby comment blocks that mirror this text (the subsequent
comment groups corresponding to the same test utilities).
🪄 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: Pro
Run ID: 88d3a96d-fc08-46c0-ba96-c892eb050bb9
⛔ Files ignored due to path filters (4)
tests/e2e/package-lock.jsonis excluded by!**/package-lock.jsontests/integration/testutils/mock_consent_server.gois excluded by!**/mock_*.gotests/integration/testutils/mock_google_oidc_server.gois excluded by!**/mock_*.gotests/integration/testutils/mock_oidc_server.gois excluded by!**/mock_*.go
📒 Files selected for processing (29)
.agent/skills/console/SKILL.md.github/actions/release-notification/action.yml.github/workflows/pr-builder.yml.github/workflows/release-builder.yml.github/workflows/windows-build-validation.ymlbackend/cmd/server/bootstrap/01-default-resources.ps1backend/cmd/server/bootstrap/01-default-resources.shinstall/helm/values.yamlsamples/api/postman/authentication/authentication_demo.jsontests/e2e/package.jsontests/e2e/tests/applications/application-edit.spec.tstests/e2e/tests/applications/application-onboarding.spec.tstests/e2e/tests/sample-app-authentication/.env.exampletests/e2e/tests/sample-app-authentication/README-MFA.mdtests/e2e/tests/sample-app-authentication/sample-app-mfa-login.spec.tstests/e2e/utils/authentication/admin-api-auth.tstests/e2e/utils/server-setup/index.tstests/e2e/utils/server-setup/mfa-flow-nodes.jsontests/e2e/utils/server-setup/mfa-registration-flow-nodes.jsontests/e2e/utils/server-setup/mfa-setup.tstests/integration/application/consent_api_test.gotests/integration/authn/oidc_auth_test.gotests/integration/composite/composite_mode_api_test.gotests/integration/importexport/import_export_fresh_pack_test.gotests/integration/main.gotests/integration/oauth/wildcard/wildcard_redirect_uri_test.gotests/integration/testutils/api_security.gotests/integration/testutils/oauth2_utils.gotests/integration/testutils/test_utils.go
brionmario
added
dependencies-approved
skip-changelog
brionmario
force-pushed
the
refactor-thunder-rebrand
branch
from
dc262a6 to
10add1a
Compare
| 1. Verify Thunder server is running at `SERVER_URL` | ||
| 2. Check `ADMIN_USERNAME` and `ADMIN_PASSWORD` are correct | ||
| 3. Verify application has basic authentication flow configured | ||
| 4. Check Thunder logs for authentication errors |
Contributor
There was a problem hiding this comment.
need to update here as well
Member
Author
There was a problem hiding this comment.
Didn't update the references in Readme's since the content does not make sense without the product name.
| **Issue**: Failed to create notification sender, flow, or user | ||
|
|
||
| **Solutions**: | ||
| 1. Check Thunder server logs for detailed error messages |
Contributor
There was a problem hiding this comment.
Suggested change
| 1. Check server logs for detailed error messages |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2499 +/- ##
==========================================
- Coverage 87.37% 87.36% -0.01%
==========================================
Files 890 890
Lines 62738 62738
==========================================
- Hits 54817 54812 -5
- Misses 5932 5939 +7
+ Partials 1989 1987 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
brionmario
force-pushed
the
refactor-thunder-rebrand
branch
from
10add1a to
561c32e
Compare
KaveeshaPiumini
approved these changes
Apr 29, 2026
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
3 participants
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.
Purpose
This pull request focuses on standardizing naming conventions across the codebase, improving configuration clarity, and enhancing release notification flexibility. The most significant changes include renaming references from "thunder-server" and "THUNDER_URL" to "server" and "SERVER_URL", updating default admin email addresses, expanding OAuth grant types, and making the release notification action more generic.
Approach
Naming and Path Standardization:
thunder-serverdirectories andTHUNDER_URLenvironment variables have been renamed toserverandSERVER_URLrespectively across CI workflows, test scripts, and documentation to improve consistency and reduce confusion. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16]Configuration and Defaults Updates:
admin@thunder.devtoadmin@example.comin both PowerShell and Bash bootstrap scripts for improved privacy and clarity. [1] [2]authorization_codeandrefresh_tokengrant types, allowing for token refresh support in the console client. [1] [2]Release Notification Improvements:
product-nameinput, making it reusable for different products and updating the notification title accordingly. [1] [2] [3]Package and Dependency Metadata:
e2e-tests-thunderto@thunder/e2ein bothpackage.jsonandpackage-lock.json. Several dependencies now explicitly declare themselves as peer dependencies. [1] [2] [3] [4] [5] [6] [7] [8]Documentation:
Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
Chores
THUNDER_URL→SERVER_URL,THUNDER_EXTRACTED_HOME→SERVER_EXTRACTED_HOME).Documentation