Fixes #29993 - Drop Puppetrun #7719

domitea · 2020-06-03T11:09:18Z

Removing puppetrun from core

theforeman-bot · 2020-06-03T11:09:27Z

ekohl · 2020-06-03T11:23:58Z

Not looking at the code, I think the descriptions of this and the issue are way too minimal. As an upgrading user this is not very useful. Normally we try to give some background about the feature, why it's being removed and what the alternatives are. It's also good refer to discussion that has happened, like RFCs.

Another thing to consider is that right now this feature hasn't been deprecated. Luckily we're still in the RC phase of 2.1 but at the very least the 2.1 manual needs a release note about this.

app/controllers/api/v2/puppet_hosts_controller.rb

app/controllers/concerns/foreman/controller/puppet/hosts_controller_extensions.rb

app/controllers/api/v2/puppet_hosts_controller.rb

ezr-ondrej

I'd agree with Ewoud we should keep the API for few versions.

ezr-ondrej · 2020-06-30T22:27:11Z

@domitea ? This would be nice to get in, what is the status here? :)

ezr-ondrej

Why have you decided to keep PUPPET_MULTIPLE_ACTIONS ?
I believe now it should be == to the MULTIPLE_EDIT_ACTIONS and thus we can use that instead.

ezr-ondrej · 2020-07-10T11:31:11Z

app/controllers/api/v2/puppet_hosts_controller.rb

-          else
-            super
+        when 'puppetrun'
+          :puppetrun


haven't we dropped the permission?

I believe we can prepend_before_action and render there directly. Also, we can add deprecations for the apidocs.

Take a look at https://github.com/theforeman/foreman/pull/7800/files#diff-36882131819ddf97a1e444b073996328R5
I believe we can do the same here and keep just empty puppetrun method with the prepended filter, that will render the status.

app/controllers/api/v2/puppet_hosts_controller.rb

test/unit/foreman/access_permissions_test.rb

ezr-ondrej · 2020-07-16T08:34:09Z

This needs rebase

ezr-ondrej

Few point just to make it bit more tidy.
Additionaly do we have at least a redmine ticket to remove the SmartProxy endpoint for this?

ezr-ondrej · 2020-07-16T10:24:06Z

app/controllers/api/v2/puppet_hosts_controller.rb

@@ -4,23 +4,23 @@ class PuppetHostsController < V2::BaseController
      include Api::Version2
      include HostsControllerExtension

+      prepend_before_action :fail_and_inform_about_plugin
+
      check_permissions_for ['puppetrun']


This can be dropped as well now :)

ezr-ondrej · 2020-07-16T10:25:52Z

app/controllers/api/v2/puppet_hosts_controller.rb

      param :id, :identifier_dottable, :required => true

      def puppetrun
-        return deny_access unless Setting[:puppetrun]
-        process_response @host.puppetrun!
      end

      def action_permission


We don't need to keep the method now, right?

ezr-ondrej · 2020-07-16T10:26:51Z

app/controllers/api/v2/puppet_hosts_controller.rb

@@ -4,23 +4,23 @@ class PuppetHostsController < V2::BaseController
      include Api::Version2
      include HostsControllerExtension

+      prepend_before_action :fail_and_inform_about_plugin
+


Look at this block: https://github.com/theforeman/foreman/pull/7800/files#diff-36882131819ddf97a1e444b073996328R7-R10
It would mark the resource as deprecated in /apidoc. I think we can use it here as well.
I'm not sure if it wouldn't deprecate the enpoints added by HostsControllerExtensions so it needs to get tested. I'd be ok if we do not have it if that turns out to be too much of strugle. If it works though, it adds visibility to the deprecation.

ezr-ondrej

Thanks @domitea, looks good now 👍
Thanks for the ticket, for reference: RM#30429

ezr-ondrej · 2020-07-17T10:52:31Z

[test katello] doesn't seem related, but it's quite a lot of failures

ezr-ondrej · 2020-07-17T11:08:51Z

[test katello]

ezr-ondrej · 2020-07-17T11:14:02Z

config/routes/puppet/api/v2.rb

