spksrc 🚀 - Meta: DSM6 compatibility of packages with user privilege issues

I am busy changing the NZBGet package in such a manner that it will be completely compatible with DSM 5 and DSM 6 and in according to Synology standards with as much possible use of DSM tools.

The current version in of that package in the DSM 6 branch is not able to do that correctly in my opinion.
It is important to use as much as possible the tools as provided by Synology, this to prevent that in the future problems arise from incompatibility, like the DSM 5 to DSM 6 transition.

I almost finished the work and will begin testing tomorrow.
If testing is successful, I will make it available to the SynoCommunity.
If agreed upon the changes I will be happy to help with other packages.

BenjV on 2 Sep 2017

May it be possible to document a set of "recipes" describing how to create a package "the right way", typically with a "installer.sh" template script when a technical user is required for service process, and how to port existing spksrc package ? Information is spread in lots of issues and comments... it would be perfect as a wiki page ! Thank you in advance

ymartin59 on 2 Sep 2017

I agree that the information is spread all over and the "3-rd-party Package Developer Guide" in not a very easy readable document.
Also there are a lot of "syno..." commands that aren't documented at all but are very needed to make a good package.
I will create a document with all my findings and specifications in a (hopefull) readable format and all the .sh (and other) files needed to build a package.

Not sure if I want to spend the time learning how to use wiki for that.

I hope someone can test the package I create on a DSM 5 machine, because I don't have one, both my machines are on DSM 6.

BenjV on 2 Sep 2017

I run a XPEnology 5.2 as vmware virtual machine. I should document how to build it too !
Please write your notes "raw", I will do wiki formatting after.

ymartin59 on 3 Sep 2017

Table above updated with WIP notes for nzbget and squidguard.

@BenjV: Great, you are in for it, too! I'm aware of your discussion with @Dr-Bean some months ago but do not have any strong feelings on the approach. I will just wait for your POC PR. Maybe @ymartin59 and @cytec can judge which route is best and then I will help implementing whatever you guys settle on.

m4tt075 on 3 Sep 2017

@m4tt075 sabnzbd and sabnzbd-testing are listed as fixed, but if I read the issues you link it seems that it's not fixed?
I also had some users reporting that after every update they have to manually fix permissions on DSM6 by changing the user to root in the boot script. I assumed this was because of it not being fixed yet due to this being still open: https://github.com/SynoCommunity/spksrc/issues/2216. If it should work for them, I will instruct them to create issues here.

Safihre on 3 Sep 2017

@Safihre : The 5.2 package on the master branch, which is published via the Synocommunity platform, is not compatible with DSM6. If people install them on DSM6 systems, they are bound to run into the kind of trouble you describe. @Dr-Bean's fixed version is only available on the DSM6 branch (therefore the comment in the above table). You should be able to cross-compile those packages and run them (correctly) on 6.1+ firmwares ("should", because I did not test it). The problem with those DSM6 compatible packages is though, that they can currently not be published via Synocommunity. This is the catch 22 with all affected packages.

m4tt075 on 3 Sep 2017

👍1

Table update:
Fix for transmission is already available on DSM6 branch (implemented by @Dr-Bean)

m4tt075 on 5 Sep 2017

@ymartin59 : Help! I just went through the DSM6 branch and realized that @Dr-Bean has already "fixed" almost all of them half a year ago. I simply checked which packages already possess the necessary privilege files and the list below showed up. It is unfortunate @Dr-Bean has suddenly disappeared but I believe he has actually done most of the DSM6 compatibility work already and was "just" lacking the publication platform to initiate broader testing. Might be wrong though. I started offering test builds initially (like you have done in the past), but given the breadth of packages we are looking at here, I don't think that is a viable option. What is your take? How should we proceed?

Here the additional packages @Dr-Bean already addressed (all untested of course):
bitlbee
boxbackup-client
couchpotatoserver
couchpotatoserver-custom,
domoticz
deluge
ejabberd
ffsync
flexget
gateone
git-server
haproxy
headphones
headphones-custom
homeassistant
htpcmanager
icecast
jackett
lazylibrarian
maraschino
memcached
mosquitto
mylar
nzbget
nzbget-testing
nzbhydra
nzbmegasearch
octoprint
plexivity
plexpy-custom
pyload
radarr
redis
rutorrent
saltpad
sickbeard-custom
sickrage
sonarr
squidguard
subliminal
syncthing
znc

m4tt075 on 5 Sep 2017

I am sorry to say that the already changes packages on the DSM 6 branch are having some flaws.

I have finished my testing of the NZBGet package.
I used the work of Dr. Bean as a basis to created a package that will work on both DSM 5 and DSM 6, with minimal impact.
I am now busy writing the concepts of such packages that can be applied to all packages..
Probably tomorrow I wil post it here, so someone can test it on a DSM 5 environment.

I still have to do some thinking on how to handle the following scenario.
Packages installed on DSM 5.
NAS was upgraded to DSM 6.
Upgrade the package to a new version after the NAS upgrade.

BenjV on 6 Sep 2017

👍1

@BenjV If same Makefile can produce both DSM 5 and 6 packages, it would be perfect.

For "upgrade path", it is really difficult to test without real hardware, as virtual machines we may find are not designed to support DSM update/upgrade.

Would it be reasonable to state that "we try to" support in place upgrade of "old" installed package version (which may be really old, some builds in repository have been produced for DSM 4 !) when already running on DSM 6. In case our "tentative" is not successful (and there is high chance it is not without proper testing), we would have to do support to help users to clean up their system, as uninstall scripts often fails on DSM 6. Worst option remains: re-install fresh DSM 6, reapply config backup and reinstall all packages....

What do you think about it ? World cannot be perfect... I wonder if there are many users having upgraded from DSM 5 to 6 with "complex" package installed (typically with database schema)

ymartin59 on 6 Sep 2017

The MakeFile can fore sure be used for DSM 5 and DSM 6, but it have to changed a little.
The placing of the firewall file .sc is wrong it should be located inside the package.tgz.
I suggest to put it in the /app folder along with the /image folder and the config file.
A resource file should be located in the /config folder in the root of the package in which a reference to that .sc is made like this.
{
"port-config":{
"protocol-file": "app/.sc"
}
}
I know it is very difficult to test this upgrade stuff, so I am trying the theoretical approach and see what issues comes up.
Later this day (I hope) I will post more details.

A cleanup script would be nice, I myself have had some trouble removing packages while testing.
I will give it some thoughs, when I have finished the current work.

BenjV on 6 Sep 2017

I have finished the package for NZBGet.
It can be found here:
https://github.com/BenjV/SYNO-packages/raw/master/nzbget_avoton-6.1_16.4-22.spk

I have used the avoton version but the important things are in the script files which can be reused.
Just get the INFO file, the scripts, WIZARD_UI, the config folder and the /app/config out of the package en put it in the spksrc framework.

The main difference is that this package will run as Synology has layed out in their documentation and that the package wil run with user privileges and not with root.
I made as much as can be done use of all the standard variables supplied by Synology.
There is no need for two users like "nzbget" and "sc-nzbget"
The "system internal user" which is used has limited visibility in DSM and priviliges can be granted via a group.
I changed the WIZARD_UIFILES so that the user can choose the name of that group and also the name of the Share which will be created.

If someone can test it under DSM 5 I would be very interested in the line of the /etc/passwd file of the nzbget user. This will show if the package is completely compatible with DSM 5 or not.
If not I may have to make some changes to use a "normal" user and not a "system internal" user if running on DSM 5.

I am working on a more elaborate document but my time is limited at the moment.
Feel free to post questions if you have.

BenjV on 9 Sep 2017

👍3 🎉1

Great. Thanks, @BenjV. Could you please PR your changes (I'd propose against the DSM6 branch until it has been tested more broadly) to make sure we are all on the same page? I have set up a VM with XPEnology 5.2. Happy to test for you...

m4tt075 on 10 Sep 2017

I am sorry but I am not familiar with the spksrc structure.
I just changed an existing package so it will work and that's not exactly the same as the structure I see on GitHub, so you have to make the translation and create the pr yourselfs.
You have scripts to create those files and I don't no how that functions and I don't have the time to figure it out at the moment.

I can list the files which I changed.
The following files where changed:
INFO
/conf/privilege
/scripts/installer
/scripts/start-stop-status
/WIZARD_UIFILES/install_uifile
/WIZARD_UIFILES/install_uifile_fre

I removed the following file because the were are not needed.
/WIZARD_UIFILES/install_uifile_enu
/WIZARD_UIFILES/upgrade_uifile
/WIZARD_UIFILES/upgrade_uifile_enu

By the way, there were structural errors in the Wizard-UI files so the input validation did not work at all and the other scripts were full of potential risc regarding filenames. There were missing a lot of "" around variables.
That's risky because of potential globbing or word spiltting, for example if someone uses a space in a filename.

BenjV on 10 Sep 2017

👍2

Many thanks for your work @BenjV. I will report your change on package into spksrc as a PR.

ymartin59 on 10 Sep 2017

👍1

Perfect. Thanks, both.

m4tt075 on 10 Sep 2017

Hi, i am new to the synology community and note that even after enabling betas in DSM package manager, i still am not able to see all packages listed. I presume particular packages have been pulled due to incompatibilities or is there something else i am seeing?

For example - Headphones is not an available package however listed on the synocommunity page.

thank you

drpau on 12 Sep 2017

@drpau: Your problem is related to a lack of DSM6 compatibility of publishable packages. This issue is part of a more general approach to sort these things out. Please open a separate issue on the package you are looking for, if it does not already exist.

@ymartin59 : I have seen you closed #2912 (your first DSM6 merge branch). No prob, but I think we need to step back and sort some things before we can advance here. I have also seen that your are currently reorganizing the issues as projects, which is great. Here my perspective on what we still need to clarify for your consideration and potential reflection in your project overview:

I have seen @BenjV's proposal, understand some (not all) of the changes and believe some concerns @Dr-Bean raised in PR #2216 are not addressed. Please correct me if I'm wrong, but my understanding is that you wanted to review and PR @BenjV's changes (with or without adaptations?), which should provide the right place for those discussions. Until learning otherwise, I will watch out for that PR.
Once it is clear which way to go and we deviate from the @Dr-Bean approach @maxrogers and I applied, I'd suggest broader testing of sample packages before we scale up. Happy to contribute TVH as guinea pig again. Others (e.g. @Safihre?) might be willing to step in, too. But I would be hesitant to scale up the approach to all the packages above before we have some reassurance that the new approach works reliably.
Once we are reassured, we need to coordinate package updates and DSM6 compatibility updates or we will overwrite updated non-compatible packages with old compatible packages or vice versa.

Ok for you?

m4tt075 on 13 Sep 2017

👍2

@m4tt075 I really agree. I have not reviewed BenjV's proposal yet but I also guessed it deviates from first approach.

About point 3 and my master-to-dsm6 merge, my aim was to be able to locally merge package branch with dsm6 to produce packages with DSM 6 toolchains without generating conflicts.

From here, I think any package compatible with DSM 6 should be sent to dsm6 branch, even if it is retrocompatible with DSM 5 too.

Probably one of our issue is the copy-paste pattern applied to installer.sh. A proposal may be to provide base features globally in mk/ as functions and invoke them from installer.sh.

ymartin59 on 13 Sep 2017

@ymartin59 : Yes, makes sense. We have got a plan then. Let me know if I can help...

m4tt075 on 13 Sep 2017

I also red @Dr-Bean's concerns and I will comment here on how I have dealt with them.

-About the privileges file:
I agree that the user must be created in the privilege file and that no group should be created there.
I disagree with the "sc" adding to the username, that only confuse thing and has no purpose.
Don't use the "wonky" busybox commands but use (as I did) the synouser and the synogroup command, I am busy creating some text explaining how to use them.
Just leave the username equal to the package name.
The comment that the existing users may not be removed because there are not our users is simply not true, they were created in the past by installing the SynoCommunity package.
We have to remove it, because if it still exitst the installer of Synology will create a user with a different name and we no longer have access to that name to be able to add it to a group.

My original idea was to also use the group in the installer, but there is a bug in the installer from Synology that would not allow the same group for different packages.
I contacted Synology about that and they agreed with me that it was a bug and that they will look into it to fix it in "some" future release. So we cannot use that at the moment.

Also we have to add the "status" to run-as root.
To check if a package is running we use the kill -0 and that commands needs root privs.

About the installer:
Again don't use the wonky busybox commands.
I would suggest to leave out the installation of busybox completely.

-Preugrade
Here I remove the legacy user, we don't want them anymore because they are member of the group "users" and so anybody who is a user has access to any file a package creates.
That is from a security standpoint undesirable.

-Postupgrade
Here I create the group and add the user to it.
I disagree to make existing users a member of the group "users" again as I explained.
Adding the user to the group sc-media or sc-download gives sufficient control.
Maybe we should think about some more different "sc" groups, because some packages would be hard to classify to one om those two groups.
I also think it is a very good idea to make the user name equal to the package name, this way there is no need to add to the installer and we do not have to change that for all packages.
This is needed if we want to make script files that are common for all packages.

About compatibility we have to wait the result of the testing, I think even under DSM 5 and with the privilege file in place, DSM 5 would create a hidden user, if not we have to create a legacy user ourselves, my package already takes care of that .

Start-stop-status
Don't use the su command to run the user the package, start the package, Synology will take care that it "run-as" the user from the privilege file, don't duplicate things.

I would suggest to move the standard stuff that is in the installer to the preinst and postinst scripts and just keep the package specifics in the installer.
That's why it is a good idea to keep the package-name and the user-name the same, we could use the environment variable supplied by Synology for both and those script could be the same for everything.

I don't know if the spksrc scripts create the firewall (package.sc) file which I found in the script folder.
If we put a resource file in the "conf" folder and the firewall (.sc) file in the /app folder we don't need the "servicetool" command to install/de-install the firewall file as I did now.
Both solutions are valid.

The main difference for the end-user will be that no longer all users (via the users group) have read/write privs on everything, but has to grant it themselves via the "sc-media" or "sc-download" groups.
But that's the price they have to pay for more security.

I will work on moving the general part of the installer to the standard scripts and finishing the documentation.
If somebody still has concerns that my solution does not address some points from @Dr-Bean please be specific so I can react.
I put a lot of thoughts in this solution and I think I covered all the angels but I am always open for other ideas.

We have to test that all packages will run with user privileges and not need root or system privileges.
If all scripting has been testen and found OK we should start releasing all packages as beta DSM 6 so end-user can test it.
We also have to tell them how granting privs via the sc-media and sc-download groups work, because a lot of end users have no idea how privs work.
I already added some of that info to the WIZARD_UI files, but mayby some text on the synocommunity.com site can help.

BenjV on 14 Sep 2017

@BenjV : Wow, you have obviously thought a lot about this. The concerns I was referring to where the ones about using different user names, but I assumed (and did not verify) that what @Dr-Bean wrote, was correct. If it wasn't and this is no longer an issue, that's great. You have also already answered most of the things I did not understand. The only additional questions I' have right now are
a) How we are going to deal with DSM upgrades
b) Whether differentiating btw major firmware versions (rather than the detailed one like in the old approach) is sufficient (if I remember correctly the breaking point was 6.0.2 or something
c) Why the comment in postinst refers to DSM5 or lower, but you test for -lt 7
But it is all academic without running tests, so please don't worry, all of this can wait. I'm keen on testing your new approach...

m4tt075 on 14 Sep 2017

The removing of the old name is in my opinion not a problem, those users were created by the SynoCommunity package and if somebody used that name for other purposes he (or she) has to create a new name. I don't think many users have done that, and the few that did it have to take some action.
The name clearly state that it is part of the packages because it is always the same a the package name.

Also if we leave the old username in place it will create enormous confusion, because it doesn't function anymore in settings privs.

We need to keep the username the same as the package name otherwise we don't have a way to find out this name within the scripts and we have to add a new name for every package we have.
Now we can use universal scripts for every package and I have already started to create them.

About your other concerns.
a) I have thought al lot about that but I cannot see a specific reason why it should not functionate.
Old installation will use an old user which keeps functioning and is still part of the "users" group which is very unsecure, but then it always was.
When a package update occurs (or a new installation) that user will be removed and change to a new "hidden" user with added security,

b) No, the breaking point was the change from DSM 5 to DSM 6, and to be honest I believe even DSM 4.2 already has all the tools in place to make this more secure working possible.
This was accomplished by adding the "support_conf_folder = yes" setting to the INFO file, this setting is as of DSM 6 deprecated, because as of that version it is mandatory.
Whether I am right we will see as soon as someone test my package on DSM 5, if so it will not create a normal user but a hidden user which we can see in the /etc/passwd file on the UID it got.
If someone has a DSM 4.2 environment available we can also test it on DSM 4.2

c) Very good spotting, I saw it yesterday myself, it is a leftover from some testing I did and should be "-lt 6" and not "-lt 7",

BenjV on 15 Sep 2017

Any news from the testing of my package on DSM 5?

