client-platform-engineering icon indicating copy to clipboard operation
client-platform-engineering copied to clipboard

Add support for dynamically repairing hubcli

Open w0de opened this issue 5 years ago • 12 comments

Recently, I challenged a support engineer to break our management tools on his Mac.

He succeeded - and the only tool that could not be easily repaired from a basis of MDM enrollment was, surprisingly, hubcli. Workspace One continues to function without it.

Since this cookbook already supports installing the agent, and is smart about the existence of hubcli, I think it would be neat if it provided something like this:

node['cpe_workspaceone']['remediate_missing_hubcli'] = true

If true, cpe_workspaceone would reinstall the package. This also leaves the door open for sophisticated health checks.

w0de avatar Jan 24 '20 22:01 w0de

Implementation could be as simple as force-forgetting the package receipt for the agent if hubcli is missing.

w0de avatar Jan 24 '20 23:01 w0de

We should probably just do this automatically. If an admin wants to install it, chances are they want a desired state (for it to actually work).

What detection logic do we need?

erikng avatar Jan 25 '20 03:01 erikng

Just if hubcli is missing?

erikng avatar Jan 25 '20 03:01 erikng

I think as simple as ws1_forget_package! if ws1_hubcli_missing works.

But I'm not sure if I want this to happen all the time. I can be timid about installing packages in the background - what if there's a logic error and we loop forgetting/installing the package?

w0de avatar Jan 25 '20 16:01 w0de

Also while we're here I think the ws1_ should be refactored to be a class. I'll open a separate issue/PR if you're interested. Ie:

class Chef
  class Node
    def ws1
      @ws1 ||= WorkspaceOneUtils.new(self)
    end

    # or
    def workspace_one
       ws1
    end

    class WorkspaceOneUtils
      def new(node_object)
        @node = node_object
      end

      def node
        @node
      end

      def forget_package!
        ...
      end
    end
  end
end

node.ws1.forget_package!

w0de avatar Jan 25 '20 20:01 w0de

I think if ws1 becomes a class, it should be outside of node.

natewalck avatar Jan 25 '20 20:01 natewalck

Yes @natewalck I was just saying to myself that I'm making Node a god class.

Going down the route, one might end up accidentally managing machine state through the node object, instead of through chef resources.

w0de avatar Jan 25 '20 21:01 w0de

Another fun discussion is if we should ever be overloading node with custom classes or if we should break it all out into a CPE class. :D

natewalck avatar Jan 25 '20 21:01 natewalck

I think hewing to an established chef patten would be ideal (I feel that cpe_profiles and cpe_launchd have flawed designs).

Starting here: https://blog.chef.io/writing-libraries-in-chef-cookbooks/ -

# Not a fan of "CPE" as a module name though
module CPE
  module WorkspaceOne

    include Chef::Mixin::ShellOut

    def hubcli
      cmd = shell_out!("#{hubcli_path}", {:returns => [0]})
    end
  end
end

# Not sure of resource name
Chef::Resource::cpe_workspaceone.send(:include, CPE::WorkspaceOne)

w0de avatar Jan 25 '20 21:01 w0de

I agree. One thing that we need to test out is can a class access node by default? I believe that answer is no...so if we want to use node data for anything, we'll have to figure out how to get it from the class. I'm sure there must be some way to do this via subclassing or some such.

natewalck avatar Jan 25 '20 23:01 natewalck

This is something that I'm playing with for an internal recipe:

# libraries/eng_helpers.rb
include Chef::Mixin::ShellOut

module EngineeringHelpers
  def self.sdk_path
    shell_out!("/usr/bin/xcrun --show-sdk-path").stdout.strip
  end
end

class Chef
  class Resource
    def eng_helpers
      EngineeringHelpers
    end
  end
  class Recipe
    def eng_helpers
      EngineeringHelpers
    end
  end
end
# recipes/default.rb
eng_helpers.sdk_path

w0de avatar Jan 30 '20 20:01 w0de

@natewalck can you put the memoizing feature in this cookbook for our next sprint so we can get some traction on this diff?

erikng avatar Feb 26 '20 20:02 erikng