Submit AYON_SERVER_URL to jobs #42

BigRoy · 2024-10-07T17:27:27Z

Changelog Description

Submit AYON_SERVER_URL with the Deadline Job.

To avoid submitting along the user's API key or exposing the service user API key in AYON settings we only submit along the AYON_SERVER_URL however we now have an extra setting on the Deadline repository for the AYON Server URL submitted with the jobs to supply their API keys.

Additional info

Fixes #27

⚠️ This requires updating the GlobalJobPreLoad and the AYON plug-in on Deadline Repository

Documentation with use case description ynput/ayon-documentation#285

Testing notes:

Enable the plug-in in settings: ayon+settings://deadline/publish/CollectAYONServerToFarmJob/enabled

Configure additional server URL if you have a custom server URL (other than defined in defaults in AYON settings)
Submitting deadline jobs will submit along AYON_SERVER_URL if plug-in is enabled.
The GlobalJobPreLoad plug-in will resolve the API key for a custom server URL submitted along as defined in its plugin settings.

BigRoy · 2024-10-07T17:29:09Z

Tagging @tweak-wtf who might be interested in this.

kalisp · 2024-10-08T10:52:09Z

client/ayon_deadline/plugins/publish/global/collect_deadline_job_env_vars.py

+
+
+class CollectAYONServerUrlToFarmJob(CollectDeadlineJobEnvVars):
+    label = "Submit AYON server URL to farm job"
+
+    # TODO: Expose the enabled/optional state to settings
+    ENV_KEYS = [
+        "AYON_SERVER_URL",
+        "AYON_USE_DEV",
+    ]


Do we want send AYON_SERVER_URL always? If thats the case, server url in AYON Deadline plugin doesn't make sense, neither code for it in GlobalJobPreload.

(Just note, API keys for all DL servers would need to be same as they would be still set via AYON DL plugin.)

kalisp · 2024-10-08T11:05:12Z

Env var is being passed, but Settings for that collector should be added to make it optional.

… enhancement/submit_ayon_server_url # Conflicts: # client/ayon_deadline/plugins/publish/global/collect_deadline_job_env_vars.py

…om/BigRoy/ayon-deadline into enhancement/submit_ayon_server_url

