Bug #12089

Unable to Associate OS to Provisioning Templates via API

Added by Ryan Dyer almost 3 years ago. Updated 8 days ago.

Status:Closed
Priority:High
Assignee:Tomáš Strachota
Category:API
Target version:1.9.3
Difficulty: Team Backlog:
Triaged: Fixed in Releases:
Bugzilla link: Found in Releases:
Pull request:https://github.com/theforeman/foreman/pull/2851

Description

this is in regards to this thread: https://groups.google.com/forum/#!topic/foreman-dev/6wOeQeuJX5o

I am unable to associate an OS to a provisioning template via the API. The GET of a config_template has an operatingsystems field (hash), but the PUT expects an operatingsystem_ids (array). When I PUT the os_ids array, it succeeds from a server response perspective, but there is no actual association that occurs. In 1.6 I achieved this using a PUT of operatingsystems as a hash. I do not know which release changed this behavior.

Associated revisions

Revision 2bb102ba
Added by Tomáš Strachota over 2 years ago

Refs #12089 - tests for config templates controller param wrapping

Revision 7a3a1052
Added by Tomáš Strachota over 2 years ago

Fixes #12089 - fix using unwrapped parameters in config templates API controller

Revision 3b404bed
Added by Tomáš Strachota over 2 years ago

Fixes #12089 - fix using unwrapped parameters in config templates API controller

Revision 1153aa6b
Added by Daniel Lobato Garcia over 2 years ago

Refs #12089 - Revert fix using unwrapped parameters in config templates API controller

This is already fixed in develop, and wrap_params is not needed. It
won't harm but it's basically dead code. The original commit was merged
by mistake into develop, it was supposed to be merged to 1.9-stable
only.

This reverts commit 7a3a1052eeddd984988f288b1b67a7a8adb7359e.

Revision 9fbacd49
Added by Tomáš Strachota over 2 years ago

Fixes #12089 - fix using unwrapped parameters in config templates API controller

(cherry picked from commit 3b404bedba8758b0a5ed3426a8720199aa47d687)

History

#1 Updated by Ryan Dyer almost 3 years ago

I have found this issue on foreman 1.9.0.

This issue can be worked around by embedding the os_ids inside a config_template hash. eg:
"config_template": { "operatingsystem_ids": [1, 2, 3] }

#2 Updated by Dominic Cleal almost 3 years ago

  • Legacy Backlogs Release (now unused) set to 91

Appears to part of the config to provisioning templates renaming, as wrap parameters looks like it's copying it into the wrong hash.

A basic update shows this:

2015-10-08T08:28:51 [app] [I] Started PUT "/api/v2/config_templates/1" for 127.0.0.1 at 2015-10-08 08:28:51 +0100
2015-10-08T08:28:51 [app] [I] Processing by Api::V2::ConfigTemplatesController#update as JSON
2015-10-08T08:28:51 [app] [I]   Parameters: {"test"=>"foo", "apiv"=>"v2", "id"=>"1", "provisioning_template"=>{}}

That provisioning_template hash should be config_template, since the update action checks params[:config_template].

#3 Updated by Tomáš Strachota almost 3 years ago

  • Status changed from New to Assigned
  • Assignee set to Tomáš Strachota

#4 Updated by Tomáš Strachota over 2 years ago

The issue got fixed in current develop by merging patch for #10988 which removes some hacks around ProvisioningTemplate rename:
https://github.com/theforeman/foreman/commit/c1755e070ee10ee026e0b3996378368369c4ca83#diff-fa8faf553918630d1afb4537fb180e90L1

I don't think we want that patch in the 1.9.3 branch. At the same time I don't think we want my fix in the develop as it has no effect there:
https://github.com/tstrachota/foreman/commit/63e2be4dfca70cfb17e4ce06fdc2f6291e6f94cc

What do you think, should I open a PR against the 1.9.3 stable branch?

#5 Updated by Dominic Cleal over 2 years ago

I think your fix is still valid for develop, Tomas. Although we removed the ConfigTemplate constant (which affects internal use and plugins), the public API still exists because it's part of API v2 - so your fix should still work in develop.

#6 Updated by Tomáš Strachota over 2 years ago

Well, the fix will do no harm in develop but it also won't fix anything as it's already fixed there. Removal of the const_missing hack affected api behaviour too.

#7 Updated by Dominic Cleal over 2 years ago

Oh yes, so it did, I didn't expect that! Then yep, send a PR against 1.9-stable and we'll take it there (tests won't run on Jenkins FYI). Once that's in, I think we should also commit the test to develop so we ensure it keeps working, even if the fix wasn't needed.

#8 Updated by The Foreman Bot over 2 years ago

  • Status changed from Assigned to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/2851 added

#10 Updated by Anonymous over 2 years ago

  • Status changed from Ready For Testing to Closed
  • % Done changed from 0 to 100

Also available in: Atom PDF