Add a separate controller for payload download.

[martinpovolny]

I implemented the option a) from #33

I did not figure how to make a "arbitrary" method other thatn index or show using what is available in the API. (calling send_file from other methods does now work due to assumptions in the API code).

example usage: curl -i -u admin:smartvm http://localhost:3000/api/red_hat_migration_analytics_payload?task_id=30

@lpichler: do you have an idea how to do this in a the existing controller?

@mturley: can you, please, try if this works for you (with use of the file-saver package)?

TODO:

if we agree on this approach I will:

• move the duplicated method to a mixin
• write some test

---

BZ for backport to ivanchuk: https://bugzilla.redhat.com/show_bug.cgi?id=1788730
JIRA task for feature: https://issues.redhat.com/browse/MIGENG-241

[mturley]

@martinpovolny on my local machine I get this response when using your curl command example with a real bundle task id:

{
    "error": {
        "kind": "internal_server_error",
        "message": "undefined method `payload_path' for #<Hash:0x00007f958cb4ddd0>",
        "klass": "NoMethodError"
    }
}

[mzazrivec]

Yep, I'm getting the same error that Mike shows above. It really seems that task.context_data is a Hash rather than an object.

     3: def index
     4:   check_feature_enabled
     5:   task_id = params['task_id']
     6:   raise "Must specify a task id via \"task_id\"" if task_id.blank?
     7: 
     8:   begin
     9:     binding.pry
    10:     task = MiqTask.find(task_id)
 => 11:     path = task&.context_data&.payload_path
    12:     if path && File.exist?(path)
    13:       send_file(path, :type => 'application/binary', :filename => 'analytics_payload.bin')
    14:     else
    15:       raise "Payload not found."
    16:     end
    17:   end
    18: end

[1] pry(#<Api::RedHatMigrationAnalyticsPayloadController>)> task
=> #<MiqTask:0x00007f09f0aeab68
 id: 451,
 name: "Collect and package inventory",
 state: "Finished",
 status: "Ok",
 message: "Task completed successfully",
 userid: "admin",
 created_on: Tue, 28 Jan 2020 10:24:22 UTC +00:00,
 updated_on: Tue, 28 Jan 2020 10:24:24 UTC +00:00,
 pct_complete: nil,
 context_data: {:payload_path=>"/tmp/cfme_inventory-20200128-23876-1tkb2gu.tar.gz", :ip_address=>"192.168.1.17"},
 results: nil,
 miq_server_id: 3,
 identifier: nil,
 started_on: Tue, 28 Jan 2020 10:24:23 UTC +00:00,
 zone: nil>
[2] pry(#<Api::RedHatMigrationAnalyticsPayloadController>)> task.context_data
=> {:payload_path=>"/tmp/cfme_inventory-20200128-23876-1tkb2gu.tar.gz", :ip_address=>"192.168.1.17"}
[3] pry(#<Api::RedHatMigrationAnalyticsPayloadController>)> task.context_data[:payload_path]
=> "/tmp/cfme_inventory-20200128-23876-1tkb2gu.tar.gz"
[4] pry(#<Api::RedHatMigrationAnalyticsPayloadController>)> task.context_data.class
=> Hash
[5] pry(#<Api::RedHatMigrationAnalyticsPayloadController>)>

[martinpovolny]

Thanks, @mzazrivec, @mturley. Fixing the hash access and doing the TODOs....

[miq-bot]

Checked commits martinpovolny/cfme-migration_analytics@c9e61ed~...9b49f54 with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
5 files checked, 2 offenses detected

**

• 💣 💥 🔥 🚒 - Linter/Rubocop - missing config files
• 💣 💥 🔥 🚒 - Linter/Yaml - missing config files

[mzazrivec]

I'm OK with this change, though I'll let @mturley chime in here before it's merged.

[lpichler]

begin block is needed here ?

[lpichler]

@lpichler: do you have an idea how to do this in a the existing controller?

@martinpovolny

I am not aware about it.As far as I know you can probably do: POST + "action mechanism" like we have for example for set_ownership
DOC: https://www.manageiq.org/docs/reference/latest/api/reference/ownership

SOURCE: https://github.com/ManageIQ/manageiq-api/blob/47e9025c58adbd398b040e5fdfd9cabf452b876e/app/controllers/api/base_controller/generic.rb#L216

question is if this "action mechanism" is proper for your requirement regard to its semantics.

[martinpovolny]

@lpichler : the action mechanism is ok, but you cannot call send_file from there. Then handling methods end with action_results or similar see?

I am talking about "arbitrary" methods, grep api.yml for "arbitrary" and see that there's just arbitrary collections. Then look at one.
There you'll see that there can be arbitrary "show" and "index" controller methods/actions and then, please read again my question ;-)

[mturley]

Hmm, @martinpovolny I'm realizing there may be an issue with using the API methods in manageiq-ui-classic (the ones that provide the custom auth header). They assume the response will always be JSON: https://github.com/ManageIQ/manageiq-ui-classic/blob/master/app/javascript/http_api/fetch.js#L79

So, either I need to make a patch to manageiq-ui-classic to make it possible to call response.blob() there instead of response.json() (maybe by detecting the content-type: application/binary header on the response), or we need to encode the payload into part of a JSON response. Maybe it would be easiest to just base64-encode it and use that as a string value within some JSON?

[mturley]

@martinpovolny I'll open a PR to manageiq-ui-classic to resolve the issue I'm describing. If it gets rejected we'll have to follow up and maybe encode in base64 or something.

[mturley]

@martinpovolny do you need anyone else to review this or are you ok with me and @mzazrivec merging it?

[lpichler]

Looks good 👍

[simaishi]

Ivanchuk backport details:

$ git log -1
commit 7d6152eff7fbba0aed29e6ecb5a67684aa522981
Author: Milan Zázrivec <[email protected]>
Date:   Tue Feb 4 11:47:35 2020 +0100

    Merge pull request #37 from martinpovolny/payload_download3

    Add a separate controller for payload download.

    (cherry picked from commit 9873a39134c2eee0ec2d03035570ebad4c6bc3ab)

    https://bugzilla.redhat.com/show_bug.cgi?id=1805818