…rmJob`)

kalisp · 2024-10-09T10:59:37Z

Is Submit AYON server URL to farm job proper label as it seems it could push AYON_API_KEY also. I am guessing it we need to push different AYON_API_KEY value, it should be set in global environment variables in ayon+settings://core/environments?

(I was thinking about usage during writing small documentation ynput/ayon-documentation#285 )

BigRoy · 2024-10-09T17:17:36Z

Is Submit AYON server URL to farm job proper label as it seems it could push AYON_API_KEY also. I am guessing it we need to push different AYON_API_KEY value, it should be set in global environment variables in ayon+settings://core/environments?

Agree. What if we generalize this further, and make the settings like a dict where one can specify any amount of keys? The default settings could have AYON_API_KEY and AYON_SERVER_URL defined, but the plugin just disabled. That way it is as easy as just enabling it by default?

However, it wouldn't really clarify why you'd want to pass that.

Ideas @antirotor @m-u-r-p-h-y @iLLiCiTiT - better naming? different approach?
This documentation PR ynput/ayon-documentation#285 explains what this PR is trying to do.

iLLiCiTiT · 2024-10-10T09:28:30Z

server/settings/publish_plugins.py

@@ -380,6 +390,11 @@ class PublishPluginsModel(BaseSettingsModel):
        "primary_pool": "",
        "secondary_pool": ""
    },
+    "CollectAYONServerUrlToFarmJob": {


Wouldn't be better to have settings for CollectDeadlineJobEnvVars with enum Pass AYON server url to job and options Always, User choice, Never instead of new plugin?

I mean, we have publish plugin attribute definitions, but working with them as we would still use pyblish pype...

client/ayon_deadline/plugins/publish/global/collect_deadline_job_env_vars.py

kalisp

Tested, worked.

My objection is to Submit AYON server URL to farm job, but otherwise fine.

…hrough the other env vars like the chosen bundle and default variant settings

…om/BigRoy/ayon-deadline into enhancement/submit_ayon_server_url # Conflicts: # client/ayon_deadline/plugins/publish/global/collect_deadline_job_env_vars.py

BigRoy · 2024-10-10T21:28:16Z

@kalisp @iLLiCiTiT can you check the code and functionality again?

With API key set:

client/ayon_deadline/plugins/publish/global/collect_deadline_job_env_vars.py

MustafaJafar

Sorry, I didn't go through all of the comments.
Anyways, This PR works and AYON_API_KEY is not propagated to the publish job when the collector is enabled.

…-in settings. This way we're not exposing the API key in the AYON settings that any admin is capable of seeing but leave it up to the Deadline admin to configure.

BigRoy · 2024-10-19T13:21:44Z

@kalisp @iLLiCiTiT I've implemented the implementation that @iLLiCiTiT proposed in the call we had about this PR. Now the AYON_API_KEY is to be configured is not configured in the AYON Settings on the server nor is it submitted along with the job. Instead only the server URL is submitted along but the API key is then resolved on the GlobalJobPreLoad by having server URLs configured on the deadline repository AYON plug-in settings. The PR description is also updated.

I personally like it less. It feels overly complicated. And only after I implemented this I figured this would also have problems if say two developers on one network are connecting to their servers using e.g. localhost as server URL, submit to the farm (to run only on their machine, because it's their dev job for example) then it still couldn't resolve the relevant API key unless both users use the same service user API key. Again, an edge case. My biggest concern is the 'somewhat more' complexity involved.

Anyway, please review and comment so we can decide to continue this way, or revert to the previous logic of this PR or do it even differently.

@tweak-wtf feel free to let you know your thoughts too.

tweak-wtf · 2024-10-21T09:38:06Z

if i understood correctly we'd now define additional AYON servers and their corresponding API keys in the AYON Deadline Plugin with this syntax <AYON_URL>:<AYON_PORT>@<AYON_API_KEY> alongside the default AYON Credentials settings resembling the main AYON server.
so we're giving the API key as a string to the Deadline Plugin as opposed to passing them around in dictionaries from a publish process. wouldn't that also expose the API KEy which we're trying to prevent?

However, this sounds fine to me really. I understand the security concerns that passing API keys in cleartext is badbadnotgood.

just a question... would it be possible/feasible to e.g. reference a secret name present in the main ayon instance instead for the actual API key? so something like dev.awesome.ayon.app@ayon-instance-dev-key-1?
imho this could make it even more secure since the keys are stored as secrets on the main instance and would only be retrieved from GlobalJobPreLoad if a job was submitted from one of the defined extra servers.

btw. from my side this is not urgent at all. i just think it'd be a great addition for devs to further isolate a prod env from a sandbox and could ease some pain in transitional phases from one ayon instance to another

BigRoy · 2024-10-21T09:45:29Z

so we're giving the API key as a string to the Deadline Plugin as opposed to passing them around in dictionaries from a publish process. wouldn't that also expose the API KEy which we're trying to prevent?

It does - but only to what's supposedly the admin/super-user on Deadline.

would it be possible/feasible to e.g. reference a secret name present in the main ayon instance instead for the actual API key?

I'd say no - because you'd first need to connect to the instance ;) using... an API key.

tweak-wtf · 2024-10-21T10:29:57Z

got ya. then it really feels fine to me.

client/ayon_deadline/repository/custom/plugins/GlobalJobPreLoad.py

…d.py Co-authored-by: Jakub Trllo <[email protected]>

BigRoy · 2024-10-23T10:20:36Z

@iLLiCiTiT should this also get bump minor label?

iLLiCiTiT · 2024-10-23T10:30:07Z

@iLLiCiTiT should this also get bump minor label?

Yes please.

MustafaJafar

This is cool, it works on my side.
I love the new idea of having Multiline AYON servers field.
I was too lazy to run an additional AYON instance so I've just removed the AYON server URL and used the new field solely. which makes me question the existence of the old setting. But anyways, this works!.

kalisp · 2024-10-29T14:58:50Z

I added additional casting to list as I was suffering with:

Submit along AYON_SERVER_URL with deadline jobs

c5e67f3

BigRoy added the type: enhancement Improvement of existing functionality or minor addition label Oct 7, 2024

BigRoy requested review from kalisp and iLLiCiTiT October 7, 2024 17:27

BigRoy self-assigned this Oct 7, 2024

BigRoy marked this pull request as ready for review October 7, 2024 17:29

Merge branch 'develop' into enhancement/submit_ayon_server_url

e0009e8

kalisp reviewed Oct 8, 2024

View reviewed changes

BigRoy and others added 8 commits October 8, 2024 14:14

Merge branch 'develop' of https://github.com/ynput/ayon-deadline into…

d718648

… enhancement/submit_ayon_server_url # Conflicts: # client/ayon_deadline/plugins/publish/global/collect_deadline_job_env_vars.py

Remove incorrect file docstring

cffab6b

Submit along AYON_API_KEY

4ce6250

Merge branch 'enhancement/submit_ayon_server_url' of https://github.c…

b17af4b

…om/BigRoy/ayon-deadline into enhancement/submit_ayon_server_url

Expose plug-in in settings

5737d4a

Submit optional state (mostly for inherited `CollectAYONServerUrlToFa…

d1cf912

