Replace legacy k6-action in test workflows #846
Conversation
📝 WalkthroughWalkthroughGitHub Actions workflows refactored to replace grafana/k6-action with grafana/setup-k6-action and grafana/run-k6-action, switching from filename-based to path-based test inputs. Test files updated to source environment variables from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-04-24T06:59:32.575ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
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. 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/regression-test-ATX.yml (1)
9-21: Add missing Slack webhook secret to required secrets.The workflow references
${{ secrets.SLACK_WEBHOOK_URL_TEST }}at line 71, but this secret is not declared in the required secrets list (lines 9–21). This will cause the workflow to fail when invoked as a reusable workflow.Add the missing secret to the workflow inputs:
secrets: WEBHOOK_ENDPOINT: required: true + SLACK_WEBHOOK_URL_TEST: + required: trueAlso applies to: 71-71
.github/workflows/use-case-TT02.yaml (1)
48-58: Confirm TT02 test environment uses PROD webhook secret—likely misconfiguration.The workflow explicitly defines
environment: TT02and runs scheduled test automation every 15 minutes against a test environment. However, it uses${{ secrets.SLACK_WEBHOOK_URL_PROD }}for Slack notifications, inconsistent with the pattern used by other test environments (e.g., ATX workflow uses${{ secrets.SLACK_WEBHOOK_URL_TEST }}).This issue also appears in
regression-test-TT02.yamlat line 53, suggesting a systematic copy-paste error across TT02 workflows.Update line 53 to use the appropriate test webhook secret, such as
${{ secrets.SLACK_WEBHOOK_URL_TEST }}.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/regression-test-ATX.yml(1 hunks).github/workflows/regression-test-PROD.yaml(2 hunks).github/workflows/regression-test-TT02.yaml(2 hunks).github/workflows/use-case-ATX.yaml(2 hunks).github/workflows/use-case-PROD.yaml(2 hunks).github/workflows/use-case-TT02.yaml(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: hggutvik
Repo: Altinn/altinn-events PR: 715
File: .github/workflows/regression-test-TT02.yaml:13-13
Timestamp: 2025-04-24T06:59:32.575Z
Learning: When reviewing GitHub Actions workflows, using commit SHAs (rather than just semver tags) is a security best practice that prevents supply chain attacks. The pattern of using a commit SHA with a version comment (e.g., `11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2`) provides both security and readability, and is supported by automated tools like Dependabot and Renovate.
🪛 actionlint (1.7.8)
.github/workflows/regression-test-ATX.yml
71-71: property "slack_webhook_url_test" is not defined in object type {actions_runner_debug: string; actions_step_debug: string; github_token: string; party_id: string; person_number: string; tokengenerator_username: string; tokengenerator_userpassword: string; user_id: string; webhook_endpoint: string}
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (csharp, autobuild)
🔇 Additional comments (10)
.github/workflows/regression-test-TT02.yaml (2)
15-17: Security posture: commit SHA pinning is correct. ✓All action references use commit SHAs with version comments (e.g.,
93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1), which is a security best practice that prevents supply chain attacks and is consistent with your established patterns.
16-40: I'll help you verify the concerns in this review comment about the k6 action migration. Let me start by checking the test file paths and then investigate the action compatibility.
<function_calls>
#!/bin/bashVerify that all k6 test file paths exist
paths=(
"test/k6/src/tests/app-events.js"
"test/k6/src/tests/events/post.js"
"test/k6/src/tests/events/get.js"
"test/k6/src/tests/subscriptions.js"
)echo "Checking k6 test file paths..."
missing=0
for path in "${paths[@]}"; do
if [ -f "$path" ]; then
echo "✓ $path"
else
echo "✗ $path (NOT FOUND)"
((missing++))
fi
doneif [ $missing -eq 0 ]; then
echo "All paths exist."
exit 0
else
echo "$missing path(s) not found."
exit 1
fi
</function_calls>Now let me search for information about the action compatibility:
<function_calls>
grafana setup-k6-action v1.1.0 run-k6-action v1.3.1 compatibility
</function_calls><function_calls>
grafana/setup-k6-action github documentation path parameter
</function_calls>.github/workflows/regression-test-PROD.yaml (6)
15-17: ✓ Actions use commit SHA references with version tags.Security best practice is followed: both
actions/checkoutandgrafana/setup-k6-actionare pinned to specific commit SHAs with version comments for readability and auditability.
41-58: ✓ Failure reporting logic preserved and well-structured.The build failure report and Slack notification steps are correctly maintained with proper conditional logic (
if: failure()), ensuring they only execute when tests fail. Step ordering is logical: report generation precedes notification.
15-17: ✓ Actions use commit SHA references with version tags.Security best practice is followed: both
actions/checkoutandgrafana/setup-k6-actionare pinned to specific commit SHAs with version comments for readability and auditability. Based on learnings, this supply-chain security approach is preferred.
19-40: ✓ Parameter migration fromfilenametopathis correct.The
grafana/run-k6-actionaction usespath:as its input parameter, so the refactoring correctly updates all four test invocations (lines 19, 25, 31, 37) from the legacyfilename:syntax. All run-k6-action steps use the same commit SHA reference, ensuring consistency. Theif: always()conditions on subsequent steps appropriately ensure all test suites execute sequentially regardless of prior failures.
41-58: ✓ Failure reporting logic is preserved and well-structured.The build failure report and Slack notification steps are correctly maintained with proper conditional logic (
if: failure()), ensuring they only execute when tests fail. Step ordering is logical: report generation precedes notification, and the Slack webhook configuration is intact.
19-40: Parameterpathis correct forgrafana/[email protected].The refactoring correctly uses the
pathparameter, which is the required input for grafana/run-k6-action v1.3.1 (notfilename). All run-k6-action steps are well-formed: they use consistent commit SHA pinning (same SHA across all invocations), maintain proper indentation, and correctly applyif: always()to ensure subsequent tests execute even if prior ones fail..github/workflows/use-case-PROD.yaml (1)
15-58: Well-structured k6 action migration.The setup-k6-action followed by multiple run-k6-action invocations follows the documented pattern for the new k6 GitHub Actions. All action versions use commit SHAs with version comments, which aligns with security best practices per learnings. Path parameters are correctly formatted, and environment secrets are appropriately scoped to the PROD environment.
.github/workflows/use-case-ATX.yaml (1)
19-44: Overall migration structure is solid.The setup-k6-action step followed by run-k6-action invocations follows the documented pattern. All action versions correctly use commit SHAs with version tags (security best practice per learnings). The matrix strategy with multiple AT environments and the use of
if: always()on subsequent tests ensures proper test continuation on failures.Aside from the path typo on line 43, confirm that the matrix-based dynamic environment variable propagation (e.g.,
${{ vars.ENV }}and${{ matrix.environment }}) is correctly configured in your GitHub environments for AT22, AT23, and AT24.
martivj
dismissed
their stale review
Outdated, we decided to test on this branch first.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/regression-test-ATX.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (csharp, autobuild)
🔇 Additional comments (3)
.github/workflows/regression-test-ATX.yml (3)
30-30: Environment context added correctly.The job now explicitly sets
environment: ${{ inputs.environment }}, which is the correct way to associate the workflow with the specified deployment environment and its protection rules.
32-34: Checkout and k6 setup steps look good.The addition of
grafana/setup-k6-action@ffe7d7290dfa715e48c2ccc924d068444c94bde2 # v1.1.0immediately after checkout follows the standard pattern and ensures k6 is available for subsequent steps.
35-56: Test execution steps migrated correctly to run-k6-action.The four test steps properly use:
- Path-based parameter (
path:instead of legacyfilename:)- Conditional execution (
if: always()) to ensure all tests run even if an earlier test fails- Consistent environment and secret passing via flags
- Pinned action version (
grafana/run-k6-action@a15e2072ede004e8d46141e33d7f7dad8ad08d9d # v1.3.1)The migration from the legacy k6-action to run-k6-action appears complete and correct.
Verify that the pinned versions of
grafana/setup-k6-action@ffe7d7290dfa715e48c2ccc924d068444c94bde2andgrafana/run-k6-action@a15e2072ede004e8d46141e33d7f7dad8ad08d9dare current and secure. You may want to check the Grafana Actions releases or update to the latest stable versions if needed.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/regression-test-ATX.yml(1 hunks)test/k6/src/api/token-generator.js(1 hunks)test/k6/src/config.js(1 hunks)test/k6/src/setup.js(1 hunks)test/k6/src/tests/subscriptions.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/k6/src/api/token-generator.js
🧰 Additional context used
🧬 Code graph analysis (2)
test/k6/src/tests/subscriptions.js (1)
test/k6/src/tests/app-events.js (1)
app(34-34)
test/k6/src/setup.js (2)
test/k6/src/config.js (1)
environment(27-27)test/k6/src/api/token-generator.js (1)
environment(11-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (csharp, autobuild)
🔇 Additional comments (2)
.github/workflows/regression-test-ATX.yml (2)
31-35: LGTM! Environment configuration aligns with test expectations.The job-level environment variables properly map workflow inputs and secrets to the expected environment variable names used in the k6 test scripts. The
ALTINN_ENVvariable correctly replaces the legacyenvpattern used in previous test invocations.
37-60: LGTM! k6 action migration follows best practices.The workflow correctly adopts the modular pattern: a single
setup-k6-actionstep followed by multiplerun-k6-actioninvocations. The use ofif: always()ensures all regression tests execute even if earlier tests fail, which is appropriate for comprehensive regression testing. Action versions are pinned to specific commit SHAs for supply chain security.
test/k6/src/config.js
Outdated
|
|
||
| //Get values from environment | ||
| const environment = __ENV.env.toLowerCase(); | ||
| const environment = (__ENV.ALTINN_ENV || 'defaultVal').toLowerCase(); |
There was a problem hiding this comment.
Invalid fallback value for environment variable.
The fallback value 'defaultVal' is not a valid key in the baseUrls object (lines 2-8), which will cause baseUrl to be undefined when ALTINN_ENV is not set. This will result in malformed URLs throughout the configuration (e.g., "https://platform.undefined/events/...").
Consider using a valid environment key as the fallback (e.g., 'at22' for a test environment) or explicitly handling the undefined case.
Apply this diff to use a valid test environment as fallback:
-const environment = (__ENV.ALTINN_ENV || 'defaultVal').toLowerCase();
+const environment = (__ENV.ALTINN_ENV || 'at22').toLowerCase();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const environment = (__ENV.ALTINN_ENV || 'defaultVal').toLowerCase(); | |
| const environment = (__ENV.ALTINN_ENV || 'at22').toLowerCase(); |
🤖 Prompt for AI Agents
In test/k6/src/config.js around line 27, the fallback value 'defaultVal' for
(__ENV.ALTINN_ENV || 'defaultVal').toLowerCase() is not a valid key in the
baseUrls map and leads to baseUrl being undefined; replace the fallback with a
valid test environment key (for example 'at22') or explicitly handle the
undefined case by checking (__ENV.ALTINN_ENV ? __ENV.ALTINN_ENV.toLowerCase() :
'at22') so that a known valid environment is used when ALTINN_ENV is not set.
test/k6/src/setup.js
Outdated
| import { b64decode } from "k6/encoding"; | ||
|
|
||
| const environment = __ENV.env.toLowerCase(); | ||
| const environment = (__ENV.ALTINN_ENV || 'defaultVal').toLowerCase(); |
There was a problem hiding this comment.
Same invalid fallback value as in config.js.
The fallback value 'defaultVal' is not a valid environment identifier. While the impact here is less severe than in config.js (the conditional checks on lines 14 and 30 will simply not match and fall through to the token generator path), it's better to use a valid test environment key for consistency across the test suite.
Apply this diff to align with the suggested fix in config.js:
-const environment = (__ENV.ALTINN_ENV || 'defaultVal').toLowerCase();
+const environment = (__ENV.ALTINN_ENV || 'at22').toLowerCase();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const environment = (__ENV.ALTINN_ENV || 'defaultVal').toLowerCase(); | |
| const environment = (__ENV.ALTINN_ENV || 'at22').toLowerCase(); |
🤖 Prompt for AI Agents
In test/k6/src/setup.js around line 6, the fallback string 'defaultVal' is not a
valid environment key; replace it with the same valid test environment key used
in config.js (for example 'local' or whatever key config.js uses) so the line
becomes the same fallback as config.js and then call toLowerCase() as before.
…it phase (before env vars are available in runtime)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
test/k6/src/config.js (1)
27-28: Empty string fallback still causes undefined baseUrl.The fallback value
''(empty string) is not a valid key in thebaseUrlsobject (lines 2-8), which will causebaseUrlto beundefinedwhenaltinn_envis not set. This results in malformed URLs throughout the configuration (e.g.,"https://platform.undefined/events/...").Use a valid test environment key as the fallback (e.g.,
'at22') to ensure proper URL construction.Apply this diff:
-const environment = (__ENV.altinn_env || '').toLowerCase(); +const environment = (__ENV.altinn_env || 'at22').toLowerCase();test/k6/src/setup.js (1)
6-6: Empty string fallback creates inconsistency and potential issues.The fallback value
''(empty string) creates two problems:
Inconsistency across files:
test/k6/src/api/token-generator.jsuses'defaultVal'as fallback (line 9), while this file andconfig.jsnow use''. All files should use the same valid test environment key.Suboptimal fallback: While the empty string won't crash (the conditionals on lines 14 and 30 simply won't match), it passes an empty
envparameter to the token generator (line 20), which may cause issues.Use a valid test environment key like
'at22'consistently across all files.Apply this diff:
-const environment = (__ENV.altinn_env || '').toLowerCase(); +const environment = (__ENV.altinn_env || 'at22').toLowerCase();
🧹 Nitpick comments (1)
test/k6/src/tests/app-events.js (1)
34-34: Good defensive change to prevent runtime errors.The fallback to empty string prevents crashes when
__ENV.appis undefined. This is a solid improvement over the previous code that would throw an error.Note that an empty
appvalue may still cause issues in downstream API calls (line 68). Consider adding validation to provide a clearer error message ifappis required.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
test/k6/src/config.js(1 hunks)test/k6/src/setup.js(1 hunks)test/k6/src/tests/app-events.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
test/k6/src/setup.js (2)
test/k6/src/config.js (1)
environment(27-27)test/k6/src/api/token-generator.js (1)
environment(10-10)
test/k6/src/tests/app-events.js (1)
test/k6/src/tests/subscriptions.js (1)
app(39-39)
test/k6/src/config.js (2)
test/k6/src/setup.js (1)
environment(6-6)test/k6/src/api/token-generator.js (1)
environment(10-10)
2 participants
Description
grafana/setup-k6-actionstep after the checkout step in all relevant workflowsgrafana/k6-actionwithgrafana/run-k6-actionfilenamewithpathSLACK_WEBHOOK_URL_TESTas a required secret inregression-test-ATX.ymlRelated Issue(s)
Verification
Documentation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.