ownCloud: Update to version 10.12

[mreid-tt]

Description

This PR contains the following:

1. Update ownCloud to version 10.12 (not upgradeable from v8.1.1-7)
2. Use SQLite database for faster performance
3. Use shared folder for data directory
4. Framework fix for icon generation

Fixes #
Closes #2710
Closes #3580

Checklist

• Build rule all-supported completed successfully
• New installation of package completed successfully
• Package upgrade completed successfully (Manually install the package again)
• Package functionality was tested
• Any needed documentation is updated/created

Type of change

• Package update
• Includes small framework changes

[hgy59]

@mreid-tt please migrate installer.sh to service-setup.sh. "installer.sh" is the (very old) version before introduction of the generic service installer and is not supported with DSM 7+ anymore.

[mreid-tt]

@hgy59, thanks for the tips while I work on this. So far I've gotten it to install cleanly on DSM7 as well as uninstall (with DB backup). I still have to work on the upgrade logic and update the UI to use modern elements as well as include additional prompts.

One strange thing is that even though I have an icon file defined, the package is not generating any of the icons in the app/images folder which doesn't seem to get created. Any thoughts on why this may be?

I also noted from the DSM 7.0 Developer Guide that the Application Config file (at app/config) does not recognise properties like protocol, port or grantPrivilege so I've removed these from the file. Instead I've made an addition to the postinstall script to amend the Apache configuration file for the same effect.

Thinking to my next steps, I don't know how I'll approach upgrades. The previous versions were based on a MySQL backend but with my performance testing I found the initial setup taking ~20x longer to complete when using MariaDB versus a standalone SQLite database. As such. I've moved to that back end. With the differing backends I don't know that a direct upgrade would be possible. Interested to hear your thoughts.

[mreid-tt]

@hgy59, so I took another look at he icons that were missing from app/images and found that in the framework file mk/spksrc.icon.mk, it did an explicit check for the 'SERVICE_PORT' variable. If this was absent it would not build the icons. This package however is running on the default port 80 so this variable would not be present. I've included a proposal to check for the 'ADMIN_URL' variable as an alternate and if present it would pass. This seems to work well.

As for the upgrade strategy, this version cannot be upgraded from the existing one in the repository (v8.1.1) and would need a minimum of v8.2.11 as starting point. As such, I've included a check for this to block such upgrades. I did however write a fairly elaborate upgrade sequence which includes data and configuration preservation as well as database backup archiving. Tests so far on DSM7 work well for a variety of scenarios including different data paths.

The next thing I think I need to update is the UI wizard files to include the relevant details of what is going on. I'm going to see if I can incorporate some scripting to check for some of the validations included in the service-setup.sh. For example ensuring that for a database backup on uninstall that the destination directory is writable. Don't know if that is possible but I'll see what I can find. Once that is done I believe the package should be good to publish.

[mreid-tt]

@hgy59, So I've come to a blocker with the DSM 6 testing. A number of the PHP scripts are very picky about the owner of the files matching the owner of the running process. Now this isn't a problem on DSM 7 with the resource worker populating the web_packages folder but on DSM 6 (where this needs to be done manually) this is an issue since a lot of the times it runs as root.

Ideally if there were a way to get both the installer and the running process to run as sc-owncloud but in a way that still allows the functions to manipulate the files in the web folder (which will likely still need root privileges), that would resolve all my issues. Any thoughts on an approach for this?

[ymartin59]

I just ask the question: is this worth maintaining DSM 6 support? We may decide to only deliver package for DSM 7.

[hgy59]

@hgy59, So I've come to a blocker with the DSM 6 testing. A number of the PHP scripts are very picky about the owner of the files matching the owner of the running process. Now this isn't a problem on DSM 7 with the resource worker populating the web_packages folder but on DSM 6 (where this needs to be done manually) this is an issue since a lot of the times it runs as root.

Ideally if there were a way to get both the installer and the running process to run as sc-owncloud but in a way that still allows the functions to manipulate the files in the web folder (which will likely still need root privileges), that would resolve all my issues. Any thoughts on an approach for this?

You always can provide a privilege file for DSM 6 with the same content as the one generated for DSM 7 (i.e. "run-as" : "package")

I highly recommend to migrate DSM 6 packages to use the same privileges (and shared folder handling) for all packages that are fully supported on DSM 7+.

Unfortunately for the upgrade of existing packages this must be done in two steps
In the first step the package must still have root access for the installer, but the service runs as package user. This is to change ownership of all package files for the package user.
If this is done correctly, then future packages can be deployed without any root access.

