Fixes #14158 - Add support for tailoring files #225

xprazak2 · 2016-11-21T13:16:07Z

TODO:

tests

houndci-bot · 2016-11-21T13:16:23Z

test/functional/tailoring_files_controller_test.rb

+    FactoryGirl.create(:openscap_proxy)
+    ForemanOpenscap::TailoringFile.any_instance.stubs(:fetch_profiles).returns({ 'test_profile_key' => 'test_profile_title' })
+  end
+end


Final newline missing.

houndci-bot · 2016-11-21T13:16:23Z

test/functional/tailoring_files_controller_test.rb

+
+  test 'index' do
+    get :index, set_session_user
+    binding.pry


Remove debugger entry point binding.pry.

houndci-bot · 2016-11-21T13:16:24Z

app/models/concerns/foreman_openscap/data_stream_content.rb

+    private
+
+    def redigest
+      self[:digest] = Digest::SHA256.hexdigest "#{scap_file}"


Prefer to_s over string interpolation.

houndci-bot · 2016-11-21T13:16:25Z

app/models/concerns/foreman_openscap/data_stream_content.rb

+    end
+
+    def digest
+      self[:digest] ||= Digest::SHA256.hexdigest "#{scap_file}"


Prefer to_s over string interpolation.

houndci-bot · 2016-11-21T13:16:25Z

app/controllers/tailoring_files_controller.rb

+
+  def index
+  @tailoring_files = resource_base.search_for(params[:search], :order => params[:order])
+    .paginate(:page => params[:page], :per_page => params[:per_page])


Place the . on the previous line, together with the method call receiver.
Align .paginate with .search_for on line 13.

houndci-bot · 2016-11-21T13:16:26Z

app/controllers/tailoring_files_controller.rb

+  end
+
+  def index
+  @tailoring_files = resource_base.search_for(params[:search], :order => params[:order])


Use 2 (not 0) spaces for indentation.

houndci-bot · 2016-11-21T13:16:26Z

app/controllers/policies_controller.rb

@@ -13,7 +14,7 @@ def index
    @policies = resource_base
                  .search_for(params[:search], :order => params[:order])
                  .paginate(:page => params[:page], :per_page => params[:per_page])
-                  .includes(:scap_content, :scap_content_profile)
+                  .includes(:scap_content, :scap_content_profile, :tailoring_file)


Place the . on the previous line, together with the method call receiver.
Align .includes with resource_base on line 14.

ares · 2016-11-22T13:38:58Z

trying to update name of tailoring file results in 500

