backup nextcloud v2 #2289

fabianfrz · 2018-03-24T14:32:21Z

@AdSchellevis here is an updated version

fichtner

looks good, I'll work on integrating SCP like this as well now

fabianfrz · 2018-03-24T15:03:08Z

@fichtner SCP is slightly different because it may be harder to do a directory listing. One of the big advantages is, that it can be safe to put the modules into plugins since it is a single file.

AdSchellevis · 2018-03-24T15:04:59Z

@fabianfrz I'll try to review this weekend and pull it in, great work!

fabianfrz · 2018-03-24T15:08:51Z

@AdSchellevis @fichtner If there are many backup implementations, it would make sense to use tabs but it's ok for now.

fichtner · 2018-03-24T15:26:46Z

Still, no-dependency backup features can go into the core. And listing is easy as long as ssh is allowed as well. 😊 Tabs are useful until we have too many plugins. Could break them out into the menu eventually?

…

On 24. Mar 2018, at 16:08, Fabian Franz BSc ***@***.***> wrote: @AdSchellevis @fichtner If there are many backup implementations, it would make sense to use tabs but it's ok for now. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

fabianfrz · 2018-03-24T16:19:56Z

Like Interfaces? May slow down the web interface and is not often needed. Probably better where it is now.

fichtner · 2018-03-24T16:22:08Z

no, static and cached via its own Menu.xml

AdSchellevis · 2018-03-25T13:53:53Z

src/opnsense/mvc/app/library/OPNsense/Backup/Nextcloud.php

+            $config = $cnf->object();
+            foreach ($fields as &$field) {
+                $fieldname = $field['name'];
+                if (isset($config->system->remotebackup->$fieldname)) {


maybe better to place the settings in it's own container (for example system->backup->nextcloud->..), even better would be to use a model class to handle validations.

AdSchellevis · 2018-03-25T13:57:58Z

src/opnsense/mvc/app/library/OPNsense/Backup/Nextcloud.php

+            $input_errors[] = gettext('The Backup Directory can only consist of alphanumeric characters.');
+        }
+        // required fields
+        if (isset($config['system']['remotebackup']['nextcloud_enabled']) && $config['system']['remotebackup']['nextcloud_enabled'])


$config not known here, either push the validations to a model or fetch the config object like

core/src/opnsense/mvc/app/library/OPNsense/Backup/GDrive.php

Lines 143 to 146 in d5cddde

$config = Config::getInstance()->object();

if (!isset($config->system->remotebackup)) {

$config->system->remotebackup = array();

}

AdSchellevis · 2018-03-25T14:03:06Z

src/opnsense/mvc/app/library/OPNsense/Backup/Nextcloud.php

+            }
+            foreach ($this->getConfigurationFields() as $field) {
+                $fieldname = $field['name'];
+                if ($field['type'] == 'file') {


In case you're not using a model (which would need a refactoring of this section), you could ditch the file handling here, the settings don't seem to require file type inputs.

fabianfrz · 2018-03-26T17:00:23Z

@AdSchellevis hope it is ok now - please note that there is some stuff inside the code, which should not be in (imho):

this foreach loop should be an helper in base - needed because my fields are prefixed:
https://github.com/opnsense/core/pull/2289/files#diff-bd726565265dd13fe6163b66d98ff0d7R116
data reformat: Models need a different format - should be done by the form itself
https://github.com/opnsense/core/pull/2289/files#diff-bd726565265dd13fe6163b66d98ff0d7R124
need to convert validation result to array - otherwise the outer script crashes:
https://github.com/opnsense/core/pull/2289/files#diff-bd726565265dd13fe6163b66d98ff0d7R135

AdSchellevis · 2018-03-26T18:24:26Z

@fabianfrz Thanks! there is indeed room for improvement, let me pull this in and refactor some when I have some time available.

AdSchellevis · 2018-04-02T13:34:53Z

@fabianfrz can you try this d465ef2 602e42f ? I don't have a nextcloud account available to test.

fabianfrz · 2018-04-02T16:12:17Z

Does nothing anymore. Probably the changes in the "if" will stop it working.

AdSchellevis · 2018-04-02T16:13:46Z

no logging, nothing?

fabianfrz · 2018-04-02T16:14:45Z

nothing.

fabianfrz · 2018-04-02T16:15:35Z

I can make you an account on my nextcloud instance if you like.

AdSchellevis · 2018-04-02T16:18:10Z

sure, it's probably something small. just to be sure, you did save settings? I moved all the properties...

When I add random data, I do have entries in syslog like:

Apr  2 18:16:53 OPNsense config[81954]: Error while fetching filelist from Nextcloud
Apr  2 18:16:53 OPNsense config[81954]: {"url":"http:\/\/dsfasdfsdx\/remote.php\/dav\/files\/dsfasdfsa1\/","content_type":null,"http_code":0,"header_size":0,"request_size":0,"filetime":-1,"ssl_verify_result":0,"redirect_count":0,"total_time":0.534771,"namelookup_time":0,"connect_time":0,"pretransfer_time":0,"size_upload":0,"size_download":0,"speed_download":0,"speed_upload":0,"download_content_length":-1,"upload_content_length":-1,"starttransfer_time":0,"redirect_time":0,"redirect_url":"","primary_ip":"","certinfo":[],"primary_port":0,"local_ip":"","local_port":0}

fabianfrz · 2018-04-02T16:21:16Z

Found out the issue: the behaviour has changed. before your refactoring the test did run even when the "enabled" flag was false. Now the check prevents the test.

When it is enabled, it is working.

AdSchellevis · 2018-04-02T16:24:27Z

ok, nice, thanks for testing. I think it's better to skip testing when disabled.

fabianfrz · 2018-04-02T16:26:59Z

so no account needed ;)

AdSchellevis · 2018-04-02T16:27:55Z

nope, easier on my end :)