…rmJob`)

Fix label for optional field

4a57ffd

Removed TODO as already implemented

86b4900

kalisp mentioned this pull request Oct 9, 2024

Added documentation for passing AYON server url to Deadline ynput/ayon-documentation#285

Merged

iLLiCiTiT reviewed Oct 10, 2024

View reviewed changes

client/ayon_deadline/plugins/publish/global/collect_deadline_job_env_vars.py Outdated Show resolved Hide resolved

iLLiCiTiT reviewed Oct 10, 2024

View reviewed changes

client/ayon_deadline/plugins/publish/global/collect_deadline_job_env_vars.py Outdated Show resolved Hide resolved

kalisp approved these changes Oct 10, 2024

View reviewed changes

BigRoy added 4 commits October 10, 2024 22:29

The applying of AYON_USE_DEV should already be solved on the farm t…

65d82a1

…hrough the other env vars like the chosen bundle and default variant settings

Set AYON API key from settings

08fa08f

Merge branch 'enhancement/submit_ayon_server_url' of https://github.c…

4b21b44

…om/BigRoy/ayon-deadline into enhancement/submit_ayon_server_url # Conflicts: # client/ayon_deadline/plugins/publish/global/collect_deadline_job_env_vars.py

Cosmetics

47d8512

MustafaJafar reviewed Oct 16, 2024

View reviewed changes

client/ayon_deadline/plugins/publish/global/collect_deadline_job_env_vars.py Outdated Show resolved Hide resolved

MustafaJafar reviewed Oct 16, 2024

View reviewed changes

BigRoy added 2 commits October 19, 2024 14:24

Merge branch 'develop' into enhancement/submit_ayon_server_url

741209c

Move the passing of the AYON_API_KEY to Deadline Repository AYON Plug…

9c6ab87

…-in settings. This way we're not exposing the API key in the AYON settings that any admin is capable of seeing but leave it up to the Deadline admin to configure.

BigRoy marked this pull request as ready for review October 19, 2024 13:02

BigRoy requested a review from iLLiCiTiT October 19, 2024 13:02

BigRoy added 3 commits October 19, 2024 15:03

Remove plug-in that was moved into other file

bc5c8d2

Cosmetics

02982d5

Remove non-existing default

7816fce

iLLiCiTiT reviewed Oct 22, 2024

View reviewed changes

client/ayon_deadline/repository/custom/plugins/GlobalJobPreLoad.py Outdated Show resolved Hide resolved

Update client/ayon_deadline/repository/custom/plugins/GlobalJobPreLoa…

91e6f92

…d.py Co-authored-by: Jakub Trllo <[email protected]>

Merge branch 'develop' into enhancement/submit_ayon_server_url

a10e7a3

BigRoy added the bump minor label Oct 23, 2024

Merge branch 'develop' into enhancement/submit_ayon_server_url

4bd2db0

MustafaJafar requested review from iLLiCiTiT, tweak-wtf and MustafaJafar October 23, 2024 15:49

MustafaJafar approved these changes Oct 23, 2024

View reviewed changes

iLLiCiTiT approved these changes Oct 29, 2024

View reviewed changes

Added casting to list

8642e66

kalisp approved these changes Oct 29, 2024

View reviewed changes

Merge branch 'develop' into enhancement/submit_ayon_server_url

cedf5d6

kalisp merged commit a04203a into ynput:develop Oct 30, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Submit AYON_SERVER_URL to jobs #42

Submit AYON_SERVER_URL to jobs #42

BigRoy commented Oct 7, 2024 •

edited

Loading

BigRoy commented Oct 7, 2024

kalisp Oct 8, 2024

kalisp commented Oct 8, 2024

kalisp commented Oct 9, 2024

BigRoy commented Oct 9, 2024 •

edited

Loading

iLLiCiTiT Oct 10, 2024 •

edited

Loading

kalisp left a comment

BigRoy commented Oct 10, 2024

MustafaJafar left a comment

BigRoy commented Oct 19, 2024

tweak-wtf commented Oct 21, 2024

BigRoy commented Oct 21, 2024

tweak-wtf commented Oct 21, 2024

BigRoy commented Oct 23, 2024

iLLiCiTiT commented Oct 23, 2024

MustafaJafar left a comment

kalisp commented Oct 29, 2024

Submit AYON_SERVER_URL to jobs #42

Submit AYON_SERVER_URL to jobs #42

Conversation

BigRoy commented Oct 7, 2024 • edited Loading

Changelog Description

Additional info

Testing notes:

BigRoy commented Oct 7, 2024

kalisp Oct 8, 2024

Choose a reason for hiding this comment

kalisp commented Oct 8, 2024

kalisp commented Oct 9, 2024

BigRoy commented Oct 9, 2024 • edited Loading

iLLiCiTiT Oct 10, 2024 • edited Loading

Choose a reason for hiding this comment

kalisp left a comment

Choose a reason for hiding this comment

BigRoy commented Oct 10, 2024

MustafaJafar left a comment

Choose a reason for hiding this comment

BigRoy commented Oct 19, 2024

tweak-wtf commented Oct 21, 2024

BigRoy commented Oct 21, 2024

tweak-wtf commented Oct 21, 2024

BigRoy commented Oct 23, 2024

iLLiCiTiT commented Oct 23, 2024

MustafaJafar left a comment

Choose a reason for hiding this comment

kalisp commented Oct 29, 2024

BigRoy commented Oct 7, 2024 •

edited

Loading

BigRoy commented Oct 9, 2024 •

edited

Loading

iLLiCiTiT Oct 10, 2024 •

edited

Loading