Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

luci-app-ddns: Update global cacert path doc to reflect it is a file not a dir #7517

Merged
merged 1 commit into from
Dec 31, 2024

Conversation

kontaxis
Copy link

@kontaxis kontaxis commented Dec 31, 2024

Description:

  1. The UI (Dynamic DNS > Global Settings) describes cacert as the path for certificates (plural)
  2. However, code in packages/net/ddns-scripts/files/usr/bin/ddns.sh expects the parameter to be a file path, i.e., a certificate bundle, not a directory
  3. Therefore (1) is misleading and if someone supplies a directory, e.g., /etc/ssl/certs, DDNS services list updates silently fail

I would expect the following to succeed given how this parameter is documented:

# uci get ddns.global.cacert
/etc/ssl/certs/
# ls /etc/ssl/certs/
ca-certificates.crt
# /usr/bin/ddns service update
Certification file not found (/etc/ssl/certs/)

When a file path is used instead, DDNS service update succeeds:

# uci get ddns.global.cacert
/etc/ssl/certs/ca-certificates.crt
# /usr/bin/ddns service update
Downloading 'https://raw.githubusercontent.com/openwrt/packages/master/net/ddns-scripts/files/usr/share/ddns/list'
[...]
Download completed (825 bytes)

This change updates the language in Dynamic DNS > Global Settings to indicate that cacert is a file and not a directory.

@kontaxis kontaxis force-pushed the ddns-cacert-file-path-doc branch from 4857fa1 to 6eda8c5 Compare December 31, 2024 04:14
o = s.taboption('global', form.Value, 'cacert', _('CA Certs path'));
o.description = _('CA certificates path that will be used to download services data. Set IGNORE to skip certificate validation.');
o = s.taboption('global', form.Value, 'cacert', _('CA bundle file path'));
o.description = _('File path of the CA bundle that will be used to download services data. Set IGNORE to skip certificate validation.');
Copy link
Contributor

@systemcrash systemcrash Dec 31, 2024

Choose a reason for hiding this comment

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

In this case, let's avoid ambiguity of path, and refer directly to the file itself. Please re-word to

		o = s.taboption('global', form.Value, 'cacert', _('CA Certs bundle file'));
		o.description = _('CA bundle file used to download services data. Set IGNORE to skip certificate validation.');

…not a dir

Signed-off-by: Georgios Kontaxis <geko1702+commits@99rst.org>
@kontaxis kontaxis force-pushed the ddns-cacert-file-path-doc branch from 6eda8c5 to 4cf3c03 Compare December 31, 2024 17:43
@systemcrash systemcrash merged commit 8916350 into openwrt:master Dec 31, 2024
1 check passed
Sign up for free to 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.

2 participants