fichtner · 2018-04-02T17:33:22Z

I get these in the background, probably during a migration run:

Apr 2 17:27:50 | config[50537]: Model OPNsense\Backup\NextcloudSettings can't be saved, skip ( Phalcon\Validation\Exception: [OPNsense\Backup\NextcloudSettings:enabled] value should be a boolean (0,1) [OPNsense\Backup\NextcloudSettings:url] An URL for the Nextcloud server must be set. [OPNsense\Backup\NextcloudSettings:user] An user for the Nextcloud server must be set. [OPNsense\Backup\NextcloudSettings:password] A password for a Nextcloud server must be set. in /usr/local/opnsense/mvc/app/models/OPNsense/Base/BaseModel.php:512 Stack trace: #0 /usr/local/opnsense/mvc/app/models/OPNsense/Base/BaseModel.php(616): OPNsense\Base\BaseModel->serializeToConfig() #1 /usr/local/opnsense/mvc/script/run_migrations.php(56): OPNsense\Base\BaseModel->runMigrations() #2 {main} )
-- | --
Apr 2 17:27:50 | config[50537]: [OPNsense\Backup\NextcloudSettings:password] A password for a Nextcloud server must be set.
Apr 2 17:27:50 | config[50537]: [OPNsense\Backup\NextcloudSettings:user] An user for the Nextcloud server must be set.
Apr 2 17:27:50 | config[50537]: [OPNsense\Backup\NextcloudSettings:url] An URL for the Nextcloud server must be set.
Apr 2 17:27:50 | config[50537]: [OPNsense\Backup\NextcloudSettings:enabled] value should be a boolean (0,1)
``

fabianfrz · 2018-04-02T17:52:28Z

the first line says it fails in runMigrations - however there is probably no data to migrate

fabianfrz · 2018-04-02T17:57:53Z

@fichtner add the following defaults?
enabled: 0
url: https://nextcloud.example.com
user: opnsensebackup
password: opnsensebackup

fichtner · 2018-04-02T18:14:34Z

I don't know, maybe we shouldn't render unused models. it seems silly to enforce this.

In any case, the error is bad and everyone will report it...

fabianfrz · 2018-04-02T18:27:25Z

@fichtner maybw the migration script should check if the mount piont exist before running a migration. That would fix some other models in the plugins as well.

AdSchellevis · 2018-04-03T08:40:15Z

@fabianfrz @fichtner I have two possible solutions here, use constraints between fields, added DependConstraint type which could help here. Let me push a simple model fix

fichtner · 2018-04-03T08:46:23Z

yes, that looks good to me

The basic trouble is we want to render model defaults in new model versions so we can use them unconditionally, but then again sometimes that makes no sense at all because it's not used unconditionally :)

(cherry picked from commit 6f7c10a) (cherry picked from commit a994558)

fichtner · 2018-04-03T13:29:18Z

works perfectly ❤️

wols · 2018-06-23T13:27:49Z

Cool new feature, thanks!

No work on my OPNsense 18.1.10-i386:

config[34300]: {"url":"https:\/\/cloud.my.domain\/remote.php\/dav\/files\/CORRECT_USERNAME\/","content_type":null,"http_code":0,"header_size":0,"request_size":0,"filetime":-1,"ssl_verify_result":19,"redirect_count":0,"total_time":0.137768,"namelookup_time":0.004656,"connect_time":0.005135,"pretransfer_time":0,"size_upload":0,"size_download":0,"speed_download":0,"speed_upload":0,"download_content_length":-1,"upload_content_length":-1,"starttransfer_time":0,"redirect_time":0,"redirect_url":"","primary_ip":"192.168.1.77","certinfo":[],"primary_port":443,"local_ip":"192.168.1.1","local_port":21665}