We currently have installer files for dsm5, dsm6 and dsm7.
For this new approach we will need another installer file for dsm6 without root permissions. This will look similar to the dsm7 installer except for DSM 7 specific features. I.e. all the permission related functions will not be supported.

When those DSM 6 packages use shared folders, it should (or must?) use the DSM 7 version and define USE_DATA_SHARE_WORKER = yes (see #5649).

[hgy59]

I just ask the question: is this worth maintaining DSM 6 support? We may decide to only deliver package for DSM 7.

My answer is YES.

There is a reasonable count of models that are limited to DSM 6.
(And there will be other models, that will not support the upcoming DSM 7.2)

Perhaps we should update the DSM 6 package to DSM 6.2.4 instead of 6.1 as soon, Synology will drop support for DSM < 6.2.4 (and remove all packages for unsupported models) see (https://www.synology.com/en-global/products/status/EOL-announcement-for-legacy-dsm).

[mreid-tt]

@hgy59, thanks for the suggestion with using a privilege file. Looking at its use in the developer guide I had a thought to perhaps move all the DSM6 specific stuff requiring elevated privileges to a separate 'tool' which would run as root but the rest of the service setup would run as the package. I don't think I did it correctly though. Is this a good solution?

EDIT: I think I may have it backwards, moving everything to package breaks the installer. I'll try moving only the specific one that needs package to a separate tool. As far as how to execute it and the configuration of the privileges file, I may need some assistance.

[mreid-tt]

I've tried to move the function that I needed run as the package to a command file but regardless of whether I define it as a tool or an executable, the log shows it not running as the package:

Console has to be executed with the user that owns the file config/config.php
Current user: root
Owner of config.php: http
Try adding 'sudo -u http ' to the beginning of the command (without the single quotes)
realpath: 'Console has to be executed with the user that owns the file config/config.php
Current user: root
Owner of config.php: http
Try adding \'sudo -u http \' to the beginning of the command (without the single quotes)': No such file or directory

Am I defining it correctly?

spksrc/spk/owncloud/src/conf/privilege

Lines 32 to 35 in 556773e

	"executable": {
	"relpath": "bin/occ-cmd.sh",
	"run-as": "package"
	},

Am I calling it correctly in my script?

spksrc/spk/owncloud/src/service-setup.sh

Line 13 in 556773e

OCC="${SYNOPKG_PKGDEST}/bin/occ-cmd.sh"

[mreid-tt]

I've completed testing for DSM 6. I had to implement a number of workarounds to get the web interface to run as the package user but it should be relatively clean. As the executable mentioned above is still not working, I included a few fail-safes to allow for the command to fail gracefully and not lose user data.

All in all it's a bit hack-y but it should be resilient and clean-up after itself on uninstall. Welcome to feedback from reviewers. Also, if the correct setup for executable with package permissions could be resolved that would be great!

[mreid-tt]

@hgy59, I believe I've resolved the issue by using a helper function in the service setup. Testing on DSM 6 and DSM 7 has been successful for all the required checklist items. Ready for final review.

EDIT: In the end I opted not to use a privilege file since (a) there is not a way to specify the DSM version it should be used with (i.e. one for DSM 6 and the default for DSM 7) and (b) I was not able to get the appropriate privileges when using either an 'executable' or a 'tool'. As such, a solution just running su in DSM 6 to manually run as the package seemed to work well.

[mreid-tt]

@ymartin59, reaching out to you as the listed maintainer of ownCloud for your review of this PR. Do let me know if you have any comments or questions.

[mreid-tt]

One open point: is this really an arch dependent package ? I don't see in the package any binary install, wouldn't it be worth flagging it as an explicit no-arch package ? (in the same manner as for cops for example).

You are correct, I don't believe this package compiles anything or is compiled beforehand since it seems to be a bunch of PHP scripts. I'll take a look at making a no-arch version.

[smaarn]

Only remaining remarks are really nitpicking IMHO

1. making it a no-arch could have some added value I believe.
2. considering using a custom service command to avoid having to handle the PID logic thingy could be nice

[mreid-tt]

Only remaining remarks are really nitpicking IMHO

1. making it a no-arch could have some added value I believe.
2. considering using a custom service command to avoid having to handle the PID logic thingy could be nice

Thanks for the feedback. For these points:

1. The no-arch config has been added and seems to work well (tested with DSM 6 and DSM 7).
2. The SERVICE_COMMAND didn't work as the OCC utility does not have a --daemon or similar option.

[mreid-tt]

On my side there are only two things:

1. Backup mechanism which I'm not necessarily sure of (overwriting it twice makes the initial backup potentially broken and potential issues if the same principle is reused by other mechanisms)
2. Usage of the single = when testing values in shell scripts which is not really inline with existing conventions from what I see. But that may be C/C++ devs PTSD (not sure if DSM supports bash in all its flavours for the supported versions but that's another topic).

Will give a crack at installing it (though it would only validate DSM 7 installations)

Thanks for the feedback. For your points:

1. Your comment on the backup and restore function being fragile due to potential overwriting was noted. It has been fixed with an explicit replacement using sed.
2. My understanding from https://www.shellcheck.net/wiki/SC2272 is that comparisons in shell should use a single = with the comparison statement in square braces [].

[mreid-tt]

Hello @th0ma7, I just wanted to let you know that the PR has been merged successfully. Thank you for taking the time to review and approve it. Could you kindly assist with publishing it? Your help is greatly appreciated.

[th0ma7]

@mreid-tt I kicked off the build + publish. I may be able to follow-up tomorrow or the day after to enable online. https://github.com/th0ma7/spksrc/actions/runs/4538303401

[th0ma7]

Well that was fast (probably due to being noarch now).
Build + upload + enablement done.
cheers!

[th0ma7]

@hgy59 I just noticed that noarch builds uses 7.0-40000 ... Not an issue but unaligned with the 7.1 for per-arch builds.

[mreid-tt]

Build + upload + enablement done.

thanks @th0ma7 !

[mreid-tt]

hey @th0ma7, I was looking at the repo and I only noted the DSM 7 build but not the DSM 6 no-arch. Is the cache still catching up?

[th0ma7]

@mreid-tt this is odd as the build process worked out all-right https://github.com/th0ma7/spksrc/actions/runs/4538303401/jobs/7997117592

I ended-up publishing manually this morning. It's now activated, should appear shortly thereafter. cheers.

[mreid-tt]

@mreid-tt this is odd as the build process worked out all-right https://github.com/th0ma7/spksrc/actions/runs/4538303401/jobs/7997117592

I ended-up publishing manually this morning. It's now activated, should appear shortly thereafter. cheers.

@th0ma7, just to close the loop on this odd behaviour, this was because the server gave an internal server error which was not handled (we have since handled it in #5701). The logs confirm this as follows:

Build (noarch-7.0) succeeds!

http --verify=no --ignore-stdin --auth ***: POST https://api.synocommunity.com/packages @/github/workspace/spk/owncloud/../../packages/owncloud_noarch-dsm7_10.12-9.spk
  {"package": "owncloud", "version": "10.12-9", "firmware": "7.0-40000", "architectures": ["noarch"]}

Build (noarch-6.0) fails

http --verify=no --ignore-stdin --auth ***: POST https://api.synocommunity.com/packages @/github/workspace/spk/owncloud/../../packages/owncloud_noarch-all_10.12-9.spk
  <html>
    <head>
      <title>Internal Server Error</title>
    </head>
    <body>
      <h1><p>Internal Server Error</p></h1>
      
    </body>
  </html>

For the root cause, I believe we have come to the answer to in SynoCommunity/spkrepo#95 with a fix proposed and in review.

[mreid-tt]

hey @publicarray, based on issue #5717, it appears that the publish process seems to have been corrupted with the database reporting an upload which is not present. I would appreciate your assistance in deleting version 10.12-9 of ownCloud from the repository. Once done I'll manually re-publish the files one at a time to avoid a race condition.

[hgy59]

@mreid-tt I can delete this for you.
shall I delete package for all (i.e. DSM >= 3.1 <6) too?
(it is not activated and the Makefile declares REQUIRED_MIN_DSM = 6.0)

EDIT:
done, left owncloud for DSM 6.1 only (of version 10.12-9)

[mreid-tt]

hey @hgy59, thanks much. The build for DSM 3.x is a bit weird and may need an adjustment in the Makefile on the next update. I believe it may have something to do with this line:

spksrc/spk/owncloud/Makefile

Lines 41 to 42 in 19ec801

	# Pure PHP package, make sure ARCH is not defined
	override ARCH=

EDIT:

• Okay, leaving the one for DSM 6.x should be okay. Let me try to re-upload the one for DSM 7.x.
• It looks like it was successful. Thanks again.