-    scope "(:apiv)", :module => :v2, :defaults => {:apiv => 'v2'}, :apiv => /v2/, :constraints => ApiConstraints.new(:version => 2, :default => true) do
-      constraints(:id => /[^\/]+/) do
-        resources :hosts, :except => [:new, :edit] do
-          put :puppetrun, :on => :member, :to => 'puppet_hosts#puppetrun'


Oh, I've missed this.
This needs to stay, otherwise the deprecated API endpoint will not work.

ezr-ondrej

Please bring back the api endpoint file, otherwise the endpoint is good as dead (404) and noone will see the informative 501 error :)

domitea · 2020-07-17T13:06:25Z

@ezr-ondrej File returned, hope It's ready for merge

ezr-ondrej

We need to skip the endpoint from access_permissions_test

test/unit/foreman/access_permissions_test.rb

ezr-ondrej

Works for me 👍

ezr-ondrej · 2020-07-18T21:06:41Z

Thanks @domitea ! 👍

ekohl · 2020-09-04T12:26:46Z

This needs to be reflected in the 2.2 and nightly manual. Section 3.5.2 still refers to the puppetrun setting.

zeha · 2020-11-06T10:27:18Z

Coming from an over-time upgraded install, the migration (2.2.0) here fails with:

== 20200602155700 DropPuppetRun: migrating ====================================
rake aborted!
StandardError: An error has occurred, this and all later migrations canceled:

Setting::Puppet is marked as readonly

FTR, DB contents for this setting:

foreman=# select * from settings where name = 'puppetrun';
-[ RECORD 1 ]-+---------------------------
id            | 22
name          | puppetrun
value         |
description   | Enable puppetrun support
category      | Setting::Puppet
settings_type | boolean
default       | --- false
created_at    | 2020-08-24 19:22:15.624772
updated_at    | 2020-08-24 19:22:15.624772
full_name     | Puppetrun
encrypted     | f

ekohl · 2020-11-06T11:54:51Z

@zeha the problem is that if puppetrun is in settings.yaml that the migration fails. #8125 should fix that but the workaround is to drop puppetrun from settings.yaml before running the migration.

zeha · 2020-11-06T12:28:58Z

@zeha the problem is that if puppetrun is in settings.yaml that the migration fails [...]

thanks!

theforeman-bot added Needs testing Not yet reviewed labels Jun 3, 2020

ekohl reviewed Jun 3, 2020

View reviewed changes

app/controllers/api/v2/puppet_hosts_controller.rb Show resolved Hide resolved

tbrisker assigned ezr-ondrej Jun 17, 2020

ezr-ondrej reviewed Jun 18, 2020

View reviewed changes

app/controllers/concerns/foreman/controller/puppet/hosts_controller_extensions.rb Show resolved Hide resolved

app/controllers/api/v2/puppet_hosts_controller.rb Show resolved Hide resolved

ezr-ondrej requested changes Jun 18, 2020

View reviewed changes

theforeman-bot added Waiting on contributor and removed Not yet reviewed labels Jun 18, 2020

domitea force-pushed the 29993_drop_puppet_run branch from fd3ea4e to 2de6ae7 Compare July 9, 2020 14:44

theforeman-bot added Needs re-review and removed Waiting on contributor labels Jul 9, 2020

ezr-ondrej requested changes Jul 10, 2020

View reviewed changes

theforeman-bot added Waiting on contributor and removed Needs re-review labels Jul 10, 2020

ezr-ondrej reviewed Jul 13, 2020

View reviewed changes

test/unit/foreman/access_permissions_test.rb Outdated Show resolved Hide resolved

domitea force-pushed the 29993_drop_puppet_run branch from 2de6ae7 to 11c0610 Compare July 16, 2020 07:45

theforeman-bot added Needs re-review and removed Waiting on contributor labels Jul 16, 2020

domitea force-pushed the 29993_drop_puppet_run branch from 11c0610 to fb44eb6 Compare July 16, 2020 08:40

ezr-ondrej requested changes Jul 16, 2020

View reviewed changes

theforeman-bot added Waiting on contributor and removed Needs re-review labels Jul 16, 2020

domitea force-pushed the 29993_drop_puppet_run branch from fb44eb6 to 6e5659c Compare July 17, 2020 09:40

theforeman-bot added Needs re-review and removed Waiting on contributor labels Jul 17, 2020

ezr-ondrej approved these changes Jul 17, 2020