I use CaCert on https://cloud.my.domain - is this and ssl_verify_result:19 my problem?
See #1460 also?

fabianfrz · 2018-06-23T14:47:28Z

yes. you need a trusted certificate.

wols · 2018-06-23T15:22:36Z

Ok. No way to add https://www.cacert.org/certs/root.crt and trust it?
All my other applications (apache, openldap...) use it...

fabianfrz · 2018-06-23T20:34:04Z

@wols The GUI currently does not support it. It relies on the Mozilla certificate bundle. You have to add it manually and keep in mind that an update of the certificate bundle may undo your changes.

wols · 2018-06-23T23:13:30Z

Thanks a lot - I added CaCert and it works:

cat root.crt >> /usr/local/share/certs/ca-root-nss.crt

backup nextcloud v2

106954b

fabianfrz mentioned this pull request Mar 24, 2018

Add Nextcloud backup #2251

Closed

fichtner assigned AdSchellevis Mar 24, 2018

fichtner approved these changes Mar 24, 2018

View reviewed changes

This was referenced Mar 24, 2018

Automatic backup via WebDAVS #2250

Closed

Updates to the scp-backup plugin. opnsense/plugins#476

Closed

AdSchellevis requested changes Mar 25, 2018

View reviewed changes

convert Nextcloud backup to use models

02d47d0

fabianfrz force-pushed the nextcloud-v2 branch from 0f1f221 to 02d47d0 Compare March 26, 2018 16:54

fabianfrz mentioned this pull request Mar 31, 2018

add nextcloud docs opnsense/docs#14

Merged

AdSchellevis approved these changes Apr 2, 2018

View reviewed changes

AdSchellevis merged commit d803e53 into opnsense:master Apr 2, 2018

fabianfrz deleted the nextcloud-v2 branch April 2, 2018 08:52

AdSchellevis added a commit that referenced this pull request Apr 3, 2018

mvc, add constraint type for dependent fields, loosly related to #2289

6f7c10a

AdSchellevis added a commit that referenced this pull request Apr 3, 2018

backup/nextcloud, fix constraints for #2289

d710513

fichtner pushed a commit that referenced this pull request Apr 3, 2018

mvc, add constraint type for dependent fields, loosly related to #2289

ea311e5

(cherry picked from commit 6f7c10a) (cherry picked from commit a994558)

	$config = Config::getInstance()->object();
	if (!isset($config->system->remotebackup)) {
	$config->system->remotebackup = array();
	}

backup nextcloud v2 #2289

backup nextcloud v2 #2289

Conversation

fabianfrz commented Mar 24, 2018

fichtner left a comment

Choose a reason for hiding this comment

fabianfrz commented Mar 24, 2018

AdSchellevis commented Mar 24, 2018

fabianfrz commented Mar 24, 2018

fichtner commented Mar 24, 2018 via email

fabianfrz commented Mar 24, 2018

fichtner commented Mar 24, 2018

AdSchellevis Mar 25, 2018

Choose a reason for hiding this comment

AdSchellevis Mar 25, 2018

Choose a reason for hiding this comment

AdSchellevis Mar 25, 2018

Choose a reason for hiding this comment

fabianfrz commented Mar 26, 2018

AdSchellevis commented Mar 26, 2018

AdSchellevis commented Apr 2, 2018

fabianfrz commented Apr 2, 2018

AdSchellevis commented Apr 2, 2018

fabianfrz commented Apr 2, 2018

fabianfrz commented Apr 2, 2018

AdSchellevis commented Apr 2, 2018

fabianfrz commented Apr 2, 2018

AdSchellevis commented Apr 2, 2018

fabianfrz commented Apr 2, 2018

AdSchellevis commented Apr 2, 2018

fichtner commented Apr 2, 2018

fabianfrz commented Apr 2, 2018

fabianfrz commented Apr 2, 2018

fichtner commented Apr 2, 2018

fabianfrz commented Apr 2, 2018

AdSchellevis commented Apr 3, 2018

fichtner commented Apr 3, 2018

fichtner commented Apr 3, 2018

wols commented Jun 23, 2018 • edited Loading

fabianfrz commented Jun 23, 2018

wols commented Jun 23, 2018 • edited Loading

fabianfrz commented Jun 23, 2018

wols commented Jun 23, 2018

wols commented Jun 23, 2018 •

edited

Loading

wols commented Jun 23, 2018 •

edited

Loading