fix(server): derive etcd endpoint on subsequent boot from cp_base_url - #278

Merged
nic-6443 merged 4 commits into
mainfrom
fix/dp-stale-etcd-endpoint
May 14, 2026
Merged

fix(server): derive etcd endpoint on subsequent boot from cp_base_url#278
nic-6443 merged 4 commits into
mainfrom
fix/dp-stale-etcd-endpoint

Conversation

@jarvis9443

@jarvis9443jarvis9443 commented May 14, 2026

Copy link
Copy Markdown
Contributor

The bundle_on_disk branch (subsequent boot with persisted mTLS bundle) restored TLS config and env_id but never updated cfg.etcd.endpoints, leaving the placeholder https://placeholder-overridden-at-register:2379 from config.managed.yaml in place. The DP then failed to connect to the control plane etcd/kine endpoint until the volume was removed.

This extracts derive_cp_etcd_url() and uses it in both the cert-bundle provision path and the bundle_on_disk path, so subsequent boots always resolve the real etcd endpoint from AISIX_MANAGED__CP_BASE_URL (or AISIX_MANAGED__CP_ETCD_ENDPOINT when explicitly set).

Closes api7/AISIX-Cloud#289

Summary by CodeRabbit

  • Bug Fixes

    • Fixed stale etcd endpoint handling in managed-mode bootstrap and when reusing persisted mTLS bundles.
    • Improved control-plane URL derivation during certificate provisioning and heartbeat construction—now validates base URLs, strips URL schemes, and trims trailing slashes.
  • Tests

    • Added unit tests for URL derivation, explicit endpoint preference, scheme stripping, and error cases.

Review Change Stack

The bundle_on_disk branch (subsequent boot with persisted mTLS bundle)
restored TLS config and env_id but never updated cfg.etcd.endpoints,
leaving the placeholder 'https://placeholder-overridden-at-register:2379'
from config.managed.yaml in place. The DP then failed to connect to the
control plane's etcd/kine endpoint.
Extract derive_cp_etcd_url() and use it in both the cert-bundle
provision path and the bundle_on_disk path so subsequent boots always
resolve the real etcd endpoint from AISIX_MANAGED__CP_BASE_URL (or
AISIX_MANAGED__CP_ETCD_ENDPOINT when set).
Closes: api7/AISIX-Cloud#289
CopilotAI review requested due to automatic review settings May 14, 2026 13:40
@coderabbitai

coderabbitaiBot commented May 14, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ffbee559-2640-44d4-94f7-dc1b532b01ed

📥 Commits

Reviewing files that changed from the base of the PR and between aef2d91 and d685da4.

📒 Files selected for processing (1)
  • crates/aisix-server/src/main.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/aisix-server/src/main.rs

📝 Walkthrough

Walkthrough

Adds derive_cp_etcd_url to normalize/produce an https:// etcd endpoint from managed config, uses it in both managed-mode bootstrap paths to set cfg.etcd.endpoints and log the derived endpoint, validates heartbeat URL construction, and adds unit tests for derivation cases and errors.

Changes

Etcd Endpoint Derivation Helper and Bootstrap Integration

Layer / File(s)Summary
Helper function implementation
crates/aisix-server/src/main.rs
Adds derive_cp_etcd_url(managed: &ManagedConfig) -> anyhow::Result<String> helper that prefers cp_etcd_endpoint, validates non-empty cp_base_url, strips http:///https:// prefixes from cp_base_url, trims trailing /, and returns an https://<host:port> endpoint string.
Dashboard cert bundle bootstrap integration
crates/aisix-server/src/main.rs
In the dashboard-issued certificate provisioning path, calls derive_cp_etcd_url to compute the etcd endpoint, logs it with dp_id and env_id, assigns it to cfg.etcd.endpoints, and validates/trims cp_base_url before building the /dp/heartbeat URL.
Persisted mTLS bundle bootstrap integration
crates/aisix-server/src/main.rs
In the persisted mTLS reuse path on subsequent boots, calls derive_cp_etcd_url to replace stale placeholder endpoints, logs the derived endpoint, and overwrites cfg.etcd.endpoints.
Unit tests
crates/aisix-server/src/main.rs
Adds unit tests validating derive_cp_etcd_url for scheme stripping (http:// and https://), explicit cp_etcd_endpoint precedence, trailing slash trimming, http inputs, and expected failures when cp_base_url is missing or empty.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • #289: QA: DP restart with persisted mTLS bundle can reuse placeholder endpoint
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title directly describes the main fix: deriving the etcd endpoint on subsequent boot from cp_base_url, which is the core change shown in the raw summary and PR objectives.
Linked Issues check✅ PassedThe code changes address the core issue by implementing derive_cp_etcd_url() to resolve the real etcd endpoint from cp_base_url (or cp_etcd_endpoint) on both initial provision and subsequent boots, fixing the placeholder endpoint problem documented in #289.
Out of Scope Changes check✅ PassedAll changes focus on resolving etcd endpoint derivation for the managed-mode bootstrap path; no unrelated alterations to other functionality or modules are evident in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes managed-mode subsequent boot behavior by deriving and applying the real etcd endpoint when an mTLS bundle already exists, avoiding reuse of the placeholder endpoint from config.managed.yaml.

Changes:

  • Extracts shared derive_cp_etcd_url() logic.
  • Uses derived etcd URL in both cert-bundle provisioning and bundle-on-disk managed boot paths.
  • Adds unit tests for deriving etcd URLs from base URL and explicit endpoint inputs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadcrates/aisix-server/src/main.rs Outdated
Comment threadcrates/aisix-server/src/main.rs

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/aisix-server/src/main.rs`:
- Around line 756-775: The code currently forces cp_base (managed.cp_base_url)
to be present before checking cp_etcd, which breaks the intended precedence
where an explicit managed.cp_etcd_endpoint should be accepted; change the logic
so you first attempt to read managed.cp_etcd_endpoint (as_deref().filter(|s|
!s.is_empty())) and if that yields Some, use it for cp_etcd; only when cp_etcd
is missing should you then try to derive it from managed.cp_base_url
(strip_prefix http/https) and if cp_base is absent/error then return the anyhow
error. Update the cp_etcd and cp_base variable handling accordingly (references:
cp_etcd, cp_base, managed.cp_etcd_endpoint, managed.cp_base_url).
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 33d6b8ef-8a81-46df-9a5a-748cdb08e39c

📥 Commits

Reviewing files that changed from the base of the PR and between 068aa1f and ac060c7.

📒 Files selected for processing (1)
  • crates/aisix-server/src/main.rs

Comment threadcrates/aisix-server/src/main.rs Outdated
Allow derive_cp_etcd_url to work with only cp_etcd_endpoint set,
without requiring cp_base_url. Add test for this case.
CopilotAI review requested due to automatic review settings May 14, 2026 13:48

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment threadcrates/aisix-server/src/main.rs Outdated
Comment threadcrates/aisix-server/src/main.rs
…tcd URL
Address review feedback:
- Require cp_base_url for heartbeat in cert-bundle provision path
(unwrap_or_default would silently produce a broken URL)
- Strip trailing slash from cp_base_url when deriving etcd endpoint
(a URL like https://host:7944/ would produce an invalid port)
@nic-6443
nic-6443 merged commit 76aee06 into mainMay 14, 2026
7 checks passed
@nic-6443
nic-6443 deleted the fix/dp-stale-etcd-endpoint branch May 14, 2026 14:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jarvis9443@nic-6443
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(server): derive etcd endpoint on subsequent boot from cp_base_url - #278

Merged
nic-6443 merged 4 commits into
mainfrom
fix/dp-stale-etcd-endpoint
May 14, 2026
Merged

fix(server): derive etcd endpoint on subsequent boot from cp_base_url#278
nic-6443 merged 4 commits into
mainfrom
fix/dp-stale-etcd-endpoint

Conversation

@jarvis9443

@jarvis9443jarvis9443 commented May 14, 2026

Copy link
Copy Markdown
Contributor

The bundle_on_disk branch (subsequent boot with persisted mTLS bundle) restored TLS config and env_id but never updated cfg.etcd.endpoints, leaving the placeholder https://placeholder-overridden-at-register:2379 from config.managed.yaml in place. The DP then failed to connect to the control plane etcd/kine endpoint until the volume was removed.

This extracts derive_cp_etcd_url() and uses it in both the cert-bundle provision path and the bundle_on_disk path, so subsequent boots always resolve the real etcd endpoint from AISIX_MANAGED__CP_BASE_URL (or AISIX_MANAGED__CP_ETCD_ENDPOINT when explicitly set).

Closes api7/AISIX-Cloud#289

Summary by CodeRabbit

  • Bug Fixes

    • Fixed stale etcd endpoint handling in managed-mode bootstrap and when reusing persisted mTLS bundles.
    • Improved control-plane URL derivation during certificate provisioning and heartbeat construction—now validates base URLs, strips URL schemes, and trims trailing slashes.
  • Tests

    • Added unit tests for URL derivation, explicit endpoint preference, scheme stripping, and error cases.

Review Change Stack

The bundle_on_disk branch (subsequent boot with persisted mTLS bundle)
restored TLS config and env_id but never updated cfg.etcd.endpoints,
leaving the placeholder 'https://placeholder-overridden-at-register:2379'
from config.managed.yaml in place. The DP then failed to connect to the
control plane's etcd/kine endpoint.
Extract derive_cp_etcd_url() and use it in both the cert-bundle
provision path and the bundle_on_disk path so subsequent boots always
resolve the real etcd endpoint from AISIX_MANAGED__CP_BASE_URL (or
AISIX_MANAGED__CP_ETCD_ENDPOINT when set).
Closes: api7/AISIX-Cloud#289
CopilotAI review requested due to automatic review settings May 14, 2026 13:40
@coderabbitai

coderabbitaiBot commented May 14, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ffbee559-2640-44d4-94f7-dc1b532b01ed

📥 Commits

Reviewing files that changed from the base of the PR and between aef2d91 and d685da4.

📒 Files selected for processing (1)
  • crates/aisix-server/src/main.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/aisix-server/src/main.rs

📝 Walkthrough

Walkthrough

Adds derive_cp_etcd_url to normalize/produce an https:// etcd endpoint from managed config, uses it in both managed-mode bootstrap paths to set cfg.etcd.endpoints and log the derived endpoint, validates heartbeat URL construction, and adds unit tests for derivation cases and errors.

Changes

Etcd Endpoint Derivation Helper and Bootstrap Integration

Layer / File(s)Summary
Helper function implementation
crates/aisix-server/src/main.rs
Adds derive_cp_etcd_url(managed: &ManagedConfig) -> anyhow::Result<String> helper that prefers cp_etcd_endpoint, validates non-empty cp_base_url, strips http:///https:// prefixes from cp_base_url, trims trailing /, and returns an https://<host:port> endpoint string.
Dashboard cert bundle bootstrap integration
crates/aisix-server/src/main.rs
In the dashboard-issued certificate provisioning path, calls derive_cp_etcd_url to compute the etcd endpoint, logs it with dp_id and env_id, assigns it to cfg.etcd.endpoints, and validates/trims cp_base_url before building the /dp/heartbeat URL.
Persisted mTLS bundle bootstrap integration
crates/aisix-server/src/main.rs
In the persisted mTLS reuse path on subsequent boots, calls derive_cp_etcd_url to replace stale placeholder endpoints, logs the derived endpoint, and overwrites cfg.etcd.endpoints.
Unit tests
crates/aisix-server/src/main.rs
Adds unit tests validating derive_cp_etcd_url for scheme stripping (http:// and https://), explicit cp_etcd_endpoint precedence, trailing slash trimming, http inputs, and expected failures when cp_base_url is missing or empty.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • #289: QA: DP restart with persisted mTLS bundle can reuse placeholder endpoint
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title directly describes the main fix: deriving the etcd endpoint on subsequent boot from cp_base_url, which is the core change shown in the raw summary and PR objectives.
Linked Issues check✅ PassedThe code changes address the core issue by implementing derive_cp_etcd_url() to resolve the real etcd endpoint from cp_base_url (or cp_etcd_endpoint) on both initial provision and subsequent boots, fixing the placeholder endpoint problem documented in #289.
Out of Scope Changes check✅ PassedAll changes focus on resolving etcd endpoint derivation for the managed-mode bootstrap path; no unrelated alterations to other functionality or modules are evident in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes managed-mode subsequent boot behavior by deriving and applying the real etcd endpoint when an mTLS bundle already exists, avoiding reuse of the placeholder endpoint from config.managed.yaml.

Changes:

  • Extracts shared derive_cp_etcd_url() logic.
  • Uses derived etcd URL in both cert-bundle provisioning and bundle-on-disk managed boot paths.
  • Adds unit tests for deriving etcd URLs from base URL and explicit endpoint inputs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadcrates/aisix-server/src/main.rs Outdated
Comment threadcrates/aisix-server/src/main.rs

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/aisix-server/src/main.rs`:
- Around line 756-775: The code currently forces cp_base (managed.cp_base_url)
to be present before checking cp_etcd, which breaks the intended precedence
where an explicit managed.cp_etcd_endpoint should be accepted; change the logic
so you first attempt to read managed.cp_etcd_endpoint (as_deref().filter(|s|
!s.is_empty())) and if that yields Some, use it for cp_etcd; only when cp_etcd
is missing should you then try to derive it from managed.cp_base_url
(strip_prefix http/https) and if cp_base is absent/error then return the anyhow
error. Update the cp_etcd and cp_base variable handling accordingly (references:
cp_etcd, cp_base, managed.cp_etcd_endpoint, managed.cp_base_url).
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 33d6b8ef-8a81-46df-9a5a-748cdb08e39c

📥 Commits

Reviewing files that changed from the base of the PR and between 068aa1f and ac060c7.

📒 Files selected for processing (1)
  • crates/aisix-server/src/main.rs

Comment threadcrates/aisix-server/src/main.rs Outdated
Allow derive_cp_etcd_url to work with only cp_etcd_endpoint set,
without requiring cp_base_url. Add test for this case.
CopilotAI review requested due to automatic review settings May 14, 2026 13:48

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment threadcrates/aisix-server/src/main.rs Outdated
Comment threadcrates/aisix-server/src/main.rs
…tcd URL
Address review feedback:
- Require cp_base_url for heartbeat in cert-bundle provision path
(unwrap_or_default would silently produce a broken URL)
- Strip trailing slash from cp_base_url when deriving etcd endpoint
(a URL like https://host:7944/ would produce an invalid port)
@nic-6443
nic-6443 merged commit 76aee06 into mainMay 14, 2026
7 checks passed
@nic-6443
nic-6443 deleted the fix/dp-stale-etcd-endpoint branch May 14, 2026 14:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jarvis9443@nic-6443
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(server): derive etcd endpoint on subsequent boot from cp_base_url - #278

Merged
nic-6443 merged 4 commits into
mainfrom
fix/dp-stale-etcd-endpoint
May 14, 2026
Merged

fix(server): derive etcd endpoint on subsequent boot from cp_base_url#278
nic-6443 merged 4 commits into
mainfrom
fix/dp-stale-etcd-endpoint

Conversation

@jarvis9443

@jarvis9443jarvis9443 commented May 14, 2026

Copy link
Copy Markdown
Contributor

The bundle_on_disk branch (subsequent boot with persisted mTLS bundle) restored TLS config and env_id but never updated cfg.etcd.endpoints, leaving the placeholder https://placeholder-overridden-at-register:2379 from config.managed.yaml in place. The DP then failed to connect to the control plane etcd/kine endpoint until the volume was removed.

This extracts derive_cp_etcd_url() and uses it in both the cert-bundle provision path and the bundle_on_disk path, so subsequent boots always resolve the real etcd endpoint from AISIX_MANAGED__CP_BASE_URL (or AISIX_MANAGED__CP_ETCD_ENDPOINT when explicitly set).

Closes api7/AISIX-Cloud#289

Summary by CodeRabbit

  • Bug Fixes

    • Fixed stale etcd endpoint handling in managed-mode bootstrap and when reusing persisted mTLS bundles.
    • Improved control-plane URL derivation during certificate provisioning and heartbeat construction—now validates base URLs, strips URL schemes, and trims trailing slashes.
  • Tests

    • Added unit tests for URL derivation, explicit endpoint preference, scheme stripping, and error cases.

Review Change Stack

The bundle_on_disk branch (subsequent boot with persisted mTLS bundle)
restored TLS config and env_id but never updated cfg.etcd.endpoints,
leaving the placeholder 'https://placeholder-overridden-at-register:2379'
from config.managed.yaml in place. The DP then failed to connect to the
control plane's etcd/kine endpoint.
Extract derive_cp_etcd_url() and use it in both the cert-bundle
provision path and the bundle_on_disk path so subsequent boots always
resolve the real etcd endpoint from AISIX_MANAGED__CP_BASE_URL (or
AISIX_MANAGED__CP_ETCD_ENDPOINT when set).
Closes: api7/AISIX-Cloud#289
CopilotAI review requested due to automatic review settings May 14, 2026 13:40
@coderabbitai

coderabbitaiBot commented May 14, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ffbee559-2640-44d4-94f7-dc1b532b01ed

📥 Commits

Reviewing files that changed from the base of the PR and between aef2d91 and d685da4.

📒 Files selected for processing (1)
  • crates/aisix-server/src/main.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/aisix-server/src/main.rs

📝 Walkthrough

Walkthrough

Adds derive_cp_etcd_url to normalize/produce an https:// etcd endpoint from managed config, uses it in both managed-mode bootstrap paths to set cfg.etcd.endpoints and log the derived endpoint, validates heartbeat URL construction, and adds unit tests for derivation cases and errors.

Changes

Etcd Endpoint Derivation Helper and Bootstrap Integration

Layer / File(s)Summary
Helper function implementation
crates/aisix-server/src/main.rs
Adds derive_cp_etcd_url(managed: &ManagedConfig) -> anyhow::Result<String> helper that prefers cp_etcd_endpoint, validates non-empty cp_base_url, strips http:///https:// prefixes from cp_base_url, trims trailing /, and returns an https://<host:port> endpoint string.
Dashboard cert bundle bootstrap integration
crates/aisix-server/src/main.rs
In the dashboard-issued certificate provisioning path, calls derive_cp_etcd_url to compute the etcd endpoint, logs it with dp_id and env_id, assigns it to cfg.etcd.endpoints, and validates/trims cp_base_url before building the /dp/heartbeat URL.
Persisted mTLS bundle bootstrap integration
crates/aisix-server/src/main.rs
In the persisted mTLS reuse path on subsequent boots, calls derive_cp_etcd_url to replace stale placeholder endpoints, logs the derived endpoint, and overwrites cfg.etcd.endpoints.
Unit tests
crates/aisix-server/src/main.rs
Adds unit tests validating derive_cp_etcd_url for scheme stripping (http:// and https://), explicit cp_etcd_endpoint precedence, trailing slash trimming, http inputs, and expected failures when cp_base_url is missing or empty.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • #289: QA: DP restart with persisted mTLS bundle can reuse placeholder endpoint
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title directly describes the main fix: deriving the etcd endpoint on subsequent boot from cp_base_url, which is the core change shown in the raw summary and PR objectives.
Linked Issues check✅ PassedThe code changes address the core issue by implementing derive_cp_etcd_url() to resolve the real etcd endpoint from cp_base_url (or cp_etcd_endpoint) on both initial provision and subsequent boots, fixing the placeholder endpoint problem documented in #289.
Out of Scope Changes check✅ PassedAll changes focus on resolving etcd endpoint derivation for the managed-mode bootstrap path; no unrelated alterations to other functionality or modules are evident in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes managed-mode subsequent boot behavior by deriving and applying the real etcd endpoint when an mTLS bundle already exists, avoiding reuse of the placeholder endpoint from config.managed.yaml.

Changes:

  • Extracts shared derive_cp_etcd_url() logic.
  • Uses derived etcd URL in both cert-bundle provisioning and bundle-on-disk managed boot paths.
  • Adds unit tests for deriving etcd URLs from base URL and explicit endpoint inputs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadcrates/aisix-server/src/main.rs Outdated
Comment threadcrates/aisix-server/src/main.rs

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/aisix-server/src/main.rs`:
- Around line 756-775: The code currently forces cp_base (managed.cp_base_url)
to be present before checking cp_etcd, which breaks the intended precedence
where an explicit managed.cp_etcd_endpoint should be accepted; change the logic
so you first attempt to read managed.cp_etcd_endpoint (as_deref().filter(|s|
!s.is_empty())) and if that yields Some, use it for cp_etcd; only when cp_etcd
is missing should you then try to derive it from managed.cp_base_url
(strip_prefix http/https) and if cp_base is absent/error then return the anyhow
error. Update the cp_etcd and cp_base variable handling accordingly (references:
cp_etcd, cp_base, managed.cp_etcd_endpoint, managed.cp_base_url).
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 33d6b8ef-8a81-46df-9a5a-748cdb08e39c

📥 Commits

Reviewing files that changed from the base of the PR and between 068aa1f and ac060c7.

📒 Files selected for processing (1)
  • crates/aisix-server/src/main.rs

Comment threadcrates/aisix-server/src/main.rs Outdated
Allow derive_cp_etcd_url to work with only cp_etcd_endpoint set,
without requiring cp_base_url. Add test for this case.
CopilotAI review requested due to automatic review settings May 14, 2026 13:48

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment threadcrates/aisix-server/src/main.rs Outdated
Comment threadcrates/aisix-server/src/main.rs
…tcd URL
Address review feedback:
- Require cp_base_url for heartbeat in cert-bundle provision path
(unwrap_or_default would silently produce a broken URL)
- Strip trailing slash from cp_base_url when deriving etcd endpoint
(a URL like https://host:7944/ would produce an invalid port)
@nic-6443
nic-6443 merged commit 76aee06 into mainMay 14, 2026
7 checks passed
@nic-6443
nic-6443 deleted the fix/dp-stale-etcd-endpoint branch May 14, 2026 14:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jarvis9443@nic-6443
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(server): derive etcd endpoint on subsequent boot from cp_base_url - #278

Merged
nic-6443 merged 4 commits into
mainfrom
fix/dp-stale-etcd-endpoint
May 14, 2026
Merged

fix(server): derive etcd endpoint on subsequent boot from cp_base_url#278
nic-6443 merged 4 commits into
mainfrom
fix/dp-stale-etcd-endpoint

Conversation

@jarvis9443

@jarvis9443jarvis9443 commented May 14, 2026

Copy link
Copy Markdown
Contributor

The bundle_on_disk branch (subsequent boot with persisted mTLS bundle) restored TLS config and env_id but never updated cfg.etcd.endpoints, leaving the placeholder https://placeholder-overridden-at-register:2379 from config.managed.yaml in place. The DP then failed to connect to the control plane etcd/kine endpoint until the volume was removed.

This extracts derive_cp_etcd_url() and uses it in both the cert-bundle provision path and the bundle_on_disk path, so subsequent boots always resolve the real etcd endpoint from AISIX_MANAGED__CP_BASE_URL (or AISIX_MANAGED__CP_ETCD_ENDPOINT when explicitly set).

Closes api7/AISIX-Cloud#289

Summary by CodeRabbit

  • Bug Fixes

    • Fixed stale etcd endpoint handling in managed-mode bootstrap and when reusing persisted mTLS bundles.
    • Improved control-plane URL derivation during certificate provisioning and heartbeat construction—now validates base URLs, strips URL schemes, and trims trailing slashes.
  • Tests

    • Added unit tests for URL derivation, explicit endpoint preference, scheme stripping, and error cases.

Review Change Stack

The bundle_on_disk branch (subsequent boot with persisted mTLS bundle)
restored TLS config and env_id but never updated cfg.etcd.endpoints,
leaving the placeholder 'https://placeholder-overridden-at-register:2379'
from config.managed.yaml in place. The DP then failed to connect to the
control plane's etcd/kine endpoint.
Extract derive_cp_etcd_url() and use it in both the cert-bundle
provision path and the bundle_on_disk path so subsequent boots always
resolve the real etcd endpoint from AISIX_MANAGED__CP_BASE_URL (or
AISIX_MANAGED__CP_ETCD_ENDPOINT when set).
Closes: api7/AISIX-Cloud#289
CopilotAI review requested due to automatic review settings May 14, 2026 13:40
@coderabbitai

coderabbitaiBot commented May 14, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ffbee559-2640-44d4-94f7-dc1b532b01ed

📥 Commits

Reviewing files that changed from the base of the PR and between aef2d91 and d685da4.

📒 Files selected for processing (1)
  • crates/aisix-server/src/main.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/aisix-server/src/main.rs

📝 Walkthrough

Walkthrough

Adds derive_cp_etcd_url to normalize/produce an https:// etcd endpoint from managed config, uses it in both managed-mode bootstrap paths to set cfg.etcd.endpoints and log the derived endpoint, validates heartbeat URL construction, and adds unit tests for derivation cases and errors.

Changes

Etcd Endpoint Derivation Helper and Bootstrap Integration

Layer / File(s)Summary
Helper function implementation
crates/aisix-server/src/main.rs
Adds derive_cp_etcd_url(managed: &ManagedConfig) -> anyhow::Result<String> helper that prefers cp_etcd_endpoint, validates non-empty cp_base_url, strips http:///https:// prefixes from cp_base_url, trims trailing /, and returns an https://<host:port> endpoint string.
Dashboard cert bundle bootstrap integration
crates/aisix-server/src/main.rs
In the dashboard-issued certificate provisioning path, calls derive_cp_etcd_url to compute the etcd endpoint, logs it with dp_id and env_id, assigns it to cfg.etcd.endpoints, and validates/trims cp_base_url before building the /dp/heartbeat URL.
Persisted mTLS bundle bootstrap integration
crates/aisix-server/src/main.rs
In the persisted mTLS reuse path on subsequent boots, calls derive_cp_etcd_url to replace stale placeholder endpoints, logs the derived endpoint, and overwrites cfg.etcd.endpoints.
Unit tests
crates/aisix-server/src/main.rs
Adds unit tests validating derive_cp_etcd_url for scheme stripping (http:// and https://), explicit cp_etcd_endpoint precedence, trailing slash trimming, http inputs, and expected failures when cp_base_url is missing or empty.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • #289: QA: DP restart with persisted mTLS bundle can reuse placeholder endpoint
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title directly describes the main fix: deriving the etcd endpoint on subsequent boot from cp_base_url, which is the core change shown in the raw summary and PR objectives.
Linked Issues check✅ PassedThe code changes address the core issue by implementing derive_cp_etcd_url() to resolve the real etcd endpoint from cp_base_url (or cp_etcd_endpoint) on both initial provision and subsequent boots, fixing the placeholder endpoint problem documented in #289.
Out of Scope Changes check✅ PassedAll changes focus on resolving etcd endpoint derivation for the managed-mode bootstrap path; no unrelated alterations to other functionality or modules are evident in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes managed-mode subsequent boot behavior by deriving and applying the real etcd endpoint when an mTLS bundle already exists, avoiding reuse of the placeholder endpoint from config.managed.yaml.

Changes:

  • Extracts shared derive_cp_etcd_url() logic.
  • Uses derived etcd URL in both cert-bundle provisioning and bundle-on-disk managed boot paths.
  • Adds unit tests for deriving etcd URLs from base URL and explicit endpoint inputs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadcrates/aisix-server/src/main.rs Outdated
Comment threadcrates/aisix-server/src/main.rs

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/aisix-server/src/main.rs`:
- Around line 756-775: The code currently forces cp_base (managed.cp_base_url)
to be present before checking cp_etcd, which breaks the intended precedence
where an explicit managed.cp_etcd_endpoint should be accepted; change the logic
so you first attempt to read managed.cp_etcd_endpoint (as_deref().filter(|s|
!s.is_empty())) and if that yields Some, use it for cp_etcd; only when cp_etcd
is missing should you then try to derive it from managed.cp_base_url
(strip_prefix http/https) and if cp_base is absent/error then return the anyhow
error. Update the cp_etcd and cp_base variable handling accordingly (references:
cp_etcd, cp_base, managed.cp_etcd_endpoint, managed.cp_base_url).
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 33d6b8ef-8a81-46df-9a5a-748cdb08e39c

📥 Commits

Reviewing files that changed from the base of the PR and between 068aa1f and ac060c7.

📒 Files selected for processing (1)
  • crates/aisix-server/src/main.rs

Comment threadcrates/aisix-server/src/main.rs Outdated
Allow derive_cp_etcd_url to work with only cp_etcd_endpoint set,
without requiring cp_base_url. Add test for this case.
CopilotAI review requested due to automatic review settings May 14, 2026 13:48

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment threadcrates/aisix-server/src/main.rs Outdated
Comment threadcrates/aisix-server/src/main.rs
…tcd URL
Address review feedback:
- Require cp_base_url for heartbeat in cert-bundle provision path
(unwrap_or_default would silently produce a broken URL)
- Strip trailing slash from cp_base_url when deriving etcd endpoint
(a URL like https://host:7944/ would produce an invalid port)
@nic-6443
nic-6443 merged commit 76aee06 into mainMay 14, 2026
7 checks passed
@nic-6443
nic-6443 deleted the fix/dp-stale-etcd-endpoint branch May 14, 2026 14:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jarvis9443@nic-6443
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(server): derive etcd endpoint on subsequent boot from cp_base_url - #278

Merged
nic-6443 merged 4 commits into
mainfrom
fix/dp-stale-etcd-endpoint
May 14, 2026
Merged

fix(server): derive etcd endpoint on subsequent boot from cp_base_url#278
nic-6443 merged 4 commits into
mainfrom
fix/dp-stale-etcd-endpoint

Conversation

@jarvis9443

@jarvis9443jarvis9443 commented May 14, 2026

Copy link
Copy Markdown
Contributor

The bundle_on_disk branch (subsequent boot with persisted mTLS bundle) restored TLS config and env_id but never updated cfg.etcd.endpoints, leaving the placeholder https://placeholder-overridden-at-register:2379 from config.managed.yaml in place. The DP then failed to connect to the control plane etcd/kine endpoint until the volume was removed.

This extracts derive_cp_etcd_url() and uses it in both the cert-bundle provision path and the bundle_on_disk path, so subsequent boots always resolve the real etcd endpoint from AISIX_MANAGED__CP_BASE_URL (or AISIX_MANAGED__CP_ETCD_ENDPOINT when explicitly set).

Closes api7/AISIX-Cloud#289

Summary by CodeRabbit

  • Bug Fixes

    • Fixed stale etcd endpoint handling in managed-mode bootstrap and when reusing persisted mTLS bundles.
    • Improved control-plane URL derivation during certificate provisioning and heartbeat construction—now validates base URLs, strips URL schemes, and trims trailing slashes.
  • Tests

    • Added unit tests for URL derivation, explicit endpoint preference, scheme stripping, and error cases.

Review Change Stack

The bundle_on_disk branch (subsequent boot with persisted mTLS bundle)
restored TLS config and env_id but never updated cfg.etcd.endpoints,
leaving the placeholder 'https://placeholder-overridden-at-register:2379'
from config.managed.yaml in place. The DP then failed to connect to the
control plane's etcd/kine endpoint.
Extract derive_cp_etcd_url() and use it in both the cert-bundle
provision path and the bundle_on_disk path so subsequent boots always
resolve the real etcd endpoint from AISIX_MANAGED__CP_BASE_URL (or
AISIX_MANAGED__CP_ETCD_ENDPOINT when set).
Closes: api7/AISIX-Cloud#289
CopilotAI review requested due to automatic review settings May 14, 2026 13:40
@coderabbitai

coderabbitaiBot commented May 14, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ffbee559-2640-44d4-94f7-dc1b532b01ed

📥 Commits

Reviewing files that changed from the base of the PR and between aef2d91 and d685da4.

📒 Files selected for processing (1)
  • crates/aisix-server/src/main.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/aisix-server/src/main.rs

📝 Walkthrough

Walkthrough

Adds derive_cp_etcd_url to normalize/produce an https:// etcd endpoint from managed config, uses it in both managed-mode bootstrap paths to set cfg.etcd.endpoints and log the derived endpoint, validates heartbeat URL construction, and adds unit tests for derivation cases and errors.

Changes

Etcd Endpoint Derivation Helper and Bootstrap Integration

Layer / File(s)Summary
Helper function implementation
crates/aisix-server/src/main.rs
Adds derive_cp_etcd_url(managed: &ManagedConfig) -> anyhow::Result<String> helper that prefers cp_etcd_endpoint, validates non-empty cp_base_url, strips http:///https:// prefixes from cp_base_url, trims trailing /, and returns an https://<host:port> endpoint string.
Dashboard cert bundle bootstrap integration
crates/aisix-server/src/main.rs
In the dashboard-issued certificate provisioning path, calls derive_cp_etcd_url to compute the etcd endpoint, logs it with dp_id and env_id, assigns it to cfg.etcd.endpoints, and validates/trims cp_base_url before building the /dp/heartbeat URL.
Persisted mTLS bundle bootstrap integration
crates/aisix-server/src/main.rs
In the persisted mTLS reuse path on subsequent boots, calls derive_cp_etcd_url to replace stale placeholder endpoints, logs the derived endpoint, and overwrites cfg.etcd.endpoints.
Unit tests
crates/aisix-server/src/main.rs
Adds unit tests validating derive_cp_etcd_url for scheme stripping (http:// and https://), explicit cp_etcd_endpoint precedence, trailing slash trimming, http inputs, and expected failures when cp_base_url is missing or empty.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • #289: QA: DP restart with persisted mTLS bundle can reuse placeholder endpoint
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title directly describes the main fix: deriving the etcd endpoint on subsequent boot from cp_base_url, which is the core change shown in the raw summary and PR objectives.
Linked Issues check✅ PassedThe code changes address the core issue by implementing derive_cp_etcd_url() to resolve the real etcd endpoint from cp_base_url (or cp_etcd_endpoint) on both initial provision and subsequent boots, fixing the placeholder endpoint problem documented in #289.
Out of Scope Changes check✅ PassedAll changes focus on resolving etcd endpoint derivation for the managed-mode bootstrap path; no unrelated alterations to other functionality or modules are evident in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes managed-mode subsequent boot behavior by deriving and applying the real etcd endpoint when an mTLS bundle already exists, avoiding reuse of the placeholder endpoint from config.managed.yaml.

Changes:

  • Extracts shared derive_cp_etcd_url() logic.
  • Uses derived etcd URL in both cert-bundle provisioning and bundle-on-disk managed boot paths.
  • Adds unit tests for deriving etcd URLs from base URL and explicit endpoint inputs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadcrates/aisix-server/src/main.rs Outdated
Comment threadcrates/aisix-server/src/main.rs

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/aisix-server/src/main.rs`:
- Around line 756-775: The code currently forces cp_base (managed.cp_base_url)
to be present before checking cp_etcd, which breaks the intended precedence
where an explicit managed.cp_etcd_endpoint should be accepted; change the logic
so you first attempt to read managed.cp_etcd_endpoint (as_deref().filter(|s|
!s.is_empty())) and if that yields Some, use it for cp_etcd; only when cp_etcd
is missing should you then try to derive it from managed.cp_base_url
(strip_prefix http/https) and if cp_base is absent/error then return the anyhow
error. Update the cp_etcd and cp_base variable handling accordingly (references:
cp_etcd, cp_base, managed.cp_etcd_endpoint, managed.cp_base_url).
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 33d6b8ef-8a81-46df-9a5a-748cdb08e39c

📥 Commits

Reviewing files that changed from the base of the PR and between 068aa1f and ac060c7.

📒 Files selected for processing (1)
  • crates/aisix-server/src/main.rs

Comment threadcrates/aisix-server/src/main.rs Outdated
Allow derive_cp_etcd_url to work with only cp_etcd_endpoint set,
without requiring cp_base_url. Add test for this case.
CopilotAI review requested due to automatic review settings May 14, 2026 13:48

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment threadcrates/aisix-server/src/main.rs Outdated
Comment threadcrates/aisix-server/src/main.rs
…tcd URL
Address review feedback:
- Require cp_base_url for heartbeat in cert-bundle provision path
(unwrap_or_default would silently produce a broken URL)
- Strip trailing slash from cp_base_url when deriving etcd endpoint
(a URL like https://host:7944/ would produce an invalid port)
@nic-6443
nic-6443 merged commit 76aee06 into mainMay 14, 2026
7 checks passed
@nic-6443
nic-6443 deleted the fix/dp-stale-etcd-endpoint branch May 14, 2026 14:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jarvis9443@nic-6443
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(server): derive etcd endpoint on subsequent boot from cp_base_url - #278

Merged
nic-6443 merged 4 commits into
mainfrom
fix/dp-stale-etcd-endpoint
May 14, 2026
Merged

fix(server): derive etcd endpoint on subsequent boot from cp_base_url#278
nic-6443 merged 4 commits into
mainfrom
fix/dp-stale-etcd-endpoint

Conversation

@jarvis9443

@jarvis9443jarvis9443 commented May 14, 2026

Copy link
Copy Markdown
Contributor

The bundle_on_disk branch (subsequent boot with persisted mTLS bundle) restored TLS config and env_id but never updated cfg.etcd.endpoints, leaving the placeholder https://placeholder-overridden-at-register:2379 from config.managed.yaml in place. The DP then failed to connect to the control plane etcd/kine endpoint until the volume was removed.

This extracts derive_cp_etcd_url() and uses it in both the cert-bundle provision path and the bundle_on_disk path, so subsequent boots always resolve the real etcd endpoint from AISIX_MANAGED__CP_BASE_URL (or AISIX_MANAGED__CP_ETCD_ENDPOINT when explicitly set).

Closes api7/AISIX-Cloud#289

Summary by CodeRabbit

  • Bug Fixes

    • Fixed stale etcd endpoint handling in managed-mode bootstrap and when reusing persisted mTLS bundles.
    • Improved control-plane URL derivation during certificate provisioning and heartbeat construction—now validates base URLs, strips URL schemes, and trims trailing slashes.
  • Tests

    • Added unit tests for URL derivation, explicit endpoint preference, scheme stripping, and error cases.

Review Change Stack

The bundle_on_disk branch (subsequent boot with persisted mTLS bundle)
restored TLS config and env_id but never updated cfg.etcd.endpoints,
leaving the placeholder 'https://placeholder-overridden-at-register:2379'
from config.managed.yaml in place. The DP then failed to connect to the
control plane's etcd/kine endpoint.
Extract derive_cp_etcd_url() and use it in both the cert-bundle
provision path and the bundle_on_disk path so subsequent boots always
resolve the real etcd endpoint from AISIX_MANAGED__CP_BASE_URL (or
AISIX_MANAGED__CP_ETCD_ENDPOINT when set).
Closes: api7/AISIX-Cloud#289
CopilotAI review requested due to automatic review settings May 14, 2026 13:40
@coderabbitai

coderabbitaiBot commented May 14, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ffbee559-2640-44d4-94f7-dc1b532b01ed

📥 Commits

Reviewing files that changed from the base of the PR and between aef2d91 and d685da4.

📒 Files selected for processing (1)
  • crates/aisix-server/src/main.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/aisix-server/src/main.rs

📝 Walkthrough

Walkthrough

Adds derive_cp_etcd_url to normalize/produce an https:// etcd endpoint from managed config, uses it in both managed-mode bootstrap paths to set cfg.etcd.endpoints and log the derived endpoint, validates heartbeat URL construction, and adds unit tests for derivation cases and errors.

Changes

Etcd Endpoint Derivation Helper and Bootstrap Integration

Layer / File(s)Summary
Helper function implementation
crates/aisix-server/src/main.rs
Adds derive_cp_etcd_url(managed: &ManagedConfig) -> anyhow::Result<String> helper that prefers cp_etcd_endpoint, validates non-empty cp_base_url, strips http:///https:// prefixes from cp_base_url, trims trailing /, and returns an https://<host:port> endpoint string.
Dashboard cert bundle bootstrap integration
crates/aisix-server/src/main.rs
In the dashboard-issued certificate provisioning path, calls derive_cp_etcd_url to compute the etcd endpoint, logs it with dp_id and env_id, assigns it to cfg.etcd.endpoints, and validates/trims cp_base_url before building the /dp/heartbeat URL.
Persisted mTLS bundle bootstrap integration
crates/aisix-server/src/main.rs
In the persisted mTLS reuse path on subsequent boots, calls derive_cp_etcd_url to replace stale placeholder endpoints, logs the derived endpoint, and overwrites cfg.etcd.endpoints.
Unit tests
crates/aisix-server/src/main.rs
Adds unit tests validating derive_cp_etcd_url for scheme stripping (http:// and https://), explicit cp_etcd_endpoint precedence, trailing slash trimming, http inputs, and expected failures when cp_base_url is missing or empty.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • #289: QA: DP restart with persisted mTLS bundle can reuse placeholder endpoint
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title directly describes the main fix: deriving the etcd endpoint on subsequent boot from cp_base_url, which is the core change shown in the raw summary and PR objectives.
Linked Issues check✅ PassedThe code changes address the core issue by implementing derive_cp_etcd_url() to resolve the real etcd endpoint from cp_base_url (or cp_etcd_endpoint) on both initial provision and subsequent boots, fixing the placeholder endpoint problem documented in #289.
Out of Scope Changes check✅ PassedAll changes focus on resolving etcd endpoint derivation for the managed-mode bootstrap path; no unrelated alterations to other functionality or modules are evident in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes managed-mode subsequent boot behavior by deriving and applying the real etcd endpoint when an mTLS bundle already exists, avoiding reuse of the placeholder endpoint from config.managed.yaml.

Changes:

  • Extracts shared derive_cp_etcd_url() logic.
  • Uses derived etcd URL in both cert-bundle provisioning and bundle-on-disk managed boot paths.
  • Adds unit tests for deriving etcd URLs from base URL and explicit endpoint inputs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadcrates/aisix-server/src/main.rs Outdated
Comment threadcrates/aisix-server/src/main.rs

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/aisix-server/src/main.rs`:
- Around line 756-775: The code currently forces cp_base (managed.cp_base_url)
to be present before checking cp_etcd, which breaks the intended precedence
where an explicit managed.cp_etcd_endpoint should be accepted; change the logic
so you first attempt to read managed.cp_etcd_endpoint (as_deref().filter(|s|
!s.is_empty())) and if that yields Some, use it for cp_etcd; only when cp_etcd
is missing should you then try to derive it from managed.cp_base_url
(strip_prefix http/https) and if cp_base is absent/error then return the anyhow
error. Update the cp_etcd and cp_base variable handling accordingly (references:
cp_etcd, cp_base, managed.cp_etcd_endpoint, managed.cp_base_url).
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 33d6b8ef-8a81-46df-9a5a-748cdb08e39c

📥 Commits

Reviewing files that changed from the base of the PR and between 068aa1f and ac060c7.

📒 Files selected for processing (1)
  • crates/aisix-server/src/main.rs

Comment threadcrates/aisix-server/src/main.rs Outdated
Allow derive_cp_etcd_url to work with only cp_etcd_endpoint set,
without requiring cp_base_url. Add test for this case.
CopilotAI review requested due to automatic review settings May 14, 2026 13:48

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment threadcrates/aisix-server/src/main.rs Outdated
Comment threadcrates/aisix-server/src/main.rs
…tcd URL
Address review feedback:
- Require cp_base_url for heartbeat in cert-bundle provision path
(unwrap_or_default would silently produce a broken URL)
- Strip trailing slash from cp_base_url when deriving etcd endpoint
(a URL like https://host:7944/ would produce an invalid port)
@nic-6443
nic-6443 merged commit 76aee06 into mainMay 14, 2026
7 checks passed
@nic-6443
nic-6443 deleted the fix/dp-stale-etcd-endpoint branch May 14, 2026 14:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jarvis9443@nic-6443
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(server): derive etcd endpoint on subsequent boot from cp_base_url - #278

Merged
nic-6443 merged 4 commits into
mainfrom
fix/dp-stale-etcd-endpoint
May 14, 2026
Merged

fix(server): derive etcd endpoint on subsequent boot from cp_base_url#278
nic-6443 merged 4 commits into
mainfrom
fix/dp-stale-etcd-endpoint

Conversation

@jarvis9443

@jarvis9443jarvis9443 commented May 14, 2026

Copy link
Copy Markdown
Contributor

The bundle_on_disk branch (subsequent boot with persisted mTLS bundle) restored TLS config and env_id but never updated cfg.etcd.endpoints, leaving the placeholder https://placeholder-overridden-at-register:2379 from config.managed.yaml in place. The DP then failed to connect to the control plane etcd/kine endpoint until the volume was removed.

This extracts derive_cp_etcd_url() and uses it in both the cert-bundle provision path and the bundle_on_disk path, so subsequent boots always resolve the real etcd endpoint from AISIX_MANAGED__CP_BASE_URL (or AISIX_MANAGED__CP_ETCD_ENDPOINT when explicitly set).

Closes api7/AISIX-Cloud#289

Summary by CodeRabbit

  • Bug Fixes

    • Fixed stale etcd endpoint handling in managed-mode bootstrap and when reusing persisted mTLS bundles.
    • Improved control-plane URL derivation during certificate provisioning and heartbeat construction—now validates base URLs, strips URL schemes, and trims trailing slashes.
  • Tests

    • Added unit tests for URL derivation, explicit endpoint preference, scheme stripping, and error cases.

Review Change Stack

The bundle_on_disk branch (subsequent boot with persisted mTLS bundle)
restored TLS config and env_id but never updated cfg.etcd.endpoints,
leaving the placeholder 'https://placeholder-overridden-at-register:2379'
from config.managed.yaml in place. The DP then failed to connect to the
control plane's etcd/kine endpoint.
Extract derive_cp_etcd_url() and use it in both the cert-bundle
provision path and the bundle_on_disk path so subsequent boots always
resolve the real etcd endpoint from AISIX_MANAGED__CP_BASE_URL (or
AISIX_MANAGED__CP_ETCD_ENDPOINT when set).
Closes: api7/AISIX-Cloud#289
CopilotAI review requested due to automatic review settings May 14, 2026 13:40
@coderabbitai

coderabbitaiBot commented May 14, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ffbee559-2640-44d4-94f7-dc1b532b01ed

📥 Commits

Reviewing files that changed from the base of the PR and between aef2d91 and d685da4.

📒 Files selected for processing (1)
  • crates/aisix-server/src/main.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/aisix-server/src/main.rs

📝 Walkthrough

Walkthrough

Adds derive_cp_etcd_url to normalize/produce an https:// etcd endpoint from managed config, uses it in both managed-mode bootstrap paths to set cfg.etcd.endpoints and log the derived endpoint, validates heartbeat URL construction, and adds unit tests for derivation cases and errors.

Changes

Etcd Endpoint Derivation Helper and Bootstrap Integration

Layer / File(s)Summary
Helper function implementation
crates/aisix-server/src/main.rs
Adds derive_cp_etcd_url(managed: &ManagedConfig) -> anyhow::Result<String> helper that prefers cp_etcd_endpoint, validates non-empty cp_base_url, strips http:///https:// prefixes from cp_base_url, trims trailing /, and returns an https://<host:port> endpoint string.
Dashboard cert bundle bootstrap integration
crates/aisix-server/src/main.rs
In the dashboard-issued certificate provisioning path, calls derive_cp_etcd_url to compute the etcd endpoint, logs it with dp_id and env_id, assigns it to cfg.etcd.endpoints, and validates/trims cp_base_url before building the /dp/heartbeat URL.
Persisted mTLS bundle bootstrap integration
crates/aisix-server/src/main.rs
In the persisted mTLS reuse path on subsequent boots, calls derive_cp_etcd_url to replace stale placeholder endpoints, logs the derived endpoint, and overwrites cfg.etcd.endpoints.
Unit tests
crates/aisix-server/src/main.rs
Adds unit tests validating derive_cp_etcd_url for scheme stripping (http:// and https://), explicit cp_etcd_endpoint precedence, trailing slash trimming, http inputs, and expected failures when cp_base_url is missing or empty.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • #289: QA: DP restart with persisted mTLS bundle can reuse placeholder endpoint
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title directly describes the main fix: deriving the etcd endpoint on subsequent boot from cp_base_url, which is the core change shown in the raw summary and PR objectives.
Linked Issues check✅ PassedThe code changes address the core issue by implementing derive_cp_etcd_url() to resolve the real etcd endpoint from cp_base_url (or cp_etcd_endpoint) on both initial provision and subsequent boots, fixing the placeholder endpoint problem documented in #289.
Out of Scope Changes check✅ PassedAll changes focus on resolving etcd endpoint derivation for the managed-mode bootstrap path; no unrelated alterations to other functionality or modules are evident in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes managed-mode subsequent boot behavior by deriving and applying the real etcd endpoint when an mTLS bundle already exists, avoiding reuse of the placeholder endpoint from config.managed.yaml.

Changes:

  • Extracts shared derive_cp_etcd_url() logic.
  • Uses derived etcd URL in both cert-bundle provisioning and bundle-on-disk managed boot paths.
  • Adds unit tests for deriving etcd URLs from base URL and explicit endpoint inputs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadcrates/aisix-server/src/main.rs Outdated
Comment threadcrates/aisix-server/src/main.rs

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/aisix-server/src/main.rs`:
- Around line 756-775: The code currently forces cp_base (managed.cp_base_url)
to be present before checking cp_etcd, which breaks the intended precedence
where an explicit managed.cp_etcd_endpoint should be accepted; change the logic
so you first attempt to read managed.cp_etcd_endpoint (as_deref().filter(|s|
!s.is_empty())) and if that yields Some, use it for cp_etcd; only when cp_etcd
is missing should you then try to derive it from managed.cp_base_url
(strip_prefix http/https) and if cp_base is absent/error then return the anyhow
error. Update the cp_etcd and cp_base variable handling accordingly (references:
cp_etcd, cp_base, managed.cp_etcd_endpoint, managed.cp_base_url).
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 33d6b8ef-8a81-46df-9a5a-748cdb08e39c

📥 Commits

Reviewing files that changed from the base of the PR and between 068aa1f and ac060c7.

📒 Files selected for processing (1)
  • crates/aisix-server/src/main.rs

Comment threadcrates/aisix-server/src/main.rs Outdated
Allow derive_cp_etcd_url to work with only cp_etcd_endpoint set,
without requiring cp_base_url. Add test for this case.
CopilotAI review requested due to automatic review settings May 14, 2026 13:48

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment threadcrates/aisix-server/src/main.rs Outdated
Comment threadcrates/aisix-server/src/main.rs
…tcd URL
Address review feedback:
- Require cp_base_url for heartbeat in cert-bundle provision path
(unwrap_or_default would silently produce a broken URL)
- Strip trailing slash from cp_base_url when deriving etcd endpoint
(a URL like https://host:7944/ would produce an invalid port)
@nic-6443
nic-6443 merged commit 76aee06 into mainMay 14, 2026
7 checks passed
@nic-6443
nic-6443 deleted the fix/dp-stale-etcd-endpoint branch May 14, 2026 14:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jarvis9443@nic-6443
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(server): derive etcd endpoint on subsequent boot from cp_base_url - #278

Merged
nic-6443 merged 4 commits into
mainfrom
fix/dp-stale-etcd-endpoint
May 14, 2026
Merged

fix(server): derive etcd endpoint on subsequent boot from cp_base_url#278
nic-6443 merged 4 commits into
mainfrom
fix/dp-stale-etcd-endpoint

Conversation

@jarvis9443

@jarvis9443jarvis9443 commented May 14, 2026

Copy link
Copy Markdown
Contributor

The bundle_on_disk branch (subsequent boot with persisted mTLS bundle) restored TLS config and env_id but never updated cfg.etcd.endpoints, leaving the placeholder https://placeholder-overridden-at-register:2379 from config.managed.yaml in place. The DP then failed to connect to the control plane etcd/kine endpoint until the volume was removed.

This extracts derive_cp_etcd_url() and uses it in both the cert-bundle provision path and the bundle_on_disk path, so subsequent boots always resolve the real etcd endpoint from AISIX_MANAGED__CP_BASE_URL (or AISIX_MANAGED__CP_ETCD_ENDPOINT when explicitly set).

Closes api7/AISIX-Cloud#289

Summary by CodeRabbit

  • Bug Fixes

    • Fixed stale etcd endpoint handling in managed-mode bootstrap and when reusing persisted mTLS bundles.
    • Improved control-plane URL derivation during certificate provisioning and heartbeat construction—now validates base URLs, strips URL schemes, and trims trailing slashes.
  • Tests

    • Added unit tests for URL derivation, explicit endpoint preference, scheme stripping, and error cases.

Review Change Stack

The bundle_on_disk branch (subsequent boot with persisted mTLS bundle)
restored TLS config and env_id but never updated cfg.etcd.endpoints,
leaving the placeholder 'https://placeholder-overridden-at-register:2379'
from config.managed.yaml in place. The DP then failed to connect to the
control plane's etcd/kine endpoint.
Extract derive_cp_etcd_url() and use it in both the cert-bundle
provision path and the bundle_on_disk path so subsequent boots always
resolve the real etcd endpoint from AISIX_MANAGED__CP_BASE_URL (or
AISIX_MANAGED__CP_ETCD_ENDPOINT when set).
Closes: api7/AISIX-Cloud#289
CopilotAI review requested due to automatic review settings May 14, 2026 13:40
@coderabbitai

coderabbitaiBot commented May 14, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ffbee559-2640-44d4-94f7-dc1b532b01ed

📥 Commits

Reviewing files that changed from the base of the PR and between aef2d91 and d685da4.

📒 Files selected for processing (1)
  • crates/aisix-server/src/main.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/aisix-server/src/main.rs

📝 Walkthrough

Walkthrough

Adds derive_cp_etcd_url to normalize/produce an https:// etcd endpoint from managed config, uses it in both managed-mode bootstrap paths to set cfg.etcd.endpoints and log the derived endpoint, validates heartbeat URL construction, and adds unit tests for derivation cases and errors.

Changes

Etcd Endpoint Derivation Helper and Bootstrap Integration

Layer / File(s)Summary
Helper function implementation
crates/aisix-server/src/main.rs
Adds derive_cp_etcd_url(managed: &ManagedConfig) -> anyhow::Result<String> helper that prefers cp_etcd_endpoint, validates non-empty cp_base_url, strips http:///https:// prefixes from cp_base_url, trims trailing /, and returns an https://<host:port> endpoint string.
Dashboard cert bundle bootstrap integration
crates/aisix-server/src/main.rs
In the dashboard-issued certificate provisioning path, calls derive_cp_etcd_url to compute the etcd endpoint, logs it with dp_id and env_id, assigns it to cfg.etcd.endpoints, and validates/trims cp_base_url before building the /dp/heartbeat URL.
Persisted mTLS bundle bootstrap integration
crates/aisix-server/src/main.rs
In the persisted mTLS reuse path on subsequent boots, calls derive_cp_etcd_url to replace stale placeholder endpoints, logs the derived endpoint, and overwrites cfg.etcd.endpoints.
Unit tests
crates/aisix-server/src/main.rs
Adds unit tests validating derive_cp_etcd_url for scheme stripping (http:// and https://), explicit cp_etcd_endpoint precedence, trailing slash trimming, http inputs, and expected failures when cp_base_url is missing or empty.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • #289: QA: DP restart with persisted mTLS bundle can reuse placeholder endpoint
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title directly describes the main fix: deriving the etcd endpoint on subsequent boot from cp_base_url, which is the core change shown in the raw summary and PR objectives.
Linked Issues check✅ PassedThe code changes address the core issue by implementing derive_cp_etcd_url() to resolve the real etcd endpoint from cp_base_url (or cp_etcd_endpoint) on both initial provision and subsequent boots, fixing the placeholder endpoint problem documented in #289.
Out of Scope Changes check✅ PassedAll changes focus on resolving etcd endpoint derivation for the managed-mode bootstrap path; no unrelated alterations to other functionality or modules are evident in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes managed-mode subsequent boot behavior by deriving and applying the real etcd endpoint when an mTLS bundle already exists, avoiding reuse of the placeholder endpoint from config.managed.yaml.

Changes:

  • Extracts shared derive_cp_etcd_url() logic.
  • Uses derived etcd URL in both cert-bundle provisioning and bundle-on-disk managed boot paths.
  • Adds unit tests for deriving etcd URLs from base URL and explicit endpoint inputs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadcrates/aisix-server/src/main.rs Outdated
Comment threadcrates/aisix-server/src/main.rs

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/aisix-server/src/main.rs`:
- Around line 756-775: The code currently forces cp_base (managed.cp_base_url)
to be present before checking cp_etcd, which breaks the intended precedence
where an explicit managed.cp_etcd_endpoint should be accepted; change the logic
so you first attempt to read managed.cp_etcd_endpoint (as_deref().filter(|s|
!s.is_empty())) and if that yields Some, use it for cp_etcd; only when cp_etcd
is missing should you then try to derive it from managed.cp_base_url
(strip_prefix http/https) and if cp_base is absent/error then return the anyhow
error. Update the cp_etcd and cp_base variable handling accordingly (references:
cp_etcd, cp_base, managed.cp_etcd_endpoint, managed.cp_base_url).
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 33d6b8ef-8a81-46df-9a5a-748cdb08e39c

📥 Commits

Reviewing files that changed from the base of the PR and between 068aa1f and ac060c7.

📒 Files selected for processing (1)
  • crates/aisix-server/src/main.rs

Comment threadcrates/aisix-server/src/main.rs Outdated
Allow derive_cp_etcd_url to work with only cp_etcd_endpoint set,
without requiring cp_base_url. Add test for this case.
CopilotAI review requested due to automatic review settings May 14, 2026 13:48

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment threadcrates/aisix-server/src/main.rs Outdated
Comment threadcrates/aisix-server/src/main.rs
…tcd URL
Address review feedback:
- Require cp_base_url for heartbeat in cert-bundle provision path
(unwrap_or_default would silently produce a broken URL)
- Strip trailing slash from cp_base_url when deriving etcd endpoint
(a URL like https://host:7944/ would produce an invalid port)
@nic-6443
nic-6443 merged commit 76aee06 into mainMay 14, 2026
7 checks passed
@nic-6443
nic-6443 deleted the fix/dp-stale-etcd-endpoint branch May 14, 2026 14:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jarvis9443@nic-6443