fix(root-domain): Check certificate before trying to add on proxy

[tanmoysrt]

• In TLS Certificate Renewal use nginx -s reload instead of systemctl restart

[mergify]

@tanmoysrt, thanks for the contribution, but we do not accept pull requests on a master. Please close this PR and raise PR on an develop branch.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.06%. Comparing base (0e5375a) to head (7aacb79).
⚠️ Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
press/press/doctype/root_domain/root_domain.py	0.00%	13 Missing ⚠️
press/press/doctype/server/server.py	0.00%	3 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6420      +/-   ##
==========================================
- Coverage   56.07%   56.06%   -0.01%     
==========================================
  Files         940      940              
  Lines       78091    78104      +13     
  Branches      510      511       +1     
==========================================
  Hits        43791    43791              
- Misses      34276    34289      +13     
  Partials       24       24              

Flag	Coverage Δ
dashboard	89.71% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

This PR improves proxy/root-domain wildcard TLS handling by preventing proxy setup when a domain lacks a usable certificate, and adjusts the TLS renewal playbook to reload NGINX instead of always restarting it.

Changes:

• Add TLS certificate presence/validity checks before allowing a Root Domain to be added to Proxy Servers.
• Skip wildcard-domain entries that don’t have all required TLS material when building the proxy wildcard configuration.
• Update TLS Ansible role to attempt nginx -s reload and only restart NGINX if reload fails.

Reviewed changes

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

File	Description
press/press/doctype/server/server.py	Adds certificate material checks when assembling wildcard domain payload for proxy setup.
press/press/doctype/root_domain/root_domain.py	Blocks adding a root domain to proxies unless a TLS certificate exists and is populated; avoids duplicate domain child rows.
press/playbooks/roles/tls/tasks/main.yml	Switches from unconditional NGINX restart to reload-first with fallback restart.

Comments suppressed due to low confidence (1)

press/press/doctype/root_domain/root_domain.py:214

• Wildcard host setup uses full_chain (e.g. /home/frappe/agent/tls/fullchain.pem), but this validation only checks certificate, private_key, and intermediate_chain. Update the check to also require full_chain (or ensure full_chain is derived for custom certs) so proxies don't proceed with an incomplete cert payload.

		tls_ceritificate: TLSCertificate = frappe.get_doc("TLS Certificate", {"domain": self.name})
		if not (
			tls_ceritificate.certificate
			and tls_ceritificate.private_key
			and tls_ceritificate.intermediate_chain
		):

---

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

[frappe-pr-bot]

🎉 This PR is included in version 0.31.6 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

[greptile-apps]

Greptile Summary

This PR adds a certificate validity guard before adding a root domain to proxy servers, prevents duplicate domain entries on proxies, and switches TLS renewal from a full nginx restart to a graceful nginx -s reload with a fallback restart when nginx isn't running.

• root_domain.py: add_to_proxies now checks that a TLS certificate exists and has its key fields populated before iterating proxies; also skips proxy.save() when the domain is already present on that proxy.
• server.py: get_wildcard_domains now skips domains whose wildcard certificate is missing private_key, full_chain, or intermediate_chain, avoiding incomplete cert data being pushed to the agent.
• tls/tasks/main.yml: TLS renewal playbook now reloads nginx in-place and only falls back to a full service restart when nginx isn't already running.

Confidence Score: 3/5

The new guard in get_wildcard_domains does not handle the case where no wildcard cert exists at all, leaving a crash path open on every proxy that has a domain without a wildcard certificate.

The guard added in server.py only checks whether a fetched certificate has its fields populated, but skips the prior step of verifying that certificate_name itself is non-None. Any domain on a proxy that has no associated wildcard TLS cert will still cause frappe.get_doc("TLS Certificate", None) to throw, making setup_wildcard_hosts fail entirely for that proxy.

press/press/doctype/server/server.py — the get_wildcard_domains method needs a None-check on certificate_name before calling frappe.get_doc.

Important Files Changed

Filename	Overview
press/press/doctype/root_domain/root_domain.py	Adds TLS certificate existence and field validity check to add_to_proxies, and prevents duplicate domain entries; typo in variable name and a field inconsistency with what setup_wildcard_hosts actually validates.
press/press/doctype/server/server.py	Adds a certificate field completeness guard in get_wildcard_domains, but misses the None case where frappe.db.get_value finds no wildcard cert, which will cause frappe.get_doc to crash before the new guard is reached.
press/playbooks/roles/tls/tasks/main.yml	Replaces unconditional systemctl restart nginx with nginx -s reload, falling back to a full service restart only when nginx is not running; logic is correct.

