Jfm/chef19 cookstyle update

.expeditor/verify.pipeline.yml

@@ -27,3 +27,25 @@ steps:
           - FORCE_FFI_YAJL=ext
           - EXPIRE_CACHE=true
           - CHEF_LICENSE=accept-no-persist
+
+- label: run-lint-and-specs-ruby-3.4
+  command:
+    - .expeditor/run_linux_tests.sh rake spec pedant style
+  expeditor:
+    executor:
+      docker:
+        image: ruby:3.4
+
+- label: run-specs-windows-ruby-3.4
+  command:
+    - .expeditor/run_windows_tests.ps1
+  expeditor:
+    executor:
+      docker:
+        host_os: windows
+        shell: ["powershell", "-Command"]
+        image: rubydistros/windows-2019:3.4
+        environment:
+          - FORCE_FFI_YAJL=ext
+          - EXPIRE_CACHE=true
+          - CHEF_LICENSE=accept-no-persist

.github/workflows/lint.yml

@@ -0,0 +1,27 @@
+---
+name: lint
+
+on:
+  pull_request:
+  push:
+    branches:
+      - main
+
+concurrency:
+  group: lint-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  chefstyle:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: 3.1
+          bundler-cache: false
+      - uses: r7kamura/rubocop-problem-matchers-action@v1  # this shows the failures in the PR
+      - run: |
+          bundle install --jobs 4 --retry 3
+          cookstyle --chefstyle -c .rubocop.yml
+

.rubocop.yml

@@ -1,5 +1,52 @@
 # Feel free to correct anything in this file
 
+AllCops:
+  TargetRubyVersion: 3.1
+  Exclude:
+    - "spec/data/**/*"
+    - "habitat/**/*"
+    - "vendor/**/*"
+Security/Eval:
+  Enabled: false
+Lint/UselessAssignment:
+  Enabled: false
+Lint/DeprecatedClassMethods:
+  Enabled: false
+Lint/AmbiguousRegexpLiteral:
+  Enabled: false
+Lint/AssignmentInCondition:
+  Enabled: false
+Lint/AmbiguousBlockAssociation:
+  Enabled: false
+Layout/EndOfLine:
+   Enabled: false
+Lint/ShadowingOuterLocalVariable:
+  Enabled: false
+Lint/IneffectiveAccessModifier:
+  Enabled: false
+Lint/InterpolationCheck:
+  Enabled: true
+  Exclude:
+    - 'spec/unit/property_spec.rb'
+    - 'spec/functional/shell_spec.rb'
+Lint/DeprecatedConstants:
+  Enabled: true
+  Exclude:
+    - lib/chef/node/attribute.rb # false alarms
 Lint/UselessAccessModifier:
   Exclude:
     - 'lib/chef_zero/chef_data/acl_path.rb'
+
+# This cop shouldn't alert on the helper / specs itself
+Chef/Ruby/LegacyPowershellOutMethods:
+  Exclude:
+    - 'lib/chef/mixin/powershell_out.rb'
+    - 'spec/functional/mixin/powershell_out_spec.rb'
+    - 'spec/unit/mixin/powershell_out_spec.rb'
+    - 'lib/chef/resource/windows_feature_powershell.rb' # https://github.com/chef/chef/issues/10927
+    - 'lib/chef/provider/package/powershell.rb' # https://github.com/chef/chef/issues/10926
+
+# set additional paths
+Chef/Ruby/UnlessDefinedRequire:
+  Include:
+    - 'lib/**/*'

Gemfile

@@ -11,11 +11,14 @@ end
 gem "ffi", ">= 1.15.5", "< 1.17.0"
 
 group :development, :test do
-  gem "chefstyle"
   gem "rake"
   gem "rspec", "~> 3.0"
 end
 
+group :style do
+  gem "cookstyle", "~> 8.1"
+end
+
 if ENV["GEMFILE_MOD"]
   puts "GEMFILE_MOD: #{ENV["GEMFILE_MOD"]}"
   instance_eval(ENV["GEMFILE_MOD"])

Rakefile

@@ -44,20 +44,25 @@ task :chef_spec do
   system("cd #{gem_path} && rspec spec/integration")
 end
 
-task :berkshelf_spec do
-  gem_path = Bundler.environment.specs["berkshelf"].first.full_gem_path
-  system("cd #{gem_path} && thor spec:ci")
-end
-
-begin
-  require "chefstyle"
+desc "Check Linting and code style."
+task :style do
   require "rubocop/rake_task"
