On October 21, 2019 12:12 pm, Wolfgang Link wrote:
> comment inline
>
> On 10/18/19 11:22 AM, Fabian Grünbichler wrote:
>> note: the comment here is not just for this patch, but also references
>> stuff that comes in later patches..
>>
>> On October 14, 2019 1:08 pm, Wolfgang Link wrote:
>>> The dynamic approach reduces failure if new plugins will included.
>>> ---
>>> src/PVE/ACME.pm | 4 ++++
>>> src/PVE/ACME/Challenge.pm | 8 ++++++++
>>> src/PVE/ACME/StandAlone.pm | 4 ++++
>>> 3 files changed, 16 insertions(+)
>>>
>>> diff --git a/src/PVE/ACME.pm b/src/PVE/ACME.pm
>>> index 38a14a5..da4cbcc 100644
>>> --- a/src/PVE/ACME.pm
>>> +++ b/src/PVE/ACME.pm
>>> @@ -17,6 +17,9 @@ use LWP::UserAgent;
>>>
>>> use Crypt::OpenSSL::RSA;
>>>
>>> +use PVE::ACME::Challenge;
>>> +use PVE::ACME::StandAlone;
>>> +
>>> use PVE::Certificate;
>>> use PVE::Tools qw(
>>> file_set_contents
>>> @@ -24,6 +27,7 @@ file_get_contents
>>> );
>>>
>>> Crypt::OpenSSL::RSA->import_random_seed();
>>> +PVE::ACME::StandAlone->register();
>>>
>>> my $LETSENCRYPT_STAGING =
>>> 'https://acme-staging-v02.api.letsencrypt.org/directory';
>>>
>>> diff --git a/src/PVE/ACME/Challenge.pm b/src/PVE/ACME/Challenge.pm
>>> index 40d32b6..786666c 100644
>>> --- a/src/PVE/ACME/Challenge.pm
>>> +++ b/src/PVE/ACME/Challenge.pm
>>> @@ -3,6 +3,14 @@ package PVE::ACME::Challenge;
>>> use strict;
>>> use warnings;
>>>
>>> +use base qw(PVE::SectionConfig);
>> this would be the only SectionConfig that is not actually a config..
>>
>> after some off-list discussion with Thomas, how about the following:
>>
>> move PVE::ACME::Challenge and PVE::ACME::StandAlone to pve-manager, or a
>> new pve-acme package (the latter could be combined with PVE::ACME, and
>> some parts of what is currently in pve-manager, and your new
>> package/repo)
>>
>> make the challenge plugins a real SectionConfig:
>>
>> standalone: http80
>> port 80
>>
>> dns: ovh1
>> api ovh
>> data encode_text("KEY1=foobar\nKEY2=foobaz")
>> nodes node1
>>
>> dns: ovh2
>> api ovh
>> data encode_text("KEY1=barfoo\nKEY2=foobaz")
>> nodes node2
>>
>> for PVE, store that section config file in
>>
>> /etc/pve/priv/acme/plugins.cfg
>
> I agree.
>
> It the ovh1/ovh2 the name of the definition like in storage.cfg?
yes. the string before the colon is the section type/plugin, the string
after is the ID of that specific section/plugin instance, just like in
storage.cfg
> Nodes should be a node list or '*' to indicate to use wildcards certificate.
I'd skip wildcard certificates for now (unless you explicitly want to
include them in your v1?) and just use the same definition like for
storages: either a list of nodes, or all nodes if absent.
for wild-card certificates we'd need more changes anyway:
- not stored under /etc/pve/local
- which nodes should use it (-> flag in node config?)
- which node(s) should renew it
- the code to order/renew needs to support it
- ... ?
>> (moving the definition together with the rest of general ACME code to a
>> new package would allow PMG to re-use most of it with just a different
>> high-level client and file location)
>>
>> the GUI could have a mapping of the most important plugins and their
>> config KEYs, with a fallback to just "edit whole data as textfield" or
>> "edit whole data as key-value pairs". this would still allow adding your
>> own acme.sh dnsapi compatible plugins, and would avoid having to import
>> the full key-value schema into our section config.
> We could get the plugins dynamically on call.
> The keys are not extractable because some plugins do not notate them.
my (or actually mostly Thomas' ;)) suggestion would be the following:
for the most important plugins, ship their known keys + description +
validation in pve-manager (just for the GUI), so that we can display
proper fields for those keys in addition to a general free-form key ->
value entry with arbitrary values. this is just a usability improvement.
the API just takes a single multi-line string and does not care about
the format at all. for storing/retrieving the keys and their values, we
don't need to know anything about their count, names, schema, ... since
we just pass them along to the acme.sh plugin anyway..
>> the node config's acme key would then just reference the concrete plugin
>> instance (but see later patches for another issue regarding this), which
>> would be available cluster-wide, but also allow different plugin
>> instances for each node..
>>
>>> +
>>> +my $defaultData = {};
>>> +
>>> +sub private {
>>> + return $defaultData;
>>> +}
>>> +
>>> sub supported_challenge_types {
>>> return {};
>>> }
>>> diff --git a/src/PVE/ACME/StandAlone.pm b/src/PVE/ACME/StandAlone.pm
>>> index f48d638..3766862 100644
>>> --- a/src/PVE/ACME/StandAlone.pm
>>> +++ b/src/PVE/ACME/StandAlone.pm
>>> @@ -8,6 +8,10 @@ use HTTP::Response;
>>>
>>> use base qw(PVE::ACME::Challenge);
>>>
>>> +sub type {
>>> + return 'standalone';
>>> +}
>>> +
>>> sub supported_challenge_types {
>>> return { 'http-01' => 1 };
>>> }
>>> --
>>> 2.20.1
>>>
>>>
>>> _______________________________________________
>>> pve-devel mailing list
>>> pve-devel@pve.proxmox.com
>>> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>>
>>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel@pve.proxmox.com
>> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
