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

acme.sh: add page #5823

Merged
merged 8 commits into from
Apr 27, 2021
Merged

acme.sh: add page #5823

merged 8 commits into from
Apr 27, 2021

Conversation

peterbabic
Copy link
Contributor

No description provided.

@tldr-bot
Copy link

The build for this PR failed with the following error(s):

pages/common/acme.sh.md:
Error: Parse error on line 4:
...sh-official/acme.sh>- Issue a certific
-----------------------^
Expecting 'END_INFORMATION_LINK_URL', got 'NEWLINE'
pages/common/acme.sh.md:4: TLDR011 Page never contains more than a single empty line

Please fix the error(s) and push again.

@bl-ue bl-ue added the new command Issues requesting creation of a new page or PRs adding a new page for a command. label Apr 23, 2021
@bl-ue
Copy link
Contributor

bl-ue commented Apr 23, 2021

@peterbabic what do you think about using example.com instead of domain.tld? While the latter is perfectly good, the former looks more like a url at first glance.

Comment on lines 34 to 36
- Install certificate file into location specified by its parameter (not displayed):

`acme.sh --install-cert --domain {{domain.tld}} --reloadcmd {{reload_commad}}`
Copy link
Contributor Author

@peterbabic peterbabic Apr 23, 2021

Choose a reason for hiding this comment

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

This would ideally contain a far longer code snippet, like:

acme.sh --install-cert -d {{domain.tld}}  \
  --key-file {{/etc/nginx/ssl/domain.tld.key}} \
  --fullchain-file {{/etc/nginx/ssl/domain.tld.cer}} \
  --reloadcmd {{"systemctl force-reload nginx"}}

But I am not sure how to proceed. All the parameters, except the --domain are optional. However, there are many parameters in a format --*-filehandling their respective certificate key files. Without a single *-file parameter, the whole command does literally nothing.

Additionally, --reloadcmd is also optional, but it is run without it only rarely (in the situations that the service restart is done completely manually, bypassing the whole point of automatic certificate renewal mechanism).

I am not entirely sure how to proceed, but this single code block is the only one I would've looked for when calling tldr acme.sh.

EDIT: the relevant docs are at https://github.com/acmesh-official/acme.sh#3-install-the-cert-to-apachenginx-etc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bl-ue @marchersimon @sbrl please, can anyone provide any kind of feedback?

Should this stay as it is, or is there any way to display long command? I did not find anything "long" related in the contribution guide.

Copy link
Contributor

Choose a reason for hiding this comment

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

This seems a little long, but it might not be... 🤔

`acme.sh --install-cert -d {{example.com}} --key-file {{/path/to/example.com.key}} --fullchain-file {{/path/to/example.com.cer}} --reloadcmd {{"systemctl force-reload nginx"}}`

Copy link
Collaborator

Choose a reason for hiding this comment

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

I also think we should try to stuff it into one line. Mainly because most clients don't support multiline code.

Copy link
Contributor Author

@peterbabic peterbabic Apr 25, 2021

Choose a reason for hiding this comment

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

Guys please, I am not sure what to do here. Would appreciate some guidance, if I am expected to commit some changes.

UPDATE:
As far as I can tell right now, we have at least three options:

  1. drop the example as it is too long (I am strongly opposed, but might be sufficient)
  2. include relatively short example with generic values (would serve more as a reminder)
  3. include full length example (most valuable, might break some clients without multiline code support)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. add the subcommand-like page, detailing the most used --install-cert options, as there are quite a few of them and mention this page in See also section

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just stuff it into one line? It's a little long, but I don't really see any strong reason why not to. As for multiline, we can't do that, because that goes against the client spec (or at least the client spec will have to be updated).

Copy link
Contributor Author

@peterbabic peterbabic Apr 25, 2021

Choose a reason for hiding this comment

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

Long option would serve the most basic information, I am also wiling to write the subcommand page for --install-cert option, as it has around 5-7 commonly used parameters.

Thinking about it further, the name of the file could probably be called acme.sh-install-cert.md similarly as for instance git-cat-file.md

Would this somehow break existing conventions?

Copy link
Member

@sbrl sbrl Apr 25, 2021

Choose a reason for hiding this comment

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

For some commands, it's unavoidable to have long commands - just do tldr whiptail and you'll see what I mean. It's not necessarily a bad thing. I say put it all on 1 line.

Edit: Those file paths have to be absolute too by the looks of things, so we can't save some character there either

@peterbabic peterbabic changed the title add page: acme.sh acme.sh: ad page Apr 23, 2021
@peterbabic peterbabic changed the title acme.sh: ad page acme.sh: add page Apr 23, 2021
@peterbabic
Copy link
Contributor Author

@peterbabic what do you think about using example.com instead of domain.tld? While the latter is perfectly good, the former looks more like a url at first glance.

$ grep -o -i -R "example.com" | wc -l 
216

On the contrary:

$ grep -o -i -R "domain.tld" | wc -l 
0

I agree with example.com


- Issue a certificate using an automatic DNS API mode:

`acme.sh --issue --dns {{dns_api}} --domain {{example.com}}`
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps a concrete example for dns_api would aid clarity here?

Copy link
Contributor Author

@peterbabic peterbabic Apr 23, 2021

Choose a reason for hiding this comment

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

@sbrl sure.

EDIT: but how please?

Instead of {{dns_api}} would be one of the many available options specified here https://github.com/acmesh-official/acme.sh/wiki/dnsapi

So, for instance for CloudFlare DNS API it is {{dns_cf}}

Also, in example used {{dns_api}} is still perfectly valid for custom scripts, if there is a dns_api.sh script present in the .acme.sh/ root folder, more info acmesh-official/acme.sh#1571 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe this would also deserved it's separate subcommand page?

Copy link
Contributor

Choose a reason for hiding this comment

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

But is it actually a subcommand? If so, then if should just be dns_api.sh.md (if we document it, which we probably will).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe it would be called more like acme.sh-dns.md, as the subcommand in question is named --dns {{ }}

Copy link
Member

@sbrl sbrl May 1, 2021

Choose a reason for hiding this comment

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

If we document it as a subcommand, we'd probably do something similar to pacman in #5268, for example.

Copy link
Contributor Author

@peterbabic peterbabic May 1, 2021

Choose a reason for hiding this comment

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

@sbrl it is already open in #5852

Co-authored-by: bl-ue <54780737+bl-ue@users.noreply.github.com>
Co-authored-by: bl-ue <54780737+bl-ue@users.noreply.github.com>
@peterbabic
Copy link
Contributor Author

So I have added specific examples according to valuable suggestions with the possibility to document subcommands and maybe even open "Lets Document: acme.sh" issue in the future.

Co-authored-by: marchersimon <50295997+marchersimon@users.noreply.github.com>
@Waples Waples merged commit f1e8d7e into tldr-pages:master Apr 27, 2021
@peterbabic peterbabic deleted the new branch April 27, 2021 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new command Issues requesting creation of a new page or PRs adding a new page for a command.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants