Support more scenarios #1

weshouman · 2020-04-06T00:53:25Z

Support the following

Scenarios where multi strings are templated.
Use of array of elements alongside with the default values.
logout method.
Many more methods.

Ansible already executes the scripts from inside the host machines, which will always make the localhost to be the synology machine. Using the 127.0.0.1 instead of any supplied input saves from mistakenly using an unresolvable uri by synology thus avoid sending the GET request with the username and password in the URI to a foreign nslookup Also enable usage of a customized synology port through synology_dsm_port

…cenario.

… strings.

Add logout, create user, delete user and many more webapi tasks. Structure the defaults into directories.

agaffney · 2020-04-06T23:38:23Z

The main feedback that I have on this is that there's no way to disable the application of a bunch of this new configuration. For example, for the mail settings, there's no options other than disabling mail or actually configuring everything. I don't really have a good suggestion for how to implement the third option of leaving it alone, though.

weshouman · 2020-04-07T04:06:06Z

Keeping some settings untouched shall be done with a check for each setting whether it's configured for the task or not.
This could be implemented by eliminating the use of multi-strings for the compound settings and (of course we know each option whether optional or not, ie: have a clear API)

Note: some options could be tricky, ie: password and __CiPhErTeXt, one and only of them is required for the user creation.

rlenferink · 2020-06-10T20:34:39Z

tasks/core/user/create.yml

+      deny:     false
+      custom:   false
+    permissions:
+      - share_dir: "shared_dir1"


Is this correct? Shouldn’t this be a list of shares which the user should have access to?

The 3 mentioned shares seem like boilerplate shares.

rlenferink · 2020-06-10T20:36:33Z

tasks/core/user/create.yml

+    quotas_defaults:
+      quota: 0
+    quotas:
+      - share_dir: "shared_dir1"


Boilerplate ?

rlenferink · 2020-06-10T20:38:21Z

tasks/core/user/create.yml

+    user_name: "{{ synology_dsm_new_user_name }}"
+    group_name: "{{ item }}"
+  with_list:
+    - group1


Boilerplate? Shouldn’t this be configurable?

rlenferink · 2020-06-10T20:39:51Z

defaults/main/user_delete.yml

@@ -0,0 +1,4 @@
+---
+synology_dsm_users_to_delete:
+  - "to_be_deleted_user1"


Preferably default this to an empty list:

synology_dsm_users_to_delete: []

rlenferink · 2020-06-10T20:46:57Z

defaults/main/user_create.yml

@@ -0,0 +1,9 @@
+---
+synology_dsm_new_user_name:             automated_user1


Why not have this as e.g. a dict?

synology_dsm_users: automated_user1: description: Automatically created test user email: "" cnt_chg_pw: false expired: normal notify_by_email: false send_pw: false pass: dXNlX2Ffc3Ryb25nX3Bhc3N3b3JkX2xpa2VfdGhpcw== automated_user2: # etc

Multiple users can be specified this way.

rlenferink · 2020-06-10T20:50:10Z

tasks/core/user/_create.yml

+            "password":         "{{ synology_dsm_new_user_pass | b64decode }}"
+          }
+        ]
+


If changing this to a list add:

loop: “{{ (synology_dsm_users | default({})) | dict2items }}”

rlenferink · 2020-06-10T20:53:00Z

tasks/core/user/_create.yml

+            "api":              "SYNO.Core.User",
+            "method":           "create",
+            "version":          "1",
+            "name":             "{{ synology_dsm_new_user_name }}",


With the loop added update this section:

"name": "{{ item.key }}", "description": "{{ item.value.description }}",

rlenferink · 2020-06-10T21:07:26Z

tasks/core/user/_set.yml

+            "email":            "{{ user_email }}",
+            "expired":          "{{ user_expiration }}",
+            "cannot_chg_passwd": {{ user_cnt_chg_pw | to_json }},
+            "password":         "{{ user_pass | b64decode }}"


Why b64decode passwords? If it’s just to obscure the password I suggest to use Ansible Vault for storing the password in a variable (will be decrypted when running Ansible). Base64 can be decoded by anyone.

Info: https://docs.ansible.com/ansible/latest/user_guide/vault.html#use-encrypt-string-to-create-encrypted-variables-to-embed-in-yaml

rlenferink · 2020-06-10T21:08:41Z

defaults/main/login.yml

+synology_dsm_login_pass: dXNlX2Ffc3Ryb25nX3Bhc3N3b3JkX2xpa2VfdGhpcw==
+
+# NOTE: for passwords, use a base64 hash, in example:
+# ```echo -n use_a_strong_password_like_this | base64```


Same note as somewhere else. Why base64? That isn’t secure when checking into version control. Better to use Ansible Vault: https://docs.ansible.com/ansible/latest/user_guide/vault.html#use-encrypt-string-to-create-encrypted-variables-to-embed-in-yaml

If the action_plugin is the only place which needs it in base64 isn’t it possible to encode it in the python file?

rlenferink · 2020-06-10T21:16:36Z

Are you planning on completing the task/vpnserver stuff? Otherwise it might be better to remove the empty files.

FYI: I don’t need it as I have my vpn configured on pfSense.

rlenferink · 2020-06-10T21:23:11Z

The main feedback that I have on this is that there's no way to disable the application of a bunch of this new configuration. For example, for the mail settings, there's no options other than disabling mail or actually configuring everything. I don't really have a good suggestion for how to implement the third option of leaving it alone, though.

The suggestion I have is to put sensible defaults in place. The default should be the same as after a fresh installation.

weshouman added 6 commits March 9, 2020 01:09

Cleanup, add comments and fix dict modificaion-while-iterating

4e44c90

Allow use of templated multi-strings, for the compound element.

3d4f509

Support detection of deeper failures, ie: occuring in user creation s…

e28debe

…cenario.

Add filters to be used to for fine-grained array-constructions within…

67b5079

… strings.

Major restructure for the tasks

3a6d4dc

Add logout, create user, delete user and many more webapi tasks. Structure the defaults into directories.

rlenferink reviewed Jun 10, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support more scenarios #1

Support more scenarios #1

weshouman commented Apr 6, 2020

agaffney commented Apr 6, 2020

weshouman commented Apr 7, 2020

rlenferink Jun 10, 2020

rlenferink Jun 10, 2020

rlenferink Jun 10, 2020

rlenferink Jun 10, 2020

rlenferink Jun 10, 2020

rlenferink Jun 10, 2020 •

edited

Loading

rlenferink Jun 10, 2020

rlenferink Jun 10, 2020

rlenferink Jun 10, 2020

rlenferink commented Jun 10, 2020

rlenferink commented Jun 10, 2020

		@@ -0,0 +1,9 @@
		---
		synology_dsm_new_user_name: automated_user1

Support more scenarios #1

Are you sure you want to change the base?

Support more scenarios #1

Conversation

weshouman commented Apr 6, 2020

Support the following

agaffney commented Apr 6, 2020

weshouman commented Apr 7, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rlenferink Jun 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rlenferink commented Jun 10, 2020

rlenferink commented Jun 10, 2020

rlenferink Jun 10, 2020 •

edited

Loading