Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Empty 'parameters' array in json output #48

Open
wiad opened this issue Aug 14, 2017 · 20 comments
Open

Empty 'parameters' array in json output #48

wiad opened this issue Aug 14, 2017 · 20 comments

Comments

@wiad
Copy link
Contributor

wiad commented Aug 14, 2017

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
Copy link

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
Copy link
Contributor Author

wiad commented Dec 8, 2017

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
Copy link
Contributor Author

wiad commented Jun 27, 2018

Still need to do this workaround on Foreman 1.17

@wiad
Copy link
Contributor Author

wiad commented Aug 15, 2018

Still need to do this workaround on Foreman 1.18

@achevalet
Copy link

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
Copy link
Member

lzap commented Sep 7, 2018

@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
Copy link

@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
Copy link
Member

lzap commented Sep 10, 2018

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
Copy link
Member

ares commented Sep 13, 2018

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
Copy link
Member

lzap commented Sep 18, 2018

@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: theforeman/foreman#6076

@achevalet
Copy link

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

@lzap
Copy link
Member

lzap commented Sep 21, 2018

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
Copy link
Member

lzap commented Sep 21, 2018

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

@wiad
Copy link
Contributor Author

wiad commented Sep 21, 2018

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
Copy link
Member

lzap commented Sep 21, 2018

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
Copy link

achevalet commented Sep 21, 2018

Do you see them in destroy?

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

@wiad
Copy link
Contributor Author

wiad commented Oct 11, 2018

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
Copy link
Contributor Author

wiad commented Feb 6, 2020

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

@wiad
Copy link
Contributor Author

wiad commented Dec 11, 2020

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
Copy link
Member

lzap commented Jan 18, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants