From a626c212f9e64434b828dadcb0cb3bd879beb612 Mon Sep 17 00:00:00 2001 From: Danilo Sousa Date: Tue, 16 Feb 2016 18:56:30 -0200 Subject: fix `githubrepo` hook for groups repositories remotes when there was a repository per group we could not use the hook for pushing since we only had **one** remote configured. This PR configures each repository its own remote based in the `groups` config, as follows: ``` --- groups: first: remote_repo: git@gitlab.lab:oxidized/first.git second: remote_repo: git@gitlab.lab:oxidized/second.git ``` --- lib/oxidized/hook/githubrepo.rb | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'lib/oxidized/hook') diff --git a/lib/oxidized/hook/githubrepo.rb b/lib/oxidized/hook/githubrepo.rb index d10b51e..80ae665 100644 --- a/lib/oxidized/hook/githubrepo.rb +++ b/lib/oxidized/hook/githubrepo.rb @@ -4,9 +4,9 @@ class GithubRepo < Oxidized::Hook end def run_hook(ctx) - repo = Rugged::Repository.new(Oxidized.config.output.git.repo) + repo = Rugged::Repository.new(ctx.node.repo) log "Pushing local repository(#{repo.path})..." - remote = repo.remotes['origin'] || repo.remotes.create('origin', cfg.remote_repo) + remote = repo.remotes['origin'] || repo.remotes.create('origin', remote_repo(ctx.node)) log "to remote: #{remote.url}" fetch_and_merge_remote(repo) @@ -54,4 +54,16 @@ class GithubRepo < Oxidized::Hook end end + def remote_repo(node) + if node.group.nil? || single_repo? + cfg.remote_repo + else + Oxidized.config.groups[node.group].remote_repo + end + end + + def single_repo? + Oxidized.config.git.single_repo? + end + end -- cgit v1.2.3 From 695883a75783e19de70e60314fab49661c387a22 Mon Sep 17 00:00:00 2001 From: Danilo Sousa Date: Thu, 18 Feb 2016 17:01:25 -0200 Subject: create tests for `GithubRepo#validate_cfg!` method --- lib/oxidized/hook/githubrepo.rb | 2 +- spec/githubrepo_spec.rb | 14 +++++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) (limited to 'lib/oxidized/hook') diff --git a/lib/oxidized/hook/githubrepo.rb b/lib/oxidized/hook/githubrepo.rb index 80ae665..0c98460 100644 --- a/lib/oxidized/hook/githubrepo.rb +++ b/lib/oxidized/hook/githubrepo.rb @@ -1,6 +1,6 @@ class GithubRepo < Oxidized::Hook def validate_cfg! - cfg.has_key?('remote_repo') or raise KeyError, 'remote_repo is required' + raise KeyError, 'hook.remote_repo is required' unless cfg.has_key?('remote_repo') end def run_hook(ctx) diff --git a/spec/githubrepo_spec.rb b/spec/githubrepo_spec.rb index 36207d3..4674035 100644 --- a/spec/githubrepo_spec.rb +++ b/spec/githubrepo_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' require 'rugged' require 'oxidized/hook/githubrepo' -describe Oxidized::GithubRepo do +describe GithubRepo do let(:credentials) { mock() } let(:remote) { mock() } let(:remotes) { mock() } @@ -17,6 +17,18 @@ describe Oxidized::GithubRepo do Oxidized.setup_logger end + describe '#validate_cfg!' do + before do + gr.expects(:respond_to?).with(:validate_cfg!).returns(false) # `cfg=` call + end + + it 'raise a error when `remote_repo` is not configured' do + Oxidized.config.hooks.github_repo_hook = { type: 'githubrepo' } + gr.cfg = Oxidized.config.hooks.github_repo_hook + proc { gr.validate_cfg! }.must_raise(KeyError) + end + end + describe "#fetch_and_merge_remote" do before(:each) do Oxidized.config.hooks.github_repo_hook.remote_repo = 'git@github.com:username/foo.git' -- cgit v1.2.3 From 66a0c3261611f737e5f36527da4ba0d13d70e092 Mon Sep 17 00:00:00 2001 From: Danilo Sousa Date: Fri, 19 Feb 2016 12:08:25 -0200 Subject: fix 'GithubRepo#single_repo?' private method --- lib/oxidized/hook/githubrepo.rb | 2 +- spec/githubrepo_spec.rb | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) (limited to 'lib/oxidized/hook') diff --git a/lib/oxidized/hook/githubrepo.rb b/lib/oxidized/hook/githubrepo.rb index 0c98460..dd9520b 100644 --- a/lib/oxidized/hook/githubrepo.rb +++ b/lib/oxidized/hook/githubrepo.rb @@ -63,7 +63,7 @@ class GithubRepo < Oxidized::Hook end def single_repo? - Oxidized.config.git.single_repo? + Oxidized.config.output.git.single_repo? end end diff --git a/spec/githubrepo_spec.rb b/spec/githubrepo_spec.rb index 4674035..a5bbeca 100644 --- a/spec/githubrepo_spec.rb +++ b/spec/githubrepo_spec.rb @@ -132,12 +132,13 @@ describe GithubRepo do repo.expects(:remotes).twice.returns(remotes) remotes.expects(:[]).with('origin').returns(nil) - remotes.expects(:create).with('origin', 'ggrroouupp#remote_repo').returns(remote) + remotes.expects(:create).with('origin', create_remote).returns(remote) remote.expects(:url).returns('url') remote.expects(:push).with(['refs/heads/master'], credentials: credentials).returns(true) end describe 'when there are several repositories' do + let(:create_remote) { 'ggrroouupp#remote_repo' } let(:repository) { './ggrroouupp.git' } let(:single_repo) { nil } @@ -148,6 +149,7 @@ describe GithubRepo do end describe 'when is a single repository' do + let(:create_remote) { 'github_repo_hook#remote_repo' } let(:repository) { 'foo.git' } let(:single_repo) { true } -- cgit v1.2.3 From a486e086fc3b52f26ade94e58a16fa2150fc9cae Mon Sep 17 00:00:00 2001 From: Danilo Sousa Date: Fri, 19 Feb 2016 18:58:47 -0200 Subject: move the groups remote to the hook config thanks to @ElvinEfendi for the idea! :+1: --- README.md | 15 ++++++++------- lib/oxidized/hook/githubrepo.rb | 2 +- spec/githubrepo_spec.rb | 14 ++++++++++---- 3 files changed, 19 insertions(+), 12 deletions(-) (limited to 'lib/oxidized/hook') diff --git a/README.md b/README.md index 21a0956..3c1a41e 100644 --- a/README.md +++ b/README.md @@ -535,14 +535,15 @@ This hook configures the repository `remote` and _push_ the code when the specif * `username`: username for repository auth. * `password`: password for repository auth. -When using groups repositories, the remotes should be passed in the `groups` config for each group. +When using groups repositories, each group must have its own `remote` in the `remote_repo` config. ``` yaml -vars: {} -groups: - routers: git@git.intranet:oxidized/routers.git - switches: git@git.intranet:oxidized/switches.git - firewalls: git@git.intranet:oxidized/firewalls.git +hooks: + push_to_remote: + remote_repo: + routers: git@git.intranet:oxidized/routers.git + switches: git@git.intranet:oxidized/switches.git + firewalls: git@git.intranet:oxidized/firewalls.git ``` @@ -550,7 +551,7 @@ groups: ``` yaml hooks: - push_to_gitlab: + push_to_remote: type: githubrepo events: [node_success, post_store] remote_repo: git@git.intranet:oxidized/test.git diff --git a/lib/oxidized/hook/githubrepo.rb b/lib/oxidized/hook/githubrepo.rb index dd9520b..0c75c68 100644 --- a/lib/oxidized/hook/githubrepo.rb +++ b/lib/oxidized/hook/githubrepo.rb @@ -58,7 +58,7 @@ class GithubRepo < Oxidized::Hook if node.group.nil? || single_repo? cfg.remote_repo else - Oxidized.config.groups[node.group].remote_repo + cfg.remote_repo[node.group] end end diff --git a/spec/githubrepo_spec.rb b/spec/githubrepo_spec.rb index a5bbeca..8d85761 100644 --- a/spec/githubrepo_spec.rb +++ b/spec/githubrepo_spec.rb @@ -126,8 +126,6 @@ describe GithubRepo do before do Rugged::Credentials::SshKeyFromAgent.expects(:new).with(username: 'git').returns(credentials) Rugged::Repository.expects(:new).with(repository).returns(repo) - Oxidized.config.groups.ggrroouupp.remote_repo = 'ggrroouupp#remote_repo' - Oxidized.config.hooks.github_repo_hook.remote_repo = 'github_repo_hook#remote_repo' Oxidized.config.output.git.single_repo = single_repo repo.expects(:remotes).twice.returns(remotes) @@ -137,22 +135,30 @@ describe GithubRepo do remote.expects(:push).with(['refs/heads/master'], credentials: credentials).returns(true) end - describe 'when there are several repositories' do + describe 'and there are several repositories' do let(:create_remote) { 'ggrroouupp#remote_repo' } let(:repository) { './ggrroouupp.git' } let(:single_repo) { nil } + before do + Oxidized.config.hooks.github_repo_hook.remote_repo.ggrroouupp = 'ggrroouupp#remote_repo' + end + it 'will push to the node group repository' do gr.cfg = Oxidized.config.hooks.github_repo_hook gr.run_hook(ctx).must_equal true end end - describe 'when is a single repository' do + describe 'and has a single repository' do let(:create_remote) { 'github_repo_hook#remote_repo' } let(:repository) { 'foo.git' } let(:single_repo) { true } + before do + Oxidized.config.hooks.github_repo_hook.remote_repo = 'github_repo_hook#remote_repo' + end + it 'will push to the correct repository' do gr.cfg = Oxidized.config.hooks.github_repo_hook gr.run_hook(ctx).must_equal true -- cgit v1.2.3 From c098f57f5dfc4588673ac62083611736c8a75136 Mon Sep 17 00:00:00 2001 From: Danilo Sousa Date: Fri, 19 Feb 2016 22:17:37 -0200 Subject: refactor `#remote_repo` to not rely on `#single_repo?` config even in the case we have groups, we can assume is only one repository if the `remote_repo` config is a String. --- lib/oxidized/hook/githubrepo.rb | 7 +------ spec/githubrepo_spec.rb | 4 +--- 2 files changed, 2 insertions(+), 9 deletions(-) (limited to 'lib/oxidized/hook') diff --git a/lib/oxidized/hook/githubrepo.rb b/lib/oxidized/hook/githubrepo.rb index 0c75c68..7fc6677 100644 --- a/lib/oxidized/hook/githubrepo.rb +++ b/lib/oxidized/hook/githubrepo.rb @@ -55,15 +55,10 @@ class GithubRepo < Oxidized::Hook end def remote_repo(node) - if node.group.nil? || single_repo? + if node.group.nil? || cfg.remote_repo.is_a?(String) cfg.remote_repo else cfg.remote_repo[node.group] end end - - def single_repo? - Oxidized.config.output.git.single_repo? - end - end diff --git a/spec/githubrepo_spec.rb b/spec/githubrepo_spec.rb index 8d85761..b6a8d9e 100644 --- a/spec/githubrepo_spec.rb +++ b/spec/githubrepo_spec.rb @@ -126,7 +126,6 @@ describe GithubRepo do before do Rugged::Credentials::SshKeyFromAgent.expects(:new).with(username: 'git').returns(credentials) Rugged::Repository.expects(:new).with(repository).returns(repo) - Oxidized.config.output.git.single_repo = single_repo repo.expects(:remotes).twice.returns(remotes) remotes.expects(:[]).with('origin').returns(nil) @@ -138,7 +137,6 @@ describe GithubRepo do describe 'and there are several repositories' do let(:create_remote) { 'ggrroouupp#remote_repo' } let(:repository) { './ggrroouupp.git' } - let(:single_repo) { nil } before do Oxidized.config.hooks.github_repo_hook.remote_repo.ggrroouupp = 'ggrroouupp#remote_repo' @@ -153,10 +151,10 @@ describe GithubRepo do describe 'and has a single repository' do let(:create_remote) { 'github_repo_hook#remote_repo' } let(:repository) { 'foo.git' } - let(:single_repo) { true } before do Oxidized.config.hooks.github_repo_hook.remote_repo = 'github_repo_hook#remote_repo' + Oxidized.config.output.git.single_repo = true end it 'will push to the correct repository' do -- cgit v1.2.3