[pve-devel] [PATCH pve-manager] api, ui: allow to remove subscription

[Thomas Lamprecht]

On 5/19/20 11:45 AM, Moayad Almalat wrote:
> Signed-off-by: Moayad Almalat <m.almalat at proxmox.com>
> ---
>  PVE/API2/Subscription.pm          | 21 +++++++++++++++++++++
>  PVE/CLI/pvesubscription.pm        |  1 +
>  www/manager6/node/Subscription.js |  9 +++++++++
>  3 files changed, 31 insertions(+)

applying this and then doing a simple `make deb` gets me:

...
make -C bin check
[...]
perl -I.. -I. -T -e "use PVE::Service::pveproxy; PVE::Service::pveproxy->verify_api();"
400 internal error - unable to verify method info
returns: property is missing and it is not optional
[...]
perl -I.. -I. -T -e "use PVE::CLI::pvesubscription; PVE::CLI::pvesubscription->verify_api();"
400 internal error - unable to verify method info
returns: property is missing and it is not optional

I.e., you forgot to define the return property and you never tested
this version of the patch at all.. Always test the exact changes you will
send out before actually sending them out. Even if you just change one little
thing you must re-test, errors happen so fast and especially for "last minute
little what-could-go-wrong changes"...

Indentation is completely wrong too, for both perl and javascript code..
See: https://pve.proxmox.com/wiki/Perl_Style_Guide#Indentation 

> 
> diff --git a/PVE/API2/Subscription.pm b/PVE/API2/Subscription.pm
> index 6657c00d..bf03dd28 100644
> --- a/PVE/API2/Subscription.pm
> +++ b/PVE/API2/Subscription.pm
> @@ -245,4 +245,25 @@ __PACKAGE__->register_method ({
>  	return undef;
>      }});
>  
> +__PACKAGE__->register_method ({
> +    name =>  'delete',
> +    path =>  '',
> +    method =>  'DELETE',
> +        permissions =>  {
> +             check =>  ['perm', '/nodes/{node}', [ 'Sys.Modify' ]],
> +        },
> +        description =>  "Delete subscription key.",
> +        proxyto =>  'node',
> +        protected =>  1,
> +        parameters =>  {
> +            additionalProperties =>  0,
> +            properties =>  {
> +                 node =>  get_standard_option('pve-node'),
> +                },
> +          },

missing here:
    returns => { type => 'null'},

> +          code =>  sub {
> +                  unlink("/etc/subscription");

check for errors from unlink, e.g., something like:
    unlink("/etc/subscription") or die "cannot delete subscription information: $!";


> +          return undef;
> +         }});
> +
>  1;
> diff --git a/PVE/CLI/pvesubscription.pm b/PVE/CLI/pvesubscription.pm
> index cd81c415..751dde58 100755
> --- a/PVE/CLI/pvesubscription.pm
> +++ b/PVE/CLI/pvesubscription.pm
> @@ -28,6 +28,7 @@ our $cmddef = {
>  		 }
>  	     }],
>      set => [ 'PVE::API2::Subscription', 'set', ['key'], { node => $nodename } ],
> +    delete => [ 'PVE::API2::Subscription', 'delete', undef, { node => $nodename } ],
>  };
>  
>  1;
> diff --git a/www/manager6/node/Subscription.js b/www/manager6/node/Subscription.js
> index e4a35874..a82d91b1 100644
> --- a/www/manager6/node/Subscription.js
> +++ b/www/manager6/node/Subscription.js
> @@ -163,6 +163,15 @@ Ext.define('PVE.node.Subscription', {
>  			win.on('destroy', reload);
>  		    }
>  		},
> +		{
> +                    text: gettext('Remove Subscription'),
> +                    xtype: 'proxmoxStdRemoveButton',
> +                    confirmMsg: gettext('Are you sure to remove the subscription key?'),
> +                    baseurl: baseurl,
> +                    dangerous: true,
> +                    selModel: false,
> +                    callback: reload,
> +		},
>  		{
>  		    text: gettext('Check'),
>  		    handler: function() {
>