View reviewed changes

theforeman-bot removed the Needs re-review label Jul 17, 2020

ezr-ondrej reviewed Jul 17, 2020

View reviewed changes

ezr-ondrej requested changes Jul 17, 2020

View reviewed changes

theforeman-bot added the Waiting on contributor label Jul 17, 2020

domitea force-pushed the 29993_drop_puppet_run branch from 6e5659c to b524fb9 Compare July 17, 2020 13:05

theforeman-bot added Needs re-review and removed Waiting on contributor labels Jul 17, 2020

ezr-ondrej requested changes Jul 17, 2020

View reviewed changes

test/unit/foreman/access_permissions_test.rb Outdated Show resolved Hide resolved

theforeman-bot added Waiting on contributor and removed Needs re-review labels Jul 17, 2020

Fixes #29993 - Drop Puppetrun

4154fbc

domitea force-pushed the 29993_drop_puppet_run branch from b524fb9 to 4154fbc Compare July 17, 2020 13:42

theforeman-bot added Needs re-review and removed Waiting on contributor labels Jul 17, 2020

ezr-ondrej approved these changes Jul 17, 2020

View reviewed changes

theforeman-bot removed the Needs re-review label Jul 17, 2020

ezr-ondrej merged commit 2f2e926 into theforeman:develop Jul 18, 2020

domitea mentioned this pull request Dec 21, 2020

fixes #31536 - Removing puppetrun command theforeman/hammer-cli-foreman#554

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #29993 - Drop Puppetrun #7719

Fixes #29993 - Drop Puppetrun #7719

domitea commented Jun 3, 2020

theforeman-bot commented Jun 3, 2020

ekohl commented Jun 3, 2020

ezr-ondrej left a comment

ezr-ondrej commented Jun 30, 2020

ezr-ondrej left a comment

ezr-ondrej Jul 10, 2020

ezr-ondrej commented Jul 16, 2020

ezr-ondrej left a comment

ezr-ondrej Jul 16, 2020

ezr-ondrej Jul 16, 2020

ezr-ondrej Jul 16, 2020

ezr-ondrej left a comment

ezr-ondrej commented Jul 17, 2020

ezr-ondrej commented Jul 17, 2020

ezr-ondrej Jul 17, 2020

ezr-ondrej left a comment

domitea commented Jul 17, 2020

ezr-ondrej left a comment

ezr-ondrej left a comment

ezr-ondrej commented Jul 18, 2020

ekohl commented Sep 4, 2020

zeha commented Nov 6, 2020 •

edited

ekohl commented Nov 6, 2020

zeha commented Nov 6, 2020

Fixes #29993 - Drop Puppetrun #7719

Fixes #29993 - Drop Puppetrun #7719

Conversation

domitea commented Jun 3, 2020

theforeman-bot commented Jun 3, 2020

ekohl commented Jun 3, 2020

ezr-ondrej left a comment

Choose a reason for hiding this comment

ezr-ondrej commented Jun 30, 2020

ezr-ondrej left a comment

Choose a reason for hiding this comment

ezr-ondrej Jul 10, 2020

Choose a reason for hiding this comment

ezr-ondrej commented Jul 16, 2020

ezr-ondrej left a comment

Choose a reason for hiding this comment

ezr-ondrej Jul 16, 2020

Choose a reason for hiding this comment

ezr-ondrej Jul 16, 2020

Choose a reason for hiding this comment

ezr-ondrej Jul 16, 2020

Choose a reason for hiding this comment

ezr-ondrej left a comment

Choose a reason for hiding this comment

ezr-ondrej commented Jul 17, 2020

ezr-ondrej commented Jul 17, 2020

ezr-ondrej Jul 17, 2020

Choose a reason for hiding this comment

ezr-ondrej left a comment

Choose a reason for hiding this comment

domitea commented Jul 17, 2020

ezr-ondrej left a comment

Choose a reason for hiding this comment

ezr-ondrej left a comment

Choose a reason for hiding this comment

ezr-ondrej commented Jul 18, 2020

ekohl commented Sep 4, 2020

zeha commented Nov 6, 2020 • edited

ekohl commented Nov 6, 2020

zeha commented Nov 6, 2020

zeha commented Nov 6, 2020 •

edited