BenjV on 24 Sep 2017

@BenjV package.tgz in avoton package I downloaded from your URL is in fact a non gziped tar and it is almost empty... May you please check and package binaries with it ?

ymartin59 on 24 Sep 2017

@ymartin59
Ok I added the binaries, although they are not necessary to install the package.
I removed it to save time during my own testing.

I am especially interested to see how the /etc/passwd file will look like on DSM 5

BenjV on 24 Sep 2017

@BenjV Installation wizard is OK except "Attention ! DSM permissions" step does not show expected content.
DSM 5.2 finally complains it fails to start package repair - no idea what it means - I found this in /var/log/message:

Sep 24 16:00:35 xpenology52 entry.cgi_SYNO.Core.Package.Control[1].start[31981]: pkgstartstop.cpp:187 Failed to start package nzbget, [127], No such file or directory
Sep 24 16:00:35 xpenology52 entry.cgi_SYNO.Core.Package.Control[1].start[31981]: pkgstartstop.cpp:196 Failed to start package nzbget, [127]

In var/log/synolog/synosys.log:

info    2017/09/24 15:55:24 admin:  User [nzbget] was created.
info    2017/09/24 15:55:25 admin:  Group [sc-download] was created.
info    2017/09/24 15:55:25 admin:  Package [NZBGet] has been successfully installed.

By the way, /etc/passwd contains nzbget:x:1026:100:User running nzbget:/var/services/homes/nzbget:/sbin/nologin
and /etc/group contains sc-download:x:65536:nzbget

ymartin59 on 24 Sep 2017

Ok I will look into that.
Likely some mistake in the start command, I don't have a Avaton NAS so I could not test it.

I now know that under DSM 5 will have to create a normal user, but that I already had anticipated.
Is the user "nzbget" added to the groups "sc-download" and "users"?
Has the share you choose in the wizard been created and does the group "sc-download" privs there?

BenjV on 24 Sep 2017

I have changed "avoton" to "bromolow" to test in XPEnology 5.2 VM. User and groups are correct. But no share has been created.

ymartin59 on 24 Sep 2017

The creating of the share was just an extra I added to the wizard and not really needed.
That can be done by the users themselves via the DSM GUI.

Mostlikly the "synoshare" command does not exitst in DSM 5.
Maybe you can test that on the commandline.
Just give the following command:

sudo synoshare --help

BenjV on 24 Sep 2017

it does exist:

diskstation> synoshare --help
Copyright (c) 2003-2014 Synology Inc. All rights reserved.

Usage: synoshare (Version 5967)
        --help
        --build
        --get sharename
        --get_by_attr PATH share_attr
        --enc_mount sharename password
        --enc_unmount sharename1 sharename2 ...
        --enum {ALL}|{LOCAL|USB|SATA|ENC|DEC|GLUSTER}{+}
        --enumpre {ALL}|{LOCAL|USB|SATA|ENC|DEC}{+} prefix caseless{0|1}
        --enummnt mntpath
        --del {TRUE|FALSE} sharename1 sharename2 ...
        --add sharename desc path na rw ro browsable{0|1} adv_privilege{0~7}
        --rename  old_sharename new_sharename
        --setdesc sharename desc
        --setvol  sharename volume_id
        --setbrowse sharename browse_flag{0|1}
        --setuser sharename user_auth{NA|RO|RW} operator{+|-|=} user_name_list_with_comma
        --setrsec section_name
        --getrsec section_name
        --getmap sharename
        --delrsec section_name
        --getrval section_name key separator
        --dbopen2 sharename
        --list_acl sharename
        --set_share_default_acl sharename

jelmerj on 26 Sep 2017

OK I will try to figure out what the problem is.

BenjV on 26 Sep 2017

No idea yet... I passed commands by hand in XPEnology DSM 5.2 VM and got expected result. No idea what went wrong in installer script, maybe direct output to log file will help.

xpenology52> synoshare --get "download"                 
Lastest SynoErr=[share_db_get.c:70]
SYNOShareGet failed. synoerr=[0x1400]
xpenology52> synoshare --add "download" "NZBGet Share" "/volume1/download" "" "rw" "" 1 0          
xpenology52> synoshare --get "download"
SYNOSHARE data dump:
     Name .......[download]
     Comment ....[NZBGet Share]
     Path .......[/volume1/download]
     Deny list ..[]
     RW list ....[]
     RO list ....[]
     fType ......[2]
     fBrowseable [yes]
     FTPPrivilege[0]
     Status .....[0x1880]
     WinShare .....[yes]
     ACL ..........[yes]
     Skip smb perm.[yes]
     Permit .......[yes]
     FileIndex ....[no]
     RecycleBin....[no]
     RecycleBinAdminOnly....[no]
     HideUnreadable ........[no]
     Snapshot browsing .....[no]

ymartin59 on 26 Sep 2017

@BenjV Please review script available in dsm6 branch at: https://github.com/SynoCommunity/spksrc/blob/dsm6/spk/nzbget/src/installer.sh

It has been processed @Dr-Bean for DSM 6 support and upgrade path. As migrating from DSM 5.2 "legacy" users and group management to DSM 6, I think it is relevant to refactor to simplify and keep closer to standard DSM packaging.

I really appreciate your proposals:

let DSM 6 create user as declared from conf/privilege
use DSM 5 synouser (so no longer depends on busybox adduser probably required for older DSM)
handle file share creation and its group permission

To validate these concepts and apply them to almost all packages, I would like to confirm some hypotheses:

DrBean script renames user with sc- prefix probably because of lack of synouser --rebuild --all invocation
BUILDNUMBER is used because SYNOPKG_DSM_VERSION_MAJOR was not available on older DSM versions
synocommunity group creation has to be enforced to allow its automated removal. A wiki page should refer synocommunity group names and which package use which group.
even if package creates share, its deletion is user responsibility

My proposals are:

hardcoded group name (no wizard)
no longer maintain DSM 4 or older specific support
generate installer script from Makefile variables so that packages are easier to migrate and maintain: legacy service user removal, hardcoded group management (creation/deletion), share creation, stop/start/log support

Please comment. Proposal implementation will come in #2915

ymartin59 on 26 Sep 2017

About your hypotheses:

DrBean created an extra user with the sc- prefix because he wanted to run the package with a run-as under the user with the sc- prefix. The same name as the package could not be used.
SYNOPKG_DSM_VERSION_MAJOR was always available according to the docs, but maybe DrBean didn't read the complete doc.
That is one solution, we could also leave it to the enduser to remove the group. It won't hurt if it stays behind.
Of course deleting a share should not be done by the package uninstaller.

The problem with the sc- user prefix is, if the package is running as a normal created user, all other users get access to anything that package creates, because of the fact that all normal users are part of the group "users".
The Synology idea is, to run the package as a hidden user that is not part of the group users.

So if we add that user to a group with the sc- prefix (or let them choose a group themselves), then the end user can decide themselves to grant privs via that group, and we keep the package user hidden.

For the DSM5 users that won't work because the hidden user is not created.
So if the packages is installed under DSM 5 (or DSM 4) we create a normal (not hidden) user and the situation will be the same as in the past.
If they upgrade from DSM 5 to DSM 6 this will stay the same, unless they remove the package and do a new install.

About your proposal:
If we use a hardcoded groups, we must make the decision which package to put in which group.
In that case just two groups won't be sufficient I think.
Also in that case we must hardcoded it in every package, but if we put it as a standard in the WIZARD_UI, we keep it the same for every package. That's a lot less maintenance.

Stop the specific DSM 4 support is OK, but not necessary.

From a programmers point of view I would create standard, postinst, postuinst,preinst etc. etc scripts. which are the same for every package.
And only the start-stop-status and the installer script would be dedicated to the package.
Because there will be variations in how to start the package and also the use of the Wizard_UI.
Like the NZBGet package that needs a folder for the downloads.

BenjV on 26 Sep 2017

@BenjV Thanks for your response. I work on script and we will discuss details on code...
@m4tt075 NZBGet package is not the easiest to test as it is arch/DSM dependant. Is there any non-arch dependant package which requires a service user to run background process (and maybe a shared folder too) ?

ymartin59 on 28 Sep 2017

@ymartin59 Hmmh, transmission maybe. Looks(!) more straightforward and is in demand, giving us potential testers later.

m4tt075 on 28 Sep 2017

@m4tt075 & @ymartin59
Transmission is not a noarch package.
You should look at python applications like SickRage or SabNzbd.

I already have created a SickRage package, but I will incorporate my latest proposal for the scripts layout.
I have time available this weekend, so I will let you know when I finished and you can test it.

