Conversation
0b7fcc7 to
fb61806
Compare
Rename settings_acme_servers translation keys to reflect ACME settings terminology, add challenge type and restart message keys, and add certificates/obsolete_tooltip keys for TLS certificates.
| "no_acme_server_description": "There is no ACME server configured", | ||
| "url": "URL", | ||
| "acme_settings": "ACME settings", | ||
| "search_acme_settings": "Search ACME settings", |
There was a problem hiding this comment.
| "search_acme_settings": "Search ACME settings", | |
| "search_acme_settings": "Search", |
| "challenge_tls_alpn_01": "TLS-ALPN-01", | ||
| "edit_acme_settings": "Edit ACME settings", | ||
| "acme_settings_for_node": "ACME settings for {node}", | ||
| "acme_restart_message": "HTTP clients on {node} will be briefly disconnected. Reload the page if necessary.", |
There was a problem hiding this comment.
| "acme_restart_message": "HTTP clients on {node} will be briefly disconnected. Reload the page if necessary.", | |
| "acme_restart_message": "HTTP clients on {node} will be briefly disconnected. The page should reload automatically — otherwise, reload it manually..", |
There was a problem hiding this comment.
Applied, minus the double period at the end.
One thing to check: the app never reloads by itself. When Traefik restarts on the leader, the websocket drops and App.vue shows a persistent warning toast with a "Reload" action, then retries the connection after 10 seconds. So the page recovers on its own, but it does not reload. Is "reload automatically" still the wording you want, or would "reconnect automatically" be closer?
There was a problem hiding this comment.
Ok, but IIRC the websocket warning toast disappears as soon as the connection is resumed, right?
Anyway, for the message I propose "HTTP clients on {node} will be briefly disconnected. The connection should resume automatically — otherwise, reload the page manually."
There was a problem hiding this comment.
Ok, but IIRC the websocket warning toast disappears as soon as the connection is resumed, right?
Yes and once reconnected we trigger all the list-/get- function
There was a problem hiding this comment.
Done, applied your wording.
| /> | ||
| </cv-data-table-cell> | ||
| <cv-data-table-cell> | ||
| <div class="justify-flex-end"> |
There was a problem hiding this comment.
We can use class="flex justify-end", these class already exist here https://github.com/NethServer/ns8-core/blob/main/core/ui/src/styles/_utils.scss
| <style scoped lang="scss"> | ||
| @import "../../styles/carbon-utils"; | ||
|
|
||
| .justify-flex-end { |
There was a problem hiding this comment.
We can remove this class
|
|
||
| // carbon renders the interactive tooltip trigger as a button with a default | ||
| // cursor, which hides the fact the tooltip opens on click | ||
| .clickable-tooltip ::v-deep .bx--tooltip__trigger, |
There was a problem hiding this comment.
Are these CSS rules needed? 🤔
There was a problem hiding this comment.
Half of them were, yes. I measured it in the browser: carbon already sets cursor: pointer via .bx--tooltip__trigger:not(.bx--btn--icon-only), so the rule on the trigger did nothing. I removed it.
But .bx--tag sets cursor: default, so the pointer was lost when hovering the "Obsolete" tag itself, which is the trigger content. I kept one narrow rule for that case:
// carbon's .bx--tag forces a default cursor, hiding that the tooltip opens on click
.clickable-tag ::v-deep .bx--tag {
cursor: pointer;
}Happy to drop it too if you would rather not special-case the tag.
There was a problem hiding this comment.
IIRC the tooltip is shown on click, not on hover, right? So we need a visual cue (the cursor pointer) to show the element as interactive. Let's keep only the .clickable-tag ::v-deep .bx--tag rule.
| :noDefaultToFirst="true" | ||
| @tab-selected="tabSelected" | ||
| > | ||
| <cv-tab |
There was a problem hiding this comment.
This file has become very big (~1500 loc), maybe we can try to externalize two components, e.g. TlsCertficatesPanel and AcmeSettingsPanel
There was a problem hiding this comment.
Done. SettingsTlsCertificates.vue is down to 289 lines: page chrome, tab state, and the list-installed-modules chain whose Traefik instance list both tabs consume. The certificates tab moved to TlsCertificatesPanel.vue, and AcmeSettings.vue is now AcmeSettingsPanel.vue for symmetry.
Two notes on the wiring:
- The node filter is bound to a query param, and
watchQueryDatais bound to the view instance inbeforeRouteEnter, so the view still ownsselectedNodeIdand passes it down with.sync. - The panel now runs
listCertificates()from a watcher ontraefikInstancesinstead of being called at the end oflistInstalledModulesCompleted, same triggerAcmeSettingsPanelalready used.
I also dropped the selectedNodeLabel computed while moving the code; it was defined and never referenced, already dead on main.
Tested on a live single-node cluster: both tabs, filters, query params, the upload modal and its error mapping, the tooltips, and the ACME edit modal.
Follow review feedback: the search box label repeated the table title, and the restart notice did not say the page comes back on its own.
styles/_utils.scss is imported globally, so the local justify-flex-end rule duplicated flex and justify-end. Removing it empties the style block.
The view had grown past 1500 lines. Move the certificates tab into TlsCertificatesPanel and rename AcmeSettings to AcmeSettingsPanel, leaving the view with the page chrome, the tab state and the shared Traefik instance list it feeds to both panels. The node filter stays bound to a query param, so the view owns it and passes it down with .sync. Drop the selectedNodeLabel computed, which was never referenced. Also narrow the tooltip cursor rule: carbon already sets a pointer on the tooltip trigger, only .bx--tag overrides it.
Drop comments the code already states and shorten the ones worth keeping.
Description
The TLS certificates page had a separate
ACME serverspage reachable through a top-right button, and the ACME challenge type — already accepted byset-acme-server— was exposed nowhere, soTLS-ALPN-01(needed when port 80 is unavailable) required editingtraefik.yamlby hand.This PR merges the two pages into two tabs,
CertificatesandACME settings, and exposes the challenge type as a tag per node plus a required radio group in the edit modal. Options come from a singleACME_CHALLENGE_TYPESarray, soDNS-01is one entry plus one translation key away; it is left out here because it needs extra fields. The ACME table columns becomeNode,ACME directory URL,Challenge type, which also fixes the previous wrong order, and/settings/acme-serversredirects to/settings/tls-certificates?view=acme.Settings remain per node. No backend change:
set-acme-serveralready validateschallengeandget-acme-serveralready returns it.Surviving the Traefik restart
set-acme-serverrestarts Traefik, which serves the admin UI and its websocket, so the completion event carrying the table refresh is lost: stale values and skeleton rows until a manual reload. The Certificates tab has the same problem throughcert_helpers.purge_acme_json_and_restart_traefik. Both tabs now watch the VuexisWebsocketConnectedflag and reload on reconnection; counters are clamped and ACME rows keyed by Traefik instance, so a late event cannot unbalance a counter or duplicate a row. TheReload requiredwarning is gone, and the restart warning is no longer gated on the leader node — the restart happens on whichever node is edited. Mechanism borrowed from #1258, which fixes the same thing on HTTP routes; a shared mixin is the follow-up once both land.Design
Mockups: Figma, design issue NethServer/dev#7973.
Refs: NethServer/dev#8099