theforeman / foreman_hooks

Run custom hook scripts on Foreman events
http://m0dlx.com/blog/Extending_Foreman_quickly_with_hook_scripts.html
GNU General Public License v3.0
56 stars 50 forks source link

Empty 'parameters' array in json output #48

Open wiad opened 7 years ago

wiad commented 7 years ago

Related to #43, which partly solved the problem but is now closed.

The json output passed to my hook scripts are missing data - the 'parameters' array is empty. If I apply the workaround mentioned in the issue #43 the parameters array is populated as expected though.

markt-uom commented 7 years ago

Looks like you can also just drop the ".authorized" off the end and it works.

{ :parameters => partial("api/v2/parameters/index", :object => host.host_parameters) }

wiad commented 6 years ago

Updated to 1.16 and ran into this again, and can confirm that what @markt-uom says is true - dropping the '.authorized' makes all my parameters appear in the json output passed to foreman-hooks

wiad commented 6 years ago

Still need to do this workaround on Foreman 1.17

wiad commented 6 years ago

Still need to do this workaround on Foreman 1.18

achevalet commented 6 years ago

Still valid on Foreman 1.19. The workaround works only for create (without priority, id .. and not showing in all_parameters though). It does not work for destroy.

lzap commented 6 years ago

@achevalet can you tell me more about the request you are doing? What effective user is the call which is causing the hook to trigger? Is that a super admin or regular admin? Can you try this with super admin?

achevalet commented 6 years ago

@lzap I'm using a super admin. Basically I just create an empty hook hosts/managed/create (or destroy) and comment out rm -f $HOOK_OBJECT_FILE in the hook_functions.sh.

After creating/deleting the host, the json does not show host parameters in 'parameters' or 'all_parameters' keys:

$ jq '.host|{parameters,all_parameters}' ~foreman/tmp/foreman_hooks-create.Z0zsIlatd5
{
  "parameters": [],
  "all_parameters": [],
}

all_parameters show only inherited parameters (from hostgroup, subnet etc).

If I drop the ".authorized" as described above, it shows my host param as below (for create only):

$ jq '.host|{parameters,all_parameters}' ~foreman/tmp/foreman_hooks-create.Z0zsIlatd5
{
  "parameters": [
    {
      "priority": null,
      "created_at": null,
      "updated_at": null,
      "id": null,
      "name": "extra-disks-layout",
      "value": "10:/test:xfs"
    }
   ],
  "all_parameters": [],
}
lzap commented 6 years ago

For the record, the change you refer to is:

index 7bd22a0fa..70f6e9147 100644
--- a/app/views/api/v2/hosts/main.json.rabl
+++ b/app/views/api/v2/hosts/main.json.rabl
@@ -43,7 +43,7 @@ end

 if @parameters
   node do |host|
-    { :parameters => partial("api/v2/parameters/index", :object => host.host_parameters.authorized) }
+    { :parameters => partial("api/v2/parameters/index", :object => host.host_parameters) }
   end
 end

@ares any ideas why authorized drops these? You mentioned something on todays scrum, is this related?

ares commented 6 years ago

What should this authorized call do? This does not specify any permission, so it find all parameters user has any parameter permission to. IMHO the correct use here would be .authorized(:view_params). If it's dropping some parameters and you feel it's unexpected, I suggest you enable debug log with permission logger and see what's going on. For some reason, user does not seem to be authorized for these parameters.

lzap commented 6 years ago

@achevalet are you using postcreate/postupdate? It works for me, I do see all params:

        "parameters": [
            {
                "priority": 70,
                "created_at": "2018-09-18 10:38:32 +0200",
                "updated_at": "2018-09-18 10:38:32 +0200",
                "id": 4,
                "name": "host_param",
                "value": "vadfsfsdsdf"
            }
        ],
        "all_parameters": [
            {
                "priority": 70,
                "created_at": "2018-09-18 10:38:32 +0200",
                "updated_at": "2018-09-18 10:38:32 +0200",
                "id": 4,
                "name": "host_param",
                "value": "vadfsfsdsdf"
            },
            {
                "priority": 60,
                "created_at": "2018-09-18 10:39:13 +0200",
                "updated_at": "2018-09-18 10:39:13 +0200",
                "id": 5,
                "name": "hostgroup_param",
                "value": "jklgdflůjgfdgfdsfgdgdf"
            },
            {
                "priority": 0,
                "created_at": "2018-08-14 13:08:25 +0200",
                "updated_at": "2018-08-14 13:08:25 +0200",
                "id": 3,
                "name": "global_param1",
                "value": "dummy value"
            }
        ],

Please try again with postcreate/postupdate, I do see all params. Help me to reproduce this.

The .authorized(:view_params) check is a bug which I reported and I am gonna fix in core: https://github.com/theforeman/foreman/pull/6076

achevalet commented 6 years ago

Yes, I can reproduce the same. I see all params with postcreate/postupdate but I don't see them with create/destroy/postdestroy.

lzap commented 6 years ago

But that's why we created post-versions! They technically cannot contain it because it's too early in the orchestration. Use post versions, we know this is a regression that's why we created them.

lzap commented 6 years ago

Do you see them in destroy? You could use postcreate/postupdate/destroy. But that sounds quite odd.

wiad commented 6 years ago

They technically cannot contain it because it's too early in the orchestration.

This confuses me since the parameters are indeed available in both 'create' and 'destroy' if you remove the 'authorized' bit in the code.

Since postcreate and postupdate, from what I understand, cannot trigger rollback on errors they dont fit my usecase. I use hooks, among other things, to validate input of specific parameters. If those parameters are not correct the orchestration needs to halt and get rolled back.

lzap commented 6 years ago

I don't know, maybe @orrabin can jump in and provide more info about why not all parameters are showing up. This looks like a regression.

achevalet commented 6 years ago

Do you see them in destroy?

No, I don't see them in destroy and postdestroy.

wiad commented 6 years ago

Upgrade to 1.19 today and was reminded of this issue. Tried to set .authorized(:view_params) but that made no difference, also tried to debug the issue with sql debugging (I did not find the 'permissions' logger mentioned earlier) which outputs: Permission Load (0.9ms) SELECT "permissions".* FROM "permissions" WHERE "permissions"."resource_type" = $1 [["resource_type", "Host"]]

right before my hook script errors out due to it not finding expected data, but that line is not helping much. The only mentioned user in the logs is my own user (Current user set to xxxx (admin)).

I'm back to removing the .authorized bit to make my scripts work again.

wiad commented 4 years ago

same thing in 1.24, still have to edit the rabl file to get all parameters available to my hook scripts.

wiad commented 3 years ago

This is still a problem in 2.3, and now, besides adjusting the host.host_parameters line I also have to comment the following line:

  node(:registration_token) { |h| h.registration_token }

which is a new addition for the 'global registration' feature.

Is there really no proper way to get host parameters passed to foreman hooks in create/destroy, without having to tinker with the main.json.rabl file?

lzap commented 3 years ago

For the record, we are working on https://github.com/theforeman/foreman_webhooks which will provide same functionality but with much better interface. Whole model is exposed in ERB template so you can render your JSON as you want. Package for 2.3 should land in the upcoming weeks.