BenjV on 29 Sep 2017

@BenjV I have already started significant work on "installer.sh" script to be include in srcspk build chain, I propose you wait before working on another package... And I think I pointed one issue with start-stop-status, on DSM 5, it has to run command after su - $USER

ymartin59 on 30 Sep 2017

I think you right.
So you can something like this in the start_stop_status script.
Where "binary" the original command to start the package is.

if [ $SYNOPKG_DSM_VERSION_MAJOR -lt 6 ]; then
su - $user binary
else
binary
fi

But maybe also under DSM su - $USER is OK
Ok I shall wait for you version, let me know if I can help with things or review the scripts.

BenjV on 30 Sep 2017

Hello. Here is a point of situtation about what will come in my PR proposal:

"service" support with generic installer and start-stop-script
optional support for SERVICE_USER creation (provided in Makefile, same as package name recommended)
optional support for group creation (name from SERVICE_WIZARD_GROUP wizard field name)
optional support for share creation based on download location configuration from wizard (SERVICE_WIZARD_SHARE field name) and if group is also configured, apply ACL to grant group write permissions
hook functions to allow additional package specific behaviors over generic scripts
extensive logging of installer script to help diagnose when failure is reported
make targets to pack generic scripts according to developer variables SERVICE_ provided in Makefile
make target to generate conf/privilege to enable DSM 6 run-as USER
a "demoservice" package to demonstrate and test this "service" support usage and capabilities

Another point I am interested to include is a generic PID_FILE support for "raw" process.
It still require lot of testing on both DSM 5.2 and DSM 6... I hope it to be over this week.

ymartin59 on 3 Oct 2017

🎉1 👍1

Sounds very good to me, great job.

I suppose that with SERVICE_USER you mean that user created via the privilege file?
If so that cannot be optional, because every package will need one.

I suppose you are already implying it but not written down her.
Add the SERVICE_USER to the SERVICED_WIZARD_GROUP

Let me know if I can help with testing, reviewing or writing scripts.
Especially I am interested in the behavior of the python package (I mean python itself).

BenjV on 3 Oct 2017

I agree. This sounds great!

m4tt075 on 3 Oct 2017

The idea behind SERVICE_USER being optional, was to allow execution as root, or only propose a share creation (without background process...)... But I agree this design can be challenged...
I forgot to tell about optional SERVICE_SHELL and SERVICE_COMMAND in start-stop-status script. I should write a wiki page about it.

My objective is to ease creating and maintaining packages... and so migrating that list to DSM 6 support

ymartin59 on 3 Oct 2017

If a package needs to run as root (which should not happen), then we still need a user in the privilege file but it should have the tag run-as root(instead as "package") in the privilege file.
Like this.

"defaults":{
    "run-as": "root"
},
"username": "nzbget",

BenjV on 3 Oct 2017

👍1

Hello. As I am desperate to find time to struggle with my Makefile design... I decided to publish draft work #2949 so that you can already review and comment, even if it is not working yet. I am offline for few days but then I should have time to get it over.

ymartin59 on 7 Oct 2017

Ok I shall take a look at it tomorrow.

BenjV on 7 Oct 2017

@ymartin59
I have looked at your work and it looks great.
Your comment about adding pidfiles for packages that doen't support it themselves is very correct.
I would suggest that you add a pidfile for all packages to make it universal.
Put it in a location where it doesn't clashed with pidfiles from the package itself and then use it for the status in the start_stop_status script.
(And of course remove it when the package is stopped or crashed)

I would suggest here:
"/var/packages/${SYNOPKG_PKGNAME}/conf/${SYNOPKG_PKGNAME}.pid"

BTW the pid of the last started job can be found in the variable $!, so after the start of the application just add this command:

echo $! > "/var/packages/${SYNOPKG_PKGNAME}/conf/${SYNOPKG_PKGNAME}.pid"

When you can build packages I would be happy to test some packages,.
I prefer package for the "Armada38x" arch.

BenjV on 16 Oct 2017

👍1

Any news on this?
If it helps: I also have a bunch of users that would be happy to test the sabnzbd package for DSM6 (would also be helpful to have instructions what Debug info to send in case of problems).

Safihre on 19 Oct 2017

Makefiles are not ready to built yet. Meanwhile, here is a hand-crafted first draft version of DSM-independant no-arch demoservice package:
demoservice_noarch_5and6_0.1-1.spk.zip

Warning: I have even not tried to load it in DSM yet, not tested at all... so be careful, but if dare to test in xpenology for instance, do not hesitate to report any issue (or success but I have no hope it may work at first shot).

First benefit of discarding busybox is that a no-arch service application will produce a no-arch package !

My next idea is that bin/demoservice.sh will run some kind of python -m SimpleHTTPServer 6789 to deliver self package content as pages, with a menu icon link would be perfect too.

ymartin59 on 19 Oct 2017

First impression it looks good for starters

Some remarks:
In service_installer you use a hardcoded username.
I would suggest to keep it the same as the package name and replace,
${USER} with ${SYNOPKG_PKGNAME} in all the scripts

In installer you use ${Package} which is not defined.
I would suggest to also replace it with ${SYNOPKG_PKGNAME}

You cannot log to a folder that is not yet created e.g. preinstall fase
Better to log to the tmp directory.
So replace it with:
INST_LOG="/tmp/${SYNOPKG_PKGNAME}_install.log"
The /tmp folder is cleaned up on a regular base and during boot.

Don't use the line below anymore, it is a leftover from pre DSM 4
INSTALL_DIR="/usr/local/${PACKAGE}"
The install dir is $SYNOPKG_PKGDEST

Also don't create and remove this link (also a leftover)
$LN "${SYNOPKG_PKGDEST}" "${INSTALL_DIR}" >> $INST_LOG

In start_stop_status you use the su ${USER} to start the deamon.
Loose the "su" command, DSM will run it as the user which is defined in the privilege file.
This way you just create an extra shell which is useless.

BenjV on 20 Oct 2017

Many thanks for your valuable feedbacks. I am working on it.

To explain, I would like to keep /usr/local/PACKAGE for backward compatibility if users have absolute path to files when configuring other tool or building scripts... I find it relevant and affordable.

"su" command is to run for DSM 5 where "privilege" file is not supported yet, if I understood well.

Makefiles begin to produce what I expect... I have hope to deliver tomorrow...

ymartin59 on 20 Oct 2017

su is not needed in DSM 6
Also the line below is not needed, because DSM will do that for you.
chown --verbose -R ${USER}:root ${SYNOPKG_PKGDEST} >> $INST_LOG

In DSM 5 and older the package will be run as root but that was always the case.
If you want to change that, you could only use su if DSM < 6 and then you need the chown also.
But my suggesting would be to leave DSM 5 behavior as it always was.

It is dangerous to use the /us/local/Package because that is on the system partition which has limited space and also there is a change that in future release DSM will remove that folder.

I would suggest to stick tot the rules that Synology has set for 3-party packages.

BenjV on 21 Oct 2017

I understand your points which are relevant for brand new packages created targeting DSM 6.x

SynoCommunity packages have always created service user so that no background service or application runs as root - the reason why busybox was included to provide adduser on older DSM (probably 3.x and 4.x)

I wrote these scripts to provide migration path for existing users and also keep support for DSM 5.x

What is wrong at the moment is that binaries are built with /usr/local/PACKAGE as installation directory for linking. I will change this to default location /var/packages/${SYNOPKG_PKGNAME}. I consider backward-compatibility symbolic link /usr/local/PACKAGE harmless but useful to users.

ymartin59 on 22 Oct 2017

@BenjV May you please inspect PR about Radarr #2970 ? I wonder if SynoCommunity package should set service user home directory to /var/packages/${SYNOPKG_PKGNAME}/var so that all service files are located at same place. Such configuration has to be included in package backup/restore process too. What is your opinion about it ?

ymartin59 on 24 Oct 2017

No don't do that , it will create all kinds of confusion and access rights problems.

Radar assumes incorrectly that is run as a user with a home directory.
As of DSM 6 that is not the case anymore and that is the way it should be for obvious security reasons.

Normally applications should write the pid file in their startup location which is the location where they are installed e.g. /@appstore/radar and not in the user home folder.
Just like for example Sonarr and SickRage do.

The problem goes away if you create a pidfile within the start-stop-status script at start of the deamon:
Bet location should be:
/var/packages/${SYNOPKG_PKGNAME}/conf/${SYNOPKG_PKGNAME}.pid

Then all packages can do whatever they want with their own pid file and the start-stop-status script is then independent of the application itself.

BenjV on 24 Oct 2017

