Skip to content

Commit ed40efb

Browse files
KT-DoanKT-Doan
committed
fix(scripts): repair migrate-deprecated-attrs.sh detection logic
The migrate-deprecated-attrs.sh script silently did nothing on files that actually contained deprecated attributes. Two problems prevented the script from working: - The `perl -ne` pre-check used `exit 0 if ...; END { exit 1 }` to signal a match. Perl's END block runs even after `exit 0` inside the implicit `while (<>)` loop that -n creates, so the END block always overrode the exit code to 1 and the outer `if` never ran the substitution. Replaced with a flag variable that END inspects. - The block-detection regex ended with `"ory_project_config"\b`. A `\b` between a non-word character (`"`) and a non-word character (space) never matches, so the substitution never entered the project_config block. Removed the redundant `\b`. Adds scripts/migrate-deprecated-attrs_test.sh with fixture-based tests covering the primary rename path, attrs outside project_config, duplicate names, removed attrs, the cross-resource base_redirect_uri warning, backup collision, and the no-op path. Adds a `make test-migrate-script` target for running the suite locally. Reported-by: John Colvin (dailypay/ory-network-terraform)
1 parent 4a6e4f0 commit ed40efb

3 files changed

Lines changed: 223 additions & 5 deletions

File tree

‎Makefile‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,10 @@ test: ## Run unit tests (no API calls)
156156
test-short: ## Run unit tests in short mode
157157
go test -v -short ./...
158158

159+
.PHONY: test-migrate-script
160+
test-migrate-script: ## Run integration tests for scripts/migrate-deprecated-attrs.sh
161+
./scripts/migrate-deprecated-attrs_test.sh
162+
159163
# Source .env file if it exists (for local acceptance tests).
160164
# Copy .env.example to .env and fill in your credentials.
161165
ifneq (,$(wildcard .env))

