diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index b69b2939..640d7f2b 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,37 +1,43 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2021-04-22 16:30:35 +0200 using RuboCop version 0.50.0. +# on 2021-04-23 16:47:47 +0200 using RuboCop version 0.50.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 1 +# Offense count: 3 Lint/UselessAssignment: Exclude: - 'lib/modulesync.rb' # Offense count: 10 Metrics/AbcSize: - Max: 67 + Max: 59 # Offense count: 2 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 128 + Max: 125 -# Offense count: 3 +# Offense count: 4 Metrics/CyclomaticComplexity: - Max: 12 + Max: 13 + +# Offense count: 3 +# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. +# URISchemes: http, https +Metrics/LineLength: + Max: 186 -# Offense count: 13 +# Offense count: 17 # Configuration parameters: CountComments. Metrics/MethodLength: - Max: 36 + Max: 32 # Offense count: 3 Metrics/PerceivedComplexity: - Max: 13 + Max: 14 # Offense count: 8 Style/Documentation: @@ -49,3 +55,12 @@ Style/Documentation: Style/EachWithObject: Exclude: - 'lib/modulesync/util.rb' + +# Offense count: 2 +# Cop supports --auto-correct. +# Configuration parameters: EnforcedStyleForMultiline, SupportedStylesForMultiline. +# SupportedStylesForMultiline: comma, consistent_comma, no_comma +Style/TrailingCommaInArguments: + Exclude: + - 'lib/modulesync/git_service/base.rb' + - 'lib/modulesync/source_code.rb' diff --git a/features/update.feature b/features/update.feature index 653eaf48..f264bee9 100644 --- a/features/update.feature +++ b/features/update.feature @@ -713,28 +713,6 @@ Feature: update And the puppet module "puppet-test" from "fakenamespace" should have only 1 commit made by "Aruba" And the puppet module "puppet-test" from "fakenamespace" should have 1 commit made by "Aruba" in branch "test" - Scenario: Creating a GitHub PR with an update - Given a basic setup with a puppet module "puppet-test" from "fakenamespace" - And a directory named "moduleroot" - And I set the environment variables to: - | variable | value | - | GITHUB_TOKEN | foobar | - When I run `msync update --noop --branch managed_update --pr` - Then the output should contain "Would submit PR " - And the exit status should be 0 - And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba" - - Scenario: Creating a GitLab MR with an update - Given a basic setup with a puppet module "puppet-test" from "fakenamespace" - And a directory named "moduleroot" - And I set the environment variables to: - | variable | value | - | GITLAB_TOKEN | foobar | - When I run `msync update --noop --branch managed_update --pr` - Then the output should contain "Would submit MR " - And the exit status should be 0 - And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba" - Scenario: Repository with a default branch other than master Given a basic setup with a puppet module "puppet-test" from "fakenamespace" And the puppet module "puppet-test" from "fakenamespace" has the default branch named "develop" diff --git a/features/update/pull_request.feature b/features/update/pull_request.feature new file mode 100644 index 00000000..c4762a5d --- /dev/null +++ b/features/update/pull_request.feature @@ -0,0 +1,184 @@ +Feature: Create a pull-request/merge-request after update + + Scenario: Run update in no-op mode and ask for GitHub PR + Given a basic setup with a puppet module "puppet-test" from "fakenamespace" + And a file named "managed_modules.yml" with: + """ + --- + puppet-test: + github: {} + """ + And I set the environment variables to: + | variable | value | + | GITHUB_TOKEN | foobar | + | GITHUB_BASE_URL | https://github.example.com | + And a file named "config_defaults.yml" with: + """ + --- + test: + name: aruba + """ + And a file named "moduleroot/test.erb" with: + """ + <%= @configs['name'] %> + """ + When I run `msync update --noop --branch managed_update --pr` + Then the output should contain "Would submit PR " + And the exit status should be 0 + And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba" + + Scenario: Run update in no-op mode and ask for GitLab MR + Given a basic setup with a puppet module "puppet-test" from "fakenamespace" + And a file named "managed_modules.yml" with: + """ + --- + puppet-test: + gitlab: { + base_url: 'https://gitlab.example.com' + } + """ + And I set the environment variables to: + | variable | value | + | GITLAB_TOKEN | foobar | + And a file named "config_defaults.yml" with: + """ + --- + test: + name: aruba + """ + And a file named "moduleroot/test.erb" with: + """ + <%= @configs['name'] %> + """ + When I run `msync update --noop --branch managed_update --pr` + Then the output should contain "Would submit MR " + And the exit status should be 0 + And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba" + + Scenario: Run update without changes in no-op mode and ask for GitLab MR + Given a basic setup with a puppet module "puppet-test" from "fakenamespace" + And a directory named "moduleroot" + And a file named "managed_modules.yml" with: + """ + --- + puppet-test: + gitlab: { + base_url: 'https://gitlab.example.com' + } + """ + And I set the environment variables to: + | variable | value | + | GITLAB_TOKEN | foobar | + When I run `msync update --noop --branch managed_update --pr` + Then the output should not contain "Would submit MR " + And the exit status should be 0 + And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba" + + Scenario: Ask for PR without credentials + Given a basic setup with a puppet module "puppet-test" from "fakenamespace" + And a file named "managed_modules.yml" with: + """ + --- + puppet-test: + gitlab: { + base_url: https://gitlab.example.com + } + """ + And a file named "config_defaults.yml" with: + """ + --- + test: + name: aruba + """ + And a file named "moduleroot/test.erb" with: + """ + <%= @configs['name'] %> + """ + When I run `msync update --noop --pr` + Then the stderr should contain "A token is required to use services from gitlab" + And the exit status should be 1 + And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba" + + Scenario: Ask for PR/MR with modules from GitHub and from GitLab + Given a basic setup with a puppet module "puppet-github" from "fakenamespace" + And a basic setup with a puppet module "puppet-gitlab" from "fakenamespace" + And a file named "managed_modules.yml" with: + """ + --- + puppet-github: + github: + base_url: https://github.example.com + token: 'secret' + puppet-gitlab: + gitlab: + base_url: https://gitlab.example.com + token: 'secret' + """ + And a file named "config_defaults.yml" with: + """ + --- + test: + name: aruba + """ + And a file named "moduleroot/test.erb" with: + """ + <%= @configs['name'] %> + """ + When I run `msync update --noop --branch managed_update --pr` + Then the exit status should be 0 + And the output should contain "Would submit PR " + And the output should contain "Would submit MR " + And the puppet module "puppet-github" from "fakenamespace" should have no commits made by "Aruba" + And the puppet module "puppet-gitlab" from "fakenamespace" should have no commits made by "Aruba" + + Scenario: Ask for PR with same source and target branch + Given a basic setup with a puppet module "puppet-test" from "fakenamespace" + And a file named "managed_modules.yml" with: + """ + --- + puppet-test: + gitlab: + token: 'secret' + base_url: 'https://gitlab.example.com' + """ + And a file named "config_defaults.yml" with: + """ + --- + test: + name: aruba + """ + And a file named "moduleroot/test.erb" with: + """ + <%= @configs['name'] %> + """ + When I run `msync update --noop --branch managed_update --pr --pr-target-branch managed_update` + Then the stderr should contain "Unable to open a pull request with the same source and target branch: 'managed_update'" + And the exit status should be 1 + And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba" + + Scenario: Ask for PR with the default branch as source and target + Given a basic setup with a puppet module "puppet-test" from "fakenamespace" + And the puppet module "puppet-test" from "fakenamespace" has the default branch named "custom_default_branch" + And a file named "managed_modules.yml" with: + """ + --- + puppet-test: + github: + token: 'secret' + base_url: 'https://gitlab.example.com' + """ + And a file named "config_defaults.yml" with: + """ + --- + test: + name: aruba + """ + And a file named "moduleroot/test.erb" with: + """ + <%= @configs['name'] %> + """ + And a directory named "moduleroot" + When I run `msync update --noop --pr` + Then the stderr should contain "Unable to open a pull request with the same source and target branch: 'custom_default_branch'" + And the exit status should be 1 + And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba" diff --git a/lib/modulesync.rb b/lib/modulesync.rb index 25baa1e4..0fa29971 100644 --- a/lib/modulesync.rb +++ b/lib/modulesync.rb @@ -129,9 +129,8 @@ def self.manage_module(puppet_module, module_files, defaults) if options[:noop] puts "Using no-op. Files in '#{puppet_module.given_name}' may be changed but will not be committed." - puppet_module.repository.show_changes(options) - options[:pr] && \ - pr(puppet_module).manage(puppet_module.repository_namespace, puppet_module.repository_name, options) + changed = puppet_module.repository.show_changes(options) + changed && options[:pr] && puppet_module.open_pull_request elsif !options[:offline] pushed = puppet_module.repository.submit_changes(files_to_manage, options) # Only bump/tag if pushing didn't fail (i.e. there were changes) @@ -139,8 +138,7 @@ def self.manage_module(puppet_module, module_files, defaults) new = puppet_module.bump(options[:message], options[:changelog]) puppet_module.repository.tag(new, options[:tag_pattern]) if options[:tag] end - pushed && options[:pr] && \ - pr(puppet_module).manage(puppet_module.repository_namespace, puppet_module.repository_name, options) + pushed && options[:pr] && puppet_module.open_pull_request end end @@ -157,15 +155,6 @@ def self.update(cli_options) @options = config_defaults.merge(cli_options) defaults = Util.parse_config(config_path(CONF_FILE, options)) - if options[:pr] - unless options[:branch] - $stderr.puts 'A branch must be specified with --branch to use --pr!' - raise - end - - @pr = create_pr_manager if options[:pr] - end - local_template_dir = config_path(MODULE_FILES_DIR, options) local_files = find_template_files(local_template_dir) module_files = relative_names(local_files, local_template_dir) @@ -189,40 +178,4 @@ def self.update(cli_options) end exit 1 if errors && options[:fail_on_warnings] end - - def self.pr(puppet_module) - module_options = puppet_module.options - github_conf = module_options[:github] - gitlab_conf = module_options[:gitlab] - - if !github_conf.nil? - base_url = github_conf[:base_url] || ENV.fetch('GITHUB_BASE_URL', 'https://api.github.com') - require 'modulesync/pr/github' - ModuleSync::PR::GitHub.new(github_conf[:token], base_url) - elsif !gitlab_conf.nil? - base_url = gitlab_conf[:base_url] || ENV.fetch('GITLAB_BASE_URL', 'https://gitlab.com/api/v4') - require 'modulesync/pr/gitlab' - ModuleSync::PR::GitLab.new(gitlab_conf[:token], base_url) - elsif @pr.nil? - $stderr.puts 'No GitHub or GitLab token specified for --pr!' - raise - else - @pr - end - end - - def self.create_pr_manager - github_token = ENV.fetch('GITHUB_TOKEN', '') - gitlab_token = ENV.fetch('GITLAB_TOKEN', '') - - if !github_token.empty? - require 'modulesync/pr/github' - ModuleSync::PR::GitHub.new(github_token, ENV.fetch('GITHUB_BASE_URL', 'https://api.github.com')) - elsif !gitlab_token.empty? - require 'modulesync/pr/gitlab' - ModuleSync::PR::GitLab.new(gitlab_token, ENV.fetch('GITLAB_BASE_URL', 'https://gitlab.com/api/v4')) - else - warn '--pr specified without environment variables GITHUB_TOKEN or GITLAB_TOKEN' - end - end end diff --git a/lib/modulesync/cli.rb b/lib/modulesync/cli.rb index bd733e67..9a90e52b 100644 --- a/lib/modulesync/cli.rb +++ b/lib/modulesync/cli.rb @@ -104,7 +104,7 @@ class Base < Thor :default => CLI.defaults[:pr_labels] || [] option :pr_target_branch, :desc => 'Target branch for the pull/merge request', - :default => CLI.defaults[:pr_target_branch] || 'master' + :default => CLI.defaults[:pr_target_branch] option :offline, :type => :boolean, :desc => 'Do not run any Git commands. Allows the user to manage Git outside of ModuleSync.', diff --git a/lib/modulesync/git_service.rb b/lib/modulesync/git_service.rb new file mode 100644 index 00000000..df606bc9 --- /dev/null +++ b/lib/modulesync/git_service.rb @@ -0,0 +1,130 @@ +module ModuleSync + class Error < StandardError; end + + # Namespace for Git service classes (ie. GitHub, GitLab) + module GitService + class MissingCredentialsError < Error; end + + def self.configuration_for(sourcecode:) + type = type_for(sourcecode: sourcecode) + + { + type: type, + endpoint: endpoint_for(sourcecode: sourcecode, type: type), + token: token_for(sourcecode: sourcecode, type: type), + } + end + + # This method attempts to guess git service's type (ie. gitlab or github) + # It process in this order + # 1. use module specific configuration entry (ie. a specific entry named `gitlab` or `github`) + # 2. guess using remote url (ie. looking for `github` or `gitlab` string) + # 3. use environment variables (ie. check if GITHUB_TOKEN or GITLAB_TOKEN is set) + # 4. fail + def self.type_for(sourcecode:) + return :github unless sourcecode.options[:github].nil? + return :gitlab unless sourcecode.options[:gitlab].nil? + return :github if sourcecode.repository_remote.include? 'github' + return :gitlab if sourcecode.repository_remote.include? 'gitlab' + + if ENV['GITLAB_TOKEN'].nil? && ENV['GITHUB_TOKEN'].nil? + raise ModuleSync::Error, <<~MESSAGE + Unable to guess Git service type without GITLAB_TOKEN or GITHUB_TOKEN sets. + MESSAGE + end + + unless ENV['GITLAB_TOKEN'].nil? || ENV['GITHUB_TOKEN'].nil? + raise ModuleSync::Error, <<~MESSAGE + Unable to guess Git service type with both GITLAB_TOKEN and GITHUB_TOKEN sets. + + Please set the wanted one in configuration (ie. add `gitlab:` or `github:` key) + MESSAGE + end + + return :github unless ENV['GITHUB_TOKEN'].nil? + return :gitlab unless ENV['GITLAB_TOKEN'].nil? + + raise NotImplementedError + end + + # This method attempts to find git service's endpoint based on sourcecode and type + # It process in this order + # 1. use module specific configuration (ie. `base_url`) + # 2. use environment variable dependending on type (e.g. GITLAB_BASE_URL) + # 3. guess using the git remote url + # 4. fail + def self.endpoint_for(sourcecode:, type:) + endpoint = sourcecode.options.dig(type, :base_url) + + endpoint ||= case type + when :github + ENV['GITHUB_BASE_URL'] + when :gitlab + ENV['GITLAB_BASE_URL'] + end + + endpoint ||= guess_endpoint_from(remote: sourcecode.repository_remote, type: type) + + raise NotImplementedError, <<~MESSAGE if endpoint.nil? + Unable to guess endpoint for remote: '#{sourcecode.repository_remote}' + Please provide `base_url` option in configuration file + MESSAGE + + endpoint + end + + # This method attempts to guess the git service endpoint based on remote and type + def self.guess_endpoint_from(remote:, type:) + hostname = extract_hostname(remote) + return nil if hostname.nil? + + endpoint = "https://#{hostname}" + endpoint += '/api/v4' if type == :gitlab + endpoint + end + + # This method attempts to find the token associated to provided sourcecode and type + # It process in this order: + # 1. use module specific configuration (ie. `token`) + # 2. use environment variable depending on type (e.g. GITLAB_TOKEN) + # 3. fail + def self.token_for(sourcecode:, type:) + token = sourcecode.options.dig(type, :token) + + token ||= case type + when :github + ENV['GITHUB_TOKEN'] + when :gitlab + ENV['GITLAB_TOKEN'] + end + + token + end + + # This method extracts hostname from URL like: + # + # - ssh://[user@]host.xz[:port]/path/to/repo.git/ + # - git://host.xz[:port]/path/to/repo.git/ + # - [user@]host.xz:path/to/repo.git/ + # - http[s]://host.xz[:port]/path/to/repo.git/ + # - ftp[s]://host.xz[:port]/path/to/repo.git/ + # + # Returns nil if + # - /path/to/repo.git/ + # - file:///path/to/repo.git/ + # - any invalid URL + def self.extract_hostname(url) + #pattern = /^((?.*)@)*(?.*):(?[a-zA-Z].*)*$/ + return nil if url.start_with?('/') || url.start_with?('file://') # local path (e.g. file:///path/to/repo) + + unless url.start_with?(/[a-z]+:\/\//) # SSH notation does not contain protocol (e.g. user@server:path/to/repo/) + pattern = /^(.*@)?(?[\w|.]*):(.*)$/ # SSH path (e.g. user@server:repo) + return url.match(pattern)[:hostname] if url.match?(pattern) + end + + return URI.parse(url).host + rescue URI::InvalidURIError => e + return nil + end + end +end diff --git a/lib/modulesync/git_service/base.rb b/lib/modulesync/git_service/base.rb new file mode 100644 index 00000000..154e00a6 --- /dev/null +++ b/lib/modulesync/git_service/base.rb @@ -0,0 +1,30 @@ +module ModuleSync + module GitService + # Generic class for git services + class Base + def open_pull_request(repo_path:, namespace:, title:, message:, source_branch:, target_branch:, labels:, noop:) # rubocop:disable Metrics/ParameterLists, Metrics/LineLength + unless source_branch != target_branch + raise ModuleSync::Error, + "Unable to open a pull request with the same source and target branch: '#{source_branch}'" + end + + _open_pull_request( + repo_path: repo_path, + namespace: namespace, + title: title, + message: message, + source_branch: source_branch, + target_branch: target_branch, + labels: labels, + noop: noop, + ) + end + + protected + + def _open_pull_request(repo_path:, namespace:, title:, message:, source_branch:, target_branch:, labels:, noop:) # rubocop:disable Metrics/ParameterLists, Metrics/LineLength + raise NotImplementedError + end + end + end +end diff --git a/lib/modulesync/git_service/factory.rb b/lib/modulesync/git_service/factory.rb new file mode 100644 index 00000000..263ad842 --- /dev/null +++ b/lib/modulesync/git_service/factory.rb @@ -0,0 +1,23 @@ +module ModuleSync + module GitService + module Factory + def self.instantiate(type:, endpoint:, token:) + raise MissingCredentialsError, <<~MESSAGE if token.nil? + A token is required to use services from #{type}: + Please set environment variable: "#{type.upcase}_TOKEN" or set the token entry in module options. + MESSAGE + + case type + when :github + require 'modulesync/git_service/github' + ModuleSync::GitService::GitHub.new(token, endpoint) + when :gitlab + require 'modulesync/git_service/gitlab' + ModuleSync::GitService::GitLab.new(token, endpoint) + else + raise NotImplementedError, "Unable to manage a PR/MR for Git service: '#{type}'" + end + end + end + end +end diff --git a/lib/modulesync/pr/github.rb b/lib/modulesync/git_service/github.rb similarity index 53% rename from lib/modulesync/pr/github.rb rename to lib/modulesync/git_service/github.rb index eaffd6c9..eef84c87 100644 --- a/lib/modulesync/pr/github.rb +++ b/lib/modulesync/git_service/github.rb @@ -1,11 +1,12 @@ +require 'modulesync/git_service' +require 'modulesync/git_service/base' require 'octokit' -require 'modulesync/util' module ModuleSync - module PR + module GitService # GitHub creates and manages pull requests on github.com or GitHub # Enterprise installations. - class GitHub + class GitHub < Base def initialize(token, endpoint) Octokit.configure do |c| c.api_endpoint = endpoint @@ -13,16 +14,15 @@ def initialize(token, endpoint) @api = Octokit::Client.new(:access_token => token) end - def manage(namespace, module_name, options) - repo_path = File.join(namespace, module_name) - branch = options[:remote_branch] || options[:branch] - head = "#{namespace}:#{branch}" - target_branch = options[:pr_target_branch] || 'master' + private - if options[:noop] + def _open_pull_request(repo_path:, namespace:, title:, message:, source_branch:, target_branch:, labels:, noop:) # rubocop:disable Metrics/ParameterLists, Metrics/LineLength + head = "#{namespace}:#{source_branch}" + + if noop $stdout.puts \ - "Using no-op. Would submit PR '#{options[:pr_title]}' to #{repo_path} " \ - "- merges #{branch} into #{target_branch}" + "Using no-op. Would submit PR '#{title}' to '#{repo_path}' " \ + "- merges '#{source_branch}' into '#{target_branch}'" return end @@ -32,25 +32,24 @@ def manage(namespace, module_name, options) :head => head) unless pull_requests.empty? # Skip creating the PR if it exists already. - $stdout.puts "Skipped! #{pull_requests.length} PRs found for branch #{branch}" + $stdout.puts "Skipped! #{pull_requests.length} PRs found for branch '#{source_branch}'" return end - pr_labels = ModuleSync::Util.parse_list(options[:pr_labels]) pr = @api.create_pull_request(repo_path, target_branch, - branch, - options[:pr_title], - options[:message]) + source_branch, + title, + message) $stdout.puts \ - "Submitted PR '#{options[:pr_title]}' to #{repo_path} " \ - "- merges #{branch} into #{target_branch}" + "Submitted PR '#{title}' to '#{repo_path}' " \ + "- merges #{source_branch} into #{target_branch}" # We only assign labels to the PR if we've discovered a list > 1. The labels MUST # already exist. We DO NOT create missing labels. - return if pr_labels.empty? - $stdout.puts "Attaching the following labels to PR #{pr['number']}: #{pr_labels.join(', ')}" - @api.add_labels_to_an_issue(repo_path, pr['number'], pr_labels) + return if labels.empty? + $stdout.puts "Attaching the following labels to PR #{pr['number']}: #{labels.join(', ')}" + @api.add_labels_to_an_issue(repo_path, pr['number'], labels) end end end diff --git a/lib/modulesync/pr/gitlab.rb b/lib/modulesync/git_service/gitlab.rb similarity index 52% rename from lib/modulesync/pr/gitlab.rb rename to lib/modulesync/git_service/gitlab.rb index 571a6795..353f59cc 100644 --- a/lib/modulesync/pr/gitlab.rb +++ b/lib/modulesync/git_service/gitlab.rb @@ -1,11 +1,12 @@ require 'gitlab' -require 'modulesync/util' +require 'modulesync/git_service' +require 'modulesync/git_service/base' module ModuleSync - module PR + module GitService # GitLab creates and manages merge requests on gitlab.com or private GitLab # installations. - class GitLab + class GitLab < Base def initialize(token, endpoint) @api = Gitlab::Client.new( :endpoint => endpoint, @@ -13,16 +14,15 @@ def initialize(token, endpoint) ) end - def manage(namespace, module_name, options) - repo_path = File.join(namespace, module_name) - branch = options[:remote_branch] || options[:branch] - head = "#{namespace}:#{branch}" - target_branch = options[:pr_target_branch] || 'master' + private - if options[:noop] + def _open_pull_request(repo_path:, namespace:, title:, message:, source_branch:, target_branch:, labels:, noop:) # rubocop:disable Metrics/ParameterLists, Lint/UnusedMethodArgument, Metrics/LineLength + head = "#{namespace}:#{source_branch}" + + if noop $stdout.puts \ - "Using no-op. Would submit MR '#{options[:pr_title]}' to #{repo_path} " \ - "- merges #{branch} into #{target_branch}" + "Using no-op. Would submit MR '#{title}' to '#{repo_path}' " \ + "- merges #{source_branch} into #{target_branch}" return end @@ -32,22 +32,21 @@ def manage(namespace, module_name, options) :target_branch => target_branch) unless merge_requests.empty? # Skip creating the MR if it exists already. - $stdout.puts "Skipped! #{merge_requests.length} MRs found for branch #{branch}" + $stdout.puts "Skipped! #{merge_requests.length} MRs found for branch '#{source_branch}'" return end - mr_labels = ModuleSync::Util.parse_list(options[:pr_labels]) mr = @api.create_merge_request(repo_path, - options[:pr_title], - :source_branch => branch, + title, + :source_branch => source_branch, :target_branch => target_branch, - :labels => mr_labels) + :labels => labels) $stdout.puts \ - "Submitted MR '#{options[:pr_title]}' to #{repo_path} " \ - "- merges #{branch} into #{target_branch}" + "Submitted MR '#{title}' to '#{repo_path}' " \ + "- merges '#{source_branch}' into '#{target_branch}'" - return if mr_labels.empty? - $stdout.puts "Attached the following labels to MR #{mr.iid}: #{mr_labels.join(', ')}" + return if labels.empty? + $stdout.puts "Attached the following labels to MR #{mr.iid}: #{labels.join(', ')}" end end end diff --git a/lib/modulesync/repository.rb b/lib/modulesync/repository.rb index 31fdcf7f..8e3e43e2 100644 --- a/lib/modulesync/repository.rb +++ b/lib/modulesync/repository.rb @@ -157,6 +157,8 @@ def show_changes(options) puts "\n\n" puts '--------------------------------' + + git.diff('HEAD', '--').any? || untracked_unignored_files.any? end end end diff --git a/lib/modulesync/source_code.rb b/lib/modulesync/source_code.rb index 7415d86b..12cfeab7 100644 --- a/lib/modulesync/source_code.rb +++ b/lib/modulesync/source_code.rb @@ -1,4 +1,6 @@ require 'modulesync' +require 'modulesync/git_service' +require 'modulesync/git_service/factory' require 'modulesync/repository' require 'modulesync/util' @@ -47,6 +49,27 @@ def path(*parts) File.join(working_directory, *parts) end + def git_service + @git_service ||= GitService::Factory.instantiate(**git_service_configuration) + end + + def git_service_configuration + @git_service_configuration ||= GitService.configuration_for(sourcecode: self) + end + + def open_pull_request + git_service.open_pull_request( + repo_path: repository_path, + namespace: repository_namespace, + title: ModuleSync.options[:pr_title], + message: ModuleSync.options[:message], + source_branch: ModuleSync.options[:remote_branch] || ModuleSync.options[:branch] || repository.default_branch, + target_branch: ModuleSync.options[:pr_target_branch] || repository.default_branch, + labels: ModuleSync::Util.parse_list(ModuleSync.options[:pr_labels]), + noop: ModuleSync.options[:noop], + ) + end + private def _repository_remote diff --git a/spec/unit/modulesync/git_service/factory_spec.rb b/spec/unit/modulesync/git_service/factory_spec.rb new file mode 100644 index 00000000..9e6234cc --- /dev/null +++ b/spec/unit/modulesync/git_service/factory_spec.rb @@ -0,0 +1,16 @@ +require 'modulesync' +require 'modulesync/git_service/factory' + +describe ModuleSync::GitService::Factory do + context 'when instantiate a GitHub service without credentials' do + it 'raises an error' do + expect { ModuleSync::GitService::Factory.instantiate(type: :github, endpoint: nil, token: nil) }.to raise_error(ModuleSync::GitService::MissingCredentialsError) + end + end + + context 'when instantiate a GitLab service without credentials' do + it 'raises an error' do + expect { ModuleSync::GitService::Factory.instantiate(type: :gitlab, endpoint: nil, token: nil) }.to raise_error(ModuleSync::GitService::MissingCredentialsError) + end + end +end diff --git a/spec/unit/modulesync/git_service/github_spec.rb b/spec/unit/modulesync/git_service/github_spec.rb new file mode 100644 index 00000000..603ece11 --- /dev/null +++ b/spec/unit/modulesync/git_service/github_spec.rb @@ -0,0 +1,81 @@ +require 'spec_helper' + +require 'modulesync/git_service/github' + +describe ModuleSync::GitService::GitHub do + context '::open_pull_request' do + before(:each) do + @client = double() + allow(Octokit::Client).to receive(:new).and_return(@client) + @it = ModuleSync::GitService::GitHub.new('test', 'https://api.github.com') + end + + let(:args) do + { + repo_path: 'test/modulesync', + namespace: 'test', + title: 'Test PR is submitted', + message: 'Hello world', + source_branch: 'test', + target_branch: 'master', + labels: labels, + noop: false, + } + end + + let(:labels) { [] } + + it 'submits PR when --pr is set' do + allow(@client).to receive(:pull_requests) + .with(args[:repo_path], + :state => 'open', + :base => 'master', + :head => "#{args[:namespace]}:#{args[:source_branch]}" + ).and_return([]) + expect(@client).to receive(:create_pull_request) + .with(args[:repo_path], + 'master', + args[:source_branch], + args[:title], + args[:message] + ).and_return({"html_url" => "http://example.com/pulls/22"}) + expect { @it.open_pull_request(**args) }.to output(/Submitted PR/).to_stdout + end + + it 'skips submitting PR if one has already been issued' do + pr = { + "title" => "Test title", + "html_url" => "https://example.com/pulls/44", + "number" => "44" + } + + expect(@client).to receive(:pull_requests) + .with(args[:repo_path], + :state => 'open', + :base => 'master', + :head => "#{args[:namespace]}:#{args[:source_branch]}" + ).and_return([pr]) + expect { @it.open_pull_request(**args) }.to output("Skipped! 1 PRs found for branch 'test'\n").to_stdout + end + + context 'when labels are set' do + let(:labels) { %w{HELLO WORLD} } + + it 'adds labels to PR' do + allow(@client).to receive(:create_pull_request).and_return({"html_url" => "http://example.com/pulls/22", "number" => "44"}) + allow(@client).to receive(:pull_requests) + .with(args[:repo_path], + :state => 'open', + :base => 'master', + :head => "#{args[:namespace]}:#{args[:source_branch]}" + ).and_return([]) + expect(@client).to receive(:add_labels_to_an_issue) + .with(args[:repo_path], + "44", + ["HELLO", "WORLD"] + ) + expect { @it.open_pull_request(**args) }.to output(/Attaching the following labels to PR 44: HELLO, WORLD/).to_stdout + end + end + end +end diff --git a/spec/unit/modulesync/git_service/gitlab_spec.rb b/spec/unit/modulesync/git_service/gitlab_spec.rb new file mode 100644 index 00000000..d50181c6 --- /dev/null +++ b/spec/unit/modulesync/git_service/gitlab_spec.rb @@ -0,0 +1,90 @@ +require 'spec_helper' + +require 'modulesync/git_service/gitlab' + +describe ModuleSync::GitService::GitLab do + context '::open_pull_request' do + before(:each) do + @client = double() + allow(Gitlab::Client).to receive(:new).and_return(@client) + @it = ModuleSync::GitService::GitLab.new('test', 'https://gitlab.com/api/v4') + end + + let(:args) do + { + repo_path: 'test/modulesync', + namespace: 'test', + title: 'Test MR is submitted', + message: 'Hello world', + source_branch: 'test', + target_branch: 'master', + labels: labels, + noop: false, + } + end + + let(:labels) { [] } + + it 'submits MR when --pr is set' do + allow(@client).to receive(:merge_requests) + .with(args[:repo_path], + :state => 'opened', + :source_branch => "#{args[:namespace]}:#{args[:source_branch]}", + :target_branch => 'master', + ).and_return([]) + + expect(@client).to receive(:create_merge_request) + .with(args[:repo_path], + args[:title], + :labels => [], + :source_branch => args[:source_branch], + :target_branch => 'master', + ).and_return({"html_url" => "http://example.com/pulls/22"}) + + expect { @it.open_pull_request(**args) }.to output(/Submitted MR/).to_stdout + end + + it 'skips submitting MR if one has already been issued' do + mr = { + "title" => "Test title", + "html_url" => "https://example.com/pulls/44", + "iid" => "44" + } + + expect(@client).to receive(:merge_requests) + .with(args[:repo_path], + :state => 'opened', + :source_branch => "#{args[:namespace]}:#{args[:source_branch]}", + :target_branch => 'master', + ).and_return([mr]) + + expect { @it.open_pull_request(**args) }.to output("Skipped! 1 MRs found for branch 'test'\n").to_stdout + end + + context 'when labels are set' do + let(:labels) { %w{HELLO WORLD} } + + it 'adds labels to MR' do + mr = double() + allow(mr).to receive(:iid).and_return("42") + + expect(@client).to receive(:create_merge_request) + .with(args[:repo_path], + args[:title], + :labels => ["HELLO", "WORLD"], + :source_branch => args[:source_branch], + :target_branch => 'master', + ).and_return(mr) + + allow(@client).to receive(:merge_requests) + .with(args[:repo_path], + :state => 'opened', + :source_branch => "#{args[:namespace]}:#{args[:source_branch]}", + :target_branch => 'master', + ).and_return([]) + + expect { @it.open_pull_request(**args) }.to output(/Attached the following labels to MR 42: HELLO, WORLD/).to_stdout + end + end + end +end diff --git a/spec/unit/modulesync/git_service_spec.rb b/spec/unit/modulesync/git_service_spec.rb new file mode 100644 index 00000000..e8016c23 --- /dev/null +++ b/spec/unit/modulesync/git_service_spec.rb @@ -0,0 +1,201 @@ +require 'modulesync' +require 'modulesync/git_service' + +describe ModuleSync::GitService do + before do + options = ModuleSync.config_defaults.merge({ + git_base: 'file:///tmp/dummy', + }) + ModuleSync.instance_variable_set '@options', options + end + + context 'when guessing the git service configuration' do + before do + allow(ENV).to receive(:[]) + .and_return(nil) + end + + let(:sourcecode) do + ModuleSync::SourceCode.new 'puppet-test', sourcecode_options + end + + context 'when using a complete git service configuration entry' do + let(:sourcecode_options) do + { + gitlab: { + base_url: 'https://vcs.example.com/api/v4', + token: 'secret', + }, + } + end + + it 'build git service arguments from configuration entry' do + expect(ModuleSync::GitService.configuration_for(sourcecode: sourcecode)).to eq({ + type: :gitlab, + endpoint: 'https://vcs.example.com/api/v4', + token: 'secret', + }) + end + end + + context 'when using a simple git service key entry' do + let(:sourcecode_options) do + { + gitlab: {}, + remote: 'git@git.example.com:namespace/puppet-test', + } + end + + context 'with GITLAB_BASE_URL and GITLAB_TOKEN environment variables sets' do + it 'build git service arguments from environment variables' do + allow(ENV).to receive(:[]) + .with('GITLAB_BASE_URL') + .and_return('https://vcs.example.com/api/v4') + allow(ENV).to receive(:[]) + .with('GITLAB_TOKEN') + .and_return('secret') + + expect(ModuleSync::GitService.configuration_for(sourcecode: sourcecode)).to eq({ + type: :gitlab, + endpoint: 'https://vcs.example.com/api/v4', + token: 'secret', + }) + end + end + + context 'with only GITLAB_TOKEN environment variable sets' do + it 'guesses the endpoint based on repository remote' do + allow(ENV).to receive(:[]) + .with('GITLAB_TOKEN') + .and_return('secret') + + expect(ModuleSync::GitService.configuration_for(sourcecode: sourcecode)).to eq({ + type: :gitlab, + endpoint: 'https://git.example.com/api/v4', + token: 'secret', + }) + end + end + + context 'without any environment variable sets' do + it 'returns a nil token' do + expect(ModuleSync::GitService.configuration_for(sourcecode: sourcecode)).to eq({ + type: :gitlab, + endpoint: 'https://git.example.com/api/v4', + token: nil, + }) + end + end + end + + context 'without git service configuration entry' do + context 'with a guessable endpoint based on repository remote' do + let(:sourcecode_options) do + { + remote: 'git@gitlab.example.com:namespace/puppet-test', + } + end + + context 'with a GITLAB_TOKEN environment variable sets' do + it 'guesses git service configuration' do + allow(ENV).to receive(:[]) + .with('GITLAB_TOKEN') + .and_return('secret') + + expect(ModuleSync::GitService.configuration_for(sourcecode: sourcecode)).to eq({ + type: :gitlab, + endpoint: 'https://gitlab.example.com/api/v4', + token: 'secret', + }) + end + end + + context 'without a GITLAB_TOKEN environment variable sets' do + it 'returns a nil token' do + expect(ModuleSync::GitService.configuration_for(sourcecode: sourcecode)).to eq({ + type: :gitlab, + endpoint: 'https://gitlab.example.com/api/v4', + token: nil, + }) + end + end + end + + context 'with a unguessable endpoint' do + let(:sourcecode_options) do + { + remote: 'git@vcs.example.com:namespace/puppet-test', + } + end + + context 'with GITHUB_TOKEN environments variable sets' do + it 'guesses git service configuration' do + allow(ENV).to receive(:[]) + .with('GITHUB_TOKEN') + .and_return('secret') + + expect(ModuleSync::GitService.configuration_for(sourcecode: sourcecode)).to eq({ + type: :github, + endpoint: 'https://vcs.example.com', + token: 'secret', + }) + end + end + + context 'with GITLAB_TOKEN and GITHUB_TOKEN environments variables sets' do + it 'raises an error' do + allow(ENV).to receive(:[]) + .with('GITHUB_TOKEN') + .and_return('secret') + + allow(ENV).to receive(:[]) + .with('GITLAB_TOKEN') + .and_return('secret') + + expect{ModuleSync::GitService.configuration_for(sourcecode: sourcecode)} + .to raise_error ModuleSync::Error + end + end + end + end + end + + RSpec.shared_examples 'hostname_extractor' do |url, hostname| + context "with '#{url}' URL" do + subject { ModuleSync::GitService.extract_hostname(url) } + it "should extract '#{hostname}' as hostname" do + expect(subject).to eq(hostname) + end + end + end + + context '#extract_hostname' do + [ + %w[ssh://user@host.xz:4444/path/to/repo.git/ host.xz], + %w[ssh://user@host.xz:/path/to/repo.git/ host.xz], + %w[ssh://host.xz/path/to/repo.git/ host.xz], + + %w[git://host.xz/path/to/repo.git/ host.xz], + %w[git://host.xz/path/to/repo/ host.xz], + %w[git://host.xz/path/to/repo host.xz], + + %w[user@host.xz:path/to/repo.git/ host.xz], + %w[user@host.xz:path/to/repo.git host.xz], + %w[user@host.xz:path/to/repo host.xz], + %w[host.xz:path/to/repo.git/ host.xz], + + %w[https://host.xz:8443/path/to/repo.git/ host.xz], + %w[https://host.xz/path/to/repo.git/ host.xz], + + %w[ftp://host.xz/path/to/repo/ host.xz], + + ['/path/to/repo.git/', nil], + + ['file:///path/to/repo.git/', nil], + + ['something-invalid', nil], + ].each do |url, hostname| + it_should_behave_like 'hostname_extractor', url, hostname + end + end +end diff --git a/spec/unit/modulesync/pr/github_spec.rb b/spec/unit/modulesync/pr/github_spec.rb deleted file mode 100644 index 830c4b05..00000000 --- a/spec/unit/modulesync/pr/github_spec.rb +++ /dev/null @@ -1,49 +0,0 @@ -require 'spec_helper' -require 'modulesync/pr/github' - -describe ModuleSync::PR::GitHub do - context '::manage' do - before(:each) do - @git_repo = 'test/modulesync' - @namespace, @repo_name = @git_repo.split('/') - @options = { - :pr => true, - :pr_title => 'Test PR is submitted', - :branch => 'test', - :message => 'Hello world', - :pr_auto_merge => false, - } - - @client = double() - allow(Octokit::Client).to receive(:new).and_return(@client) - @it = ModuleSync::PR::GitHub.new('test', 'https://api.github.com') - end - - it 'submits PR when --pr is set' do - allow(@client).to receive(:pull_requests).with(@git_repo, :state => 'open', :base => 'master', :head => "#{@namespace}:#{@options[:branch]}").and_return([]) - expect(@client).to receive(:create_pull_request).with(@git_repo, 'master', @options[:branch], @options[:pr_title], @options[:message]).and_return({"html_url" => "http://example.com/pulls/22"}) - expect { @it.manage(@namespace, @repo_name, @options) }.to output(/Submitted PR/).to_stdout - end - - it 'skips submitting PR if one has already been issued' do - pr = { - "title" => "Test title", - "html_url" => "https://example.com/pulls/44", - "number" => "44" - } - - expect(@client).to receive(:pull_requests).with(@git_repo, :state => 'open', :base => 'master', :head => "#{@namespace}:#{@options[:branch]}").and_return([pr]) - expect { @it.manage(@namespace, @repo_name, @options) }.to output(/Skipped! 1 PRs found for branch test/).to_stdout - end - - it 'adds labels to PR when --pr-labels is set' do - @options[:pr_labels] = "HELLO,WORLD" - - allow(@client).to receive(:create_pull_request).and_return({"html_url" => "http://example.com/pulls/22", "number" => "44"}) - allow(@client).to receive(:pull_requests).with(@git_repo, :state => 'open', :base => 'master', :head => "#{@namespace}:#{@options[:branch]}").and_return([]) - - expect(@client).to receive(:add_labels_to_an_issue).with(@git_repo, "44", ["HELLO", "WORLD"]) - expect { @it.manage(@namespace, @repo_name, @options) }.to output(/Attaching the following labels to PR 44: HELLO, WORLD/).to_stdout - end - end -end diff --git a/spec/unit/modulesync/pr/gitlab_spec.rb b/spec/unit/modulesync/pr/gitlab_spec.rb deleted file mode 100644 index 360a773b..00000000 --- a/spec/unit/modulesync/pr/gitlab_spec.rb +++ /dev/null @@ -1,81 +0,0 @@ -require 'spec_helper' -require 'modulesync/pr/gitlab' - -describe ModuleSync::PR::GitLab do - context '::manage' do - before(:each) do - @git_repo = 'test/modulesync' - @namespace, @repo_name = @git_repo.split('/') - @options = { - :pr => true, - :pr_title => 'Test PR is submitted', - :branch => 'test', - :message => 'Hello world', - :pr_auto_merge => false, - } - - @client = double() - allow(Gitlab::Client).to receive(:new).and_return(@client) - @it = ModuleSync::PR::GitLab.new('test', 'https://gitlab.com/api/v4') - end - - it 'submits MR when --pr is set' do - allow(@client).to receive(:merge_requests) - .with(@git_repo, - :state => 'opened', - :source_branch => "#{@namespace}:#{@options[:branch]}", - :target_branch => 'master', - ).and_return([]) - - expect(@client).to receive(:create_merge_request) - .with(@git_repo, - @options[:pr_title], - :labels => [], - :source_branch => @options[:branch], - :target_branch => 'master', - ).and_return({"html_url" => "http://example.com/pulls/22"}) - - expect { @it.manage(@namespace, @repo_name, @options) }.to output(/Submitted MR/).to_stdout - end - - it 'skips submitting MR if one has already been issued' do - mr = { - "title" => "Test title", - "html_url" => "https://example.com/pulls/44", - "iid" => "44" - } - - expect(@client).to receive(:merge_requests) - .with(@git_repo, - :state => 'opened', - :source_branch => "#{@namespace}:#{@options[:branch]}", - :target_branch => 'master', - ).and_return([mr]) - - expect { @it.manage(@namespace, @repo_name, @options) }.to output(/Skipped! 1 MRs found for branch test/).to_stdout - end - - it 'adds labels to MR when --pr-labels is set' do - @options[:pr_labels] = "HELLO,WORLD" - mr = double() - allow(mr).to receive(:iid).and_return("42") - - expect(@client).to receive(:create_merge_request) - .with(@git_repo, - @options[:pr_title], - :labels => ["HELLO", "WORLD"], - :source_branch => @options[:branch], - :target_branch => 'master', - ).and_return(mr) - - allow(@client).to receive(:merge_requests) - .with(@git_repo, - :state => 'opened', - :source_branch => "#{@namespace}:#{@options[:branch]}", - :target_branch => 'master', - ).and_return([]) - - expect { @it.manage(@namespace, @repo_name, @options) }.to output(/Attached the following labels to MR 42: HELLO, WORLD/).to_stdout - end - end -end diff --git a/spec/unit/modulesync/source_code_spec.rb b/spec/unit/modulesync/source_code_spec.rb new file mode 100644 index 00000000..8317aeb9 --- /dev/null +++ b/spec/unit/modulesync/source_code_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe ModuleSync::SourceCode do + before do + options = ModuleSync.config_defaults.merge({ + git_base: 'file:///tmp/dummy', + }) + ModuleSync.instance_variable_set '@options', options + end + + subject do + ModuleSync::SourceCode.new('namespace/name', nil) + end + + it 'has a repository namespace sets to "namespace"' do + expect(subject.repository_namespace).to eq 'namespace' + end + + it 'has a repository name sets to "name"' do + expect(subject.repository_name).to eq 'name' + end +end diff --git a/spec/unit/modulesync_spec.rb b/spec/unit/modulesync_spec.rb index 42b82168..34a559e5 100644 --- a/spec/unit/modulesync_spec.rb +++ b/spec/unit/modulesync_spec.rb @@ -11,16 +11,4 @@ ModuleSync.update(options) end end - - context '::pr' do - describe "Raise Error" do - let(:puppet_module) do - ModuleSync::PuppetModule.new 'puppet-test', remote: 'dummy' - end - - it 'raises an error when neither GITHUB_TOKEN nor GITLAB_TOKEN are set for PRs' do - expect { ModuleSync.pr(puppet_module) }.to raise_error(RuntimeError).and output(/No GitHub or GitLab token specified for --pr/).to_stderr - end - end - end end