Hi I just want to thank the DSM6-compatibility contributors and let you know how much I appreciate your hard work!

aniceberg on 27 Oct 2017

Here is produced demoservice package:
demoservice_noarch-all_0.1-1.spk.zip

Tested on DSM 5.2 and works properly except uninstall do not remove service configuration, leaving port marked as used.
Now have to test and check details on DSM 6.x

ymartin59 on 4 Nov 2017

@ymartin59
You are missing the following line in the INFO file
dsmuidir="app"
Also you are missing this folder in the "package" folder.

In that folder there should be:

folder "images" with the *.png files in there
config file
*.sc file

If you could put the *.sc file in there, you don't need the servicetool commands anymore because DSM will take care of that.

By the way, nice logging functionality.

BenjV on 4 Nov 2017

@BenjV Thanks for feedback... I agree but "should" does not mean "must"... I was not aware of automated loading of "app/*.sc" file. Do you know from which DSM version it is supported ?

ymartin59 on 4 Nov 2017

In this case it is a must, because otherwise the application will not appear in the user interface and will only be visible in the package centre, also the firewall settings (*.sc) will not function.

The documentation clearly state that the *.sc file should be within the package.tgz file in the subdirectory specified by the "dsmuidir" in the "INFO" file.
This is supported as of DSM 5 when they introduced this *.sc (eg firewall settings).

BenjV on 4 Nov 2017

@BenjV In my mind, admin port may be a technical protocol like api or non http stream... in that case a DSM UI link is not relevant. In case of demoservice and most of packages we have to upgrade, it is. So here is additional automated support to produce "app/config" with DSM URL link.
demoservice_noarch-all_0.8-2.spk.zip

ymartin59 on 5 Nov 2017

I commented design and usage as wiki package: https://github.com/SynoCommunity/spksrc/wiki/Service-Support

ymartin59 on 5 Nov 2017

Hi Guys
I have an opportunity to go from DS418 to DS418play for something like 30$
As I understand the DS418play is an intel apollo lake, do you think I would have more chance seeing these packages being usable with the DS418play than with the DS418 (which has a realtek) ?
Or once the problems are sorted through with DSM 6, they will be available for all machines ?
Thanks a lot

tchirou on 10 Nov 2017

I have a similar question. I just got a DS218play and it has a Reaktek proc as well. Wondering if support for it will come when this releases.

rafieek on 11 Nov 2017

@tchirou @rafieek Apollolake support is already fully prepared, rtd1296 support isn't (see my comment in #2890). The issue we are discussing here is independent, and concerning DSM6+ support for certain kind of packages across platforms. It will take both, DSM6+ and rtd1296 support for your arm platforms to benefit. DSM6+ support is being worked on. I'm not aware of broader activities for rtd1296 today, but it might well be some people are working on it. You can "gamble" or wait...

m4tt075 on 11 Nov 2017

By gamble do you mean try to install packages once DSM 6+ support is released?

rafieek on 11 Nov 2017

Hi @m4tt075
Thanks for your feeback
I understand that I should migrate to DS418play to get the fastest Access to packages like python 2, git, hence SABnzb, SickRage, couchpotato. So I am going to take this path. Amazon says I should have it Monday 😊
Cheers

tchirou on 11 Nov 2017

I now think it may be useful to include also python wheels installation and deployment in generic installer script. Would do that with first python package to process.

ymartin59 on 25 Nov 2017

I agree

BenjV on 25 Nov 2017

@m4tt075 About transition from DSM 5 to new generic service support for both DSM 5 (synouser) and 6 (privilege), I also find out busybox userdel execution is required to clean up user from /etc/passwd.

As a result, I propose to add that userdel command in preupgrade per package scripts in master (only for these which create user) and rebuild for DSM 5.x only and publish them to repository.
After a grace period (one or two weeks), package for DSM 5+6 can be published and old user account will be removed before new creation according to current DSM version.

ymartin59 on 29 Nov 2017

Maybe a stupid question, say a user has sabnzbd installed: upon upgrading, folders owned by the previous sabnzbd user will then now be owned by the new sc-sabnzbd?

Safihre on 29 Nov 2017

@ymartin59 Might get us into trouble if users skip the preupgrade and upgrade to the DSM 5+6 package directly. If the old user is an "sc-*" user, this should not be a problem (you just have two users, one being obsolete), but if it is a user without the sc-prefix it will screw things up. This is what happened to me when playing around on the XPE 5.2 system.

My thought was to simply keep the busybox dependency in the DSM 5+6 packages and amend the remove_legacy_user and group functions to remove using syno-commands first (as is) and add the same removals using busybox-commands as fallbacks. Shouldn't do any harm and clean all installations over time...

m4tt075 on 29 Nov 2017

There should not be a problem because priv should be given via the group and not via the user.
In DSM 6 the package user is not visible to the DSM interface so privs cannot be given via the user and should be handled via the group.

So if we made shure that the package user is part of the sc-media group there will be no problem.
The only point we have to make to the end-user is to make clear privs should be handed out via the group.

The busybox is not needed at all and is not reliable as of DSM 6 (most likely also not on DSM 5), see also the initial problem DrBean had with creating user using busybox.
After a reboot DSM will rollback changes that were not made via the syno -commands.
The syno -commands will do the job fine, I have tested that very thoroughly.

BTW it is not possible for an end-user to skip the pre-upgrade phase.
If they want to install the package directly then they have to remove the old package first.

PS.
I have made my own python application Autosub and a package for SickRage this way and they are functioning very well for a very large group of users already for almost a year now.
The only question I every get is how to handle read/write privs and when I tell them to use the group sc-media all problems are gone.

BenjV on 1 Dec 2017

@Safihre BenjV is right, we should not care about user ownership, DSM 6 privilege framework generates random service user... the only proper way to handle file permission is thanks to group like sc-media or sc-download

ymartin59 on 2 Dec 2017

@m4tt075 Your objection is correct. Suppose a user skipped the intermediate package upgrade we publish with busybox userdel and jumped directly to DSM 6 package with privilege. What I tested is that DSM 6 framework does not re-use existing user with same name serviceuser but generate a technical service user appending a number (from memories, I have not wrote down precisely) serviceuser_PACKAGE_569087 . As a result, situation is not clean but package works properly.

I have to check again if synouser command really fails to remove service user created from busybox. I remember command fails with error message BUT user entry has been removed from /etc/passwd, and remains in group membership. This scenario concerns DSM 5 system upgrading old busybox package to new concept.

ymartin59 on 2 Dec 2017

I don't understand what the problem would be if a user jumped directly to the DSM 6 package.
If the package is already on the system, then DSM will do an upgrade and the old user can be removed in the pre-upgrade script with the syno tools.
If the old package is not there then DSM will do a full install instead of an upgrade.
To be double sure the old user is removed, use the syno tools to remove the old user also in the pre-install script.
It just fails with an error if the old user is not there, but then you have made sure that user is gone, to prevent DSM to create a user with a strange name.

About the question of the synotool, it always worked for me and if it does not we should put in a ticket and let Synology fix it.
What I observed is that in rare cases the removal of a user from the etc/password file with a editor and leave the user in etc/group, then after a reboot the user is gone from the etc/group.
But removing the user with the syno tool, it always was also removed from etc/group.

Please don't use busybox for removing old users, it does not remove it from the DSM database and so it can reappear after a reboot.
In the upgrade process from older to newer DSM versions Synology has imported those old users in their database so using busybox to remove them will create unpredictable results.

BTW You should really read the "3rd-Party Package Developer Guide" from Synology, all the things you are now finding out by trial and error are descripbed in there.

From page 99 of that guide:
"When the package user / group name conflicts with a local user / group, it will be renamed by adding a $PKG${time} postfix at the end."

BenjV on 2 Dec 2017

👍1

@BenjV Well I still have doubts, and I am just testing package I expect to deliver, creating initial situations to confirm it works well. One of them is what happened during DSM upgrade from 5 to 6.

Some findings:

user created by busybox are not visible in Control Panel / Users but appears in Groups / Edit Members
same behavior for user created by privilege framework on DSM 6
user is visible in Control Panel / Users when created by synouser. Admin may broke package removing service user from interface...

I am testing and still find troubles...

ymartin59 on 2 Dec 2017

Testing is fine and I will try to explain the results if I can or give my opinion about the issue.

• user created by busybox are not visible in Control Panel / Users but appears in Groups / Edit Members
This I don't understand, the new package should not create users with busybox, I explained that it will create lots of problems if you do so.