‎scripts/migrate-deprecated-attrs.sh‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,13 @@ while IFS= read -r -d '' file; do
118118
for entry in "${RENAMES[@]}"; do
119119
old="${entry%%=*}"
120120
new="${entry#*=}"
121-
# Only match HCL attribute assignments: fixed-string pre-check + perl assignment match
122-
if grep -qF "${old}" "$file" 2>/dev/null && perl -ne "exit 0 if /^\\s*\\Q${old}\\E\\s*=/; END { exit 1 }" "$file" 2>/dev/null; then
121+
# Only match HCL attribute assignments: fixed-string pre-check + perl assignment match.
122+
# Use a flag variable with the END block so the exit code survives the implicit
123+
# `while (<>)` loop that perl -n wraps around the code. A bare `exit 0` inside the
124+
# loop still triggers END, which would override the exit code back to 1.
125+
if grep -qF "${old}" "$file" 2>/dev/null && perl -ne "BEGIN { \$f = 0 } \$f = 1 if /^\\s*\\Q${old}\\E\\s*=/; END { exit(\$f ? 0 : 1) }" "$file" 2>/dev/null; then
123126
# Check if the new name already exists in the file to avoid duplicates
124-
if perl -ne "exit 0 if /^\\s*\\Q${new}\\E\\s*=/; END { exit 1 }" "$file" 2>/dev/null; then
127+
if perl -ne "BEGIN { \$f = 0 } \$f = 1 if /^\\s*\\Q${new}\\E\\s*=/; END { exit(\$f ? 0 : 1) }" "$file" 2>/dev/null; then
125128
echo " WARNING: $file: both '$old' and '$new' exist — skipping rename to avoid duplicate"
126129
continue
127130
fi
@@ -138,7 +141,7 @@ while IFS= read -r -d '' file; do
138141
# Tracks brace depth; handles opening brace on same or next line.
139142
perl -pi -e '
140143
BEGIN { $in_block = 0; $depth = 0; $seen_open = 0; }
141-
if (!$in_block && /^\s*resource\s+"ory_project_config"\b/ && !/^\s*#/ && !/^\s*\/\//) { $in_block = 1; $depth = 0; $seen_open = 0; }
144+
if (!$in_block && /^\s*resource\s+"ory_project_config"/ && !/^\s*#/ && !/^\s*\/\//) { $in_block = 1; $depth = 0; $seen_open = 0; }
142145
if ($in_block) {
143146
my $opens = () = /\{/g;
144147
my $closes = () = /\}/g;
@@ -160,7 +163,7 @@ done < <(find "$DIR" -type f -name '*.tf' -not -path '*/.terraform/*' -print0)
160163
FOUND_REMOVED=false
161164
while IFS= read -r -d '' file; do
162165
for removed in "${REMOVED[@]}"; do
163-
if grep -qF "$removed" "$file" 2>/dev/null && perl -ne "exit 0 if /^\\s*\\Q${removed}\\E\\s*=/; END { exit 1 }" "$file" 2>/dev/null; then
166+
if grep -qF "$removed" "$file" 2>/dev/null && perl -ne "BEGIN { \$f = 0 } \$f = 1 if /^\\s*\\Q${removed}\\E\\s*=/; END { exit(\$f ? 0 : 1) }" "$file" 2>/dev/null; then
164167
if [ "$FOUND_REMOVED" = false ]; then
165168
echo ""
166169
echo "WARNING: The following attributes have been removed and should be deleted:"

‎scripts/migrate-deprecated-attrs_test.sh‎

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
#!/usr/bin/env bash
2+
# migrate-deprecated-attrs_test.sh
3+
#
4+
# Integration tests for migrate-deprecated-attrs.sh. Exercises the script
5+
# against temporary fixture directories and verifies it renames the
6+
# attributes users actually hit in real project config files.
7+
#
8+
# Run:
9+
# ./scripts/migrate-deprecated-attrs_test.sh
10+
# or:
11+
# make test-migrate-script
12+
#
13+
# Exits non-zero on any failing assertion.
14+
15+
set -euo pipefail
16+
17+
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
18+
MIGRATE_SCRIPT="${SCRIPT_DIR}/migrate-deprecated-attrs.sh"
19+
20+
if [ ! -x "$MIGRATE_SCRIPT" ]; then
21+
echo "FAIL: migration script not executable at $MIGRATE_SCRIPT" >&2
22+
exit 1
23+
fi
24+
25+
PASS=0
26+
FAIL=0
27+
TMPROOT="$(mktemp -d)"
28+
trap 'rm -rf "$TMPROOT"' EXIT
29+
30+
pass() { echo " PASS: $1"; PASS=$((PASS + 1)); }
31+
fail() { echo " FAIL: $1" >&2; FAIL=$((FAIL + 1)); }
32+
33+
assert_contains() {
34+
local file="$1" needle="$2" desc="$3"
35+
if grep -qF "$needle" "$file"; then
36+
pass "$desc"
37+
else
38+
fail "$desc — expected to find '$needle' in $file"
39+
echo "--- $file ---" >&2
40+
cat "$file" >&2
41+
fi
42+
}
43+
44+
assert_not_contains() {
45+
local file="$1" needle="$2" desc="$3"
46+
if grep -qF "$needle" "$file"; then
47+
fail "$desc — did not expect to find '$needle' in $file"
48+
echo "--- $file ---" >&2
49+
cat "$file" >&2
50+
else
51+
pass "$desc"
52+
fi
53+
}
54+
55+
# ------------------------------------------------------------------------------
56+
# Test 1: renames deprecated attrs inside ory_project_config
57+
# ------------------------------------------------------------------------------
58+
echo "Test 1: renames deprecated attrs inside ory_project_config"
59+
t1="$TMPROOT/test1"
60+
mkdir -p "$t1"
61+
cat > "$t1/project.tf" <<'EOF'
62+
resource "ory_project_config" "main" {
63+
oauth2_login_url = "https://example.com/login"
64+
oauth2_consent_url = "https://example.com/consent"
65+
oauth2_issuer_url = "https://example.com/"
66+
login_ui_url = "https://example.com/ui/login"
67+
registration_ui_url = "https://example.com/ui/register"
68+
error_ui_url = "https://example.com/ui/error"
69+
settings_ui_url = "https://example.com/ui/settings"
70+
enable_password = true
71+
enable_oidc = true
72+
}
73+
EOF
74+
"$MIGRATE_SCRIPT" "$t1" >"$t1/out.log" 2>&1 || { fail "script exited non-zero"; cat "$t1/out.log" >&2; }
75+
assert_contains "$t1/project.tf" "oauth2_urls_login" "renames oauth2_login_url"
76+
assert_contains "$t1/project.tf" "oauth2_urls_consent" "renames oauth2_consent_url"
77+
assert_contains "$t1/project.tf" "oauth2_urls_self_issuer" "renames oauth2_issuer_url"
78+
assert_contains "$t1/project.tf" "selfservice_flows_login_ui_url" "renames login_ui_url"
79+
assert_contains "$t1/project.tf" "selfservice_flows_registration_ui_url" "renames registration_ui_url"
80+
assert_contains "$t1/project.tf" "selfservice_flows_error_ui_url" "renames error_ui_url"
81+
assert_contains "$t1/project.tf" "selfservice_flows_settings_ui_url" "renames settings_ui_url"
82+
assert_contains "$t1/project.tf" "selfservice_methods_password_enabled" "renames enable_password"
83+
assert_contains "$t1/project.tf" "selfservice_methods_oidc_enabled" "renames enable_oidc"
84+
assert_contains "$t1/out.log" "Migration complete. 1 file(s) updated." "reports 1 file updated"
85+
assert_contains "$t1/project.tf.bak" "oauth2_login_url" "backup preserves original attribute name"
86+
87+
# ------------------------------------------------------------------------------
88+
# Test 2: does not rename deprecated attrs outside ory_project_config blocks
89+
# ------------------------------------------------------------------------------
90+
echo "Test 2: does not rename deprecated attrs outside ory_project_config blocks"
91+
t2="$TMPROOT/test2"
92+
mkdir -p "$t2"
93+
cat > "$t2/other.tf" <<'EOF'
94+
resource "some_other_resource" "main" {
95+
login_ui_url = "https://example.com/ui/login"
96+
}
97+
EOF
98+
"$MIGRATE_SCRIPT" "$t2" >"$t2/out.log" 2>&1 || true
99+
assert_contains "$t2/other.tf" "login_ui_url" "leaves attr outside ory_project_config untouched"
100+
assert_not_contains "$t2/other.tf" "selfservice_flows_login_ui_url" "does not inject new name outside ory_project_config"
101+
102+
# ------------------------------------------------------------------------------
103+
# Test 3: warns when both old and new names are present
104+
# ------------------------------------------------------------------------------
105+
echo "Test 3: warns when both old and new names are present"
106+
t3="$TMPROOT/test3"
107+
mkdir -p "$t3"
108+
cat > "$t3/duplicate.tf" <<'EOF'
109+
resource "ory_project_config" "main" {
110+
login_ui_url = "https://example.com/old"
111+
selfservice_flows_login_ui_url = "https://example.com/new"
112+
}
113+
EOF
114+
"$MIGRATE_SCRIPT" "$t3" >"$t3/out.log" 2>&1 || true
115+
assert_contains "$t3/out.log" "both 'login_ui_url' and 'selfservice_flows_login_ui_url' exist" "reports duplicate"
116+
assert_contains "$t3/duplicate.tf" "login_ui_url" "leaves old name in place when duplicate"
117+
assert_contains "$t3/duplicate.tf" "selfservice_flows_login_ui_url" "leaves new name in place when duplicate"
118+
119+
# ------------------------------------------------------------------------------
120+
# Test 4: warns for removed attributes but does not rename them
121+
# ------------------------------------------------------------------------------
122+
echo "Test 4: warns for removed attributes"
123+
t4="$TMPROOT/test4"
124+
mkdir -p "$t4"
125+
cat > "$t4/removed.tf" <<'EOF'
126+
resource "ory_project_config" "main" {
127+
account_experience_name = "My App"
128+
account_experience_stylesheet = "https://example.com/style.css"
129+
oauth2_session_encrypt_at_rest = true
130+
}
131+
EOF
132+
"$MIGRATE_SCRIPT" "$t4" >"$t4/out.log" 2>&1 || true
133+
assert_contains "$t4/out.log" "following attributes have been removed" "emits removed-attrs warning"
134+
assert_contains "$t4/out.log" "account_experience_name" "lists account_experience_name"
135+
assert_contains "$t4/out.log" "oauth2_session_encrypt_at_rest" "lists oauth2_session_encrypt_at_rest"
136+
assert_contains "$t4/removed.tf" "account_experience_name" "removed attributes are not auto-deleted"
137+
138+
# ------------------------------------------------------------------------------
139+
# Test 5: warns about base_redirect_uri on ory_social_provider
140+
# ------------------------------------------------------------------------------
141+
echo "Test 5: warns about cross-resource base_redirect_uri"
142+
t5="$TMPROOT/test5"
143+
mkdir -p "$t5"
144+
cat > "$t5/social.tf" <<'EOF'
145+
resource "ory_social_provider" "github" {
146+
provider = "github"
147+
base_redirect_uri = "https://example.com"
148+
}
149+
EOF
150+
"$MIGRATE_SCRIPT" "$t5" >"$t5/out.log" 2>&1 || true
151+
assert_contains "$t5/out.log" "base_redirect_uri on ory_social_provider" "warns about cross-resource attr"
152+
assert_contains "$t5/out.log" "selfservice_methods_oidc_config_base_redirect_uri" "suggests correct new attribute"
153+
assert_not_contains "$t5/social.tf" "selfservice_methods_oidc_config_base_redirect_uri" "does not silently move cross-resource attr"
154+
155+
# ------------------------------------------------------------------------------
156+
# Test 6: refuses to overwrite an existing .bak file
157+
# ------------------------------------------------------------------------------
158+
echo "Test 6: refuses to overwrite existing .bak"
159+
t6="$TMPROOT/test6"
160+
mkdir -p "$t6"
161+
cat > "$t6/project.tf" <<'EOF'
162+
resource "ory_project_config" "main" {
163+
login_ui_url = "https://example.com/ui/login"
164+
}
165+
EOF
166+
cp "$t6/project.tf" "$t6/project.tf.bak"
167+
set +e
168+
"$MIGRATE_SCRIPT" "$t6" >"$t6/out.log" 2>&1
169+
rc=$?
170+
set -e
171+
if [ "$rc" -ne 0 ]; then
172+
pass "exits non-zero when .bak already exists"
173+
else
174+
fail "expected non-zero exit when .bak already exists, got $rc"
175+
fi
176+
assert_contains "$t6/out.log" "Backup file" "reports backup collision"
177+
178+
# ------------------------------------------------------------------------------
179+
# Test 7: no deprecated attrs → no changes, 0 files updated
180+
# ------------------------------------------------------------------------------
181+
echo "Test 7: file without deprecated attrs is untouched"
182+
t7="$TMPROOT/test7"
183+
mkdir -p "$t7"
184+
cat > "$t7/clean.tf" <<'EOF'
185+
resource "ory_project_config" "main" {
186+
selfservice_flows_login_ui_url = "https://example.com/ui/login"
187+
}
188+
EOF
189+
cp "$t7/clean.tf" "$t7/clean.tf.orig"
190+
"$MIGRATE_SCRIPT" "$t7" >"$t7/out.log" 2>&1 || true
191+
if cmp -s "$t7/clean.tf" "$t7/clean.tf.orig"; then
192+
pass "already-migrated file is unchanged"
193+
else
194+
fail "already-migrated file was modified"
195+
fi
196+
assert_contains "$t7/out.log" "Migration complete. 0 file(s) updated." "reports 0 files updated"
197+
if [ -e "$t7/clean.tf.bak" ]; then
198+
fail "no .bak should be created when nothing changes"
199+
else
200+
pass "no .bak created when nothing changes"
201+
fi
202+
203+
# ------------------------------------------------------------------------------
204+
# Summary
205+
# ------------------------------------------------------------------------------
206+
echo ""
207+
echo "Passed: $PASS"
208+
echo "Failed: $FAIL"
209+
if [ "$FAIL" -gt 0 ]; then
210+
exit 1
211+
fi

0 commit comments

Comments
 (0)