[#1654] Added config verification check to provision.sh.#2304
Merged
AlexSkrypnyk merged 1 commit intomainfrom Feb 18, 2026
Merged
[#1654] Added config verification check to provision.sh.#2304AlexSkrypnyk merged 1 commit intomainfrom
AlexSkrypnyk merged 1 commit intomainfrom
Conversation
WalkthroughA new feature flag Changes
Sequence DiagramsequenceDiagram
participant Provisioning as Provisioning Script
participant Drush
participant ConfigBefore as Config Export (Before)
participant ConfigAfter as Config Export (After)
participant Diff as Diff Comparison
participant FileSystem as File System
rect rgba(100, 150, 200, 0.5)
Note over Provisioning: VORTEX_PROVISION_VERIFY_CONFIG_UNCHANGED_AFTER_UPDATE enabled
end
Provisioning->>Drush: drush config:export (pre-update)
Drush->>ConfigBefore: Write config snapshot
ConfigBefore->>FileSystem: Store to temp directory
rect rgba(150, 150, 100, 0.5)
Provisioning->>Drush: drush updatedb (run updates)
end
Provisioning->>Drush: drush config:export (post-update)
Drush->>ConfigAfter: Write config snapshot
ConfigAfter->>FileSystem: Store to temp directory
rect rgba(200, 150, 150, 0.5)
Provisioning->>Diff: Compare pre and post config
Diff->>FileSystem: Recursive diff
end
alt Differences detected
Diff-->>Provisioning: Diff output with changes
Provisioning->>Provisioning: FAIL provisioning<br/>Output diff & paths
else No differences
Diff-->>Provisioning: No output
Provisioning->>Provisioning: Continue provisioning
end
Provisioning->>FileSystem: Clean temp directories
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vortex/docs/content/development/variables.mdx:
- Line 404: The docs incorrectly list the default for the environment variable
VORTEX_PROVISION_VERIFY_CONFIG_UNCHANGED_AFTER_UPDATE as UNDEFINED; update the
table entry to show the concrete default value 0 so it matches the runtime
default set by the provisioning script (the variable uses shell parameter
expansion to default to 0 when unset).
In `@scripts/vortex/provision.sh`:
- Around line 316-343: Add a brief inline comment near the block that creates
and conditionally removes the temp dirs (around variables config_before and
config_after and the rm -rf call) stating that the temporary export directories
are intentionally preserved when configuration drift is detected (i.e., when
fail is invoked) to allow debugging/inspection, while they are removed on
success (pass), so the preserved dirs are not a resource leak but expected
behavior; place the comment just above the config_before/config_after creation
or just above the rm -rf to make the intent clear next to the relevant symbols.
| | <a id="vortex_provision_skip"></a>`VORTEX_PROVISION_SKIP` | Flag to skip site provisioning. | `UNDEFINED` | `scripts/vortex/provision.sh` | | ||
| | <a id="vortex_provision_type"></a>`VORTEX_PROVISION_TYPE` | Set to 'profile' to install a site from profile instead of the database dump. | `database` | `.env`, `scripts/vortex/provision.sh` | | ||
| | <a id="vortex_provision_use_maintenance_mode"></a>`VORTEX_PROVISION_USE_MAINTENANCE_MODE` | Put the site into a maintenance mode during site provisioning. | `1` | `.env`, `scripts/vortex/provision.sh` | | ||
| | <a id="vortex_provision_verify_config_unchanged_after_update"></a>`VORTEX_PROVISION_VERIFY_CONFIG_UNCHANGED_AFTER_UPDATE` | Verify that configuration was not changed by database updates. If enabled and config files are present, the provision will fail if database update hooks modify active configuration, preventing drush config:import from silently overwriting those changes. | `UNDEFINED` | `.env`, `scripts/vortex/provision.sh` | |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for the variable and its context in provision.sh
rg -n -C5 'VORTEX_PROVISION_VERIFY_CONFIG_UNCHANGED_AFTER_UPDATE' scripts/vortex/provision.shRepository: drevops/vortex
Length of output: 1967
Update documentation: default value should be 0, not UNDEFINED.
Line 404 documents the default as UNDEFINED, but scripts/vortex/provision.sh line 45 sets the actual default to 0:
VORTEX_PROVISION_VERIFY_CONFIG_UNCHANGED_AFTER_UPDATE="${VORTEX_PROVISION_VERIFY_CONFIG_UNCHANGED_AFTER_UPDATE:-0}"Update the table to reflect the concrete default value of 0.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vortex/docs/content/development/variables.mdx at line 404, The docs
incorrectly list the default for the environment variable
VORTEX_PROVISION_VERIFY_CONFIG_UNCHANGED_AFTER_UPDATE as UNDEFINED; update the
table entry to show the concrete default value 0 so it matches the runtime
default set by the provisioning script (the variable uses shell parameter
expansion to default to 0 when unset).
Comment on lines
+316
to
+343
|
|
||
| if [ "${VORTEX_PROVISION_VERIFY_CONFIG_UNCHANGED_AFTER_UPDATE}" = "1" ] && [ "${site_has_config_files}" = "1" ]; then | ||
| config_before=$(mktemp -d) | ||
| drush config:export --destination="${config_before}" | ||
|
|
||
| drush updatedb --no-cache-clear | ||
|
|
||
| config_after=$(mktemp -d) | ||
| drush config:export --destination="${config_after}" | ||
|
|
||
| config_diff=$(diff -rq "${config_before}" "${config_after}" || true) | ||
|
|
||
| if [ -n "${config_diff}" ]; then | ||
| fail "Configuration was changed by database updates." | ||
| note "The following configuration files were changed:" | ||
| echo "${config_diff}" | ||
| note "Configuration before updates: ${config_before}" | ||
| note "Configuration after updates: ${config_after}" | ||
| note "Review the update hooks and manually export updated configuration." | ||
| exit 1 | ||
| fi | ||
|
|
||
| rm -rf "${config_before}" "${config_after}" | ||
|
|
||
| pass "Verified that database updates did not change configuration." | ||
| else | ||
| drush updatedb --no-cache-clear | ||
| fi |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider documenting that temp directories are intentionally preserved on failure.
The implementation correctly preserves the config_before and config_after directories when configuration drift is detected (lines 332-333), allowing users to inspect the differences. However, these directories are cleaned up on success (line 338) but not on failure.
This is good behavior for debugging, but consider adding a brief comment noting this is intentional, as it could appear to be a resource leak at first glance.
💡 Optional: Add clarifying comment
if [ -n "${config_diff}" ]; then
fail "Configuration was changed by database updates."
note "The following configuration files were changed:"
echo "${config_diff}"
+ # Preserving temp directories for inspection - not cleaning up intentionally.
note "Configuration before updates: ${config_before}"
note "Configuration after updates: ${config_after}"
note "Review the update hooks and manually export updated configuration."
exit 1
fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/vortex/provision.sh` around lines 316 - 343, Add a brief inline
comment near the block that creates and conditionally removes the temp dirs
(around variables config_before and config_after and the rm -rf call) stating
that the temporary export directories are intentionally preserved when
configuration drift is detected (i.e., when fail is invoked) to allow
debugging/inspection, while they are removed on success (pass), so the preserved
dirs are not a resource leak but expected behavior; place the comment just above
the config_before/config_after creation or just above the rm -rf to make the
intent clear next to the relevant symbols.
Member
Author
|
|
Member
Author
|
Member
Author
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2304 +/- ##
==========================================
- Coverage 77.72% 77.19% -0.54%
==========================================
Files 117 110 -7
Lines 6182 6047 -135
Branches 44 0 -44
==========================================
- Hits 4805 4668 -137
- Misses 1377 1379 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
1 participant
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.
Closes #1654
Summary by CodeRabbit
New Features
Documentation
Tests