• same behavior for user created by privilege framework on DSM 6
This is as designed by Synology for the privilege user and I advise not to use the group in the privilege file because you cannot use the same group twice this way. It is a bug in the Synology framework which I contacted Synology about, they are aware of it but are not planning to change it.
So my solution was to not use the group in the privilege file but to create a group via the synotools and add the user created by the privilege file to that group (e.g. sc-media, sc-download)

•user is visible in Control Panel / Users when created by synouser. Admin may broke package removing service user from interface...
We should not create users at all is my opinion, leave it as Synology designed and let the package run as the user which is created via the privilege file.
This way it will stay hidden and end-users cannot remove it.
It also will not be visible as a member of the sc-media (sc-download) groups.

I strongly advice not to run the package as a self created user but just run it as the user created via the privilege file.
Don't deviate from the path Synology set out, this will bring trouble in the future.
The problem we are facing now stem from the fact that busybox was used in the first place and running as a own created user will produces the same problems in the near future most likely.

BTW If you are testing and use the synotool commands, don't forget to add a --rebuild all command.
It does not seem to do anything but it is crucial to update the DSM user/group databases.

BenjV on 2 Dec 2017

You missed some points - we try to provide support for both DSM 5 and DSM 6, with the easiest upgrade path for existing packages. Service user have already been created by busybox in packages delivered for DSM 5... And you have proposed to create user with synouser on DSM 5 to replace busybox. That is why I am testing varia situation to avoid support request when delivering package updates.

ymartin59 on 2 Dec 2017

No I did not proposed to create a user to run-as the package.

I said that it is necessary to test if a user will be created via the privilege file if running DSM 5, if that user is not created then we need to create it via the synotools, but don't do it via busybox.
I believe it will function under DSM 5 just as under DSM 6 but you have to test it.

And yes if we have to create it via the syno tools it wil be visible in the GUI but that cannot be helped and is the same behavior as the current situation.

And if the user is running a package with a user that was created via busybox then the upgrade can remove that user in the pre-upgrade script and/or it can be removed in the pre-install script.
Both can and must be removed with the synotools, otherwise the DSM user/group database will not be updated an we can expect strange behavior after a reboot.

BenjV on 2 Dec 2017

OK right. You will have less "if" statements, knowing that privilege framework has been introduced with DSM 6 (yes written in Dev Guide too), so user creation is now done with synouser on DSM 5.

As a result, I have to test:

upgrade from busybox user from DSM 5 to synouser user on DSM 5
upgrade from busybox user from DSM 5 to privilege user on DSM 6
upgrade from synouser user on DSM 5 to privilege user on DSM 6 (maybe for guys still waiting for package availability before upgrading to DSM 6... probably none)

ymartin59 on 2 Dec 2017

From @Safihre

The Service user transition part seems a bit of a problem to me. What if a user doesn't get this intermediate package but goes straight from an old package to the then published newer package?
There is no way to do this in 1 go? From @BenjV comment here I understand that just using the syno commands to remove the old busybox user/group should work, even if it errors?

In fact, as synouser is unable to remove busybox user neither on DSM 5 nor on DSM 6, it is required to use busybox to remove that old service account. @m4tt075 proposal was to keep busybox in intermediate step to invoke userdel... but it means to test an additional context.

My option is "easier" as current context is known (in master) and adding userdel in preupgrade is straight forward. Target context with new generic service script has been tested with demoservice and will be easier to validate for each package migration.

If a user missed intermediate package (what ever it is designed), it seems harmless according to my tests. Synology privilege framework is expected to create xxx_PGKtime additional account (I would say it only happens with synouser account), either reuse busybox account (suprisingly it happened once in my testing) but most likely it fails to update package... so simple action for admin is to uninstall and reinstall, should not be a problem from my point of view.

If you have better ideas, they are welcome but I spent one month around it... so are left these two options requiring a "intermediate" package.

ymartin59 on 3 Dec 2017

After mosquitto, I migrate transmission as it seems me as a most-wanted package. I just hope it does not depends on C++ std libraries...

ymartin59 on 3 Dec 2017

@ymartin59 Yes I see why you choose this way.

If a user missed intermediate package (what ever it is designed), it seems harmless according to my tests. Synology privilege framework is expected to create xxx_PGKtime additional account (I would say it only happens with synouser account), either reuse busybox account (suprisingly it happened once in my testing) but most likely it fails to update package... so simple action for admin is to uninstall and reinstall, should not be a problem from my point of view.

So, to be clear, if we were to skip this intermediate package it will work fine or _always_ requires reinstall of the package? If it works fine/harmless, then why not skip the intermediate package and just leave that old (hidden) busybox user?

For your most-wanted list I would actually include jackett, couchpotato, sabnzbd and nzbget.

Safihre on 3 Dec 2017

I still don't understand the need for an intermediate package, anything that has to be done can be done in the pre-upgrade script of the DSM5+6 package.
To skip removing the old (hidden) busybox user seems a good solution but only if they have another name then the packagename, otherwise DSM will add xxx_PGKtime to the user name and that you don't want.
You need the username to be equal to the package name because the scripts rely on that.

BenjV on 4 Dec 2017

@ymartin59 Sorry, have been offline for some days. You have been doing a phantastic job with this! Many, many thanks for taking the lead!

I will (obviously) take care of updating the TVH package, once you give the green light. Have you decided how you want to proceed in the meantime, i.e. are you going to merge your generic service support into the existing DSM6 branch (which includes the now obsolete @dr-bean changes to the privilege issue) or a new (DSM5+6) branch based off master? Or do you want us to PR against one of the branches in your repo (if so, which one)?

m4tt075 on 4 Dec 2017

@m4tt075 In fact I planed to reuse dsm6 branch to apply changes to all impacted packages... @Dr-Bean
changes are not so obsolete, he mainly added privilege files and useradd/userdel busybox invocation with renaming user account as sc-xxx. I decided to generate all these stuffs. Probably it is time to merge PR now.

ymartin59 on 4 Dec 2017

👍1

@BenjV I have now understood why @Dr-Bean decided to

keep busybox as it is the only tool able to remove allready existing user account created by busybox
to prefix service account with sc-

Here is why:

Synology framework creates service account from privilege before any scripts to be run in new/upgraded package (so situation must be clean BEFORE)
busybox user account cannot be removed with synouser, it requires userdel to be available in package

As a result, there is no option, an intermediate package with busybox is required to clean (at next upgrade) existing service account created by old busybox-based package, before installing a package declaring same account with privilege.

A package containing both privilege and busybox with script to invoke userdel in preinst will not be able to clean busybox user account which it is immediately reused as-is by installer (without creating a proper package service account from privilege) or in some case simply fail with a laconic Not available pop message for which I have no explanation/log yet to understand. And more difficult, without care, script may remove user account that has been created from privilege... and I find it complex to evaluate if user account in passwd comes from Synology privilege framework, busybox useradd or synouser --add

ymartin59 on 4 Dec 2017

❤1

@ymartin59 Since all new users will be sc-, there is no conflict with old users, right? At least that's what I see for the sabnzbd package (current package user sabnzbd, new DSM6 one sc-sabnzbd).
So just leave them? And continue with the new ones, not bothering with intermediate packages?

Safihre on 4 Dec 2017

@Safihre
No new users will not have the sc- prefix, that was a path DrBean chose but is not feasible.
The user must be the same as the package name because that is the only way the installation script will know the username.

@ymartin59
Thanks for the explanation.
I assumed that because the synotools were already available in DSM 5 that they also worked for the users created with busybox.
Presumably I was wrong with this assumption.

How do you propose this intermediate package will work?
As far as I know users cannot choose a package they just get an update that is available in the repository.

How are you planning this intermediate package to work.
Maybe remove the old username with busybox and create a new username with the sc prefix and in the final package use a username without the sc prefix and remove the username with sc prefix with the synotools?

I don't know much about how the repository works but is it possible to push the right package depending upon the version the user has?
Otherwise I don't see how it could work, the package centre on the NAS only sees updates which a version number that is higher then the current version and if so will prompt the user to update.

BenjV on 5 Dec 2017

@BenjV @ymartin59
Here I see that @Dr-Bean implemented a special .dsm6_upgrade state file to mark this event:
https://github.com/SynoCommunity/spksrc/blob/dsm6/spk/sabnzbd/src/installer.sh#L157-L160

The user must be the same as the package name because that is the only way the installation script will know the username.

I think I am misunderstanding, why would it not know the username? It's just the sc- + package_name?

Safihre on 5 Dec 2017

@Safihre
As @ymartin59 explained above, the solotion of DrBean you are referring to, does not work because the new user that is created via the privilege file is already created at the point that this script is becoming active.
So it sees a user with the packagename (created by busybox) and it will append the PKGtime to the username it create and then we don't know that name anymore which is needed.