-  desc "Run Chefstyle tests"
-  RuboCop::RakeTask.new(:style) do |task|
-    task.options += ["--display-cop-names", "--no-color"]
+  require "cookstyle/chefstyle"
+
+  if RbConfig::CONFIG["host_os"] =~ /mswin|mingw|cygwin/
+    # Windows-specific command, rubocop erroneously reports the CRLF in each file which is removed when your PR is uploaeded to GitHub.
+    # This is a workaround to ignore the CRLF from the files before running cookstyle.
+    sh "cookstyle --chefstyle -c .rubocop.yml --except Layout/EndOfLine"
+  else
+    sh "cookstyle --chefstyle -c .rubocop.yml"
   end
 rescue LoadError
-  puts "chefstyle gem is not installed. bundle install first to make sure all dependencies are installed."
+  puts "Rubocop or Cookstyle gems are not installed. bundle install first to make sure all dependencies are installed."
+end
+
+task :berkshelf_spec do
+  gem_path = Bundler.environment.specs["berkshelf"].first.full_gem_path
+  system("cd #{gem_path} && thor spec:ci")
 end
 
 begin

bin/chef-zero

@@ -20,7 +20,7 @@ def parse_port(port)
     if b
       array = array.concat(a.to_i.upto(b.to_i).to_a)
     else
-      array = array.concat([a.to_i])
+      array = array.push(a.to_i)
     end
   end
   array

chef-zero.gemspec

@@ -11,7 +11,7 @@ Gem::Specification.new do |s|
   s.homepage = "https://github.com/chef/chef-zero"
   s.license = "Apache-2.0"
 
-  s.required_ruby_version = ">= 3.0"
+  s.required_ruby_version = ">= 3.1"
 
   # Note: 7.1.0 does not defaults its cache_format_version to 7.1 but 6.1 instead which gives deprecation warnings
   # Remove the version constraint when we can upgrade to 7.1.1 post stable release of Activesupport 7.1
@@ -25,6 +25,8 @@ Gem::Specification.new do |s|
   s.add_dependency "rackup", "~> 2.2", ">= 2.2.1"
   s.add_dependency "webrick"
 
+  s.add_development_dependency "yard"
+
   s.bindir       = "bin"
   s.executables  = ["chef-zero"]
   s.require_path = "lib"

lib/chef_zero/chef_data/default_creator.rb

@@ -422,15 +422,15 @@ def default_acl(acl_path, acl = {})
           acl[perm] ||= {}
           acl[perm]["users"] = owners[:users]
           acl[perm]["clients"] = owners[:clients]
-          acl[perm]["groups"] ||= begin
-            # When we create containers, we don't merge groups (not sure why).
-            if acl_path[0] == "organizations" && acl_path[3] == "containers"
-              []
-            else
-              container_acl ||= get_container_acl(acl_path) || {}
-              (container_acl[perm] ? container_acl[perm]["groups"] : []) || []
-            end
-          end
+
+          # When we create containers, we don't merge groups (not sure why).
+          acl[perm]["groups"] ||= if acl_path[0] == "organizations" && acl_path[3] == "containers"
+                                    []
+                                  else
+                                    container_acl ||= get_container_acl(acl_path) || {}
+                                    (container_acl[perm] ? container_acl[perm]["groups"] : []) || []
+                                  end
+
           acl[perm]["actors"] = acl[perm]["clients"] + acl[perm]["users"]
         end
         acl

lib/chef_zero/data_store/default_facade.rb

@@ -127,11 +127,7 @@ def list(path)
             real_results
           end
         rescue DataNotFoundError
-          if default_results
-            default_results
-          else
-            raise
-          end
+          default_results || raise
         end
       end
 

lib/chef_zero/endpoints/actor_endpoint.rb

@@ -23,10 +23,10 @@ def delete(request)
 
         if request.rest_path[0] == "users"
           list_data(request, [ "organizations" ]).each do |org|
-            begin
-              delete_data(request, [ "organizations", org, "users", request.rest_path[1] ], :data_store_exceptions)
-            rescue DataStore::DataNotFoundError
-            end
+
+            delete_data(request, [ "organizations", org, "users", request.rest_path[1] ], :data_store_exceptions)
+          rescue DataStore::DataNotFoundError
+
           end
         end
 
@@ -82,7 +82,7 @@ def put(request)
           end
 
           if client?(request)