2016-11-22T14:37:56 26c05376 [app] [I] Started PATCH "/compliance/tailoring_files/1" for ::1 at 2016-11-22 14:37:56 +0100
2016-11-22T14:37:56 26c05376 [app] [I] Processing by TailoringFilesController#update as HTML
2016-11-22T14:37:56 26c05376 [app] [I]   Parameters: {"utf8"=>"✓", "authenticity_token"=>"D1uNodKYx/YcaE5LYINGIvMwNFi2TigegZ+iW4UOzM1+9PToKnvAgtTCGQjpte6Cnli56y/0NVcw907tdoazgg==", "tailoring_file"=>{"name"=>"my_tailor2", "location_ids"=>["", "2"], "organization_ids"=>["", "1"]}, "commit"=>"Submit", "id"=>"1"}
2016-11-22T14:37:56 26c05376 [sql] [D]   ActiveRecord::SessionStore::Session Load (0.3ms)  SELECT  "sessions".* FROM "sessions" WHERE "sessions"."session_id" = $1  ORDER BY "sessions"."id" ASC LIMIT 1  [["session_id", "26c05376c1e8353aad4a78ba13461875"]]
2016-11-22T14:37:56 26c05376 [sql] [D]   User Load (0.2ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = $1 LIMIT 1  [["id", 3]]
2016-11-22T14:37:56 26c05376 [app] [D] Setting current user thread-local variable to admin
2016-11-22T14:37:56 26c05376 [sql] [D]    (0.3ms)  SELECT COUNT(*) FROM "taxonomies" WHERE "taxonomies"."type" IN ('Organization')
2016-11-22T14:37:56 26c05376 [app] [D] Setting current organization thread-local variable to none
2016-11-22T14:37:56 26c05376 [sql] [D]    (0.2ms)  SELECT COUNT(*) FROM "taxonomies" WHERE "taxonomies"."type" IN ('Location')
2016-11-22T14:37:56 26c05376 [app] [D] Setting current location thread-local variable to none
2016-11-22T14:37:56 26c05376 [sql] [D]   AuthSource Load (0.1ms)  SELECT  "auth_sources".* FROM "auth_sources" WHERE "auth_sources"."id" = $1 LIMIT 1  [["id", 1]]
2016-11-22T14:37:56 26c05376 [sql] [D]    (0.2ms)  SELECT  "taxonomies"."id" FROM "taxonomies" WHERE "taxonomies"."type" IN ('Location') LIMIT 1
2016-11-22T14:37:56 26c05376 [sql] [D]    (0.1ms)  SELECT  "taxonomies"."id" FROM "taxonomies" WHERE "taxonomies"."type" IN ('Organization') LIMIT 1
2016-11-22T14:37:56 26c05376 [sql] [D]   ForemanOpenscap::TailoringFile Load (0.2ms)  SELECT  "foreman_openscap_tailoring_files".* FROM "foreman_openscap_tailoring_files" WHERE "foreman_openscap_tailoring_files"."id" = $1 LIMIT 1  [["id", 1]]
2016-11-22T14:37:56 26c05376 [sql] [D]   ForemanOpenscap::TailoringFile Load (0.3ms)  SELECT "foreman_openscap_tailoring_files".* FROM "foreman_openscap_tailoring_files"
2016-11-22T14:37:56 26c05376 [app] [I] Completed 500 Internal Server Error in 19ms (ActiveRecord: 1.5ms)
2016-11-22T14:37:56 26c05376 [app] [F] 
 | NameError (undefined local variable or method `scap_content_params' for #<TailoringFilesController:0x0055e42e8f7a10>):
 |   /home/ares/Projekty/Zdrojaky/foreman_openscap/app/controllers/tailoring_files_controller.rb:34:in `update'
 |   app/controllers/concerns/application_shared.rb:14:in `set_timezone'
 |   app/models/concerns/foreman/thread_session.rb:32:in `clear_thread'
 |   lib/middleware/catch_json_parse_errors.rb:8:in `call'
 |   lib/middleware/tagged_logging.rb:18:in `call'
 | 
 | 
2016-11-22T14:37:57 26c05376 [app] [I]   Rendered /home/ares/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/actionpack-4.2.7.1/lib/action_dispatch/middleware/templates/rescues/_source.erb (4.7ms)
2016-11-22T14:37:57 26c05376 [app] [I]   Rendered /home/ares/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/actionpack-4.2.7.1/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb (1.9ms)
2016-11-22T14:37:57 26c05376 [app] [I]   Rendered /home/ares/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/actionpack-4.2.7.1/lib/action_dispatch/middleware/templates/rescues/_request_and_response.html.erb (0.7ms)
2016-11-22T14:37:57 26c05376 [app] [I]   Rendered /home/ares/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/actionpack-4.2.7.1/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb within rescues/layout (20.0ms

ares · 2016-11-22T14:11:14Z

One more issue found during testing, when I change tailoring file for a policy, the tailoring file is already cached on smart proxy at tailoring/5/5_tailoring_file.xml. Even if foreman_scap_client gets new hash, it downloads old tailoring file. We either need to change the way we store files on proxy and start using hashes there too or proxy needs invalidating the files which should be triggered from Foreman whenever tailoring file was changed for policy or the content of the file changed itself. Do we have the same issue for regular scap content file?

Except for this issue it works pretty well 👍

theforeman-bot · 2016-11-22T15:23:56Z

@xprazak2, this pull request is currently not mergeable. Please rebase against the master branch and push again.

If you have a remote called 'upstream' that points to this repository, you can do this by running:

    $ git pull --rebase upstream master

This message was auto-generated by Foreman's prprocessor

houndci-bot · 2016-11-22T15:24:14Z

app/controllers/policies_controller.rb

-                  .includes(:scap_content, :scap_content_profile)
+    @policies = resource_base.search_for(params[:search], :order => params[:order])
+                             .paginate(:page => params[:page], :per_page => params[:per_page])
+                             .includes(:scap_content, :scap_content_profile, :tailoring_file)


Place the . on the previous line, together with the method call receiver.

houndci-bot · 2016-11-22T15:24:14Z

app/controllers/policies_controller.rb

-                  .paginate(:page => params[:page], :per_page => params[:per_page])
-                  .includes(:scap_content, :scap_content_profile)
+    @policies = resource_base.search_for(params[:search], :order => params[:order])
+                             .paginate(:page => params[:page], :per_page => params[:per_page])


Place the . on the previous line, together with the method call receiver.

xprazak2 · 2016-11-22T15:29:35Z

I added more tests, fixed style most of style issues, disabled a couple of obnoxious cops, fixed the 500. Not sure if we have the problems when changing scap contents for policy, I will have to verify.

houndci-bot · 2017-01-02T12:52:15Z

test/test_plugin_helper.rb

+    ProxyAPI::Openscap.any_instance.stubs(:fetch_policies_for_scap_content).
+                       returns({'xccdf_org.ssgproject.content_profile_common' => 'Common Profile for General-Purpose Fedora Systems'})
+    ProxyAPI::Openscap.any_instance.stubs(:fetch_profiles_for_tailoring_file).
+                       returns({'xccdf_org.ssgproject.test_profile_common' => 'Stubbed test profile'})


Use 2 (not 19) spaces for indenting an expression spanning multiple lines.

houndci-bot · 2017-01-02T12:52:15Z

test/test_plugin_helper.rb

+    ProxyAPI::Version.any_instance.stubs(:proxy_versions).returns(versions)
+    ProxyAPI::Openscap.any_instance.stubs(:validate_scap_file).returns({'errors' => []})
+    ProxyAPI::Openscap.any_instance.stubs(:fetch_policies_for_scap_content).
+                       returns({'xccdf_org.ssgproject.content_profile_common' => 'Common Profile for General-Purpose Fedora Systems'})


Use 2 (not 19) spaces for indenting an expression spanning multiple lines.

xprazak2 · 2017-01-02T12:52:16Z

Changes:

I moved the dots to end of lines and enabled the corresponding cop
Tailoring file XML can be downloaded from UI through action menu
When uploading a tailoring file with old proxy version, notification is shown
grouping dropdowns for tailoring file and its profiles in policy wizard, tailoring profiles dropdown is not shown when tailoring file is not selected

houndci-bot · 2017-01-02T13:17:03Z

app/controllers/tailoring_files_controller.rb

+    end
+  end
+
+  def edit; end


Put the end of empty method definitions on the next line.

houndci-bot · 2017-01-02T13:17:03Z

app/controllers/api/v2/compliance/tailoring_files_controller.rb

+
+      api :GET, '/compliance/tailoring_files/:id', N_('Show a Tailoring file')
+      param :id, :identifier, :required => true
+      def show; end


Put the end of empty method definitions on the next line.

houndci-bot · 2017-01-03T12:38:42Z

app/controllers/tailoring_files_controller.rb

+
+  def index
+    @tailoring_files = resource_base.search_for(params[:search], :order => params[:order]).
+    paginate(:page => params[:page], :per_page => params[:per_page])


Align paginate with resource_base.search_for(params[:search], :order => params[:order]). on line 13.

xprazak2 · 2017-01-05T12:25:20Z

Adjusted column width on tailoring_files/index page.

ares · 2017-01-05T12:34:24Z

Cool, you even added Created column 👍 I think it's ready for merge now pending jenkins. Once we get smart proxy PR also confirmed by Jenkins I'll merge the whole train of PRs

shlomizadok · 2017-01-06T11:36:59Z

Thanks @xprazak2, @ares 🎉

ares · 2017-01-06T12:00:11Z

👍 @xprazak2 I think you have a good material for next community demo :-)

theforeman-bot added Needs testing Not yet reviewed labels Nov 21, 2016

houndci-bot reviewed Nov 21, 2016

View reviewed changes

ares added Waiting on contributor and removed Needs testing Not yet reviewed labels Nov 22, 2016

xprazak2 force-pushed the tailoring branch from e21a602 to 73d2996 Compare November 22, 2016 15:13

theforeman-bot added Needs re-review Needs testing and removed Waiting on contributor labels Nov 22, 2016

xprazak2 force-pushed the tailoring branch from 73d2996 to 7f320bd Compare November 22, 2016 15:23

theforeman-bot added Waiting on contributor and removed Needs testing Needs re-review labels Nov 22, 2016

houndci-bot reviewed Nov 22, 2016

View reviewed changes

xprazak2 force-pushed the tailoring branch from 7f320bd to 4d8d11e Compare November 22, 2016 15:35

theforeman-bot added Needs re-review Needs testing and removed Waiting on contributor labels Nov 22, 2016

theforeman-bot added the Waiting on contributor label Dec 23, 2016

xprazak2 force-pushed the tailoring branch from a1a6251 to f41f99f Compare January 2, 2017 12:52

theforeman-bot added Needs re-review Needs testing and removed Waiting on contributor labels Jan 2, 2017

houndci-bot reviewed Jan 2, 2017

View reviewed changes

xprazak2 force-pushed the tailoring branch 3 times, most recently from 7284d2d to 5099c84 Compare January 2, 2017 13:16

houndci-bot reviewed Jan 2, 2017

View reviewed changes

app/controllers/tailoring_files_controller.rb

end

end

def edit; end

Copy link

houndci-bot Jan 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put the end of empty method definitions on the next line.

houndci-bot reviewed Jan 2, 2017

View reviewed changes

xprazak2 force-pushed the tailoring branch 4 times, most recently from a8a8b15 to 1651821 Compare January 3, 2017 12:38

houndci-bot reviewed Jan 3, 2017

View reviewed changes

xprazak2 force-pushed the tailoring branch from 1651821 to 647a8ea Compare January 3, 2017 12:39

Fixes #14158 - Add support for tailoring files

7ce8992

xprazak2 force-pushed the tailoring branch from 647a8ea to 7ce8992 Compare January 5, 2017 12:22

ares approved these changes Jan 5, 2017

View reviewed changes

ares added Ready to merge and removed Needs re-review Needs testing labels Jan 5, 2017

shlomizadok merged commit 4a75cb4 into theforeman:master Jan 6, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #14158 - Add support for tailoring files #225

Fixes #14158 - Add support for tailoring files #225

xprazak2 commented Nov 21, 2016 •

edited

houndci-bot Nov 21, 2016

houndci-bot Nov 21, 2016

houndci-bot Nov 21, 2016

houndci-bot Nov 21, 2016

houndci-bot Nov 21, 2016

houndci-bot Nov 21, 2016

houndci-bot Nov 21, 2016

ares commented Nov 22, 2016

ares commented Nov 22, 2016

theforeman-bot commented Nov 22, 2016

houndci-bot Nov 22, 2016

houndci-bot Nov 22, 2016

ares Nov 30, 2016

xprazak2 commented Nov 22, 2016

houndci-bot Jan 2, 2017

houndci-bot Jan 2, 2017

xprazak2 commented Jan 2, 2017

houndci-bot Jan 2, 2017

houndci-bot Jan 2, 2017

houndci-bot Jan 3, 2017

xprazak2 commented Jan 5, 2017

ares commented Jan 5, 2017

shlomizadok commented Jan 6, 2017

ares commented Jan 6, 2017

Fixes #14158 - Add support for tailoring files #225

Fixes #14158 - Add support for tailoring files #225

Conversation

xprazak2 commented Nov 21, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ares commented Nov 22, 2016

ares commented Nov 22, 2016

theforeman-bot commented Nov 22, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xprazak2 commented Nov 22, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xprazak2 commented Jan 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xprazak2 commented Jan 5, 2017

ares commented Jan 5, 2017

shlomizadok commented Jan 6, 2017

ares commented Jan 6, 2017

xprazak2 commented Nov 21, 2016 •

edited