You are right adding sc prefix will work if they are not already packages with sc prefix deployed.
I don't know if that is the case, but if so we could choose another prefix.

BenjV on 5 Dec 2017

👍1

@BenjV @ymartin59 : Hmh, another thought: Would it make sense to create "cleaning" packages which do nothing but remove the old busybox users and replace them by syno users (should work on DSM5+6). We could then add those "cleaning" packages as dependency for the new DSM5+6 packages? This would force users to install the cleaning package first and allow the privilege method for subsequent installations or upgrades of the actual packages. What do you think?

m4tt075 on 5 Dec 2017

Do you really need busybox to remove the user? Can't you get rid of the user from /etc/passwd and /etc/shadow with a simple sed command?

jelmerj on 5 Dec 2017

@m4tt075
That's a possibility but not very userfriendly, @Safihre's suggestion to just create a new user by adding the sc prefix (or some other prfix) and removing the old busybox user in the post-upgrade script is the preferred solution in my opinion.
An intermediate package will create a lot of confusion.

@jelmerj
You are right, that is also a good solution to not need to include busybox.

BenjV on 6 Dec 2017

❤1

OK. So here is a proposal for next packages content:

conf/privilege file with sc- prefix user account for DSM 6
synouser commands to create svc- prefix for service user account on DSM 5 and previous
busybox to clean "legacy" user account without prefix in service_preinst package specific function

After a while, we will decide to remove busybox definitely.

As such, dsm6 content may be considered as "approved" and merged to master. Then we should review each package from list, migrate to generic service support, package, test and publish (if repository agrees to accept these packages). As a reference, I propose to keep a pre-dsm6 branch with current latest master commit.

ymartin59 on 6 Dec 2017

👍1

Perfect 👍! That sounds user-friendly.

Indeed the different branches are confusing, since they deviate substantially for a number of files.
Unfortunately with some major packages like spk/python not working its a bit challenging to just merge. Not sure what is the best to do?

Do a merge and then cleanup package-by-package?
'Freeze' master and fix it first within dsm6 branch?

EDIT: I see now this last option is what @ymartin59 proposing 👍

Safihre on 6 Dec 2017

@ymartin59 Yep! Makes a lot of sense! Happy to do some monkey work as soon as the generic service scripts and demoservice are adapted to this strategy and all branches are in place. 👍
Do you think the spkrepo side of things is going to work out?

m4tt075 on 6 Dec 2017

Seems to me there is a plan.

BenjV on 7 Dec 2017

dsm6 branch has been merged to master. Here is first package mosquitto to be upgraded and migrated according to proposal: #3049
I will go on with sabnzbd and nzbget...

ymartin59 on 16 Dec 2017

👍4 🎉2 ❤1

@ymartin59 It might make sense to close this issue now, and create a new one using the same table (but empty) as starting point to track conversion of packages to your new service method. Do you want me to do that or keep this issue open and amend the text and the table on the top?

Otherwise, working on TVH in the meantime...

m4tt075 on 17 Dec 2017

@ymartin59 Just realized that you edited the table above anyways, so let's keep tracking here.

m4tt075 on 18 Dec 2017

@ymartin59 @m4tt075 Finally finished the academic part and have setup a XPenology system.
Wanted to start fixing sabnzbd and spk/python, did you already start on it maybe?

Safihre on 19 Dec 2017

@Safihre Nope, I didn't, but @ymartin59 wanted to work on sabnzbd (see his comment above). This is why he also appears as reviewer in the above table. I have just seen your PR. If I were you I would just wait for @ymartin59 to comment. Maybe he has done that package already. If not and he wants you to continue he will certainly let you know as well. But if you could work on python, that would be great. To my knowledge nobody is working on that yet. I will update the table above to mark it as WIP and put you as reviewer in the comment column, ok?

m4tt075 on 19 Dec 2017

I wanted to use sabnzbd as a test-case for the python package. Since python itself is not really anything without a package using it. All other packages depending on python still need the Generic-Service makeover too.

Safihre on 19 Dec 2017

I am not sure about Sabnzbd, but lots of packages that needs Python also needs Git so I would suggest that the Git package also get high on the to-do list.

BenjV on 20 Dec 2017

@BenjV The git package already works when just compiled with the DMS6 toolchains: https://github.com/Safihre/spksrc/releases/tag/git-2-dsm61
Just tested 👍
But could still be converted to Generic Service approach to remove busybox dependency, would that be desired?

Safihre on 20 Dec 2017

In my opinion it would be good to convert everything to an universal approach.
Also because then the username is created by the Generic service approach with that standard naming and you never know if that will be usefull in the future.

BenjV on 20 Dec 2017

👍2

@ymartin59 might be a good idea to write a short introduction/how to for contributors so they can easily adjust their packages or help migrate them by creating PR's

cytec on 21 Dec 2017

@cytec He has started already: See Wiki on Package Migration. But the devil is in the detail, as you can see from my attempts in PR #3050. Personally, I would also prioritize updating the packages which require a user over those which don't. The former are not DSM 5+6 compatible, the latter are. Would take it slowly, step by step. I thought spksrc was dead. @ymartin59 has just resurrected it... ;-)

m4tt075 on 21 Dec 2017

Thanks but I am quite exhausted about it... So here are bugs left in current support:

Service user created for DSM 5 only works if "home account" support is enabled in DSM. Fix: change home folder in /etc/passwd with a sed command (no energy to search for better)
Generated service-setup should provide EFF_USER (at the moment in installer.sh) so that it is available for start-stop-script
Add support and document how to run start command as root from start-stop-script, in addition to generic service user creation by installer

ymartin59 on 21 Dec 2017

Sorry but I have no idea why I could deliver these fixes. The only "example" of migration is "mosquitto" PR #3049 at the moment and I selected it because it was quite simple and straight forward.

ymartin59 on 21 Dec 2017

@ymartin59 Not surprised! But as to your "bugs", you should not worry:

The number of DSM 5 users without "home account" should really be limited. A comment in the Wiki that "home accounts" might have to be activated is probably all that's needed.
EFF_USER can be reconstructed in the su executed scripts as PR #3050 demonstrates.
That's the only one, I'm not sure about. I know that for TVH we probably won't need it anymore. For other packages, no idea. But worst case, we will have to look for non-generic solutions.

We are in a way, way better spot than a couple of weeks ago. Many thanks again and merry Christmas!

m4tt075 on 21 Dec 2017

@ymartin59 I took another look into the DSM6 developers guide. If I'm not mistaken, we can execute start commands as root by "outsourcing" them to some [package-name].sh file (like you did for demoservice) and adding that with "run-as": "root" to an "executable" section in the privilege file... :smile:

m4tt075 on 23 Dec 2017

That is right, but I documented something shorter (create user but run all scripts as root) in Package-Migration.md

ymartin59 on 24 Dec 2017

👍1

@m4tt075 I have just pushed an additional commit where I have replace demoservice.sh by service_prestart function in service-setup script (leading to another trouble to work-around because tar package is now empty ! by the way only a demo). See https://github.com/SynoCommunity/spksrc/pull/3049/commits/e47af44ecffcd4780ab64218179324860a24e769

ymartin59 on 24 Dec 2017

@Safihre @BenjV In fact git and python package may benefit of generic installer, but without need of a service account, group or share, and with STARTABLE=no, there should be almost nothing left in spk/ folder

ymartin59 on 24 Dec 2017

Fixed a bug in the general service support to preserve package settings upon update, it's in my PR for SABnzbd.
In case anyone is spending Christmas free time like me, making Syno packages 🤣
https://github.com/SynoCommunity/spksrc/pull/3053/commits/c797f1276bf7f6b21b899d8cafb02eafec4d5337

Safihre on 24 Dec 2017

There is one more permissions issue:
How to correct ownership of SHARE_PATH/SERVICE_WIZARD_SHARE after upgrade from old-style package?
I notice with SABnzbd that when upgrading from old-style package it has the wrong permissions since set_syno_permissions is never called.

Safihre on 24 Dec 2017

After thinking about it over Christmas dinners, it seems really there is no way to know upon upgrade of old-style package what the user's shared directory and group were.
In the case of SABnzbd I had to extract from the config.ini file what the paths were and then use these functions to correct ownership.
Additionally in case of upgrade from old-style package the sc-download group might not exist, and as such that also had to be created and added. So I had to edit spksrc.service.installer functions to be more flexible and then just checked if the folders had any kind of group associated with it.
@ymartin59 Maybe it's good to create general function for this, like my correct_upgrade_permissions that every package can then use on the folders that need correct permissions enforced?
See:
https://github.com/SynoCommunity/spksrc/pull/3053/commits/a7ac13fc4f878343743dffb671a94bb4297e483a#diff-5d6bd92e6f38c326c2344d53c1dfb20a

