mirror of
https://github.com/taylanbakircioglu/haproxy-openmanager.git
synced 2026-09-20 17:43:29 +00:00
fix: ACME setup guide accuracy, reject rollback, and UX improvements
- Step 3 (Enable ACME on Cluster) now shows a process icon instead of a misleading green checkmark when ACME is enabled but not yet applied. Per-cluster "(pending apply)" annotation for multi-cluster setups. - Step 4 button and all /apply-management navigation buttons now say "Apply Changes" instead of "Configure" for clearer guidance. - Setup Guide auto-selects the correct cluster before navigating to Apply Management, showing pending cluster names in alerts. - Pending ACME disable changes are now correctly detected in Step 4 even when acme_enabled is already FALSE in the database. - Entity snapshot rollback for cluster ACME settings: reject correctly restores acme_enabled/acme_backend_url to pre-change values. - Deduplication logic prevents "last wins" bug when multiple ACME toggles are rejected in sequence. - Connection leak prevention with try/finally around conn2 in ACME config version creation. - Step 4 branching uses boolean has_enabled instead of fragile string truthiness check. Made-with: Cursor
This commit is contained in:
+36
-12
@@ -337,19 +337,40 @@ async def update_cluster(cluster_id: int, cluster: HAProxyClusterUpdate, authori
|
||||
update_query = f"UPDATE haproxy_clusters SET {', '.join(update_fields)} WHERE id = $1"
|
||||
await conn.execute(update_query, *update_values)
|
||||
|
||||
# If acme_enabled actually changed, create a PENDING config version
|
||||
# If acme_enabled actually changed, create a PENDING config version with entity snapshot
|
||||
if cluster.acme_enabled is not None and cluster.acme_enabled != existing_cluster.get('acme_enabled', False):
|
||||
try:
|
||||
from services.haproxy_config import generate_haproxy_config_for_cluster
|
||||
config_content = await generate_haproxy_config_for_cluster(cluster_id)
|
||||
import time as _time
|
||||
import json as _json
|
||||
version_name = f"cluster-{cluster_id}-acme-{'enable' if cluster.acme_enabled else 'disable'}-{int(_time.time())}"
|
||||
|
||||
from utils.entity_snapshot import save_entity_snapshot
|
||||
snapshot_metadata = await save_entity_snapshot(
|
||||
conn=conn,
|
||||
entity_type="cluster",
|
||||
entity_id=cluster_id,
|
||||
old_values={
|
||||
"acme_enabled": existing_cluster.get('acme_enabled', False),
|
||||
"acme_backend_url": existing_cluster.get('acme_backend_url'),
|
||||
},
|
||||
new_values={
|
||||
"acme_enabled": cluster.acme_enabled,
|
||||
"acme_backend_url": getattr(cluster, 'acme_backend_url', None) or existing_cluster.get('acme_backend_url'),
|
||||
},
|
||||
operation="UPDATE"
|
||||
)
|
||||
metadata_json = _json.dumps(snapshot_metadata) if snapshot_metadata else None
|
||||
|
||||
conn2 = await get_database_connection()
|
||||
await conn2.execute("""
|
||||
INSERT INTO config_versions (cluster_id, version_name, config_content, status, created_by)
|
||||
VALUES ($1, $2, $3, 'PENDING', $4)
|
||||
""", cluster_id, version_name, config_content, current_user.get('id', 1))
|
||||
await close_database_connection(conn2)
|
||||
try:
|
||||
await conn2.execute("""
|
||||
INSERT INTO config_versions (cluster_id, version_name, config_content, status, created_by, metadata)
|
||||
VALUES ($1, $2, $3, 'PENDING', $4, $5)
|
||||
""", cluster_id, version_name, config_content, current_user.get('id', 1), metadata_json)
|
||||
finally:
|
||||
await close_database_connection(conn2)
|
||||
except Exception as acme_err:
|
||||
logger.error(f"Failed to create ACME config version for cluster {cluster_id}: {acme_err}")
|
||||
|
||||
@@ -4572,6 +4593,7 @@ async def reject_all_pending_changes(cluster_id: int, authorization: str = Heade
|
||||
rollback_success_count = 0
|
||||
rollback_fail_count = 0
|
||||
rollback_skip_count = 0
|
||||
rolled_back_entities = set()
|
||||
|
||||
for version in pending_versions:
|
||||
try:
|
||||
@@ -4586,24 +4608,25 @@ async def reject_all_pending_changes(cluster_id: int, authorization: str = Heade
|
||||
logger.info(f"REJECT DEBUG: entity_snapshot exists={entity_snapshot is not None}")
|
||||
|
||||
if entity_snapshot:
|
||||
# SSL entity rollback: Always rollback since Auto-Reject handles cross-cluster consistency
|
||||
# Auto-Reject (below) marks ALL remaining PENDING SSL versions as REJECTED on all clusters,
|
||||
# so there's no cross-cluster downgrade risk from rolling back the SSL content
|
||||
if entity_snapshot.get('entity_type') == 'ssl_certificate':
|
||||
entity_key = (entity_snapshot.get('entity_type'), entity_snapshot.get('entity_id'))
|
||||
|
||||
if entity_key in rolled_back_entities:
|
||||
logger.info(f"REJECT ROLLBACK: Skipping duplicate rollback for {entity_key[0]} {entity_key[1]} (already restored from oldest snapshot)")
|
||||
rollback_skip_count += 1
|
||||
elif entity_snapshot.get('entity_type') == 'ssl_certificate':
|
||||
ssl_entity_id = entity_snapshot.get('entity_id')
|
||||
|
||||
logger.info(
|
||||
f"REJECT: Rolling back SSL certificate {ssl_entity_id} content to pre-update state"
|
||||
)
|
||||
success = await rollback_entity_from_snapshot(conn, entity_snapshot)
|
||||
if success:
|
||||
rollback_success_count += 1
|
||||
rolled_back_entities.add(entity_key)
|
||||
logger.info(f"REJECT ROLLBACK: Rolled back SSL certificate {ssl_entity_id}")
|
||||
else:
|
||||
rollback_fail_count += 1
|
||||
logger.warning(f"REJECT ROLLBACK: Failed to rollback SSL certificate {ssl_entity_id}")
|
||||
else:
|
||||
# Single entity rollback (non-SSL)
|
||||
logger.info(f"REJECT DEBUG: Calling rollback for {entity_snapshot.get('entity_type')} {entity_snapshot.get('entity_id')}")
|
||||
logger.info(f"REJECT DEBUG: old_values exists={('old_values' in entity_snapshot)}")
|
||||
logger.info(f"REJECT DEBUG: operation={entity_snapshot.get('operation')}")
|
||||
@@ -4614,6 +4637,7 @@ async def reject_all_pending_changes(cluster_id: int, authorization: str = Heade
|
||||
|
||||
if success:
|
||||
rollback_success_count += 1
|
||||
rolled_back_entities.add(entity_key)
|
||||
logger.info(f"REJECT ROLLBACK: Rolled back {entity_snapshot['entity_type']} {entity_snapshot['entity_id']}")
|
||||
else:
|
||||
rollback_fail_count += 1
|
||||
|
||||
@@ -773,45 +773,90 @@ async def check_prerequisites(authorization: str = Header(None)):
|
||||
"action": "register_account",
|
||||
})
|
||||
|
||||
# Pre-fetch ACME-related pending versions (used by both Step 3 and Step 4)
|
||||
pending_rows = await conn.fetch("""
|
||||
SELECT c.id AS cluster_id, c.name AS cluster_name, COUNT(*) AS cnt
|
||||
FROM config_versions cv
|
||||
JOIN haproxy_clusters c ON c.id = cv.cluster_id
|
||||
WHERE cv.status = 'PENDING'
|
||||
AND c.is_active = TRUE
|
||||
AND (c.acme_enabled = TRUE OR cv.version_name LIKE 'cluster-%-acme-%')
|
||||
GROUP BY c.id, c.name
|
||||
""")
|
||||
pending_count = sum(r['cnt'] for r in pending_rows)
|
||||
pending_clusters = [{"id": r['cluster_id'], "name": r['cluster_name'], "count": r['cnt']} for r in pending_rows]
|
||||
pending_enable_cluster_ids = set()
|
||||
pe_rows = await conn.fetch("""
|
||||
SELECT DISTINCT cv.cluster_id
|
||||
FROM config_versions cv
|
||||
JOIN haproxy_clusters c ON c.id = cv.cluster_id
|
||||
WHERE cv.status = 'PENDING'
|
||||
AND c.is_active = TRUE
|
||||
AND cv.version_name LIKE 'cluster-%-acme-enable-%'
|
||||
""")
|
||||
for r in pe_rows:
|
||||
pending_enable_cluster_ids.add(r['cluster_id'])
|
||||
|
||||
# Step 3: Cluster ACME enabled
|
||||
acme_cluster_count = await conn.fetchval(
|
||||
"SELECT COUNT(*) FROM haproxy_clusters WHERE acme_enabled = TRUE AND is_active = TRUE"
|
||||
)
|
||||
acme_cluster_names = await conn.fetch(
|
||||
"SELECT name FROM haproxy_clusters WHERE acme_enabled = TRUE AND is_active = TRUE"
|
||||
)
|
||||
cluster_ok = acme_cluster_count > 0
|
||||
cluster_detail = (
|
||||
f"Enabled on: {', '.join(r['name'] for r in acme_cluster_names)}"
|
||||
if cluster_ok
|
||||
else "No clusters have ACME Challenge Routing enabled"
|
||||
acme_clusters_rows = await conn.fetch(
|
||||
"SELECT id, name FROM haproxy_clusters WHERE acme_enabled = TRUE AND is_active = TRUE"
|
||||
)
|
||||
has_enabled = len(acme_clusters_rows) > 0
|
||||
any_pending_enable = has_enabled and any(r['id'] in pending_enable_cluster_ids for r in acme_clusters_rows)
|
||||
if has_enabled:
|
||||
name_parts = []
|
||||
for r in acme_clusters_rows:
|
||||
if r['id'] in pending_enable_cluster_ids:
|
||||
name_parts.append(f"{r['name']} (pending apply)")
|
||||
else:
|
||||
name_parts.append(r['name'])
|
||||
if any_pending_enable:
|
||||
cluster_ok = "pending"
|
||||
cluster_detail = f"Enabled on: {', '.join(name_parts)} — go to Apply Management to activate"
|
||||
cluster_navigate = "/apply-management"
|
||||
step3_pending_clusters = [{"id": r['cluster_id'], "name": r['cluster_name'], "count": r['cnt']} for r in pending_rows if r['cluster_id'] in pending_enable_cluster_ids]
|
||||
else:
|
||||
cluster_ok = True
|
||||
cluster_detail = f"Enabled on: {', '.join(name_parts)}"
|
||||
cluster_navigate = "/clusters"
|
||||
step3_pending_clusters = []
|
||||
else:
|
||||
cluster_ok = False
|
||||
cluster_detail = "No clusters have ACME Challenge Routing enabled"
|
||||
cluster_navigate = "/clusters"
|
||||
step3_pending_clusters = []
|
||||
steps.append({
|
||||
"key": "cluster_acme_enabled",
|
||||
"title": "Enable ACME on Cluster",
|
||||
"ok": cluster_ok,
|
||||
"detail": cluster_detail,
|
||||
"navigate": "/clusters",
|
||||
"navigate": cluster_navigate,
|
||||
"pending_clusters": step3_pending_clusters,
|
||||
})
|
||||
|
||||
# Step 4: Configuration applied (depends on step 3)
|
||||
if cluster_ok:
|
||||
pending_count = await conn.fetchval("""
|
||||
SELECT COUNT(*) FROM config_versions
|
||||
WHERE status = 'PENDING'
|
||||
AND cluster_id IN (SELECT id FROM haproxy_clusters WHERE acme_enabled = TRUE AND is_active = TRUE)
|
||||
""")
|
||||
# Step 4: Configuration applied
|
||||
if has_enabled:
|
||||
config_ok = pending_count == 0
|
||||
config_detail = "All ACME cluster configurations are applied" if config_ok else f"{pending_count} pending configuration change(s) need to be applied"
|
||||
if config_ok:
|
||||
config_detail = "All ACME cluster configurations are applied"
|
||||
else:
|
||||
names = ', '.join(r['cluster_name'] for r in pending_rows)
|
||||
config_detail = f"{pending_count} pending configuration change(s) on cluster: {names}"
|
||||
elif pending_count > 0:
|
||||
config_ok = False
|
||||
names = ', '.join(r['cluster_name'] for r in pending_rows)
|
||||
config_detail = f"{pending_count} pending configuration change(s) on cluster: {names}"
|
||||
else:
|
||||
config_ok = None
|
||||
config_detail = "Enable ACME on a cluster first, then apply changes"
|
||||
pending_clusters = []
|
||||
steps.append({
|
||||
"key": "config_applied",
|
||||
"title": "Apply Configuration Changes",
|
||||
"ok": config_ok,
|
||||
"detail": config_detail,
|
||||
"navigate": "/apply-management",
|
||||
"pending_clusters": pending_clusters,
|
||||
})
|
||||
|
||||
# Step 5: DNS & Network (informational only)
|
||||
|
||||
@@ -479,6 +479,21 @@ async def _rollback_update(
|
||||
logger.info(f"ROLLBACK UPDATE: SSL certificate {entity_id} restored to previous state (including content and expiry)")
|
||||
return True
|
||||
|
||||
elif entity_type == "cluster":
|
||||
await conn.execute("""
|
||||
UPDATE haproxy_clusters SET
|
||||
acme_enabled = $1,
|
||||
acme_backend_url = $2,
|
||||
updated_at = CURRENT_TIMESTAMP
|
||||
WHERE id = $3
|
||||
""",
|
||||
old_values.get('acme_enabled'),
|
||||
old_values.get('acme_backend_url'),
|
||||
entity_id
|
||||
)
|
||||
logger.info(f"ROLLBACK UPDATE: Cluster {entity_id} acme_enabled restored to {old_values.get('acme_enabled')}")
|
||||
return True
|
||||
|
||||
elif entity_type == "server":
|
||||
# Server'ı eski değerlerine geri yükle
|
||||
# SCHEMA: backend_servers (ALL FIELDS from migrations.py line 2010-2036)
|
||||
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
RocketOutlined
|
||||
} from '@ant-design/icons';
|
||||
import { useNavigate } from 'react-router-dom';
|
||||
import { useCluster } from '../contexts/ClusterContext';
|
||||
import axios from 'axios';
|
||||
|
||||
const { Option } = Select;
|
||||
@@ -21,6 +22,7 @@ const getErrorMsg = (err, fallback) =>
|
||||
|
||||
const ACMEAutomation = () => {
|
||||
const navigate = useNavigate();
|
||||
const { clusters: allClusters, selectCluster } = useCluster();
|
||||
const [orders, setOrders] = useState([]);
|
||||
const [accounts, setAccounts] = useState([]);
|
||||
const [renewalSchedule, setRenewalSchedule] = useState([]);
|
||||
@@ -400,20 +402,31 @@ const ACMEAutomation = () => {
|
||||
style={{ marginBottom: 16 }}
|
||||
/>
|
||||
)}
|
||||
{prerequisites?.steps?.find(s => s.key === 'config_applied' && s.ok === false) && (
|
||||
<Alert
|
||||
type="warning"
|
||||
showIcon
|
||||
message="Configuration Not Applied"
|
||||
description={
|
||||
<span>
|
||||
ACME routing rules have not been pushed to HAProxy yet. Certificate validation will fail without this.{' '}
|
||||
<Button type="link" size="small" style={{ padding: 0 }} onClick={() => navigate('/apply-management')}>Go to Apply Management</Button>
|
||||
</span>
|
||||
}
|
||||
style={{ marginBottom: 16 }}
|
||||
/>
|
||||
)}
|
||||
{prerequisites?.steps?.find(s => s.key === 'config_applied' && s.ok === false) && (() => {
|
||||
const configStep = prerequisites.steps.find(s => s.key === 'config_applied');
|
||||
const pendingNames = (configStep?.pending_clusters || []).map(c => c.name).join(', ');
|
||||
return (
|
||||
<Alert
|
||||
type="warning"
|
||||
showIcon
|
||||
message="Configuration Not Applied"
|
||||
description={
|
||||
<span>
|
||||
ACME routing rules have not been pushed to HAProxy yet. Certificate validation will fail without this.
|
||||
{pendingNames && <> Pending on cluster: <strong>{pendingNames}</strong>.</>}{' '}
|
||||
<Button type="link" size="small" style={{ padding: 0 }} onClick={() => {
|
||||
if (configStep?.pending_clusters?.length) {
|
||||
const target = allClusters.find(c => c.id === configStep.pending_clusters[0].id);
|
||||
if (target) selectCluster(target);
|
||||
}
|
||||
navigate('/apply-management');
|
||||
}}>Go to Apply Management</Button>
|
||||
</span>
|
||||
}
|
||||
style={{ marginBottom: 16 }}
|
||||
/>
|
||||
);
|
||||
})()}
|
||||
<Alert
|
||||
type="info"
|
||||
showIcon
|
||||
@@ -447,8 +460,14 @@ const ACMEAutomation = () => {
|
||||
<span>
|
||||
{step.title}
|
||||
{step.navigate && (
|
||||
<Button type="link" size="small" onClick={() => navigate(step.navigate)} style={{ marginLeft: 8, padding: 0 }}>
|
||||
Configure
|
||||
<Button type="link" size="small" onClick={() => {
|
||||
if (step.pending_clusters?.length) {
|
||||
const target = allClusters.find(c => c.id === step.pending_clusters[0].id);
|
||||
if (target) selectCluster(target);
|
||||
}
|
||||
navigate(step.navigate);
|
||||
}} style={{ marginLeft: 8, padding: 0 }}>
|
||||
{step.ok === 'pending' || step.navigate === '/apply-management' ? 'Apply Changes' : 'Configure'}
|
||||
</Button>
|
||||
)}
|
||||
{step.action === 'register_account' && !step.ok && (
|
||||
@@ -459,7 +478,7 @@ const ACMEAutomation = () => {
|
||||
</span>
|
||||
),
|
||||
description: step.detail,
|
||||
status: step.ok === true ? 'finish' : step.ok === false ? 'error' : 'wait',
|
||||
status: step.ok === true ? 'finish' : step.ok === false ? 'error' : step.ok === 'pending' ? 'process' : 'wait',
|
||||
}))}
|
||||
/>
|
||||
</Card>
|
||||
|
||||
Reference in New Issue
Block a user