Sequence Diagram

sequenceDiagram
    participant UI as Whitelist Caller
    participant RD as RootDomain.add_to_proxies
    participant DB as Frappe DB
    participant Proxy as ProxyServer
    participant Agent as Agent (setup_wildcard_hosts)

    UI->>RD: add_to_proxies()
    RD->>DB: "exists(TLS Certificate, {domain})"
    alt No certificate
        DB-->>RD: False
        RD-->>UI: frappe.throw(missing cert)
    else Certificate exists
        DB-->>RD: True
        RD->>DB: "get_doc(TLS Certificate, {domain})"
        DB-->>RD: tls_certificate
        alt Missing fields
            RD-->>UI: frappe.throw(invalid cert)
        else Valid certificate
            loop Each active proxy
                RD->>Proxy: get_doc(proxy_name)
                alt Domain already on proxy
                    RD-->>Proxy: skip append/save
                else Domain not on proxy
                    RD->>Proxy: append domain + save()
                end
                Proxy->>Agent: setup_wildcard_hosts()
                Agent->>DB: get_wildcard_domains()
                DB-->>Agent: wildcard certs (skips missing full_chain)
            end
        end
    end

Loading

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
press/press/doctype/server/server.py:2522-2528
**Potential crash when no wildcard certificate exists**

`frappe.db.get_value` returns `None` when no wildcard TLS certificate is found for a domain. Passing `None` directly to `frappe.get_doc("TLS Certificate", None)` will raise a `frappe.exceptions.MandatoryError` or similar before the new `if not (…)` guard is ever reached. Any proxy with a domain that lacks a wildcard cert will cause `get_wildcard_domains` to throw, breaking the entire `setup_wildcard_hosts` call for that proxy.

### Issue 2 of 3
press/press/doctype/root_domain/root_domain.py:210-214
**Inconsistent field check vs. what proxy setup actually requires**

The guard here validates `tls_certificate.certificate` (the domain-only cert PEM), but `get_wildcard_domains` in `server.py` (called via `proxy.setup_wildcard_hosts()` just below) checks `full_chain`, not `certificate`. A TLS Certificate doc that has `certificate` populated but an empty `full_chain` will pass this guard and successfully save the domain to the proxy, yet `setup_wildcard_hosts` will silently skip it, leaving the proxy with a domain entry but no working wildcard configuration.

### Issue 3 of 3
press/press/doctype/root_domain/root_domain.py:209-219
Typo in variable name: `tls_ceritificate` has a transposed `i` (`ceri` instead of `certi`). Python won't surface this at parse time since both the assignment and the attribute reads use the same misspelled name, but it makes the code harder to read and search.

```suggestion
		tls_certificate: TLSCertificate = frappe.get_doc("TLS Certificate", {"domain": self.name})
		if not (
			tls_certificate.certificate
			and tls_certificate.private_key
			and tls_certificate.intermediate_chain
		):
			frappe.throw(
				"Please obtain a valid TLS certificate for this domain before adding it to proxy servers."
			)

		proxies = frappe.get_all("Proxy Server", {"status": "Active"}, pluck="name")
```

_{Reviews (1): Last reviewed commit: "Merge branch 'master' into fix_tls_failu..." | Re-trigger Greptile}

[greptile-apps]

Potential crash when no wildcard certificate exists

frappe.db.get_value returns None when no wildcard TLS certificate is found for a domain. Passing None directly to frappe.get_doc("TLS Certificate", None) will raise a frappe.exceptions.MandatoryError or similar before the new if not (…) guard is ever reached. Any proxy with a domain that lacks a wildcard cert will cause get_wildcard_domains to throw, breaking the entire setup_wildcard_hosts call for that proxy.

Prompt To Fix With AI

This is a comment left during a code review.
Path: press/press/doctype/server/server.py
Line: 2522-2528

Comment:
**Potential crash when no wildcard certificate exists**

`frappe.db.get_value` returns `None` when no wildcard TLS certificate is found for a domain. Passing `None` directly to `frappe.get_doc("TLS Certificate", None)` will raise a `frappe.exceptions.MandatoryError` or similar before the new `if not (…)` guard is ever reached. Any proxy with a domain that lacks a wildcard cert will cause `get_wildcard_domains` to throw, breaking the entire `setup_wildcard_hosts` call for that proxy.

How can I resolve this? If you propose a fix, please make it concise.