diff --git a/.expeditor/verify.pipeline.yml b/.expeditor/verify.pipeline.yml index e873f04c..84cb31fd 100644 --- a/.expeditor/verify.pipeline.yml +++ b/.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 \ No newline at end of file diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 00000000..48c96927 --- /dev/null +++ b/.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 + \ No newline at end of file diff --git a/.rubocop.yml b/.rubocop.yml index f0f04590..a384b051 100644 --- a/.rubocop.yml +++ b/.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/**/*' \ No newline at end of file diff --git a/Gemfile b/Gemfile index efe530f0..223f63b9 100644 --- a/Gemfile +++ b/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"]) diff --git a/Rakefile b/Rakefile index b7342fab..c5d98bd6 100644 --- a/Rakefile +++ b/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 diff --git a/bin/chef-zero b/bin/chef-zero index 87c894af..c01ee0b0 100755 --- a/bin/chef-zero +++ b/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 diff --git a/chef-zero.gemspec b/chef-zero.gemspec index d0c0d593..00d4ed23 100644 --- a/chef-zero.gemspec +++ b/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" diff --git a/lib/chef_zero/chef_data/default_creator.rb b/lib/chef_zero/chef_data/default_creator.rb index 3144e3a1..5df07ce8 100644 --- a/lib/chef_zero/chef_data/default_creator.rb +++ b/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 diff --git a/lib/chef_zero/data_store/default_facade.rb b/lib/chef_zero/data_store/default_facade.rb index df6afc66..732dd8a8 100644 --- a/lib/chef_zero/data_store/default_facade.rb +++ b/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 diff --git a/lib/chef_zero/endpoints/actor_endpoint.rb b/lib/chef_zero/endpoints/actor_endpoint.rb index aed6cfc3..0fa7143e 100644 --- a/lib/chef_zero/endpoints/actor_endpoint.rb +++ b/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 diff --git a/lib/chef_zero/endpoints/cookbook_version_endpoint.rb b/lib/chef_zero/endpoints/cookbook_version_endpoint.rb index 6953f204..8239eb07 100644 --- a/lib/chef_zero/endpoints/cookbook_version_endpoint.rb +++ b/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 diff --git a/lib/chef_zero/endpoints/policy_revision_endpoint.rb b/lib/chef_zero/endpoints/policy_revision_endpoint.rb index ee2c6df6..17a56e0a 100644 --- a/lib/chef_zero/endpoints/policy_revision_endpoint.rb +++ b/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,7 +24,7 @@ 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] @@ -32,7 +32,7 @@ def get(request) # 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 diff --git a/lib/chef_zero/rest_base.rb b/lib/chef_zero/rest_base.rb index d735c1e6..04eff17a 100644 --- a/lib/chef_zero/rest_base.rb +++ b/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, diff --git a/lib/chef_zero/rest_request.rb b/lib/chef_zero/rest_request.rb index 2363b11f..572aa183 100644 --- a/lib/chef_zero/rest_request.rb +++ b/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 diff --git a/lib/chef_zero/rspec.rb b/lib/chef_zero/rspec.rb index 9bb15cad..0451ef8b 100644 --- a/lib/chef_zero/rspec.rb +++ b/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 diff --git a/lib/chef_zero/server.rb b/lib/chef_zero/server.rb index 0a0621ed..e0673271 100644 --- a/lib/chef_zero/server.rb +++ b/lib/chef_zero/server.rb @@ -36,7 +36,7 @@ require_relative "data_store/v1_to_v2_adapter" require_relative "data_store/default_facade" require_relative "version" -require "chef_zero/dist.rb" +require "chef_zero/dist" require_relative "endpoints/rest_list_endpoint" require_relative "endpoints/authenticate_user_endpoint" @@ -314,13 +314,12 @@ def start_background(wait = 5) # Start the server in the background @thread = Thread.new do - begin - Thread.current.abort_on_exception = true - @server.start - ensure - @port = nil - @running = false - end + Thread.current.abort_on_exception = true + @server.start + ensure + @port = nil + @running = false + # end end # Do not return until the web server is genuinely started. @@ -656,41 +655,40 @@ def app rest_base_prefix = [] end @app = proc do |env| - begin - prefix = global_endpoint?(env["PATH_INFO"]) ? [] : rest_base_prefix - request = RestRequest.new(env, prefix) - if @on_request_proc - @on_request_proc.call(request) - end - response = nil - if @request_handler - response = @request_handler.call(request) - end - unless response - response = router.call(request) - end - if @on_response_proc - @on_response_proc.call(request, response) - end + prefix = global_endpoint?(env["PATH_INFO"]) ? [] : rest_base_prefix - # Insert Server header - response[1]["Server"] = "chef-zero" + request = RestRequest.new(env, prefix) + if @on_request_proc + @on_request_proc.call(request) + end + response = nil + if @request_handler + response = @request_handler.call(request) + end + unless response + response = router.call(request) + end + if @on_response_proc + @on_response_proc.call(request, response) + end - # Add CORS header - response[1]["Access-Control-Allow-Origin"] = "*" + # Insert Server header + response[1]["Server"] = "chef-zero" - # Puma expects the response to be an array (chunked responses). Since - # we are statically generating data, we won't ever have said chunked - # response, so fake it. - response[-1] = Array(response[-1]) + # Add CORS header + response[1]["Access-Control-Allow-Origin"] = "*" - response - rescue - if options[:log_level] == :debug - STDERR.puts "Request Error: #{$!}" - STDERR.puts $!.backtrace.join("\n") - end + # Puma expects the response to be an array (chunked responses). Since + # we are statically generating data, we won't ever have said chunked + # response, so fake it. + response[-1] = Array(response[-1]) + + response + rescue + if options[:log_level] == :debug + STDERR.puts "Request Error: #{$!}" + STDERR.puts $!.backtrace.join("\n") end end @app diff --git a/lib/chef_zero/socketless_server_map.rb b/lib/chef_zero/socketless_server_map.rb index a52c2f5f..37942087 100644 --- a/lib/chef_zero/socketless_server_map.rb +++ b/lib/chef_zero/socketless_server_map.rb @@ -16,7 +16,6 @@ # limitations under the License. # -require "thread" require "singleton" unless defined?(Singleton) require_relative "dist" diff --git a/lib/chef_zero/solr/query/regexpable_query.rb b/lib/chef_zero/solr/query/regexpable_query.rb index 4cf67484..45e1db4c 100644 --- a/lib/chef_zero/solr/query/regexpable_query.rb +++ b/lib/chef_zero/solr/query/regexpable_query.rb @@ -22,8 +22,8 @@ def matches_values?(values) end DEFAULT_FIELD = "text".freeze - WORD_CHARACTER = "[A-Za-z0-9@._':\-]".freeze - NON_WORD_CHARACTER = "[^A-Za-z0-9@._':\-]".freeze + WORD_CHARACTER = "[A-Za-z0-9@._':-]".freeze + NON_WORD_CHARACTER = "[^A-Za-z0-9@._':-]".freeze end end end diff --git a/lib/chef_zero/solr/query/term.rb b/lib/chef_zero/solr/query/term.rb index f90b750c..8548468d 100644 --- a/lib/chef_zero/solr/query/term.rb +++ b/lib/chef_zero/solr/query/term.rb @@ -22,7 +22,7 @@ def initialize(term) elsif term[index] == "~" raise "~ unsupported" else - if term[index] == '\\' + if term[index] == "\\" index += 1 if index >= term.length raise "Backslash at end of string '#{term}'" diff --git a/lib/chef_zero/solr/solr_parser.rb b/lib/chef_zero/solr/solr_parser.rb index 843a1b52..009bab2c 100644 --- a/lib/chef_zero/solr/solr_parser.rb +++ b/lib/chef_zero/solr/solr_parser.rb @@ -47,7 +47,7 @@ def parse_token # (characters plus backslashed escaped characters) start_index = @index loop do - if @query_string[@index] == '\\' + if @query_string[@index] == "\\" @index += 1 end @index += 1 unless eof? diff --git a/spec/run_oc_pedant.rb b/spec/run_oc_pedant.rb index 6694597e..de04bc46 100644 --- a/spec/run_oc_pedant.rb +++ b/spec/run_oc_pedant.rb @@ -192,7 +192,7 @@ def args_from_env(key) "--skip-status", "--skip=email_case_insensitive", "--skip=nginx_default_error", - "--skip=response_headers" + "--skip=response_headers", ] # The knife tests are very slow and don't give us a lot of extra coverage, diff --git a/spec/search_spec.rb b/spec/search_spec.rb index 72e4b535..0659784a 100644 --- a/spec/search_spec.rb +++ b/spec/search_spec.rb @@ -17,20 +17,20 @@ def search_for(query) end it "handles terms" do - search_for("foo:d").size.should eq(1) + expect(search_for("foo:d").size).to eq(1) end it "handles ranges" do - search_for("foo:[a TO c]").size.should eq(1) + expect(search_for("foo:[a TO c]").size).to eq(1) end it "handles -" do - search_for("-foo:a").size.should eq(1) + expect(search_for("-foo:a").size).to eq(1) end it "handles wildcard ranges" do - search_for("foo:[* TO c]").size.should eq(1) - search_for("foo:[c TO *]").size.should eq(1) - search_for("foo:[* TO *]").size.should eq(2) + expect(search_for("foo:[* TO c]").size).to eq(1) + expect(search_for("foo:[c TO *]").size).to eq(1) + expect(search_for("foo:[* TO *]").size).to eq(2) end end diff --git a/spec/support/oc_pedant.rb b/spec/support/oc_pedant.rb index 803d43e0..05257f78 100644 --- a/spec/support/oc_pedant.rb +++ b/spec/support/oc_pedant.rb @@ -71,8 +71,8 @@ key = "spec/support/stickywicket.pem" org(name: "pedant-testorg", - create_me: !ENV["CHEF_FS"], - validator_key: key) + create_me: !ENV["CHEF_FS"], + validator_key: key) internal_account_url chef_server delete_org true