-            response["private_key"] = private_key ? private_key : false
+            response["private_key"] == private_key ? private_key : false
           else
             response["private_key"] = private_key if private_key
             response.delete("public_key") unless updating_public_key

lib/chef_zero/endpoints/cookbook_version_endpoint.rb

@@ -98,23 +98,24 @@ def hoover_unused_checksums(deleted_checksums, request)
           end
           cookbooks.each do |cookbook_name|
             # as below, this can be racy.
-            begin
-              data_store.list(request.rest_path[0..1] + [cookbook_type, cookbook_name]).each do |version|
-                cookbook = data_store.get(request.rest_path[0..1] + [cookbook_type, cookbook_name, version], request)
-                deleted_checksums -= get_checksums(cookbook)
-              end
-            rescue ChefZero::DataStore::DataNotFoundError
+
+            data_store.list(request.rest_path[0..1] + [cookbook_type, cookbook_name]).each do |version|
+              cookbook = data_store.get(request.rest_path[0..1] + [cookbook_type, cookbook_name, version], request)
+              deleted_checksums -= get_checksums(cookbook)
             end
+          rescue ChefZero::DataStore::DataNotFoundError
+            # this space intentionally left blank
+
           end
         end
         deleted_checksums.each do |checksum|
           # There can be a race here if multiple cookbooks are uploading.
           # This deals with an exception on delete, but things can still get deleted
           # that shouldn't be.
-          begin
-            delete_data(request, request.rest_path[0..1] + ["file_store", "checksums", checksum], :data_store_exceptions)
-          rescue ChefZero::DataStore::DataNotFoundError
-          end
+
+          delete_data(request, request.rest_path[0..1] + ["file_store", "checksums", checksum], :data_store_exceptions)
+        rescue ChefZero::DataStore::DataNotFoundError
+
         end
       end
 

lib/chef_zero/endpoints/policy_revision_endpoint.rb

@@ -7,7 +7,7 @@ class PolicyRevisionEndpoint < RestBase
       # GET /organizations/ORG/policies/NAME/revisions/REVISION
       def get(request)
         data = parse_json(get_data(request))
-        
+
         # need to add another field in the response called 'policy_group_list'
         # example response
         #     {
@@ -24,15 +24,15 @@ def get(request)
         #       },
         #       "policy_group_list": ["some_policy_group"]
         #     }
-        data[:policy_group_list] = Array.new
+        data[:policy_group_list] = []
 
         # extracting policy name and revision
         request_policy_name = request.rest_path[3]
         request_policy_revision = request.rest_path[5]
 
         # updating the request to fetch the policy group list
         request.rest_path[2] = "policy_groups"
-        request.rest_path = request.rest_path.slice(0,3)
+        request.rest_path = request.rest_path.slice(0, 3)
 
         list_data(request).each do |group_name|
           group_path = request.rest_path + [group_name]
@@ -49,7 +49,7 @@ def get(request)
             end
           end
         end
-        
+
         data = ChefData::DataNormalizer.normalize_policy(data, request_policy_name, request_policy_revision)
         json_response(200, data)
       end

lib/chef_zero/rest_base.rb

@@ -29,9 +29,7 @@ def check_api_version(request)
           "max_api_version" => MAX_API_VERSION,
         }
 
-        return json_response(406,
-          response,
-          request_version: version, response_version: -1)
+        json_response(406, response, request_version: version, response_version: -1)
       end
     rescue ArgumentError
       json_response(406,

lib/chef_zero/rest_request.rb

@@ -13,11 +13,7 @@ def initialize(env, rest_base_prefix = [])
 
     def base_uri
       # Load balancer awareness
-      if env["HTTP_X_FORWARDED_PROTO"]
-        scheme = env["HTTP_X_FORWARDED_PROTO"]
-      else
-        scheme = env["rack.url_scheme"]
-      end
+      scheme = env["HTTP_X_FORWARDED_PROTO"] || env["rack.url_scheme"]
       @base_uri ||= "#{scheme}://#{env["HTTP_HOST"]}#{env["SCRIPT_NAME"]}"
     end
 

lib/chef_zero/rspec.rb

@@ -345,9 +345,7 @@ def current_org
       def with_object_path(object_path)
         old_object_path = @current_object_path
         @current_object_path = object_path
-        begin
-          yield if block_given?
-        end
+        yield if block_given?
         @current_object_path = old_object_path
       end
     end