Safihre on 25 Dec 2017

@Safihre I would not recommend such a complex option. Here are two options

Generic service expects to get group and share folder from wizard variables wizard_group and wizard_download_dir (see beginning of generated work*/script/service-setup. As a first proposal you can add a "help tip" in wizard text description so that user can grab current value from its file on system
Other option is to replace GROUP and SHARE_PATH in service-setup to provide INST_VARIABLES file normally created in save_wizard_variables
These are proposals... as I am convinced current installer script should allow you to provide this additional comfort to users

ymartin59 on 25 Dec 2017

This is specific to the upgrade path, not install.
If a user has set a path different from /volume1/downloads/, they will have to edit this value in the upgrade wizard _every_ single update.. If they forget it once, the process will just create new folder even though they don't use it.
Same if they (for some reason) selected a different group than sc-download, it messes up permissions.

_Maybe I should explain better:_
The old SABnzbd (and NZBGet, etc.) packages would only let you select an already existing folder, so users had to manually create one using DSM interface. This allows lots of freedom for users to have chosen something else than /volume1/downloads/ back then.
Since the wizard UI-files are fixed to the default of /volume1/downloads, users would have to change it on every update from now on.
Not very user-friendly I think, then it's almost weird to even ask them for a folder-option since we basically enforce /volume1/downloads/.
That would also be the case for new users of this package, on the next update they would have to remember again to correctly set the directory.
Are you also sure INST_VARIABLES doesn't get removed in between the upgrade steps? Since it's in the /var/pkg/ folder? (didn't test yet, just from seeing the upgrade process)

Safihre on 25 Dec 2017

As far as I remember, wizard is only shown at first install, not during upgrade. By the way, you can enforce value you expect at beginning of src/service-setup.sh file. To read previous config.ini you probably have to grab it from its upgrade saved location.

ymartin59 on 26 Dec 2017

There are also wizard files who can be used during upgrade and uninstall.
See this part of the manual:

_These files can be regarded as user settings or used to control the flow of script execution.
install_uifile: Describes UI components for the installation process. During the process of the preinst and postinst scripts, these
component keys and values can be found in the environment variables.
upgrade_uifile: Describes UI components for the upgrade process. During the process of the preupgrade, postupgrade, preuninst, postuninst, preinst and postinst scripts, these component keys and values can be found in the environment variables.
uninstall_uifile: Describes UI components for the un-installation process. During the process of the preuninst and postuninst
scripts, these component keys and values can be found in the environment variables_

BenjV on 26 Dec 2017

Alright, I could extract the folder name from the config file. The docs also describe that wizard files can be generated dynamically.
But what about the user-group? (when upgrading)
How do I extract if it was already set and what the name is?

Safihre on 26 Dec 2017

Just add the group with the add group command.
It will give a message if it already exists.
An old group that is left behind will not do any harm.

Most likely you already know this but the format of all those service tools commands kan be found here:
https://www.synology.com/en-global/support/developer#synogroup

BenjV on 26 Dec 2017

Then I suggest we do not even offer the option to the user to choose a group, but that we always use sc-download?
For _all_ download related packages.

Safihre on 26 Dec 2017

I fully agree with that.

BenjV on 26 Dec 2017

👍1

I have no objection to enforce group name, as it was before - just start src/service-setup.sh script with GROUP=sc-download ... but keep wizard step explaining group permission management to inform users.

ymartin59 on 26 Dec 2017

Ok, done 💯
Still had to include minor SABnzbd specific code to fix permissions of the actual incomplete and complete folders used by SABnzbd.
Could maybe be an idea to include a function like this in the general service support? Can imagine that more packages need to force permissions on existing folders.

Safihre on 26 Dec 2017

Why not, I expect set_syno_permissions can be extended with group and path parameters

ymartin59 on 27 Dec 2017

I was going to convert sonarr/sickrage/etc packages today but noticed that they are set to GROUP=sc-media'.
Why are they sc-media? Wouldn't it be much more useful if they were also sc-download so they can work in harmony with the download packages without any user intervention?

Safihre on 11 Jan 2018

@Safihre PRed quicker than I could update our tracking table... :smirk:
Just updated again. About time...

m4tt075 on 12 Jan 2018

About start-stop-daemon, I was not aware most services have no "daemon" option. So I will extend generic support to start SERVICE_COMMAND as background process with PID_FILE generation included.

ymartin59 on 14 Jan 2018

@ymartin59, I don't think this is sufficient as it doesn't cover the case when the process restarts itself and the PID changes (resulting in the Stopped status in DSM).
The start-stop-deamon allows to locate a process based on executable name and user, without PID. This is how it's used now in some of these packages like Syncthing that really don't generate a PID. Is it really that terrible to use busybox for this? These are compiled packages anyway, and we include it also with the general Python package.

Safihre on 14 Jan 2018

OK I propose to create a third generic start-stop-status script to support start-stop-daemon usage

ymartin59 on 14 Jan 2018

+1

m4tt075 on 14 Jan 2018

Welcome back, by the way!
Do you see any light at the end of the spkrepo tunnel in the meantime?

m4tt075 on 14 Jan 2018

Yes, I have identified 502 failure when uploading x64 packages... the architecture list was so long that package filename in storage gets out of postgresql column length. One alter table later, I was able to upload mosquitto x64 package.
For DSM 6.x, firmware version is rejected because of a too restricted regular expression: https://github.com/Diaoul/spkrepo/blob/master/spkrepo/views/api.py#L23
Now firmware version has 5 digits... Should not be long to fix, just have to find proper way to restart uwsgi.

ymartin59 on 14 Jan 2018

@ymartin59 : 👍 Happy New Year, Yves!

m4tt075 on 14 Jan 2018

@ymartin59 Great news 🎉👍👍

Safihre on 15 Jan 2018

@Safihre I am currently publishing pending packages from master...
About group naming, I wonder if it may be possible to transition with:

wizard update informing about group name to use
service_postinst function testing if previous group name exists, and if so adds EFF_USER as member

I do not agree with you - it is never too late... and sooner any is fixed, better it is.

ymartin59 on 15 Jan 2018

@ymartin59 That would be great and also what I was thinking, I was working a bit before on this for Sonarr/Radarr but abandoned it when I saw that so many packages had sc-media. I didn't think you would like to make such changes 😉

I imagined:

If sc-media exists and USER is part of it, make EFF_USER also part of sc-media
Make this code that adds new users to existing group a function syno_add_to_group
Or: something a function syno_add_backup_group_membership, that does the check and adds directlt if necessary?
Simply update the wizard texts on the "DSM6 Permissions page" of all packages involved from sc-media -> sc-download.

Safihre on 15 Jan 2018

What's the progress on getting ejabberd functional?

Heptite on 24 Feb 2018

@Heptite Not aware of any progress on this particular package. Maybe you are lucky and somebody picks it up, but we still have some packages in testing which need to be published first (see WIP in table above). We are getting there, it just takes time...

m4tt075 on 24 Feb 2018

@m4tt075 Is there any temporary workaround to getting it working?

Also, I'd like to point out that there's a small bounty (which I added to) on that particular package.

Heptite on 24 Feb 2018

@Heptite Nope, the package needs proper adaptation. Seems it hasn't been updated for quite some time. The bounty might raise people's interest though. Good luck.

m4tt075 on 24 Feb 2018

@ymartin59 @Safihre I know that you have been keeping track with the progress, also on the list above. Since most of the packages we had converted have been merged in the meantime, I was wondering whether I should continue with list. Having said that, my feeling is that there is not a lot of demand for the packages that are missing and that it might not be worthwhile to just keep them for the sake of keeping them. On the contrary, there might be a benefit in cleaning up and focusing more on maintainance of those packages we provide. Can you judge (maybe through downloads from spkrepo), which packages should be continued and which ones retired?

m4tt075 on 6 Jun 2018

Asked @Diaoul if we can somehow figure this out 👍 Indeed we should convert now the most popular packages.

Safihre on 24 Jun 2018

👍1

Gateone is WIP: #3431
Table updated...

m4tt075 on 26 Aug 2018

Is synocli supported on dsm6?

markboston34 on 1 Jul 2019

Spksrc: Meta: DSM6 compatibility of packages with user privilege issues

Most helpful comment

All 153 comments

Related issues